| Message ID | 20230510074359.2837818-7-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9] tpm: Fix spelling for tpmu_ha union | expand |
Hi Ilias, On Wed, 10 May 2023 at 01:44, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > We currently use PCR 0 for testing the PCR read/extend functionality in > our selftests. How ever those PCRs are defined by the TCG spec for > platform use. For example if the tests run *after* the efi subsystem > initialization, which extends PCRs 0 & 7 it will give a false positive. > > So let's switch over to a PCR which is more suitable and is defined for > OS use. It's worth noting that we are using PCR10 here, since PCR9 is > used internally by U-Boot if we choose to measure the loaded DTB > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > test/py/tests/test_tpm2.py | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) We should be able to reset the tpm emulator in state_reset_for_test(). Regards, Simon
Hi Simon, On Wed, 10 May 2023 at 17:32, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Wed, 10 May 2023 at 01:44, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > We currently use PCR 0 for testing the PCR read/extend functionality in > > our selftests. How ever those PCRs are defined by the TCG spec for > > platform use. For example if the tests run *after* the efi subsystem > > initialization, which extends PCRs 0 & 7 it will give a false positive. > > > > So let's switch over to a PCR which is more suitable and is defined for > > OS use. It's worth noting that we are using PCR10 here, since PCR9 is > > used internally by U-Boot if we choose to measure the loaded DTB > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > test/py/tests/test_tpm2.py | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > We should be able to reset the tpm emulator in state_reset_for_test(). That's irrelevant to the current patchset though. It's also not true for non sandbox testing, so I think we should just change the PCR we do our measurements on Thanks /Ilias > > Regards, > Simon
On Wed, 10 May 2023 at 09:27, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Wed, 10 May 2023 at 17:32, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Ilias, > > > > On Wed, 10 May 2023 at 01:44, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > We currently use PCR 0 for testing the PCR read/extend functionality in > > > our selftests. How ever those PCRs are defined by the TCG spec for > > > platform use. For example if the tests run *after* the efi subsystem > > > initialization, which extends PCRs 0 & 7 it will give a false positive. > > > > > > So let's switch over to a PCR which is more suitable and is defined for > > > OS use. It's worth noting that we are using PCR10 here, since PCR9 is > > > used internally by U-Boot if we choose to measure the loaded DTB > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > --- > > > test/py/tests/test_tpm2.py | 14 +++++++------- > > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > We should be able to reset the tpm emulator in state_reset_for_test(). > > That's irrelevant to the current patchset though. It's also not true > for non sandbox testing, so I think we should just change the PCR we > do our measurements on I don't see much point in running this test on real hardware, unless you are checking that the test is actually sane. But so long as it doesn't break anything, this seems fine to me. Reviewed-by: Simon Glass <sjg@chromium.org>
diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py index d2ad6f9e73c0..bae3095393c2 100644 --- a/test/py/tests/test_tpm2.py +++ b/test/py/tests/test_tpm2.py @@ -245,7 +245,7 @@ def test_tpm2_dam_parameters(u_boot_console): def test_tpm2_pcr_read(u_boot_console): """Execute a TPM2_PCR_Read command. - Perform a PCR read of the 0th PCR. Must be zero. + Perform a PCR read of the 10th PCR. Must be zero. """ if is_sandbox(u_boot_console): tpm2_sandbox_init(u_boot_console) @@ -253,7 +253,7 @@ def test_tpm2_pcr_read(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) + read_pcr = u_boot_console.run_command('tpm2 pcr_read 10 0x%x' % ram) output = u_boot_console.run_command('echo $?') assert output.endswith('0') @@ -263,7 +263,7 @@ def test_tpm2_pcr_read(u_boot_console): updates = int(re.findall(r'\d+', str)[0]) # Check the output value - assert 'PCR #0 content' in read_pcr + assert 'PCR #10 content' in read_pcr assert '00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00' in read_pcr @pytest.mark.buildconfigspec('cmd_tpm_v2') @@ -281,13 +281,13 @@ def test_tpm2_pcr_extend(u_boot_console): force_init(u_boot_console) ram = u_boot_utils.find_ram_base(u_boot_console) - u_boot_console.run_command('tpm2 pcr_extend 0 0x%x' % ram) + u_boot_console.run_command('tpm2 pcr_extend 10 0x%x' % ram) output = u_boot_console.run_command('echo $?') assert output.endswith('0') # Read the value back into a different place so we can still use 'ram' as # our zero bytes - read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % (ram + 0x20)) + read_pcr = u_boot_console.run_command('tpm2 pcr_read 10 0x%x' % (ram + 0x20)) output = u_boot_console.run_command('echo $?') assert output.endswith('0') assert 'f5 a5 fd 42 d1 6a 20 30 27 98 ef 6e d3 09 97 9b' in read_pcr @@ -297,11 +297,11 @@ def test_tpm2_pcr_extend(u_boot_console): new_updates = int(re.findall(r'\d+', str)[0]) assert (updates + 1) == new_updates - u_boot_console.run_command('tpm2 pcr_extend 0 0x%x' % ram) + u_boot_console.run_command('tpm2 pcr_extend 10 0x%x' % ram) output = u_boot_console.run_command('echo $?') assert output.endswith('0') - read_pcr = u_boot_console.run_command('tpm2 pcr_read 0 0x%x' % (ram + 0x20)) + read_pcr = u_boot_console.run_command('tpm2 pcr_read 10 0x%x' % (ram + 0x20)) output = u_boot_console.run_command('echo $?') assert output.endswith('0') assert '7a 05 01 f5 95 7b df 9c b3 a8 ff 49 66 f0 22 65' in read_pcr
We currently use PCR 0 for testing the PCR read/extend functionality in our selftests. How ever those PCRs are defined by the TCG spec for platform use. For example if the tests run *after* the efi subsystem initialization, which extends PCRs 0 & 7 it will give a false positive. So let's switch over to a PCR which is more suitable and is defined for OS use. It's worth noting that we are using PCR10 here, since PCR9 is used internally by U-Boot if we choose to measure the loaded DTB Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- test/py/tests/test_tpm2.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)