[U-Boot] [PATCH 05/16] efi_loader: add signature verification functions

Heinrich Schuchardt
2019-11-16T20:00:12Z

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:
> In this commit, implemented are a couple of helper functions which will be
> used to materialize variable authentication as well as image authentication
> in later patches.

In which patch do you implement the unit tests for the new functions? I
would expect them in /test/lib/ as tests invoked via the ut command.

>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> include/efi_api.h | 47 +++
> include/efi_loader.h | 46 +++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_signature.c | 600 +++++++++++++++++++++++++++++++++
> 4 files changed, 694 insertions(+)
> create mode 100644 lib/efi_loader/efi_signature.c
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 22396172e15f..24d034246927 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -18,6 +18,7 @@
>
> #include <efi.h>
> #include <charset.h>
> +#include <pe.h>
>
> #ifdef CONFIG_EFI_LOADER
> #include <asm/setjmp.h>
> @@ -307,6 +308,10 @@ struct efi_runtime_services {
> EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, \
> 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c)
>
> +#define EFI_IMAGE_SECURITY_DATABASE_GUID \
> + EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, \
> + 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
> +
> #define EFI_FDT_GUID \
> EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \
> 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)
> @@ -1616,4 +1621,46 @@ struct efi_unicode_collation_protocol {
> #define LOAD_OPTION_CATEGORY_BOOT 0x00000000
> #define LOAD_OPTION_CATEGORY_APP 0x00000100
>
> +/* Secure boot */

Please, add a comment for each GUID.

> +#define EFI_CERT_SHA256_GUID \
> + EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, \
> + 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28)
> +#define EFI_CERT_RSA2048_GUID \
> + EFI_GUID(0x3c5766e8, 0x269c, 0x4e34, 0xaa, 0x14, \
> + 0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6)
> +#define EFI_CERT_X509_GUID \
> + EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, \
> + 0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)
> +#define EFI_CERT_X509_SHA256_GUID \
> + EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, \
> + 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)
> +#define EFI_CERT_TYPE_PKCS7_GUID \
> + EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \
> + 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
> +

Please add comments in Spinx style.

What is the structure used for?
What information do the fields hold?

Same for all structures.

> +struct win_certificate_uefi_guid {
> + WIN_CERTIFICATE hdr;

See comment to 01/16 (avoid typedefs).

> + efi_guid_t cert_type;
> + u8 cert_data[];
> +} __attribute__((__packed__));
> +
> +struct efi_variable_authentication_2 {
> + struct efi_time time_stamp;
> + struct win_certificate_uefi_guid auth_info;
> +} __attribute__((__packed__));
> +
> +struct efi_signature_data {
> + efi_guid_t signature_owner;
> + u8 signature_data[];
> +} __attribute__((__packed__));
> +
> +struct efi_signature_list {
> + efi_guid_t signature_type;
> + u32 signature_list_size;
> + u32 signature_header_size;
> + u32 signature_size;
> +/* u8 signature_header[signature_header_size]; */
> +/* struct efi_signature_data signatures[...][signature_size]; */
> +} __attribute__((__packed__));
> +
> #endif
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 381da80cdce0..5a3df557e9f2 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -21,6 +21,7 @@ static inline int guidcmp(const void *g1, const void *g2)
> #if CONFIG_IS_ENABLED(EFI_LOADER)
>
> #include <linux/list.h>
> +#include <linux/oid_registry.h>
>
> /* Maximum number of configuration tables */
> #define EFI_MAX_CONFIGURATION_TABLES 16
> @@ -169,6 +170,11 @@ extern const efi_guid_t efi_guid_hii_config_routing_protocol;
> extern const efi_guid_t efi_guid_hii_config_access_protocol;
> extern const efi_guid_t efi_guid_hii_database_protocol;
> extern const efi_guid_t efi_guid_hii_string_protocol;
> +/* GUID for authentication */

%s/GUID/GUIDs/g ?

> +extern const efi_guid_t efi_guid_image_security_database;
> +extern const efi_guid_t efi_guid_sha256;
> +extern const efi_guid_t efi_guid_cert_x509;
> +extern const efi_guid_t efi_guid_cert_x509_sha256;
>
> extern unsigned int __efi_runtime_start, __efi_runtime_stop;
> extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
> @@ -650,6 +656,46 @@ 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

Why do we need an #ifdef here?

