Re: [PATCH 2/2] efi: Add basic EFI_TCG2_PROTOCOL support

Hi Heinrich

[...]

...

+#define TPM2_NUM_PCR_BANKS 16

+/* Table 22 — Definition of (UINT32) TPM_CAP Constants */ +#define TPM_CAP_FIRST 0x00000000U +#define TPM_CAP_ALGS 0x00000000U +#define TPM_CAP_HANDLES 0x00000001U +#define TPM_CAP_COMMANDS 0x00000002U +#define TPM_CAP_PP_COMMANDS 0x00000003U +#define TPM_CAP_AUDIT_COMMANDS 0x00000004U +#define TPM_CAP_PCRS 0x00000005U +#define TPM_CAP_TPM_PROPERTIES 0x00000006U +#define TPM_CAP_PCR_PROPERTIES 0x00000007U +#define TPM_CAP_ECC_CURVES 0x00000008U +#define TPM_CAP_LAST 0x00000008U +#define TPM_CAP_VENDOR_PROPERTY 0x00000100U

Thanks a lot for implementing this protocol which may be consumed by GRUB for example.

There's still some way ahead for that, but at least this implements the basics

The changes to tpm-v2.h should be in a separate patch as they are not tied to UEFI.

Ok I'll split them off in v2

[...]

...

+const efi_guid_t efi_guid_tcg2_protocol = EFI_TCG2_PROTOCOL_GUID;

+/**

• • platform_get_tpm_device() - retrieve TPM device
• • This function retrieves the udevice implementing a TPM
• • This function may be overridden if special initialization is needed.
• • @dev: udevice
• • Return: status code
• */

+__weak efi_status_t platform_get_tpm2_device(struct udevice **dev) +{

• int ret;
• struct udevice *devp;
• ret = uclass_get_device(UCLASS_TPM, 0, &devp);
• if (ret) {

• log_warning("Unable to get tpm device\n");
  

• return EFI_DEVICE_ERROR;
  

• }
• *dev = devp;
• return EFI_SUCCESS;

+}

We are talking to the DM driver. That DM driver should take care of any initialization needed to make the TPM usable. I do not understand why we need a weak function here.

I was defined as a __weak function in efi_rng.c, so I assumed that was the common practice. Shall I remove it?

...

+/**

• • tpm2_get_max_command_size() - TPM2 command to get the supported max command size
• • @dev: TPM device
• • @max_command_size: output buffer for the size
• • Return: 0 on success -1 on error
• */

+static int tpm2_get_max_command_size(struct udevice *dev, u16 *max_command_size) +{

• u8 response[COMMAND_BUFFER_SIZE];
• u32 ret;
• ret = tpm2_get_capability(dev, TPM_CAP_TPM_PROPERTIES,

• 		  TPM_PT_MAX_COMMAND_SIZE, response, 1, false);
  

• if (ret)

• return -1;
  

• /*

• * On the definition of TPMS_TAGGED_PROPERTY Structure
  

• * property: u32
  

• * value: u32
  

• * So advance the response for TPM_CAP_TPM_PROPERTIES by sizeof(u32)
  

• * to get the value
  

• */
  

• *max_command_size = (uint16_t)get_unaligned_be32(response + sizeof(u32));

Why are we using COMMAND_BUFFER_SIZE throughout the TPM code if the required buffer size for commands and responses can be read from the TPM device?

I think the logic is that 256b is enough for the basic commands we needed. I can change that here. Get the TPM response during efi_tcg2_register() and use that for the rest of the code?

...

• return 0;

+}

+/**

• • tpm2_get_max_response_size() - TPM2 command to get the supported max response size
• • @dev: TPM device
• • @max_response_size: output buffer for the size
• • Return: 0 on success -1 on error
• */

