[PATCH 0/3] extend EFI_TCG2_PROTOCOL support

[PATCH 0/3] extend EFI_TCG2_PROTOCOL support

[Ilias Apalodimas]

A previous patch introduced the basic functionality of the protocol, only
adding support for GetCapability().
In order for the protocol to be useful an eventlog is required.
EFI applications can use the service to extend the TPM PCRs and log the 
events on an eventlog, that can be passed into linux for further validation.

This patch adds support for creating the eventlog upon the protocol 
registration and registers GetEventLog() and HashLogExtendEvent() as well.

It's currently not adding support for measuring PE/COFF images.
It also doesn't add any specific U-Boot events that would extend the PCRs
and log events as described in PC Client Platform Firmware Profile spec [1].
This can be extended in the future.

An easy way to test this is run Grub2 on top of U-Boot and use it's 
tpm module.  Once inserted Grub2 extends PCRs for all the commands 
and logs the events.
Since events that need logging after GetEventLog() has been called, must be
logged in a different location [2] has been used to test that.

This is how the eventlog looks like when booting a kernel, using grub2 and 
with [2] applied, which measures the initrd.
Hex to string on Grub2 events (marked as EV_IPL), will reveal the commands
used.

# Grub2 commands:
insmod tpm
insmod part_msdos
linux (hd0,msdos1)/boot/Image root=/dev/mmcblk1p2
boot

- Event[0]:
  pcrIndex: 0
  eventType: EV_NO_ACTION
  digest: 0000000000000000000000000000000000000000
  eventDataSize: 45
  SpecID:
    - Signature: Spec ID Event03
      platformClass: 0
      specVersionMinor: 0
      specVersionMajor: 2
      specErrata: 0
      uintnSize: 2
      numberOfAlgorithms: 4
      Algorithms:
        - Algorithm[0]:
          algorithmId: sha1
          digestSize: 20
        - Algorithm[1]:
          algorithmId: sha256
          digestSize: 32
        - Algorithm[2]:
          algorithmId: sha384
          digestSize: 48
        - Algorithm[3]:
          algorithmId: sha512
          digestSize: 64
      vendorInfoSize: 0
- Event[1]:
  PCRIndex: 8
  EventType: EV_IPL
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 610a881e824bb6fd57bea1c994805116171e3c9f
    - AlgorithmId: sha256
      Digest: 9e7b6286ce2d38c3a8d09d770ecc476868a0b38d39344e84458a52dbcf7ba6a6
    - AlgorithmId: sha384
      Digest: bba389f3cef6d483731aea81d96c55ca31e5d11df80c1e047b72b627a77a33bf636508f1479a3f11a18031a08a889a9f
    - AlgorithmId: sha512
      Digest: 34aa479c642e24905d86e5b00f84fd9d723b1cbb892dcdee1d9fdd016a374ff8d0da6b23a1699d6d3211bd59ae1202292aa0a947a198a9461b6b595bb300a3ce
  EventSize: 28
  Event: 677275625f636d643a20696e736d6f6420706172745f6d73646f7300
- Event[1]:
  PCRIndex: 8
  EventType: EV_IPL
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 2e8826950606c091843586479bd88b1fa7e287b0
    - AlgorithmId: sha256
      Digest: f13c2e564af1a92a14473a38f973b858974ab33c1bd525cd3a75e7ec4f48f718
    - AlgorithmId: sha384
      Digest: e1bf3f7b2c908fb5e16acf26991fa756ed307fb05315313e1055e0d3766027321db4f44dd6f97f9209cfbf9d5554ee2c
    - AlgorithmId: sha512
      Digest: 8bb77e6d3bc5c2766dde438f5aa70e2e733cf6cda30ab2274ab0ba4d4a75b764eef8d8fb728e0dc17ba121b3ff9d1885162d66b1376d649d74b1d3631a4d9ead
  EventSize: 60
  Event: 677275625f636d643a206c696e757820286864302c6d73646f7331292f626f6f742f496d61676520726f6f743d2f6465762f6d6d63626c6b31703200
- Event[1]:
  PCRIndex: 9
  EventType: EV_IPL
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 26e90b0fbd376b6c1a351da20814ebbd9d3ee8e7
    - AlgorithmId: sha256
      Digest: dc6cb872d9d2ecadfadb27e13c2b6b3bd1ba47ad992028aea6ca0440f82f5a2e
    - AlgorithmId: sha384
      Digest: f501cd5cbe7c775e3d39fed75979777a15bcc52f6d91dda3efda8071391104267af4bbeb04696b43f98fd7c47a71c246
    - AlgorithmId: sha512
      Digest: 30993c55f94163ed538b6d9495e390e1902cc0d376e0e3300101d1a2ae462dd99ae5e52ef3735142d29904ae6efe0d252580637d7ee0ae0d91424ba42aa3f5fc
  EventSize: 24
  Event: 286864302c6d73646f7331292f626f6f742f496d61676500
- Event[1]:
  PCRIndex: 8
  EventType: EV_IPL
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 1056664c244c8e8ac9f635c78109d2f57239d8d0
    - AlgorithmId: sha256
      Digest: 3aebbdb035d70ea02463b766683e40349485103fafc477885872ad0c8cc81dab
    - AlgorithmId: sha384
      Digest: 78da59e54c901c01df67d237e549d81c3c3af2cf33d1243ca7c2980610ffd2bcaa829a378a6fa33d11d66a8c4153a51a
    - AlgorithmId: sha512
      Digest: 6df0f14e1c4deefb0a7a2e3c6c0ce6d33df75e4c4707bd56de556ee3d4f4b60862c021ad5e3d1be54dc2b714cc45bbccdeb7fd95329a87f5265251a71e793d58
  EventSize: 60
  Event: 6b65726e656c5f636d646c696e653a20286864302c6d73646f7331292f626f6f742f496d61676520726f6f743d2f6465762f6d6d63626c6b31703200
- Event[1]:
  PCRIndex: 8
  EventType: EV_IPL
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 5c73b0c6f476ded38de389f894770f06f4d02b2f
    - AlgorithmId: sha256
      Digest: 4509beb0ab401d71fa4a5cd94a55c9a74f13332776ae4019c5bfc4c2005157ff
    - AlgorithmId: sha384
      Digest: 05ddaca1f7569ce3f10b731a040b646b182508b30ffa2daf834b88fe5ec00774edf3b06ad8bd90dde261a4c8e055fa28
    - AlgorithmId: sha512
      Digest: 2280c97f7da38410772dade5ec9feb005bd6643c8f06260b70619fe8e99da015dc9d2bb302d4762143824bff722541b87f2256d96abdf94d99edb35aa48aa24f
  EventSize: 15
  Event: 677275625f636d643a20626f6f7400
- Event[1]:
  PCRIndex: 0
  EventType: EV_EFI_ACTION
  DigestCount: 4
  Digests:
    - AlgorithmId: sha1
      Digest: 0c26b18b54c23976e3aa531463e0e8c69463a6b3
    - AlgorithmId: sha256
      Digest: 8eaa153c7724aac177a845a3a718d66bd61103da8b06c7d3fd453ef2e47ab272
    - AlgorithmId: sha384
      Digest: 5a56517994f45015105f4dc8f71b2b37cc6a9797a45c55b5edecd9b9dc9ab00966049d5c7ba7eae71347ad16c07cd92a
    - AlgorithmId: sha512
      Digest: 3c04a5ae692635725db702678883e61b7c458976d27d6424ef7a1ffa77715fa9126ae2e4eee8477ead4320c6124300022288a4c9c6d8afd7bc42b3451767b35a
  EventSize: 13
  Event: Linux initrd

[1] https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientSpecPlat_TPM_2p0_1p04_pub.pdf
[2] https://lore.kernel.org/linux-efi/20201102170634.20575-1-ardb at kernel.org/

Ilias Apalodimas (3):
  tpm: Add tpm2 headers for TCG2 eventlog support
  efi_loader: Introduce eventlog support for TCG2_PROTOCOL
  cmd: efidebug: Add support for TCG2 final events table

 cmd/efidebug.c             |   4 +
 include/efi_api.h          |   4 +
 include/efi_tcg2.h         |  48 +++-
 include/tpm-v2.h           |  59 ++++
 lib/efi_loader/efi_setup.c |  12 +-
 lib/efi_loader/efi_tcg2.c  | 554 +++++++++++++++++++++++++++++++++++--
 6 files changed, 657 insertions(+), 24 deletions(-)

-- 
2.29.2

[PATCH 1/3] tpm: Add tpm2 headers for TCG2 eventlog support

[Ilias Apalodimas]

A following patch introduces support for the EFI_TCG2_PROTOCOL
evenlog management.
Introduce the necessary tpm related headers

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/tpm-v2.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index d8c9feda28dc..9637f9be998e 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -18,6 +18,12 @@
 
 #define TPM2_DIGEST_LEN		32
 
+#define TPM2_SHA1_DIGEST_SIZE 20
+#define TPM2_SHA256_DIGEST_SIZE	32
+#define TPM2_SHA384_DIGEST_SIZE	48
+#define TPM2_SHA512_DIGEST_SIZE	64
+#define TPM2_SM3_256_DIGEST_SIZE 32
+
 #define TPM2_MAX_PCRS 32
 #define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
 #define TPM2_MAX_CAP_BUFFER 1024
@@ -45,6 +51,15 @@
 #define TPM2_PT_MAX_COMMAND_SIZE	(u32)(TPM2_PT_FIXED + 30)
 #define TPM2_PT_MAX_RESPONSE_SIZE	(u32)(TPM2_PT_FIXED + 31)
 
