RE: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips

From: "Fuchs, Andreas" <andreas.fuchs@sit.fraunhofer.de>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: "tpmdd-devel@lists.sourceforge.net" 
	<tpmdd-devel@lists.sourceforge.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	David Howells <dhowells@redhat.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"open list:KEYS-TRUSTED" <linux-security-module@vger.kernel.org>,
	"open list:KEYS-TRUSTED" <keyrings@vger.kernel.org>,
	James Morris <james.l.morris@oracle.com>,
	"David Safford" <safford@us.ibm.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"josh@joshtripplet.org" <josh@joshtripplet.org>,
	"richard.l.maliszewski@intel.com"
	<richard.l.maliszewski@intel.com>,
	"monty.wiseman@intel.com" <monty.wiseman@intel.com>,
	"will.c.arthur@intel.com" <will.c.arthur@intel.com>,
	"artem.bityutskiy@linux.intel.com"
	<artem.bityutskiy@linux.intel.com>
Subject: RE: [tpmdd-devel] [PATCH 4/4] keys,	trusted: seal/unseal with TPM 2.0 chips
Date: Wed, 7 Oct 2015 10:32:41 +0000	[thread overview]
Message-ID: <9F48E1A823B03B4790B7E6E69430724D9D7B1EEE@EXCH2010A.sit.fraunhofer.de> (raw)
In-Reply-To: <20151007102537.GA7261@intel.com>

> > > > > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()-
> > > > > > and TPM2_Load()-failures ?
> > > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY
> > > > > > in those cases. Would you agree ?
> > > > > > (P.S. I can cross-post there if that's prefered ?)
> > > > >
> > > > > Have to check the return values. I posted this patch set already in
> > > > > early July. You are the first reviewer in three months for this patch
> > > > > set.
> > > > >
> > > > > I think the reason was that for TPM 1.x returned -EPERM in all error
> > > > > scenarios and I didn't want to endanger behaviour of command-line tools
> > > > > such as 'keyctl'. I would keep it that way unless you can guarantee that
> > > > > command-line tools will continue work correctly if I change it to
> > > > > -EBUSY.
> > > > >
> > > > > Anyway, I will recheck this part of the patch set but likely are not
> > > > > going to do any changes because I don't want to break the user space.
> > > > >
> > > > > I will consider revising the patch set with keyhandle required as an
> > > > > explicit option.
> > > >
> > > > Hmm... Will the old keyctl work without modification with the 2.0 patches
> > > > anyways ?
> > >
> > > Yes it does and it should. I've been using keyctl utility to test my
> > > patch set.
> > >
> > > > The different keyHandle values and missing default keyHandle will yield
> > > > "differences" anyways, I'd say.
> > > > IMHO, we should get it as correct as possible given that TPM 2.0 is still
> > > > very young.
> > > >
> > > > Is adding "additional" ReturnCodes considered ABI-incompatible breaking
> > > > anyways ?
> > >
> > > Yes they are if they make the user space utiltiy malfunction.
> >
> > AFAICT, keyctl just perror()s. Which is what I would have hoped.
> > So it guess it should work with -EBUSY.
> > Example-Trace of calls for key_adding:
> > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyutils.c#n43
> > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n379
> > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n131
> >
> > Wish I could test it myself.
> > I understand, if you don't want to test my thoughts on this.
> > I just cannot perform the tests myself right now... :-(
> 
> I would submit this change as a separate patch later anyway and not
> include it into this patch set. If it doesn't do harm it can be added
> later on. This patch set has been now in queue for three months so I
> only make modifications that are absolutely necessary.
> 
> Changing keyhandle as mandatory option seems like such changes. This
> doesn't.

Fine with me.

P.S. do you have a git repo with all your queued and future patches at HEAD ?

Cheers,
Andreas