[v7,02/17] efi_loader: add signature verification functions

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.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_api.h              |  87 +++++
 include/efi_loader.h           |  72 ++++
 lib/efi_loader/Makefile        |   1 +
 lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
 4 files changed, 743 insertions(+)
 create mode 100644 lib/efi_loader/efi_signature.c

On 2020-04-14 04:51, 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.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/efi_api.h              |  87 +++++
>  include/efi_loader.h           |  72 ++++
>  lib/efi_loader/Makefile        |   1 +
>  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
>  4 files changed, 743 insertions(+)
>  create mode 100644 lib/efi_loader/efi_signature.c
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 1c40ffc4f56c..77d6bf2660b9 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>
> @@ -329,6 +330,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)
> @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
>
> +/* Certificate types in signature database */
> +#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)
> +
> +/**
> + * win_certificate_uefi_guid - A certificate that encapsulates
> + * a GUID-specific signature
> + *
> + * @hdr:	Windows certificate header
> + * @cert_type:	Certificate type
> + * @cert_data:	Certificate data
> + */
> +struct win_certificate_uefi_guid {
> +	WIN_CERTIFICATE	hdr;
> +	efi_guid_t	cert_type;
> +	u8		cert_data[];
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_variable_authentication_2 - A time-based authentication method
> + * descriptor
> + *
> + * This structure describes an authentication information for
> + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> + * and should be included as part of a variable's value.
> + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
> + *
> + * @time_stamp:	Descriptor's time stamp
> + * @auth_info:	Authentication info
> + */
> +struct efi_variable_authentication_2 {
> +	struct efi_time			 time_stamp;
> +	struct win_certificate_uefi_guid auth_info;
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_data - A format of signature
> + *
> + * This structure describes a single signature in signature database.
> + *
> + * @signature_owner:	Signature owner
> + * @signature_data:	Signature data
> + */
> +struct efi_signature_data {
> +	efi_guid_t	signature_owner;
> +	u8		signature_data[];

Please, use [0].

> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_list - A format of signature database
> + *
> + * This structure describes a list of signatures with the same type.
> + * An authenticated variable's value is a concatenation of one or more
> + * efi_signature_list's.
> + *
> + * @signature_type:		Signature type
> + * @signature_list_size:	Size of signature list
> + * @signature_header_size:	Size of signature header
> + * @signature_size:		Size of signature
> + */
> +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 3f2792892f34..8cf85d2fb7e2 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
>  #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
> @@ -178,6 +179,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;
> +/* GUIDs for authentication */
> +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;
>
>  /* GUID of RNG protocol */
>  extern const efi_guid_t efi_guid_rng_protocol;
> @@ -680,6 +686,72 @@ 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
> +#include <image.h>
> +
> +/**
> + * efi_image_regions - A list of memory regions
> + *
> + * @max:	Maximum number of regions
> + * @num:	Number of regions
> + * @reg:	array of regions
> + */
> +struct efi_image_regions {
> +	int			max;
> +	int			num;
> +	struct image_region	reg[];
> +};
> +
> +/**
> + * efi_sig_data - A decoded data of struct efi_signature_data
> + *
> + * This structure represents an internal form of signature in
> + * signature database. A listed list may represent a signature list.
> + *
> + * @next:	Pointer to next entry
> + * @onwer:	Signature owner
> + * @data:	Pointer to signature data
> + * @size:	Size of signature data
> + */
> +struct efi_sig_data {
> +	struct efi_sig_data *next;
> +	efi_guid_t owner;
> +	void *data;
> +	size_t size;
> +};
> +
> +/**
> + * efi_signature_store - A decoded data of signature database
> + *
> + * This structure represents an internal form of signature database.
> + *
> + * @next:		Pointer to next entry
> + * @sig_type:		Signature type
> + * @sig_data_list:	Pointer to signature list
> + */
> +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 9b3b70447336..eff3c25ec301 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
> --- /dev/null
> +++ b/lib/efi_loader/efi_signature.c
> @@ -0,0 +1,583 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
> + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> + */
> +
> +#include <common.h>
> +#include <charset.h>
> +#include <efi_loader.h>
> +#include <image.h>
> +#include <hexdump.h>
> +#include <malloc.h>
> +#include <pe.h>

You just added pe.h to efi_api.h which is included in efi_loader.h.
Please, avoid adding the includes twice.

> +#include <linux/compat.h>
> +#include <linux/oid_registry.h>
> +#include <u-boot/rsa.h>
> +#include <u-boot/sha256.h>
> +/*
> + * avoid duplicated inclusion:

Why don't you fix the lib/crypto/x509_parser.h that you created? Just
add the missing

#ifdef _X509_PARSER_H
#define _X509_PARSER_H
#endif /* _X509_PARSER_H */

Same problem in pkcs7_parser.h.

> + * #include "../lib/crypto/x509_parser.h"
> + */

If you patch is working I see no reason to add the comment here.

> +#include "../lib/crypto/pkcs7_parser.h"

Why is pkcs7_parser.h in lib/ and not in include/?

I would prefer to avoid relative paths.

I will remove the superfluous comment and include when merging. The rest
can be fixed later.

Best regards

Heinrich

> +
> +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
> +
> +/**
> + * 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.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> +			     size_t *size)
> +{
> +	*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");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	return true;
> +}
> +
> +/**
> + * efi_hash_msg_content - calculate a hash value of contentInfo
> + * @msg:	Signature
> + * @hash:	Pointer to a pointer to buffer holding a hash value
> + * @size:	Size of buffer to be returned
> + *
> + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> +				 size_t *size)
> +{
> +	struct image_region regtmp;
> +
> +	*size = 0;
> +	*hash = calloc(1, SHA256_SUM_LEN);
> +	if (!*hash) {
> +		debug("Out of memory\n");
> +		free(msg);
> +		return false;
> +	}
> +	*size = SHA256_SUM_LEN;
> +
> +	regtmp.data = msg->data;
> +	regtmp.size = msg->data_len;
> +
> +	hash_calculate("sha256", &regtmp, 1, *hash);
> +#ifdef DEBUG
> +	debug("hash calculated based on contentInfo:\n");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	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_message *msg,
> +				 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
> +		/* against contentInfo first */
> +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> +				/* for signed image */
> +		    efi_hash_regions(regs, &hash, &size)) {
> +				/* for authenticated variable */
> +			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
> + * @msg:		Signature
> + * @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_message *msg,
> +				    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);
> +
> +	if (!signed_info) {
> +		void *hash;
> +		size_t size;
> +
> +		debug("%s: unsigned image\n", __func__);
> +		/*
> +		 * 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);
> +#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, msg, signed_info, cert);
> +
> +		if (verified) {
> +			if (valid_cert)
> +				*valid_cert = cert;
> +			else
> +				x509_free_certificate(cert);
> +			break;
> +		}
> +		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, 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, msg, 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);
> +
> +	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 */
> +
> +			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 >= 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 */
>

On 2020-04-14 04:51, 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.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/efi_api.h              |  87 +++++
>  include/efi_loader.h           |  72 ++++
>  lib/efi_loader/Makefile        |   1 +
>  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
>  4 files changed, 743 insertions(+)
>  create mode 100644 lib/efi_loader/efi_signature.c
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 1c40ffc4f56c..77d6bf2660b9 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>
> @@ -329,6 +330,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)
> @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
>
> +/* Certificate types in signature database */
> +#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)
> +
> +/**
> + * win_certificate_uefi_guid - A certificate that encapsulates
> + * a GUID-specific signature
> + *
> + * @hdr:	Windows certificate header
> + * @cert_type:	Certificate type
> + * @cert_data:	Certificate data
> + */
> +struct win_certificate_uefi_guid {
> +	WIN_CERTIFICATE	hdr;
> +	efi_guid_t	cert_type;
> +	u8		cert_data[];
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_variable_authentication_2 - A time-based authentication method
> + * descriptor
> + *
> + * This structure describes an authentication information for
> + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> + * and should be included as part of a variable's value.
> + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
> + *
> + * @time_stamp:	Descriptor's time stamp
> + * @auth_info:	Authentication info
> + */
> +struct efi_variable_authentication_2 {
> +	struct efi_time			 time_stamp;
> +	struct win_certificate_uefi_guid auth_info;
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_data - A format of signature
> + *
> + * This structure describes a single signature in signature database.
> + *
> + * @signature_owner:	Signature owner
> + * @signature_data:	Signature data
> + */
> +struct efi_signature_data {
> +	efi_guid_t	signature_owner;
> +	u8		signature_data[];
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_list - A format of signature database
> + *
> + * This structure describes a list of signatures with the same type.
> + * An authenticated variable's value is a concatenation of one or more
> + * efi_signature_list's.
> + *
> + * @signature_type:		Signature type
> + * @signature_list_size:	Size of signature list
> + * @signature_header_size:	Size of signature header
> + * @signature_size:		Size of signature
> + */
> +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 3f2792892f34..8cf85d2fb7e2 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
>  #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
> @@ -178,6 +179,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;
> +/* GUIDs for authentication */
> +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;
>
>  /* GUID of RNG protocol */
>  extern const efi_guid_t efi_guid_rng_protocol;
> @@ -680,6 +686,72 @@ 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>
> +
> +/**
> + * efi_image_regions - A list of memory regions
> + *
> + * @max:	Maximum number of regions
> + * @num:	Number of regions
> + * @reg:	array of regions
> + */
> +struct efi_image_regions {
> +	int			max;
> +	int			num;
> +	struct image_region	reg[];
> +};
> +
> +/**
> + * efi_sig_data - A decoded data of struct efi_signature_data
> + *
> + * This structure represents an internal form of signature in
> + * signature database. A listed list may represent a signature list.
> + *
> + * @next:	Pointer to next entry
> + * @onwer:	Signature owner
> + * @data:	Pointer to signature data
> + * @size:	Size of signature data
> + */
> +struct efi_sig_data {
> +	struct efi_sig_data *next;
> +	efi_guid_t owner;
> +	void *data;
> +	size_t size;
> +};
> +
> +/**
> + * efi_signature_store - A decoded data of signature database
> + *
> + * This structure represents an internal form of signature database.
> + *
> + * @next:		Pointer to next entry
> + * @sig_type:		Signature type
> + * @sig_data_list:	Pointer to signature list
> + */
> +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 9b3b70447336..eff3c25ec301 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
> --- /dev/null
> +++ b/lib/efi_loader/efi_signature.c
> @@ -0,0 +1,583 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
> + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> + */
> +
> +#include <common.h>
> +#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>
> +#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
> +
> +/**
> + * 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.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> +			     size_t *size)
> +{
> +	*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");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	return true;
> +}
> +
> +/**
> + * efi_hash_msg_content - calculate a hash value of contentInfo
> + * @msg:	Signature
> + * @hash:	Pointer to a pointer to buffer holding a hash value
> + * @size:	Size of buffer to be returned
> + *
> + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> +				 size_t *size)
> +{
> +	struct image_region regtmp;
> +
> +	*size = 0;
> +	*hash = calloc(1, SHA256_SUM_LEN);
> +	if (!*hash) {
> +		debug("Out of memory\n");
> +		free(msg);
> +		return false;
> +	}
> +	*size = SHA256_SUM_LEN;
> +
> +	regtmp.data = msg->data;
> +	regtmp.size = msg->data_len;
> +
> +	hash_calculate("sha256", &regtmp, 1, *hash);
> +#ifdef DEBUG
> +	debug("hash calculated based on contentInfo:\n");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	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_message *msg,
> +				 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
> +		/* against contentInfo first */
> +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> +				/* for signed image */
> +		    efi_hash_regions(regs, &hash, &size)) {
> +				/* for authenticated variable */
> +			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
> + * @msg:		Signature
> + * @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_message *msg,
> +				    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);
> +
> +	if (!signed_info) {
> +		void *hash;
> +		size_t size;
> +
> +		debug("%s: unsigned image\n", __func__);
> +		/*
> +		 * 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);
> +#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, msg, signed_info, cert);
> +
> +		if (verified) {
> +			if (valid_cert)
> +				*valid_cert = cert;
> +			else
> +				x509_free_certificate(cert);
> +			break;
> +		}
> +		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, 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, msg, 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);
> +
> +	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 */
> +
> +			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 >= 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 */
>

If I define CONFIG_EFI_SECURE_BOOT I get a warning:

lib/efi_loader/efi_signature.c: In function ‘efi_hash_regions’:
lib/efi_loader/efi_signature.c:52:2: warning: implicit declaration of
function ‘hash_calculate’ [-Wimplicit-function-declaration]
   52 |  hash_calculate("sha256", regs->reg, regs->num, *hash);
      |  ^~~~~~~~~~~~~~


#include u-boot/rsa-checksum.h
seems to be missing in the code.

Best regards

Heinrich

On 2020-04-14 16:52, Heinrich Schuchardt wrote:
> On 2020-04-14 04:51, 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.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> ---
>>  include/efi_api.h              |  87 +++++
>>  include/efi_loader.h           |  72 ++++
>>  lib/efi_loader/Makefile        |   1 +
>>  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
>>  4 files changed, 743 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_signature.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index 1c40ffc4f56c..77d6bf2660b9 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>
>> @@ -329,6 +330,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)
>> @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
>>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
>>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
>>
>> +/* Certificate types in signature database */
>> +#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)
>> +
>> +/**
>> + * win_certificate_uefi_guid - A certificate that encapsulates
>> + * a GUID-specific signature
>> + *
>> + * @hdr:	Windows certificate header
>> + * @cert_type:	Certificate type
>> + * @cert_data:	Certificate data
>> + */
>> +struct win_certificate_uefi_guid {
>> +	WIN_CERTIFICATE	hdr;
>> +	efi_guid_t	cert_type;
>> +	u8		cert_data[];
>> +} __attribute__((__packed__));
>> +
>> +/**
>> + * efi_variable_authentication_2 - A time-based authentication method
>> + * descriptor
>> + *
>> + * This structure describes an authentication information for
>> + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>> + * and should be included as part of a variable's value.
>> + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
>> + *
>> + * @time_stamp:	Descriptor's time stamp
>> + * @auth_info:	Authentication info
>> + */
>> +struct efi_variable_authentication_2 {
>> +	struct efi_time			 time_stamp;
>> +	struct win_certificate_uefi_guid auth_info;
>> +} __attribute__((__packed__));
>> +
>> +/**
>> + * efi_signature_data - A format of signature
>> + *
>> + * This structure describes a single signature in signature database.
>> + *
>> + * @signature_owner:	Signature owner
>> + * @signature_data:	Signature data
>> + */
>> +struct efi_signature_data {
>> +	efi_guid_t	signature_owner;
>> +	u8		signature_data[];
>> +} __attribute__((__packed__));
>> +
>> +/**
>> + * efi_signature_list - A format of signature database
>> + *
>> + * This structure describes a list of signatures with the same type.
>> + * An authenticated variable's value is a concatenation of one or more
>> + * efi_signature_list's.
>> + *
>> + * @signature_type:		Signature type
>> + * @signature_list_size:	Size of signature list
>> + * @signature_header_size:	Size of signature header
>> + * @signature_size:		Size of signature
>> + */
>> +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 3f2792892f34..8cf85d2fb7e2 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
>>  #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
>> @@ -178,6 +179,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;
>> +/* GUIDs for authentication */
>> +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;
>>
>>  /* GUID of RNG protocol */
>>  extern const efi_guid_t efi_guid_rng_protocol;
>> @@ -680,6 +686,72 @@ 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>
>> +
>> +/**
>> + * efi_image_regions - A list of memory regions
>> + *
>> + * @max:	Maximum number of regions
>> + * @num:	Number of regions
>> + * @reg:	array of regions
>> + */
>> +struct efi_image_regions {
>> +	int			max;
>> +	int			num;
>> +	struct image_region	reg[];
>> +};
>> +
>> +/**
>> + * efi_sig_data - A decoded data of struct efi_signature_data
>> + *
>> + * This structure represents an internal form of signature in
>> + * signature database. A listed list may represent a signature list.
>> + *
>> + * @next:	Pointer to next entry
>> + * @onwer:	Signature owner
>> + * @data:	Pointer to signature data
>> + * @size:	Size of signature data
>> + */
>> +struct efi_sig_data {
>> +	struct efi_sig_data *next;
>> +	efi_guid_t owner;
>> +	void *data;
>> +	size_t size;
>> +};
>> +
>> +/**
>> + * efi_signature_store - A decoded data of signature database
>> + *
>> + * This structure represents an internal form of signature database.
>> + *
>> + * @next:		Pointer to next entry
>> + * @sig_type:		Signature type
>> + * @sig_data_list:	Pointer to signature list
>> + */
>> +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 9b3b70447336..eff3c25ec301 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_signature.c
>> @@ -0,0 +1,583 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
>> + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
>> + */
>> +
>> +#include <common.h>
>> +#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>
>> +#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
>> +
>> +/**
>> + * 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.
>> + *
>> + * Return:	true on success, false on error
>> + */
>> +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
>> +			     size_t *size)
>> +{
>> +	*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");
>> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
>> +		       *hash, SHA256_SUM_LEN, false);
>> +#endif
>> +
>> +	return true;
>> +}
>> +
>> +/**
>> + * efi_hash_msg_content - calculate a hash value of contentInfo
>> + * @msg:	Signature
>> + * @hash:	Pointer to a pointer to buffer holding a hash value
>> + * @size:	Size of buffer to be returned
>> + *
>> + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
>> + *
>> + * Return:	true on success, false on error
>> + */
>> +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
>> +				 size_t *size)
>> +{
>> +	struct image_region regtmp;
>> +
>> +	*size = 0;
>> +	*hash = calloc(1, SHA256_SUM_LEN);
>> +	if (!*hash) {
>> +		debug("Out of memory\n");
>> +		free(msg);
>> +		return false;
>> +	}
>> +	*size = SHA256_SUM_LEN;
>> +
>> +	regtmp.data = msg->data;
>> +	regtmp.size = msg->data_len;
>> +
>> +	hash_calculate("sha256", &regtmp, 1, *hash);
>> +#ifdef DEBUG
>> +	debug("hash calculated based on contentInfo:\n");
>> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
>> +		       *hash, SHA256_SUM_LEN, false);
>> +#endif
>> +
>> +	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_message *msg,
>> +				 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
>> +		/* against contentInfo first */
>> +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
>> +				/* for signed image */
>> +		    efi_hash_regions(regs, &hash, &size)) {
>> +				/* for authenticated variable */
>> +			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
>> + * @msg:		Signature
>> + * @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_message *msg,
>> +				    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);
>> +
>> +	if (!signed_info) {
>> +		void *hash;
>> +		size_t size;
>> +
>> +		debug("%s: unsigned image\n", __func__);
>> +		/*
>> +		 * 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);
>> +#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, msg, signed_info, cert);
>> +
>> +		if (verified) {
>> +			if (valid_cert)
>> +				*valid_cert = cert;
>> +			else
>> +				x509_free_certificate(cert);
>> +			break;
>> +		}
>> +		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, 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, msg, 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);
>> +
>> +	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 */
>> +
>> +			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 >= 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 */
>>
>
> If I define CONFIG_EFI_SECURE_BOOT I get a warning:
>
> lib/efi_loader/efi_signature.c: In function ‘efi_hash_regions’:
> lib/efi_loader/efi_signature.c:52:2: warning: implicit declaration of
> function ‘hash_calculate’ [-Wimplicit-function-declaration]
>    52 |  hash_calculate("sha256", regs->reg, regs->num, *hash);
>       |  ^~~~~~~~~~~~~~
>
>
> #include u-boot/rsa-checksum.h
> seems to be missing in the code.


The problem only occurs with an old version of your patch. Sorry.

>
> Best regards
>
> Heinrich
>

On 4/14/20 4:51 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.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/efi_api.h              |  87 +++++
>  include/efi_loader.h           |  72 ++++
>  lib/efi_loader/Makefile        |   1 +
>  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
>  4 files changed, 743 insertions(+)
>  create mode 100644 lib/efi_loader/efi_signature.c
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 1c40ffc4f56c..77d6bf2660b9 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>
> @@ -329,6 +330,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)
> @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
>  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
>
> +/* Certificate types in signature database */
> +#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)
> +
> +/**
> + * win_certificate_uefi_guid - A certificate that encapsulates
> + * a GUID-specific signature
> + *
> + * @hdr:	Windows certificate header
> + * @cert_type:	Certificate type
> + * @cert_data:	Certificate data
> + */
> +struct win_certificate_uefi_guid {
> +	WIN_CERTIFICATE	hdr;
> +	efi_guid_t	cert_type;
> +	u8		cert_data[];
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_variable_authentication_2 - A time-based authentication method
> + * descriptor
> + *
> + * This structure describes an authentication information for
> + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> + * and should be included as part of a variable's value.
> + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
> + *
> + * @time_stamp:	Descriptor's time stamp
> + * @auth_info:	Authentication info
> + */
> +struct efi_variable_authentication_2 {
> +	struct efi_time			 time_stamp;
> +	struct win_certificate_uefi_guid auth_info;
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_data - A format of signature
> + *
> + * This structure describes a single signature in signature database.
> + *
> + * @signature_owner:	Signature owner
> + * @signature_data:	Signature data
> + */
> +struct efi_signature_data {
> +	efi_guid_t	signature_owner;
> +	u8		signature_data[];
> +} __attribute__((__packed__));
> +
> +/**
> + * efi_signature_list - A format of signature database
> + *
> + * This structure describes a list of signatures with the same type.
> + * An authenticated variable's value is a concatenation of one or more
> + * efi_signature_list's.
> + *
> + * @signature_type:		Signature type
> + * @signature_list_size:	Size of signature list
> + * @signature_header_size:	Size of signature header
> + * @signature_size:		Size of signature
> + */
> +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 3f2792892f34..8cf85d2fb7e2 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
>  #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
> @@ -178,6 +179,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;
> +/* GUIDs for authentication */
> +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;
>
>  /* GUID of RNG protocol */
>  extern const efi_guid_t efi_guid_rng_protocol;
> @@ -680,6 +686,72 @@ 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
> +#include <image.h>
> +
> +/**
> + * efi_image_regions - A list of memory regions
> + *
> + * @max:	Maximum number of regions
> + * @num:	Number of regions
> + * @reg:	array of regions
> + */
> +struct efi_image_regions {
> +	int			max;
> +	int			num;
> +	struct image_region	reg[];
> +};
> +
> +/**
> + * efi_sig_data - A decoded data of struct efi_signature_data
> + *
> + * This structure represents an internal form of signature in
> + * signature database. A listed list may represent a signature list.
> + *
> + * @next:	Pointer to next entry
> + * @onwer:	Signature owner
> + * @data:	Pointer to signature data
> + * @size:	Size of signature data
> + */
> +struct efi_sig_data {
> +	struct efi_sig_data *next;
> +	efi_guid_t owner;
> +	void *data;
> +	size_t size;
> +};
> +
> +/**
> + * efi_signature_store - A decoded data of signature database
> + *
> + * This structure represents an internal form of signature database.
> + *
> + * @next:		Pointer to next entry
> + * @sig_type:		Signature type
> + * @sig_data_list:	Pointer to signature list
> + */
> +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 9b3b70447336..eff3c25ec301 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
>  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
>  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
> --- /dev/null
> +++ b/lib/efi_loader/efi_signature.c
> @@ -0,0 +1,583 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
> + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> + */
> +
> +#include <common.h>
> +#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>
> +#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
> +
> +/**
> + * 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.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> +			     size_t *size)
> +{
> +	*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");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	return true;
> +}
> +
> +/**
> + * efi_hash_msg_content - calculate a hash value of contentInfo
> + * @msg:	Signature
> + * @hash:	Pointer to a pointer to buffer holding a hash value
> + * @size:	Size of buffer to be returned
> + *
> + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> + *
> + * Return:	true on success, false on error
> + */
> +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> +				 size_t *size)
> +{
> +	struct image_region regtmp;
> +
> +	*size = 0;
> +	*hash = calloc(1, SHA256_SUM_LEN);
> +	if (!*hash) {
> +		debug("Out of memory\n");
> +		free(msg);
> +		return false;
> +	}
> +	*size = SHA256_SUM_LEN;
> +
> +	regtmp.data = msg->data;
> +	regtmp.size = msg->data_len;
> +
> +	hash_calculate("sha256", &regtmp, 1, *hash);
> +#ifdef DEBUG
> +	debug("hash calculated based on contentInfo:\n");
> +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> +		       *hash, SHA256_SUM_LEN, false);
> +#endif
> +
> +	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_message *msg,
> +				 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")) {

There is a call
efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
So ps_info may be NULL

> +		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)) {

There is a call
efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
So ps_info may be NULL

> +		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
> +		/* against contentInfo first */
> +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> +				/* for signed image */

There is a call
efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
So msg may be NULL.

Either avoid the call will NULL or check the parameters before
dereferencing.

Please, provide a follow up patch.

I used cppcheck to identify this problem.

Best regards

Heinrich

> +		    efi_hash_regions(regs, &hash, &size)) {
> +				/* for authenticated variable */
> +			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
> + * @msg:		Signature
> + * @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_message *msg,
> +				    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);
> +
> +	if (!signed_info) {
> +		void *hash;
> +		size_t size;
> +
> +		debug("%s: unsigned image\n", __func__);
> +		/*
> +		 * 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);
> +#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, msg, signed_info, cert);
> +
> +		if (verified) {
> +			if (valid_cert)
> +				*valid_cert = cert;
> +			else
> +				x509_free_certificate(cert);
> +			break;
> +		}
> +		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, 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, msg, 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);
> +
> +	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 */
> +
> +			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 >= 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 */
>

Heinrich,

On Tue, Apr 14, 2020 at 04:36:19PM +0200, Heinrich Schuchardt wrote:
> On 2020-04-14 04:51, 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.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  include/efi_api.h              |  87 +++++
> >  include/efi_loader.h           |  72 ++++
> >  lib/efi_loader/Makefile        |   1 +
> >  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
> >  4 files changed, 743 insertions(+)
> >  create mode 100644 lib/efi_loader/efi_signature.c
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 1c40ffc4f56c..77d6bf2660b9 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>
> > @@ -329,6 +330,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)
> > @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
> >  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
> >  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
> >
> > +/* Certificate types in signature database */
> > +#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)
> > +
> > +/**
> > + * win_certificate_uefi_guid - A certificate that encapsulates
> > + * a GUID-specific signature
> > + *
> > + * @hdr:	Windows certificate header
> > + * @cert_type:	Certificate type
> > + * @cert_data:	Certificate data
> > + */
> > +struct win_certificate_uefi_guid {
> > +	WIN_CERTIFICATE	hdr;
> > +	efi_guid_t	cert_type;
> > +	u8		cert_data[];
> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_variable_authentication_2 - A time-based authentication method
> > + * descriptor
> > + *
> > + * This structure describes an authentication information for
> > + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> > + * and should be included as part of a variable's value.
> > + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
> > + *
> > + * @time_stamp:	Descriptor's time stamp
> > + * @auth_info:	Authentication info
> > + */
> > +struct efi_variable_authentication_2 {
> > +	struct efi_time			 time_stamp;
> > +	struct win_certificate_uefi_guid auth_info;
> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_signature_data - A format of signature
> > + *
> > + * This structure describes a single signature in signature database.
> > + *
> > + * @signature_owner:	Signature owner
> > + * @signature_data:	Signature data
> > + */
> > +struct efi_signature_data {
> > +	efi_guid_t	signature_owner;
> > +	u8		signature_data[];
> 
> Please, use [0].

Actually, there are a bunch of uses of [] notation in
efi.h, efi_api.h and efi_loader.h.

Do you really want to modify all?

> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_signature_list - A format of signature database
> > + *
> > + * This structure describes a list of signatures with the same type.
> > + * An authenticated variable's value is a concatenation of one or more
> > + * efi_signature_list's.
> > + *
> > + * @signature_type:		Signature type
> > + * @signature_list_size:	Size of signature list
> > + * @signature_header_size:	Size of signature header
> > + * @signature_size:		Size of signature
> > + */
> > +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 3f2792892f34..8cf85d2fb7e2 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
> >  #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
> > @@ -178,6 +179,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;
> > +/* GUIDs for authentication */
> > +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;
> >
> >  /* GUID of RNG protocol */
> >  extern const efi_guid_t efi_guid_rng_protocol;
> > @@ -680,6 +686,72 @@ 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
> > +#include <image.h>
> > +
> > +/**
> > + * efi_image_regions - A list of memory regions
> > + *
> > + * @max:	Maximum number of regions
> > + * @num:	Number of regions
> > + * @reg:	array of regions
> > + */
> > +struct efi_image_regions {
> > +	int			max;
> > +	int			num;
> > +	struct image_region	reg[];
> > +};
> > +
> > +/**
> > + * efi_sig_data - A decoded data of struct efi_signature_data
> > + *
> > + * This structure represents an internal form of signature in
> > + * signature database. A listed list may represent a signature list.
> > + *
> > + * @next:	Pointer to next entry
> > + * @onwer:	Signature owner
> > + * @data:	Pointer to signature data
> > + * @size:	Size of signature data
> > + */
> > +struct efi_sig_data {
> > +	struct efi_sig_data *next;
> > +	efi_guid_t owner;
> > +	void *data;
> > +	size_t size;
> > +};
> > +
> > +/**
> > + * efi_signature_store - A decoded data of signature database
> > + *
> > + * This structure represents an internal form of signature database.
> > + *
> > + * @next:		Pointer to next entry
> > + * @sig_type:		Signature type
> > + * @sig_data_list:	Pointer to signature list
> > + */
> > +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 9b3b70447336..eff3c25ec301 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> >  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -0,0 +1,583 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
> > + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> > + */
> > +
> > +#include <common.h>
> > +#include <charset.h>
> > +#include <efi_loader.h>
> > +#include <image.h>
> > +#include <hexdump.h>
> > +#include <malloc.h>
> > +#include <pe.h>
> 
> You just added pe.h to efi_api.h which is included in efi_loader.h.
> Please, avoid adding the includes twice.

I agree as there is no use of any symbols from pe.h here.

> > +#include <linux/compat.h>
> > +#include <linux/oid_registry.h>
> > +#include <u-boot/rsa.h>
> > +#include <u-boot/sha256.h>
> > +/*
> > + * avoid duplicated inclusion:
> 
> Why don't you fix the lib/crypto/x509_parser.h that you created? Just
> add the missing
> 
> #ifdef _X509_PARSER_H
> #define _X509_PARSER_H
> #endif /* _X509_PARSER_H */

Okay.

> Same problem in pkcs7_parser.h.
> 
> > + * #include "../lib/crypto/x509_parser.h"
> > + */
> 
> If you patch is working I see no reason to add the comment here.
> 
> > +#include "../lib/crypto/pkcs7_parser.h"
> 
> Why is pkcs7_parser.h in lib/ and not in include/?
> 
> I would prefer to avoid relative paths.

My basic approach was to minimize the modification to the original
(i.e. linux code) as much as possible.
Anyhow, I will try to fix it.

Thanks,
-Takahiro Akashi

> I will remove the superfluous comment and include when merging. The rest
> can be fixed later.
> 
> Best regards
> 
> Heinrich
> 
> > +
> > +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
> > +
> > +/**
> > + * 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.
> > + *
> > + * Return:	true on success, false on error
> > + */
> > +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> > +			     size_t *size)
> > +{
> > +	*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");
> > +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> > +		       *hash, SHA256_SUM_LEN, false);
> > +#endif
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * efi_hash_msg_content - calculate a hash value of contentInfo
> > + * @msg:	Signature
> > + * @hash:	Pointer to a pointer to buffer holding a hash value
> > + * @size:	Size of buffer to be returned
> > + *
> > + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> > + *
> > + * Return:	true on success, false on error
> > + */
> > +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> > +				 size_t *size)
> > +{
> > +	struct image_region regtmp;
> > +
> > +	*size = 0;
> > +	*hash = calloc(1, SHA256_SUM_LEN);
> > +	if (!*hash) {
> > +		debug("Out of memory\n");
> > +		free(msg);
> > +		return false;
> > +	}
> > +	*size = SHA256_SUM_LEN;
> > +
> > +	regtmp.data = msg->data;
> > +	regtmp.size = msg->data_len;
> > +
> > +	hash_calculate("sha256", &regtmp, 1, *hash);
> > +#ifdef DEBUG
> > +	debug("hash calculated based on contentInfo:\n");
> > +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> > +		       *hash, SHA256_SUM_LEN, false);
> > +#endif
> > +
> > +	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_message *msg,
> > +				 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
> > +		/* against contentInfo first */
> > +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> > +				/* for signed image */
> > +		    efi_hash_regions(regs, &hash, &size)) {
> > +				/* for authenticated variable */
> > +			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
> > + * @msg:		Signature
> > + * @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_message *msg,
> > +				    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);
> > +
> > +	if (!signed_info) {
> > +		void *hash;
> > +		size_t size;
> > +
> > +		debug("%s: unsigned image\n", __func__);
> > +		/*
> > +		 * 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);
> > +#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, msg, signed_info, cert);
> > +
> > +		if (verified) {
> > +			if (valid_cert)
> > +				*valid_cert = cert;
> > +			else
> > +				x509_free_certificate(cert);
> > +			break;
> > +		}
> > +		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, 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, msg, 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);
> > +
> > +	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 */
> > +
> > +			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 >= 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 */
> >
>

Heinrich,

On Fri, Apr 17, 2020 at 08:16:06PM +0200, Heinrich Schuchardt wrote:
> On 4/14/20 4:51 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.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  include/efi_api.h              |  87 +++++
> >  include/efi_loader.h           |  72 ++++
> >  lib/efi_loader/Makefile        |   1 +
> >  lib/efi_loader/efi_signature.c | 583 +++++++++++++++++++++++++++++++++
> >  4 files changed, 743 insertions(+)
> >  create mode 100644 lib/efi_loader/efi_signature.c
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 1c40ffc4f56c..77d6bf2660b9 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>
> > @@ -329,6 +330,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)
> > @@ -1682,4 +1687,86 @@ struct efi_load_file_protocol {
> >  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MIN 0x00001000
> >  #define LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX 0x00004000
> >
> > +/* Certificate types in signature database */
> > +#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)
> > +
> > +/**
> > + * win_certificate_uefi_guid - A certificate that encapsulates
> > + * a GUID-specific signature
> > + *
> > + * @hdr:	Windows certificate header
> > + * @cert_type:	Certificate type
> > + * @cert_data:	Certificate data
> > + */
> > +struct win_certificate_uefi_guid {
> > +	WIN_CERTIFICATE	hdr;
> > +	efi_guid_t	cert_type;
> > +	u8		cert_data[];
> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_variable_authentication_2 - A time-based authentication method
> > + * descriptor
> > + *
> > + * This structure describes an authentication information for
> > + * a variable with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> > + * and should be included as part of a variable's value.
> > + * Only EFI_CERT_TYPE_PKCS7_GUID is accepted.
> > + *
> > + * @time_stamp:	Descriptor's time stamp
> > + * @auth_info:	Authentication info
> > + */
> > +struct efi_variable_authentication_2 {
> > +	struct efi_time			 time_stamp;
> > +	struct win_certificate_uefi_guid auth_info;
> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_signature_data - A format of signature
> > + *
> > + * This structure describes a single signature in signature database.
> > + *
> > + * @signature_owner:	Signature owner
> > + * @signature_data:	Signature data
> > + */
> > +struct efi_signature_data {
> > +	efi_guid_t	signature_owner;
> > +	u8		signature_data[];
> > +} __attribute__((__packed__));
> > +
> > +/**
> > + * efi_signature_list - A format of signature database
> > + *
> > + * This structure describes a list of signatures with the same type.
> > + * An authenticated variable's value is a concatenation of one or more
> > + * efi_signature_list's.
> > + *
> > + * @signature_type:		Signature type
> > + * @signature_list_size:	Size of signature list
> > + * @signature_header_size:	Size of signature header
> > + * @signature_size:		Size of signature
> > + */
> > +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 3f2792892f34..8cf85d2fb7e2 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -26,6 +26,7 @@ static inline void *guidcpy(void *dst, const void *src)
> >  #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
> > @@ -178,6 +179,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;
> > +/* GUIDs for authentication */
> > +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;
> >
> >  /* GUID of RNG protocol */
> >  extern const efi_guid_t efi_guid_rng_protocol;
> > @@ -680,6 +686,72 @@ 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
> > +#include <image.h>
> > +
> > +/**
> > + * efi_image_regions - A list of memory regions
> > + *
> > + * @max:	Maximum number of regions
> > + * @num:	Number of regions
> > + * @reg:	array of regions
> > + */
> > +struct efi_image_regions {
> > +	int			max;
> > +	int			num;
> > +	struct image_region	reg[];
> > +};
> > +
> > +/**
> > + * efi_sig_data - A decoded data of struct efi_signature_data
> > + *
> > + * This structure represents an internal form of signature in
> > + * signature database. A listed list may represent a signature list.
> > + *
> > + * @next:	Pointer to next entry
> > + * @onwer:	Signature owner
> > + * @data:	Pointer to signature data
> > + * @size:	Size of signature data
> > + */
> > +struct efi_sig_data {
> > +	struct efi_sig_data *next;
> > +	efi_guid_t owner;
> > +	void *data;
> > +	size_t size;
> > +};
> > +
> > +/**
> > + * efi_signature_store - A decoded data of signature database
> > + *
> > + * This structure represents an internal form of signature database.
> > + *
> > + * @next:		Pointer to next entry
> > + * @sig_type:		Signature type
> > + * @sig_data_list:	Pointer to signature list
> > + */
> > +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 9b3b70447336..eff3c25ec301 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -44,3 +44,4 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o
> >  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> >  obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o
> >  obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.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..23dac94c0593
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -0,0 +1,583 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2018 Patrick Wildt <patrick@blueri.se>
> > + * Copyright (c) 2019 Linaro Limited, Author: AKASHI Takahiro
> > + */
> > +
> > +#include <common.h>
> > +#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>
> > +#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
> > +
> > +/**
> > + * 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.
> > + *
> > + * Return:	true on success, false on error
> > + */
> > +static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> > +			     size_t *size)
> > +{
> > +	*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");
> > +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> > +		       *hash, SHA256_SUM_LEN, false);
> > +#endif
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * efi_hash_msg_content - calculate a hash value of contentInfo
> > + * @msg:	Signature
> > + * @hash:	Pointer to a pointer to buffer holding a hash value
> > + * @size:	Size of buffer to be returned
> > + *
> > + * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> > + *
> > + * Return:	true on success, false on error
> > + */
> > +static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> > +				 size_t *size)
> > +{
> > +	struct image_region regtmp;
> > +
> > +	*size = 0;
> > +	*hash = calloc(1, SHA256_SUM_LEN);
> > +	if (!*hash) {
> > +		debug("Out of memory\n");
> > +		free(msg);
> > +		return false;
> > +	}
> > +	*size = SHA256_SUM_LEN;
> > +
> > +	regtmp.data = msg->data;
> > +	regtmp.size = msg->data_len;
> > +
> > +	hash_calculate("sha256", &regtmp, 1, *hash);
> > +#ifdef DEBUG
> > +	debug("hash calculated based on contentInfo:\n");
> > +	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
> > +		       *hash, SHA256_SUM_LEN, false);
> > +#endif
> > +
> > +	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_message *msg,
> > +				 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")) {
> 
> There is a call
> efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
> So ps_info may be NULL

efi_signature_verify() is called only if signed_info is not NULL.

> > +		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)) {
> 
> There is a call
> efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
> So ps_info may be NULL

ditto

> > +		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
> > +		/* against contentInfo first */
> > +		if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> > +				/* for signed image */
> 
> There is a call
> efi_signature_verify_with_list(regs, NULL, NULL, siglist, cert)
> So msg may be NULL.

msg is used only if signed_info is also provided.
Please note that <msg,psinfo> == <NULL,NULL> case means that the image
is not signed. That is why the code causes no runtime memory fault.

> Either avoid the call will NULL or check the parameters before
> dereferencing.
> 
> Please, provide a follow up patch.

Therefore I don't think that any fix will be necessary.

Thanks,
-Takahiro Akashi


> I used cppcheck to identify this problem.
> 
> Best regards
> 
> Heinrich
> 
> > +		    efi_hash_regions(regs, &hash, &size)) {
> > +				/* for authenticated variable */
> > +			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
> > + * @msg:		Signature
> > + * @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_message *msg,
> > +				    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);
> > +
> > +	if (!signed_info) {
> > +		void *hash;
> > +		size_t size;
> > +
> > +		debug("%s: unsigned image\n", __func__);
> > +		/*
> > +		 * 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);
> > +#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, msg, signed_info, cert);
> > +
> > +		if (verified) {
> > +			if (valid_cert)
> > +				*valid_cert = cert;
> > +			else
> > +				x509_free_certificate(cert);
> > +			break;
> > +		}
> > +		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, 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, msg, 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);
> > +
> > +	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 */
> > +
> > +			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 >= 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 */
> >
>