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.g...@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
>
