Re: [PATCH v3 1/2] efi_loader: expose efi_image_parse() even if UEFI Secure Boot is disabled

On 28.04.21 14:19, Masahisa Kojima wrote:

This is preparation for PE/COFF measurement support. PE/COFF image hash calculation is same in both UEFI Secure Boot image verification and measurement in measured boot. PE/COFF image parsing functions are gathered into efi_image_loader.c, and exposed even if UEFI Secure Boot is not enabled.

This commit also adds the EFI_SIGNATURE_SUPPORT option to decide if efi_signature.c shall be compiled.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

Changes in v3:

• hide EFI_SIGNATURE_SUPPORT option

Changes in v2:

• Remove all #ifdef from efi_image_loader.c and efi_signature.c
• Add EFI_SIGNATURE_SUPPORT option
• Explicitly include <u-boot/hash-checksum.h>
• Gather PE/COFF parsing functions into efi_image_loader.c
• Move efi_guid_t efi_guid_image_security_database in efi_var_common.c

lib/efi_loader/Kconfig | 6 +++ lib/efi_loader/Makefile | 2 +- lib/efi_loader/efi_image_loader.c | 73 +++++++++++++++++++++++++++---- lib/efi_loader/efi_signature.c | 67 +--------------------------- lib/efi_loader/efi_var_common.c | 3 ++ 5 files changed, 76 insertions(+), 75 deletions(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 0b99d7c774..b76e77180e 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -174,6 +174,7 @@ config EFI_CAPSULE_AUTHENTICATE select PKCS7_MESSAGE_PARSER select PKCS7_VERIFY select IMAGE_SIGN_INFO

• select EFI_SIGNATURE_SUPPORT default n help Select this option if you want to enable capsule

@@ -336,6 +337,7 @@ config EFI_SECURE_BOOT select X509_CERTIFICATE_PARSER select PKCS7_MESSAGE_PARSER select PKCS7_VERIFY

• select EFI_SIGNATURE_SUPPORT default n help Select this option to enable EFI secure boot support.

@@ -343,6 +345,10 @@ config EFI_SECURE_BOOT it is signed with a trusted key. To do that, you need to install, at least, PK, KEK and db.

+config EFI_SIGNATURE_SUPPORT

• bool
• depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE

config EFI_ESRT bool "Enable the UEFI ESRT generation" depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 8bd343e258..fd344cea29 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -63,7 +63,7 @@ obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o -obj-y += efi_signature.o +obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o

EFI_VAR_SEED_FILE := $(subst $",,$(CONFIG_EFI_VAR_SEED_FILE)) $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE) diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index f53ef367ec..b8a790bcb9 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -213,7 +213,68 @@ static void efi_set_code_and_data_type( } }

-#ifdef CONFIG_EFI_SECURE_BOOT +/**

• • efi_image_region_add() - add an entry of region
• • @regs: Pointer to array of regions
• • @start: Start address of region (included)
• • @end: End address of region (excluded)
• • @nocheck: flag against overlapped regions
• • Take one entry of region [@start, @end[ and insert it into the list.
• • • If @nocheck is false, the list will be sorted ascending by address.
• • Overlapping entries will not be allowed.
• • • If @nocheck is true, the list will be sorted ascending by sequence
• • of adding the entries. Overlapping is allowed.
• • Return: status code
• */

+efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck)
  

+{

• struct image_region *reg;
• int i, j;
• if (regs->num >= regs->max) {

• EFI_PRINT("%s: no more room for regions\n", __func__);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• if (end < start)

• return EFI_INVALID_PARAMETER;
  

• for (i = 0; i < regs->num; i++) {

• reg = &regs->reg[i];
  

• if (nocheck)
  

• 	continue;
  

• /* new data after registered region */
  

• if (start >= reg->data + reg->size)
  

• 	continue;
  

• /* new data preceding registered region */
  

• if (end <= reg->data) {
  

• 	for (j = regs->num - 1; j >= i; j--)
  

• 		memcpy(&regs->reg[j + 1], &regs->reg[j],
  

• 		       sizeof(*reg));
  

• 	break;
  

• }
  

• /* new data overlapping registered region */
  

• EFI_PRINT("%s: new region already part of another\n", __func__);
  

• return EFI_INVALID_PARAMETER;
  

• }
• reg = &regs->reg[i];
• reg->data = start;
• reg->size = end - start;
• regs->num++;
• return EFI_SUCCESS;

+}

/**

• cmp_pe_section() - compare virtual addresses of two PE image sections
• @arg1: pointer to pointer to first section header

@@ -504,6 +565,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)

EFI_PRINT("%s: Enter, %d\n", __func__, ret);

• if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT))

• return true;
  

Why is this needed? Doesn't efi_secure_boot_enabled() return false in this case?

Best regards

Heinrich

if (!efi_secure_boot_enabled()) return true;

@@ -668,13 +732,6 @@ err: EFI_PRINT("%s: Exit, %d\n", __func__, ret); return ret; } -#else -static bool efi_image_authenticate(void *efi, size_t efi_size) -{

• return true;

-} -#endif /* CONFIG_EFI_SECURE_BOOT */

/**

• efi_check_pe() - check if a memory buffer contains a PE-COFF image

diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index c7ec275414..bdd09881fc 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -15,18 +15,16 @@ #include <crypto/public_key.h> #include <linux/compat.h> #include <linux/oid_registry.h> +#include <u-boot/hash-checksum.h> #include <u-boot/rsa.h> #include <u-boot/sha256.h>

-const efi_guid_t efi_guid_image_security_database =

• EFI_IMAGE_SECURITY_DATABASE_GUID;
  

const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID; const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID; const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;

-#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) static u8 pkcs7_hdr[] = { /* SEQUENCE */ 0x30, 0x82, 0x05, 0xc7, @@ -539,68 +537,6 @@ out: return !revoked; }

-/**

• • efi_image_region_add() - add an entry of region
• • @regs: Pointer to array of regions
• • @start: Start address of region (included)
• • @end: End address of region (excluded)
• • @nocheck: flag against overlapped regions
• • Take one entry of region [@start, @end[ and insert it into the list.
• • • If @nocheck is false, the list will be sorted ascending by address.
• • Overlapping entries will not be allowed.
• • • If @nocheck is true, the list will be sorted ascending by sequence
• • of adding the entries. Overlapping is allowed.
• • Return: status code
• */

-efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck)
  

-{

• struct image_region *reg;
• int i, j;
• if (regs->num >= regs->max) {

• EFI_PRINT("%s: no more room for regions\n", __func__);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• if (end < start)

• return EFI_INVALID_PARAMETER;
  

• for (i = 0; i < regs->num; i++) {

• reg = &regs->reg[i];
  

• if (nocheck)
  

• 	continue;
  

• /* new data after registered region */
  

• if (start >= reg->data + reg->size)
  

• 	continue;
  

• /* new data preceding registered region */
  

• if (end <= reg->data) {
  

• 	for (j = regs->num - 1; j >= i; j--)
  

• 		memcpy(&regs->reg[j + 1], &regs->reg[j],
  

• 		       sizeof(*reg));
  

• 	break;
  

• }
  

• /* new data overlapping registered region */
  

• EFI_PRINT("%s: new region already part of another\n", __func__);
  

• return EFI_INVALID_PARAMETER;
  

• }
• reg = &regs->reg[i];
• reg->data = start;
• reg->size = end - start;
• regs->num++;
• return EFI_SUCCESS;

-}

/**

• efi_sigstore_free - free signature store
• @sigstore: Pointer to signature store structure

@@ -846,4 +782,3 @@ struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name)

return efi_build_signature_store(db, db_size); } -#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */ diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index b11ed91a74..83479dd142 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -24,6 +24,9 @@ struct efi_auth_var_name_type { const enum efi_auth_var_type type; };

+const efi_guid_t efi_guid_image_security_database =

• EFI_IMAGE_SECURITY_DATABASE_GUID;
  

static const struct efi_auth_var_name_type name_type[] = { {u"PK", &efi_global_variable_guid, EFI_AUTH_VAR_PK}, {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK},