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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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/

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 |