| Message ID | 20230510074359.2837818-8-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| 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: > > 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 > => > > Instead of relying on the initial read do a read just before updating > the PCR to ensure we read the correct values before testing > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > test/py/tests/test_tpm2.py | 6 ++++++ > 1 file changed, 6 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> How do these tests pass today? Or do they not? 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: > > > > 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 > > => > > > > Instead of relying on the initial read do a read just before updating > > the PCR to ensure we read the correct values before testing > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > test/py/tests/test_tpm2.py | 6 ++++++ > > 1 file changed, 6 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > How do these tests pass today? Or do they not? They, what I suspect was happening is that the order of testing, or init changed with Eddies patches. As a consequence the test started failing because it ended up with updates bumped by two instead of 1. Regardless I think this makes sense to apply overall as the current logic was making too many assumptions on the order of tests or the TPM state. Regards /Ilias > > Regards, > Simon
Hi Ilias, On Wed, 10 May 2023 at 09:26, 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: > > > > > > 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 > > > => > > > > > > Instead of relying on the initial read do a read just before updating > > > the PCR to ensure we read the correct values before testing > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > --- > > > test/py/tests/test_tpm2.py | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > How do these tests pass today? Or do they not? > > They, what I suspect was happening is that the order of testing, or > init changed with Eddies patches. As a consequence the test started > failing because it ended up with updates bumped by two instead of 1. > Regardless I think this makes sense to apply overall as the current > logic was making too many assumptions on the order of tests or the TPM > state. The test order should not matter. But perhaps a board reset is needed between these tests? That's what's so nice about sandbox. It is easy to reset the test state. Regards, SImon
diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py index bae3095393c2..57722fdc5977 100644 --- a/test/py/tests/test_tpm2.py +++ b/test/py/tests/test_tpm2.py @@ -281,6 +281,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 10 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 10 0x%x' % ram) output = u_boot_console.run_command('echo $?') assert output.endswith('0')
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 => Instead of relying on the initial read do a read just before updating the PCR to ensure we read the correct values before testing Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- test/py/tests/test_tpm2.py | 6 ++++++ 1 file changed, 6 insertions(+)