| Message ID | 20210427130811.11121-2-masahisa.kojima@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PE/COFF measurement support | expand |
On 27.04.21 15:08, 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 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 | 9 ++++ > 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, 79 insertions(+), 75 deletions(-) > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 0b99d7c774..f012eb7718 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 Select means that you cannot switch it off. Is this really what you want? If you want the user to decide it is enabled, just make EFI_SIGNATURE_SUPPORT default y. > 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 see above > default n > help > Select this option to enable EFI secure boot support. > @@ -343,6 +345,13 @@ 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 "Enable signature verification support" > + depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE > + default n see above > + help > + Select this option to enable signature verification support. > + > 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) Why are you moving this function to a different C module? > +{ > + 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; > + Yes, lib/efi_loader/efi_var_common.c is the best place for it. Best regards Heinrich > 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 Tue, 27 Apr 2021 at 22:52, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 27.04.21 15:08, 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 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 | 9 ++++ > > 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, 79 insertions(+), 75 deletions(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 0b99d7c774..f012eb7718 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 > > Select means that you cannot switch it off. Is this really what you > want? If you want the user to decide it is enabled, just make > EFI_SIGNATURE_SUPPORT default y. EFI_SIGNATURE_SUPPORT is mandatory only when EFI_SECURE_BOOT or EFI_CAPSULE_AUTHENTICATE is enabled, so I use "select" here. > > > 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 > > see above > > > default n > > help > > Select this option to enable EFI secure boot support. > > @@ -343,6 +345,13 @@ 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 "Enable signature verification support" > > + depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE > > + default n > > see above EFI_SIGNATURE_SUPPORT is only required in the case that EFI_SECURE_BOOT or EFI_CAPSULE_AUTHENTICATE is enabled. > > > + help > > + Select this option to enable signature verification support. > > + > > 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) > > Why are you moving this function to a different C module? The major goal of this patch is exposing efi_image_loader.c::efi_image_parse() for PE/COFF image measurement. Measured Boot itself does not require EFI_SIGNATURE_SUPPORT, efi_signature.c will not be compiled. That is why I would like to move efi_image_region_add() into efi_image_loader.c, efi_image_parse() calles efi_image_region_add(). Thank you for your review. Best regards, Masahisa > > > +{ > > + 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; > > + > > Yes, lib/efi_loader/efi_var_common.c is the best place for it. > > Best regards > > Heinrich > > > 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}, > > >
Am 28. April 2021 03:06:15 MESZ schrieb Masahisa Kojima <masahisa.kojima@linaro.org>: >On Tue, 27 Apr 2021 at 22:52, Heinrich Schuchardt <xypron.glpk@gmx.de> >wrote: >> >> On 27.04.21 15:08, 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 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 | 9 ++++ >> > 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, 79 insertions(+), 75 deletions(-) >> > >> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >> > index 0b99d7c774..f012eb7718 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 >> >> Select means that you cannot switch it off. Is this really what you >> want? If you want the user to decide it is enabled, just make >> EFI_SIGNATURE_SUPPORT default y. > >EFI_SIGNATURE_SUPPORT is mandatory only when EFI_SECURE_BOOT or >EFI_CAPSULE_AUTHENTICATE is enabled, so I use "select" here. > >> >> > 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 >> >> see above >> >> > default n >> > help >> > Select this option to enable EFI secure boot support. >> > @@ -343,6 +345,13 @@ 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 "Enable signature verification support" >> > + depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE >> > + default n >> >> see above > >EFI_SIGNATURE_SUPPORT is only required in the case that >EFI_SECURE_BOOT or EFI_CAPSULE_AUTHENTICATE is enabled. > The line 'default n' is superfluous as this is default behavior. If it does not make sense to select this option manually, you would remove help and short text to hide the symbol. Best regards Heinrich >> >> > + help >> > + Select this option to enable signature verification >support. >> > + >> > 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) >> >> Why are you moving this function to a different C module? > >The major goal of this patch is exposing >efi_image_loader.c::efi_image_parse() >for PE/COFF image measurement. >Measured Boot itself does not require EFI_SIGNATURE_SUPPORT, >efi_signature.c will not be compiled. That is why I would like to move >efi_image_region_add() into efi_image_loader.c, efi_image_parse() >calles >efi_image_region_add(). > >Thank you for your review. > >Best regards, >Masahisa > >> >> > +{ >> > + 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; >> > + >> >> Yes, lib/efi_loader/efi_var_common.c is the best place for it. >> >> Best regards >> >> Heinrich >> >> > 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 Wed, 28 Apr 2021 at 11:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > Am 28. April 2021 03:06:15 MESZ schrieb Masahisa Kojima <masahisa.kojima@linaro.org>: > >On Tue, 27 Apr 2021 at 22:52, Heinrich Schuchardt <xypron.glpk@gmx.de> > >wrote: > >> > >> On 27.04.21 15:08, 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 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 | 9 ++++ > >> > 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, 79 insertions(+), 75 deletions(-) > >> > > >> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >> > index 0b99d7c774..f012eb7718 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 > >> > >> Select means that you cannot switch it off. Is this really what you > >> want? If you want the user to decide it is enabled, just make > >> EFI_SIGNATURE_SUPPORT default y. > > > >EFI_SIGNATURE_SUPPORT is mandatory only when EFI_SECURE_BOOT or > >EFI_CAPSULE_AUTHENTICATE is enabled, so I use "select" here. > > > >> > >> > 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 > >> > >> see above > >> > >> > default n > >> > help > >> > Select this option to enable EFI secure boot support. > >> > @@ -343,6 +345,13 @@ 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 "Enable signature verification support" > >> > + depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE > >> > + default n > >> > >> see above > > > >EFI_SIGNATURE_SUPPORT is only required in the case that > >EFI_SECURE_BOOT or EFI_CAPSULE_AUTHENTICATE is enabled. > > > > The line 'default n' is superfluous as this is default behavior. > > If it does not make sense to select this option manually, you would remove help and short text to hide the symbol. Hi Heinrich, You are correct, I will update and send v3. Thanks, Masahisa > > Best regards > > Heinrich > > >> > >> > + help > >> > + Select this option to enable signature verification > >support. > >> > + > >> > 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) > >> > >> Why are you moving this function to a different C module? > > > >The major goal of this patch is exposing > >efi_image_loader.c::efi_image_parse() > >for PE/COFF image measurement. > >Measured Boot itself does not require EFI_SIGNATURE_SUPPORT, > >efi_signature.c will not be compiled. That is why I would like to move > >efi_image_region_add() into efi_image_loader.c, efi_image_parse() > >calles > >efi_image_region_add(). > > > >Thank you for your review. > > > >Best regards, > >Masahisa > > > >> > >> > +{ > >> > + 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; > >> > + > >> > >> Yes, lib/efi_loader/efi_var_common.c is the best place for it. > >> > >> Best regards > >> > >> Heinrich > >> > >> > 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}, > >> > > >> >
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 0b99d7c774..f012eb7718 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,13 @@ 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 "Enable signature verification support" + depends on EFI_SECURE_BOOT || EFI_CAPSULE_AUTHENTICATE + default n + help + Select this option to enable signature verification support. + 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 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 | 9 ++++ 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, 79 insertions(+), 75 deletions(-) -- 2.17.1