[PATCH 0/9] tpm: Enhance sandbox tpm2 emulation
At present the TPM2 emulator lacks the ability to load and save the state. This means it cannot be used for verify-boot flow that includes multiple phases (e.g. VPL and SPL). It also lacks support for non-volatile data storage.
This series adds these features to the TPM2 emulator, with some code from TPM1 moving into a common file.
A few other clean-ups are included to make the two emulators more similar.
Simon Glass (9): sandbox: tpm: Split out common nvdata code sandbox: tpm: Tidy up reading and writing of device state sandbox: tpm: Support the define-space command sandbox: tpm: Correct handling of get-capability sandbox: tpm: Finish comments for struct sandbox_tpm2 sandbox: tpm: Track whether the state is valid sandbox: tpm: Support nvdata in TPM2 sandbox: tpm: Support storing device state in tpm2 sandbox: tpm: Support extending a PCR multiple times
drivers/tpm/Makefile | 4 +- drivers/tpm/sandbox_common.c | 77 ++++++++++ drivers/tpm/sandbox_common.h | 108 ++++++++++++++ drivers/tpm/tpm2_tis_sandbox.c | 256 +++++++++++++++++++++++++++++++-- drivers/tpm/tpm_tis_sandbox.c | 171 ++++++---------------- include/tpm-v2.h | 2 + 6 files changed, 479 insertions(+), 139 deletions(-) create mode 100644 drivers/tpm/sandbox_common.c create mode 100644 drivers/tpm/sandbox_common.h
We want to support nvdata in TPM2 as well. To avoid code duplicating the associated code, move it into a common file.
Drop the special-case logic for the kernel space. This can be handled by the higher-level code now, i.e. in vboot itself.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/Makefile | 4 +- drivers/tpm/sandbox_common.c | 66 ++++++++++++++++++++ drivers/tpm/sandbox_common.h | 96 +++++++++++++++++++++++++++++ drivers/tpm/tpm_tis_sandbox.c | 111 +++------------------------------- 4 files changed, 172 insertions(+), 105 deletions(-) create mode 100644 drivers/tpm/sandbox_common.c create mode 100644 drivers/tpm/sandbox_common.h
diff --git a/drivers/tpm/Makefile b/drivers/tpm/Makefile index f64d20067f8..c65be526700 100644 --- a/drivers/tpm/Makefile +++ b/drivers/tpm/Makefile @@ -6,11 +6,11 @@ obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm-uclass.o obj-$(CONFIG_TPM_ATMEL_TWI) += tpm_atmel_twi.o obj-$(CONFIG_TPM_TIS_INFINEON) += tpm_tis_infineon.o obj-$(CONFIG_TPM_TIS_LPC) += tpm_tis_lpc.o -obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o +obj-$(CONFIG_TPM_TIS_SANDBOX) += tpm_tis_sandbox.o sandbox_common.o obj-$(CONFIG_TPM_ST33ZP24_I2C) += tpm_tis_st33zp24_i2c.o obj-$(CONFIG_TPM_ST33ZP24_SPI) += tpm_tis_st33zp24_spi.o
obj-$(CONFIG_$(SPL_TPL_)TPM2_CR50_I2C) += cr50_i2c.o -obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o +obj-$(CONFIG_TPM2_TIS_SANDBOX) += tpm2_tis_sandbox.o sandbox_common.o obj-$(CONFIG_TPM2_TIS_SPI) += tpm2_tis_spi.o obj-$(CONFIG_TPM2_FTPM_TEE) += tpm2_ftpm_tee.o diff --git a/drivers/tpm/sandbox_common.c b/drivers/tpm/sandbox_common.c new file mode 100644 index 00000000000..13f5e030a5f --- /dev/null +++ b/drivers/tpm/sandbox_common.c @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Common features for sandbox TPM1 and TPM2 implementations + * + * Copyright 2021 Google LLC + */ + +#define LOG_CATEGORY UCLASS_TPM + +#include <common.h> +#include <tpm-v1.h> +#include <tpm-v2.h> +#include <asm/unaligned.h> +#include "sandbox_common.h" + +#define TPM_ERR_CODE_OFS (2 + 4) /* after tag and size */ + +int sb_tpm_index_to_seq(u32 index) +{ + index &= ~HR_NV_INDEX; + switch (index) { + case FIRMWARE_NV_INDEX: + return NV_SEQ_FIRMWARE; + case KERNEL_NV_INDEX: + return NV_SEQ_KERNEL; + case BACKUP_NV_INDEX: + return NV_SEQ_BACKUP; + case FWMP_NV_INDEX: + return NV_SEQ_FWMP; + case MRC_REC_HASH_NV_INDEX: + return NV_SEQ_REC_HASH; + case 0: + return NV_SEQ_GLOBAL_LOCK; + case TPM_NV_INDEX_LOCK: + return NV_SEQ_ENABLE_LOCKING; + } + + printf("Invalid nv index %#x\n", index); + return -1; +} + +void sb_tpm_read_data(const struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, u8 *buf, int data_ofs, + int length) +{ + const struct nvdata_state *nvd = &nvdata[seq]; + + if (!nvd->present) + put_unaligned_be32(TPM_BADINDEX, buf + TPM_ERR_CODE_OFS); + else if (length > nvd->length) + put_unaligned_be32(TPM_BAD_DATASIZE, buf + TPM_ERR_CODE_OFS); + else + memcpy(buf + data_ofs, &nvd->data, length); +} + +void sb_tpm_write_data(struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, const u8 *buf, int data_ofs, + int length) +{ + struct nvdata_state *nvd = &nvdata[seq]; + + if (length > nvd->length) + log_err("Invalid length %x (max %x)\n", length, nvd->length); + else + memcpy(&nvdata[seq].data, buf + data_ofs, length); +} diff --git a/drivers/tpm/sandbox_common.h b/drivers/tpm/sandbox_common.h new file mode 100644 index 00000000000..aa5292d7945 --- /dev/null +++ b/drivers/tpm/sandbox_common.h @@ -0,0 +1,96 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Common features for sandbox TPM1 and TPM2 implementations + * + * Copyright 2021 Google LLC + */ + +#ifndef __TPM_SANDBOX_COMMON_H +#define __TPM_SANDBOX_COMMON_H + +/* + * These numbers derive from adding the sizes of command fields as shown in + * the TPM commands manual. + */ +#define TPM_HDR_LEN 10 + +/* These are the different non-volatile spaces that we emulate */ +enum sandbox_nv_space { + NV_SEQ_ENABLE_LOCKING, + NV_SEQ_GLOBAL_LOCK, + NV_SEQ_FIRMWARE, + NV_SEQ_KERNEL, + NV_SEQ_BACKUP, + NV_SEQ_FWMP, + NV_SEQ_REC_HASH, + + NV_SEQ_COUNT, +}; + +/* TPM NVRAM location indices */ +#define FIRMWARE_NV_INDEX 0x1007 +#define KERNEL_NV_INDEX 0x1008 +#define BACKUP_NV_INDEX 0x1009 +#define FWMP_NV_INDEX 0x100a +#define MRC_REC_HASH_NV_INDEX 0x100b + +/* Size of each non-volatile space */ +#define NV_DATA_SIZE 0x28 + +/** + * struct nvdata_state - state of a single non-volatile-data 'space' + * + * @present: true if present + * @length: length in bytes (max NV_DATA_SIZE) + * @data: contents of non-volatile space + */ +struct nvdata_state { + bool present; + int length; + u8 data[NV_DATA_SIZE]; +}; + +/** + * sb_tpm_index_to_seq() - convert an index into a space sequence number + * + * This converts the index as used by the vboot code into an internal sequence + * number used by the sandbox emulation. + * + * @index: Index to use (FIRMWARE_NV_INDEX, etc.) + * @return associated space (enum sandbox_nv_space) + */ +int sb_tpm_index_to_seq(uint index); + +/** + * sb_tpm_read_data() - Read non-volatile data + * + * This handles a TPM read of nvdata. If the nvdata is not present, a + * TPM_BADINDEX error is put in the buffer. If @length is too large, + * TPM_BAD_DATASIZE is put in the buffer. + * + * @nvdata: Current nvdata state + * @seq: Sequence number to read + * @recvbuf: Buffer to update with the TPM response, assumed to contain zeroes + * @data_ofs: Offset of the 'data' portion of @recvbuf + * @length: Number of bytes to read + */ +void sb_tpm_read_data(const struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, u8 *recvbuf, int data_ofs, + int length); + +/** + * sb_tpm_write_data() - Write non-volatile data + * + * If @length is too large, an error is logged and nothing is written. + * + * @nvdata: Current nvdata state + * @seq: Sequence number to read + * @buf: Buffer containing the data to write + * @data_ofs: Offset of the 'data' portion of @buf + * @length: Number of bytes to write + */ +void sb_tpm_write_data(struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, const u8 *buf, int data_ofs, + int length); + +#endif diff --git a/drivers/tpm/tpm_tis_sandbox.c b/drivers/tpm/tpm_tis_sandbox.c index 67139cea3be..294d98da606 100644 --- a/drivers/tpm/tpm_tis_sandbox.c +++ b/drivers/tpm/tpm_tis_sandbox.c @@ -9,61 +9,10 @@ #include <asm/state.h> #include <asm/unaligned.h> #include <u-boot/crc.h> - -/* TPM NVRAM location indices. */ -#define FIRMWARE_NV_INDEX 0x1007 -#define KERNEL_NV_INDEX 0x1008 -#define BACKUP_NV_INDEX 0x1009 -#define FWMP_NV_INDEX 0x100a -#define REC_HASH_NV_INDEX 0x100b -#define REC_HASH_NV_SIZE VB2_SHA256_DIGEST_SIZE +#include "sandbox_common.h"
#define NV_DATA_PUBLIC_PERMISSIONS_OFFSET 60
-/* Kernel TPM space - KERNEL_NV_INDEX, locked with physical presence */ -#define ROLLBACK_SPACE_KERNEL_VERSION 2 -#define ROLLBACK_SPACE_KERNEL_UID 0x4752574C /* 'GRWL' */ - -struct rollback_space_kernel { - /* Struct version, for backwards compatibility */ - uint8_t struct_version; - /* Unique ID to detect space redefinition */ - uint32_t uid; - /* Kernel versions */ - uint32_t kernel_versions; - /* Reserved for future expansion */ - uint8_t reserved[3]; - /* Checksum (v2 and later only) */ - uint8_t crc8; -} __packed rollback_space_kernel; - -/* - * These numbers derive from adding the sizes of command fields as shown in - * the TPM commands manual. - */ -#define TPM_REQUEST_HEADER_LENGTH 10 -#define TPM_RESPONSE_HEADER_LENGTH 10 - -/* These are the different non-volatile spaces that we emulate */ -enum { - NV_GLOBAL_LOCK, - NV_SEQ_FIRMWARE, - NV_SEQ_KERNEL, - NV_SEQ_BACKUP, - NV_SEQ_FWMP, - NV_SEQ_REC_HASH, - - NV_SEQ_COUNT, -}; - -/* Size of each non-volatile space */ -#define NV_DATA_SIZE 0x20 - -struct nvdata_state { - bool present; - u8 data[NV_DATA_SIZE]; -}; - /* * Information about our TPM emulation. This is preserved in the sandbox * state file if enabled. @@ -140,27 +89,6 @@ static int sandbox_tpm_write_state(void *blob, int node) SANDBOX_STATE_IO(sandbox_tpm, "google,sandbox-tpm", sandbox_tpm_read_state, sandbox_tpm_write_state);
-static int index_to_seq(uint32_t index) -{ - switch (index) { - case FIRMWARE_NV_INDEX: - return NV_SEQ_FIRMWARE; - case KERNEL_NV_INDEX: - return NV_SEQ_KERNEL; - case BACKUP_NV_INDEX: - return NV_SEQ_BACKUP; - case FWMP_NV_INDEX: - return NV_SEQ_FWMP; - case REC_HASH_NV_INDEX: - return NV_SEQ_REC_HASH; - case 0: - return NV_GLOBAL_LOCK; - } - - printf("Invalid nv index %#x\n", index); - return -1; -} - static void handle_cap_flag_space(u8 **datap, uint index) { struct tpm_nv_data_public pub; @@ -246,48 +174,25 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, case TPM_CMD_NV_WRITE_VALUE: index = get_unaligned_be32(sendbuf + 10); length = get_unaligned_be32(sendbuf + 18); - seq = index_to_seq(index); + seq = sb_tpm_index_to_seq(index); if (seq < 0) return -EINVAL; printf("tpm: nvwrite index=%#02x, len=%#02x\n", index, length); - memcpy(&tpm->nvdata[seq].data, sendbuf + 22, length); - tpm->nvdata[seq].present = true; - *recv_len = 12; - memset(recvbuf, '\0', *recv_len); + sb_tpm_write_data(tpm->nvdata, seq, sendbuf, 22, length); break; case TPM_CMD_NV_READ_VALUE: /* nvread */ index = get_unaligned_be32(sendbuf + 10); length = get_unaligned_be32(sendbuf + 18); - seq = index_to_seq(index); + seq = sb_tpm_index_to_seq(index); if (seq < 0) return -EINVAL; printf("tpm: nvread index=%#02x, len=%#02x, seq=%#02x\n", index, length, seq); - *recv_len = TPM_RESPONSE_HEADER_LENGTH + sizeof(uint32_t) + - length; + *recv_len = TPM_HDR_LEN + sizeof(uint32_t) + length; memset(recvbuf, '\0', *recv_len); - put_unaligned_be32(length, recvbuf + - TPM_RESPONSE_HEADER_LENGTH); - if (seq == NV_SEQ_KERNEL) { - struct rollback_space_kernel rsk; - - data = recvbuf + TPM_RESPONSE_HEADER_LENGTH + - sizeof(uint32_t); - memset(&rsk, 0, sizeof(struct rollback_space_kernel)); - rsk.struct_version = 2; - rsk.uid = ROLLBACK_SPACE_KERNEL_UID; - rsk.crc8 = crc8(0, (unsigned char *)&rsk, - offsetof(struct rollback_space_kernel, - crc8)); - memcpy(data, &rsk, sizeof(rsk)); - } else if (!tpm->nvdata[seq].present) { - put_unaligned_be32(TPM_BADINDEX, recvbuf + - sizeof(uint16_t) + sizeof(uint32_t)); - } else { - memcpy(recvbuf + TPM_RESPONSE_HEADER_LENGTH + - sizeof(uint32_t), &tpm->nvdata[seq].data, - length); - } + put_unaligned_be32(length, recvbuf + TPM_HDR_LEN); + sb_tpm_read_data(tpm->nvdata, seq, recvbuf, TPM_HDR_LEN + 4, + length); break; case TPM_CMD_EXTEND: *recv_len = 30;
At present this code assumes that the TPM data has been read but this may not be the case. Refactor the code to use a separate pointer so we know the current state of the data.
Add error checking for the data size.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm_tis_sandbox.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/tpm/tpm_tis_sandbox.c b/drivers/tpm/tpm_tis_sandbox.c index 294d98da606..f22ed846f0a 100644 --- a/drivers/tpm/tpm_tis_sandbox.c +++ b/drivers/tpm/tpm_tis_sandbox.c @@ -20,7 +20,7 @@ static struct tpm_state { bool valid; struct nvdata_state nvdata[NV_SEQ_COUNT]; -} g_state; +} s_state, *g_state;
/** * sandbox_tpm_read_state() - read the sandbox EC state from the state file @@ -33,6 +33,7 @@ static struct tpm_state { */ static int sandbox_tpm_read_state(const void *blob, int node) { + struct tpm_state *state = &s_state; const char *prop; int len; int i; @@ -41,22 +42,27 @@ static int sandbox_tpm_read_state(const void *blob, int node) return 0;
for (i = 0; i < NV_SEQ_COUNT; i++) { + struct nvdata_state *nvd = &state->nvdata[i]; char prop_name[20];
sprintf(prop_name, "nvdata%d", i); prop = fdt_getprop(blob, node, prop_name, &len); - if (prop && len == NV_DATA_SIZE) { - memcpy(g_state.nvdata[i].data, prop, NV_DATA_SIZE); - g_state.nvdata[i].present = true; + if (len >= NV_DATA_SIZE) + return log_msg_ret("nvd", -E2BIG); + if (prop) { + memcpy(nvd->data, prop, len); + nvd->length = len; + nvd->present = true; } } - g_state.valid = true; + + s_state.valid = true;
return 0; }
/** - * cros_ec_write_state() - Write out our state to the state file + * sandbox_tpm_write_state() - Write out our state to the state file * * The caller will ensure that there is a node ready for the state. The node * may already contain the old state, in which case it is overridden. @@ -66,20 +72,25 @@ static int sandbox_tpm_read_state(const void *blob, int node) */ static int sandbox_tpm_write_state(void *blob, int node) { + const struct tpm_state *state = g_state; int i;
+ if (!state) + return 0; + /* * We are guaranteed enough space to write basic properties. * We could use fdt_add_subnode() to put each set of data in its * own node - perhaps useful if we add access informaiton to each. */ for (i = 0; i < NV_SEQ_COUNT; i++) { + const struct nvdata_state *nvd = &state->nvdata[i]; char prop_name[20];
- if (g_state.nvdata[i].present) { - sprintf(prop_name, "nvdata%d", i); - fdt_setprop(blob, node, prop_name, - g_state.nvdata[i].data, NV_DATA_SIZE); + if (nvd->present) { + snprintf(prop_name, sizeof(prop_name), "nvdata%d", i); + fdt_setprop(blob, node, prop_name, nvd->data, + nvd->length); } }
@@ -233,7 +244,9 @@ static int sandbox_tpm_probe(struct udevice *dev) { struct tpm_state *tpm = dev_get_priv(dev);
- memcpy(tpm, &g_state, sizeof(*tpm)); + if (s_state.valid) + memcpy(tpm, &s_state, sizeof(*tpm)); + g_state = tpm;
return 0; }
Add support for this command, moving away from the previous approach of hard-coding the initial data in the driver, now that the kernel-space data has to be set up by the higher-level vboot code.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/sandbox_common.c | 11 +++++++++++ drivers/tpm/sandbox_common.h | 12 ++++++++++++ drivers/tpm/tpm_tis_sandbox.c | 11 +++++++++++ 3 files changed, 34 insertions(+)
diff --git a/drivers/tpm/sandbox_common.c b/drivers/tpm/sandbox_common.c index 13f5e030a5f..7e0b2502e35 100644 --- a/drivers/tpm/sandbox_common.c +++ b/drivers/tpm/sandbox_common.c @@ -64,3 +64,14 @@ void sb_tpm_write_data(struct nvdata_state nvdata[NV_SEQ_COUNT], else memcpy(&nvdata[seq].data, buf + data_ofs, length); } + +void sb_tpm_define_data(struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, int length) +{ + struct nvdata_state *nvd = &nvdata[seq]; + + if (length > NV_DATA_SIZE) + log_err("Invalid length %x (max %x)\n", length, NV_DATA_SIZE); + nvd->length = length; + nvd->present = true; +} diff --git a/drivers/tpm/sandbox_common.h b/drivers/tpm/sandbox_common.h index aa5292d7945..e822a200fd3 100644 --- a/drivers/tpm/sandbox_common.h +++ b/drivers/tpm/sandbox_common.h @@ -93,4 +93,16 @@ void sb_tpm_write_data(struct nvdata_state nvdata[NV_SEQ_COUNT], enum sandbox_nv_space seq, const u8 *buf, int data_ofs, int length);
+/** + * sb_tpm_define_data() - Set up non-volatile data + * + * If @length is too large, an error is logged and nothing is written. + * + * @nvdata: Current nvdata state + * @seq: Sequence number to set up + * @length: Length of space in bytes + */ +void sb_tpm_define_data(struct nvdata_state nvdata[NV_SEQ_COUNT], + enum sandbox_nv_space seq, int length); + #endif diff --git a/drivers/tpm/tpm_tis_sandbox.c b/drivers/tpm/tpm_tis_sandbox.c index f22ed846f0a..85b22afa4d9 100644 --- a/drivers/tpm/tpm_tis_sandbox.c +++ b/drivers/tpm/tpm_tis_sandbox.c @@ -210,6 +210,17 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, memset(recvbuf, '\0', *recv_len); break; case TPM_CMD_NV_DEFINE_SPACE: + index = get_unaligned_be32(sendbuf + 12); + length = get_unaligned_be32(sendbuf + 77); + seq = sb_tpm_index_to_seq(index); + if (seq < 0) + return -EINVAL; + printf("tpm: define_space index=%#02x, len=%#02x, seq=%#02x\n", + index, length, seq); + sb_tpm_define_data(tpm->nvdata, seq, length); + *recv_len = 12; + memset(recvbuf, '\0', *recv_len); + break; case 0x15: /* pcr read */ case 0x5d: /* force clear */ case 0x6f: /* physical enable */
This function current handles the kernel case incorrectly. Fix it, and use the shorter TPM_HDR_LEN while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm_tis_sandbox.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/tpm/tpm_tis_sandbox.c b/drivers/tpm/tpm_tis_sandbox.c index 85b22afa4d9..efbeb00ab63 100644 --- a/drivers/tpm/tpm_tis_sandbox.c +++ b/drivers/tpm/tpm_tis_sandbox.c @@ -140,16 +140,13 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, printf("Get flags index %#02x\n", index); *recv_len = 22; memset(recvbuf, '\0', *recv_len); - data = recvbuf + TPM_RESPONSE_HEADER_LENGTH + - sizeof(uint32_t); + data = recvbuf + TPM_HDR_LEN + sizeof(uint32_t); switch (index) { case FIRMWARE_NV_INDEX: break; case KERNEL_NV_INDEX: handle_cap_flag_space(&data, index); - *recv_len = data - recvbuf - - TPM_RESPONSE_HEADER_LENGTH - - sizeof(uint32_t); + *recv_len = data - recvbuf; break; case TPM_CAP_FLAG_PERMANENT: { struct tpm_permanent_flags *pflags; @@ -166,15 +163,12 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, printf(" ** Unknown flags index %x\n", index); return -ENOSYS; } - put_unaligned_be32(*recv_len, - recvbuf + - TPM_RESPONSE_HEADER_LENGTH); + put_unaligned_be32(*recv_len, recvbuf + TPM_HDR_LEN); break; case TPM_CAP_NV_INDEX: index = get_unaligned_be32(sendbuf + 18); printf("Get cap nv index %#02x\n", index); - put_unaligned_be32(22, recvbuf + - TPM_RESPONSE_HEADER_LENGTH); + put_unaligned_be32(22, recvbuf + TPM_HDR_LEN); break; default: printf(" ** Unknown 0x65 command type %#02x\n",
Hi Simon,
On Mon, Jul 05, 2021 at 09:48:44AM -0600, Simon Glass wrote:
This function current handles the kernel case incorrectly. Fix it, and use the shorter TPM_HDR_LEN while we are here.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/tpm_tis_sandbox.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/tpm/tpm_tis_sandbox.c b/drivers/tpm/tpm_tis_sandbox.c index 85b22afa4d9..efbeb00ab63 100644 --- a/drivers/tpm/tpm_tis_sandbox.c +++ b/drivers/tpm/tpm_tis_sandbox.c @@ -140,16 +140,13 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, printf("Get flags index %#02x\n", index); *recv_len = 22; memset(recvbuf, '\0', *recv_len);
data = recvbuf + TPM_RESPONSE_HEADER_LENGTH +sizeof(uint32_t);
data = recvbuf + TPM_HDR_LEN + sizeof(uint32_t); switch (index) { case FIRMWARE_NV_INDEX: break; case KERNEL_NV_INDEX: handle_cap_flag_space(&data, index);
*recv_len = data - recvbuf -TPM_RESPONSE_HEADER_LENGTH -sizeof(uint32_t);
*recv_len = data - recvbuf;
This is the only actual fix right? TPM_RESPONSE_HEADER_LENGTH and TPM_HDR_LEN are both defined to 10? Is it worth updating the commit message with exactly the problem that was fixed?
break; case TPM_CAP_FLAG_PERMANENT: { struct tpm_permanent_flags *pflags;@@ -166,15 +163,12 @@ static int sandbox_tpm_xfer(struct udevice *dev, const uint8_t *sendbuf, printf(" ** Unknown flags index %x\n", index); return -ENOSYS; }
put_unaligned_be32(*recv_len,recvbuf +TPM_RESPONSE_HEADER_LENGTH);
put_unaligned_be32(*recv_len, recvbuf + TPM_HDR_LEN); break;
put_unaligned_be32(22, recvbuf +TPM_RESPONSE_HEADER_LENGTH);
put_unaligned_be32(22, recvbuf + TPM_HDR_LEN); break;-- 2.32.0.93.g670b81a890-goog
Tidy up the missing comments for this struct.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm2_tis_sandbox.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index 24c804a5645..5e0bd304699 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -45,19 +45,31 @@ static const u8 sandbox_extended_once_pcr[] = { 0xea, 0x98, 0x31, 0xa9, 0x27, 0x59, 0xfb, 0x4b, };
+/* + * Information about our TPM emulation. This is preserved in the sandbox + * state file if enabled. + * + * @init_done: true if open() has been called + * @startup_done: true if TPM2_CC_STARTUP has been processed + * @tests_done: true if TPM2_CC_SELF_TEST has be processed + * @pw: TPM password per hierarchy + * @pw_sz: Size of each password in bytes + * @properties: TPM properties + * @pcr: TPM Platform Configuration Registers. Each of these holds a hash and + * can be 'extended' a number of times, meaning another hash is added into + * its value (initial value all zeroes) + * @pcr_extensions: Number of times each PCR has been extended (starts at 0) + * @nvdata: non-volatile data, used to store important things for the platform + */ struct sandbox_tpm2 { /* TPM internal states */ bool init_done; bool startup_done; bool tests_done; - /* TPM password per hierarchy */ char pw[TPM2_HIERARCHY_NB][TPM2_DIGEST_LEN + 1]; int pw_sz[TPM2_HIERARCHY_NB]; - /* TPM properties */ u32 properties[TPM2_PROPERTY_NB]; - /* TPM PCRs */ u8 pcr[SANDBOX_TPM_PCR_NB][TPM2_DIGEST_LEN]; - /* TPM PCR extensions */ u32 pcr_extensions[SANDBOX_TPM_PCR_NB]; };
On Mon, Jul 05, 2021 at 09:48:45AM -0600, Simon Glass wrote:
Tidy up the missing comments for this struct.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/tpm2_tis_sandbox.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index 24c804a5645..5e0bd304699 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -45,19 +45,31 @@ static const u8 sandbox_extended_once_pcr[] = { 0xea, 0x98, 0x31, 0xa9, 0x27, 0x59, 0xfb, 0x4b, };
+/*
- Information about our TPM emulation. This is preserved in the sandbox
- state file if enabled.
- @init_done: true if open() has been called
- @startup_done: true if TPM2_CC_STARTUP has been processed
- @tests_done: true if TPM2_CC_SELF_TEST has be processed
- @pw: TPM password per hierarchy
- @pw_sz: Size of each password in bytes
- @properties: TPM properties
- @pcr: TPM Platform Configuration Registers. Each of these holds a hash and
- can be 'extended' a number of times, meaning another hash is added into
- its value (initial value all zeroes)
- @pcr_extensions: Number of times each PCR has been extended (starts at 0)
- @nvdata: non-volatile data, used to store important things for the platform
- */
struct sandbox_tpm2 { /* TPM internal states */ bool init_done; bool startup_done; bool tests_done;
- /* TPM password per hierarchy */ char pw[TPM2_HIERARCHY_NB][TPM2_DIGEST_LEN + 1]; int pw_sz[TPM2_HIERARCHY_NB];
- /* TPM properties */ u32 properties[TPM2_PROPERTY_NB];
- /* TPM PCRs */ u8 pcr[SANDBOX_TPM_PCR_NB][TPM2_DIGEST_LEN];
- /* TPM PCR extensions */ u32 pcr_extensions[SANDBOX_TPM_PCR_NB];
};
-- 2.32.0.93.g670b81a890-goog
Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Add checking as to whether the current TPM state is valid, so we can implement reading/writing the state.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm2_tis_sandbox.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index 5e0bd304699..c287ca2278f 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -49,6 +49,7 @@ static const u8 sandbox_extended_once_pcr[] = { * Information about our TPM emulation. This is preserved in the sandbox * state file if enabled. * + * @valid: true if this is valid (only used in s_state) * @init_done: true if open() has been called * @startup_done: true if TPM2_CC_STARTUP has been processed * @tests_done: true if TPM2_CC_SELF_TEST has be processed @@ -62,6 +63,7 @@ static const u8 sandbox_extended_once_pcr[] = { * @nvdata: non-volatile data, used to store important things for the platform */ struct sandbox_tpm2 { + bool valid; /* TPM internal states */ bool init_done; bool startup_done; @@ -73,6 +75,8 @@ struct sandbox_tpm2 { u32 pcr_extensions[SANDBOX_TPM_PCR_NB]; };
+static struct sandbox_tpm2 s_state, *g_state; + /* * Check the tag validity depending on the command (authentication required or * not). If authentication is required, check it is valid. Update the auth @@ -606,11 +610,13 @@ static int sandbox_tpm2_probe(struct udevice *dev) /* Use the TPM v2 stack */ priv->version = TPM_V2;
- memset(tpm, 0, sizeof(*tpm)); - priv->pcr_count = 32; priv->pcr_select_min = 2;
+ if (s_state.valid) + memcpy(tpm, &s_state, sizeof(*tpm)); + g_state = tpm; + return 0; }
Add support for this feature in the TPM2 emulator, to support Chromium OS vboot.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm2_tis_sandbox.c | 68 ++++++++++++++++++++++++++++++++++ include/tpm-v2.h | 2 + 2 files changed, 70 insertions(+)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index c287ca2278f..1d38a79a867 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include "sandbox_common.h"
/* Hierarchies */ enum tpm2_hierarchy { @@ -73,6 +74,7 @@ struct sandbox_tpm2 { u32 properties[TPM2_PROPERTY_NB]; u8 pcr[SANDBOX_TPM_PCR_NB][TPM2_DIGEST_LEN]; u32 pcr_extensions[SANDBOX_TPM_PCR_NB]; + struct nvdata_state nvdata[NV_SEQ_COUNT]; };
static struct sandbox_tpm2 s_state, *g_state; @@ -109,6 +111,10 @@ static int sandbox_tpm2_check_session(struct udevice *dev, u32 command, u16 tag, case TPM2_CC_DAM_RESET: case TPM2_CC_DAM_PARAMETERS: case TPM2_CC_PCR_EXTEND: + case TPM2_CC_NV_READ: + case TPM2_CC_NV_WRITE: + case TPM2_CC_NV_WRITELOCK: + case TPM2_CC_NV_DEFINE_SPACE: if (tag != TPM2_ST_SESSIONS) { printf("Session required for command 0x%x\n", command); return TPM2_RC_AUTH_CONTEXT; @@ -137,6 +143,10 @@ static int sandbox_tpm2_check_session(struct udevice *dev, u32 command, u16 tag, break; case TPM2_RH_PLATFORM: *hierarchy = TPM2_HIERARCHY_PLATFORM; + if (command == TPM2_CC_NV_READ || + command == TPM2_CC_NV_WRITE || + command == TPM2_CC_NV_WRITELOCK) + *auth += sizeof(u32); break; default: printf("Wrong handle 0x%x\n", handle); @@ -573,6 +583,64 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 *sendbuf, sandbox_tpm2_fill_buf(recv, recv_len, tag, rc); break;
+ case TPM2_CC_NV_READ: { + int index, seq; + + index = get_unaligned_be32(sendbuf + TPM2_HDR_LEN + 4); + length = get_unaligned_be16(sent); + /* ignore offset */ + seq = sb_tpm_index_to_seq(index); + if (seq < 0) + return log_msg_ret("index", -EINVAL); + printf("tpm: nvread index=%#02x, len=%#02x, seq=%#02x\n", index, + length, seq); + *recv_len = TPM2_HDR_LEN + 6 + length; + memset(recvbuf, '\0', *recv_len); + put_unaligned_be32(length, recvbuf + 2); + sb_tpm_read_data(tpm->nvdata, seq, recvbuf, + TPM2_HDR_LEN + 4 + 2, length); + break; + } + case TPM2_CC_NV_WRITE: { + int index, seq; + + index = get_unaligned_be32(sendbuf + TPM2_HDR_LEN + 4); + length = get_unaligned_be16(sent); + sent += sizeof(u16); + + /* ignore offset */ + seq = sb_tpm_index_to_seq(index); + if (seq < 0) + return log_msg_ret("index", -EINVAL); + printf("tpm: nvwrite index=%#02x, len=%#02x, seq=%#02x\n", index, + length, seq); + memcpy(&tpm->nvdata[seq].data, sent, length); + tpm->nvdata[seq].present = true; + *recv_len = TPM2_HDR_LEN + 2; + memset(recvbuf, '\0', *recv_len); + break; + } + case TPM2_CC_NV_DEFINE_SPACE: { + int policy_size, index, seq; + + policy_size = get_unaligned_be16(sent + 12); + index = get_unaligned_be32(sent + 2); + sent += 14 + policy_size; + length = get_unaligned_be16(sent); + seq = sb_tpm_index_to_seq(index); + if (seq < 0) + return -EINVAL; + printf("tpm: define_space index=%x, len=%x, seq=%x, policy_size=%x\n", + index, length, seq, policy_size); + sb_tpm_define_data(tpm->nvdata, seq, length); + *recv_len = 12; + memset(recvbuf, '\0', *recv_len); + break; + } + case TPM2_CC_NV_WRITELOCK: + *recv_len = 12; + memset(recvbuf, '\0', *recv_len); + 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 247b3869676..949a13c917a 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -32,6 +32,8 @@ struct udevice; #define TPM2_MAX_TPM_PROPERTIES ((TPM2_MAX_CAP_BUFFER - sizeof(u32) /* TPM2_CAP */ - \ sizeof(u32)) / sizeof(struct tpms_tagged_property))
+#define TPM2_HDR_LEN 10 + /* * We deviate from this draft of the specification by increasing the value of * TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2
At present the tpm2 emulator does not support storing the device state. Add this so we can handle the normal vboot flow through the sandbox executables (VPL->SPL etc.) with the TPM contents staying in place.
Note: sandbox has not yet been converted to use livetree for the state information, since livetree does not yet support writing to the tree.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm2_tis_sandbox.c | 139 +++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index 1d38a79a867..ed9c9a0bc9f 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -79,6 +79,145 @@ struct sandbox_tpm2 {
static struct sandbox_tpm2 s_state, *g_state;
+/** + * sandbox_tpm2_read_state() - read the sandbox EC state from the state file + * + * If data is available, then blob and node will provide access to it. If + * not this function sets up an empty TPM. + * + * @blob: Pointer to device tree blob, or NULL if no data to read + * @node: Node offset to read from + */ +static int sandbox_tpm2_read_state(const void *blob, int node) +{ + struct sandbox_tpm2 *state = &s_state; + char prop_name[20]; + const char *prop; + int len; + int i; + + if (!blob) + return 0; + state->tests_done = fdtdec_get_int(blob, node, "tests-done", 0); + + for (i = 0; i < TPM2_HIERARCHY_NB; i++) { + snprintf(prop_name, sizeof(prop_name), "pw%d", i); + + prop = fdt_getprop(blob, node, prop_name, &len); + if (len > TPM2_DIGEST_LEN) + return log_msg_ret("pw", -E2BIG); + if (prop) { + memcpy(state->pw[i], prop, len); + state->pw_sz[i] = len; + } + } + + for (i = 0; i < TPM2_PROPERTY_NB; i++) { + snprintf(prop_name, sizeof(prop_name), "properties%d", i); + state->properties[i] = fdtdec_get_uint(blob, node, prop_name, + 0); + } + + for (i = 0; i < SANDBOX_TPM_PCR_NB; i++) { + int subnode; + + snprintf(prop_name, sizeof(prop_name), "pcr%d", i); + subnode = fdt_subnode_offset(blob, node, prop_name); + if (subnode < 0) + continue; + prop = fdt_getprop(blob, subnode, "value", &len); + if (len != TPM2_DIGEST_LEN) + return log_msg_ret("pcr", -E2BIG); + memcpy(state->pcr[i], prop, TPM2_DIGEST_LEN); + state->pcr_extensions[i] = fdtdec_get_uint(blob, subnode, + "extensions", 0); + } + + for (i = 0; i < NV_SEQ_COUNT; i++) { + struct nvdata_state *nvd = &state->nvdata[i]; + + sprintf(prop_name, "nvdata%d", i); + prop = fdt_getprop(blob, node, prop_name, &len); + if (len > NV_DATA_SIZE) + return log_msg_ret("nvd", -E2BIG); + if (prop) { + memcpy(nvd->data, prop, len); + nvd->length = len; + nvd->present = true; + } + } + s_state.valid = true; + + return 0; +} + +/** + * sandbox_tpm2_write_state() - Write out our state to the state file + * + * The caller will ensure that there is a node ready for the state. The node + * may already contain the old state, in which case it is overridden. + * + * @blob: Device tree blob holding state + * @node: Node to write our state into + */ +static int sandbox_tpm2_write_state(void *blob, int node) +{ + const struct sandbox_tpm2 *state = g_state; + char prop_name[20]; + int i; + + if (!state) + return 0; + + /* + * We are guaranteed enough space to write basic properties. This is + * SANDBOX_STATE_MIN_SPACE. + * + * We could use fdt_add_subnode() to put each set of data in its + * own node - perhaps useful if we add access information to each. + */ + fdt_setprop_u32(blob, node, "tests-done", state->tests_done); + + for (i = 0; i < TPM2_HIERARCHY_NB; i++) { + if (state->pw_sz[i]) { + snprintf(prop_name, sizeof(prop_name), "pw%d", i); + fdt_setprop(blob, node, prop_name, state->pw[i], + state->pw_sz[i]); + } + } + + for (i = 0; i < TPM2_PROPERTY_NB; i++) { + snprintf(prop_name, sizeof(prop_name), "properties%d", i); + fdt_setprop_u32(blob, node, prop_name, state->properties[i]); + } + + for (i = 0; i < SANDBOX_TPM_PCR_NB; i++) { + int subnode; + + snprintf(prop_name, sizeof(prop_name), "pcr%d", i); + subnode = fdt_add_subnode(blob, node, prop_name); + fdt_setprop(blob, subnode, "value", state->pcr[i], + TPM2_DIGEST_LEN); + fdt_setprop_u32(blob, subnode, "extensions", + state->pcr_extensions[i]); + } + + for (i = 0; i < NV_SEQ_COUNT; i++) { + const struct nvdata_state *nvd = &state->nvdata[i]; + + if (nvd->present) { + snprintf(prop_name, sizeof(prop_name), "nvdata%d", i); + fdt_setprop(blob, node, prop_name, nvd->data, + nvd->length); + } + } + + return 0; +} + +SANDBOX_STATE_IO(sandbox_tpm2, "sandbox,tpm2", sandbox_tpm2_read_state, + sandbox_tpm2_write_state); + /* * Check the tag validity depending on the command (authentication required or * not). If authentication is required, check it is valid. Update the auth
It is fairly easy to handle this case and it makes the emulator more useful, since PCRs are commonly extended several times.
Add support for this, using U-Boot's sha256 support.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index ed9c9a0bc9f..e84bcc7c4c8 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include <u-boot/sha256.h> #include "sandbox_common.h"
/* Hierarchies */ @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, const u8 *extension) { struct sandbox_tpm2 *tpm = dev_get_priv(dev); - int i;
- /* Only simulate the first extensions from all '0' with only '0' */ - for (i = 0; i < TPM2_DIGEST_LEN; i++) - if (tpm->pcr[pcr_index][i] || extension[i]) - return TPM2_RC_FAILURE; + if (!pcr_index) { + memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, + TPM2_DIGEST_LEN); + } else { + sha256_context ctx; + + sha256_starts(&ctx); + sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN); + sha256_update(&ctx, extension, TPM2_DIGEST_LEN); + sha256_finish(&ctx, tpm->pcr[pcr_index]); + }
- memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr, - TPM2_DIGEST_LEN); tpm->pcr_extensions[pcr_index]++;
return 0;
On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote:
It is fairly easy to handle this case and it makes the emulator more useful, since PCRs are commonly extended several times.
Add support for this, using U-Boot's sha256 support.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index ed9c9a0bc9f..e84bcc7c4c8 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include <u-boot/sha256.h> #include "sandbox_common.h"
/* Hierarchies */ @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, const u8 *extension) { struct sandbox_tpm2 *tpm = dev_get_priv(dev);
int i;
/* Only simulate the first extensions from all '0' with only '0' */
for (i = 0; i < TPM2_DIGEST_LEN; i++)
if (tpm->pcr[pcr_index][i] || extension[i])return TPM2_RC_FAILURE;
- if (!pcr_index) {
memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr,TPM2_DIGEST_LEN);
Why do we have to treat pcr 0 specially? Can't we just get rid of sandbox_extended_once_pcr and just extend all the PCRs with the value wee get?
- } else {
sha256_context ctx;sha256_starts(&ctx);sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN);sha256_update(&ctx, extension, TPM2_DIGEST_LEN);sha256_finish(&ctx, tpm->pcr[pcr_index]);- }
memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr,
TPM2_DIGEST_LEN);tpm->pcr_extensions[pcr_index]++;
return 0;
-- 2.32.0.93.g670b81a890-goog
On Thu, 15 Jul 2021 at 22:04, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote:
It is fairly easy to handle this case and it makes the emulator more useful, since PCRs are commonly extended several times.
Add support for this, using U-Boot's sha256 support.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index ed9c9a0bc9f..e84bcc7c4c8 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include <u-boot/sha256.h> #include "sandbox_common.h"
/* Hierarchies */ @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, const u8 *extension) { struct sandbox_tpm2 *tpm = dev_get_priv(dev);
int i;/* Only simulate the first extensions from all '0' with only '0' */for (i = 0; i < TPM2_DIGEST_LEN; i++)if (tpm->pcr[pcr_index][i] || extension[i])return TPM2_RC_FAILURE;
if (!pcr_index) {memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr,TPM2_DIGEST_LEN);Why do we have to treat pcr 0 specially? Can't we just get rid of sandbox_extended_once_pcr and just extend all the PCRs with the value wee get?
Also looking at the entire series, SANDBOX_TPM_PCR_NB doesn't seem to change. I don't know too much about the tpm sandbox, but that effectively limits us to a single PCR?
} else {sha256_context ctx;sha256_starts(&ctx);sha256_update(&ctx, tpm->pcr[pcr_index], TPM2_DIGEST_LEN);sha256_update(&ctx, extension, TPM2_DIGEST_LEN);sha256_finish(&ctx, tpm->pcr[pcr_index]);}
memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr,TPM2_DIGEST_LEN); tpm->pcr_extensions[pcr_index]++; return 0;-- 2.32.0.93.g670b81a890-goog
Hi Ilias,
On Thu, 15 Jul 2021 at 13:21, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Thu, 15 Jul 2021 at 22:04, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Mon, Jul 05, 2021 at 09:48:49AM -0600, Simon Glass wrote:
It is fairly easy to handle this case and it makes the emulator more useful, since PCRs are commonly extended several times.
Add support for this, using U-Boot's sha256 support.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/tpm/tpm2_tis_sandbox.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c index ed9c9a0bc9f..e84bcc7c4c8 100644 --- a/drivers/tpm/tpm2_tis_sandbox.c +++ b/drivers/tpm/tpm2_tis_sandbox.c @@ -11,6 +11,7 @@ #include <asm/unaligned.h> #include <linux/bitops.h> #include <u-boot/crc.h> +#include <u-boot/sha256.h> #include "sandbox_common.h"
/* Hierarchies */ @@ -407,15 +408,19 @@ static int sandbox_tpm2_extend(struct udevice *dev, int pcr_index, const u8 *extension) { struct sandbox_tpm2 *tpm = dev_get_priv(dev);
int i;/* Only simulate the first extensions from all '0' with only '0' */for (i = 0; i < TPM2_DIGEST_LEN; i++)if (tpm->pcr[pcr_index][i] || extension[i])return TPM2_RC_FAILURE;
if (!pcr_index) {memcpy(tpm->pcr[pcr_index], sandbox_extended_once_pcr,TPM2_DIGEST_LEN);Why do we have to treat pcr 0 specially? Can't we just get rid of sandbox_extended_once_pcr and just extend all the PCRs with the value wee get?
Also looking at the entire series, SANDBOX_TPM_PCR_NB doesn't seem to change. I don't know too much about the tpm sandbox, but that effectively limits us to a single PCR?
I did send an updated series.
That's right, only one PCR is currently used by the tests.
Regards, Simon
Hi Ilias,
On Mon, 5 Jul 2021 at 09:48, Simon Glass sjg@chromium.org wrote:
At present the TPM2 emulator lacks the ability to load and save the state. This means it cannot be used for verify-boot flow that includes multiple phases (e.g. VPL and SPL). It also lacks support for non-volatile data storage.
This series adds these features to the TPM2 emulator, with some code from TPM1 moving into a common file.
A few other clean-ups are included to make the two emulators more similar.
Simon Glass (9): sandbox: tpm: Split out common nvdata code sandbox: tpm: Tidy up reading and writing of device state sandbox: tpm: Support the define-space command sandbox: tpm: Correct handling of get-capability sandbox: tpm: Finish comments for struct sandbox_tpm2 sandbox: tpm: Track whether the state is valid sandbox: tpm: Support nvdata in TPM2 sandbox: tpm: Support storing device state in tpm2 sandbox: tpm: Support extending a PCR multiple times
drivers/tpm/Makefile | 4 +- drivers/tpm/sandbox_common.c | 77 ++++++++++ drivers/tpm/sandbox_common.h | 108 ++++++++++++++ drivers/tpm/tpm2_tis_sandbox.c | 256 +++++++++++++++++++++++++++++++-- drivers/tpm/tpm_tis_sandbox.c | 171 ++++++---------------- include/tpm-v2.h | 2 + 6 files changed, 479 insertions(+), 139 deletions(-) create mode 100644 drivers/tpm/sandbox_common.c create mode 100644 drivers/tpm/sandbox_common.h
-- 2.32.0.93.g670b81a890-goog
Not sure if you have any comments on this one?
Regards, Simon
Hi Simon,
Unfortunately i had no time to look into this. I'll have a look tomorrow
Cheers /Ilias
On Wed, 14 Jul 2021 at 22:51, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Mon, 5 Jul 2021 at 09:48, Simon Glass sjg@chromium.org wrote:
At present the TPM2 emulator lacks the ability to load and save the state. This means it cannot be used for verify-boot flow that includes multiple phases (e.g. VPL and SPL). It also lacks support for non-volatile data storage.
This series adds these features to the TPM2 emulator, with some code from TPM1 moving into a common file.
A few other clean-ups are included to make the two emulators more similar.
Simon Glass (9): sandbox: tpm: Split out common nvdata code sandbox: tpm: Tidy up reading and writing of device state sandbox: tpm: Support the define-space command sandbox: tpm: Correct handling of get-capability sandbox: tpm: Finish comments for struct sandbox_tpm2 sandbox: tpm: Track whether the state is valid sandbox: tpm: Support nvdata in TPM2 sandbox: tpm: Support storing device state in tpm2 sandbox: tpm: Support extending a PCR multiple times
drivers/tpm/Makefile | 4 +- drivers/tpm/sandbox_common.c | 77 ++++++++++ drivers/tpm/sandbox_common.h | 108 ++++++++++++++ drivers/tpm/tpm2_tis_sandbox.c | 256 +++++++++++++++++++++++++++++++-- drivers/tpm/tpm_tis_sandbox.c | 171 ++++++---------------- include/tpm-v2.h | 2 + 6 files changed, 479 insertions(+), 139 deletions(-) create mode 100644 drivers/tpm/sandbox_common.c create mode 100644 drivers/tpm/sandbox_common.h
-- 2.32.0.93.g670b81a890-goog
Not sure if you have any comments on this one?
Regards, Simon
Hi Ilias,
On Wed, 14 Jul 2021 at 15:08, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Simon,
Unfortunately i had no time to look into this. I'll have a look tomorrow
OK thanks.
- Simon
Cheers /Ilias
On Wed, 14 Jul 2021 at 22:51, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Mon, 5 Jul 2021 at 09:48, Simon Glass sjg@chromium.org wrote:
At present the TPM2 emulator lacks the ability to load and save the state. This means it cannot be used for verify-boot flow that includes multiple phases (e.g. VPL and SPL). It also lacks support for non-volatile data storage.
This series adds these features to the TPM2 emulator, with some code from TPM1 moving into a common file.
A few other clean-ups are included to make the two emulators more similar.
Simon Glass (9): sandbox: tpm: Split out common nvdata code sandbox: tpm: Tidy up reading and writing of device state sandbox: tpm: Support the define-space command sandbox: tpm: Correct handling of get-capability sandbox: tpm: Finish comments for struct sandbox_tpm2 sandbox: tpm: Track whether the state is valid sandbox: tpm: Support nvdata in TPM2 sandbox: tpm: Support storing device state in tpm2 sandbox: tpm: Support extending a PCR multiple times
drivers/tpm/Makefile | 4 +- drivers/tpm/sandbox_common.c | 77 ++++++++++ drivers/tpm/sandbox_common.h | 108 ++++++++++++++ drivers/tpm/tpm2_tis_sandbox.c | 256 +++++++++++++++++++++++++++++++-- drivers/tpm/tpm_tis_sandbox.c | 171 ++++++---------------- include/tpm-v2.h | 2 + 6 files changed, 479 insertions(+), 139 deletions(-) create mode 100644 drivers/tpm/sandbox_common.c create mode 100644 drivers/tpm/sandbox_common.h
-- 2.32.0.93.g670b81a890-goog
Not sure if you have any comments on this one?
Regards, Simon
participants (2)
-
Ilias Apalodimas -
Simon Glass