selftest/trustedkeys: TPM 1.2 trusted keys test
diff mbox series

Message ID 1568157511-5464-1-git-send-email-zohar@linux.ibm.com
State New
Headers show
Series
  • selftest/trustedkeys: TPM 1.2 trusted keys test
Related show

Commit Message

Mimi Zohar Sept. 10, 2019, 11:18 p.m. UTC
Create, save and load trusted keys test

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 tools/testing/selftests/tpm2/Makefile            |  2 +-
 tools/testing/selftests/tpm2/test_trustedkeys.sh | 99 ++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/tpm2/test_trustedkeys.sh

Comments

Mimi Zohar Sept. 10, 2019, 11:24 p.m. UTC | #1
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)

Mimi
Mimi Zohar Sept. 11, 2019, noon UTC | #2
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.

Mimi
Jarkko Sakkinen Sept. 13, 2019, 2:08 p.m. UTC | #3
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?

Can you bisect the failing commit?

/Jarkko
Mimi Zohar Sept. 15, 2019, 8:52 p.m. UTC | #4
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

Mimi
Mimi Zohar Sept. 16, 2019, 3:27 a.m. UTC | #5
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
Jarkko Sakkinen Sept. 16, 2019, 7:15 a.m. UTC | #6
On Sun, Sep 15, 2019 at 04:52:22PM -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

Thank you. As I said I'll add your test to the tests that I regulary
run so that this cannot happen again.

/Jarkko
Jarkko Sakkinen Sept. 16, 2019, 7:35 a.m. UTC | #7
On Sun, Sep 15, 2019 at 11:27:51PM -0400, 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.

