[PATCH 6/8] efi: capsule: Add support for uefi capsule authentication
Sughosh Ganu
sughosh.ganu at linaro.org
Thu May 7 13:50:35 CEST 2020
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
More information about the U-Boot
mailing list