+static int tpm2_get_max_response_size(struct udevice *dev, u16 *max_response_size) +{

• u8 response[COMMAND_BUFFER_SIZE];
• u32 ret;
• ret = tpm2_get_capability(dev, TPM_CAP_TPM_PROPERTIES,

• 		  TPM_PT_MAX_RESPONSE_SIZE, response, 1, false);
  

• if (ret)

• return -1;
  

• /*

• * On the definition of TPMS_TAGGED_PROPERTY Structure
  

• * property: u32
  

• * value: u32
  

• * So advance the response for TPM_CAP_TPM_PROPERTIES by sizeof(u32)
  

• * to get the value
  

• */
  

• *max_response_size = (uint16_t)get_unaligned_be32(response + sizeof(u32));
• return 0;

+}

+/**

• • tpm2_get_manufacturer_id() - Issue a TPM2 command to get the manufacturer ID
• • @dev: TPM device
• • @manufacturer_id: output buffer for the id
• • Return: 0 on success -1 on error
• */

+static int tpm2_get_manufacturer_id(struct udevice *dev, u32 *manufacturer_id) +{

• u8 response[COMMAND_BUFFER_SIZE];
• u32 ret;
• ret = tpm2_get_capability(dev, TPM_CAP_TPM_PROPERTIES,

• 		  TPM_PT_MANUFACTURER, response, 1, false);
  

• if (ret)

• return -1;
  

• *manufacturer_id = get_unaligned_be32(response + sizeof(u32));
• return 0;

+}

+/**

• • is_active_pcr() - Check if an supported algorithm is active

%s/an supported/a supported/

Ok

...

• • @dev: TPM device
• • @selection: struct of PCR information
• • Return: true if PCR is active
• */

+bool is_active_pcr(struct tpms_pcr_selection *selection) +{

• int i;
• /*

• * check the pcr_select. If at least one of the PCRs supports the algorithm
  

• * add it on the active ones
  

• */
  

• for (i = 0; i < selection->size_of_select; i++) {

• if (selection->pcr_select[i])
  

• 	return true;
  

• }
• return false;

+}

+/**

• • tpm2_get_pcr_info() - Issue a TPM2 command to get the supported, active PCRs and number of banks
• • @dev: TPM device
• • @supported_pcr: bitmask with the algorithms supported
• • @active_pcr: bitmask with the active algorithms
• • @pcr_banks: number of PCR banks
• • Return: 0 on success -1 on error
• */

+static int tpm2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,

• 	     u32 *pcr_banks)
  

+{

• u8 response[COMMAND_BUFFER_SIZE];
• struct tpml_pcr_selection pcrs;
• u32 ret;
• int i;
• ret = tpm2_get_capability(dev, TPM_CAP_PCRS, 0, response, 1, true);
• if (ret)

• return -1;
  

• pcrs.count = get_unaligned_be32(response);
• if (pcrs.count > MAX_HASH_COUNT || pcrs.count < 1)

• return -1;
  

• for (i = 0; i < pcrs.count; i++) {

• /*
  

•  * Definition of TPMS_PCR_SELECTION Structure
  

•  * hash: u16
  

•  * size_of_select: u8
  

•  * pcr_select: u8 array
  

•  */
  

• u32 hash_offset = sizeof(pcrs.count) + i * sizeof(pcrs.selection[0]);
  

• u32 size_select_offset =
  

• 	hash_offset + offsetof(struct tpms_pcr_selection, size_of_select);
  

• u32 pcr_select_offset =
  

• 	hash_offset + offsetof(struct tpms_pcr_selection, pcr_select);
  

• pcrs.selection[i].hash = get_unaligned_be16(response + hash_offset);
  

• pcrs.selection[i].size_of_select =
  

• 	__get_unaligned_be(response + size_select_offset);
  

• /* copy the array of pcr_select */
  

• memcpy(pcrs.selection[i].pcr_select, response + pcr_select_offset,
  

•        pcrs.selection[i].size_of_select);
  

• }
• for (i = 0; i < pcrs.count; i++) {

• switch (pcrs.selection[i].hash) {
  

• case TPM2_ALG_SHA1:
  

• 	 *supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
  

• 	if (is_active_pcr(&pcrs.selection[i]))
  

• 		*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA1;
  

• 	break;
  

• case TPM2_ALG_SHA256:
  

• 	*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
  

• 	if (is_active_pcr(&pcrs.selection[i]))
  

• 		*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA256;
  

• 	break;
  

• case TPM2_ALG_SHA384:
  

• 	*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
  

• 	if (is_active_pcr(&pcrs.selection[i]))
  

• 		*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA384;
  

• 	break;
  

• case TPM2_ALG_SHA512:
  

• 	*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
  

• 	if (is_active_pcr(&pcrs.selection[i]))
  

• 		*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SHA512;
  

• 	break;
  

• case TPM2_ALG_SM3_256:
  

• 	*supported_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
  

• 	if (is_active_pcr(&pcrs.selection[i]))
  

• 		*active_pcr |= EFI_TCG2_BOOT_HASH_ALG_SM3_256;
  

• 	break;
  

• default:
  

• 	log_err("Unknown algorithm %x\n", pcrs.selection[i].hash);
  

You do not want to mess up the screen output of an EFI application.

Please, use EFI_PRINT().

Will do

...

• 	break;
  

• }
  

• }
• *pcr_banks = pcrs.count;
• return 0;

+}