I see. Obviously memcpy() does not tpm_buf length. The implementation is
kind of clunky but the point is to move building the tpm_buf to the
caller (which is soon possible thanks to Sumit's patches).

I sent a fix candidate.

/Jarkko
Jerry Snitselaar Sept. 16, 2019, 7:42 a.m. UTC | #8
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.

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.
Jarkko Sakkinen Sept. 16, 2019, 7:48 a.m. UTC | #9
On Mon, Sep 16, 2019 at 10:35:52AM +0300, Jarkko Sakkinen wrote:
> On Sun, Sep 15, 2019 at 11:27:51PM -0400, 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.
> 
> I see. Obviously memcpy() does not tpm_buf length. The implementation is
> kind of clunky but the point is to move building the tpm_buf to the
> caller (which is soon possible thanks to Sumit's patches).
> 
> I sent a fix candidate.

Ugh. Sorry about the "fix candidate". Your fix is the way it should be
done. I'll put background to it once I've slept a bit.

/Jarkko
Sumit Garg Sept. 16, 2019, 10:44 a.m. UTC | #10
On Mon, 16 Sep 2019 at 13:19, Jerry Snitselaar <jsnitsel@redhat.com> 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.
>
> 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.

This is exactly what patch #2 in this patch-set [1] tries to achieve.

[1] https://lkml.org/lkml/2019/9/16/196

-Sumit
Mimi Zohar Sept. 16, 2019, 11:36 a.m. UTC | #11
On Mon, 2019-09-16 at 10:48 +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 16, 2019 at 10:35:52AM +0300, Jarkko Sakkinen wrote:
> > On Sun, Sep 15, 2019 at 11:27:51PM -0400, 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.
> > 
> > I see. Obviously memcpy() does not tpm_buf length. The implementation is
> > kind of clunky but the point is to move building the tpm_buf to the
> > caller (which is soon possible thanks to Sumit's patches).
> > 
> > I sent a fix candidate.
> 
> Ugh. Sorry about the "fix candidate". Your fix is the way it should be
> done. I'll put background to it once I've slept a bit.

After trying different approaches that didn't work, I was about to
give up .  It was late, but at the last second, I tried this.  Glad it
works.

thanks,

Mimi
Mimi Zohar Sept. 16, 2019, 11:52 a.m. UTC | #12
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
Jerry Snitselaar Sept. 16, 2019, 2 p.m. UTC | #13
On Mon Sep 16 19, Sumit Garg wrote:
>On Mon, 16 Sep 2019 at 13:19, Jerry Snitselaar <jsnitsel@redhat.com> 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.
>>
>> 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.
>
>This is exactly what patch #2 in this patch-set [1] tries to achieve.
>
>[1] https://lkml.org/lkml/2019/9/16/196
>
>-Sumit

I'll take a look at the patchset today.

Regards,
Jerry
Jerry Snitselaar Sept. 16, 2019, 2:28 p.m. UTC | #14
On Mon Sep 16 19, Mimi Zohar wrote:
>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 don't know the security/keys/trusted.c code well at all, but it looks
like parts of it are wired up to differentiate between tpm1.2 and tpm2.0,
and others are not. There is a tpm_seal_trusted and tpm_unseal_trusted that
gets used for tpm2, and are called in trusted_instantiate, which does a
tpm_is_tpm2 check, but trusted_update just calls the tpm1.2 code and
has no tpm2 checks from what I can tell.

>> 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.
>

Agreed, I was just talking about in general. but it looks like Sumit has
already taken that task on.

>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
>
Jarkko Sakkinen Oct. 11, 2019, 12:34 p.m. UTC | #15
On Tue, Sep 10, 2019 at 07:18:31PM -0400, Mimi Zohar wrote:
> Create, save and load trusted keys test
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

I'm sorry. I have forgot this one! Can you send this linux-integrity
and kselftest? On surface looks somewhat appropriate.

/Jarkko
Jarkko Sakkinen Oct. 11, 2019, 1:01 p.m. UTC | #16
On Tue, Sep 10, 2019 at 07:18:31PM -0400, Mimi Zohar wrote:
> Create, save and load trusted keys test
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

Also can be used to test Sumit's patches i.e. there is an immediate
application for this one. I'll use this check TPM 1.x and TPM 2.0
trusted keys code.

You could sanity check your script for sending with my master, which
already has those patches.

/Jarkko
Mimi Zohar Oct. 11, 2019, 1:21 p.m. UTC | #17
On Fri, 2019-10-11 at 16:01 +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 10, 2019 at 07:18:31PM -0400, Mimi Zohar wrote:
> > Create, save and load trusted keys test
> > 
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> 
> Also can be used to test Sumit's patches i.e. there is an immediate
> application for this one. I'll use this check TPM 1.x and TPM 2.0
> trusted keys code.
> 
> You could sanity check your script for sending with my master, which
> already has those patches.

Sure, but due to the holidays that won't happen until the middle of
next week.

Mimi
Jarkko Sakkinen Oct. 14, 2019, 7:57 p.m. UTC | #18
On Fri, Oct 11, 2019 at 09:21:33AM -0400, Mimi Zohar wrote:
> On Fri, 2019-10-11 at 16:01 +0300, Jarkko Sakkinen wrote:
> > On Tue, Sep 10, 2019 at 07:18:31PM -0400, Mimi Zohar wrote:
> > > Create, save and load trusted keys test
> > > 
> > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > 
> > Also can be used to test Sumit's patches i.e. there is an immediate
> > application for this one. I'll use this check TPM 1.x and TPM 2.0
> > trusted keys code.
> > 
> > You could sanity check your script for sending with my master, which
> > already has those patches.
> 
> Sure, but due to the holidays that won't happen until the middle of
> next week.

There is no immediate rush, or more like, this should not be rushed.

I'm also waiting v8 now because of sparse issues.

/Jarkko
Jarkko Sakkinen Oct. 22, 2019, 2:43 p.m. UTC | #19
On Mon, Oct 14, 2019 at 10:57:25PM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 11, 2019 at 09:21:33AM -0400, Mimi Zohar wrote:
> > On Fri, 2019-10-11 at 16:01 +0300, Jarkko Sakkinen wrote:
> > > On Tue, Sep 10, 2019 at 07:18:31PM -0400, Mimi Zohar wrote:
> > > > Create, save and load trusted keys test
> > > > 
> > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > > 
> > > Also can be used to test Sumit's patches i.e. there is an immediate
> > > application for this one. I'll use this check TPM 1.x and TPM 2.0
> > > trusted keys code.
> > > 
> > > You could sanity check your script for sending with my master, which
> > > already has those patches.
> > 
> > Sure, but due to the holidays that won't happen until the middle of
> > next week.
> 
> There is no immediate rush, or more like, this should not be rushed.
> 
> I'm also waiting v8 now because of sparse issues.

OK, so you must get rid of TrouSerS dependency. Otherwise this
is unsuitable for kernel selftests. Please do TPM 1.x with the
raw TPM device.

A legit selftest should be implemented only with POSIX assets.

/Jarkko

Patch
diff mbox series

diff --git a/tools/testing/selftests/tpm2/Makefile b/tools/testing/selftests/tpm2/Makefile
index 9dd848427a7b..6c76495c9a69 100644
--- a/tools/testing/selftests/tpm2/Makefile
+++ b/tools/testing/selftests/tpm2/Makefile
@@ -1,4 +1,4 @@ 
 # SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
 include ../lib.mk
 
-TEST_PROGS := test_smoke.sh test_space.sh
+TEST_PROGS := test_smoke.sh test_space.sh test_trustedkey.sh
diff --git a/tools/testing/selftests/tpm2/test_trustedkeys.sh b/tools/testing/selftests/tpm2/test_trustedkeys.sh
new file mode 100755
index 000000000000..00b041d31646
--- /dev/null
+++ b/tools/testing/selftests/tpm2/test_trustedkeys.sh
@@ -0,0 +1,99 @@ 
+#!/bin/sh
+
+VERBOSE="${VERBOSE:-1}"
+TRUSTEDKEY1="$(mktemp -u XXXX).blob"
+TRUSTEDKEY2="$(mktemp -u XXXX).blob"
+trap "{ rm -f $TRUSTEDKEY1 $TRUSTEDKEY2 ; }" EXIT
+
+log_info()
+{
+        [ $VERBOSE -ne 0 ] && echo "[INFO] $1"
+}
+
+# The ksefltest framework requirement returns 0 for PASS.
+log_pass()
+{
+        [ $VERBOSE -ne 0 ] && echo "$1 [PASS]"
+        exit 0
+}
+
+# The ksefltest framework requirement returns 1 for FAIL.
+log_fail()
+{
+        [ $VERBOSE -ne 0 ] && echo "$1 [FAIL]"
+        exit 1
+}
+
+# The ksefltest framework requirement returns 4 for SKIP.
+log_skip()
+{
+        [ $VERBOSE -ne 0 ] && echo "$1"
+        exit 4
+}
+
+is_tpm1()
+{
+	local pcrs_path="/sys/class/tpm/tpm0/device/pcrs"
+	if [ ! -f "$pcrs_path" ]; then
+		pcrs_path="/sys/class/misc/tpm0/device/pcrs"
+	fi
+
+	if [ ! -f "$pcrs_path" ]; then
+		log_skip "TPM 1.2 chip not found"
+	fi
+}
+
+takeownership_info()
+{
+	which tcsd > /dev/null 2>&1 || \
+		log_skip "tcsd not found, install trousers"
+
+	which tpm_takeownership > /dev/null 2>&1 || \
+		log_skip "tpm_takeownerhip not found, install tpm-tools"
+
+	log_info "creating trusted key failed, probably requires taking TPM ownership:"
+	log_info "	systemctl start tcsd"
+	log_info "	tpm_takeownership -u -z"
+	log_fail "creating trusted key"
+}
+
+test_trustedkey()
+{
+	local keyid="$(keyctl add trusted kmk-test "new 64" @u)" &> /dev/null
+	if [ -z "$keyid" ]; then
+		takeownership_info
+	fi
+
+	if [ $? -eq 0 ]; then
+		log_info "creating trusted key succeeded"
+	else
+		log_fail "creating trusted key failed"
+	fi
+
+	# save newly created trusted key and remove from keyring
+	keyctl pipe "$keyid" > "$TRUSTEDKEY1"
+	keyctl unlink "$keyid" &> /dev/null
+
+	keyid=$(keyctl add trusted kmk-test "load `cat $TRUSTEDKEY1`" @u)
+	if [ $? -eq 0 ]; then
+		log_info "loading trusted key succeeded"
+	else
+		log_fail "loading trusted key failed"
+	fi
+
+	# save loaded trusted key and remove from keyring again
+	keyctl pipe "$keyid" > "$TRUSTEDKEY2"
+	keyctl unlink "$keyid" &> /dev/null
+
+	# compare trusted keys
+	diff "$TRUSTEDKEY1" "$TRUSTEDKEY2" &> /dev/null
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		log_pass "trusted key test succeeded"
+	else
+		log_fail "trusted key test failed"
+	fi
+}
+
+is_tpm1
+test_trustedkey