+/* event types */
+#define EV_POST_CODE		((u32)0x00000001)
+#define EV_NO_ACTION		((u32)0x00000003)
+#define EV_SEPARATOR		((u32)0x00000004)
+#define EV_S_CRTM_CONTENTS	((u32)0x00000007)
+#define EV_S_CRTM_VERSION	((u32)0x00000008)
+#define EV_CPU_MICROCODE	((u32)0x00000009)
+#define EV_TABLE_OF_DEVICES	((u32)0x0000000B)
+
 /* TPMS_TAGGED_PROPERTY Structure */
 struct tpms_tagged_property {
 	u32 property;
@@ -86,6 +101,50 @@ struct tpms_capability_data {
 	union tpmu_capabilities data;
 } __packed;
 
+/* defined as TPM_SHA1_160_HASH_LEN in spec */
+struct tpm_digest {
+	u8 digest[TPM2_SHA1_DIGEST_SIZE];
+} __packed;
+
+/* SHA1 Event Log Entry Format */
+struct tcg_pcr_event {
+	u32 pcr_index;
+	u32 event_type;
+	struct tpm_digest digest;
+	u32 event_size;
+	u8 event[];
+} __packed;
+
+/* Definition of TPMU_HA Union */
+union tmpu_ha {
+	u8 sha1[TPM2_SHA1_DIGEST_SIZE];
+	u8 sha256[TPM2_SHA256_DIGEST_SIZE];
+	u8 sm3_256[TPM2_SM3_256_DIGEST_SIZE];
+	u8 sha384[TPM2_SHA384_DIGEST_SIZE];
+	u8 sha512[TPM2_SHA512_DIGEST_SIZE];
+} __packed;
+
+/* Definition of TPMT_HA Structure */
+struct tpmt_ha {
+	u16 hash_alg;
+	union tmpu_ha digest;
+} __packed;
+
+/* Definition of TPML_DIGEST_VALUES Structure */
+struct tpml_digest_values {
+	u32 count;
+	struct tpmt_ha digests[TPM2_NUM_PCR_BANKS];
+} __packed;
+
+/* Crypto Agile Log Entry Format */
+struct tcg_pcr_event2 {
+	u32 pcr_index;
+	u32 event_type;
+	struct tpml_digest_values digests;
+	u32 event_size;
+	u8 event[];
+} __packed;
+
 /**
  * TPM2 Structure Tags for command/response buffers.
  *
-- 
2.29.2

[PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL

[Ilias Apalodimas]

In the previous patches we only introduced a minimal subset of the
EFI_TCG2_PROTOCOL protocol implementing GetCapability().
So let's continue adding features to it, introducing the
GetEventLog() and HashLogExtendEvent() functions.

In order to do that we first need to construct the eventlog in memory,
specifically in EFI_BOOT_SERVICES_DATA memory and a configuration table
from EFI_ACPI_MEMORY_NVS.
U-Boot won't currently add any events to the log or measure any
components, but will expose the necessary EFI APIs for applications
to do so.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/efi_api.h          |   4 +
 include/efi_tcg2.h         |  48 +++-
 lib/efi_loader/efi_setup.c |  12 +-
 lib/efi_loader/efi_tcg2.c  | 554 +++++++++++++++++++++++++++++++++++--
 4 files changed, 594 insertions(+), 24 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 5744f6aed86d..364c578a3b1b 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -356,6 +356,10 @@ struct efi_runtime_services {
 	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
 		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
 
+#define EFI_TCG2_FINAL_EVENTS_TABLE_GUID \
+	EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, \
+		 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
+
 struct efi_configuration_table {
 	efi_guid_t guid;
 	void *table;
diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
index 86b8fe4c01af..6de86156ac4d 100644
--- a/include/efi_tcg2.h
+++ b/include/efi_tcg2.h
@@ -17,6 +17,8 @@
 
 /* TPMV2 only */
 #define TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
+#define EFI_TCG2_EXTEND_ONLY 0x0000000000000001
+#define PE_COFF_IMAGE 0x0000000000000010
 
 /* Algorithm Registry */
 #define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
@@ -25,6 +27,15 @@
 #define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x00000008
 #define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
 
+#define TPM2_SHA1_DIGEST_SIZE 20
+#define TPM2_SHA256_DIGEST_SIZE 32
+#define TPM2_SHA384_DIGEST_SIZE 48
+#define TPM2_SHA512_DIGEST_SIZE 64
+
+#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
+
+#define TPM2_EVENT_LOG_SIZE 4096
+
 typedef u32 efi_tcg_event_log_bitmap;
 typedef u32 efi_tcg_event_log_format;
 typedef u32 efi_tcg_event_algorithm_bitmap;
@@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability {
 	sizeof(struct efi_tcg2_boot_service_capability) - \
 	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
 
+#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03"
+
+#define tcg_efi_spec_id_event_spec_version_major_tpm2 2
+#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0
+#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0
+
+struct tcg_efi_spec_id_event_algorithm_size {
+	u16      algorithm_id;
+	u16      digest_size;
+} __packed;
+
+struct tcg_efi_spec_id_event {
+	u8 signature[16];
+	u32 platform_class;
+	u8 spec_version_minor;
+	u8 spec_version_major;
+	u8 spec_errata;
+	u8 uintn_size;
+	u32 number_of_algorithms;
+	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
+	u8 vendor_info_size;
+	/*
+	 * filled in with vendor_info_size
+	 * currently vendor_info_size = 0
+	 */
+	u8 vendor_info[];
+} __packed;
+
+struct efi_tcg2_final_events_table {
+	u64 version;
+	u64 number_of_events;
+	struct tcg_pcr_event2 event[];
+};
+
 struct efi_tcg2_protocol {
 	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
 					       struct efi_tcg2_boot_service_capability *capability);
@@ -73,7 +118,8 @@ struct efi_tcg2_protocol {
 					     u64 *event_log_location, u64 *event_log_last_entry,
 					     bool *event_log_truncated);
 	efi_status_t (EFIAPI * hash_log_extend_event)(struct efi_tcg2_protocol *this,
-						      u64 flags, u64 data_to_hash,
+						      u64 flags,
+						      efi_physical_addr_t data_to_hash,
 						      u64 data_to_hash_len,
 						      struct efi_tcg2_event *efi_tcg_event);
 	efi_status_t (EFIAPI * submit_command)(struct efi_tcg2_protocol *this,
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index e206b60bb82c..2bb2c3c7aafa 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -157,12 +157,6 @@ efi_status_t efi_init_obj_list(void)
 			goto out;
 	}
 
-	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
-		ret = efi_tcg2_register();
-		if (ret != EFI_SUCCESS)
-			goto out;
-	}
-
 	/* Initialize variable services */
 	ret = efi_init_variables();
 	if (ret != EFI_SUCCESS)
@@ -189,6 +183,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
+		ret = efi_tcg2_register();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	/* Secure boot */
 	ret = efi_init_secure_boot();
 	if (ret != EFI_SUCCESS)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 62f2f9427b6e..d65a48bc65a3 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -14,11 +14,24 @@
 #include <efi_tcg2.h>
 #include <log.h>
 #include <tpm-v2.h>
+#include <u-boot/sha1.h>
+#include <u-boot/sha256.h>
+#include <u-boot/sha512.h>
 #include <linux/unaligned/access_ok.h>
 #include <linux/unaligned/generic.h>
+#include <hexdump.h>
+
+struct event_log_buffer {
+	void *buffer;
+	void *final_buffer;
+	size_t pos; /* eventlog position */
+	size_t final_pos; /* final events config table position */
+	size_t last_event_size;
+	bool get_event_called;
+	bool truncated;
+};
 
-DECLARE_GLOBAL_DATA_PTR;
-
+static struct event_log_buffer event_log;
 /*
  * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
  * Since the current tpm2_get_capability() response buffers starts at
@@ -30,33 +43,40 @@ DECLARE_GLOBAL_DATA_PTR;
 #define properties_offset (offsetof(struct tpml_tagged_tpm_property, tpm_property) + \
 			   offsetof(struct tpms_tagged_property, value))
 
-struct {
+static const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
+static const efi_guid_t efi_guid_final_events = EFI_TCG2_FINAL_EVENTS_TABLE_GUID;
+
+struct digest_info {
 	u16 hash_alg;
 	u32 hash_mask;
-} hash_algo_list[] = {
+	u16 hash_len;
+};
+
+const static struct digest_info hash_algo_list[] = {
 	{
 		TPM2_ALG_SHA1,
 		EFI_TCG2_BOOT_HASH_ALG_SHA1,
+		TPM2_SHA1_DIGEST_SIZE,
 	},
 	{
 		TPM2_ALG_SHA256,
 		EFI_TCG2_BOOT_HASH_ALG_SHA256,
+		TPM2_SHA256_DIGEST_SIZE,
 	},
 	{
 		TPM2_ALG_SHA384,
 		EFI_TCG2_BOOT_HASH_ALG_SHA384,
+		TPM2_SHA384_DIGEST_SIZE,
 	},
 	{
 		TPM2_ALG_SHA512,
 		EFI_TCG2_BOOT_HASH_ALG_SHA512,
-	},
-	{
-		TPM2_ALG_SM3_256,
-		EFI_TCG2_BOOT_HASH_ALG_SM3_256,
+		TPM2_SHA512_DIGEST_SIZE,
 	},
 };
 
 #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
+
 /**
  * alg_to_mask - Get a TCG hash mask for algorithms
  *
@@ -76,7 +96,146 @@ static u32 alg_to_mask(u16 hash_alg)
 	return 0;
 }
 
-const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
+/**
+ * alg_to_len - Get a TCG hash len for algorithms
+ *
+ * @hash_alg: TCG defined algorithm
+ *
+ * @Return: len of chosen algorithm, 0 if the algorithm is not supported
+ */
+static u16 alg_to_len(u16 hash_alg)
+{
+	int i;
+
+	for (i = 0; i < MAX_HASH_COUNT; i++) {
+		if (hash_algo_list[i].hash_alg == hash_alg)
+			return hash_algo_list[i].hash_len;
+	}
+
+	return 0;
+}
+
+static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
+{
+	u32 len;
+	int i;
+
+	len = offsetof(struct tcg_pcr_event2, digests);
+	len += offsetof(struct tpml_digest_values, digests);
+	for (i = 0; i < digest_list->count; i++) {
+		u16 hash_alg = digest_list->digests[i].hash_alg;
+
+		len += offsetof(struct tpmt_ha, digest);
+		len += alg_to_len(hash_alg);
+	}
+	len += sizeof(u32); /* tcg_pcr_event2 event_size*/
+
+	return len;
+}
+
+/* tcg2_pcr_extend - Extend PCRs for a TPM2 device for a given tpml_digest_values
+ *
+ * @dev:		device
+ * @digest_list:	list of digest algorithms to extend
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
+				    struct tpml_digest_values *digest_list)
+{
+	u32 rc;
+	int i;
+
+	for (i = 0; i < digest_list->count; i++) {
+		u32 alg = digest_list->digests[i].hash_alg;
+
+		rc = tpm2_pcr_extend(dev, pcr_index, alg,
+				     (u8 *)&digest_list->digests[i].digest,
+				     alg_to_len(alg));
+		if (rc) {
+			EFI_PRINT("Failed to extend PCR\n");
+			return EFI_DEVICE_ERROR;
+		}
+	}
+
+	return EFI_SUCCESS;
+}
+
+/* tcg2_agile_log_append - Append an agile event to out eventlog
+ *
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @digest_list:	list of digest algorithms to add
+ * @size:		size of event
+ * @event:		event to add
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
+					  struct tpml_digest_values *digest_list,
+					  u32 size, u8 event[])
+{
+	void *log = event_log.buffer + event_log.pos;
+	size_t pos;
+	int i;
+	u32 event_size;
+
+	if (event_log.get_event_called)
+		log = event_log.final_buffer + event_log.final_pos;
+
+	/*
+	 * size refers to the length of event[] only, we need to check against
+	 * the final tcg_pcr_event2 size
+	 */
+	event_size = size + tcg_event_final_size(digest_list);
+	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
+	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
+		event_log.truncated = true;
+		return EFI_VOLUME_FULL;
+	}
+
+	put_unaligned_le32(pcr_index, log);
+	pos = offsetof(struct tcg_pcr_event2, event_type);
+	put_unaligned_le32(event_type, log + pos);
+	pos = offsetof(struct tcg_pcr_event2, digests); /* count */
+	put_unaligned_le32(digest_list->count, log + pos);
+
+	pos += offsetof(struct tpml_digest_values, digests);
+	for (i = 0; i < digest_list->count; i++) {
+		u16 hash_alg = digest_list->digests[i].hash_alg;
+		u8 *digest = (u8 *)&digest_list->digests[i].digest;
+
+		put_unaligned_le16(hash_alg, log + pos);
+		pos += offsetof(struct tpmt_ha, digest);
+		memcpy(log + pos, digest, alg_to_len(hash_alg));
+		pos += alg_to_len(hash_alg);
+	}
+
+	put_unaligned_le32(size, log + pos);
+	pos += sizeof(u32); /* tcg_pcr_event2 event_size*/
+	memcpy(log + pos, event, size);
+	pos += size;
+
+	/* make sure the calculated buffer is what we checked against */
+	if (pos != event_size)
+		return EFI_INVALID_PARAMETER;
+
+	/* if GetEventLog hasn't been called update the normal log */
+	if (!event_log.get_event_called) {
+		event_log.pos += pos;
+		event_log.last_event_size = pos;
+	} else {
+	/* if GetEventLog has been called update config table log */
+		struct efi_tcg2_final_events_table *final_event;
+
+		final_event =
+			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
+		final_event->number_of_events++;
+		event_log.final_pos += pos;
+	}
+
+	return EFI_SUCCESS;
+}
 
 /**
  * platform_get_tpm_device() - retrieve TPM device
@@ -208,7 +367,7 @@ static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
  *
  * Return: true if PCR is active
  */
