New TPM commands.
Ilias Apalodimas
ilias.apalodimas at linaro.org
Wed Dec 20 09:17:49 CET 2023
Hi Niek
On Wed, 20 Dec 2023 at 09:11, niek.nooijens at omron.com
<niek.nooijens at omron.com> wrote:
>
> Hi There
>
> I added some new commands to the TPM2 command to allow read/writes to nv_memory. I also implemented the nv_define and nv_undefine commands so spaces can be created/deleted.
> Still need to test with PCR policies, but at least for now we can store values in the TPM.
Thanks for the patch. There's a standard process for sending patches
[0]. Try to follow the guidelines on your next revision, it makes
things substantially easier for me. Also, spend some time writing a
clear patch description on *why* you need this merged.
>
> Here's the patch:
>
> Signed-off-by: Niek Nooijens <niek.nooijens at omron.com>
> ================BEGIN OF PATCH==============
> diff --git a/cmd/tpm-v2.c b/cmd/tpm-v2.c
> index d93b83ada9..d2a06b9f65 100644
> --- a/cmd/tpm-v2.c
> +++ b/cmd/tpm-v2.c
> @@ -356,6 +356,133 @@ static int do_tpm_pcr_setauthvalue(struct cmd_tbl *cmdtp, int flag,
> key, key_sz));
> }
>
> +static int do_tpm_nv_define(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + struct tpm_chip_priv *priv;
> + u32 nv_addr, nv_size,nv_attributes, rc;
> + void *policy_addr = NULL;
> + size_t policy_size = 0;
> + int ret;
> +
> + nv_attributes = 0;
> +
> + if ((argc < 3 && argc > 6) || argc == 4)
> + return CMD_RET_USAGE;
> +
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + priv = dev_get_uclass_priv(dev);
> + if (!priv)
> + return -EINVAL;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
Ditch the '//' comments throughout the patch. It's pretty clear what's happening
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size
ditto
> +
> + if(argc > 3) { //attributes
> + nv_attributes = simple_strtoul(argv[3], NULL, 0);
> + } else {
> + nv_attributes = TPMA_NV_PLATFORMCREATE|TPMA_NV_OWNERWRITE|TPMA_NV_OWNERREAD|TPMA_NV_PPWRITE|TPMA_NV_PPREAD;
> + }
You don't need {}
> +
> + if(argc > 4) {//policy
> + policy_addr = map_sysmem(simple_strtoul(argv[4], NULL, 0), 0);
> + if((nv_attributes & (TPMA_NV_POLICYREAD|TPMA_NV_POLICYWRITE)) == 0) { //not sure if I should enforce this or just warn the user?
> + printf("Warning: policy provided, but TPMA_NV_POLICYREAD and TPMA_NV_POLICYWRITE are NOT set!\n");
Just return an error here.
> + }
> + policy_size = simple_strtoul(argv[5], NULL, 0);
> + }
> +
> + rc = tpm2_nv_define_space(dev, nv_addr, nv_size, nv_attributes,policy_addr, policy_size);
> +
> + if (rc) {
> + printf("ERROR: nv_define #%u returns: 0x%x\n", nv_addr, rc);
> + }
> + if(argc > 4) {
policy_addr is initialized to NULL, just look at the ptr here instead of argc
> + unmap_sysmem(policy_addr);
> + }
You don't need {} in any of the above
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_undefine(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr,ret, rc;
> +
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + if (argc !=2)
> + return CMD_RET_USAGE;
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> + rc = tpm2_nv_undefine_space(dev, nv_addr);
> +
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_read_value(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, nv_size, rc;
> + int ret;
> + void *out_data;
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + if (argc != 4)
> + return CMD_RET_USAGE;
Indentation is off
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size
> +
> + out_data = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
> +
> + rc = tpm2_nv_read_value(dev,nv_addr, out_data, nv_size);
> +
> + if (rc) {
> + printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
> + }
> + unmap_sysmem(out_data);
> + return report_return_code(rc);
> +}
> +
> +static int do_tpm_nv_write_value(struct cmd_tbl *cmdtp, int flag,
> + int argc, char *const argv[])
> +{
> + struct udevice *dev;
> + u32 nv_addr, nv_size, rc;
> + int ret;
> + ret = get_tpm(&dev);
> + if (ret)
> + return ret;
> +
> + if (argc != 4)
> + return CMD_RET_USAGE;
> +
> + nv_addr = simple_strtoul(argv[1], NULL, 0); //tpm_addr
> +
> + nv_size = simple_strtoul(argv[2], NULL, 0); //size
> +
> + void *data_to_write = map_sysmem(simple_strtoul(argv[3], NULL, 0), 0);
> +
> + rc = tpm2_nv_write_value(dev,nv_addr, data_to_write, nv_size);
> +
> + if (rc) {
> + printf("ERROR: nv_read #%u returns: #%u\n", nv_addr, rc);
> + }
Get rid of {}
[...]
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index 737e57551d..b9801e91eb 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -214,6 +214,7 @@ struct tcg_pcr_event2 {
> u8 event[];
> } __packed;
>
> +
> /**
> * TPM2 Structure Tags for command/response buffers.
> *
> @@ -286,6 +287,7 @@ enum tpm2_command_codes {
> TPM2_CC_CLEARCONTROL = 0x0127,
> TPM2_CC_HIERCHANGEAUTH = 0x0129,
> TPM2_CC_NV_DEFINE_SPACE = 0x012a,
> + TPM2_CC_NV_UNDEFINE_SPACE = 0x0122,
> TPM2_CC_PCR_SETAUTHPOL = 0x012C,
> TPM2_CC_NV_WRITE = 0x0137,
> TPM2_CC_NV_WRITELOCK = 0x0138,
> @@ -469,6 +471,20 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
> size_t space_size, u32 nv_attributes,
> const u8 *nv_policy, size_t nv_policy_size);
>
> +
> +
> +
> +/**
> + * Issue a TPM_NV_UnDefineSpace command
run make htmldocs before the next revision and make sure you get no
errors. We usually start with the function name here
> + *
> + * This allows a space to be removed. Needed because TPM_clear doesn't clear platform entries
> + *
> + * @dev TPM device
> + * @space_index index of the area
> + * Return: return code of the operation
> + */
> +u32 tpm2_nv_undefine_space(struct udevice *dev, u32 space_index);
> +
> /**
> * Issue a TPM2_PCR_Extend command.
> *
> @@ -494,6 +510,7 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm,
> */
> u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count);
>
> +
> /**
> * Write data to the secure storage
> *
> diff --git a/lib/tpm-common.c b/lib/tpm-common.c
> index 82ffdc5341..fbb78a941f 100644
> --- a/lib/tpm-common.c
> +++ b/lib/tpm-common.c
> @@ -3,7 +3,7 @@
> * Copyright (c) 2013 The Chromium OS Authors.
> * Coypright (c) 2013 Guntermann & Drunck GmbH
> */
> -
> +#define LOG_DEBUG
> #define LOG_CATEGORY UCLASS_TPM
>
> #include <common.h>
> @@ -13,6 +13,8 @@
> #include <tpm-common.h>
> #include "tpm-utils.h"
>
> +
> +
> enum tpm_version tpm_get_version(struct udevice *dev)
> {
> struct tpm_chip_priv *priv = dev_get_uclass_priv(dev);
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 697b982e07..9df3968033 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -90,7 +90,7 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
> * chunks below.
> */
> const int platform_len = sizeof(u32);
> - const int session_hdr_len = 13;
> + const int session_hdr_len = 15;
> const int message_len = 14;
> uint offset = TPM2_HDR_LEN + platform_len + session_hdr_len +
> message_len;
> @@ -103,11 +103,15 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
> /* handles 4 bytes */
> tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */
>
> - /* session header 13 bytes */
> +
> + /* session header 15 bytes */
> + /*null auth session*/
> tpm_u32(9), /* Header size */
> tpm_u32(TPM2_RS_PW), /* Password authorisation */
> tpm_u16(0), /* nonce_size */
> 0, /* session_attrs */
> + tpm_u16(0), /* HMAC size */
> + /*end auth area*/
> tpm_u16(0), /* auth_size */
>
> /* message 14 bytes + policy */
> @@ -136,6 +140,35 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
> return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
> }
>
> +u32 tpm2_nv_undefine_space(struct udevice *dev, u32 space_index) {
> +
> + const int platform_len = sizeof(u32);
> + const int session_hdr_len = 13;
> + const int message_len = 4;
> + u8 command_v2[COMMAND_BUFFER_SIZE] = {
> + /* header 10 bytes */
> + tpm_u16(TPM2_ST_SESSIONS), /* TAG */
> + tpm_u32(TPM2_HDR_LEN + platform_len + session_hdr_len +
> + message_len),/* Length - header + provision + index + auth area*/
> + tpm_u32(TPM2_CC_NV_UNDEFINE_SPACE),/* Command code */
> +
> + /* handles 4 bytes */
> + tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */
> + /* nv_index */
> + tpm_u32(space_index),
> +
> + /*null auth session*/
> + tpm_u32(9), /* Header size */
> + tpm_u32(TPM2_RS_PW), /* Password authorisation FIXME: allow PCR authorization */
> + tpm_u16(0), /* nonce_size */
> + 0, /* session_attrs */
> + tpm_u16(0), /* HMAC size */
> + /*end auth area*/
> +
> + };
> + return tpm_sendrecv_command(dev, command_v2, NULL, NULL);
> +}
> +
> u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm,
> const u8 *digest, u32 digest_len)
> {
> @@ -184,22 +217,23 @@ u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count)
> u8 command_v2[COMMAND_BUFFER_SIZE] = {
> /* header 10 bytes */
> tpm_u16(TPM2_ST_SESSIONS), /* TAG */
> - tpm_u32(10 + 8 + 4 + 9 + 4), /* Length */
> + tpm_u32(TPM2_HDR_LEN + 8 + 4 + 9 + 4), /* Length */
I would *really* love to get rid of the magic values (8, 4 etc) and
define them to something readable. But this is not a problem of your
patch, we can fix it later.
> tpm_u32(TPM2_CC_NV_READ), /* Command code */
>
> /* handles 8 bytes */
> tpm_u32(TPM2_RH_PLATFORM), /* Primary platform seed */
[...]
[0] https://docs.u-boot.org/en/latest/develop/sending_patches.html
/Ilias
More information about the U-Boot
mailing list