[PATCH 0/3 v2] tpm: add 'tpm autostart command'
Hi!
This is a resend of [0]. There are no functional or code changes in patches #1 and #2. Running this through the CI though exposed a problem on our python test scripts, whcih the newly added patch #3 is trying to address
[0] https://lore.kernel.org/u-boot/20230601062041.524010-1-ilias.apalodimas@lina...
Ilias Apalodimas (3): tpm: Add 'tpm autostart' shell command test/py: replace 'tpm2 init, startup, selftest' sequences test/py: Account PCR updates properly during testing
cmd/tpm-common.c | 16 ++++++++++++++++ cmd/tpm-user-utils.h | 1 + cmd/tpm-v1.c | 6 +++++- cmd/tpm-v2.c | 6 ++++++ test/py/tests/test_tpm2.py | 19 ++++++++----------- 5 files changed, 36 insertions(+), 12 deletions(-)
-- 2.39.2
For a TPM device to be operational we need to initialize it and perform its startup sequence. The 'tpm init' command currently calls tpm_init() which ends up calling the ->open() per-device callback and performs the initial hardware configuration as well as requesting locality 0 for the caller. There no code that currently calls tpm_init() without following up with a tpm_startup() and tpm_self_test_full() or tpm_continue_self_test().
So let's add a 'tpm autostart' command and call tpm_auto_start() which leaves the device in an operational state.
It's worth noting that calling tpm_init() only, doesn't allow a someone to use the TPM since the startup sequence is mandatory. We always repeat the pattern of calling - tpm_init() - tpm_startup() - tpm_self_test_full() or tpm_continue_self_test()
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- - Changes since v1: None
cmd/tpm-common.c | 16 ++++++++++++++++ cmd/tpm-user-utils.h | 1 + cmd/tpm-v1.c | 6 +++++- cmd/tpm-v2.c | 6 ++++++ 4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/cmd/tpm-common.c b/cmd/tpm-common.c index d0c63cadf413..a7dc23d85d5d 100644 --- a/cmd/tpm-common.c +++ b/cmd/tpm-common.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/string.h> #include <tpm-common.h> +#include <tpm_api.h> #include "tpm-user-utils.h"
static struct udevice *tpm_dev; @@ -367,6 +368,21 @@ int do_tpm_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return report_return_code(tpm_init(dev)); }
+int do_tpm_autostart(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + int rc; + + if (argc != 1) + return CMD_RET_USAGE; + rc = get_tpm(&dev); + if (rc) + return rc; + + return report_return_code(tpm_auto_start(dev)); +} + int do_tpm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct cmd_tbl *tpm_commands, *cmd; diff --git a/cmd/tpm-user-utils.h b/cmd/tpm-user-utils.h index de4a934aab6c..dfa11353e122 100644 --- a/cmd/tpm-user-utils.h +++ b/cmd/tpm-user-utils.h @@ -20,6 +20,7 @@ int get_tpm(struct udevice **devp); int do_tpm_device(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_tpm_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +int do_tpm_autostart(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_tpm_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_tpm_report_state(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c index 0efb079b0a9b..3b95c950cc96 100644 --- a/cmd/tpm-v1.c +++ b/cmd/tpm-v1.c @@ -655,6 +655,7 @@ TPM_COMMAND_NO_ARG(tpm_physical_disable) static struct cmd_tbl tpm1_commands[] = { U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""), U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""), + U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_autostart, "", ""), U_BOOT_CMD_MKENT(init, 0, 1, do_tpm_init, "", ""), U_BOOT_CMD_MKENT(startup, 0, 1, do_tpm_startup, "", ""), @@ -733,9 +734,12 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm, " device [num device]\n" " - Show all devices or set the specified device\n" " info - Show information about the TPM\n" +" autostart\n" +" - Initalize the tpm, perform a Startup(clear) and run a full selftest\n" +" sequence\n" " init\n" " - Put TPM into a state where it waits for 'startup' command.\n" -" startup mode\n" +" startup mode\n" " - Issue TPM_Starup command. <mode> is one of TPM_ST_CLEAR,\n" " TPM_ST_STATE, and TPM_ST_DEACTIVATED.\n" "Admin Testing Commands:\n" diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c index d93b83ada934..7e479b9dfe36 100644 --- a/cmd/tpm-v2.c +++ b/cmd/tpm-v2.c @@ -370,6 +370,7 @@ static struct cmd_tbl tpm2_commands[] = { U_BOOT_CMD_MKENT(dam_reset, 0, 1, do_tpm_dam_reset, "", ""), U_BOOT_CMD_MKENT(dam_parameters, 0, 1, do_tpm_dam_parameters, "", ""), U_BOOT_CMD_MKENT(change_auth, 0, 1, do_tpm_change_auth, "", ""), + U_BOOT_CMD_MKENT(autostart, 0, 1, do_tpm_autostart, "", ""), U_BOOT_CMD_MKENT(pcr_setauthpolicy, 0, 1, do_tpm_pcr_setauthpolicy, "", ""), U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1, @@ -392,8 +393,13 @@ U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command", " Show information about the TPM.\n" "state\n" " Show internal state from the TPM (if available)\n" +"autostart\n" +" Initalize the tpm, perform a Startup(clear) and run a full selftest\n" +" sequence\n" "init\n" " Initialize the software stack. Always the first command to issue.\n" +" 'tpm startup' is the only acceptable command after a 'tpm init' has been\n" +" issued\n" "startup <mode>\n" " Issue a TPM2_Startup command.\n" " <mode> is one of:\n" -- 2.39.2
Instead of copy pasting the commands needed to start a TPM consisting of: - tpm init - tpm startup TPM2_SU_CLEAR - tpm2 self_test full use the newly added 'autostart' which does the same thing and simplify our python scripts
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org --- Changes since v1: None
test/py/tests/test_tpm2.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/test/py/tests/test_tpm2.py b/test/py/tests/test_tpm2.py index d2ad6f9e73c0..1ade66a7eda4 100644 --- a/test/py/tests/test_tpm2.py +++ b/test/py/tests/test_tpm2.py @@ -41,11 +41,9 @@ def force_init(u_boot_console, force=False): skip_test = u_boot_console.config.env.get('env__tpm_device_test_skip', False) if skip_test: pytest.skip('skip TPM device test') - output = u_boot_console.run_command('tpm2 init') + output = u_boot_console.run_command('tpm2 autostart') if force or not 'Error' in output: u_boot_console.run_command('echo --- start of init ---') - u_boot_console.run_command('tpm2 startup TPM2_SU_CLEAR') - u_boot_console.run_command('tpm2 self_test full') u_boot_console.run_command('tpm2 clear TPM2_RH_LOCKOUT') output = u_boot_console.run_command('echo $?') if not output.endswith('0'): @@ -83,20 +81,13 @@ def tpm2_sandbox_init(u_boot_console): This allows all tests to run in parallel, since no test depends on another. """ u_boot_console.restart_uboot() - u_boot_console.run_command('tpm2 init') + u_boot_console.run_command('tpm2 autostart') output = u_boot_console.run_command('echo $?') assert output.endswith('0')
skip_test = u_boot_console.config.env.get('env__tpm_device_test_skip', False) if skip_test: pytest.skip('skip TPM device test') - u_boot_console.run_command('tpm2 startup TPM2_SU_CLEAR') - output = u_boot_console.run_command('echo $?') - assert output.endswith('0') - - u_boot_console.run_command('tpm2 self_test full') - output = u_boot_console.run_command('echo $?') - assert output.endswith('0')
@pytest.mark.buildconfigspec('cmd_tpm_v2') def test_tpm2_sandbox_self_test_full(u_boot_console): -- 2.39.2
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(+)
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 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
participants (2)
-
Ilias Apalodimas -
Simon Glass