| Message ID | 20200430173630.15608-7-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qemu: arm64: Add support for uefi firmware management protocol routines | expand |
Sughosh, On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote: > Add support for authenticating uefi capsules. Most of the signature > verification functionality is shared with the uefi secure boot > feature. > > The root certificate containing the public key used for the signature > verification is stored as an efi variable, similar to the variables > used for uefi secure boot. The root certificate is stored as an efi > signature list(esl) file -- this file contains the x509 certificate > which is the root certificate. > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> > --- > include/efi_api.h | 17 +++++ > include/efi_loader.h | 8 ++- > lib/efi_loader/Kconfig | 16 +++++ > lib/efi_loader/efi_capsule.c | 126 +++++++++++++++++++++++++++++++++ > lib/efi_loader/efi_signature.c | 4 +- > 5 files changed, 167 insertions(+), 4 deletions(-) > > diff --git a/include/efi_api.h b/include/efi_api.h > index e518ffa838..8dfa479db4 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -1794,6 +1794,23 @@ struct efi_variable_authentication_2 { > struct win_certificate_uefi_guid auth_info; > } __attribute__((__packed__)); > > +/** > + * efi_firmware_image_authentication - Capsule authentication method > + * descriptor > + * > + * This structure describes an authentication information for > + * a capsule with IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED set > + * and should be included as part of the capsule. > + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted. > + * > + * @monotonic_count: Count to prevent replay > + * @auth_info: Authentication info > + */ > +struct efi_firmware_image_authentication { > + uint64_t monotonic_count; > + struct win_certificate_uefi_guid auth_info; > +} __attribute__((__packed__)); > + > /** > * efi_signature_data - A format of signature > * > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 8d923451ce..897710ae3f 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -708,7 +708,7 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); > unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); > efi_status_t efi_bootmgr_load(efi_handle_t *handle); > > -#ifdef CONFIG_EFI_SECURE_BOOT > +#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > #include <image.h> > > /** > @@ -783,7 +783,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, > WIN_CERTIFICATE **auth, size_t *auth_len); > struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t buflen); > > -#endif /* CONFIG_EFI_SECURE_BOOT */ > +#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */ > > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT > /* Capsule update */ > @@ -798,6 +798,10 @@ efi_status_t EFIAPI efi_query_capsule_caps( > u32 *reset_type); > #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ > > +efi_status_t efi_capsule_authenticate(const void *capsule, > + efi_uintn_t capsule_size, > + void **image, efi_uintn_t *image_size); > + > #ifdef CONFIG_EFI_CAPSULE_ON_DISK > #define EFI_CAPSULE_DIR L"\\EFI\\UpdateCapsule\\" > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index ec2976ceba..245deabbed 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE > help > Define storage device, say 1:1, for storing FIT image > > +config EFI_CAPSULE_AUTHENTICATE > + bool "Update Capsule authentication" > + depends on EFI_HAVE_CAPSULE_SUPPORT > + depends on EFI_CAPSULE_ON_DISK > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL Do we need those two configurations? > + select SHA256 > + select RSA > + select RSA_VERIFY > + select RSA_VERIFY_WITH_PKEY > + select X509_CERTIFICATE_PARSER > + select PKCS7_MESSAGE_PARSER > + default n > + help > + Select this option if you want to enable capsule > + authentication > + > config EFI_DEVICE_PATH_TO_TEXT > bool "Device path to text protocol" > default y > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 931d363edc..a265d36ff0 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -12,6 +12,10 @@ > #include <malloc.h> > #include <mapmem.h> > #include <sort.h> > +#include "../lib/crypto/pkcs7_parser.h" > + As you might notice, the location was changed by my recent patch. > +#include <crypto/pkcs7.h> > +#include <linux/err.h> > > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > static const efi_guid_t efi_guid_firmware_management_capsule_id = > @@ -245,6 +249,128 @@ out: > > return ret; > } > + > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > + > +const efi_guid_t efi_guid_capsule_root_cert_guid = > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > + > +__weak u16 *efi_get_truststore_name(void) > +{ > + return L"CRT"; This variable is not defined by UEFI specification, isn't it? How can this variable be protected? > +} > + > +__weak const efi_guid_t *efi_get_truststore_vendor(void) > +{ > + > + return &efi_guid_capsule_root_cert_guid; > +} > + > +/** > + * efi_capsule_authenticate() - Authenticate a uefi capsule > + * > + * @capsule: Capsule file with the authentication > + * header > + * @capsule_size: Size of the entire capsule > + * @image: pointer to the image payload minus the > + * authentication header > + * @image_size: size of the image payload > + * > + * Authenticate the capsule signature with the public key contained > + * in the root certificate stored as an efi environment variable > + * > + * Return: EFI_SUCCESS on successfull authentication or > + * EFI_SECURITY_VIOLATION on authentication failure > + */ > +efi_status_t efi_capsule_authenticate(const void *capsule, > + efi_uintn_t capsule_size, > + void **image, efi_uintn_t *image_size) > +{ > + uint64_t monotonic_count; > + struct efi_signature_store *truststore; > + struct pkcs7_message *capsule_sig; > + struct efi_image_regions *regs; > + struct efi_firmware_image_authentication *auth_hdr; > + efi_status_t status; > + > + status = EFI_SECURITY_VIOLATION; > + capsule_sig = NULL; > + truststore = NULL; > + regs = NULL; > + > + /* Sanity checks */ > + if (capsule == NULL || capsule_size == 0) > + goto out; > + > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > + if (capsule_size < sizeof(*auth_hdr)) > + goto out; > + > + if (auth_hdr->auth_info.hdr.dwLength <= > + offsetof(struct win_certificate_uefi_guid, cert_data)) > + goto out; > + > + if (guidcmp(&auth_hdr->auth_info.cert_type, &efi_guid_cert_type_pkcs7)) > + goto out; > + > + *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) + > + auth_hdr->auth_info.hdr.dwLength; > + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > + sizeof(auth_hdr->monotonic_count); > + memcpy(&monotonic_count, &auth_hdr->monotonic_count, > + sizeof(monotonic_count)); > + > + /* data to be digested */ > + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, 1); > + if (!regs) > + goto out; > + > + regs->max = 2; > + efi_image_region_add(regs, (uint8_t *)*image, > + (uint8_t *)*image + *image_size, 1); > + > + efi_image_region_add(regs, (uint8_t *)&monotonic_count, > + (uint8_t *)&monotonic_count + sizeof(monotonic_count), > + 1); Is the order of regions to be calculated correct? It seems that 'monotonic_count' precedes 'image' itself in a capsule header. > + > + capsule_sig = efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, > + auth_hdr->auth_info.hdr.dwLength > + - sizeof(auth_hdr->auth_info)); > + if (IS_ERR(capsule_sig)) { As Patrick reported, ex-efi_variable_parse_signature() returns NULL on error cases, this check should be "!capsule_sig." Thanks, -Takahiro Akashi > + debug("Parsing variable's pkcs7 header failed\n"); > + capsule_sig = NULL; > + goto out; > + } > + > + truststore = efi_sigstore_parse_sigdb(efi_get_truststore_name(), > + efi_get_truststore_vendor()); > + if (!truststore) > + goto out; > + > + /* verify signature */ > + if (efi_signature_verify_with_sigdb(regs, capsule_sig, truststore, NULL)) { > + debug("Verified\n"); > + } else { > + debug("Verifying variable's signature failed\n"); > + goto out; > + } > + > + status = EFI_SUCCESS; > + > +out: > + efi_sigstore_free(truststore); > + pkcs7_free_message(capsule_sig); > + free(regs); > + > + return status; > +} > +#else > +efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, > + void **image, efi_uintn_t *image_size) > +{ > + return EFI_UNSUPPORTED; > +} > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */ > #else > static efi_status_t efi_capsule_update_firmware( > struct efi_capsule_header *capsule_data) > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > index 9897f5418e..4c722e0da9 100644 > --- a/lib/efi_loader/efi_signature.c > +++ b/lib/efi_loader/efi_signature.c > @@ -23,7 +23,7 @@ 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; > > -#ifdef CONFIG_EFI_SECURE_BOOT > +#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > static u8 pkcs7_hdr[] = { > /* SEQUENCE */ > @@ -871,4 +871,4 @@ err: > > return NULL; > } > -#endif /* CONFIG_EFI_SECURE_BOOT */ > +#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */ > -- > 2.17.1 >
On Thu, 7 May 2020 at 13:49, Akashi Takahiro <takahiro.akashi at linaro.org> wrote: > Sughosh, > > On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote: > > Add support for authenticating uefi capsules. Most of the signature > > verification functionality is shared with the uefi secure boot > > feature. > > > > The root certificate containing the public key used for the signature > > verification is stored as an efi variable, similar to the variables > > used for uefi secure boot. The root certificate is stored as an efi > > signature list(esl) file -- this file contains the x509 certificate > > which is the root certificate. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> > > --- > > include/efi_api.h | 17 +++++ > > include/efi_loader.h | 8 ++- > > lib/efi_loader/Kconfig | 16 +++++ > > lib/efi_loader/efi_capsule.c | 126 +++++++++++++++++++++++++++++++++ > > lib/efi_loader/efi_signature.c | 4 +- > > 5 files changed, 167 insertions(+), 4 deletions(-) > > > <snip> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index ec2976ceba..245deabbed 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE > > help > > Define storage device, say 1:1, for storing FIT image > > > > +config EFI_CAPSULE_AUTHENTICATE > > + bool "Update Capsule authentication" > > + depends on EFI_HAVE_CAPSULE_SUPPORT > > + depends on EFI_CAPSULE_ON_DISK > > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL > > Do we need those two configurations? > Right, I think we can just do with the EFI_CAPSULE_ON_DISK. Will change. > > > + select SHA256 > > + select RSA > > + select RSA_VERIFY > > + select RSA_VERIFY_WITH_PKEY > > + select X509_CERTIFICATE_PARSER > > + select PKCS7_MESSAGE_PARSER > > + default n > > + help > > + Select this option if you want to enable capsule > > + authentication > > + > > config EFI_DEVICE_PATH_TO_TEXT > > bool "Device path to text protocol" > > default y > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 931d363edc..a265d36ff0 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -12,6 +12,10 @@ > > #include <malloc.h> > > #include <mapmem.h> > > #include <sort.h> > > +#include "../lib/crypto/pkcs7_parser.h" > > + > > As you might notice, the location was changed by > my recent patch. > Will check and update. > > > +#include <crypto/pkcs7.h> > > +#include <linux/err.h> > > > > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > > static const efi_guid_t efi_guid_firmware_management_capsule_id = > > @@ -245,6 +249,128 @@ out: > > > > return ret; > > } > > + > > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > + > > +const efi_guid_t efi_guid_capsule_root_cert_guid = > > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > + > > +__weak u16 *efi_get_truststore_name(void) > > +{ > > + return L"CRT"; > > This variable is not defined by UEFI specification, isn't it? > How can this variable be protected? > This is not part of the uefi specification. The uefi specification does not mandate any particular method for storing the root certificate to be used for capsule authentication. The edk2 code gets the root certificate through a pcd. I had tried using KEK variable for storing the root certificate, and had also come up with an implementation. However, the addition/deletion and update of the secure variables is very closely tied with the secure boot feature and the secure boot state of the system, which is the reason why i dropped that solution and used a separate variable, which can be defined by the vendor to store the root certificate. > > > +} > > + > > +__weak const efi_guid_t *efi_get_truststore_vendor(void) > > +{ > > + > > + return &efi_guid_capsule_root_cert_guid; > > +} > > + > > +/** > > + * efi_capsule_authenticate() - Authenticate a uefi capsule > > + * > > + * @capsule: Capsule file with the authentication > > + * header > > + * @capsule_size: Size of the entire capsule > > + * @image: pointer to the image payload minus the > > + * authentication header > > + * @image_size: size of the image payload > > + * > > + * Authenticate the capsule signature with the public key contained > > + * in the root certificate stored as an efi environment variable > > + * > > + * Return: EFI_SUCCESS on successfull authentication or > > + * EFI_SECURITY_VIOLATION on authentication failure > > + */ > > +efi_status_t efi_capsule_authenticate(const void *capsule, > > + efi_uintn_t capsule_size, > > + void **image, efi_uintn_t > *image_size) > > +{ > > + uint64_t monotonic_count; > > + struct efi_signature_store *truststore; > > + struct pkcs7_message *capsule_sig; > > + struct efi_image_regions *regs; > > + struct efi_firmware_image_authentication *auth_hdr; > > + efi_status_t status; > > + > > + status = EFI_SECURITY_VIOLATION; > > + capsule_sig = NULL; > > + truststore = NULL; > > + regs = NULL; > > + > > + /* Sanity checks */ > > + if (capsule == NULL || capsule_size == 0) > > + goto out; > > + > > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > + if (capsule_size < sizeof(*auth_hdr)) > > + goto out; > > + > > + if (auth_hdr->auth_info.hdr.dwLength <= > > + offsetof(struct win_certificate_uefi_guid, cert_data)) > > + goto out; > > + > > + if (guidcmp(&auth_hdr->auth_info.cert_type, > &efi_guid_cert_type_pkcs7)) > > + goto out; > > + > > + *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) + > > + auth_hdr->auth_info.hdr.dwLength; > > + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > > + sizeof(auth_hdr->monotonic_count); > > + memcpy(&monotonic_count, &auth_hdr->monotonic_count, > > + sizeof(monotonic_count)); > > + > > + /* data to be digested */ > > + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, 1); > > + if (!regs) > > + goto out; > > + > > + regs->max = 2; > > + efi_image_region_add(regs, (uint8_t *)*image, > > + (uint8_t *)*image + *image_size, 1); > > + > > + efi_image_region_add(regs, (uint8_t *)&monotonic_count, > > + (uint8_t *)&monotonic_count + > sizeof(monotonic_count), > > + 1); > > Is the order of regions to be calculated correct? > It seems that 'monotonic_count' precedes 'image' itself > in a capsule header. > Does that have any impact on the hash value computed. I did not see any difference in the hash value computed due to the order in which the regions are added. But that could be because of the way the hash value gets computed at the time of building the capsule. I will check this out. > > > + > > + capsule_sig = efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, > > + > auth_hdr->auth_info.hdr.dwLength > > + - > sizeof(auth_hdr->auth_info)); > > + if (IS_ERR(capsule_sig)) { > > As Patrick reported, ex-efi_variable_parse_signature() > returns NULL on error cases, this check should be "!capsule_sig." > Will change. -sughosh
On Thu, May 07, 2020 at 05:20:35PM +0530, Sughosh Ganu wrote: > On Thu, 7 May 2020 at 13:49, Akashi Takahiro <takahiro.akashi at linaro.org> > wrote: > > > Sughosh, > > > > On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote: > > > Add support for authenticating uefi capsules. Most of the signature > > > verification functionality is shared with the uefi secure boot > > > feature. > > > > > > The root certificate containing the public key used for the signature > > > verification is stored as an efi variable, similar to the variables > > > used for uefi secure boot. The root certificate is stored as an efi > > > signature list(esl) file -- this file contains the x509 certificate > > > which is the root certificate. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> > > > --- > > > include/efi_api.h | 17 +++++ > > > include/efi_loader.h | 8 ++- > > > lib/efi_loader/Kconfig | 16 +++++ > > > lib/efi_loader/efi_capsule.c | 126 +++++++++++++++++++++++++++++++++ > > > lib/efi_loader/efi_signature.c | 4 +- > > > 5 files changed, 167 insertions(+), 4 deletions(-) > > > > > > > <snip> > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index ec2976ceba..245deabbed 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE > > > help > > > Define storage device, say 1:1, for storing FIT image > > > > > > +config EFI_CAPSULE_AUTHENTICATE > > > + bool "Update Capsule authentication" > > > + depends on EFI_HAVE_CAPSULE_SUPPORT > > > + depends on EFI_CAPSULE_ON_DISK > > > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL > > > > Do we need those two configurations? > > > > Right, I think we can just do with the EFI_CAPSULE_ON_DISK. Will change. Actually I don't think we need EFI_CAPSULE_ON_DISK neither. > > > > > > + select SHA256 > > > + select RSA > > > + select RSA_VERIFY > > > + select RSA_VERIFY_WITH_PKEY > > > + select X509_CERTIFICATE_PARSER > > > + select PKCS7_MESSAGE_PARSER > > > + default n > > > + help > > > + Select this option if you want to enable capsule > > > + authentication > > > + > > > config EFI_DEVICE_PATH_TO_TEXT > > > bool "Device path to text protocol" > > > default y > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > > index 931d363edc..a265d36ff0 100644 > > > --- a/lib/efi_loader/efi_capsule.c > > > +++ b/lib/efi_loader/efi_capsule.c > > > @@ -12,6 +12,10 @@ > > > #include <malloc.h> > > > #include <mapmem.h> > > > #include <sort.h> > > > +#include "../lib/crypto/pkcs7_parser.h" > > > + > > > > As you might notice, the location was changed by > > my recent patch. > > > > Will check and update. > > > > > > > +#include <crypto/pkcs7.h> > > > +#include <linux/err.h> > > > > > > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > > > static const efi_guid_t efi_guid_firmware_management_capsule_id = > > > @@ -245,6 +249,128 @@ out: > > > > > > return ret; > > > } > > > + > > > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > > + > > > +const efi_guid_t efi_guid_capsule_root_cert_guid = > > > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > + > > > +__weak u16 *efi_get_truststore_name(void) > > > +{ > > > + return L"CRT"; > > > > This variable is not defined by UEFI specification, isn't it? > > How can this variable be protected? > > > > This is not part of the uefi specification. The uefi specification does not > mandate any particular method for storing the root certificate to be used > for capsule authentication. The edk2 code gets the root certificate through > a pcd. I had tried using KEK variable for storing the root certificate, and > had also come up with an implementation. However, the addition/deletion and > update of the secure variables is very closely tied with the secure boot > feature and the secure boot state of the system, which is the reason why i > dropped that solution and used a separate variable, which can be defined by > the vendor to store the root certificate. My concern is that, without any protection, the value of this variable can be modified by a mal-user. (One simple solution would be to set this variable read-only.) > > > > > > +} > > > + > > > +__weak const efi_guid_t *efi_get_truststore_vendor(void) > > > +{ > > > + > > > + return &efi_guid_capsule_root_cert_guid; > > > +} > > > + > > > +/** > > > + * efi_capsule_authenticate() - Authenticate a uefi capsule > > > + * > > > + * @capsule: Capsule file with the authentication > > > + * header > > > + * @capsule_size: Size of the entire capsule > > > + * @image: pointer to the image payload minus the > > > + * authentication header > > > + * @image_size: size of the image payload > > > + * > > > + * Authenticate the capsule signature with the public key contained > > > + * in the root certificate stored as an efi environment variable > > > + * > > > + * Return: EFI_SUCCESS on successfull authentication or > > > + * EFI_SECURITY_VIOLATION on authentication failure > > > + */ > > > +efi_status_t efi_capsule_authenticate(const void *capsule, > > > + efi_uintn_t capsule_size, > > > + void **image, efi_uintn_t > > *image_size) > > > +{ > > > + uint64_t monotonic_count; > > > + struct efi_signature_store *truststore; > > > + struct pkcs7_message *capsule_sig; > > > + struct efi_image_regions *regs; > > > + struct efi_firmware_image_authentication *auth_hdr; > > > + efi_status_t status; > > > + > > > + status = EFI_SECURITY_VIOLATION; > > > + capsule_sig = NULL; > > > + truststore = NULL; > > > + regs = NULL; > > > + > > > + /* Sanity checks */ > > > + if (capsule == NULL || capsule_size == 0) > > > + goto out; > > > + > > > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > > + if (capsule_size < sizeof(*auth_hdr)) > > > + goto out; > > > + > > > + if (auth_hdr->auth_info.hdr.dwLength <= > > > + offsetof(struct win_certificate_uefi_guid, cert_data)) > > > + goto out; > > > + > > > + if (guidcmp(&auth_hdr->auth_info.cert_type, > > &efi_guid_cert_type_pkcs7)) > > > + goto out; > > > + > > > + *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) + > > > + auth_hdr->auth_info.hdr.dwLength; > > > + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > > > + sizeof(auth_hdr->monotonic_count); > > > + memcpy(&monotonic_count, &auth_hdr->monotonic_count, > > > + sizeof(monotonic_count)); > > > + > > > + /* data to be digested */ > > > + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, 1); > > > + if (!regs) > > > + goto out; > > > + > > > + regs->max = 2; > > > + efi_image_region_add(regs, (uint8_t *)*image, > > > + (uint8_t *)*image + *image_size, 1); > > > + > > > + efi_image_region_add(regs, (uint8_t *)&monotonic_count, > > > + (uint8_t *)&monotonic_count + > > sizeof(monotonic_count), > > > + 1); > > > > Is the order of regions to be calculated correct? > > It seems that 'monotonic_count' precedes 'image' itself > > in a capsule header. > > > > Does that have any impact on the hash value computed. Not 100% sure, but if it doesn't, it cannot guarantee uniqueness of digest values. Thanks, -Takahiro Akashi > I did not see any > difference in the hash value computed due to the order in which the regions > are added. But that could be because of the way the hash value gets > computed at the time of building the capsule. I will check this out. > > > > > > > + > > > + capsule_sig = efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, > > > + > > auth_hdr->auth_info.hdr.dwLength > > > + - > > sizeof(auth_hdr->auth_info)); > > > + if (IS_ERR(capsule_sig)) { > > > > As Patrick reported, ex-efi_variable_parse_signature() > > returns NULL on error cases, this check should be "!capsule_sig." > > > > Will change. > > -sughosh
On Fri, 8 May 2020 at 06:12, Akashi Takahiro <takahiro.akashi at linaro.org> wrote: > On Thu, May 07, 2020 at 05:20:35PM +0530, Sughosh Ganu wrote: > > On Thu, 7 May 2020 at 13:49, Akashi Takahiro <takahiro.akashi at linaro.org > > > > wrote: > > > > > Sughosh, > > > > > > On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote: > > > > Add support for authenticating uefi capsules. Most of the signature > > > > verification functionality is shared with the uefi secure boot > > > > feature. > > > > > > > > The root certificate containing the public key used for the signature > > > > verification is stored as an efi variable, similar to the variables > > > > used for uefi secure boot. The root certificate is stored as an efi > > > > signature list(esl) file -- this file contains the x509 certificate > > > > which is the root certificate. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> > > > > --- > > > > include/efi_api.h | 17 +++++ > > > > include/efi_loader.h | 8 ++- > > > > lib/efi_loader/Kconfig | 16 +++++ > > > > lib/efi_loader/efi_capsule.c | 126 > +++++++++++++++++++++++++++++++++ > > > > lib/efi_loader/efi_signature.c | 4 +- > > > > 5 files changed, 167 insertions(+), 4 deletions(-) > > > > > > > > > > > <snip> > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > index ec2976ceba..245deabbed 100644 > > > > --- a/lib/efi_loader/Kconfig > > > > +++ b/lib/efi_loader/Kconfig > > > > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE > > > > help > > > > Define storage device, say 1:1, for storing FIT image > > > > > > > > +config EFI_CAPSULE_AUTHENTICATE > > > > + bool "Update Capsule authentication" > > > > + depends on EFI_HAVE_CAPSULE_SUPPORT > > > > + depends on EFI_CAPSULE_ON_DISK > > > > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL > > > > > > Do we need those two configurations? > > > > > > > Right, I think we can just do with the EFI_CAPSULE_ON_DISK. Will change. > > Actually I don't think we need EFI_CAPSULE_ON_DISK neither. > We at least need EFI_HAVE_CAPSULE_SUPPORT, isn't it. > > > > > > > > > > + select SHA256 > > > > + select RSA > > > > + select RSA_VERIFY > > > > + select RSA_VERIFY_WITH_PKEY > > > > + select X509_CERTIFICATE_PARSER > > > > + select PKCS7_MESSAGE_PARSER > > > > + default n > > > > + help > > > > + Select this option if you want to enable capsule > > > > + authentication > > > > + > > > > config EFI_DEVICE_PATH_TO_TEXT > > > > bool "Device path to text protocol" > > > > default y > > > > diff --git a/lib/efi_loader/efi_capsule.c > b/lib/efi_loader/efi_capsule.c > > > > index 931d363edc..a265d36ff0 100644 > > > > --- a/lib/efi_loader/efi_capsule.c > > > > +++ b/lib/efi_loader/efi_capsule.c > > > > @@ -12,6 +12,10 @@ > > > > #include <malloc.h> > > > > #include <mapmem.h> > > > > #include <sort.h> > > > > +#include "../lib/crypto/pkcs7_parser.h" > > > > + > > > > > > As you might notice, the location was changed by > > > my recent patch. > > > > > > > Will check and update. > > > > > > > > > > > +#include <crypto/pkcs7.h> > > > > +#include <linux/err.h> > > > > > > > > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > > > > static const efi_guid_t efi_guid_firmware_management_capsule_id = > > > > @@ -245,6 +249,128 @@ out: > > > > > > > > return ret; > > > > } > > > > + > > > > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > > > + > > > > +const efi_guid_t efi_guid_capsule_root_cert_guid = > > > > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > > + > > > > +__weak u16 *efi_get_truststore_name(void) > > > > +{ > > > > + return L"CRT"; > > > > > > This variable is not defined by UEFI specification, isn't it? > > > How can this variable be protected? > > > > > > > This is not part of the uefi specification. The uefi specification does > not > > mandate any particular method for storing the root certificate to be used > > for capsule authentication. The edk2 code gets the root certificate > through > > a pcd. I had tried using KEK variable for storing the root certificate, > and > > had also come up with an implementation. However, the addition/deletion > and > > update of the secure variables is very closely tied with the secure boot > > feature and the secure boot state of the system, which is the reason why > i > > dropped that solution and used a separate variable, which can be defined > by > > the vendor to store the root certificate. > > My concern is that, without any protection, the value of this variable > can be modified by a mal-user. > (One simple solution would be to set this variable read-only.) > Yes, that is one solution. This will also be taken care of in a scenario where the platform is booted in a trusted chain, and the env is also part of the image which gets verified by the previous stage(e.g BL2) before jumping to the u-boot image. If there is any change in the u-boot image, that would result in a boot failure -- but that would need the env to be part of the u-boot image which gets verified. I will explore making this variable read-only. > > > > > > > > > +} > > > > + > > > > +__weak const efi_guid_t *efi_get_truststore_vendor(void) > > > > +{ > > > > + > > > > + return &efi_guid_capsule_root_cert_guid; > > > > +} > > > > + > > > > +/** > > > > + * efi_capsule_authenticate() - Authenticate a uefi capsule > > > > + * > > > > + * @capsule: Capsule file with the authentication > > > > + * header > > > > + * @capsule_size: Size of the entire capsule > > > > + * @image: pointer to the image payload minus the > > > > + * authentication header > > > > + * @image_size: size of the image payload > > > > + * > > > > + * Authenticate the capsule signature with the public key contained > > > > + * in the root certificate stored as an efi environment variable > > > > + * > > > > + * Return: EFI_SUCCESS on successfull authentication or > > > > + * EFI_SECURITY_VIOLATION on authentication failure > > > > + */ > > > > +efi_status_t efi_capsule_authenticate(const void *capsule, > > > > + efi_uintn_t capsule_size, > > > > + void **image, efi_uintn_t > > > *image_size) > > > > +{ > > > > + uint64_t monotonic_count; > > > > + struct efi_signature_store *truststore; > > > > + struct pkcs7_message *capsule_sig; > > > > + struct efi_image_regions *regs; > > > > + struct efi_firmware_image_authentication *auth_hdr; > > > > + efi_status_t status; > > > > + > > > > + status = EFI_SECURITY_VIOLATION; > > > > + capsule_sig = NULL; > > > > + truststore = NULL; > > > > + regs = NULL; > > > > + > > > > + /* Sanity checks */ > > > > + if (capsule == NULL || capsule_size == 0) > > > > + goto out; > > > > + > > > > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > > > + if (capsule_size < sizeof(*auth_hdr)) > > > > + goto out; > > > > + > > > > + if (auth_hdr->auth_info.hdr.dwLength <= > > > > + offsetof(struct win_certificate_uefi_guid, cert_data)) > > > > + goto out; > > > > + > > > > + if (guidcmp(&auth_hdr->auth_info.cert_type, > > > &efi_guid_cert_type_pkcs7)) > > > > + goto out; > > > > + > > > > + *image = (uint8_t *)capsule + > sizeof(auth_hdr->monotonic_count) + > > > > + auth_hdr->auth_info.hdr.dwLength; > > > > + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > > > > + > sizeof(auth_hdr->monotonic_count); > > > > + memcpy(&monotonic_count, &auth_hdr->monotonic_count, > > > > + sizeof(monotonic_count)); > > > > + > > > > + /* data to be digested */ > > > > + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, > 1); > > > > + if (!regs) > > > > + goto out; > > > > + > > > > + regs->max = 2; > > > > + efi_image_region_add(regs, (uint8_t *)*image, > > > > + (uint8_t *)*image + *image_size, 1); > > > > + > > > > + efi_image_region_add(regs, (uint8_t *)&monotonic_count, > > > > + (uint8_t *)&monotonic_count + > > > sizeof(monotonic_count), > > > > + 1); > > > > > > Is the order of regions to be calculated correct? > > > It seems that 'monotonic_count' precedes 'image' itself > > > in a capsule header. > > > > > > > Does that have any impact on the hash value computed. > > Not 100% sure, but if it doesn't, it cannot guarantee uniqueness > of digest values. > Will check this. -sughosh > > Thanks, > -Takahiro Akashi > > > I did not see any > > difference in the hash value computed due to the order in which the > regions > > are added. But that could be because of the way the hash value gets > > computed at the time of building the capsule. I will check this out. > > > > > > > > > > > + > > > > + capsule_sig = > efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, > > > > + > > > auth_hdr->auth_info.hdr.dwLength > > > > + - > > > sizeof(auth_hdr->auth_info)); > > > > + if (IS_ERR(capsule_sig)) { > > > > > > As Patrick reported, ex-efi_variable_parse_signature() > > > returns NULL on error cases, this check should be "!capsule_sig." > > > > > > > Will change. > > > > -sughosh >
On Sun, May 10, 2020 at 04:56:21PM +0530, Sughosh Ganu wrote: > On Fri, 8 May 2020 at 06:12, Akashi Takahiro <takahiro.akashi at linaro.org> > wrote: > > > On Thu, May 07, 2020 at 05:20:35PM +0530, Sughosh Ganu wrote: > > > On Thu, 7 May 2020 at 13:49, Akashi Takahiro <takahiro.akashi at linaro.org > > > > > > wrote: > > > > > > > Sughosh, > > > > > > > > On Thu, Apr 30, 2020 at 11:06:28PM +0530, Sughosh Ganu wrote: > > > > > Add support for authenticating uefi capsules. Most of the signature > > > > > verification functionality is shared with the uefi secure boot > > > > > feature. > > > > > > > > > > The root certificate containing the public key used for the signature > > > > > verification is stored as an efi variable, similar to the variables > > > > > used for uefi secure boot. The root certificate is stored as an efi > > > > > signature list(esl) file -- this file contains the x509 certificate > > > > > which is the root certificate. > > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> > > > > > --- > > > > > include/efi_api.h | 17 +++++ > > > > > include/efi_loader.h | 8 ++- > > > > > lib/efi_loader/Kconfig | 16 +++++ > > > > > lib/efi_loader/efi_capsule.c | 126 > > +++++++++++++++++++++++++++++++++ > > > > > lib/efi_loader/efi_signature.c | 4 +- > > > > > 5 files changed, 167 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > > <snip> > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > > index ec2976ceba..245deabbed 100644 > > > > > --- a/lib/efi_loader/Kconfig > > > > > +++ b/lib/efi_loader/Kconfig > > > > > @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE > > > > > help > > > > > Define storage device, say 1:1, for storing FIT image > > > > > > > > > > +config EFI_CAPSULE_AUTHENTICATE > > > > > + bool "Update Capsule authentication" > > > > > + depends on EFI_HAVE_CAPSULE_SUPPORT > > > > > + depends on EFI_CAPSULE_ON_DISK > > > > > + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL > > > > > > > > Do we need those two configurations? > > > > > > > > > > Right, I think we can just do with the EFI_CAPSULE_ON_DISK. Will change. > > > > Actually I don't think we need EFI_CAPSULE_ON_DISK neither. > > > > We at least need EFI_HAVE_CAPSULE_SUPPORT, isn't it. Ah, yes of course. -Takahiro Akashi > > > > > > > > > > > > > > > + select SHA256 > > > > > + select RSA > > > > > + select RSA_VERIFY > > > > > + select RSA_VERIFY_WITH_PKEY > > > > > + select X509_CERTIFICATE_PARSER > > > > > + select PKCS7_MESSAGE_PARSER > > > > > + default n > > > > > + help > > > > > + Select this option if you want to enable capsule > > > > > + authentication > > > > > + > > > > > config EFI_DEVICE_PATH_TO_TEXT > > > > > bool "Device path to text protocol" > > > > > default y > > > > > diff --git a/lib/efi_loader/efi_capsule.c > > b/lib/efi_loader/efi_capsule.c > > > > > index 931d363edc..a265d36ff0 100644 > > > > > --- a/lib/efi_loader/efi_capsule.c > > > > > +++ b/lib/efi_loader/efi_capsule.c > > > > > @@ -12,6 +12,10 @@ > > > > > #include <malloc.h> > > > > > #include <mapmem.h> > > > > > #include <sort.h> > > > > > +#include "../lib/crypto/pkcs7_parser.h" > > > > > + > > > > > > > > As you might notice, the location was changed by > > > > my recent patch. > > > > > > > > > > Will check and update. > > > > > > > > > > > > > > > +#include <crypto/pkcs7.h> > > > > > +#include <linux/err.h> > > > > > > > > > > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > > > > > static const efi_guid_t efi_guid_firmware_management_capsule_id = > > > > > @@ -245,6 +249,128 @@ out: > > > > > > > > > > return ret; > > > > > } > > > > > + > > > > > +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > > > > + > > > > > +const efi_guid_t efi_guid_capsule_root_cert_guid = > > > > > + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > > > > + > > > > > +__weak u16 *efi_get_truststore_name(void) > > > > > +{ > > > > > + return L"CRT"; > > > > > > > > This variable is not defined by UEFI specification, isn't it? > > > > How can this variable be protected? > > > > > > > > > > This is not part of the uefi specification. The uefi specification does > > not > > > mandate any particular method for storing the root certificate to be used > > > for capsule authentication. The edk2 code gets the root certificate > > through > > > a pcd. I had tried using KEK variable for storing the root certificate, > > and > > > had also come up with an implementation. However, the addition/deletion > > and > > > update of the secure variables is very closely tied with the secure boot > > > feature and the secure boot state of the system, which is the reason why > > i > > > dropped that solution and used a separate variable, which can be defined > > by > > > the vendor to store the root certificate. > > > > My concern is that, without any protection, the value of this variable > > can be modified by a mal-user. > > (One simple solution would be to set this variable read-only.) > > > > Yes, that is one solution. This will also be taken care of in a scenario > where the platform is booted in a trusted chain, and the env is also part > of the image which gets verified by the previous stage(e.g BL2) before > jumping to the u-boot image. If there is any change in the u-boot image, > that would result in a boot failure -- but that would need the env to be > part of the u-boot image which gets verified. I will explore making this > variable read-only. > > > > > > > > > > > > > > +} > > > > > + > > > > > +__weak const efi_guid_t *efi_get_truststore_vendor(void) > > > > > +{ > > > > > + > > > > > + return &efi_guid_capsule_root_cert_guid; > > > > > +} > > > > > + > > > > > +/** > > > > > + * efi_capsule_authenticate() - Authenticate a uefi capsule > > > > > + * > > > > > + * @capsule: Capsule file with the authentication > > > > > + * header > > > > > + * @capsule_size: Size of the entire capsule > > > > > + * @image: pointer to the image payload minus the > > > > > + * authentication header > > > > > + * @image_size: size of the image payload > > > > > + * > > > > > + * Authenticate the capsule signature with the public key contained > > > > > + * in the root certificate stored as an efi environment variable > > > > > + * > > > > > + * Return: EFI_SUCCESS on successfull authentication or > > > > > + * EFI_SECURITY_VIOLATION on authentication failure > > > > > + */ > > > > > +efi_status_t efi_capsule_authenticate(const void *capsule, > > > > > + efi_uintn_t capsule_size, > > > > > + void **image, efi_uintn_t > > > > *image_size) > > > > > +{ > > > > > + uint64_t monotonic_count; > > > > > + struct efi_signature_store *truststore; > > > > > + struct pkcs7_message *capsule_sig; > > > > > + struct efi_image_regions *regs; > > > > > + struct efi_firmware_image_authentication *auth_hdr; > > > > > + efi_status_t status; > > > > > + > > > > > + status = EFI_SECURITY_VIOLATION; > > > > > + capsule_sig = NULL; > > > > > + truststore = NULL; > > > > > + regs = NULL; > > > > > + > > > > > + /* Sanity checks */ > > > > > + if (capsule == NULL || capsule_size == 0) > > > > > + goto out; > > > > > + > > > > > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > > > > + if (capsule_size < sizeof(*auth_hdr)) > > > > > + goto out; > > > > > + > > > > > + if (auth_hdr->auth_info.hdr.dwLength <= > > > > > + offsetof(struct win_certificate_uefi_guid, cert_data)) > > > > > + goto out; > > > > > + > > > > > + if (guidcmp(&auth_hdr->auth_info.cert_type, > > > > &efi_guid_cert_type_pkcs7)) > > > > > + goto out; > > > > > + > > > > > + *image = (uint8_t *)capsule + > > sizeof(auth_hdr->monotonic_count) + > > > > > + auth_hdr->auth_info.hdr.dwLength; > > > > > + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > > > > > + > > sizeof(auth_hdr->monotonic_count); > > > > > + memcpy(&monotonic_count, &auth_hdr->monotonic_count, > > > > > + sizeof(monotonic_count)); > > > > > + > > > > > + /* data to be digested */ > > > > > + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, > > 1); > > > > > + if (!regs) > > > > > + goto out; > > > > > + > > > > > + regs->max = 2; > > > > > + efi_image_region_add(regs, (uint8_t *)*image, > > > > > + (uint8_t *)*image + *image_size, 1); > > > > > + > > > > > + efi_image_region_add(regs, (uint8_t *)&monotonic_count, > > > > > + (uint8_t *)&monotonic_count + > > > > sizeof(monotonic_count), > > > > > + 1); > > > > > > > > Is the order of regions to be calculated correct? > > > > It seems that 'monotonic_count' precedes 'image' itself > > > > in a capsule header. > > > > > > > > > > Does that have any impact on the hash value computed. > > > > Not 100% sure, but if it doesn't, it cannot guarantee uniqueness > > of digest values. > > > > Will check this. > > -sughosh > > > > > > Thanks, > > -Takahiro Akashi > > > > > I did not see any > > > difference in the hash value computed due to the order in which the > > regions > > > are added. But that could be because of the way the hash value gets > > > computed at the time of building the capsule. I will check this out. > > > > > > > > > > > > > > > + > > > > > + capsule_sig = > > efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, > > > > > + > > > > auth_hdr->auth_info.hdr.dwLength > > > > > + - > > > > sizeof(auth_hdr->auth_info)); > > > > > + if (IS_ERR(capsule_sig)) { > > > > > > > > As Patrick reported, ex-efi_variable_parse_signature() > > > > returns NULL on error cases, this check should be "!capsule_sig." > > > > > > > > > > Will change. > > > > > > -sughosh > >
diff --git a/include/efi_api.h b/include/efi_api.h index e518ffa838..8dfa479db4 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1794,6 +1794,23 @@ struct efi_variable_authentication_2 { struct win_certificate_uefi_guid auth_info; } __attribute__((__packed__)); +/** + * efi_firmware_image_authentication - Capsule authentication method + * descriptor + * + * This structure describes an authentication information for + * a capsule with IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED set + * and should be included as part of the capsule. + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted. + * + * @monotonic_count: Count to prevent replay + * @auth_info: Authentication info + */ +struct efi_firmware_image_authentication { + uint64_t monotonic_count; + struct win_certificate_uefi_guid auth_info; +} __attribute__((__packed__)); + /** * efi_signature_data - A format of signature * diff --git a/include/efi_loader.h b/include/efi_loader.h index 8d923451ce..897710ae3f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -708,7 +708,7 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); efi_status_t efi_bootmgr_load(efi_handle_t *handle); -#ifdef CONFIG_EFI_SECURE_BOOT +#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) #include <image.h> /** @@ -783,7 +783,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, WIN_CERTIFICATE **auth, size_t *auth_len); struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t buflen); -#endif /* CONFIG_EFI_SECURE_BOOT */ +#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */ #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT /* Capsule update */ @@ -798,6 +798,10 @@ efi_status_t EFIAPI efi_query_capsule_caps( u32 *reset_type); #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ +efi_status_t efi_capsule_authenticate(const void *capsule, + efi_uintn_t capsule_size, + void **image, efi_uintn_t *image_size); + #ifdef CONFIG_EFI_CAPSULE_ON_DISK #define EFI_CAPSULE_DIR L"\\EFI\\UpdateCapsule\\" diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index ec2976ceba..245deabbed 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -110,6 +110,22 @@ config EFI_CAPSULE_FIT_DEVICE help Define storage device, say 1:1, for storing FIT image +config EFI_CAPSULE_AUTHENTICATE + bool "Update Capsule authentication" + depends on EFI_HAVE_CAPSULE_SUPPORT + depends on EFI_CAPSULE_ON_DISK + depends on EFI_FIRMWARE_MANAGEMENT_PROTOCOL + select SHA256 + select RSA + select RSA_VERIFY + select RSA_VERIFY_WITH_PKEY + select X509_CERTIFICATE_PARSER + select PKCS7_MESSAGE_PARSER + default n + help + Select this option if you want to enable capsule + authentication + config EFI_DEVICE_PATH_TO_TEXT bool "Device path to text protocol" default y diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 931d363edc..a265d36ff0 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -12,6 +12,10 @@ #include <malloc.h> #include <mapmem.h> #include <sort.h> +#include "../lib/crypto/pkcs7_parser.h" + +#include <crypto/pkcs7.h> +#include <linux/err.h> const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; static const efi_guid_t efi_guid_firmware_management_capsule_id = @@ -245,6 +249,128 @@ out: return ret; } + +#if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) + +const efi_guid_t efi_guid_capsule_root_cert_guid = + EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; + +__weak u16 *efi_get_truststore_name(void) +{ + return L"CRT"; +} + +__weak const efi_guid_t *efi_get_truststore_vendor(void) +{ + + return &efi_guid_capsule_root_cert_guid; +} + +/** + * efi_capsule_authenticate() - Authenticate a uefi capsule + * + * @capsule: Capsule file with the authentication + * header + * @capsule_size: Size of the entire capsule + * @image: pointer to the image payload minus the + * authentication header + * @image_size: size of the image payload + * + * Authenticate the capsule signature with the public key contained + * in the root certificate stored as an efi environment variable + * + * Return: EFI_SUCCESS on successfull authentication or + * EFI_SECURITY_VIOLATION on authentication failure + */ +efi_status_t efi_capsule_authenticate(const void *capsule, + efi_uintn_t capsule_size, + void **image, efi_uintn_t *image_size) +{ + uint64_t monotonic_count; + struct efi_signature_store *truststore; + struct pkcs7_message *capsule_sig; + struct efi_image_regions *regs; + struct efi_firmware_image_authentication *auth_hdr; + efi_status_t status; + + status = EFI_SECURITY_VIOLATION; + capsule_sig = NULL; + truststore = NULL; + regs = NULL; + + /* Sanity checks */ + if (capsule == NULL || capsule_size == 0) + goto out; + + auth_hdr = (struct efi_firmware_image_authentication *)capsule; + if (capsule_size < sizeof(*auth_hdr)) + goto out; + + if (auth_hdr->auth_info.hdr.dwLength <= + offsetof(struct win_certificate_uefi_guid, cert_data)) + goto out; + + if (guidcmp(&auth_hdr->auth_info.cert_type, &efi_guid_cert_type_pkcs7)) + goto out; + + *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) + + auth_hdr->auth_info.hdr.dwLength; + *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - + sizeof(auth_hdr->monotonic_count); + memcpy(&monotonic_count, &auth_hdr->monotonic_count, + sizeof(monotonic_count)); + + /* data to be digested */ + regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 2, 1); + if (!regs) + goto out; + + regs->max = 2; + efi_image_region_add(regs, (uint8_t *)*image, + (uint8_t *)*image + *image_size, 1); + + efi_image_region_add(regs, (uint8_t *)&monotonic_count, + (uint8_t *)&monotonic_count + sizeof(monotonic_count), + 1); + + capsule_sig = efi_parse_pkcs7_header(auth_hdr->auth_info.cert_data, + auth_hdr->auth_info.hdr.dwLength + - sizeof(auth_hdr->auth_info)); + if (IS_ERR(capsule_sig)) { + debug("Parsing variable's pkcs7 header failed\n"); + capsule_sig = NULL; + goto out; + } + + truststore = efi_sigstore_parse_sigdb(efi_get_truststore_name(), + efi_get_truststore_vendor()); + if (!truststore) + goto out; + + /* verify signature */ + if (efi_signature_verify_with_sigdb(regs, capsule_sig, truststore, NULL)) { + debug("Verified\n"); + } else { + debug("Verifying variable's signature failed\n"); + goto out; + } + + status = EFI_SUCCESS; + +out: + efi_sigstore_free(truststore); + pkcs7_free_message(capsule_sig); + free(regs); + + return status; +} +#else +efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_size, + void **image, efi_uintn_t *image_size) +{ + return EFI_UNSUPPORTED; +} +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */ #else static efi_status_t efi_capsule_update_firmware( struct efi_capsule_header *capsule_data) diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 9897f5418e..4c722e0da9 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -23,7 +23,7 @@ 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; -#ifdef CONFIG_EFI_SECURE_BOOT +#if defined(CONFIG_EFI_SECURE_BOOT) || defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) static u8 pkcs7_hdr[] = { /* SEQUENCE */ @@ -871,4 +871,4 @@ err: return NULL; } -#endif /* CONFIG_EFI_SECURE_BOOT */ +#endif /* CONFIG_EFI_SECURE_BOOT || CONFIG_EFI_CAPSULE_AUTHENTICATE */
Add support for authenticating uefi capsules. Most of the signature verification functionality is shared with the uefi secure boot feature. The root certificate containing the public key used for the signature verification is stored as an efi variable, similar to the variables used for uefi secure boot. The root certificate is stored as an efi signature list(esl) file -- this file contains the x509 certificate which is the root certificate. Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org> --- include/efi_api.h | 17 +++++ include/efi_loader.h | 8 ++- lib/efi_loader/Kconfig | 16 +++++ lib/efi_loader/efi_capsule.c | 126 +++++++++++++++++++++++++++++++++ lib/efi_loader/efi_signature.c | 4 +- 5 files changed, 167 insertions(+), 4 deletions(-)