[3/3,v2] test/py: Account PCR updates properly during testing

Currently we only read the pcr updates once on test_tpm2_pcr_read().
It turns out that the tpm init sequence of force_init() which consists
of:
- tpm2 init
- tpm2 startup TPM2_SU_CLEAR
- tpm2 self_test full
- tpm2 clear TPM2_RH_LOCKOUT

also counts as an update.  Running this in the console verifies the
update bump
=> tpm2 init
=> tpm2 startup TPM2_SU_CLEAR
=> tpm2 self_test full
=> tpm pcr_read 10 $loadaddr
PCR #10 content (28 known updates):
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=> tpm2 clear TPM2_RH_LOCKOUT
=> tpm pcr_read 10 $loadaddr
PCR #10 content (29 known updates):
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>

With the recent changes of replacing 'tpm2 init' with 'tpm2 autostart'
we end up always running the full init.  The reason is 'tpm init'
returns -EBUSY if the tpm is already open, while 'tpm autostart' handles
ths gracefully and continues with the initialization.  It's worth noting
that this won't affect the device functionality at all since
retriggering the startup sequence and selftests has no side effects.

Instead of relying on the initial value, reread the 'known updates'
just before updating the PCR to ensure we read the correct values
before testing

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes since v1:
- new patch to fix the python testing failures

 test/py/tests/test_tpm2.py | 6 ++++++
 1 file changed, 6 insertions(+)

--
2.39.2

Hi Ilias,

On Wed, 7 Jun 2023 at 10:18, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Currently we only read the pcr updates once on test_tpm2_pcr_read().
> It turns out that the tpm init sequence of force_init() which consists
> of:
> - tpm2 init
> - tpm2 startup TPM2_SU_CLEAR
> - tpm2 self_test full
> - tpm2 clear TPM2_RH_LOCKOUT
>
> also counts as an update.  Running this in the console verifies the
> update bump
> => tpm2 init
> => tpm2 startup TPM2_SU_CLEAR
> => tpm2 self_test full
> => tpm pcr_read 10 $loadaddr
> PCR #10 content (28 known updates):
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> => tpm2 clear TPM2_RH_LOCKOUT
> => tpm pcr_read 10 $loadaddr
> PCR #10 content (29 known updates):
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>
>
> With the recent changes of replacing 'tpm2 init' with 'tpm2 autostart'
> we end up always running the full init.  The reason is 'tpm init'
> returns -EBUSY if the tpm is already open, while 'tpm autostart' handles
> ths gracefully and continues with the initialization.  It's worth noting
> that this won't affect the device functionality at all since
> retriggering the startup sequence and selftests has no side effects.

This may be true for some TPMs.

>
> Instead of relying on the initial value, reread the 'known updates'
> just before updating the PCR to ensure we read the correct values
> before testing
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Changes since v1:
> - new patch to fix the python testing failures
>
>  test/py/tests/test_tpm2.py | 6 ++++++
>  1 file changed, 6 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

BTW this is an example of why I still want to be able to just init the
TPM to a basic level. Here we see that autostart changes the PCRs.


> diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
> index 1ade66a7eda4..fce689cd992d 100644
> --- a/test/py/tests/test_tpm2.py
> +++ b/test/py/tests/test_tpm2.py
> @@ -272,6 +272,12 @@ def test_tpm2_pcr_extend(u_boot_console):
>      force_init(u_boot_console)
>      ram = u_boot_utils.find_ram_base(u_boot_console)
>
> +    read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % (ram + 0x20))
> +    output = u_boot_console.run_command('echo $?')
> +    assert output.endswith('0')
> +    str = re.findall(r'\d+ known updates', read_pcr)[0]
> +    updates = int(re.findall(r'\d+', str)[0])
> +
>      u_boot_console.run_command('tpm2 pcr_extend 0 0x%x' % ram)
>      output = u_boot_console.run_command('echo $?')
>      assert output.endswith('0')
> --
> 2.39.2
>

On Mon, Jun 12, 2023 at 10:17:28PM +0100, Simon Glass wrote:
> Hi Ilias,
>
> On Wed, 7 Jun 2023 at 10:18, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Currently we only read the pcr updates once on test_tpm2_pcr_read().
> > It turns out that the tpm init sequence of force_init() which consists
> > of:
> > - tpm2 init
> > - tpm2 startup TPM2_SU_CLEAR
> > - tpm2 self_test full
> > - tpm2 clear TPM2_RH_LOCKOUT
> >
> > also counts as an update.  Running this in the console verifies the
> > update bump
> > => tpm2 init
> > => tpm2 startup TPM2_SU_CLEAR
> > => tpm2 self_test full
> > => tpm pcr_read 10 $loadaddr
> > PCR #10 content (28 known updates):
> >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > => tpm2 clear TPM2_RH_LOCKOUT
> > => tpm pcr_read 10 $loadaddr
> > PCR #10 content (29 known updates):
> >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > =>
> >
> > With the recent changes of replacing 'tpm2 init' with 'tpm2 autostart'
> > we end up always running the full init.  The reason is 'tpm init'
> > returns -EBUSY if the tpm is already open, while 'tpm autostart' handles
> > ths gracefully and continues with the initialization.  It's worth noting
> > that this won't affect the device functionality at all since
> > retriggering the startup sequence and selftests has no side effects.
>
> This may be true for some TPMs.

