On Thu, 13 May 2021 at 13:35, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 5/12/21 1:32 PM, 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.koj...@linaro.org> > > This patch leads to an error: > > lib/crypto/x509_public_key.c:85: more undefined references to > `hash_calculate' follow > collect2: error: ld returned 1 exit status > make: *** [Makefile:1726: u-boot] Error 1 > > Applying the patches in the given sequence will break git bisect. > > Please, correct the sequence of the patches in the series.
Sorry, my patch sequence is wrong. Modification to include hash-checksum.o as compilation target must be the first patch. > > Best regards > > Heinrich > > > --- > > > > (no changes since v4) > > > > Changes in v4: > > - revert #ifdef instead of using "if (!IS_ENABLED())" statement, > > not to rely on the compiler optimization. > > > > 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 | 64 ++++++++++++++++++++++++++++- > > lib/efi_loader/efi_signature.c | 67 +------------------------------ > > lib/efi_loader/efi_var_common.c | 3 ++ > > 5 files changed, 74 insertions(+), 68 deletions(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index c259abe033..385a81d7d9 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 > > @@ -342,6 +343,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. > > @@ -349,6 +351,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..fe1ee198e2 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 = ®s->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(®s->reg[j + 1], ®s->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 = ®s->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 > > @@ -422,6 +483,7 @@ err: > > return false; > > } > > > > +#ifdef CONFIG_EFI_SECURE_BOOT > > /** > > * efi_image_unsigned_authenticate() - authenticate unsigned image with > > * SHA256 hash > > 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 = ®s->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(®s->reg[j + 1], ®s->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 = ®s->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}, > > >