[PATCH 0/9] tpm: Enhance sandbox tpm2 emulation

[PATCH 0/9] tpm: Enhance sandbox tpm2 emulation

[Simon Glass]

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

[PATCH 1/9] sandbox: tpm: Split out common nvdata code

[Simon Glass]

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;
-- 
2.32.0.93.g670b81a890-goog

[PATCH 2/9] sandbox: tpm: Tidy up reading and writing of device state

[Simon Glass]

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;
 }
-- 
2.32.0.93.g670b81a890-goog

[PATCH 3/9] sandbox: tpm: Support the define-space command

[Simon Glass]

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 */
-- 
2.32.0.93.g670b81a890-goog

[PATCH 4/9] sandbox: tpm: Correct handling of get-capability

[Simon Glass]

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",
-- 
2.32.0.93.g670b81a890-goog

[PATCH 5/9] sandbox: tpm: Finish comments for struct sandbox_tpm2

[Simon Glass]

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

[PATCH 6/9] sandbox: tpm: Track whether the state is valid

[Simon Glass]

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;
 }
 
-- 
2.32.0.93.g670b81a890-goog

[PATCH 7/9] sandbox: tpm: Support nvdata in TPM2

[Simon Glass]

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
-- 
2.32.0.93.g670b81a890-goog

[PATCH 8/9] sandbox: tpm: Support storing device state in tpm2

[Simon Glass]

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
-- 
2.32.0.93.g670b81a890-goog

[PATCH 9/9] sandbox: tpm: Support extending a PCR multiple times

[Simon Glass]

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;
-- 
2.32.0.93.g670b81a890-goog

Re: [PATCH 0/9] tpm: Enhance sandbox tpm2 emulation

[Simon Glass]

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

Re: [PATCH 0/9] tpm: Enhance sandbox tpm2 emulation

[Ilias Apalodimas]

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

Re: [PATCH 0/9] tpm: Enhance sandbox tpm2 emulation

[Simon Glass]

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

Re: [PATCH 4/9] sandbox: tpm: Correct handling of get-capability

[Ilias Apalodimas]

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;
>  		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",
> -- 
> 2.32.0.93.g670b81a890-goog
> 

Re: [PATCH 5/9] sandbox: tpm: Finish comments for struct sandbox_tpm2

[Ilias Apalodimas]

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>

Re: [PATCH 9/9] sandbox: tpm: Support extending a PCR multiple times

[Ilias Apalodimas]

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
> 

Re: [PATCH 9/9] sandbox: tpm: Support extending a PCR multiple times

[Ilias Apalodimas]

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
> >

Re: [PATCH 9/9] sandbox: tpm: Support extending a PCR multiple times

[Simon Glass]

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