-bool is_active_pcr(struct tpms_pcr_selection *selection)
+static bool is_active_pcr(struct tpms_pcr_selection *selection)
 {
 	int i;
 	/*
@@ -308,6 +467,103 @@ out:
 	return -1;
 }
 
+/**
+ * __get_active_pcr_banks() - returns the currently active PCR banks
+ *
+ * @active_pcr_banks:		pointer for receiving the bitmap of currently
+ *				active PCR banks
+ *
+ * Return:	status code
+ */
+static efi_status_t __get_active_pcr_banks(u32 *active_pcr_banks)
+{
+	struct udevice *dev;
+	u32 active, supported, pcr_banks;
+	efi_status_t ret;
+	int err;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	err = tpm2_get_pcr_info(dev, &supported, &active, &pcr_banks);
+	if (err) {
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	}
+
+	*active_pcr_banks = active;
+
+out:
+	return ret;
+}
+
+/* tcg2_create_digest - create a list of digests of the supported PCR banks
+ *			for a given memory range
+ *
+ * @input:		input memory
+ * @length:		length of buffer to calculate the digest
+ * @digest_list:	list of digests to fill in
+ *
+ * Return:		status code
+ */
+static efi_status_t tcg2_create_digest(const u8 *input, u32 length,
+				       struct tpml_digest_values *digest_list)
+{
+	sha1_context ctx;
+	sha256_context ctx_256;
+	sha512_context ctx_512;
+	u8 final[TPM2_ALG_SHA512];
+	efi_status_t ret;
+	u32 active;
+	int i;
+
+	ret = __get_active_pcr_banks(&active);
+	if (ret != EFI_SUCCESS)
+		return ret;
+
+	digest_list->count = 0;
+	for (i = 0; i < MAX_HASH_COUNT; i++) {
+		u16 hash_alg = hash_algo_list[i].hash_alg;
+
+		if (!(active & alg_to_mask(hash_alg)))
+			continue;
+		switch (hash_alg) {
+		case TPM2_ALG_SHA1:
+			sha1_starts(&ctx);
+			sha1_update(&ctx, input, length);
+			sha1_finish(&ctx, final);
+			digest_list->count++;
+			break;
+		case TPM2_ALG_SHA256:
+			sha256_starts(&ctx_256);
+			sha256_update(&ctx_256, input, length);
+			sha256_finish(&ctx_256, final);
+			digest_list->count++;
+			break;
+		case TPM2_ALG_SHA384:
+			sha384_starts(&ctx_512);
+			sha384_update(&ctx_512, input, length);
+			sha384_finish(&ctx_512, final);
+			digest_list->count++;
+			break;
+		case TPM2_ALG_SHA512:
+			sha512_starts(&ctx_512);
+			sha512_update(&ctx_512, input, length);
+			sha512_finish(&ctx_512, final);
+			digest_list->count++;
+			break;
+		default:
+			EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
+			return EFI_INVALID_PARAMETER;
+		}
+		digest_list->digests[i].hash_alg = hash_alg;
+		memcpy(&digest_list->digests[i].digest, final, (u32)alg_to_len(hash_alg));
+	}
+
+	return EFI_SUCCESS;
+}
+
 /**
  * efi_tcg2_get_capability() - protocol capability information and state information
  *
@@ -427,7 +683,28 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
 		      u64 *event_log_location, u64 *event_log_last_entry,
 		      bool *event_log_truncated)
 {
-	return EFI_UNSUPPORTED;
+	efi_status_t ret = EFI_SUCCESS;
+	struct udevice *dev;
+
+	EFI_ENTRY("%p, %u, %p, %p,  %p", this, log_format, event_log_location,
+		  event_log_last_entry, event_log_truncated);
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS) {
+		event_log_location = NULL;
+		event_log_last_entry = NULL;
+		*event_log_truncated = false;
+		ret = EFI_SUCCESS;
+		goto out;
+	}
+	*event_log_location = (uintptr_t)event_log.buffer;
+	*event_log_last_entry = (uintptr_t)(event_log.buffer + event_log.pos -
+					    event_log.last_event_size);
+	*event_log_truncated = event_log.truncated;
+	event_log.get_event_called = true;
+
+out:
+	return EFI_EXIT(ret);
 }
 
 /**
@@ -450,7 +727,76 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
 			       u64 data_to_hash, u64 data_to_hash_len,
 			       struct efi_tcg2_event *efi_tcg_event)
 {
-	return EFI_UNSUPPORTED;
+	struct udevice *dev;
+	efi_status_t ret;
+	u32 event_type, pcr_index, event_size;
+	struct tpml_digest_values digest_list;
+
+	EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
+		  data_to_hash_len, efi_tcg_event);
+
+	if (!this || !data_to_hash || !efi_tcg_event) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	if (efi_tcg_event->size < efi_tcg_event->header.header_size +
+	    sizeof(u32)) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	if (efi_tcg_event->header.pcr_index < 0 ||
+	    efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	/*
+	 * if PE_COFF_IMAGE is set we need to make sure the image is not
+	 * corrupted, verify it and hash the PE/COFF image in accordance with
+	 * the  procedure  specified  in  "Calculating  the  PE  Image  Hash"
+	 * section  of the "Windows Authenticode Portable Executable Signature
+	 * Format"
+	 * Not supported for now
+	 */
+	if (flags & PE_COFF_IMAGE) {
+		ret = EFI_UNSUPPORTED;
+		goto out;
+	}
+
+	pcr_index = efi_tcg_event->header.pcr_index;
+	event_type = efi_tcg_event->header.event_type;
+
+	ret = tcg2_create_digest((u8 *)data_to_hash, data_to_hash_len,
+				 &digest_list);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	if (flags & EFI_TCG2_EXTEND_ONLY) {
+		if (event_log.truncated)
+			ret = EFI_VOLUME_FULL;
+		goto out;
+	}
+
+	/*
+	 * The efi_tcg_event size includes the size component and the
+	 * headersize
+	 */
+	event_size = efi_tcg_event->size - sizeof(efi_tcg_event->size) -
+		efi_tcg_event->header.header_size;
+	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
+				    event_size, efi_tcg_event->event);
+out:
+	return EFI_EXIT(ret);
 }
 
 /**
@@ -464,7 +810,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
  *
  * Return:	status code
  */
-efi_status_t EFIAPI
+static efi_status_t EFIAPI
 efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
 			u32 input_param_block_size, u8 *input_param_block,
 			u32 output_param_block_size, u8 *output_param_block)
@@ -481,11 +827,16 @@ efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
  *
  * Return:	status code
  */
-efi_status_t EFIAPI
+static efi_status_t EFIAPI
 efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
 			      u32 *active_pcr_banks)
 {
-	return EFI_UNSUPPORTED;
+	efi_status_t ret;
+
+	EFI_ENTRY("%p, %p", this, active_pcr_banks);
+	ret = __get_active_pcr_banks(active_pcr_banks);
+
+	return EFI_EXIT(ret);
 }
 
 /**
@@ -496,7 +847,7 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
  *
  * Return:	status code
  */
