Re: [PATCH] selftest/trustedkeys: TPM 1.2 trusted keys test

From: Mimi Zohar <zohar@linux.ibm.com>
To: Jerry Snitselaar <jsnitsel@redhat.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	linux-integrity@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>,
	Petr Vorel <pvorel@suse.cz>
Subject: Re: [PATCH] selftest/trustedkeys: TPM 1.2 trusted keys test
Date: Mon, 16 Sep 2019 07:52:26 -0400	[thread overview]
Message-ID: <1568634746.4975.32.camel@linux.ibm.com> (raw)
In-Reply-To: <20190916074225.fouluhpylge6fmlj@cantor>

On Mon, 2019-09-16 at 00:42 -0700, Jerry Snitselaar wrote:
> On Sun Sep 15 19, Mimi Zohar wrote:
> >On Sun, 2019-09-15 at 16:52 -0400, Mimi Zohar wrote:
> >> On Fri, 2019-09-13 at 15:08 +0100, Jarkko Sakkinen wrote:
> >> > On Wed, Sep 11, 2019 at 08:00:40AM -0400, Mimi Zohar wrote:
> >> > > On Tue, 2019-09-10 at 19:24 -0400, Mimi Zohar wrote:
> >> > > > On Tue, 2019-09-10 at 19:18 -0400, Mimi Zohar wrote:
> >> > > > > Create, save and load trusted keys test
> >> > > >
> >> > > > Creating trusted keys is failing with the following messages.  Any idea why?
> >> > > >
> >> > > > [  147.014653] tpm tpm0: A TPM error (34) occurred attempting to a send a command
> >> > > > [  147.014678] trusted_key: srkseal failed (-1)
> >> > > > [  147.014687] trusted_key: key_seal failed (-1)
> >> > >
> >> > > This is a regression, that needs to be resolved.  The test works on
> >> > > kernels prior to 5.1.
> >> >
> >> > It breaks on 5.2?
> >>
> >> No, the regression is in 5.1.
> >>
> >> >
> >> > Can you bisect the failing commit?
> >>
> >> git bisect start -- drivers/char/tpm/
> >> git bisect bad
> >> git bisect good v5.0 
> >>
> >> # first bad commit: [412eb585587a1dc43c9622db79de9663b6c4c238] tpm:
> >> use tpm_buf in tpm_transmit_cmd() as the IO parameter
> >
> >In tpm_send(), setting buf.data directly to cmd, instead of calling
> >tpm_buf_init() fixes the problem.
> >
> >Mimi
> >
> >
> 
> The problem is that the command buffer is copied into the tpm_buf with
> the memcpy, but after the tpm_transmit_cmd, nothing gets copied back
> to be used by the code path that called tpm_send. There is code that
> looks at that buffer after trusted_tpm_send returns. Both
> security/keys/trusted.c and crypto/asymmetric_keys/asym_tpm.c run into
> this.
> 
> While playing around with it, adding a memcpy() after the transmit
> call worked for me as well as setting buf.data directly instead of the
> tpm_buf_init/memcpy/tpm_buf_destroy calls.

Thanks!  I really appreciate your looking at the problem.  I haven't
yet tried the fix with a TPM 2.0.
> 
> I'm wondering if it would be worthwhile to convert the
> security/keys/trusted.c and crypto/asymmertic_keys/aym_tpm.c code to
> use the same tpm_buf and tpm_buf manipulation code as gets used in
> drivers/char/tpm.

For now, let's keep the regression fix, that is backported, simple and
straight forward.

Trusted keys is evolving to be more than just TPM based keys.  I'm
sure there will be more changes coming.  At least, now, there is a TPM
1.2 regression test.

thanks,

Mimi