+/**

• • get_capability() - provide protocol capability information and state information
• • @this: TCG2 protocol instance
• • @capability: caller allocated memory with size field to the size of the

• • 	structure allocated
    

• • Return: status code
• */

+static efi_status_t EFIAPI +get_capability(struct efi_tcg2_protocol *this,

•       struct efi_tcg2_boot_service_capability *capability)
  

+{

• struct udevice *dev;
• int ret;
• EFI_ENTRY("%p, %p", this, capability);
• if (!this || !capability)

• return EFI_EXIT(EFI_INVALID_PARAMETER);
  

• if (capability->size < boot_service_capability_min) {

• capability->size = boot_service_capability_min;
  

• return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
  

• }
• if (capability->size < sizeof(*capability)) {

• capability->size = sizeof(*capability);
  

• return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
  

• }
• capability->structure_version.major = 1;
• capability->structure_version.minor = 1;
• capability->protocol_version.major = 1;
• capability->protocol_version.minor = 1;
• ret = platform_get_tpm2_device(&dev);

The return value is of type efi_uintn_t and not int.

Yep, missed that, thanks!

...

• if (ret != EFI_SUCCESS) {

• capability->supported_event_logs = 0;
  

• capability->hash_algorithm_bitmap = 0;
  

• capability->tpm_present_flag = false;
  

• capability->max_command_size = 0;
  

• capability->max_response_size = 0;
  

• capability->manufacturer_id = 0;
  

• capability->number_of_pcr_banks = 0;
  

• capability->active_pcr_banks = 0;
  

Why don't you simply return EFI_DEVICE_ERROR if there is no device?

So actually I would write:

if (ret != EFI_SUCCESS) goto out;

Where out: is the central exit point.

Because that's what the TPM TCG2 spec says I should do, if a device is not present [1]

out: EFI_EXIT(ret);

...

• .get_result_of_set_active_pcr_banks = get_result_of_set_active_pcr_banks,

[...]

...

+};

+/**

• • efi_tcg2_register() - register EFI_TCG2_PROTOCOL
• • If a TPM2 device is available, the TPM TCG2 Protocol is registered
• • Return: An error status is only returned if adding the protocol fails.
• */

+efi_status_t efi_tcg2_register(void) +{

• int ret;

efi_unintn_t ret;

Yep, I'll fix it for v2

Best regards

Heinrich

...

• struct udevice *dev;
• enum tpm_version tpm_ver;
• ret = platform_get_tpm2_device(&dev);

The return value is of type efi_uintn_t (aka size_t).

...

• if (ret != EFI_SUCCESS)

• return EFI_SUCCESS;
  

• tpm_ver = tpm_get_version(dev);
• if (tpm_ver != TPM_V2) {

• log_warning("Only TPMv2 supported for EFI_TCG2_PROTOCOL");
  

• return EFI_SUCCESS;
  

• }
• ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol,

• 	       (void *)&efi_tcg2_protocol);
  

• if (ret != EFI_SUCCESS)

• log_err("Cannot install EFI_TCG2_PROTOCOL");
  

• return ret;

+}

[1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specificat... Section 6.4.4

Regards /Ilias