Re: [PATCH v1 08/14] efi: loader: verify Authenticode signatures against built-in keys

[SCHNEIDER Johannes <johannes.schneider@leica-geosystems.com>]

Hoi Ahmad,


> >
> > Hello Johannes,
> >
> > On 10/4/26 03:19, Johannes Schneider wrote:
> > > barebox's EFI loader verifies no signatures: efi_image_authenticate()
> > > accepts every image. Add efi_authenticode_verify() as the verifier for
> > > signed EFI images: compute the Authenticode digest over the regions
> > > efi_image_parse() collects, check it against the SpcIndirectDataContent
> > > of the PKCS#7 signature, check the messageDigest attribute against the
> > > digest of that content, and verify the signature over the attributes
> > > with the keys of a barebox keyring. The following commits use it.
> > >
> > > barebox has no ASN.1 decoder: keys are converted from certificates at
> > > build time, and FIT signatures carry none. The PKCS#7 structure is
> > > walked as plain DER for the fields needed, with every length checked
> > > against what remains; the certificates it carries are skipped.
> > >
> > > Supported are one signer, SHA-256 and RSA. As for FIT images, trust
> > > comes from the keyring, not from X.509 chains or db/dbx.
> >
> > We should import mbedTLS and then make use of its PKCS#7 support.
> > The goal being mbedTLS being updated regularly like we already do
> > with dts/
> >
>
> thanks for the pointer! you picked up the one patch i was least confident about :-D
>
> i (or we ;-) where contemplating importing the linux kernel code here ... but
> that would have pulled in lots of other/related parts
>
> -> taking a look at mbedTLS instead.... so expect a v2,
> any other things that caught your eye and {c,s}hould be reworked?
>

