| Message ID | 20210428121945.12586-2-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | PE/COFF measurement support | expand |
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 = ®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 > @@ -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 = ®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}, >
On 4/28/21 3:16 PM, Heinrich Schuchardt wrote: > On 28.04.21 14:19, Masahisa Kojima wrote: <snip /> >> /** >> * 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? Hello Masahisa, I did not see any reply yet. Was a mail lost? Best regards Heinrich
Hi Heinrich, Sorry for the late reply. On Sat, 8 May 2021 at 23:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 4/28/21 3:16 PM, Heinrich Schuchardt wrote: > > On 28.04.21 14:19, Masahisa Kojima wrote: > <snip /> > >> /** > >> * 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? The original code is as follows. #ifdef CONFIG_EFI_SECURE_BOOT static bool efi_image_authenticate(void *efi, size_t efi_size) { < snip > } #else static bool efi_image_authenticate(void *efi, size_t efi_size) { return true; } #endif /* CONFIG_EFI_SECURE_BOOT */ The purpose of this commit is removing #if compilation switch, so I keep the original implementation, always return true if CONFIG_EFI_SECURE_BOOT is disabled. Thanks, Masahisa > > Hello Masahisa, > > I did not see any reply yet. Was a mail lost? > > Best regards > > Heinrich
On Mon, May 10, 2021 at 09:49:03AM +0900, Masahisa Kojima wrote: > Hi Heinrich, > > Sorry for the late reply. > > On Sat, 8 May 2021 at 23:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > On 4/28/21 3:16 PM, Heinrich Schuchardt wrote: > > > On 28.04.21 14:19, Masahisa Kojima wrote: > > <snip /> > > >> /** > > >> * 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? > > The original code is as follows. Heinrich's concern was, I guess, that > > >> + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) > > >> + return true; and the succeeding check, if (!efi_secure_boot_enabled()) return true; are somehow redundant. But in the latter case, I'm afraid that a compiler cannot optimize out the rest of the logic in efi_image_authenticate(). -Takahiro Akashi > #ifdef CONFIG_EFI_SECURE_BOOT > static bool efi_image_authenticate(void *efi, size_t efi_size) { > > < snip > > > } > #else > static bool efi_image_authenticate(void *efi, size_t efi_size) > { > return true; > } > #endif /* CONFIG_EFI_SECURE_BOOT */ > > The purpose of this commit is removing #if compilation switch, > so I keep the original implementation, always return true > if CONFIG_EFI_SECURE_BOOT is disabled. > > Thanks, > Masahisa > > > > > Hello Masahisa, > > > > I did not see any reply yet. Was a mail lost? > > > > Best regards > > > > Heinrich
On Mon, 10 May 2021 at 11:07, Takahiro Akashi <takahiro.akashi@linaro.org> wrote: > > On Mon, May 10, 2021 at 09:49:03AM +0900, Masahisa Kojima wrote: > > Hi Heinrich, > > > > Sorry for the late reply. > > > > On Sat, 8 May 2021 at 23:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > On 4/28/21 3:16 PM, Heinrich Schuchardt wrote: > > > > On 28.04.21 14:19, Masahisa Kojima wrote: > > > <snip /> > > > >> /** > > > >> * 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? > > > > The original code is as follows. > > Heinrich's concern was, I guess, that > > > > >> + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) > > > >> + return true; > > and the succeeding check, > > if (!efi_secure_boot_enabled()) > return true; > > are somehow redundant. > But in the latter case, I'm afraid that a compiler cannot optimize out > the rest of the logic in efi_image_authenticate(). Hi Heinrich, Takahiro, Sorry for the late reply. I now understand Takahiro's concern. If I remove following check, > + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) > + return true; compiler optimization does not work and link error occurs. lib/built-in.o: In function `efi_image_authenticate': /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:601: undefined reference to `efi_sigstore_parse_sigdb' /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:607: undefined reference to `efi_sigstore_parse_sigdb' /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:613: undefined reference to `efi_signature_lookup_digest' I would like to propose two resolution. 1) keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition 2) remove if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition and always include efi_signature.c as compilation target. Please advise. Thanks, Masahisa > > -Takahiro Akashi > > > > #ifdef CONFIG_EFI_SECURE_BOOT > > static bool efi_image_authenticate(void *efi, size_t efi_size) { > > > > < snip > > > > > } > > #else > > static bool efi_image_authenticate(void *efi, size_t efi_size) > > { > > return true; > > } > > #endif /* CONFIG_EFI_SECURE_BOOT */ > > > > The purpose of this commit is removing #if compilation switch, > > so I keep the original implementation, always return true > > if CONFIG_EFI_SECURE_BOOT is disabled. > > > > Thanks, > > Masahisa > > > > > > > > Hello Masahisa, > > > > > > I did not see any reply yet. Was a mail lost? > > > > > > Best regards > > > > > > Heinrich
Hi Heinrich, I'm about to send v4 patch series. > 1) keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition I chose this option, but I reverted #ifdef statement instead of using "if (IS_ENABLED)" because I think it is better not to rely on compiler optimization. > 2) remove if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition and > always include efi_signature.c as compilation target. In this option, CONFIG_PKCS7_VERIFY is required for EFI_TCG2_PROTOCOL just for successful build. To minimize dependency, I did not proceed with 2). Please kindly review v4. Thanks, Masahisa On Tue, 11 May 2021 at 07:06, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > > On Mon, 10 May 2021 at 11:07, Takahiro Akashi > <takahiro.akashi@linaro.org> wrote: > > > > On Mon, May 10, 2021 at 09:49:03AM +0900, Masahisa Kojima wrote: > > > Hi Heinrich, > > > > > > Sorry for the late reply. > > > > > > On Sat, 8 May 2021 at 23:08, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > On 4/28/21 3:16 PM, Heinrich Schuchardt wrote: > > > > > On 28.04.21 14:19, Masahisa Kojima wrote: > > > > <snip /> > > > > >> /** > > > > >> * 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? > > > > > > The original code is as follows. > > > > Heinrich's concern was, I guess, that > > > > > > >> + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) > > > > >> + return true; > > > > and the succeeding check, > > > > if (!efi_secure_boot_enabled()) > > return true; > > > > are somehow redundant. > > But in the latter case, I'm afraid that a compiler cannot optimize out > > the rest of the logic in efi_image_authenticate(). > > Hi Heinrich, Takahiro, > > Sorry for the late reply. > I now understand Takahiro's concern. > If I remove following check, > > > + if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) > > + return true; > > compiler optimization does not work and link error occurs. > > lib/built-in.o: In function `efi_image_authenticate': > /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:601: > undefined reference to `efi_sigstore_parse_sigdb' > /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:607: > undefined reference to `efi_sigstore_parse_sigdb' > /home/ubuntu/SynQuacer/ledge/u-boot/lib/efi_loader/efi_image_loader.c:613: > undefined reference to `efi_signature_lookup_digest' > > I would like to propose two resolution. > > 1) keep if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition > 2) remove if (!IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) condition and > always include efi_signature.c as compilation target. > > Please advise. > > Thanks, > Masahisa > > > > > > -Takahiro Akashi > > > > > > > #ifdef CONFIG_EFI_SECURE_BOOT > > > static bool efi_image_authenticate(void *efi, size_t efi_size) { > > > > > > < snip > > > > > > > } > > > #else > > > static bool efi_image_authenticate(void *efi, size_t efi_size) > > > { > > > return true; > > > } > > > #endif /* CONFIG_EFI_SECURE_BOOT */ > > > > > > The purpose of this commit is removing #if compilation switch, > > > so I keep the original implementation, always return true > > > if CONFIG_EFI_SECURE_BOOT is disabled. > > > > > > Thanks, > > > Masahisa > > > > > > > > > > > Hello Masahisa, > > > > > > > > I did not see any reply yet. Was a mail lost? > > > > > > > > Best regards > > > > > > > > Heinrich
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 = ®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 @@ -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; + 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 = ®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},
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(-)