Re: [PATCH] tpm: Add TPM2_GetTestResult command support

Hi Julia,

Apologies for the late response, I was on vacation.

On Mon, 3 Jul 2023 at 16:03, Julia Daxenberger julia.daxenberger@infineon.com wrote:

Add TPM2_GetTestResult command support and change the command file and the help accordingly. Add Python tests and sandbox driver functionality.

The TPM2_GetTestResult command is performed after the TPM2_SelfTest command and returns manufacturer-specific information regarding the results of the self-test and an indication of the test status.

What the code does is quite obvious, can you please elaborate on why we need this applied?

[...]

+#define TEST_RESULT_DATA_BUFFER_SIZE 256

static int do_tpm2_startup(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -356,6 +359,57 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl *cmdtp, int flag, key, key_sz)); }

+static int do_tpm2_get_test_result(struct cmd_tbl *cmdtp, int flag, int argc,

•                              char *const argv[])
  

+{

•   struct udevice *dev;
  

•   int ret;
  

•   u8 data[TEST_RESULT_DATA_BUFFER_SIZE];
  

•   size_t data_len = TEST_RESULT_DATA_BUFFER_SIZE;
  

= sizeof(data)

•   u32 test_result;
  

•   u32 rc;
  

•   ret = get_tpm(&dev);
  

•   if (ret)
  

•           return ret;
  

•   if (argc != 1)
  

•           return CMD_RET_USAGE;
  

Can you move this one above the get_tpm?

•   rc = tpm2_get_test_result(dev, data, &data_len, &test_result);
  

•   if (rc)
  

•           goto out;
  

•   printf("Test Result:\n\t0x%08X", test_result);
  

•   switch (test_result) {
  

•   case 0x0000:
  

We have enum tpm2_return_codes. Please add the missing cases there and use that instead of magic values

•           printf("\tTPM2_RC_SUCCESS\n");
  

•           break;
  

•   case 0x0101:
  

•           printf("\tTPM2_RC_FAILURE\n");
  

•           break;
  

•   case 0x0153:
  

•           printf("\tTPM2_RC_NEEDS_TEST\n");
  

•           break;
  

•   case 0x090A:
  

•           printf("\tTPM2_RC_TESTING\n");
  

•           break;
  

•   }
  

•   if (!data_len) {
  

•           printf("No Test Result Data available\n");
  

•           goto out;
  

•   }
  

•   printf("Test Result Data of Self Test:\n\t0x");
  

•   for (int i = 0; i < data_len; i++)
  

•           printf("%02X", data[i]);
  

•   printf("\n");
  

+out:

•   return report_return_code(rc);
  

+}

static struct cmd_tbl tpm2_commands[] = { U_BOOT_CMD_MKENT(device, 0, 1, do_tpm_device, "", ""), U_BOOT_CMD_MKENT(info, 0, 1, do_tpm_info, "", ""), @@ -374,6 +428,8 @@ static struct cmd_tbl tpm2_commands[] = { do_tpm_pcr_setauthpolicy, "", ""), U_BOOT_CMD_MKENT(pcr_setauthvalue, 0, 1, do_tpm_pcr_setauthvalue, "", ""),

•   U_BOOT_CMD_MKENT(get_test_result, 0, 1,
  

•                    do_tpm2_get_test_result, "", ""),
  

};

struct cmd_tbl *get_tpm2_commands(unsigned int *size) @@ -447,4 +503,8 @@ U_BOOT_CMD(tpm2, CONFIG_SYS_MAXARGS, 1, do_tpm, "Issue a TPMv2.x command", " <pcr>: index of the PCR\n" " <key>: secret to protect the access of PCR #<pcr>\n" " <password>: optional password of the PLATFORM hierarchy\n" +"get_test_result\n" +" Show manufacturer-specific information regarding the results of a\n" +" self-test and an indication of the test status.\n"

); diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index e4004cfcca..17979cd33a 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -2,6 +2,7 @@ /*

• Copyright (c) 2018, Bootlin
• Author: Miquel Raynal miquel.raynal@bootlin.com

• • Copyright (C) 2023 Infineon Technologies AG
  */

#include <common.h> @@ -231,6 +232,7 @@ static int sandbox_tpm2_check_session(struct udevice *dev, u32 command, u16 tag, case TPM2_CC_SELF_TEST: case TPM2_CC_GET_CAPABILITY: case TPM2_CC_PCR_READ:

•   case TPM2_CC_GET_TEST_RESULT:
            if (tag != TPM2_ST_NO_SESSIONS) {
                    printf("No session required for command 0x%x\n",
                           command);
  

@@ -364,6 +366,13 @@ static int sandbox_tpm2_check_readyness(struct udevice *dev, int command) if (!tpm->startup_done) return TPM2_RC_INITIALIZE;

•           break;
  

•   case TPM2_CC_GET_TEST_RESULT:
  

•           if (!tpm->init_done || !tpm->startup_done)
  

•                   return TPM2_RC_INITIALIZE;
  

•           if (!tpm->tests_done)
  

•                   return TPM2_RC_NEEDS_TEST;
  

•           break;
    default:
            /* Skip this, since the startup may have happened in SPL
  

@@ -458,7 +467,7 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 *sendbuf, command = get_unaligned_be32(sent); sent += sizeof(command); rc = sandbox_tpm2_check_readyness(dev, command);

•   if (rc) {
  

•   if (rc && rc != TPM2_RC_NEEDS_TEST) {
            sandbox_tpm2_fill_buf(recv, recv_len, tag, rc);
            return 0;
    }
  

@@ -778,6 +787,42 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 *sendbuf, *recv_len = 12; memset(recvbuf, '\0', *recv_len); break;

•   case TPM2_CC_GET_TEST_RESULT: {
  

•           u32 testresult = 0;
  

•           u8 data[10] = { 0 };
  

•           /* Check readiness */
  

•           testresult = sandbox_tpm2_check_readyness(dev, command);
  

•           /* Write tag */
  

•           put_unaligned_be16(tag, recv);
  

•           recv += sizeof(tag);
  

•           /* Ignore length for now */
  

•           recv += sizeof(u32);
  

•           /* Write return code */
  

•           put_unaligned_be32(rc, recv);
  

•           recv += sizeof(rc);
  

•           /* Write manufacturer-specific test result data */
  

•           memcpy(recv, data, sizeof(data));
  

•           recv += sizeof(data);
  

•           /* Write test result */
  

•           put_unaligned_be32(testresult, recv);
  

•           recv += sizeof(testresult);
  

•           /* Add trailing \0 */
  

•           *recv = '\0';
  

•           /* Write response length */
  

•           *recv_len = recv - recvbuf;
  

•           put_unaligned_be32(*recv_len, recvbuf + sizeof(tag));
  

•           break;
  

•   }
    default:
            printf("TPM2 command %02x unknown in Sandbox\n", command);
            rc = TPM2_RC_COMMAND_CODE;
  

diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 2b6980e441..f6ec430316 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -3,6 +3,7 @@

• Defines APIs and structures that allow software to interact with a
• TPM2 device

• • Copyright (C) 2023 Infineon Technologies AG
  • Copyright (c) 2020 Linaro
  • Copyright (c) 2018 Bootlin