u-boot also imports mbetls - but:
1 has an older 3.6.0 version :-(
see: u-boot/lib/mbedtls/external/mbedtls/include/mbedtls/build_info.h
2 has patches ontop, to get the needed authenticode support
which is still a PR into upstream mbedtls: #9001 [1]

I would propose importing mbedtls first, then porting over the
pkcs7/authenticode patches, and afterward reworking/resuming this
=> so three separate, but order dependent patch-series - would that be OK?

about actually importing mbedtls into barebox: shall this go over the
mailinglist - which will be rather large patch series - or are there
alternate routes open for you maintainers?


gruß
Johannes


Link: https://github.com/Mbed-TLS/mbedtls/pull/9001

>
>
>
> > Cheers,
> > Ahmad
> >
> > >
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> > > ---
> > >  efi/loader/Kconfig                |  16 ++
> > >  efi/loader/Makefile               |   1 +
> > >  efi/loader/authenticode.c         | 435 ++++++++++++++++++++++++++++++
> > >  include/efi/loader/authenticode.h |   9 +
> > >  4 files changed, 461 insertions(+)
> > >  create mode 100644 efi/loader/authenticode.c
> > >  create mode 100644 include/efi/loader/authenticode.h
> > >
> > > diff --git a/efi/loader/Kconfig b/efi/loader/Kconfig
> > > index 5692e54ebe..4099da0689 100644
> > > --- a/efi/loader/Kconfig
> > > +++ b/efi/loader/Kconfig
> > > @@ -24,6 +24,22 @@ config EFI_LOADER_DEBUG_SUPPORT
> > >  config EFI_LOADER_SECURE_BOOT
> > >       bool
> > >
> > > +config EFI_LOADER_AUTHENTICODE
> > > +     bool "Verify Authenticode signatures of booted EFI images"
> > > +     depends on CRYPTO_BUILTIN_KEYS && HAVE_DIGEST_SHA256
> > > +     select CRYPTO_RSA
> > > +     help
> > > +       Verify the Authenticode (PKCS#7, SHA-256, RSA) signature of an EFI
> > > +       image booted with bootm against the keys compiled into the "efi"
> > > +       keyring (CONFIG_CRYPTO_PUBLIC_KEYS, keyring=efi). With signed images
> > > +       forced, an EFI image then boots only if one of those keys verifies
> > > +       it, the same way a FIT image must carry a valid signature.
> > > +
> > > +       X.509 certificates in the signature are not evaluated: trust is
> > > +       anchored in the keyring. barebox does not report UEFI Secure Boot
> > > +       to the payload, so a UKI keeps taking its command line from
> > > +       barebox.
> > > +
> > >  menu "UEFI services"
> > >
> > >  config EFI_LOADER_GET_TIME
> > > diff --git a/efi/loader/Makefile b/efi/loader/Makefile
> > > index 24850e87b1..775014dc22 100644
> > > --- a/efi/loader/Makefile
> > > +++ b/efi/loader/Makefile
> > > @@ -14,6 +14,7 @@ obj-y += boot.o
> > >  obj-y += runtime.o
> > >  obj-y += setup.o
> > >  obj-y += watchdog.o
> > > +obj-$(CONFIG_EFI_LOADER_AUTHENTICODE) += authenticode.o
> > >  obj-y += loadopts.o
> > >  obj-y += efi_var_common.o
> > >  obj-y += efi_variable.o
> > > diff --git a/efi/loader/authenticode.c b/efi/loader/authenticode.c
> > > new file mode 100644
> > > index 0000000000..36e5a46fc6
> > > --- /dev/null
> > > +++ b/efi/loader/authenticode.c
> > > @@ -0,0 +1,435 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Authenticode verification of PE images against barebox built-in keys:
> > > + * one signer, SHA-256 and RSA, the certificates in the signature are ignored
> > > + */
> > > +
> > > +#define pr_fmt(fmt) "efi-loader: authenticode: " fmt
> > > +
> > > +#include <common.h>
> > > +#include <digest.h>
> > > +#include <crypto/sha.h>
> > > +#include <malloc.h>
> > > +#include <crypto/public_key.h>
> > > +#include <efi/loader/pe.h>
> > > +#include <efi/loader/authenticode.h>
> > > +#include <efi/error.h>
> > > +#include <pe.h>
> > > +
> > > +struct der {
> > > +     const u8 *p;
> > > +     const u8 *end;
> > > +};
> > > +
> > > +struct der_elem {
> > > +     u8 tag;
> > > +     const u8 *start;        /* tag byte */
> > > +     const u8 *val;
> > > +     size_t len;
> > > +     size_t total;           /* tag + length + value */
> > > +};
> > > +
> > > +#define DER_INTEGER  0x02
> > > +#define DER_OCTET    0x04
> > > +#define DER_OID              0x06
> > > +#define DER_SEQ              0x30
> > > +#define DER_SET              0x31
> > > +#define DER_CTX0     0xa0
> > > +#define DER_CTX1     0xa1
> > > +
> > > +static const u8 oid_signed_data[] = { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02 };
> > > +static const u8 oid_spc_indirect_data[] = {
> > > +     0x2b, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37, 0x02, 0x01, 0x04
> > > +};
> > > +static const u8 oid_sha256[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01 };
> > > +static const u8 oid_content_type[] = { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x03 };
> > > +static const u8 oid_message_digest[] = { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x04 };
> > > +
> > > +static int der_next(struct der *d, struct der_elem *e)
> > > +{
> > > +     const u8 *p = d->p;
> > > +     size_t len, n;
> > > +
> > > +     if (d->end - p < 2)
> > > +             return -EBADMSG;
> > > +
> > > +     e->start = p;
> > > +     e->tag = *p++;
> > > +     if ((e->tag & 0x1f) == 0x1f)
> > > +             return -EBADMSG;
> > > +
> > > +     len = *p++;
> > > +     if (len & 0x80) {
> > > +             n = len & 0x7f;
> > > +             if (!n || n > 4 || d->end - p < n)
> > > +                     return -EBADMSG;
> > > +             len = 0;
> > > +             while (n--)
> > > +                     len = (len << 8) | *p++;
> > > +     }
> > > +
> > > +     if (d->end - p < len)
> > > +             return -EBADMSG;
> > > +
> > > +     e->val = p;
> > > +     e->len = len;
> > > +     e->total = p + len - e->start;
> > > +     d->p = p + len;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int der_expect(struct der *d, u8 tag, struct der_elem *e)
> > > +{
> > > +     int ret = der_next(d, e);
> > > +
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return e->tag == tag ? 0 : -EBADMSG;
> > > +}
> > > +
> > > +static struct der der_enter(const struct der_elem *e)
> > > +{
> > > +     return (struct der) { .p = e->val, .end = e->val + e->len };
> > > +}
> > > +
> > > +static bool der_oid_is(const struct der_elem *e, const u8 *oid, size_t len)
> > > +{
> > > +     return e->tag == DER_OID && e->len == len && !memcmp(e->val, oid, len);
> > > +}
> > > +
> > > +/* AlgorithmIdentifier ::= SEQUENCE { OID, parameters OPTIONAL } */
> > > +static int der_expect_sha256(struct der *d)
> > > +{
> > > +     struct der_elem seq, oid;
> > > +     struct der in;
> > > +     int ret;
> > > +
> > > +     ret = der_expect(d, DER_SEQ, &seq);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     in = der_enter(&seq);
> > > +     ret = der_expect(&in, DER_OID, &oid);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return der_oid_is(&oid, oid_sha256, sizeof(oid_sha256)) ? 0 : -EOPNOTSUPP;
> > > +}
> > > +
> > > +struct authenticode {
> > > +     const u8 *pe_digest;    /* SpcIndirectDataContent.messageDigest */
> > > +     const u8 *spc;          /* SpcIndirectDataContent content octets */
> > > +     size_t spc_len;
> > > +     const u8 *attrs;        /* [0] IMPLICIT authenticatedAttributes */
> > > +     size_t attrs_len;
> > > +     const u8 *attr_digest;  /* messageDigest attribute value */
> > > +     bool attr_content_type_ok;
> > > +     const u8 *sig;
> > > +     size_t sig_len;
> > > +};
> > > +
> > > +static int authenticode_parse_attrs(struct authenticode *a,
> > > +                                 const struct der_elem *attrs)
> > > +{
> > > +     struct der in = der_enter(attrs);
> > > +     struct der_elem attr, oid, set, val;
> > > +     struct der ain, sin;
> > > +     int ret;
> > > +
> > > +     while (in.p < in.end) {
> > > +             ret = der_expect(&in, DER_SEQ, &attr);
> > > +             if (ret)
> > > +                     return ret;
> > > +
> > > +             ain = der_enter(&attr);
> > > +             ret = der_expect(&ain, DER_OID, &oid);
> > > +             if (ret)
> > > +                     return ret;
> > > +             ret = der_expect(&ain, DER_SET, &set);
> > > +             if (ret)
> > > +                     return ret;
> > > +             sin = der_enter(&set);
> > > +
> > > +             if (der_oid_is(&oid, oid_message_digest, sizeof(oid_message_digest))) {
> > > +                     ret = der_expect(&sin, DER_OCTET, &val);
> > > +                     if (ret || val.len != SHA256_DIGEST_SIZE)
> > > +                             return -EBADMSG;
> > > +                     a->attr_digest = val.val;
> > > +             } else if (der_oid_is(&oid, oid_content_type, sizeof(oid_content_type))) {
> > > +                     ret = der_expect(&sin, DER_OID, &val);
> > > +                     if (ret)
> > > +                             return ret;
> > > +                     a->attr_content_type_ok =
> > > +                             der_oid_is(&val, oid_spc_indirect_data,
> > > +                                        sizeof(oid_spc_indirect_data));
> > > +             }
> > > +     }
> > > +
> > > +     return a->attr_digest ? 0 : -EBADMSG;
> > > +}
> > > +
> > > +static int authenticode_parse(struct authenticode *a, const void *buf, size_t len)
> > > +{
> > > +     struct der d = { .p = buf, .end = (const u8 *)buf + len };
> > > +     struct der_elem e, ci, sd, spc, dinfo, si;
> > > +     struct der in, sdin, ciin, spcin, dinin, siin;
> > > +     int ret;
> > > +
> > > +     /* ContentInfo ::= SEQUENCE { contentType, [0] EXPLICIT content } */
> > > +     ret = der_expect(&d, DER_SEQ, &ci);
> > > +     if (ret)
> > > +             return ret;
> > > +     in = der_enter(&ci);
> > > +     ret = der_expect(&in, DER_OID, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     if (!der_oid_is(&e, oid_signed_data, sizeof(oid_signed_data)))
> > > +             return -EBADMSG;
> > > +     ret = der_expect(&in, DER_CTX0, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     in = der_enter(&e);
> > > +
> > > +     /* SignedData ::= SEQUENCE { version, digestAlgorithms, contentInfo, ... } */
> > > +     ret = der_expect(&in, DER_SEQ, &sd);
> > > +     if (ret)
> > > +             return ret;
> > > +     sdin = der_enter(&sd);
> > > +     ret = der_expect(&sdin, DER_INTEGER, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect(&sdin, DER_SET, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /* contentInfo: SPC_INDIRECT_DATA carrying the PE image digest */
> > > +     ret = der_expect(&sdin, DER_SEQ, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     ciin = der_enter(&e);
> > > +     ret = der_expect(&ciin, DER_OID, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     if (!der_oid_is(&e, oid_spc_indirect_data, sizeof(oid_spc_indirect_data)))
> > > +             return -EBADMSG;
> > > +     ret = der_expect(&ciin, DER_CTX0, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     ciin = der_enter(&e);
> > > +     ret = der_expect(&ciin, DER_SEQ, &spc);
> > > +     if (ret)
> > > +             return ret;
> > > +     a->spc = spc.val;
> > > +     a->spc_len = spc.len;
> > > +
> > > +     spcin = der_enter(&spc);
> > > +     ret = der_expect(&spcin, DER_SEQ, &e);  /* SpcAttributeTypeAndOptionalValue */
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect(&spcin, DER_SEQ, &dinfo);      /* DigestInfo */
> > > +     if (ret)
> > > +             return ret;
> > > +     dinin = der_enter(&dinfo);
> > > +     ret = der_expect_sha256(&dinin);
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect(&dinin, DER_OCTET, &e);
> > > +     if (ret || e.len != SHA256_DIGEST_SIZE)
> > > +             return -EBADMSG;
> > > +     a->pe_digest = e.val;
> > > +
> > > +     /* skip optional certificates [0] and crls [1] */
> > > +     do {
> > > +             ret = der_next(&sdin, &e);
> > > +             if (ret)
> > > +                     return ret;
> > > +     } while (e.tag == DER_CTX0 || e.tag == DER_CTX1);
> > > +
> > > +     if (e.tag != DER_SET)
> > > +             return -EBADMSG;
> > > +
> > > +     /* first SignerInfo only */
> > > +     in = der_enter(&e);
> > > +     ret = der_expect(&in, DER_SEQ, &si);
> > > +     if (ret)
> > > +             return ret;
> > > +     siin = der_enter(&si);
> > > +     ret = der_expect(&siin, DER_INTEGER, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect(&siin, DER_SEQ, &e);   /* issuerAndSerialNumber */
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect_sha256(&siin);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = der_expect(&siin, DER_CTX0, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     a->attrs = e.start;
> > > +     a->attrs_len = e.total;
> > > +     ret = authenticode_parse_attrs(a, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = der_expect(&siin, DER_SEQ, &e);   /* digestEncryptionAlgorithm */
> > > +     if (ret)
> > > +             return ret;
> > > +     ret = der_expect(&siin, DER_OCTET, &e);
> > > +     if (ret)
> > > +             return ret;
> > > +     a->sig = e.val;
> > > +     a->sig_len = e.len;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int sha256_regions(const struct efi_image_regions *regs, u8 *out)
> > > +{
> > > +     struct digest *d = digest_alloc_by_algo(HASH_ALGO_SHA256);
> > > +     int i, ret;
> > > +
> > > +     if (!d)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     ret = digest_init(d);
> > > +     for (i = 0; !ret && i < regs->num; i++)
> > > +             ret = digest_update(d, regs->reg[i].data, regs->reg[i].size);
> > > +     if (!ret)
> > > +             ret = digest_final(d, out);
> > > +
> > > +     digest_free(d);
> > > +     return ret;
> > > +}
> > > +
> > > +static int sha256_buf(const void *buf, size_t len, u8 *out)
> > > +{
> > > +     struct digest *d = digest_alloc_by_algo(HASH_ALGO_SHA256);
> > > +     int ret;
> > > +
> > > +     if (!d)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     ret = digest_digest(d, buf, len, out);
> > > +     digest_free(d);
> > > +     return ret;
> > > +}
> > > +
> > > +/* The signature covers the attributes DER-encoded as SET OF, not as [0] */
> > > +static int sha256_attrs(const struct authenticode *a, u8 *out)
> > > +{
> > > +     struct digest *d = digest_alloc_by_algo(HASH_ALGO_SHA256);
> > > +     const u8 set_tag = DER_SET;
> > > +     int ret;
> > > +
> > > +     if (!d)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     ret = digest_init(d);
> > > +     if (!ret)
> > > +             ret = digest_update(d, &set_tag, 1);
> > > +     if (!ret)
> > > +             ret = digest_update(d, a->attrs + 1, a->attrs_len - 1);
> > > +     if (!ret)
> > > +             ret = digest_final(d, out);
> > > +
> > > +     digest_free(d);
> > > +     return ret;
> > > +}
> > > +
> > > +/**
> > > + * efi_authenticode_verify() - verify a PE image's Authenticode signature
> > > + * @efi:     PE image
> > > + * @len:     exact size of the image, see efi_pe_file_size()
> > > + * @keyring: barebox keyring holding the trusted keys
> > > + *
> > > + * Return: 0 if the image is signed by a key in @keyring, negative error code
> > > + * otherwise.
> > > + */
> > > +int efi_authenticode_verify(void *efi, size_t len, const char *keyring)
> > > +{
> > > +     struct efi_image_regions *regs = NULL;
> > > +     const struct public_key *key;
> > > +     struct authenticode a = {};
> > > +     WIN_CERTIFICATE *wincert;
> > > +     size_t auth_len;
> > > +     u8 pe_hash[SHA256_DIGEST_SIZE], hash[SHA256_DIGEST_SIZE];
> > > +     const struct keyring *kr;
> > > +     int ret;
> > > +
> > > +     if (!efi_image_parse(efi, len, &regs, &wincert, &auth_len))
> > > +             return -EBADMSG;
> > > +
> > > +     if (!wincert) {
> > > +             pr_err("image is not signed\n");
> > > +             ret = -ENOKEY;
> > > +             goto out;
> > > +     }
> > > +
> > > +     if (wincert->dwLength > auth_len || wincert->dwLength <= sizeof(*wincert) ||
> > > +         wincert->wRevision != WIN_CERT_REVISION_2_0 ||
> > > +         wincert->wCertificateType != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
> > > +             pr_err("unsupported certificate table entry\n");
> > > +             ret = -EBADMSG;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ret = authenticode_parse(&a, wincert + 1, wincert->dwLength - sizeof(*wincert));
> > > +     if (ret) {
> > > +             pr_err("cannot parse signature: %pe\n", ERR_PTR(ret));
> > > +             goto out;
> > > +     }
> > > +
> > > +     if (!a.attr_content_type_ok) {
> > > +             pr_err("signed content is not SpcIndirectDataContent\n");
> > > +             ret = -EBADMSG;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ret = sha256_regions(regs, pe_hash);
> > > +     if (ret)
> > > +             goto out;
> > > +     if (memcmp(pe_hash, a.pe_digest, sizeof(pe_hash))) {
> > > +             pr_err("image digest mismatch\n");
> > > +             ret = -EBADMSG;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ret = sha256_buf(a.spc, a.spc_len, hash);
> > > +     if (ret)
> > > +             goto out;
> > > +     if (memcmp(hash, a.attr_digest, sizeof(hash))) {
> > > +             pr_err("signed attributes do not match the content\n");
> > > +             ret = -EBADMSG;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ret = sha256_attrs(&a, hash);
> > > +     if (ret)
> > > +             goto out;
> > > +
> > > +     kr = keyring_find(keyring);
> > > +     if (!kr) {
> > > +             pr_err("keyring '%s' not registered\n", keyring);
> > > +             ret = -ENOKEY;
> > > +             goto out;
> > > +     }
> > > +
> > > +     ret = -ENOKEY;
> > > +     for_each_key_in_keyring(key, kr) {
> > > +             if (!public_key_verify(key, a.sig, a.sig_len, hash, HASH_ALGO_SHA256)) {
> > > +                     pr_info("verified with key '%s'\n", key->key_name_hint ?: "?");
> > > +                     ret = 0;
> > > +                     break;
> > > +             }
> > > +     }
> > > +
> > > +     if (ret)
> > > +             pr_err("no key in keyring '%s' verifies the signature\n", keyring);
> > > +out:
> > > +     free(regs);
> > > +     return ret;
> > > +}
> > > diff --git a/include/efi/loader/authenticode.h b/include/efi/loader/authenticode.h
> > > new file mode 100644
> > > index 0000000000..24c46ca7cf
> > > --- /dev/null
> > > +++ b/include/efi/loader/authenticode.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +#ifndef __EFI_LOADER_AUTHENTICODE_H
> > > +#define __EFI_LOADER_AUTHENTICODE_H
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +int efi_authenticode_verify(void *efi, size_t len, const char *keyring);
> > > +
> > > +#endif
> >
> >
> > --
> > Pengutronix e.K.                           |                             |
> > Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> > 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >
>