The responses to the startup command are described by the spec, so unless
the device isn't a standard TPM, this should be safe

>
> >
> > Instead of relying on the initial value, reread the 'known updates'
> > just before updating the PCR to ensure we read the correct values
> > before testing
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > Changes since v1:
> > - new patch to fix the python testing failures
> >
> >  test/py/tests/test_tpm2.py | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> BTW this is an example of why I still want to be able to just init the
> TPM to a basic level. Here we see that autostart changes the PCRs.
>
It doesnt change the PCRs.  That code is checking how many commands have
been sent to the TPM in total.  In the previous version of the code 'tpm
init' would return -EBUSY and we would never re-run the next commands.  The
new command returns 0 and as a result we end up running the TPM2_RH_LOCKOUT
again.

Thanks
/Ilias
>
> > diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py
> > index 1ade66a7eda4..fce689cd992d 100644
> > --- a/test/py/tests/test_tpm2.py
> > +++ b/test/py/tests/test_tpm2.py
> > @@ -272,6 +272,12 @@ def test_tpm2_pcr_extend(u_boot_console):
> >      force_init(u_boot_console)
> >      ram = u_boot_utils.find_ram_base(u_boot_console)
> >
> > +    read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % (ram + 0x20))
> > +    output = u_boot_console.run_command('echo $?')
> > +    assert output.endswith('0')
> > +    str = re.findall(r'\d+ known updates', read_pcr)[0]
> > +    updates = int(re.findall(r'\d+', str)[0])
> > +
> >      u_boot_console.run_command('tpm2 pcr_extend 0 0x%x' % ram)
> >      output = u_boot_console.run_command('echo $?')
> >      assert output.endswith('0')
> > --
> > 2.39.2
> >

Hi Ilias,

On Tue, 13 Jun 2023 at 06:48, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Mon, Jun 12, 2023 at 10:17:28PM +0100, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Wed, 7 Jun 2023 at 10:18, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Currently we only read the pcr updates once on test_tpm2_pcr_read().
> > > It turns out that the tpm init sequence of force_init() which consists
> > > of:
> > > - tpm2 init
> > > - tpm2 startup TPM2_SU_CLEAR
> > > - tpm2 self_test full
> > > - tpm2 clear TPM2_RH_LOCKOUT
> > >
> > > also counts as an update.  Running this in the console verifies the
> > > update bump
> > > => tpm2 init
> > > => tpm2 startup TPM2_SU_CLEAR
> > > => tpm2 self_test full
> > > => tpm pcr_read 10 $loadaddr
> > > PCR #10 content (28 known updates):
> > >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > => tpm2 clear TPM2_RH_LOCKOUT
> > > => tpm pcr_read 10 $loadaddr
> > > PCR #10 content (29 known updates):
> > >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > =>
> > >
> > > With the recent changes of replacing 'tpm2 init' with 'tpm2 autostart'
> > > we end up always running the full init.  The reason is 'tpm init'
> > > returns -EBUSY if the tpm is already open, while 'tpm autostart' handles
> > > ths gracefully and continues with the initialization.  It's worth noting
> > > that this won't affect the device functionality at all since
> > > retriggering the startup sequence and selftests has no side effects.
> >
> > This may be true for some TPMs.
>
> The responses to the startup command are described by the spec, so unless
> the device isn't a standard TPM, this should be safe

Yes, that could be the problem.

>
> >
> > >
> > > Instead of relying on the initial value, reread the 'known updates'
> > > just before updating the PCR to ensure we read the correct values
> > > before testing
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > ---
> > > Changes since v1:
> > > - new patch to fix the python testing failures
> > >
> > >  test/py/tests/test_tpm2.py | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > BTW this is an example of why I still want to be able to just init the
> > TPM to a basic level. Here we see that autostart changes the PCRs.
> >
> It doesnt change the PCRs.  That code is checking how many commands have
> been sent to the TPM in total.  In the previous version of the code 'tpm
> init' would return -EBUSY and we would never re-run the next commands.  The
> new command returns 0 and as a result we end up running the TPM2_RH_LOCKOUT
> again.

OK.

Regards,
Simon