@@ -276,6 +277,7 @@ enum tpm2_handles {

• @TPM2_CC_DAM_PARAMETERS: TPM2_DictionaryAttackParameters().
• @TPM2_CC_GET_CAPABILITY: TPM2_GetCapibility().
• @TPM2_CC_GET_RANDOM: TPM2_GetRandom().

• • @TPM2_CC_GET_TEST_RESULT: TPM2_GetTestResult().
  • @TPM2_CC_PCR_READ: TPM2_PCR_Read().
  • @TPM2_CC_PCR_EXTEND: TPM2_PCR_Extend().
  • @TPM2_CC_PCR_SETAUTHVAL: TPM2_PCR_SetAuthValue().

@@ -296,6 +298,7 @@ enum tpm2_command_codes { TPM2_CC_NV_READ = 0x014E, TPM2_CC_GET_CAPABILITY = 0x017A, TPM2_CC_GET_RANDOM = 0x017B,

•   TPM2_CC_GET_TEST_RESULT = 0x017C,
    TPM2_CC_PCR_READ        = 0x017E,
    TPM2_CC_PCR_EXTEND      = 0x0182,
    TPM2_CC_PCR_SETAUTHVAL  = 0x0183,
  

@@ -706,4 +709,24 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd, */ u32 tpm2_auto_start(struct udevice *dev);

+/**

• • Issue a TPM2_GetTestResult command.
• • @dev TPM device
• • @data output buffer for manufacturer-specific test result data
• • @data_len input/output parameter

• •         input: size of data buffer, output: size of test result data
    

• • @test_result output parameter: test result response code:

• •         TPM2_RC_NEEDS_TEST, if TPM2 self-test has not been executed and
    

• •         a testable function has not been tested
    

• •         TPM2_RC_TESTING, if TPM2 self-test is in progress.
    

• •         TPM2_RC_SUCCESS, if testing of all functions is complete without
    

• •         functional failures.
    

• •         TPM2_RC_FAILURE, if any test failed.
    

• • Return: code of the operation
• */

+u32 tpm2_get_test_result(struct udevice *dev, void *data, size_t *data_len,

•                    u32 *test_result);
  

#endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 9ab5b46df1..a3a29048e3 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -2,12 +2,14 @@ /*

• Copyright (c) 2018 Bootlin
• Author: Miquel Raynal miquel.raynal@bootlin.com

• • Copyright (C) 2023 Infineon Technologies AG
  */

#include <common.h> #include <dm.h> #include <tpm-common.h> #include <tpm-v2.h> +#include <asm/unaligned.h> #include <linux/bitops.h> #include "tpm-utils.h"

@@ -742,3 +744,83 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd,

    return 0;

}

+u32 tpm2_get_test_result(struct udevice *dev, void *data,

•                    size_t *data_len, u32 *test_result)
  

+{

•   const u8 command_v2[COMMAND_BUFFER_SIZE] = {
  

•           /* header 10 bytes */
  

•           tpm_u16(TPM2_ST_NO_SESSIONS),           /* TAG */
  

•           tpm_u32(10),                            /* Length */
  

•           tpm_u32(TPM2_CC_GET_TEST_RESULT),       /* Command code */
  

•   };
  

•   int ret;
  

•   u8 response[COMMAND_BUFFER_SIZE];
  

•   size_t response_len = COMMAND_BUFFER_SIZE;
  

•   u32 response_size;                              /*  Size of response bytestream */
  

reposnse_size and response_len is a bit confusing. Please find some names that indicate the first one is the reserved size while the second is the actual

•   unsigned int response_size_off;
  

•   unsigned int data_off;
  

•   unsigned int test_result_off;
  

•   size_t data_buff_len;
  

•   ret = tpm_sendrecv_command(dev, command_v2, response, &response_len);
  

•   if (ret) {
  

•           log_debug("Returned on: ret = tpm_sendrecv_command()\n");
  

•           return ret;
  

Isn't this an error? Can we use return log_msg_ret("test", ret); or something along those lines?

•   }
  

•   /*
  

•    * Get responseSize from return bytestream, switch endianness: big->little
  

•    * In the response buffer, the responseSize is located after:
  

•    * tag (u16)
  

•    */
  

•   response_size_off = sizeof(u16);
  

Keep the comment and just do response + sizeof(u16)

•   response_size = get_unaligned_be32(response + response_size_off);
  

•   if (response_len < 10) {
  

•           log_debug
  

•               ("Returned on plausibility check: response_len < 10\n");
  

Why is this an error?

•           return -EINVAL;
  

•   }
  

•   if (response_size > response_len) {
  

•           log_debug
  

•               ("Returned on plausibility check: response_size > response_len\n\t"
  

Can you remove this line and just report the size mismatch? It should be obvious why it's wrong.

•                "Response Size: %d\n\tResponse Length: %ld\n",
  

•                response_size, response_len);
  

•           return -E2BIG;
  

•   }
  

•   /*
  

•    * Copy the test result data to buffer, transfer data_len
  

•    * In the response buffer, the test result data is located after:
  

•    * tag (u16), response size (u32), response code (u32).
  

•    */
  

•   data_off = sizeof(u16) + sizeof(u32) + sizeof(u32);
  

•   data_buff_len = *data_len;
  

•   *data_len = response_size - data_off - sizeof(u32);
  

•   /* Checks, if the reserved data buffer size is insufficient */
  

•   if (*data_len > data_buff_len) {
  

•           log_debug
  

•               ("Returned on plausibility check: data_len > data_buff_len\n\t"
  

•                "Data Length: %ld\n\tData Buffer Length: %ld\n\t",
  

•                *data_len, data_buff_len);
  

•           return -E2BIG;
  

•   }
  

•   memcpy(data, &response[data_off], *data_len);
  

•   /*
  

•    * Get testResult from return bytestream, switch endianness: big->little
  

•    * In the response buffer, the test result is located after:
  

•    * tag (u16), response size (u32), response code (u32),
  

•    * test result data (*data_len).
  

•    */
  

•   test_result_off = data_off + *data_len;
  

•   *test_result = get_unaligned_be32(response + test_result_off);
  

•   return 0;
  

+}

[...]

Thanks /Ilias