> +#include <image.h>
> +
> +#define EFI_REGS_MAX 16 /* currently good enough */
> +

Do you mean: this could be exceeded in special cases?

Can't we use a linked list for regions? We should not embed any
unnecessary restrictions.

Add comments explaining what the structures are used for.

> +struct efi_image_regions {
> + int num;
> + struct image_region reg[EFI_REGS_MAX];
> +};
> +
> +struct efi_sig_data {
> + struct efi_sig_data *next;
> + efi_guid_t owner;
> + void *data;
> + size_t size;
> +};
> +
> +struct efi_signature_store {
> + struct efi_signature_store *next;
> + efi_guid_t sig_type;
> + struct efi_sig_data *sig_data_list;
> +};
> +
> +struct x509_certificate;
> +struct pkcs7_message;
> +
> +bool efi_signature_verify_cert(struct x509_certificate *cert,
> + struct efi_signature_store *dbx);
> +bool efi_signature_verify_signers(struct pkcs7_message *msg,
> + struct efi_signature_store *dbx);
> +bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
> + struct pkcs7_message *msg,
> + struct efi_signature_store *db,
> + struct x509_certificate **cert);
> +
> +efi_status_t efi_image_region_add(struct efi_image_regions *regs,
> + const void *start, const void *end,
> + int nocheck);
> +#endif /* CONFIG_EFI_SECURE_BOOT */
> +
> #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
>
> /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 01769ea58ba6..49c996c89052 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_PARTITIONS) += efi_disk.o
> obj-$(CONFIG_NET) += efi_net.o
> obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> +obj-y += efi_signature.o
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> new file mode 100644
> index 000000000000..04f43e47d1a3
> --- /dev/null
> +++ b/lib/efi_loader/efi_signature.c
> @@ -0,0 +1,600 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>

From which upstream did you take the code?

> + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> + */
> +

debug() checks the value of _DEBUG.
Please, include common.h as first include.

Use the sequence of includes as defined in
https://www.denx.de/wiki/U-Boot/CodingStyle

> +#include <charset.h>
> +#include <efi_loader.h>
> +#include <image.h>
> +#include <hexdump.h>
> +#include <malloc.h>
> +#include <pe.h>
> +#include <linux/compat.h>
> +#include <linux/oid_registry.h>
> +#include <u-boot/rsa.h>

Shouldn't we move all cryptography stuff into a directory crypto/?

> +#include <u-boot/sha256.h>
> +/*
> + * avoid duplicated inclusion:
> + * #include "../lib/crypto/x509_parser.h"
> + */
> +#include "../lib/crypto/pkcs7_parser.h"
> +
> +const efi_guid_t efi_guid_image_security_database =
> + EFI_IMAGE_SECURITY_DATABASE_GUID;
> +const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID;
> +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
> +static const unsigned char WinIndirectSha256[] = {

We don't use camel case.

> + 0x30, 0x33, 0x06, 0x0a, 0x2b, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37, 0x02,
> + 0x01, 0x0f, 0x30, 0x25, 0x03, 0x01, 0x00, 0xa0, 0x20, 0xa2, 0x1e, 0x80,
> + 0x1c, 0x00, 0x3c, 0x00, 0x3c, 0x00, 0x3c, 0x00, 0x4f, 0x00, 0x62, 0x00,
> + 0x73, 0x00, 0x6f, 0x00, 0x6c, 0x00, 0x65, 0x00, 0x74, 0x00, 0x65, 0x00,
> + 0x3e, 0x00, 0x3e, 0x00, 0x3e, 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60,
> + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20
> +};

What secret sauce is this? - Please, add comments where applicable. Add
references where needed for verification of correctness.

Below you describe it as DER wrapper. So why call the variable Sha256?

> +
> +/**
> + * efi_hash_regions - calculate a hash value
> + * @regs: List of regions
> + * @hash: Pointer to a pointer to buffer holding a hash value
> + * @size: Size of buffer to be returned
> + *
> + * Calculate a sha256 value of @regs and return a value in @hash.

Is this a hash of the concatenation of all regions - or is this a list
of hashes one for each regions?

> + *
> + * Return: true on success, false on error
> + */
> +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> + size_t *size)

Why use void** for hash? There should be a structure for SHA256 hashes.