-efi_status_t EFIAPI
+static efi_status_t EFIAPI
 efi_tcg2_set_active_pcr_banks(struct efi_tcg2_protocol *this,
 			      u32 active_pcr_banks)
 {
@@ -515,7 +866,7 @@ efi_tcg2_set_active_pcr_banks(struct efi_tcg2_protocol *this,
  *
  * Return:	status code
  */
-efi_status_t EFIAPI
+static efi_status_t EFIAPI
 efi_tcg2_get_result_of_set_active_pcr_banks(struct efi_tcg2_protocol *this,
 					    u32 *operation_present, u32 *response)
 {
@@ -532,6 +883,170 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = {
 	.get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks,
 };
 
+/**
+ * create_specid_event() - Create the first event in the eventlog
+ *
+ * @dev:			tpm device
+ * @event_header:		Pointer to the final event header
+ * @event_size:			final spec event size
+ *
+ * Return:	status code
+ */
+static efi_status_t create_specid_event(struct udevice *dev, void *buffer,
+					size_t *event_size)
+{
+	struct tcg_efi_spec_id_event *spec_event;
+	size_t spec_event_size;
+	efi_status_t ret = EFI_DEVICE_ERROR;
+	u32 active, supported;
+	int err, i;
+
+	/*
+	 * Create Spec event. This needs to be the first event in the log
+	 * according to the TCG EFI protocol spec
+	 */
+
+	/* Setup specID event data */
+	spec_event = (struct tcg_efi_spec_id_event *)buffer;
+	memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03,
+	       sizeof(spec_event->signature));
+	put_unaligned_le32(0, &spec_event->platform_class); /* type client */
+	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2,
+			   &spec_event->spec_version_minor);
+	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2,
+			   &spec_event->spec_version_major);
+	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2,
+			   &spec_event->spec_errata);
+	__put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32),
+			   &spec_event->uintn_size);
+
+	err = tpm2_get_pcr_info(dev, &supported, &active,
+				&spec_event->number_of_algorithms);
+	if (err)
+		goto out;
+	if (spec_event->number_of_algorithms > MAX_HASH_COUNT ||
+	    spec_event->number_of_algorithms < 1)
+		goto out;
+
+	for (i = 0; i < spec_event->number_of_algorithms; i++) {
+		u16 hash_alg = hash_algo_list[i].hash_alg;
+		u16 hash_len = hash_algo_list[i].hash_len;
+
+		if (active && alg_to_mask(hash_alg)) {
+			put_unaligned_le16(hash_alg,
+					   &spec_event->digest_sizes[i].algorithm_id);
+			put_unaligned_le32(hash_len,
+					   &spec_event->digest_sizes[i].digest_size);
+		}
+	}
+	/*
+	 * the size of the spec event and placement of vendor_info_size
+	 * depends on supported algoriths
+	 */
+	spec_event_size =
+		offsetof(struct tcg_efi_spec_id_event, digest_sizes) +
+		spec_event->number_of_algorithms * sizeof(spec_event->digest_sizes[0]);
+	/* no vendor info for us */
+	memset(buffer + spec_event_size, 0,
+	       sizeof(spec_event->vendor_info_size));
+	spec_event_size += sizeof(spec_event->vendor_info_size);
+	*event_size = spec_event_size;
+
+	return EFI_SUCCESS;
+
+out:
+	return ret;
+}
+
+/**
+ * create_final_event() - Create the final event and install the config
+ *			defined by the TCG EFI spec
+ */
+static efi_status_t create_final_event(void)
+{
+	struct efi_tcg2_final_events_table *final_event;
+	efi_status_t ret;
+
+	/*
+	 * All events generated after the invocation of
+	 * EFI_TCG2_GET_EVENT_LOGS need to be stored in an instance of an
+	 * EFI_CONFIGURATION_TABLE
+	 */
+	ret = efi_allocate_pool(EFI_ACPI_MEMORY_NVS, TPM2_EVENT_LOG_SIZE,
+				&event_log.final_buffer);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	memset(event_log.final_buffer, 0xff, TPM2_EVENT_LOG_SIZE);
+	final_event = event_log.final_buffer;
+	final_event->number_of_events = 0;
+	final_event->version = EFI_TCG2_FINAL_EVENTS_TABLE_VERSION;
+	event_log.final_pos = sizeof(*final_event);
+	ret = efi_install_configuration_table(&efi_guid_final_events,
+					      final_event);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	return EFI_SUCCESS;
+out:
+	return ret;
+}
+
+/**
+ * efi_init_event_log() - initialize an eventlog
+ */
+static efi_status_t efi_init_event_log(void)
+{
+	/*
+	 * vendor_info_size is currently set to 0, we need to change the length
+	 * and allocate the flexible array member if this changes
+	 */
+	struct tcg_pcr_event *event_header = NULL;
+	struct udevice *dev;
+	size_t spec_event_size;
+	efi_status_t ret;
+
+	ret = platform_get_tpm2_device(&dev);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
+				(void **)&event_log.buffer);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	/*
+	 * initialize log area as 0xff so the OS can easily figure out the
+	 * last log entry
+	 */
+	memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
+	event_log.pos = 0;
+	event_log.last_event_size = 0;
+	event_log.get_event_called = false;
+	event_log.truncated = false;
+
+	/*
+	 * The log header is defined to be in SHA1 event log entry format.
+	 * Setup event header
+	 */
+	event_header =  (struct tcg_pcr_event *)event_log.buffer;
+	put_unaligned_le32(0, &event_header->pcr_index);
+	put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
+	memset(&event_header->digest, 0, sizeof(event_header->digest));
+	ret = create_specid_event(dev, event_log.buffer + sizeof(*event_header),
+				  &spec_event_size);
+	if (ret != EFI_SUCCESS)
+		goto out;
+	put_unaligned_le32(spec_event_size, &event_header->event_size);
+	event_log.pos = spec_event_size + sizeof(*event_header);
+	event_log.last_event_size = event_log.pos;
+
+	ret = create_final_event();
+
+out:
+	return ret;
+}
+
 /**
  * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
  *
@@ -549,6 +1064,11 @@ efi_status_t efi_tcg2_register(void)
 		log_warning("Unable to find TPMv2 device\n");
 		return EFI_SUCCESS;
 	}
+
+	ret = efi_init_event_log();
+	if (ret != EFI_SUCCESS)
+		return ret;
+
 	ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
 			       (void *)&efi_tcg2_protocol);
 	if (ret != EFI_SUCCESS)
-- 
2.29.2

[PATCH 3/3] cmd: efidebug: Add support for TCG2 final events table

[Ilias Apalodimas]

A previous commit is adding EFI_TCG2_PROTOCOL, which in it's eventlog
support registers an EFI configuration table.
Let's add the necessary GUID so 'efidebug table' command can display
table names properly.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 cmd/efidebug.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 5288b9920b4d..70bb73ba025f 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -278,6 +278,10 @@ static const struct {
 		"Runtime properties",
 		EFI_RT_PROPERTIES_TABLE_GUID,
 	},
+	{
+		"TCG2 Final Events Table",
+		EFI_TCG2_FINAL_EVENTS_TABLE_GUID,
+	},
 };
 
 /**
-- 
2.29.2

[PATCH 1/3] tpm: Add tpm2 headers for TCG2 eventlog support

[Heinrich Schuchardt]

On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> A following patch introduces support for the EFI_TCG2_PROTOCOL
> evenlog management.

%s/evenlog/eventlog/

> Introduce the necessary tpm related headers
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   include/tpm-v2.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)
>
> diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> index d8c9feda28dc..9637f9be998e 100644
> --- a/include/tpm-v2.h
> +++ b/include/tpm-v2.h
> @@ -18,6 +18,12 @@
>
>   #define TPM2_DIGEST_LEN		32

Shouldn't TPM2_DIGEST_LEN be renamed to TPM2_SHA256_DIGEST_SIZE in all
of our code?

>
> +#define TPM2_SHA1_DIGEST_SIZE 20
> +#define TPM2_SHA256_DIGEST_SIZE	32
> +#define TPM2_SHA384_DIGEST_SIZE	48
> +#define TPM2_SHA512_DIGEST_SIZE	64
> +#define TPM2_SM3_256_DIGEST_SIZE 32
> +
>   #define TPM2_MAX_PCRS 32
>   #define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
>   #define TPM2_MAX_CAP_BUFFER 1024
> @@ -45,6 +51,15 @@
>   #define TPM2_PT_MAX_COMMAND_SIZE	(u32)(TPM2_PT_FIXED + 30)
>   #define TPM2_PT_MAX_RESPONSE_SIZE	(u32)(TPM2_PT_FIXED + 31)
>
> +/* event types */
> +#define EV_POST_CODE		((u32)0x00000001)
> +#define EV_NO_ACTION		((u32)0x00000003)
> +#define EV_SEPARATOR		((u32)0x00000004)
> +#define EV_S_CRTM_CONTENTS	((u32)0x00000007)
> +#define EV_S_CRTM_VERSION	((u32)0x00000008)
> +#define EV_CPU_MICROCODE	((u32)0x00000009)
> +#define EV_TABLE_OF_DEVICES	((u32)0x0000000B)
> +
>   /* TPMS_TAGGED_PROPERTY Structure */
>   struct tpms_tagged_property {
>   	u32 property;
> @@ -86,6 +101,50 @@ struct tpms_capability_data {
>   	union tpmu_capabilities data;
>   } __packed;
>
> +/* defined as TPM_SHA1_160_HASH_LEN in spec */
> +struct tpm_digest {
> +	u8 digest[TPM2_SHA1_DIGEST_SIZE];
> +} __packed;
> +
> +/* SHA1 Event Log Entry Format */

Please, use Sphinx style comments for structures. This allows us to add
them to the HTML documentation. See

https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

I would appreciate member descriptions, too.

> +struct tcg_pcr_event {
> +	u32 pcr_index;
> +	u32 event_type;
> +	struct tpm_digest digest;

struct tpm_digest is only used here in your patch series.

The standard has

     typedef UINT8 TCG_DIGEST[TPM2_SHA1_DIGEST_SIZE];

Can't we simply write

	u8 digest[20];

here and get rid of struct tpm_digest?

Otherwise looks ok to me.

Best regards

Heinrich

> +	u32 event_size;
> +	u8 event[];
> +} __packed;
> +
> +/* Definition of TPMU_HA Union */
> +union tmpu_ha {
> +	u8 sha1[TPM2_SHA1_DIGEST_SIZE];
> +	u8 sha256[TPM2_SHA256_DIGEST_SIZE];
> +	u8 sm3_256[TPM2_SM3_256_DIGEST_SIZE];
> +	u8 sha384[TPM2_SHA384_DIGEST_SIZE];
> +	u8 sha512[TPM2_SHA512_DIGEST_SIZE];
> +} __packed;
> +
> +/* Definition of TPMT_HA Structure */
> +struct tpmt_ha {
> +	u16 hash_alg;
> +	union tmpu_ha digest;
> +} __packed;
> +
> +/* Definition of TPML_DIGEST_VALUES Structure */
> +struct tpml_digest_values {
> +	u32 count;
> +	struct tpmt_ha digests[TPM2_NUM_PCR_BANKS];
> +} __packed;
> +
> +/* Crypto Agile Log Entry Format */
> +struct tcg_pcr_event2 {
> +	u32 pcr_index;
> +	u32 event_type;
> +	struct tpml_digest_values digests;
> +	u32 event_size;
> +	u8 event[];
> +} __packed;
> +
>   /**
>    * TPM2 Structure Tags for command/response buffers.
>    *
>

[PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL

[Heinrich Schuchardt]

On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> In the previous patches we only introduced a minimal subset of the
> EFI_TCG2_PROTOCOL protocol implementing GetCapability().
> So let's continue adding features to it, introducing the
> GetEventLog() and HashLogExtendEvent() functions.
>
> In order to do that we first need to construct the eventlog in memory,
> specifically in EFI_BOOT_SERVICES_DATA memory and a configuration table
> from EFI_ACPI_MEMORY_NVS.
> U-Boot won't currently add any events to the log or measure any
> components, but will expose the necessary EFI APIs for applications
> to do so.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   include/efi_api.h          |   4 +
>   include/efi_tcg2.h         |  48 +++-
>   lib/efi_loader/efi_setup.c |  12 +-
>   lib/efi_loader/efi_tcg2.c  | 554 +++++++++++++++++++++++++++++++++++--
>   4 files changed, 594 insertions(+), 24 deletions(-)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 5744f6aed86d..364c578a3b1b 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -356,6 +356,10 @@ struct efi_runtime_services {
>   	EFI_GUID(0x4006c0c1, 0xfcb3, 0x403e, \
>   		 0x99, 0x6d, 0x4a, 0x6c, 0x87, 0x24, 0xe0, 0x6d)
>
> +#define EFI_TCG2_FINAL_EVENTS_TABLE_GUID \
> +	EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, \
> +		 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
> +
>   struct efi_configuration_table {
>   	efi_guid_t guid;
>   	void *table;
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index 86b8fe4c01af..6de86156ac4d 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -17,6 +17,8 @@
>
>   /* TPMV2 only */
>   #define TCG2_EVENT_LOG_FORMAT_TCG_2 0x00000002
> +#define EFI_TCG2_EXTEND_ONLY 0x0000000000000001
> +#define PE_COFF_IMAGE 0x0000000000000010
>
>   /* Algorithm Registry */
>   #define EFI_TCG2_BOOT_HASH_ALG_SHA1    0x00000001
> @@ -25,6 +27,15 @@
>   #define EFI_TCG2_BOOT_HASH_ALG_SHA512  0x00000008
>   #define EFI_TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
>
> +#define TPM2_SHA1_DIGEST_SIZE 20
> +#define TPM2_SHA256_DIGEST_SIZE 32
> +#define TPM2_SHA384_DIGEST_SIZE 48
> +#define TPM2_SHA512_DIGEST_SIZE 64

lib/efi_loader/efi_tcg2.c already includes tpm-v2.h.

Why should we define the same constant twice?

Best regards

Heinrich

> +
> +#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
> +
> +#define TPM2_EVENT_LOG_SIZE 4096

What does this size derive from? A comment describing the constant could
help.

> +
>   typedef u32 efi_tcg_event_log_bitmap;
>   typedef u32 efi_tcg_event_log_format;
>   typedef u32 efi_tcg_event_algorithm_bitmap;
> @@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability {
>   	sizeof(struct efi_tcg2_boot_service_capability) - \
>   	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
>
> +#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03"
> +
> +#define tcg_efi_spec_id_event_spec_version_major_tpm2 2
> +#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0
> +#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0

Constants should be capitalized.

> +

Please, provide Sphinx style comments for structures. Cf.
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

> +struct tcg_efi_spec_id_event_algorithm_size {
> +	u16      algorithm_id;
> +	u16      digest_size;
> +} __packed;
> +
> +struct tcg_efi_spec_id_event {
> +	u8 signature[16];
> +	u32 platform_class;
> +	u8 spec_version_minor;
> +	u8 spec_version_major;
> +	u8 spec_errata;
> +	u8 uintn_size;
> +	u32 number_of_algorithms;
> +	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
> +	u8 vendor_info_size;
> +	/*
> +	 * filled in with vendor_info_size
> +	 * currently vendor_info_size = 0

%s/vendor_info_size = 0/U-Boot does not provide any vendor info/

> +	 */
> +	u8 vendor_info[];
> +} __packed;
> +
> +struct efi_tcg2_final_events_table {
> +	u64 version;
> +	u64 number_of_events;
> +	struct tcg_pcr_event2 event[];
> +};
> +
>   struct efi_tcg2_protocol {
>   	efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this,
>   					       struct efi_tcg2_boot_service_capability *capability);
> @@ -73,7 +118,8 @@ struct efi_tcg2_protocol {
>   					     u64 *event_log_location, u64 *event_log_last_entry,
>   					     bool *event_log_truncated);
>   	efi_status_t (EFIAPI * hash_log_extend_event)(struct efi_tcg2_protocol *this,
> -						      u64 flags, u64 data_to_hash,
> +						      u64 flags,
> +						      efi_physical_addr_t data_to_hash,
>   						      u64 data_to_hash_len,
>   						      struct efi_tcg2_event *efi_tcg_event);
>   	efi_status_t (EFIAPI * submit_command)(struct efi_tcg2_protocol *this,
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index e206b60bb82c..2bb2c3c7aafa 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -157,12 +157,6 @@ efi_status_t efi_init_obj_list(void)
>   			goto out;
>   	}
>
> -	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> -		ret = efi_tcg2_register();
> -		if (ret != EFI_SUCCESS)
> -			goto out;
> -	}
> -
>   	/* Initialize variable services */
>   	ret = efi_init_variables();
>   	if (ret != EFI_SUCCESS)
> @@ -189,6 +183,12 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
> +	if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
> +		ret = efi_tcg2_register();
> +		if (ret != EFI_SUCCESS)
> +			goto out;
> +	}
> +
>   	/* Secure boot */
>   	ret = efi_init_secure_boot();
>   	if (ret != EFI_SUCCESS)
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 62f2f9427b6e..d65a48bc65a3 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -14,11 +14,24 @@
>   #include <efi_tcg2.h>
>   #include <log.h>
>   #include <tpm-v2.h>
> +#include <u-boot/sha1.h>
> +#include <u-boot/sha256.h>
> +#include <u-boot/sha512.h>
>   #include <linux/unaligned/access_ok.h>
>   #include <linux/unaligned/generic.h>
> +#include <hexdump.h>
> +
> +struct event_log_buffer {
> +	void *buffer;
> +	void *final_buffer;
> +	size_t pos; /* eventlog position */
> +	size_t final_pos; /* final events config table position */
> +	size_t last_event_size;
> +	bool get_event_called;
> +	bool truncated;
> +};
>
> -DECLARE_GLOBAL_DATA_PTR;
> -
> +static struct event_log_buffer event_log;
>   /*
>    * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
>    * Since the current tpm2_get_capability() response buffers starts at
> @@ -30,33 +43,40 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define properties_offset (offsetof(struct tpml_tagged_tpm_property, tpm_property) + \
>   			   offsetof(struct tpms_tagged_property, value))
>
> -struct {
> +static const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
> +static const efi_guid_t efi_guid_final_events = EFI_TCG2_FINAL_EVENTS_TABLE_GUID;
> +
> +struct digest_info {
>   	u16 hash_alg;
>   	u32 hash_mask;
> -} hash_algo_list[] = {
> +	u16 hash_len;
> +};
> +
> +const static struct digest_info hash_algo_list[] = {
>   	{
>   		TPM2_ALG_SHA1,
>   		EFI_TCG2_BOOT_HASH_ALG_SHA1,
> +		TPM2_SHA1_DIGEST_SIZE,
>   	},
>   	{
>   		TPM2_ALG_SHA256,
>   		EFI_TCG2_BOOT_HASH_ALG_SHA256,
> +		TPM2_SHA256_DIGEST_SIZE,
>   	},
>   	{
>   		TPM2_ALG_SHA384,
>   		EFI_TCG2_BOOT_HASH_ALG_SHA384,
> +		TPM2_SHA384_DIGEST_SIZE,
>   	},
>   	{
>   		TPM2_ALG_SHA512,
>   		EFI_TCG2_BOOT_HASH_ALG_SHA512,
> -	},
> -	{
> -		TPM2_ALG_SM3_256,
> -		EFI_TCG2_BOOT_HASH_ALG_SM3_256,
> +		TPM2_SHA512_DIGEST_SIZE,
>   	},
>   };
>
>   #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)
> +
>   /**
>    * alg_to_mask - Get a TCG hash mask for algorithms
>    *
> @@ -76,7 +96,146 @@ static u32 alg_to_mask(u16 hash_alg)
>   	return 0;
>   }
>
> -const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;
> +/**
> + * alg_to_len - Get a TCG hash len for algorithms
> + *
> + * @hash_alg: TCG defined algorithm
> + *
> + * @Return: len of chosen algorithm, 0 if the algorithm is not supported
> + */
> +static u16 alg_to_len(u16 hash_alg)
> +{
> +	int i;
> +
> +	for (i = 0; i < MAX_HASH_COUNT; i++) {
> +		if (hash_algo_list[i].hash_alg == hash_alg)
> +			return hash_algo_list[i].hash_len;
> +	}
> +
> +	return 0;
> +}
> +
> +static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
> +{
> +	u32 len;
> +	int i;
> +
> +	len = offsetof(struct tcg_pcr_event2, digests);
> +	len += offsetof(struct tpml_digest_values, digests);
> +	for (i = 0; i < digest_list->count; i++) {
> +		u16 hash_alg = digest_list->digests[i].hash_alg;
> +
> +		len += offsetof(struct tpmt_ha, digest);
> +		len += alg_to_len(hash_alg);
> +	}
> +	len += sizeof(u32); /* tcg_pcr_event2 event_size*/
> +
> +	return len;
> +}
> +
> +/* tcg2_pcr_extend - Extend PCRs for a TPM2 device for a given tpml_digest_values
> + *
> + * @dev:		device
> + * @digest_list:	list of digest algorithms to extend
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
> +				    struct tpml_digest_values *digest_list)
> +{
> +	u32 rc;
> +	int i;
> +
> +	for (i = 0; i < digest_list->count; i++) {
> +		u32 alg = digest_list->digests[i].hash_alg;
> +
> +		rc = tpm2_pcr_extend(dev, pcr_index, alg,
> +				     (u8 *)&digest_list->digests[i].digest,
> +				     alg_to_len(alg));
> +		if (rc) {
> +			EFI_PRINT("Failed to extend PCR\n");
> +			return EFI_DEVICE_ERROR;
> +		}
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/* tcg2_agile_log_append - Append an agile event to out eventlog
> + *
> + * @pcr_index:		PCR index
> + * @event_type:		type of event added
> + * @digest_list:	list of digest algorithms to add
> + * @size:		size of event
> + * @event:		event to add
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> +					  struct tpml_digest_values *digest_list,
> +					  u32 size, u8 event[])
> +{
> +	void *log = event_log.buffer + event_log.pos;
> +	size_t pos;
> +	int i;
> +	u32 event_size;
> +
> +	if (event_log.get_event_called)
> +		log = event_log.final_buffer + event_log.final_pos;
> +
> +	/*
> +	 * size refers to the length of event[] only, we need to check against
> +	 * the final tcg_pcr_event2 size
> +	 */
> +	event_size = size + tcg_event_final_size(digest_list);
> +	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
> +	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
> +		event_log.truncated = true;
> +		return EFI_VOLUME_FULL;
> +	}
> +
> +	put_unaligned_le32(pcr_index, log);
> +	pos = offsetof(struct tcg_pcr_event2, event_type);
> +	put_unaligned_le32(event_type, log + pos);
> +	pos = offsetof(struct tcg_pcr_event2, digests); /* count */
> +	put_unaligned_le32(digest_list->count, log + pos);
> +
> +	pos += offsetof(struct tpml_digest_values, digests);
> +	for (i = 0; i < digest_list->count; i++) {
> +		u16 hash_alg = digest_list->digests[i].hash_alg;
> +		u8 *digest = (u8 *)&digest_list->digests[i].digest;
> +
> +		put_unaligned_le16(hash_alg, log + pos);
> +		pos += offsetof(struct tpmt_ha, digest);
> +		memcpy(log + pos, digest, alg_to_len(hash_alg));
> +		pos += alg_to_len(hash_alg);
> +	}
> +
> +	put_unaligned_le32(size, log + pos);
> +	pos += sizeof(u32); /* tcg_pcr_event2 event_size*/
> +	memcpy(log + pos, event, size);
> +	pos += size;
> +
> +	/* make sure the calculated buffer is what we checked against */
> +	if (pos != event_size)
> +		return EFI_INVALID_PARAMETER;
> +
> +	/* if GetEventLog hasn't been called update the normal log */
> +	if (!event_log.get_event_called) {
> +		event_log.pos += pos;
> +		event_log.last_event_size = pos;
> +	} else {
> +	/* if GetEventLog has been called update config table log */
> +		struct efi_tcg2_final_events_table *final_event;
> +
> +		final_event =
> +			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
> +		final_event->number_of_events++;
> +		event_log.final_pos += pos;
> +	}
> +
> +	return EFI_SUCCESS;
> +}
>
>   /**
>    * platform_get_tpm_device() - retrieve TPM device
> @@ -208,7 +367,7 @@ static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr)
>    *
>    * Return: true if PCR is active
>    */
> -bool is_active_pcr(struct tpms_pcr_selection *selection)
> +static bool is_active_pcr(struct tpms_pcr_selection *selection)
>   {
>   	int i;
>   	/*
> @@ -308,6 +467,103 @@ out:
>   	return -1;
>   }
>
> +/**
> + * __get_active_pcr_banks() - returns the currently active PCR banks
> + *
> + * @active_pcr_banks:		pointer for receiving the bitmap of currently
> + *				active PCR banks
> + *
> + * Return:	status code
> + */
> +static efi_status_t __get_active_pcr_banks(u32 *active_pcr_banks)
> +{
> +	struct udevice *dev;
> +	u32 active, supported, pcr_banks;
> +	efi_status_t ret;
> +	int err;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	err = tpm2_get_pcr_info(dev, &supported, &active, &pcr_banks);
> +	if (err) {
> +		ret = EFI_DEVICE_ERROR;
> +		goto out;
> +	}
> +
> +	*active_pcr_banks = active;
> +
> +out:
> +	return ret;
> +}
> +
> +/* tcg2_create_digest - create a list of digests of the supported PCR banks
> + *			for a given memory range
> + *
> + * @input:		input memory
> + * @length:		length of buffer to calculate the digest
> + * @digest_list:	list of digests to fill in
> + *
> + * Return:		status code
> + */
> +static efi_status_t tcg2_create_digest(const u8 *input, u32 length,
> +				       struct tpml_digest_values *digest_list)
> +{
> +	sha1_context ctx;
> +	sha256_context ctx_256;
> +	sha512_context ctx_512;
> +	u8 final[TPM2_ALG_SHA512];
> +	efi_status_t ret;
> +	u32 active;
> +	int i;
> +
> +	ret = __get_active_pcr_banks(&active);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
> +	digest_list->count = 0;
> +	for (i = 0; i < MAX_HASH_COUNT; i++) {
> +		u16 hash_alg = hash_algo_list[i].hash_alg;
> +
> +		if (!(active & alg_to_mask(hash_alg)))
> +			continue;
> +		switch (hash_alg) {
> +		case TPM2_ALG_SHA1:
> +			sha1_starts(&ctx);
> +			sha1_update(&ctx, input, length);
> +			sha1_finish(&ctx, final);
> +			digest_list->count++;
> +			break;
> +		case TPM2_ALG_SHA256:
> +			sha256_starts(&ctx_256);
> +			sha256_update(&ctx_256, input, length);
> +			sha256_finish(&ctx_256, final);
> +			digest_list->count++;
> +			break;
> +		case TPM2_ALG_SHA384:
> +			sha384_starts(&ctx_512);
> +			sha384_update(&ctx_512, input, length);
> +			sha384_finish(&ctx_512, final);
> +			digest_list->count++;
> +			break;
> +		case TPM2_ALG_SHA512:
> +			sha512_starts(&ctx_512);
> +			sha512_update(&ctx_512, input, length);
> +			sha512_finish(&ctx_512, final);
> +			digest_list->count++;
> +			break;
> +		default:
> +			EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
> +			return EFI_INVALID_PARAMETER;
> +		}
> +		digest_list->digests[i].hash_alg = hash_alg;
> +		memcpy(&digest_list->digests[i].digest, final, (u32)alg_to_len(hash_alg));
> +	}
> +
> +	return EFI_SUCCESS;
> +}
> +
>   /**
>    * efi_tcg2_get_capability() - protocol capability information and state information
>    *
> @@ -427,7 +683,28 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this,
>   		      u64 *event_log_location, u64 *event_log_last_entry,
>   		      bool *event_log_truncated)
>   {
> -	return EFI_UNSUPPORTED;
> +	efi_status_t ret = EFI_SUCCESS;
> +	struct udevice *dev;
> +
> +	EFI_ENTRY("%p, %u, %p, %p,  %p", this, log_format, event_log_location,
> +		  event_log_last_entry, event_log_truncated);
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS) {
> +		event_log_location = NULL;
> +		event_log_last_entry = NULL;
> +		*event_log_truncated = false;
> +		ret = EFI_SUCCESS;
> +		goto out;
> +	}
> +	*event_log_location = (uintptr_t)event_log.buffer;
> +	*event_log_last_entry = (uintptr_t)(event_log.buffer + event_log.pos -
> +					    event_log.last_event_size);
> +	*event_log_truncated = event_log.truncated;
> +	event_log.get_event_called = true;
> +
> +out:
> +	return EFI_EXIT(ret);
>   }
>
>   /**
> @@ -450,7 +727,76 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>   			       u64 data_to_hash, u64 data_to_hash_len,
>   			       struct efi_tcg2_event *efi_tcg_event)
>   {
> -	return EFI_UNSUPPORTED;
> +	struct udevice *dev;
> +	efi_status_t ret;
> +	u32 event_type, pcr_index, event_size;
> +	struct tpml_digest_values digest_list;
> +
> +	EFI_ENTRY("%p, %llu, %llu, %llu, %p", this, flags, data_to_hash,
> +		  data_to_hash_len, efi_tcg_event);
> +
> +	if (!this || !data_to_hash || !efi_tcg_event) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	if (efi_tcg_event->size < efi_tcg_event->header.header_size +
> +	    sizeof(u32)) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	if (efi_tcg_event->header.pcr_index < 0 ||
> +	    efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	/*
> +	 * if PE_COFF_IMAGE is set we need to make sure the image is not
> +	 * corrupted, verify it and hash the PE/COFF image in accordance with
> +	 * the  procedure  specified  in  "Calculating  the  PE  Image  Hash"
> +	 * section  of the "Windows Authenticode Portable Executable Signature
> +	 * Format"
> +	 * Not supported for now
> +	 */
> +	if (flags & PE_COFF_IMAGE) {
> +		ret = EFI_UNSUPPORTED;
> +		goto out;
> +	}
> +
> +	pcr_index = efi_tcg_event->header.pcr_index;
> +	event_type = efi_tcg_event->header.event_type;
> +
> +	ret = tcg2_create_digest((u8 *)data_to_hash, data_to_hash_len,
> +				 &digest_list);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	if (flags & EFI_TCG2_EXTEND_ONLY) {
> +		if (event_log.truncated)
> +			ret = EFI_VOLUME_FULL;
> +		goto out;
> +	}
> +
> +	/*
> +	 * The efi_tcg_event size includes the size component and the
> +	 * headersize
> +	 */
> +	event_size = efi_tcg_event->size - sizeof(efi_tcg_event->size) -
> +		efi_tcg_event->header.header_size;
> +	ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,
> +				    event_size, efi_tcg_event->event);
> +out:
> +	return EFI_EXIT(ret);
>   }
>
>   /**
> @@ -464,7 +810,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags,
>    *
>    * Return:	status code
>    */
> -efi_status_t EFIAPI
> +static efi_status_t EFIAPI
>   efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
>   			u32 input_param_block_size, u8 *input_param_block,
>   			u32 output_param_block_size, u8 *output_param_block)
> @@ -481,11 +827,16 @@ efi_tcg2_submit_command(struct efi_tcg2_protocol *this,
>    *
>    * Return:	status code
>    */
> -efi_status_t EFIAPI
> +static efi_status_t EFIAPI
>   efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
>   			      u32 *active_pcr_banks)
>   {
> -	return EFI_UNSUPPORTED;
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("%p, %p", this, active_pcr_banks);
> +	ret = __get_active_pcr_banks(active_pcr_banks);
> +
> +	return EFI_EXIT(ret);
>   }
>
>   /**
> @@ -496,7 +847,7 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this,
>    *
>    * Return:	status code
>    */
> -efi_status_t EFIAPI
> +static efi_status_t EFIAPI
>   efi_tcg2_set_active_pcr_banks(struct efi_tcg2_protocol *this,
>   			      u32 active_pcr_banks)
>   {
> @@ -515,7 +866,7 @@ efi_tcg2_set_active_pcr_banks(struct efi_tcg2_protocol *this,
>    *
>    * Return:	status code
>    */
> -efi_status_t EFIAPI
> +static efi_status_t EFIAPI
>   efi_tcg2_get_result_of_set_active_pcr_banks(struct efi_tcg2_protocol *this,
>   					    u32 *operation_present, u32 *response)
>   {
> @@ -532,6 +883,170 @@ static const struct efi_tcg2_protocol efi_tcg2_protocol = {
>   	.get_result_of_set_active_pcr_banks = efi_tcg2_get_result_of_set_active_pcr_banks,
>   };
>
> +/**
> + * create_specid_event() - Create the first event in the eventlog
> + *
> + * @dev:			tpm device
> + * @event_header:		Pointer to the final event header
> + * @event_size:			final spec event size
> + *
> + * Return:	status code
> + */
> +static efi_status_t create_specid_event(struct udevice *dev, void *buffer,
> +					size_t *event_size)
> +{
> +	struct tcg_efi_spec_id_event *spec_event;
> +	size_t spec_event_size;
> +	efi_status_t ret = EFI_DEVICE_ERROR;
> +	u32 active, supported;
> +	int err, i;
> +
> +	/*
> +	 * Create Spec event. This needs to be the first event in the log
> +	 * according to the TCG EFI protocol spec
> +	 */
> +
> +	/* Setup specID event data */
> +	spec_event = (struct tcg_efi_spec_id_event *)buffer;
> +	memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03,
> +	       sizeof(spec_event->signature));
> +	put_unaligned_le32(0, &spec_event->platform_class); /* type client */
> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2,
> +			   &spec_event->spec_version_minor);
> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2,
> +			   &spec_event->spec_version_major);
> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2,
> +			   &spec_event->spec_errata);
> +	__put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32),
> +			   &spec_event->uintn_size);
> +
> +	err = tpm2_get_pcr_info(dev, &supported, &active,
> +				&spec_event->number_of_algorithms);
> +	if (err)
> +		goto out;
> +	if (spec_event->number_of_algorithms > MAX_HASH_COUNT ||
> +	    spec_event->number_of_algorithms < 1)
> +		goto out;
> +
> +	for (i = 0; i < spec_event->number_of_algorithms; i++) {
> +		u16 hash_alg = hash_algo_list[i].hash_alg;
> +		u16 hash_len = hash_algo_list[i].hash_len;
> +
> +		if (active && alg_to_mask(hash_alg)) {
> +			put_unaligned_le16(hash_alg,
> +					   &spec_event->digest_sizes[i].algorithm_id);
> +			put_unaligned_le32(hash_len,
> +					   &spec_event->digest_sizes[i].digest_size);
> +		}
> +	}
> +	/*
> +	 * the size of the spec event and placement of vendor_info_size
> +	 * depends on supported algoriths
> +	 */
> +	spec_event_size =
> +		offsetof(struct tcg_efi_spec_id_event, digest_sizes) +
> +		spec_event->number_of_algorithms * sizeof(spec_event->digest_sizes[0]);
> +	/* no vendor info for us */
> +	memset(buffer + spec_event_size, 0,
> +	       sizeof(spec_event->vendor_info_size));
> +	spec_event_size += sizeof(spec_event->vendor_info_size);
> +	*event_size = spec_event_size;
> +
> +	return EFI_SUCCESS;
> +
> +out:
> +	return ret;
> +}
> +
> +/**
> + * create_final_event() - Create the final event and install the config
> + *			defined by the TCG EFI spec
> + */
> +static efi_status_t create_final_event(void)
> +{
> +	struct efi_tcg2_final_events_table *final_event;
> +	efi_status_t ret;
> +
> +	/*
> +	 * All events generated after the invocation of
> +	 * EFI_TCG2_GET_EVENT_LOGS need to be stored in an instance of an
> +	 * EFI_CONFIGURATION_TABLE
> +	 */
> +	ret = efi_allocate_pool(EFI_ACPI_MEMORY_NVS, TPM2_EVENT_LOG_SIZE,
> +				&event_log.final_buffer);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	memset(event_log.final_buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> +	final_event = event_log.final_buffer;
> +	final_event->number_of_events = 0;
> +	final_event->version = EFI_TCG2_FINAL_EVENTS_TABLE_VERSION;
> +	event_log.final_pos = sizeof(*final_event);
> +	ret = efi_install_configuration_table(&efi_guid_final_events,
> +					      final_event);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	return EFI_SUCCESS;
> +out:
> +	return ret;
> +}
> +
> +/**
> + * efi_init_event_log() - initialize an eventlog
> + */
> +static efi_status_t efi_init_event_log(void)
> +{
> +	/*
> +	 * vendor_info_size is currently set to 0, we need to change the length
> +	 * and allocate the flexible array member if this changes
> +	 */
> +	struct tcg_pcr_event *event_header = NULL;
> +	struct udevice *dev;
> +	size_t spec_event_size;
> +	efi_status_t ret;
> +
> +	ret = platform_get_tpm2_device(&dev);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, TPM2_EVENT_LOG_SIZE,
> +				(void **)&event_log.buffer);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +
> +	/*
> +	 * initialize log area as 0xff so the OS can easily figure out the
> +	 * last log entry
> +	 */
> +	memset(event_log.buffer, 0xff, TPM2_EVENT_LOG_SIZE);
> +	event_log.pos = 0;
> +	event_log.last_event_size = 0;
> +	event_log.get_event_called = false;
> +	event_log.truncated = false;
> +
> +	/*
> +	 * The log header is defined to be in SHA1 event log entry format.
> +	 * Setup event header
> +	 */
> +	event_header =  (struct tcg_pcr_event *)event_log.buffer;
> +	put_unaligned_le32(0, &event_header->pcr_index);
> +	put_unaligned_le32(EV_NO_ACTION, &event_header->event_type);
> +	memset(&event_header->digest, 0, sizeof(event_header->digest));
> +	ret = create_specid_event(dev, event_log.buffer + sizeof(*event_header),
> +				  &spec_event_size);
> +	if (ret != EFI_SUCCESS)
> +		goto out;
> +	put_unaligned_le32(spec_event_size, &event_header->event_size);
> +	event_log.pos = spec_event_size + sizeof(*event_header);
> +	event_log.last_event_size = event_log.pos;
> +
> +	ret = create_final_event();
> +
> +out:
> +	return ret;
> +}
> +
>   /**
>    * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
>    *
> @@ -549,6 +1064,11 @@ efi_status_t efi_tcg2_register(void)
>   		log_warning("Unable to find TPMv2 device\n");
>   		return EFI_SUCCESS;
>   	}
> +
> +	ret = efi_init_event_log();
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +
>   	ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,
>   			       (void *)&efi_tcg2_protocol);
>   	if (ret != EFI_SUCCESS)
>

[PATCH 1/3] tpm: Add tpm2 headers for TCG2 eventlog support

[Ilias Apalodimas]

Hi Heinrich,

On Sun, Nov 29, 2020 at 06:28:39AM +0100, Heinrich Schuchardt wrote:
> On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> > A following patch introduces support for the EFI_TCG2_PROTOCOL
> > evenlog management.
> 
> %s/evenlog/eventlog/
> 
> > Introduce the necessary tpm related headers
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   include/tpm-v2.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 59 insertions(+)
> > 
> > diff --git a/include/tpm-v2.h b/include/tpm-v2.h
> > index d8c9feda28dc..9637f9be998e 100644
> > --- a/include/tpm-v2.h
> > +++ b/include/tpm-v2.h
> > @@ -18,6 +18,12 @@
> > 
> >   #define TPM2_DIGEST_LEN		32
> 
> Shouldn't TPM2_DIGEST_LEN be renamed to TPM2_SHA256_DIGEST_SIZE in all
> of our code?

Ideally yes. The current tpm2 pre-existing code (apart from tpm2_pcr_extend()
which I changed) uses a hardware SHA256 algorithm.
Should we do it in this patchset though?

> 
> > 
> > +#define TPM2_SHA1_DIGEST_SIZE 20
> > +#define TPM2_SHA256_DIGEST_SIZE	32
> > +#define TPM2_SHA384_DIGEST_SIZE	48
> > +#define TPM2_SHA512_DIGEST_SIZE	64
> > +#define TPM2_SM3_256_DIGEST_SIZE 32
> > +
> >   #define TPM2_MAX_PCRS 32
> >   #define TPM2_PCR_SELECT_MAX ((TPM2_MAX_PCRS + 7) / 8)
> >   #define TPM2_MAX_CAP_BUFFER 1024
> > @@ -45,6 +51,15 @@
> >   #define TPM2_PT_MAX_COMMAND_SIZE	(u32)(TPM2_PT_FIXED + 30)
> >   #define TPM2_PT_MAX_RESPONSE_SIZE	(u32)(TPM2_PT_FIXED + 31)
> > 
> > +/* event types */
> > +#define EV_POST_CODE		((u32)0x00000001)
> > +#define EV_NO_ACTION		((u32)0x00000003)
> > +#define EV_SEPARATOR		((u32)0x00000004)
> > +#define EV_S_CRTM_CONTENTS	((u32)0x00000007)
> > +#define EV_S_CRTM_VERSION	((u32)0x00000008)
> > +#define EV_CPU_MICROCODE	((u32)0x00000009)
> > +#define EV_TABLE_OF_DEVICES	((u32)0x0000000B)
> > +
> >   /* TPMS_TAGGED_PROPERTY Structure */
> >   struct tpms_tagged_property {
> >   	u32 property;
> > @@ -86,6 +101,50 @@ struct tpms_capability_data {
> >   	union tpmu_capabilities data;
> >   } __packed;
> > 
> > +/* defined as TPM_SHA1_160_HASH_LEN in spec */
> > +struct tpm_digest {
> > +	u8 digest[TPM2_SHA1_DIGEST_SIZE];
> > +} __packed;
> > +
> > +/* SHA1 Event Log Entry Format */
> 
> Please, use Sphinx style comments for structures. This allows us to add
> them to the HTML documentation. See
> 
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
> 
> I would appreciate member descriptions, too.
> 

Ok I assumed refering to the spec would be fine.
I'll add description on v2

> > +struct tcg_pcr_event {
> > +	u32 pcr_index;
> > +	u32 event_type;
> > +	struct tpm_digest digest;
> 
> struct tpm_digest is only used here in your patch series.
> 
> The standard has
> 
>     typedef UINT8 TCG_DIGEST[TPM2_SHA1_DIGEST_SIZE];
> 
> Can't we simply write
> 
> 	u8 digest[20];
> 
> here and get rid of struct tpm_digest?

Yea we can. since it's a complex spec though I am trying to adhere
to it as much as possible to make review and future extentions easier.
I'd prefer keeping this as is tbh.

> 
> Otherwise looks ok to me.
> 

Thanks for the review!. I'll wait a few more days, fix your remarks and post a v2

Cheers
/Ilias
> Best regards
> 
> Heinrich
> 
> > +	u32 event_size;
> > +	u8 event[];
> > +} __packed;
> > +
> > +/* Definition of TPMU_HA Union */
> > +union tmpu_ha {
> > +	u8 sha1[TPM2_SHA1_DIGEST_SIZE];
> > +	u8 sha256[TPM2_SHA256_DIGEST_SIZE];
> > +	u8 sm3_256[TPM2_SM3_256_DIGEST_SIZE];
> > +	u8 sha384[TPM2_SHA384_DIGEST_SIZE];
> > +	u8 sha512[TPM2_SHA512_DIGEST_SIZE];
> > +} __packed;
> > +
> > +/* Definition of TPMT_HA Structure */
> > +struct tpmt_ha {
> > +	u16 hash_alg;
> > +	union tmpu_ha digest;
> > +} __packed;
> > +
> > +/* Definition of TPML_DIGEST_VALUES Structure */
> > +struct tpml_digest_values {
> > +	u32 count;
> > +	struct tpmt_ha digests[TPM2_NUM_PCR_BANKS];
> > +} __packed;
> > +
> > +/* Crypto Agile Log Entry Format */
> > +struct tcg_pcr_event2 {
> > +	u32 pcr_index;
> > +	u32 event_type;
> > +	struct tpml_digest_values digests;
> > +	u32 event_size;
> > +	u8 event[];
> > +} __packed;
> > +
> >   /**
> >    * TPM2 Structure Tags for command/response buffers.
> >    *
> > 
> 

[PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL

[Ilias Apalodimas]

On Sun, Nov 29, 2020 at 07:02:39AM +0100, Heinrich Schuchardt wrote:
> On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> > In the previous patches we only introduced a minimal subset of the
> > 
[...]
> > +#define TPM2_SHA1_DIGEST_SIZE 20
> > +#define TPM2_SHA256_DIGEST_SIZE 32
> > +#define TPM2_SHA384_DIGEST_SIZE 48
> > +#define TPM2_SHA512_DIGEST_SIZE 64
> 
> lib/efi_loader/efi_tcg2.c already includes tpm-v2.h.
> 
> Why should we define the same constant twice?
> 

We don't. That was probably a c/p I forgot to fix when I creted the 
patches. 
I'll only keep the declarations in tpm-v2.h.

> Best regards
> 
> Heinrich
> 
> > +
> > +#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
> > +
> > +#define TPM2_EVENT_LOG_SIZE 4096
> 
> What does this size derive from? A comment describing the constant could
> help.
> 

There's no guidance for this. This is the size of the eventlog and it's up to us
to define something that makes sense. It obviously depends on:
- Number of supported algoriths
- Number of events
We could move it to a Kconfig?

> > +
> >   typedef u32 efi_tcg_event_log_bitmap;
> >   typedef u32 efi_tcg_event_log_format;
> >   typedef u32 efi_tcg_event_algorithm_bitmap;
> > @@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability {
> >   	sizeof(struct efi_tcg2_boot_service_capability) - \
> >   	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
> > 
> > +#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03"
> > +
> > +#define tcg_efi_spec_id_event_spec_version_major_tpm2 2
> > +#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0
> > +#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0
> 
> Constants should be capitalized.
> 

Ok

> > +
> 
> Please, provide Sphinx style comments for structures. Cf.
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
> 

Ok

> > +struct tcg_efi_spec_id_event_algorithm_size {
> > +	u16      algorithm_id;
> > +	u16      digest_size;
> > +} __packed;
> > +
> > +struct tcg_efi_spec_id_event {
> > +	u8 signature[16];
> > +	u32 platform_class;
> > +	u8 spec_version_minor;
> > +	u8 spec_version_major;
> > +	u8 spec_errata;
> > +	u8 uintn_size;
> > +	u32 number_of_algorithms;
> > +	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
> > +	u8 vendor_info_size;
> > +	/*
> > +	 * filled in with vendor_info_size
> > +	 * currently vendor_info_size = 0
> 
> %s/vendor_info_size = 0/U-Boot does not provide any vendor info/
> 

Ok

[...]
> > +	/* Setup specID event data */
> > +	spec_event = (struct tcg_efi_spec_id_event *)buffer;
> > +	memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03,
> > +	       sizeof(spec_event->signature));
> > +	put_unaligned_le32(0, &spec_event->platform_class); /* type client */
> > +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2,
> > +			   &spec_event->spec_version_minor);
> > +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2,
> > +			   &spec_event->spec_version_major);
> > +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2,
> > +			   &spec_event->spec_errata);
> > +	__put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32),
> > +			   &spec_event->uintn_size);

