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

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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