Re: [PATCH 3/4] tpm: seal/unseal for TPM 2.0

From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
	peterhuewe@gmx.de, gregkh@linuxfoundation.org,
	akpm@linux-foundation.org, mjg59@srcf.ucam.org,
	Marcel Selhorst <tpmdd@selhorst.net>,
	David Safford <safford@us.ibm.com>,
	Mimi Zohar <zohar@linux.vnet.ibm.com>,
	David Howells <dhowells@redhat.com>,
	"open list:KEYS-TRUSTED" <linux-security-module@vger.kernel.org>,
	"open list:KEYS-TRUSTED" <keyrings@vger.kernel.org>
Subject: Re: [PATCH 3/4] tpm: seal/unseal for TPM 2.0
Date: Tue, 13 Oct 2015 22:49:15 +0300	[thread overview]
Message-ID: <20151013194915.GB3669@intel.com> (raw)
In-Reply-To: <20151013173442.GB22160@obsidianresearch.com>

On Tue, Oct 13, 2015 at 11:34:42AM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 02, 2015 at 11:38:17AM +0300, Jarkko Sakkinen wrote:
> > Added tpm_trusted_seal() and tpm_trusted_unseal() API for sealing
> > trusted keys.
> > 
> > This patch implements basic sealing and unsealing functionality for
> > TPM 2.0:
> 
> We really need to stop using chip id's as a handle - the caller should
> be using a pointer, it is just a horrible API, and the TPM_ANY_NUM
> business is awful too.. TPM's are stateful devices!

Eventually this needs to be refactored out. I don't see it in the scope
of these patches or as high priority ATM.

> Is it feasible to introduce new APIs with a saner scheme?
> 
> The api layering also seems really weird to me. At a minimum the
> tpm_seal_trusted should be called within key_seal, but really, should
> key_seal be migrated into the TPM core? I'm not sure it makes alot of
> sense to have a tpm_seal_trusted which uses the high level key structs
> when other tpm functions are all low level RPC wrappers...

I think tpm_seal() inside trusted.c is not a very good API. It takes the
ad hoc version of the structs given to key_seal from stack. I don't see
a problem here.

My viewpoint has been that key_seal/unseal in trusted.c should be
refactored out and TPM1 implementations seal/unseal should be moved to
the TPM subsystem. There's so little amount of in-kernel low-level TPM
code that IMHO it makes sense to keep in one place (as are all the other
TPM utility functions).

I can work on the TPM1 migration when we have the basic TPM2 stuff in
place.

> Jason

/Jakrkko