Any preference on this?
The put_unaligned here is not strictly needed since it's u8 values. 
It just seemed 'easier' to read since all the other additions to the log
are done with put_unaligned16/32.

[...]

cheerrs
/Ilias

[PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL

[Heinrich Schuchardt]

On 11/29/20 2:27 PM, Ilias Apalodimas wrote:
> On Sun, Nov 29, 2020 at 07:02:39AM +0100, Heinrich Schuchardt wrote:
>> On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
>>> In the previous patches we only introduced a minimal subset of the
>>>
> [...]
>>> +#define TPM2_SHA1_DIGEST_SIZE 20
>>> +#define TPM2_SHA256_DIGEST_SIZE 32
>>> +#define TPM2_SHA384_DIGEST_SIZE 48
>>> +#define TPM2_SHA512_DIGEST_SIZE 64
>>
>> lib/efi_loader/efi_tcg2.c already includes tpm-v2.h.
>>
>> Why should we define the same constant twice?
>>
>
> We don't. That was probably a c/p I forgot to fix when I creted the
> patches.
> I'll only keep the declarations in tpm-v2.h.
>
>> Best regards
>>
>> Heinrich
>>
>>> +
>>> +#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
>>> +
>>> +#define TPM2_EVENT_LOG_SIZE 4096
>>
>> What does this size derive from? A comment describing the constant could
>> help.
>>
>
> There's no guidance for this. This is the size of the eventlog and it's up to us
> to define something that makes sense. It obviously depends on:
> - Number of supported algoriths
> - Number of events
> We could move it to a Kconfig?

That is probably the best choice.

>
>>> +
>>>   typedef u32 efi_tcg_event_log_bitmap;
>>>   typedef u32 efi_tcg_event_log_format;
>>>   typedef u32 efi_tcg_event_algorithm_bitmap;
>>> @@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability {
>>>   	sizeof(struct efi_tcg2_boot_service_capability) - \
>>>   	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
>>>
>>> +#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03"
>>> +
>>> +#define tcg_efi_spec_id_event_spec_version_major_tpm2 2
>>> +#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0
>>> +#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0
>>
>> Constants should be capitalized.
>>
>
> Ok
>
>>> +
>>
>> Please, provide Sphinx style comments for structures. Cf.
>> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
>>
>
> Ok
>
>>> +struct tcg_efi_spec_id_event_algorithm_size {
>>> +	u16      algorithm_id;
>>> +	u16      digest_size;
>>> +} __packed;
>>> +
>>> +struct tcg_efi_spec_id_event {
>>> +	u8 signature[16];
>>> +	u32 platform_class;
>>> +	u8 spec_version_minor;
>>> +	u8 spec_version_major;
>>> +	u8 spec_errata;
>>> +	u8 uintn_size;
>>> +	u32 number_of_algorithms;
>>> +	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
>>> +	u8 vendor_info_size;
>>> +	/*
>>> +	 * filled in with vendor_info_size
>>> +	 * currently vendor_info_size = 0
>>
>> %s/vendor_info_size = 0/U-Boot does not provide any vendor info/
>>
>
> Ok
>
> [...]
>>> +	/* Setup specID event data */
>>> +	spec_event = (struct tcg_efi_spec_id_event *)buffer;
>>> +	memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03,
>>> +	       sizeof(spec_event->signature));
>>> +	put_unaligned_le32(0, &spec_event->platform_class); /* type client */
>>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2,
>>> +			   &spec_event->spec_version_minor);
>>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2,
>>> +			   &spec_event->spec_version_major);
>>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2,
>>> +			   &spec_event->spec_errata);
>>> +	__put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32),
>>> +			   &spec_event->uintn_size);
>
> Any preference on this?
> The put_unaligned here is not strictly needed since it's u8 values.
> It just seemed 'easier' to read since all the other additions to the log
> are done with put_unaligned16/32.