> +{
> + *size = 0;
> + *hash = calloc(1, SHA256_SUM_LEN);
> + if (!*hash) {
> + debug("Out of memory\n");
> + return false;
> + }
> + *size = SHA256_SUM_LEN;
> +
> + hash_calculate("sha256", regs->reg, regs->num, *hash);
> +#ifdef DEBUG
> + debug("hash calculated:\n");

debug checks the value of _DEBUG not of DEBUG. So wouldn't you use
#ifdef _DEBUG to be consistent?

> + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> + *hash, SHA256_SUM_LEN, false);

This will fail to compile if CONFIG_HEXDUMP is not defined?

> +#endif
> +
> + return true;
> +}
> +
> +/**
> + * efi_hash_regions_in_der - calculate a hash value
> + * @regs: List of regions
> + * @hash: Pointer to a pointer to buffer holding a hash value
> + * @size: Size of buffer to be returned
> + *
> + * Calculate a sha256 value of @regs and return a value in @hash.
> + * This function is a variant of efi_hash_regions(), and the data will
> + * be wrapped with some header in DER format before hash calculation.
> + * Message digest in signature of PE format will be calculated this way.
> + *
> + * Return: true on success, false on error
> + */
> +static bool efi_hash_regions_in_der(struct efi_image_regions *regs, void **hash,
> + size_t *size)
> +{
> + void *msg;
> + size_t msg_size;
> + struct image_region regtmp[2];
> +
> + if (!efi_hash_regions(regs, &msg, &msg_size)) {
> + debug("Hash calculation failed\n");
> + return false;
> + ;
> + }
> +
> + *size = 0;
> + *hash = calloc(1, SHA256_SUM_LEN);
> + if (!*hash) {
> + debug("Out of memory\n");
> + free(msg);
> + return false;
> + }
> + *size = SHA256_SUM_LEN;
> +
> + /* File image hash is digested with some DER wrapper. */
> + regtmp[0].data = WinIndirectSha256;
> + regtmp[0].size = sizeof(WinIndirectSha256);
> + regtmp[1].data = msg;
> + regtmp[1].size = msg_size;
> +
> + hash_calculate("sha256", regtmp, 2, *hash);
> +#ifdef DEBUG
> + debug("hash calculated with extra header:\n");
> + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> + *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> + free(msg);
> +
> + return true;
> +}
> +
> +/**
> + * efi_signature_verify - verify a signature with a certificate
> + * @regs: List of regions to be authenticated
> + * @signed_info: Pointer to PKCS7's signed_info
> + * @cert: x509 certificate
> + *
> + * Signature pointed to by @signed_info against image pointed to by @regs
> + * is verified by a certificate pointed to by @cert.
> + * @signed_info holds a signature, including a message digest which is to be
> + * compared with a hash value calculated from @regs.
> + *
> + * Return: true if signature is verified, false if not
> + */
> +static bool efi_signature_verify(struct efi_image_regions *regs,
> + struct pkcs7_signed_info *ps_info,
> + struct x509_certificate *cert)
> +{
> + struct image_sign_info info;
> + struct image_region regtmp[2];
> + void *hash;
> + size_t size;
> + char c;
> + bool verified;
> +
> + debug("%s: Enter, %p, %p, %p(issuer: %s, subject: %s)\n", __func__,
> + regs, ps_info, cert, cert->issuer, cert->subject);
> +
> + verified = false;
> +
> + memset(&info, '\0', sizeof(info));
> + info.padding = image_get_padding_algo("pkcs-1.5");
> + /*
> + * Note: image_get_[checksum|crypto]_algo takes an string
> + * argument like "<checksum>,<crypto>"
> + * TODO: support other hash algorithms
> + */
> + if (!strcmp(ps_info->sig->hash_algo, "sha1")) {
> + info.checksum = image_get_checksum_algo("sha1,rsa2048");
> + info.name = "sha1,rsa2048";
> + } else if (!strcmp(ps_info->sig->hash_algo, "sha256")) {
> + info.checksum = image_get_checksum_algo("sha256,rsa2048");
> + info.name = "sha256,rsa2048";
> + } else {
> + debug("unknown msg digest algo: %s\n", ps_info->sig->hash_algo);
> + goto out;
> + }
> + info.crypto = image_get_crypto_algo(info.name);
> +
> + info.key = cert->pub->key;
> + info.keylen = cert->pub->keylen;
> +
> + /* verify signature */
> + debug("%s: crypto: %s, signature len:%x\n", __func__,
> + info.name, ps_info->sig->s_size);
> + if (ps_info->aa_set & (1UL << sinfo_has_message_digest)) {
> + debug("%s: RSA verify authentication attribute\n", __func__);
> + /*
> + * NOTE: This path will be executed only for
> + * PE image authentication
> + */
> +
> + /* check if hash matches digest first */
> + debug("checking msg digest first, len:0x%x\n",
> + ps_info->msgdigest_len);
> +
> +#ifdef DEBUG
> + debug("hash in database:\n");
> + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> + ps_info->msgdigest, ps_info->msgdigest_len,
> + false);
> +#endif
> + if (efi_hash_regions_in_der(regs, &hash, &size)) {
> + if (ps_info->msgdigest_len != size ||
> + memcmp(hash, ps_info->msgdigest, size)) {
> + debug("Digest doesn't match\n");
> + free(hash);
> + goto out;
> + }
> +
> + free(hash);
> + } else {
> + debug("Digesting image failed\n");
> + goto out;
> + }
> +
> + /* against digest */
> + c = 0x31;
> + regtmp[0].data = &c;
> + regtmp[0].size = 1;
> + regtmp[1].data = ps_info->authattrs;
> + regtmp[1].size = ps_info->authattrs_len;
> +
> + if (!rsa_verify(&info, regtmp, 2,
> + ps_info->sig->s, ps_info->sig->s_size))
> + verified = true;
> + } else {
> + debug("%s: RSA verify content data\n", __func__);
> + /* against all data */
> + if (!rsa_verify(&info, regs->reg, regs->num,
> + ps_info->sig->s, ps_info->sig->s_size))
> + verified = true;
> + }
> +
> +out:
> + debug("%s: Exit, verified: %d\n", __func__, verified);
> + return verified;
> +}
> +
> +/**
> + * efi_signature_verify_with_list - verify a signature with signature list
> + * @regs: List of regions to be authenticated
> + * @signed_info: Pointer to PKCS7's signed_info
> + * @siglist: Signature list for certificates
> + * @valid_cert: x509 certificate that verifies this signature
> + *
> + * Signature pointed to by @signed_info against image pointed to by @regs
> + * is verified by signature list pointed to by @siglist.
> + * Signature database is a simple concatenation of one or more
> + * signature list(s).
> + *
> + * Return: true if signature is verified, false if not
> + */
> +static
> +bool efi_signature_verify_with_list(struct efi_image_regions *regs,
> + struct pkcs7_signed_info *signed_info,
> + struct efi_signature_store *siglist,
> + struct x509_certificate **valid_cert)
> +{
> + struct x509_certificate *cert;
> + struct efi_sig_data *sig_data;
> + bool verified = false;
> +
> + debug("%s: Enter, %p, %p, %p, %p\n", __func__,
> + regs, signed_info, siglist, valid_cert);

Shouldn't we use EFI_PRINT() to get proper indentation?

> +
> + if (!signed_info) {
> + void *hash;
> + size_t size;
> +
> + debug("%s: unsigned image\n", __func__);

We already have indicated the function above.

> + /*
> + * verify based on calculated hash value
> + * TODO: support other hash algorithms
> + */
> + if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) {
> + debug("Digest algorithm is not supported: %pUl\n",
> + &siglist->sig_type);
> + goto out;
> + }
> +
> + if (!efi_hash_regions(regs, &hash, &size)) {
> + debug("Digesting unsigned image failed\n");
> + goto out;
> + }
> +
> + /* go through the list */
> + for (sig_data = siglist->sig_data_list; sig_data;
> + sig_data = sig_data->next) {
> +#ifdef DEBUG
> + debug("Msg digest in database:\n");
> + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> + sig_data->data, sig_data->size, false);

Does this compile if CONFIG_HEXDUMP is not defined?

> +#endif
> + if ((sig_data->size == size) &&
> + !memcmp(sig_data->data, hash, size)) {
> + verified = true;
> + free(hash);
> + goto out;
> + }
> + }
> + free(hash);
> + goto out;
> + }
> +
> + debug("%s: signed image\n", __func__);
> + if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) {
> + debug("Signature type is not supported: %pUl\n",
> + &siglist->sig_type);
> + goto out;
> + }
> +
> + /* go through the list */
> + for (sig_data = siglist->sig_data_list; sig_data;
> + sig_data = sig_data->next) {
> + /* TODO: support owner check based on policy */
> +
> + cert = x509_cert_parse(sig_data->data, sig_data->size);
> + if (IS_ERR(cert)) {
> + debug("Parsing x509 certificate failed\n");
> + goto out;
> + }
> +
> + verified = efi_signature_verify(regs, signed_info, cert);
> +
> + if (verified) {
> + if (valid_cert)
> + *valid_cert = cert;
> + else
> + x509_free_certificate(cert);
> + break;
> + } else {
> + x509_free_certificate(cert);
> + }
> + }
> +
> +out:
> + debug("%s: Exit, verified: %d\n", __func__, verified);
> + return verified;
> +}
> +
> +/**
> + * efi_signature_verify_with_sigdb - verify a signature with db
> + * @regs: List of regions to be authenticated
> + * @msg: Signature
> + * @db: Signature database for trusted certificates
> + * @cert: x509 certificate that verifies this signature
> + *
> + * Signature pointed to by @msg against image pointed to by @regs
> + * is verified by signature database pointed to by @db.
> + *
> + * Return: true if signature is verified, false if not
> + */
> +bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
> + struct pkcs7_message *msg,
> + struct efi_signature_store *db,
> + struct x509_certificate **cert)
> +{
> + struct pkcs7_signed_info *info;
> + struct efi_signature_store *siglist;
> + bool verified = false;
> +
> + debug("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, cert);
> +
> + if (!db)
> + goto out;
> +
> + if (!db->sig_data_list)
> + goto out;
> +
> + /* for unsigned image */
> + if (!msg) {
> + debug("%s: Verify unsigned image with db\n", __func__);
> + for (siglist = db; siglist; siglist = siglist->next)
> + if (efi_signature_verify_with_list(regs, NULL,
> + siglist, cert)) {
> + verified = true;
> + goto out;
> + }
> +
> + goto out;
> + }
> +
> + /* for signed image or variable */
> + debug("%s: Verify signed image with db\n", __func__);
> + for (info = msg->signed_infos; info; info = info->next) {
> + debug("Signed Info: digest algo: %s, pkey algo: %s\n",
> + info->sig->hash_algo, info->sig->pkey_algo);
> +
> + for (siglist = db; siglist; siglist = siglist->next) {
> + if (efi_signature_verify_with_list(regs, info,
> + siglist, cert)) {
> + verified = true;
> + goto out;
> + }
> + }
> + }
> +
> +out:
> + debug("%s: Exit, verified: %d\n", __func__, verified);
> + return verified;
> +}
> +
> +/**
> + * efi_search_siglist - search signature list for a certificate
> + * @cert: x509 certificate
> + * @siglist: Signature list
> + * @revoc_time: Pointer to buffer for revocation time
> + *
> + * Search signature list pointed to by @siglist and find a certificate
> + * pointed to by @cert.
> + * If found, revocation time that is specified in signature database is
> + * returned in @revoc_time.
> + *
> + * Return: true if certificate is found, false if not
> + */
> +static bool efi_search_siglist(struct x509_certificate *cert,
> + struct efi_signature_store *siglist,
> + time64_t *revoc_time)
> +{
> + struct image_region reg[1];
> + void *hash = NULL, *msg = NULL;
> + struct efi_sig_data *sig_data;
> + bool found = false;
> +
> + /* can be null */
> + if (!siglist->sig_data_list)
> + return false;
> +
> + if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) {
> + /* TODO: other hash algos */
> + debug("Certificate's digest type is not supported: %pUl\n",
> + &siglist->sig_type);
> + goto out;
> + }
> +
> + /* calculate hash of TBSCertificate */
> + msg = calloc(1, SHA256_SUM_LEN);
> + if (!msg) {
> + debug("Out of memory\n");
> + goto out;
> + }
> +
> + hash = calloc(1, SHA256_SUM_LEN);
> + if (!hash) {
> + debug("Out of memory\n");
> + goto out;
> + }
> +
> + reg[0].data = cert->tbs;
> + reg[0].size = cert->tbs_size;
> + hash_calculate("sha256", reg, 1, msg);
> +
> + /* go through signature list */
> + for (sig_data = siglist->sig_data_list; sig_data;
> + sig_data = sig_data->next) {
> + /*
> + * struct efi_cert_x509_sha256 {
> + * u8 tbs_hash[256/8];
> + * time64_t revocation_time;
> + * };
> + */
> + if ((sig_data->size == SHA256_SUM_LEN) &&
> + !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) {
> + memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN,
> + sizeof(*revoc_time));
> + found = true;
> + goto out;
> + }
> + }
> +
> +out:
> + free(hash);
> + free(msg);
> +
> + return found;
> +}
> +
> +/**
> + * efi_signature_verify_cert - verify a certificate with dbx
> + * @cert: x509 certificate
> + * @dbx: Signature database
> + *
> + * Search signature database pointed to by @dbx and find a certificate
> + * pointed to by @cert.
> + * This function is expected to be used against "dbx".
> + *
> + * Return: true if a certificate is not rejected, false otherwise.
> + */
> +bool efi_signature_verify_cert(struct x509_certificate *cert,
> + struct efi_signature_store *dbx)
> +{
> + struct efi_signature_store *siglist;
> + time64_t revoc_time;
> + bool found = false;
> +
> + debug("%s: Enter, %p, %p\n", __func__, dbx, cert);

EFI_PRINT() ?

> +
> + if (!cert)
> + return false;
> +
> + for (siglist = dbx; siglist; siglist = siglist->next) {
> + if (efi_search_siglist(cert, siglist, &revoc_time)) {
> + /* TODO */
> + /* compare signing time with revocation time */

Is this a difficult thing to do? If not, please, fill the gap.

Best regards

Heinrich

> +
> + found = true;
> + break;
> + }
> + }
> +
> + debug("%s: Exit, verified: %d\n", __func__, !found);
> + return !found;
> +}
> +
> +/**
> + * efi_signature_verify_signers - verify signers' certificates with dbx
> + * @msg: Signature
> + * @dbx: Signature database
> + *
> + * Determine if any of signers' certificates in @msg may be verified
> + * by any of certificates in signature database pointed to by @dbx.
> + * This function is expected to be used against "dbx".
> + *
> + * Return: true if none of certificates is rejected, false otherwise.
> + */
> +bool efi_signature_verify_signers(struct pkcs7_message *msg,
> + struct efi_signature_store *dbx)
> +{
> + struct pkcs7_signed_info *info;
> + bool found = false;
> +
> + debug("%s: Enter, %p, %p\n", __func__, msg, dbx);
> +
> + if (!msg)
> + goto out;
> +
> + for (info = msg->signed_infos; info; info = info->next) {
> + if (info->signer &&
> + !efi_signature_verify_cert(info->signer, dbx)) {
> + found = true;
> + goto out;
> + }
> + }
> +out:
> + debug("%s: Exit, verified: %d\n", __func__, !found);
> + return !found;
> +}
> +
> +/**
> + * efi_image_region_add - add an entry of region
> + * @regs: Pointer to array of regions
> + * @start: Start address of region
> + * @end: End address of region
> + * @nocheck: flag against overlapped regions
> + *
> + * Take one entry of region [@start, @end] and append it to the list
> + * pointed to by @regs. If @nocheck is false, overlapping among entries
> + * will be checked first.
> + *
> + * Return: 0 on success, status code (negative) on error
> + */
> +efi_status_t efi_image_region_add(struct efi_image_regions *regs,
> + const void *start, const void *end,
> + int nocheck)
> +{
> + struct image_region *reg;
> + int i, j;
> +
> + if (regs->num >= EFI_REGS_MAX) {
> + debug("%s: no more room for regions\n", __func__);
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (end < start)
> + return EFI_INVALID_PARAMETER;
> +
> + for (i = 0; i < regs->num; i++) {
> + reg = &regs->reg[i];
> + if (nocheck)
> + continue;
> +
> + if (start > reg->data + reg->size)
> + continue;
> +
> + if ((start >= reg->data && start < reg->data + reg->size) ||
> + (end > reg->data && end < reg->data + reg->size)) {
> + debug("%s: new region already part of another\n",
> + __func__);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (start < reg->data && end < reg->data + reg->size) {
> + for (j = regs->num - 1; j >= i; j--)
> + memcpy(&regs->reg[j], &regs->reg[j + 1],
> + sizeof(*reg));
> + break;
> + }
> + }
> +
> + reg = &regs->reg[i];
> + reg->data = start;
> + reg->size = end - start;
> + regs->num++;
> +
> + return EFI_SUCCESS;
> +}
> +#endif /* CONFIG_EFI_SECURE_BOOT */
>