[PATCH v5 1/3] efi_loader: expose efi_image_parse() even if UEFI Secure Boot is disabled
Masahisa Kojima
masahisa.kojima at linaro.org
Thu May 13 07:10:56 CEST 2021
On Thu, 13 May 2021 at 13:35, Heinrich Schuchardt <xypron.glpk at 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.kojima at 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},
> >
>
More information about the U-Boot
mailing list