U-Boot does not support unaligned access on some platforms before
allow_unaligned() is called. Furthermore endianness depends on the platform.

The current function is called after the initialization of the UEFI
sub-system (actually during the TCG2 protocol invocation). At this point
U-Boot supports unaligned access. And you know that your system is
little-endian. So you should use normal assignments which is much more
readable and reduces code size.

spec_event->platform_class = 0;
...
spec_event->uintn_size = efi_uintn_t) / sizeof(u32);

Best regards

Heinrich

>
> [...]
>
> cheerrs
> /Ilias
>

[PATCH 2/3] efi_loader: Introduce eventlog support for TCG2_PROTOCOL

[Ilias Apalodimas]

On Sun, Nov 29, 2020 at 02:49:33PM +0100, Heinrich Schuchardt wrote:
> On 11/29/20 2:27 PM, Ilias Apalodimas wrote:
> > On Sun, Nov 29, 2020 at 07:02:39AM +0100, Heinrich Schuchardt wrote:
> >> On 11/27/20 5:29 PM, Ilias Apalodimas wrote:
> >>> In the previous patches we only introduced a minimal subset of the
> >>>
> > [...]
> >>> +#define TPM2_SHA1_DIGEST_SIZE 20
> >>> +#define TPM2_SHA256_DIGEST_SIZE 32
> >>> +#define TPM2_SHA384_DIGEST_SIZE 48
> >>> +#define TPM2_SHA512_DIGEST_SIZE 64
> >>
> >> lib/efi_loader/efi_tcg2.c already includes tpm-v2.h.
> >>
> >> Why should we define the same constant twice?
> >>
> >
> > We don't. That was probably a c/p I forgot to fix when I creted the
> > patches.
> > I'll only keep the declarations in tpm-v2.h.
> >
> >> Best regards
> >>
> >> Heinrich
> >>
> >>> +
> >>> +#define EFI_TCG2_FINAL_EVENTS_TABLE_VERSION 1
> >>> +
> >>> +#define TPM2_EVENT_LOG_SIZE 4096
> >>
> >> What does this size derive from? A comment describing the constant could
> >> help.
> >>
> >
> > There's no guidance for this. This is the size of the eventlog and it's up to us
> > to define something that makes sense. It obviously depends on:
> > - Number of supported algoriths
> > - Number of events
> > We could move it to a Kconfig?
> 
> That is probably the best choice.
> 
> >
> >>> +
> >>>   typedef u32 efi_tcg_event_log_bitmap;
> >>>   typedef u32 efi_tcg_event_log_format;
> >>>   typedef u32 efi_tcg_event_algorithm_bitmap;
> >>> @@ -65,6 +76,40 @@ struct efi_tcg2_boot_service_capability {
> >>>   	sizeof(struct efi_tcg2_boot_service_capability) - \
> >>>   	offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
> >>>
> >>> +#define tcg_efi_spec_id_event_signature_03 "Spec ID Event03"
> >>> +
> >>> +#define tcg_efi_spec_id_event_spec_version_major_tpm2 2
> >>> +#define tcg_efi_spec_id_event_spec_version_minor_tpm2 0
> >>> +#define tcg_efi_spec_id_event_spec_version_errata_tpm2 0
> >>
> >> Constants should be capitalized.
> >>
> >
> > Ok
> >
> >>> +
> >>
> >> Please, provide Sphinx style comments for structures. Cf.
> >> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation
> >>
> >
> > Ok
> >
> >>> +struct tcg_efi_spec_id_event_algorithm_size {
> >>> +	u16      algorithm_id;
> >>> +	u16      digest_size;
> >>> +} __packed;
> >>> +
> >>> +struct tcg_efi_spec_id_event {
> >>> +	u8 signature[16];
> >>> +	u32 platform_class;
> >>> +	u8 spec_version_minor;
> >>> +	u8 spec_version_major;
> >>> +	u8 spec_errata;
> >>> +	u8 uintn_size;
> >>> +	u32 number_of_algorithms;
> >>> +	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
> >>> +	u8 vendor_info_size;
> >>> +	/*
> >>> +	 * filled in with vendor_info_size
> >>> +	 * currently vendor_info_size = 0
> >>
> >> %s/vendor_info_size = 0/U-Boot does not provide any vendor info/
> >>
> >
> > Ok
> >
> > [...]
> >>> +	/* Setup specID event data */
> >>> +	spec_event = (struct tcg_efi_spec_id_event *)buffer;
> >>> +	memcpy(spec_event->signature, tcg_efi_spec_id_event_signature_03,
> >>> +	       sizeof(spec_event->signature));
> >>> +	put_unaligned_le32(0, &spec_event->platform_class); /* type client */
> >>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_minor_tpm2,
> >>> +			   &spec_event->spec_version_minor);
> >>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_major_tpm2,
> >>> +			   &spec_event->spec_version_major);
> >>> +	__put_unaligned_le(tcg_efi_spec_id_event_spec_version_errata_tpm2,
> >>> +			   &spec_event->spec_errata);
> >>> +	__put_unaligned_le(sizeof(efi_uintn_t) / sizeof(u32),
> >>> +			   &spec_event->uintn_size);
> >
> > Any preference on this?
> > The put_unaligned here is not strictly needed since it's u8 values.
> > It just seemed 'easier' to read since all the other additions to the log
> > are done with put_unaligned16/32.
> 
> U-Boot does not support unaligned access on some platforms before
> allow_unaligned() is called. Furthermore endianness depends on the platform.
> 
> The current function is called after the initialization of the UEFI
> sub-system (actually during the TCG2 protocol invocation). At this point
> U-Boot supports unaligned access. And you know that your system is
> little-endian. So you should use normal assignments which is much more
> readable and reduces code size.
> 
> spec_event->platform_class = 0;

The native cpu endianess is irrelevant here. 
The eventlog data structures need to be LE if I understand the spec 
correctly [1].

Since this is not supposed to run on Arm only (or any platform that supports 
unaligned accesses) I'd prefer keeping the put_unaligned16/32 calls.
I can remove the access to u8 on v2.
Furthermore tcg2_agile_log_append() can be called outside UEFI, in case we want
to extend U-boot and measure events before initializing EFI. In that case 
we got no guarantee on alignment.

[1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
chapter 3.1 "Data Structures"

Cheers
/Ilias

> ...
> spec_event->uintn_size = efi_uintn_t) / sizeof(u32);
> 
> Best regards
> 
> Heinrich
> 
> >
> > [...]
> >
> > cheerrs
> > /Ilias
> >
>