diff mbox series

[v5,07/16] efi_loader: image_loader: support image authentication

Message ID 20200128082532.15943-8-takahiro.akashi@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: add secure boot support | expand

Commit Message

Takahiro Akashi Jan. 28, 2020, 8:25 a.m. UTC
With this commit, image validation can be enforced, as UEFI specification
section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.

Currently we support
* authentication based on db and dbx,
  so dbx-validated image will always be rejected.
* following signature types:
    EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
    EFI_CERT_X509_GUID (x509 certificate for signed images)
Timestamp-based certificate revocation is not supported here.

Internally, authentication data is stored in one of certificates tables
of PE image (See efi_image_parse()) and will be verified by
efi_image_authenticate() before loading a given image.

It seems that UEFI specification defines the verification process
in a bit ambiguous way. I tried to implement it as closely to as
EDK2 does.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_loader.h              |  13 +-
 lib/efi_loader/efi_boottime.c     |  10 +-
 lib/efi_loader/efi_image_loader.c | 460 +++++++++++++++++++++++++++++-
 3 files changed, 467 insertions(+), 16 deletions(-)

Comments

Heinrich Schuchardt Feb. 24, 2020, 6:29 p.m. UTC | #1
On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> With this commit, image validation can be enforced, as UEFI specification
> section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
>
> Currently we support
> * authentication based on db and dbx,
>    so dbx-validated image will always be rejected.
> * following signature types:
>      EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
>      EFI_CERT_X509_GUID (x509 certificate for signed images)
> Timestamp-based certificate revocation is not supported here.
>
> Internally, authentication data is stored in one of certificates tables
> of PE image (See efi_image_parse()) and will be verified by
> efi_image_authenticate() before loading a given image.
>
> It seems that UEFI specification defines the verification process
> in a bit ambiguous way. I tried to implement it as closely to as
> EDK2 does.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

According to git bisect this patch breaks the test
test/py/tests/test_efi_fit.py.

Best regards

Heinrich
Takahiro Akashi Feb. 25, 2020, 5:25 a.m. UTC | #2
On Mon, Feb 24, 2020 at 07:29:17PM +0100, Heinrich Schuchardt wrote:
> On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> > With this commit, image validation can be enforced, as UEFI specification
> > section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
> > 
> > Currently we support
> > * authentication based on db and dbx,
> >    so dbx-validated image will always be rejected.
> > * following signature types:
> >      EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
> >      EFI_CERT_X509_GUID (x509 certificate for signed images)
> > Timestamp-based certificate revocation is not supported here.
> > 
> > Internally, authentication data is stored in one of certificates tables
> > of PE image (See efi_image_parse()) and will be verified by
> > efi_image_authenticate() before loading a given image.
> > 
> > It seems that UEFI specification defines the verification process
> > in a bit ambiguous way. I tried to implement it as closely to as
> > EDK2 does.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> According to git bisect this patch breaks the test
> test/py/tests/test_efi_fit.py.

This error only occurs on "compressed" FIT images. While I'm not sure
whether it is directly related to efi support in bootm or not, I've
fixed it any way.

Thanks,
-Takahiro Akashi


> Best regards
> 
> Heinrich
Heinrich Schuchardt Feb. 25, 2020, 6:40 a.m. UTC | #3
On 2/25/20 6:25 AM, AKASHI Takahiro wrote:
> On Mon, Feb 24, 2020 at 07:29:17PM +0100, Heinrich Schuchardt wrote:
>> On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
>>> With this commit, image validation can be enforced, as UEFI specification
>>> section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
>>>
>>> Currently we support
>>> * authentication based on db and dbx,
>>>     so dbx-validated image will always be rejected.
>>> * following signature types:
>>>       EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
>>>       EFI_CERT_X509_GUID (x509 certificate for signed images)
>>> Timestamp-based certificate revocation is not supported here.
>>>
>>> Internally, authentication data is stored in one of certificates tables
>>> of PE image (See efi_image_parse()) and will be verified by
>>> efi_image_authenticate() before loading a given image.
>>>
>>> It seems that UEFI specification defines the verification process
>>> in a bit ambiguous way. I tried to implement it as closely to as
>>> EDK2 does.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> According to git bisect this patch breaks the test
>> test/py/tests/test_efi_fit.py.
>
> This error only occurs on "compressed" FIT images. While I'm not sure
> whether it is directly related to efi support in bootm or not, I've
> fixed it any way.

Hello Takahiro,

where can I find the fix?

Best regards

Heinrich

>
> Thanks,
> -Takahiro Akashi
>
>
>> Best regards
>>
>> Heinrich
Heinrich Schuchardt Feb. 25, 2020, 6:44 a.m. UTC | #4
On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> With this commit, image validation can be enforced, as UEFI specification
> section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
>
> Currently we support
> * authentication based on db and dbx,
>    so dbx-validated image will always be rejected.
> * following signature types:
>      EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
>      EFI_CERT_X509_GUID (x509 certificate for signed images)
> Timestamp-based certificate revocation is not supported here.
>
> Internally, authentication data is stored in one of certificates tables
> of PE image (See efi_image_parse()) and will be verified by
> efi_image_authenticate() before loading a given image.
>
> It seems that UEFI specification defines the verification process
> in a bit ambiguous way. I tried to implement it as closely to as
> EDK2 does.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/efi_loader.h              |  13 +-
>   lib/efi_loader/efi_boottime.c     |  10 +-
>   lib/efi_loader/efi_image_loader.c | 460 +++++++++++++++++++++++++++++-
>   3 files changed, 467 insertions(+), 16 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index f461c6195834..0e15470d9c17 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -11,6 +11,7 @@
>   #include <common.h>
>   #include <part_efi.h>
>   #include <efi_api.h>
> +#include <pe.h>
>
>   static inline int guidcmp(const void *g1, const void *g2)
>   {
> @@ -263,6 +264,11 @@ struct efi_object {
>   	enum efi_object_type type;
>   };
>
> +enum efi_image_auth_status {
> +	EFI_IMAGE_AUTH_FAILED = 0,
> +	EFI_IMAGE_AUTH_PASSED,
> +};
> +
>   /**
>    * struct efi_loaded_image_obj - handle of a loaded image
>    *
> @@ -282,6 +288,7 @@ struct efi_loaded_image_obj {
>   	EFIAPI efi_status_t (*entry)(efi_handle_t image_handle,
>   				     struct efi_system_table *st);
>   	u16 image_type;
> +	enum efi_image_auth_status auth_status;
>   };
>
>   /**
> @@ -414,7 +421,8 @@ efi_status_t efi_set_watchdog(unsigned long timeout);
>   /* Called from places to check whether a timer expired */
>   void efi_timer_check(void);
>   /* PE loader implementation */
> -efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> +efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> +			 void *efi, size_t efi_size,
>   			 struct efi_loaded_image *loaded_image_info);
>   /* Called once to store the pristine gd pointer */
>   void efi_save_gd(void);
> @@ -741,6 +749,9 @@ void efi_sigstore_free(struct efi_signature_store *sigstore);
>   struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
>
>   bool efi_secure_boot_enabled(void);
> +
> +bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> +		     WIN_CERTIFICATE **auth, size_t *auth_len);
>   #endif /* CONFIG_EFI_SECURE_BOOT */
>
>   #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 1f598b357a5c..cc8cc4cb5408 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1882,12 +1882,12 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
>   	efi_dp_split_file_path(file_path, &dp, &fp);
>   	ret = efi_setup_loaded_image(dp, fp, image_obj, &info);
>   	if (ret == EFI_SUCCESS)
> -		ret = efi_load_pe(*image_obj, dest_buffer, info);
> +		ret = efi_load_pe(*image_obj, dest_buffer, source_size, info);
>   	if (!source_buffer)
>   		/* Release buffer to which file was loaded */
>   		efi_free_pages((uintptr_t)dest_buffer,
>   			       efi_size_in_pages(source_size));
> -	if (ret == EFI_SUCCESS) {
> +	if (ret == EFI_SUCCESS || ret == EFI_SECURITY_VIOLATION) {
>   		info->system_table = &systab;
>   		info->parent_handle = parent_image;
>   	} else {
> @@ -2885,10 +2885,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>
>   	EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data);
>
> +	if (!efi_search_obj(image_handle))
> +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> +
>   	/* Check parameters */
>   	if (image_obj->header.type != EFI_OBJECT_TYPE_LOADED_IMAGE)
>   		return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> +	if (image_obj->auth_status != EFI_IMAGE_AUTH_PASSED)
> +		return EFI_EXIT(EFI_SECURITY_VIOLATION);
> +
>   	ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image,
>   					 &info, NULL, NULL,
>   					 EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index d5de6df16d84..f6ddddb44cdd 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -10,7 +10,10 @@
>   #include <common.h>
>   #include <cpu_func.h>
>   #include <efi_loader.h>
> +#include <malloc.h>
>   #include <pe.h>
> +#include <sort.h>
> +#include "../lib/crypto/pkcs7_parser.h"
>
>   const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
>   const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID;
> @@ -206,6 +209,367 @@ static void efi_set_code_and_data_type(
>   	}
>   }
>
> +#ifdef CONFIG_EFI_SECURE_BOOT
> +/**
> + * cmp_pe_section - compare two sections
> + * @arg1:	Pointer to first section
> + * @arg2:	Pointer to second section
> + *
> + * Compare two sections in PE image.
> + *
> + * Return:	-1, 0, 1 respectively if arg1 < arg2, arg1 == arg2 or
> +		arg1 > arg2

make htmldocs creates a warning
./lib/efi_loader/efi_image_loader.c:222: warning: bad line:
    arg1 > arg2

All warnings in 'make htmldocs' will be treated as errors in an upcoming
'make htmldocs' build step for Travis CI.

> + */
> +static int cmp_pe_section(const void *arg1, const void *arg2)
> +{
> +	const IMAGE_SECTION_HEADER *section1 = arg1, *section2 = arg2;
> +
> +	if (section1->VirtualAddress < section2->VirtualAddress)
> +		return -1;
> +	else if (section1->VirtualAddress == section2->VirtualAddress)
> +		return 0;
> +	else
> +		return 1;
> +}
> +
> +/**
> + * efi_image_parse - parse a PE image
> + * @efi:	Pointer to image
> + * @len:	Size of @efi
> + * @regs:	Pointer to a list of regions
> + * @auth:	Pointer to a pointer to authentication data in PE
> + * @auth_len:	Size of @auth

./lib/efi_loader/efi_image_loader.c:253: warning: Function parameter or
member 'regp' not described in 'efi_image_parse'

> + *
> + * Parse image binary in PE32(+) format, assuming that sanity of PE image
> + * has been checked by a caller.
> + * On success, an address of authentication data in @efi and its size will
> + * be returned in @auth and @auth_len, respectively.
> + *
> + * Return:	true on success, false on error
> + */
> +bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> +		     WIN_CERTIFICATE **auth, size_t *auth_len)
> +{
> +	struct efi_image_regions *regs;
> +	IMAGE_DOS_HEADER *dos;
> +	IMAGE_NT_HEADERS32 *nt;
> +	IMAGE_SECTION_HEADER *sections, **sorted;
> +	int num_regions, num_sections, i;
> +	int ctidx = IMAGE_DIRECTORY_ENTRY_SECURITY;
> +	u32 align, size, authsz, authoff;
> +	size_t bytes_hashed;
> +
> +	dos = (void *)efi;
> +	nt = (void *)(efi + dos->e_lfanew);
> +
> +	/*
> +	 * Count maximum number of regions to be digested.
> +	 * We don't have to have an exact number here.
> +	 * See efi_image_region_add()'s in parsing below.
> +	 */
> +	num_regions = 3; /* for header */
> +	num_regions += nt->FileHeader.NumberOfSections;
> +	num_regions++; /* for extra */
> +
> +	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
> +		      1);
> +	if (!regs)
> +		goto err;
> +	regs->max = num_regions;
> +
> +	/*
> +	 * Collect data regions for hash calculation
> +	 * 1. File headers
> +	 */
> +	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> +		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
> +		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
> +
> +		/* Skip CheckSum */
> +		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
> +		if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
> +			efi_image_region_add(regs,
> +					     &opt->CheckSum + 1,
> +					     efi + opt->SizeOfHeaders, 0);
> +		} else {
> +			/* Skip Certificates Table */
> +			efi_image_region_add(regs,
> +					     &opt->CheckSum + 1,
> +					     &opt->DataDirectory[ctidx], 0);
> +			efi_image_region_add(regs,
> +					     &opt->DataDirectory[ctidx] + 1,
> +					     efi + opt->SizeOfHeaders, 0);
> +		}
> +
> +		bytes_hashed = opt->SizeOfHeaders;
> +		align = opt->FileAlignment;
> +		authoff = opt->DataDirectory[ctidx].VirtualAddress;
> +		authsz = opt->DataDirectory[ctidx].Size;
> +	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
> +
> +		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
> +		efi_image_region_add(regs, &opt->CheckSum + 1,
> +				     &opt->DataDirectory[ctidx], 0);
> +		efi_image_region_add(regs, &opt->DataDirectory[ctidx] + 1,
> +				     efi + opt->SizeOfHeaders, 0);
> +
> +		bytes_hashed = opt->SizeOfHeaders;
> +		align = opt->FileAlignment;
> +		authoff = opt->DataDirectory[ctidx].VirtualAddress;
> +		authsz = opt->DataDirectory[ctidx].Size;
> +	} else {
> +		debug("%s: Invalid optional header magic %x\n", __func__,
> +		      nt->OptionalHeader.Magic);
> +		goto err;
> +	}
> +
> +	/* 2. Sections */
> +	num_sections = nt->FileHeader.NumberOfSections;
> +	sections = (void *)((uint8_t *)&nt->OptionalHeader +
> +			    nt->FileHeader.SizeOfOptionalHeader);
> +	sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections);
> +	if (!sorted) {
> +		debug("%s: Out of memory\n", __func__);
> +		goto err;
> +	}
> +
> +	/*
> +	 * Make sure the section list is in ascending order.
> +	 */
> +	for (i = 0; i < num_sections; i++)
> +		sorted[i] = &sections[i];
> +	qsort(sorted, num_sections, sizeof(&sections[0]), cmp_pe_section);
> +
> +	for (i = 0; i < num_sections; i++) {
> +		if (!sorted[i]->SizeOfRawData)
> +			continue;
> +
> +		size = (sorted[i]->SizeOfRawData + align - 1) & ~(align - 1);
> +		efi_image_region_add(regs, efi + sorted[i]->PointerToRawData,
> +				     efi + sorted[i]->PointerToRawData + size,
> +				     0);
> +		debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
> +		      i, sorted[i]->Name,
> +		      sorted[i]->PointerToRawData,
> +		      sorted[i]->PointerToRawData + size,
> +		      sorted[i]->VirtualAddress,
> +		      sorted[i]->VirtualAddress
> +			+ sorted[i]->Misc.VirtualSize);
> +
> +		bytes_hashed += size;
> +	}
> +	free(sorted);
> +
> +	/* 3. Extra data excluding Certificates Table */
> +	if (bytes_hashed + authsz < len) {
> +		debug("extra data for hash: %lu\n",
> +		      len - (bytes_hashed + authsz));
> +		efi_image_region_add(regs, efi + bytes_hashed,
> +				     efi + len - authsz, 0);
> +	}
> +
> +	/* Return Certificates Table */
> +	if (authsz) {
> +		if (len < authoff + authsz) {
> +			debug("%s: Size for auth too large: %u >= %zu\n",
> +			      __func__, authsz, len - authoff);
> +			goto err;
> +		}
> +		if (authsz < sizeof(*auth)) {
> +			debug("%s: Size for auth too small: %u < %zu\n",
> +			      __func__, authsz, sizeof(*auth));
> +			goto err;
> +		}
> +		*auth = efi + authoff;
> +		*auth_len = authsz;
> +		debug("WIN_CERTIFICATE: 0x%x, size: 0x%x\n", authoff, authsz);
> +	} else {
> +		*auth = NULL;
> +		*auth_len = 0;
> +	}
> +
> +	*regp = regs;
> +
> +	return true;
> +
> +err:
> +	free(regs);
> +
> +	return false;
> +}
> +
> +/**
> + * efi_image_unsigned_authenticate - authenticate unsigned image with
> + * SHA256 hash
> + * @regs:	List of regions to be verified
> + *
> + * If an image is not signed, it doesn't have a signature. In this case,
> + * its message digest is calculated and it will be compared with one of
> + * hash values stored in signature databases.
> + *
> + * Return:	true if authenticated, false if not
> + */
> +static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs)
> +{
> +	struct efi_signature_store *db = NULL, *dbx = NULL;
> +	bool ret = false;
> +
> +	dbx = efi_sigstore_parse_sigdb(L"dbx");
> +	if (!dbx) {
> +		debug("Getting signature database(dbx) failed\n");
> +		goto out;
> +	}
> +
> +	db = efi_sigstore_parse_sigdb(L"db");
> +	if (!db) {
> +		debug("Getting signature database(db) failed\n");
> +		goto out;
> +	}
> +
> +	/* try black-list first */
> +	if (efi_signature_verify_with_sigdb(regs, NULL, dbx, NULL)) {
> +		debug("Image is not signed and rejected by \"dbx\"\n");
> +		goto out;
> +	}
> +
> +	/* try white-list */
> +	if (efi_signature_verify_with_sigdb(regs, NULL, db, NULL))
> +		ret = true;
> +	else
> +		debug("Image is not signed and not found in \"db\" or \"dbx\"\n");
> +
> +out:
> +	efi_sigstore_free(db);
> +	efi_sigstore_free(dbx);
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_image_authenticate - verify a signature of signed image
> + * @efi:	Pointer to image
> + * @len:	Size of @efi
> + *
> + * A signed image should have its signature stored in a table of its PE header.
> + * So if an image is signed and only if if its signature is verified using
> + * signature databases, an image is authenticated.
> + * If an image is not signed, its validity is checked by using
> + * efi_image_unsigned_authenticated().
> + * TODO:
> + * When AuditMode==0, if the image's signature is not found in
> + * the authorized database, or is found in the forbidden database,
> + * the image will not be started and instead, information about it
> + * will be placed in this table.
> + * When AuditMode==1, an EFI_IMAGE_EXECUTION_INFO element is created
> + * in the EFI_IMAGE_EXECUTION_INFO_TABLE for every certificate found
> + * in the certificate table of every image that is validated.
> + *
> + * Return:	true if authenticated, false if not
> + */
> +static bool efi_image_authenticate(void *efi, size_t len)
> +{
> +	struct efi_image_regions *regs = NULL;
> +	WIN_CERTIFICATE *wincerts = NULL, *wincert;
> +	size_t wincerts_len;
> +	struct pkcs7_message *msg = NULL;
> +	struct efi_signature_store *db = NULL, *dbx = NULL;
> +	struct x509_certificate *cert = NULL;
> +	bool ret = false;
> +
> +	if (!efi_secure_boot_enabled())
> +		return true;
> +
> +	if (!efi_image_parse(efi, len, &regs, &wincerts,
> +			     &wincerts_len)) {
> +		debug("Parsing PE executable image failed\n");
> +		return false;
> +	}
> +
> +	if (!wincerts) {
> +		/* The image is not signed */
> +		ret = efi_image_unsigned_authenticate(regs);
> +		free(regs);
> +
> +		return ret;
> +	}
> +
> +	/*
> +	 * verify signature using db and dbx
> +	 */
> +	db = efi_sigstore_parse_sigdb(L"db");
> +	if (!db) {
> +		debug("Getting signature database(db) failed\n");
> +		goto err;
> +	}
> +
> +	dbx = efi_sigstore_parse_sigdb(L"dbx");
> +	if (!dbx) {
> +		debug("Getting signature database(dbx) failed\n");
> +		goto err;
> +	}
> +
> +	/* go through WIN_CERTIFICATE list */
> +	for (wincert = wincerts;
> +	     (void *)wincert < (void *)wincerts + wincerts_len;
> +	     wincert = (void *)wincert + ALIGN(wincert->dwLength, 8)) {
> +		if (wincert->dwLength < sizeof(*wincert)) {
> +			debug("%s: dwLength too small: %u < %zu\n",
> +			      __func__, wincert->dwLength, sizeof(*wincert));
> +			goto err;
> +		}
> +		msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
> +					  wincert->dwLength - sizeof(*wincert));
> +		if (!msg) {
> +			debug("Parsing image's signature failed\n");
> +			goto err;
> +		}
> +
> +		/* try black-list first */
> +		if (efi_signature_verify_with_sigdb(regs, msg, dbx, NULL)) {
> +			debug("Signature was rejected by \"dbx\"\n");
> +			goto err;
> +		}
> +
> +		if (!efi_signature_verify_signers(msg, dbx)) {
> +			debug("Signer was rejected by \"dbx\"\n");
> +			goto err;
> +		} else {
> +			ret = true;
> +		}
> +
> +		/* try white-list */
> +		if (!efi_signature_verify_with_sigdb(regs, msg, db, &cert)) {
> +			debug("Verifying signature with \"db\" failed\n");
> +			goto err;
> +		} else {
> +			ret = true;
> +		}
> +
> +		if (!efi_signature_verify_cert(cert, dbx)) {
> +			debug("Certificate was rejected by \"dbx\"\n");
> +			goto err;
> +		} else {
> +			ret = true;
> +		}
> +	}
> +
> +err:
> +	x509_free_certificate(cert);
> +	efi_sigstore_free(db);
> +	efi_sigstore_free(dbx);
> +	pkcs7_free_message(msg);
> +	free(regs);
> +
> +	return ret;
> +}
> +#else
> +static bool efi_image_authenticate(void *efi, size_t len)
> +{
> +	return true;
> +}
> +#endif /* CONFIG_EFI_SECURE_BOOT */
> +
>   /**
>    * efi_load_pe() - relocate EFI binary
>    *
> @@ -217,7 +581,8 @@ static void efi_set_code_and_data_type(
>    * @loaded_image_info:	loaded image protocol
>    * Return:		status code
>    */

./lib/efi_loader/efi_image_loader.c:588: warning: Function parameter or
member 'efi_size' not described in 'efi_load_pe'

Best regards

Heinrich

> -efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> +efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> +			 void *efi, size_t efi_size,
>   			 struct efi_loaded_image *loaded_image_info)
>   {
>   	IMAGE_NT_HEADERS32 *nt;
> @@ -232,17 +597,57 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   	uint64_t image_base;
>   	unsigned long virt_size = 0;
>   	int supported = 0;
> +	void *new_efi = NULL;
> +	size_t new_efi_size;
> +	efi_status_t ret;
> +
> +	/*
> +	 * Size must be 8-byte aligned and the trailing bytes must be
> +	 * zero'ed. Otherwise hash value may be incorrect.
> +	 */
> +	if (efi_size & 0x7) {
> +		new_efi_size = (efi_size + 0x7) & ~0x7ULL;
> +		new_efi = calloc(new_efi_size, 1);
> +		if (!new_efi)
> +			return EFI_OUT_OF_RESOURCES;
> +		memcpy(new_efi, efi, efi_size);
> +		efi = new_efi;
> +		efi_size = new_efi_size;
> +	}
> +
> +	/* Sanity check for a file header */
> +	if (efi_size < sizeof(*dos)) {
> +		printf("%s: Truncated DOS Header\n", __func__);
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
> +	}
>
>   	dos = efi;
>   	if (dos->e_magic != IMAGE_DOS_SIGNATURE) {
>   		printf("%s: Invalid DOS Signature\n", __func__);
> -		return EFI_LOAD_ERROR;
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
> +	}
> +
> +	/* assume sizeof(IMAGE_NT_HEADERS32) <= sizeof(IMAGE_NT_HEADERS64) */
> +	if (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32)) {
> +		printf("%s: Invalid offset for Extended Header\n", __func__);
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
>   	}
>
>   	nt = (void *) ((char *)efi + dos->e_lfanew);
> +	if ((nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
> +	    (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS64))) {
> +		printf("%s: Invalid offset for Extended Header\n", __func__);
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
> +	}
> +
>   	if (nt->Signature != IMAGE_NT_SIGNATURE) {
>   		printf("%s: Invalid NT Signature\n", __func__);
> -		return EFI_LOAD_ERROR;
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
>   	}
>
>   	for (i = 0; machines[i]; i++)
> @@ -254,14 +659,29 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   	if (!supported) {
>   		printf("%s: Machine type 0x%04x is not supported\n",
>   		       __func__, nt->FileHeader.Machine);
> -		return EFI_LOAD_ERROR;
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
>   	}
>
> -	/* Calculate upper virtual address boundary */
>   	num_sections = nt->FileHeader.NumberOfSections;
>   	sections = (void *)&nt->OptionalHeader +
>   			    nt->FileHeader.SizeOfOptionalHeader;
>
> +	if (efi_size < ((void *)sections + sizeof(sections[0]) * num_sections
> +			- efi)) {
> +		printf("%s: Invalid number of sections: %d\n",
> +		       __func__, num_sections);
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
> +	}
> +
> +	/* Authenticate an image */
> +	if (efi_image_authenticate(efi, efi_size))
> +		handle->auth_status = EFI_IMAGE_AUTH_PASSED;
> +	else
> +		handle->auth_status = EFI_IMAGE_AUTH_FAILED;
> +
> +	/* Calculate upper virtual address boundary */
>   	for (i = num_sections - 1; i >= 0; i--) {
>   		IMAGE_SECTION_HEADER *sec = &sections[i];
>   		virt_size = max_t(unsigned long, virt_size,
> @@ -280,7 +700,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   		if (!efi_reloc) {
>   			printf("%s: Could not allocate %lu bytes\n",
>   			       __func__, virt_size);
> -			return EFI_OUT_OF_RESOURCES;
> +			ret = EFI_OUT_OF_RESOURCES;
> +			goto err;
>   		}
>   		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
>   		rel_size = opt->DataDirectory[rel_idx].Size;
> @@ -296,7 +717,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   		if (!efi_reloc) {
>   			printf("%s: Could not allocate %lu bytes\n",
>   			       __func__, virt_size);
> -			return EFI_OUT_OF_RESOURCES;
> +			ret = EFI_OUT_OF_RESOURCES;
> +			goto err;
>   		}
>   		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
>   		rel_size = opt->DataDirectory[rel_idx].Size;
> @@ -305,13 +727,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   	} else {
>   		printf("%s: Invalid optional header magic %x\n", __func__,
>   		       nt->OptionalHeader.Magic);
> -		return EFI_LOAD_ERROR;
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
>   	}
>
>   	/* Copy PE headers */
> -	memcpy(efi_reloc, efi, sizeof(*dos) + sizeof(*nt)
> -	       + nt->FileHeader.SizeOfOptionalHeader
> -	       + num_sections * sizeof(IMAGE_SECTION_HEADER));
> +	memcpy(efi_reloc, efi,
> +	       sizeof(*dos)
> +		 + sizeof(*nt)
> +		 + nt->FileHeader.SizeOfOptionalHeader
> +		 + num_sections * sizeof(IMAGE_SECTION_HEADER));
>
>   	/* Load sections into RAM */
>   	for (i = num_sections - 1; i >= 0; i--) {
> @@ -328,7 +753,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   				(unsigned long)image_base) != EFI_SUCCESS) {
>   		efi_free_pages((uintptr_t) efi_reloc,
>   			       (virt_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT);
> -		return EFI_LOAD_ERROR;
> +		ret = EFI_LOAD_ERROR;
> +		goto err;
>   	}
>
>   	/* Flush cache */
> @@ -340,5 +766,13 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>   	loaded_image_info->image_base = efi_reloc;
>   	loaded_image_info->image_size = virt_size;
>
> -	return EFI_SUCCESS;
> +	if (handle->auth_status == EFI_IMAGE_AUTH_PASSED)
> +		return EFI_SUCCESS;
> +	else
> +		return EFI_SECURITY_VIOLATION;
> +
> +err:
> +	free(new_efi);
> +
> +	return ret;
>   }
>
Takahiro Akashi Feb. 25, 2020, 6:57 a.m. UTC | #5
On Tue, Feb 25, 2020 at 07:40:01AM +0100, Heinrich Schuchardt wrote:
> On 2/25/20 6:25 AM, AKASHI Takahiro wrote:
> > On Mon, Feb 24, 2020 at 07:29:17PM +0100, Heinrich Schuchardt wrote:
> > > On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> > > > With this commit, image validation can be enforced, as UEFI specification
> > > > section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
> > > > 
> > > > Currently we support
> > > > * authentication based on db and dbx,
> > > >     so dbx-validated image will always be rejected.
> > > > * following signature types:
> > > >       EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
> > > >       EFI_CERT_X509_GUID (x509 certificate for signed images)
> > > > Timestamp-based certificate revocation is not supported here.
> > > > 
> > > > Internally, authentication data is stored in one of certificates tables
> > > > of PE image (See efi_image_parse()) and will be verified by
> > > > efi_image_authenticate() before loading a given image.
> > > > 
> > > > It seems that UEFI specification defines the verification process
> > > > in a bit ambiguous way. I tried to implement it as closely to as
> > > > EDK2 does.
> > > > 
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > 
> > > According to git bisect this patch breaks the test
> > > test/py/tests/test_efi_fit.py.
> > 
> > This error only occurs on "compressed" FIT images. While I'm not sure
> > whether it is directly related to efi support in bootm or not, I've
> > fixed it any way.
> 
> Hello Takahiro,
> 
> where can I find the fix?

Only in my local repository.
Since I'm running Travis CI now, I will post a new version
once the test is completed AND if you have no more comments
on my v5.

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > 
> > Thanks,
> > -Takahiro Akashi
> > 
> > 
> > > Best regards
> > > 
> > > Heinrich
>
Takahiro Akashi Feb. 26, 2020, 12:50 a.m. UTC | #6
On Tue, Feb 25, 2020 at 07:44:10AM +0100, Heinrich Schuchardt wrote:
> On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> > With this commit, image validation can be enforced, as UEFI specification
> > section 32.5 describes, if CONFIG_EFI_SECURE_BOOT is enabled.
> > 
> > Currently we support
> > * authentication based on db and dbx,
> >    so dbx-validated image will always be rejected.
> > * following signature types:
> >      EFI_CERT_SHA256_GUID (SHA256 digest for unsigned images)
> >      EFI_CERT_X509_GUID (x509 certificate for signed images)
> > Timestamp-based certificate revocation is not supported here.
> > 
> > Internally, authentication data is stored in one of certificates tables
> > of PE image (See efi_image_parse()) and will be verified by
> > efi_image_authenticate() before loading a given image.
> > 
> > It seems that UEFI specification defines the verification process
> > in a bit ambiguous way. I tried to implement it as closely to as
> > EDK2 does.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   include/efi_loader.h              |  13 +-
> >   lib/efi_loader/efi_boottime.c     |  10 +-
> >   lib/efi_loader/efi_image_loader.c | 460 +++++++++++++++++++++++++++++-
> >   3 files changed, 467 insertions(+), 16 deletions(-)
> > 
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index f461c6195834..0e15470d9c17 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -11,6 +11,7 @@
> >   #include <common.h>
> >   #include <part_efi.h>
> >   #include <efi_api.h>
> > +#include <pe.h>
> > 
> >   static inline int guidcmp(const void *g1, const void *g2)
> >   {
> > @@ -263,6 +264,11 @@ struct efi_object {
> >   	enum efi_object_type type;
> >   };
> > 
> > +enum efi_image_auth_status {
> > +	EFI_IMAGE_AUTH_FAILED = 0,
> > +	EFI_IMAGE_AUTH_PASSED,
> > +};
> > +
> >   /**
> >    * struct efi_loaded_image_obj - handle of a loaded image
> >    *
> > @@ -282,6 +288,7 @@ struct efi_loaded_image_obj {
> >   	EFIAPI efi_status_t (*entry)(efi_handle_t image_handle,
> >   				     struct efi_system_table *st);
> >   	u16 image_type;
> > +	enum efi_image_auth_status auth_status;
> >   };
> > 
> >   /**
> > @@ -414,7 +421,8 @@ efi_status_t efi_set_watchdog(unsigned long timeout);
> >   /* Called from places to check whether a timer expired */
> >   void efi_timer_check(void);
> >   /* PE loader implementation */
> > -efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> > +efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> > +			 void *efi, size_t efi_size,
> >   			 struct efi_loaded_image *loaded_image_info);
> >   /* Called once to store the pristine gd pointer */
> >   void efi_save_gd(void);
> > @@ -741,6 +749,9 @@ void efi_sigstore_free(struct efi_signature_store *sigstore);
> >   struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
> > 
> >   bool efi_secure_boot_enabled(void);
> > +
> > +bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> > +		     WIN_CERTIFICATE **auth, size_t *auth_len);
> >   #endif /* CONFIG_EFI_SECURE_BOOT */
> > 
> >   #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index 1f598b357a5c..cc8cc4cb5408 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -1882,12 +1882,12 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
> >   	efi_dp_split_file_path(file_path, &dp, &fp);
> >   	ret = efi_setup_loaded_image(dp, fp, image_obj, &info);
> >   	if (ret == EFI_SUCCESS)
> > -		ret = efi_load_pe(*image_obj, dest_buffer, info);
> > +		ret = efi_load_pe(*image_obj, dest_buffer, source_size, info);
> >   	if (!source_buffer)
> >   		/* Release buffer to which file was loaded */
> >   		efi_free_pages((uintptr_t)dest_buffer,
> >   			       efi_size_in_pages(source_size));
> > -	if (ret == EFI_SUCCESS) {
> > +	if (ret == EFI_SUCCESS || ret == EFI_SECURITY_VIOLATION) {
> >   		info->system_table = &systab;
> >   		info->parent_handle = parent_image;
> >   	} else {
> > @@ -2885,10 +2885,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> > 
> >   	EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data);
> > 
> > +	if (!efi_search_obj(image_handle))
> > +		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +
> >   	/* Check parameters */
> >   	if (image_obj->header.type != EFI_OBJECT_TYPE_LOADED_IMAGE)
> >   		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > 
> > +	if (image_obj->auth_status != EFI_IMAGE_AUTH_PASSED)
> > +		return EFI_EXIT(EFI_SECURITY_VIOLATION);
> > +
> >   	ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image,
> >   					 &info, NULL, NULL,
> >   					 EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > index d5de6df16d84..f6ddddb44cdd 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -10,7 +10,10 @@
> >   #include <common.h>
> >   #include <cpu_func.h>
> >   #include <efi_loader.h>
> > +#include <malloc.h>
> >   #include <pe.h>
> > +#include <sort.h>
> > +#include "../lib/crypto/pkcs7_parser.h"
> > 
> >   const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> >   const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID;
> > @@ -206,6 +209,367 @@ static void efi_set_code_and_data_type(
> >   	}
> >   }
> > 
> > +#ifdef CONFIG_EFI_SECURE_BOOT
> > +/**
> > + * cmp_pe_section - compare two sections
> > + * @arg1:	Pointer to first section
> > + * @arg2:	Pointer to second section
> > + *
> > + * Compare two sections in PE image.
> > + *
> > + * Return:	-1, 0, 1 respectively if arg1 < arg2, arg1 == arg2 or
> > +		arg1 > arg2
> 
> make htmldocs creates a warning
> ./lib/efi_loader/efi_image_loader.c:222: warning: bad line:
>    arg1 > arg2
> 
> All warnings in 'make htmldocs' will be treated as errors in an upcoming
> 'make htmldocs' build step for Travis CI.

Okay, I fixed all of them in efi_image_loader.c.

Thanks,
-Takahiro Akashi

> > + */
> > +static int cmp_pe_section(const void *arg1, const void *arg2)
> > +{
> > +	const IMAGE_SECTION_HEADER *section1 = arg1, *section2 = arg2;
> > +
> > +	if (section1->VirtualAddress < section2->VirtualAddress)
> > +		return -1;
> > +	else if (section1->VirtualAddress == section2->VirtualAddress)
> > +		return 0;
> > +	else
> > +		return 1;
> > +}
> > +
> > +/**
> > + * efi_image_parse - parse a PE image
> > + * @efi:	Pointer to image
> > + * @len:	Size of @efi
> > + * @regs:	Pointer to a list of regions
> > + * @auth:	Pointer to a pointer to authentication data in PE
> > + * @auth_len:	Size of @auth
> 
> ./lib/efi_loader/efi_image_loader.c:253: warning: Function parameter or
> member 'regp' not described in 'efi_image_parse'
> 
> > + *
> > + * Parse image binary in PE32(+) format, assuming that sanity of PE image
> > + * has been checked by a caller.
> > + * On success, an address of authentication data in @efi and its size will
> > + * be returned in @auth and @auth_len, respectively.
> > + *
> > + * Return:	true on success, false on error
> > + */
> > +bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
> > +		     WIN_CERTIFICATE **auth, size_t *auth_len)
> > +{
> > +	struct efi_image_regions *regs;
> > +	IMAGE_DOS_HEADER *dos;
> > +	IMAGE_NT_HEADERS32 *nt;
> > +	IMAGE_SECTION_HEADER *sections, **sorted;
> > +	int num_regions, num_sections, i;
> > +	int ctidx = IMAGE_DIRECTORY_ENTRY_SECURITY;
> > +	u32 align, size, authsz, authoff;
> > +	size_t bytes_hashed;
> > +
> > +	dos = (void *)efi;
> > +	nt = (void *)(efi + dos->e_lfanew);
> > +
> > +	/*
> > +	 * Count maximum number of regions to be digested.
> > +	 * We don't have to have an exact number here.
> > +	 * See efi_image_region_add()'s in parsing below.
> > +	 */
> > +	num_regions = 3; /* for header */
> > +	num_regions += nt->FileHeader.NumberOfSections;
> > +	num_regions++; /* for extra */
> > +
> > +	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
> > +		      1);
> > +	if (!regs)
> > +		goto err;
> > +	regs->max = num_regions;
> > +
> > +	/*
> > +	 * Collect data regions for hash calculation
> > +	 * 1. File headers
> > +	 */
> > +	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> > +		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
> > +		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
> > +
> > +		/* Skip CheckSum */
> > +		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
> > +		if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
> > +			efi_image_region_add(regs,
> > +					     &opt->CheckSum + 1,
> > +					     efi + opt->SizeOfHeaders, 0);
> > +		} else {
> > +			/* Skip Certificates Table */
> > +			efi_image_region_add(regs,
> > +					     &opt->CheckSum + 1,
> > +					     &opt->DataDirectory[ctidx], 0);
> > +			efi_image_region_add(regs,
> > +					     &opt->DataDirectory[ctidx] + 1,
> > +					     efi + opt->SizeOfHeaders, 0);
> > +		}
> > +
> > +		bytes_hashed = opt->SizeOfHeaders;
> > +		align = opt->FileAlignment;
> > +		authoff = opt->DataDirectory[ctidx].VirtualAddress;
> > +		authsz = opt->DataDirectory[ctidx].Size;
> > +	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> > +		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
> > +
> > +		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
> > +		efi_image_region_add(regs, &opt->CheckSum + 1,
> > +				     &opt->DataDirectory[ctidx], 0);
> > +		efi_image_region_add(regs, &opt->DataDirectory[ctidx] + 1,
> > +				     efi + opt->SizeOfHeaders, 0);
> > +
> > +		bytes_hashed = opt->SizeOfHeaders;
> > +		align = opt->FileAlignment;
> > +		authoff = opt->DataDirectory[ctidx].VirtualAddress;
> > +		authsz = opt->DataDirectory[ctidx].Size;
> > +	} else {
> > +		debug("%s: Invalid optional header magic %x\n", __func__,
> > +		      nt->OptionalHeader.Magic);
> > +		goto err;
> > +	}
> > +
> > +	/* 2. Sections */
> > +	num_sections = nt->FileHeader.NumberOfSections;
> > +	sections = (void *)((uint8_t *)&nt->OptionalHeader +
> > +			    nt->FileHeader.SizeOfOptionalHeader);
> > +	sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections);
> > +	if (!sorted) {
> > +		debug("%s: Out of memory\n", __func__);
> > +		goto err;
> > +	}
> > +
> > +	/*
> > +	 * Make sure the section list is in ascending order.
> > +	 */
> > +	for (i = 0; i < num_sections; i++)
> > +		sorted[i] = &sections[i];
> > +	qsort(sorted, num_sections, sizeof(&sections[0]), cmp_pe_section);
> > +
> > +	for (i = 0; i < num_sections; i++) {
> > +		if (!sorted[i]->SizeOfRawData)
> > +			continue;
> > +
> > +		size = (sorted[i]->SizeOfRawData + align - 1) & ~(align - 1);
> > +		efi_image_region_add(regs, efi + sorted[i]->PointerToRawData,
> > +				     efi + sorted[i]->PointerToRawData + size,
> > +				     0);
> > +		debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
> > +		      i, sorted[i]->Name,
> > +		      sorted[i]->PointerToRawData,
> > +		      sorted[i]->PointerToRawData + size,
> > +		      sorted[i]->VirtualAddress,
> > +		      sorted[i]->VirtualAddress
> > +			+ sorted[i]->Misc.VirtualSize);
> > +
> > +		bytes_hashed += size;
> > +	}
> > +	free(sorted);
> > +
> > +	/* 3. Extra data excluding Certificates Table */
> > +	if (bytes_hashed + authsz < len) {
> > +		debug("extra data for hash: %lu\n",
> > +		      len - (bytes_hashed + authsz));
> > +		efi_image_region_add(regs, efi + bytes_hashed,
> > +				     efi + len - authsz, 0);
> > +	}
> > +
> > +	/* Return Certificates Table */
> > +	if (authsz) {
> > +		if (len < authoff + authsz) {
> > +			debug("%s: Size for auth too large: %u >= %zu\n",
> > +			      __func__, authsz, len - authoff);
> > +			goto err;
> > +		}
> > +		if (authsz < sizeof(*auth)) {
> > +			debug("%s: Size for auth too small: %u < %zu\n",
> > +			      __func__, authsz, sizeof(*auth));
> > +			goto err;
> > +		}
> > +		*auth = efi + authoff;
> > +		*auth_len = authsz;
> > +		debug("WIN_CERTIFICATE: 0x%x, size: 0x%x\n", authoff, authsz);
> > +	} else {
> > +		*auth = NULL;
> > +		*auth_len = 0;
> > +	}
> > +
> > +	*regp = regs;
> > +
> > +	return true;
> > +
> > +err:
> > +	free(regs);
> > +
> > +	return false;
> > +}
> > +
> > +/**
> > + * efi_image_unsigned_authenticate - authenticate unsigned image with
> > + * SHA256 hash
> > + * @regs:	List of regions to be verified
> > + *
> > + * If an image is not signed, it doesn't have a signature. In this case,
> > + * its message digest is calculated and it will be compared with one of
> > + * hash values stored in signature databases.
> > + *
> > + * Return:	true if authenticated, false if not
> > + */
> > +static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs)
> > +{
> > +	struct efi_signature_store *db = NULL, *dbx = NULL;
> > +	bool ret = false;
> > +
> > +	dbx = efi_sigstore_parse_sigdb(L"dbx");
> > +	if (!dbx) {
> > +		debug("Getting signature database(dbx) failed\n");
> > +		goto out;
> > +	}
> > +
> > +	db = efi_sigstore_parse_sigdb(L"db");
> > +	if (!db) {
> > +		debug("Getting signature database(db) failed\n");
> > +		goto out;
> > +	}
> > +
> > +	/* try black-list first */
> > +	if (efi_signature_verify_with_sigdb(regs, NULL, dbx, NULL)) {
> > +		debug("Image is not signed and rejected by \"dbx\"\n");
> > +		goto out;
> > +	}
> > +
> > +	/* try white-list */
> > +	if (efi_signature_verify_with_sigdb(regs, NULL, db, NULL))
> > +		ret = true;
> > +	else
> > +		debug("Image is not signed and not found in \"db\" or \"dbx\"\n");
> > +
> > +out:
> > +	efi_sigstore_free(db);
> > +	efi_sigstore_free(dbx);
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * efi_image_authenticate - verify a signature of signed image
> > + * @efi:	Pointer to image
> > + * @len:	Size of @efi
> > + *
> > + * A signed image should have its signature stored in a table of its PE header.
> > + * So if an image is signed and only if if its signature is verified using
> > + * signature databases, an image is authenticated.
> > + * If an image is not signed, its validity is checked by using
> > + * efi_image_unsigned_authenticated().
> > + * TODO:
> > + * When AuditMode==0, if the image's signature is not found in
> > + * the authorized database, or is found in the forbidden database,
> > + * the image will not be started and instead, information about it
> > + * will be placed in this table.
> > + * When AuditMode==1, an EFI_IMAGE_EXECUTION_INFO element is created
> > + * in the EFI_IMAGE_EXECUTION_INFO_TABLE for every certificate found
> > + * in the certificate table of every image that is validated.
> > + *
> > + * Return:	true if authenticated, false if not
> > + */
> > +static bool efi_image_authenticate(void *efi, size_t len)
> > +{
> > +	struct efi_image_regions *regs = NULL;
> > +	WIN_CERTIFICATE *wincerts = NULL, *wincert;
> > +	size_t wincerts_len;
> > +	struct pkcs7_message *msg = NULL;
> > +	struct efi_signature_store *db = NULL, *dbx = NULL;
> > +	struct x509_certificate *cert = NULL;
> > +	bool ret = false;
> > +
> > +	if (!efi_secure_boot_enabled())
> > +		return true;
> > +
> > +	if (!efi_image_parse(efi, len, &regs, &wincerts,
> > +			     &wincerts_len)) {
> > +		debug("Parsing PE executable image failed\n");
> > +		return false;
> > +	}
> > +
> > +	if (!wincerts) {
> > +		/* The image is not signed */
> > +		ret = efi_image_unsigned_authenticate(regs);
> > +		free(regs);
> > +
> > +		return ret;
> > +	}
> > +
> > +	/*
> > +	 * verify signature using db and dbx
> > +	 */
> > +	db = efi_sigstore_parse_sigdb(L"db");
> > +	if (!db) {
> > +		debug("Getting signature database(db) failed\n");
> > +		goto err;
> > +	}
> > +
> > +	dbx = efi_sigstore_parse_sigdb(L"dbx");
> > +	if (!dbx) {
> > +		debug("Getting signature database(dbx) failed\n");
> > +		goto err;
> > +	}
> > +
> > +	/* go through WIN_CERTIFICATE list */
> > +	for (wincert = wincerts;
> > +	     (void *)wincert < (void *)wincerts + wincerts_len;
> > +	     wincert = (void *)wincert + ALIGN(wincert->dwLength, 8)) {
> > +		if (wincert->dwLength < sizeof(*wincert)) {
> > +			debug("%s: dwLength too small: %u < %zu\n",
> > +			      __func__, wincert->dwLength, sizeof(*wincert));
> > +			goto err;
> > +		}
> > +		msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
> > +					  wincert->dwLength - sizeof(*wincert));
> > +		if (!msg) {
> > +			debug("Parsing image's signature failed\n");
> > +			goto err;
> > +		}
> > +
> > +		/* try black-list first */
> > +		if (efi_signature_verify_with_sigdb(regs, msg, dbx, NULL)) {
> > +			debug("Signature was rejected by \"dbx\"\n");
> > +			goto err;
> > +		}
> > +
> > +		if (!efi_signature_verify_signers(msg, dbx)) {
> > +			debug("Signer was rejected by \"dbx\"\n");
> > +			goto err;
> > +		} else {
> > +			ret = true;
> > +		}
> > +
> > +		/* try white-list */
> > +		if (!efi_signature_verify_with_sigdb(regs, msg, db, &cert)) {
> > +			debug("Verifying signature with \"db\" failed\n");
> > +			goto err;
> > +		} else {
> > +			ret = true;
> > +		}
> > +
> > +		if (!efi_signature_verify_cert(cert, dbx)) {
> > +			debug("Certificate was rejected by \"dbx\"\n");
> > +			goto err;
> > +		} else {
> > +			ret = true;
> > +		}
> > +	}
> > +
> > +err:
> > +	x509_free_certificate(cert);
> > +	efi_sigstore_free(db);
> > +	efi_sigstore_free(dbx);
> > +	pkcs7_free_message(msg);
> > +	free(regs);
> > +
> > +	return ret;
> > +}
> > +#else
> > +static bool efi_image_authenticate(void *efi, size_t len)
> > +{
> > +	return true;
> > +}
> > +#endif /* CONFIG_EFI_SECURE_BOOT */
> > +
> >   /**
> >    * efi_load_pe() - relocate EFI binary
> >    *
> > @@ -217,7 +581,8 @@ static void efi_set_code_and_data_type(
> >    * @loaded_image_info:	loaded image protocol
> >    * Return:		status code
> >    */
> 
> ./lib/efi_loader/efi_image_loader.c:588: warning: Function parameter or
> member 'efi_size' not described in 'efi_load_pe'
> 
> Best regards
> 
> Heinrich
> 
> > -efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> > +efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
> > +			 void *efi, size_t efi_size,
> >   			 struct efi_loaded_image *loaded_image_info)
> >   {
> >   	IMAGE_NT_HEADERS32 *nt;
> > @@ -232,17 +597,57 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   	uint64_t image_base;
> >   	unsigned long virt_size = 0;
> >   	int supported = 0;
> > +	void *new_efi = NULL;
> > +	size_t new_efi_size;
> > +	efi_status_t ret;
> > +
> > +	/*
> > +	 * Size must be 8-byte aligned and the trailing bytes must be
> > +	 * zero'ed. Otherwise hash value may be incorrect.
> > +	 */
> > +	if (efi_size & 0x7) {
> > +		new_efi_size = (efi_size + 0x7) & ~0x7ULL;
> > +		new_efi = calloc(new_efi_size, 1);
> > +		if (!new_efi)
> > +			return EFI_OUT_OF_RESOURCES;
> > +		memcpy(new_efi, efi, efi_size);
> > +		efi = new_efi;
> > +		efi_size = new_efi_size;
> > +	}
> > +
> > +	/* Sanity check for a file header */
> > +	if (efi_size < sizeof(*dos)) {
> > +		printf("%s: Truncated DOS Header\n", __func__);
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> > +	}
> > 
> >   	dos = efi;
> >   	if (dos->e_magic != IMAGE_DOS_SIGNATURE) {
> >   		printf("%s: Invalid DOS Signature\n", __func__);
> > -		return EFI_LOAD_ERROR;
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> > +	}
> > +
> > +	/* assume sizeof(IMAGE_NT_HEADERS32) <= sizeof(IMAGE_NT_HEADERS64) */
> > +	if (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32)) {
> > +		printf("%s: Invalid offset for Extended Header\n", __func__);
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> >   	}
> > 
> >   	nt = (void *) ((char *)efi + dos->e_lfanew);
> > +	if ((nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
> > +	    (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS64))) {
> > +		printf("%s: Invalid offset for Extended Header\n", __func__);
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> > +	}
> > +
> >   	if (nt->Signature != IMAGE_NT_SIGNATURE) {
> >   		printf("%s: Invalid NT Signature\n", __func__);
> > -		return EFI_LOAD_ERROR;
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> >   	}
> > 
> >   	for (i = 0; machines[i]; i++)
> > @@ -254,14 +659,29 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   	if (!supported) {
> >   		printf("%s: Machine type 0x%04x is not supported\n",
> >   		       __func__, nt->FileHeader.Machine);
> > -		return EFI_LOAD_ERROR;
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> >   	}
> > 
> > -	/* Calculate upper virtual address boundary */
> >   	num_sections = nt->FileHeader.NumberOfSections;
> >   	sections = (void *)&nt->OptionalHeader +
> >   			    nt->FileHeader.SizeOfOptionalHeader;
> > 
> > +	if (efi_size < ((void *)sections + sizeof(sections[0]) * num_sections
> > +			- efi)) {
> > +		printf("%s: Invalid number of sections: %d\n",
> > +		       __func__, num_sections);
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> > +	}
> > +
> > +	/* Authenticate an image */
> > +	if (efi_image_authenticate(efi, efi_size))
> > +		handle->auth_status = EFI_IMAGE_AUTH_PASSED;
> > +	else
> > +		handle->auth_status = EFI_IMAGE_AUTH_FAILED;
> > +
> > +	/* Calculate upper virtual address boundary */
> >   	for (i = num_sections - 1; i >= 0; i--) {
> >   		IMAGE_SECTION_HEADER *sec = &sections[i];
> >   		virt_size = max_t(unsigned long, virt_size,
> > @@ -280,7 +700,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   		if (!efi_reloc) {
> >   			printf("%s: Could not allocate %lu bytes\n",
> >   			       __func__, virt_size);
> > -			return EFI_OUT_OF_RESOURCES;
> > +			ret = EFI_OUT_OF_RESOURCES;
> > +			goto err;
> >   		}
> >   		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
> >   		rel_size = opt->DataDirectory[rel_idx].Size;
> > @@ -296,7 +717,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   		if (!efi_reloc) {
> >   			printf("%s: Could not allocate %lu bytes\n",
> >   			       __func__, virt_size);
> > -			return EFI_OUT_OF_RESOURCES;
> > +			ret = EFI_OUT_OF_RESOURCES;
> > +			goto err;
> >   		}
> >   		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
> >   		rel_size = opt->DataDirectory[rel_idx].Size;
> > @@ -305,13 +727,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   	} else {
> >   		printf("%s: Invalid optional header magic %x\n", __func__,
> >   		       nt->OptionalHeader.Magic);
> > -		return EFI_LOAD_ERROR;
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> >   	}
> > 
> >   	/* Copy PE headers */
> > -	memcpy(efi_reloc, efi, sizeof(*dos) + sizeof(*nt)
> > -	       + nt->FileHeader.SizeOfOptionalHeader
> > -	       + num_sections * sizeof(IMAGE_SECTION_HEADER));
> > +	memcpy(efi_reloc, efi,
> > +	       sizeof(*dos)
> > +		 + sizeof(*nt)
> > +		 + nt->FileHeader.SizeOfOptionalHeader
> > +		 + num_sections * sizeof(IMAGE_SECTION_HEADER));
> > 
> >   	/* Load sections into RAM */
> >   	for (i = num_sections - 1; i >= 0; i--) {
> > @@ -328,7 +753,8 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   				(unsigned long)image_base) != EFI_SUCCESS) {
> >   		efi_free_pages((uintptr_t) efi_reloc,
> >   			       (virt_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT);
> > -		return EFI_LOAD_ERROR;
> > +		ret = EFI_LOAD_ERROR;
> > +		goto err;
> >   	}
> > 
> >   	/* Flush cache */
> > @@ -340,5 +766,13 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
> >   	loaded_image_info->image_base = efi_reloc;
> >   	loaded_image_info->image_size = virt_size;
> > 
> > -	return EFI_SUCCESS;
> > +	if (handle->auth_status == EFI_IMAGE_AUTH_PASSED)
> > +		return EFI_SUCCESS;
> > +	else
> > +		return EFI_SECURITY_VIOLATION;
> > +
> > +err:
> > +	free(new_efi);
> > +
> > +	return ret;
> >   }
> > 
>
diff mbox series

Patch

diff --git a/include/efi_loader.h b/include/efi_loader.h
index f461c6195834..0e15470d9c17 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -11,6 +11,7 @@ 
 #include <common.h>
 #include <part_efi.h>
 #include <efi_api.h>
+#include <pe.h>
 
 static inline int guidcmp(const void *g1, const void *g2)
 {
@@ -263,6 +264,11 @@  struct efi_object {
 	enum efi_object_type type;
 };
 
+enum efi_image_auth_status {
+	EFI_IMAGE_AUTH_FAILED = 0,
+	EFI_IMAGE_AUTH_PASSED,
+};
+
 /**
  * struct efi_loaded_image_obj - handle of a loaded image
  *
@@ -282,6 +288,7 @@  struct efi_loaded_image_obj {
 	EFIAPI efi_status_t (*entry)(efi_handle_t image_handle,
 				     struct efi_system_table *st);
 	u16 image_type;
+	enum efi_image_auth_status auth_status;
 };
 
 /**
@@ -414,7 +421,8 @@  efi_status_t efi_set_watchdog(unsigned long timeout);
 /* Called from places to check whether a timer expired */
 void efi_timer_check(void);
 /* PE loader implementation */
-efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
+efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
+			 void *efi, size_t efi_size,
 			 struct efi_loaded_image *loaded_image_info);
 /* Called once to store the pristine gd pointer */
 void efi_save_gd(void);
@@ -741,6 +749,9 @@  void efi_sigstore_free(struct efi_signature_store *sigstore);
 struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
 
 bool efi_secure_boot_enabled(void);
+
+bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
+		     WIN_CERTIFICATE **auth, size_t *auth_len);
 #endif /* CONFIG_EFI_SECURE_BOOT */
 
 #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 1f598b357a5c..cc8cc4cb5408 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1882,12 +1882,12 @@  efi_status_t EFIAPI efi_load_image(bool boot_policy,
 	efi_dp_split_file_path(file_path, &dp, &fp);
 	ret = efi_setup_loaded_image(dp, fp, image_obj, &info);
 	if (ret == EFI_SUCCESS)
-		ret = efi_load_pe(*image_obj, dest_buffer, info);
+		ret = efi_load_pe(*image_obj, dest_buffer, source_size, info);
 	if (!source_buffer)
 		/* Release buffer to which file was loaded */
 		efi_free_pages((uintptr_t)dest_buffer,
 			       efi_size_in_pages(source_size));
-	if (ret == EFI_SUCCESS) {
+	if (ret == EFI_SUCCESS || ret == EFI_SECURITY_VIOLATION) {
 		info->system_table = &systab;
 		info->parent_handle = parent_image;
 	} else {
@@ -2885,10 +2885,16 @@  efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 
 	EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data);
 
+	if (!efi_search_obj(image_handle))
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+
 	/* Check parameters */
 	if (image_obj->header.type != EFI_OBJECT_TYPE_LOADED_IMAGE)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
+	if (image_obj->auth_status != EFI_IMAGE_AUTH_PASSED)
+		return EFI_EXIT(EFI_SECURITY_VIOLATION);
+
 	ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image,
 					 &info, NULL, NULL,
 					 EFI_OPEN_PROTOCOL_GET_PROTOCOL));
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index d5de6df16d84..f6ddddb44cdd 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -10,7 +10,10 @@ 
 #include <common.h>
 #include <cpu_func.h>
 #include <efi_loader.h>
+#include <malloc.h>
 #include <pe.h>
+#include <sort.h>
+#include "../lib/crypto/pkcs7_parser.h"
 
 const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
 const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID;
@@ -206,6 +209,367 @@  static void efi_set_code_and_data_type(
 	}
 }
 
+#ifdef CONFIG_EFI_SECURE_BOOT
+/**
+ * cmp_pe_section - compare two sections
+ * @arg1:	Pointer to first section
+ * @arg2:	Pointer to second section
+ *
+ * Compare two sections in PE image.
+ *
+ * Return:	-1, 0, 1 respectively if arg1 < arg2, arg1 == arg2 or
+		arg1 > arg2
+ */
+static int cmp_pe_section(const void *arg1, const void *arg2)
+{
+	const IMAGE_SECTION_HEADER *section1 = arg1, *section2 = arg2;
+
+	if (section1->VirtualAddress < section2->VirtualAddress)
+		return -1;
+	else if (section1->VirtualAddress == section2->VirtualAddress)
+		return 0;
+	else
+		return 1;
+}
+
+/**
+ * efi_image_parse - parse a PE image
+ * @efi:	Pointer to image
+ * @len:	Size of @efi
+ * @regs:	Pointer to a list of regions
+ * @auth:	Pointer to a pointer to authentication data in PE
+ * @auth_len:	Size of @auth
+ *
+ * Parse image binary in PE32(+) format, assuming that sanity of PE image
+ * has been checked by a caller.
+ * On success, an address of authentication data in @efi and its size will
+ * be returned in @auth and @auth_len, respectively.
+ *
+ * Return:	true on success, false on error
+ */
+bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
+		     WIN_CERTIFICATE **auth, size_t *auth_len)
+{
+	struct efi_image_regions *regs;
+	IMAGE_DOS_HEADER *dos;
+	IMAGE_NT_HEADERS32 *nt;
+	IMAGE_SECTION_HEADER *sections, **sorted;
+	int num_regions, num_sections, i;
+	int ctidx = IMAGE_DIRECTORY_ENTRY_SECURITY;
+	u32 align, size, authsz, authoff;
+	size_t bytes_hashed;
+
+	dos = (void *)efi;
+	nt = (void *)(efi + dos->e_lfanew);
+
+	/*
+	 * Count maximum number of regions to be digested.
+	 * We don't have to have an exact number here.
+	 * See efi_image_region_add()'s in parsing below.
+	 */
+	num_regions = 3; /* for header */
+	num_regions += nt->FileHeader.NumberOfSections;
+	num_regions++; /* for extra */
+
+	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * num_regions,
+		      1);
+	if (!regs)
+		goto err;
+	regs->max = num_regions;
+
+	/*
+	 * Collect data regions for hash calculation
+	 * 1. File headers
+	 */
+	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
+		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
+
+		/* Skip CheckSum */
+		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
+		if (nt64->OptionalHeader.NumberOfRvaAndSizes <= ctidx) {
+			efi_image_region_add(regs,
+					     &opt->CheckSum + 1,
+					     efi + opt->SizeOfHeaders, 0);
+		} else {
+			/* Skip Certificates Table */
+			efi_image_region_add(regs,
+					     &opt->CheckSum + 1,
+					     &opt->DataDirectory[ctidx], 0);
+			efi_image_region_add(regs,
+					     &opt->DataDirectory[ctidx] + 1,
+					     efi + opt->SizeOfHeaders, 0);
+		}
+
+		bytes_hashed = opt->SizeOfHeaders;
+		align = opt->FileAlignment;
+		authoff = opt->DataDirectory[ctidx].VirtualAddress;
+		authsz = opt->DataDirectory[ctidx].Size;
+	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
+
+		efi_image_region_add(regs, efi, &opt->CheckSum, 0);
+		efi_image_region_add(regs, &opt->CheckSum + 1,
+				     &opt->DataDirectory[ctidx], 0);
+		efi_image_region_add(regs, &opt->DataDirectory[ctidx] + 1,
+				     efi + opt->SizeOfHeaders, 0);
+
+		bytes_hashed = opt->SizeOfHeaders;
+		align = opt->FileAlignment;
+		authoff = opt->DataDirectory[ctidx].VirtualAddress;
+		authsz = opt->DataDirectory[ctidx].Size;
+	} else {
+		debug("%s: Invalid optional header magic %x\n", __func__,
+		      nt->OptionalHeader.Magic);
+		goto err;
+	}
+
+	/* 2. Sections */
+	num_sections = nt->FileHeader.NumberOfSections;
+	sections = (void *)((uint8_t *)&nt->OptionalHeader +
+			    nt->FileHeader.SizeOfOptionalHeader);
+	sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections);
+	if (!sorted) {
+		debug("%s: Out of memory\n", __func__);
+		goto err;
+	}
+
+	/*
+	 * Make sure the section list is in ascending order.
+	 */
+	for (i = 0; i < num_sections; i++)
+		sorted[i] = &sections[i];
+	qsort(sorted, num_sections, sizeof(&sections[0]), cmp_pe_section);
+
+	for (i = 0; i < num_sections; i++) {
+		if (!sorted[i]->SizeOfRawData)
+			continue;
+
+		size = (sorted[i]->SizeOfRawData + align - 1) & ~(align - 1);
+		efi_image_region_add(regs, efi + sorted[i]->PointerToRawData,
+				     efi + sorted[i]->PointerToRawData + size,
+				     0);
+		debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
+		      i, sorted[i]->Name,
+		      sorted[i]->PointerToRawData,
+		      sorted[i]->PointerToRawData + size,
+		      sorted[i]->VirtualAddress,
+		      sorted[i]->VirtualAddress
+			+ sorted[i]->Misc.VirtualSize);
+
+		bytes_hashed += size;
+	}
+	free(sorted);
+
+	/* 3. Extra data excluding Certificates Table */
+	if (bytes_hashed + authsz < len) {
+		debug("extra data for hash: %lu\n",
+		      len - (bytes_hashed + authsz));
+		efi_image_region_add(regs, efi + bytes_hashed,
+				     efi + len - authsz, 0);
+	}
+
+	/* Return Certificates Table */
+	if (authsz) {
+		if (len < authoff + authsz) {
+			debug("%s: Size for auth too large: %u >= %zu\n",
+			      __func__, authsz, len - authoff);
+			goto err;
+		}
+		if (authsz < sizeof(*auth)) {
+			debug("%s: Size for auth too small: %u < %zu\n",
+			      __func__, authsz, sizeof(*auth));
+			goto err;
+		}
+		*auth = efi + authoff;
+		*auth_len = authsz;
+		debug("WIN_CERTIFICATE: 0x%x, size: 0x%x\n", authoff, authsz);
+	} else {
+		*auth = NULL;
+		*auth_len = 0;
+	}
+
+	*regp = regs;
+
+	return true;
+
+err:
+	free(regs);
+
+	return false;
+}
+
+/**
+ * efi_image_unsigned_authenticate - authenticate unsigned image with
+ * SHA256 hash
+ * @regs:	List of regions to be verified
+ *
+ * If an image is not signed, it doesn't have a signature. In this case,
+ * its message digest is calculated and it will be compared with one of
+ * hash values stored in signature databases.
+ *
+ * Return:	true if authenticated, false if not
+ */
+static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs)
+{
+	struct efi_signature_store *db = NULL, *dbx = NULL;
+	bool ret = false;
+
+	dbx = efi_sigstore_parse_sigdb(L"dbx");
+	if (!dbx) {
+		debug("Getting signature database(dbx) failed\n");
+		goto out;
+	}
+
+	db = efi_sigstore_parse_sigdb(L"db");
+	if (!db) {
+		debug("Getting signature database(db) failed\n");
+		goto out;
+	}
+
+	/* try black-list first */
+	if (efi_signature_verify_with_sigdb(regs, NULL, dbx, NULL)) {
+		debug("Image is not signed and rejected by \"dbx\"\n");
+		goto out;
+	}
+
+	/* try white-list */
+	if (efi_signature_verify_with_sigdb(regs, NULL, db, NULL))
+		ret = true;
+	else
+		debug("Image is not signed and not found in \"db\" or \"dbx\"\n");
+
+out:
+	efi_sigstore_free(db);
+	efi_sigstore_free(dbx);
+
+	return ret;
+}
+
+/**
+ * efi_image_authenticate - verify a signature of signed image
+ * @efi:	Pointer to image
+ * @len:	Size of @efi
+ *
+ * A signed image should have its signature stored in a table of its PE header.
+ * So if an image is signed and only if if its signature is verified using
+ * signature databases, an image is authenticated.
+ * If an image is not signed, its validity is checked by using
+ * efi_image_unsigned_authenticated().
+ * TODO:
+ * When AuditMode==0, if the image's signature is not found in
+ * the authorized database, or is found in the forbidden database,
+ * the image will not be started and instead, information about it
+ * will be placed in this table.
+ * When AuditMode==1, an EFI_IMAGE_EXECUTION_INFO element is created
+ * in the EFI_IMAGE_EXECUTION_INFO_TABLE for every certificate found
+ * in the certificate table of every image that is validated.
+ *
+ * Return:	true if authenticated, false if not
+ */
+static bool efi_image_authenticate(void *efi, size_t len)
+{
+	struct efi_image_regions *regs = NULL;
+	WIN_CERTIFICATE *wincerts = NULL, *wincert;
+	size_t wincerts_len;
+	struct pkcs7_message *msg = NULL;
+	struct efi_signature_store *db = NULL, *dbx = NULL;
+	struct x509_certificate *cert = NULL;
+	bool ret = false;
+
+	if (!efi_secure_boot_enabled())
+		return true;
+
+	if (!efi_image_parse(efi, len, &regs, &wincerts,
+			     &wincerts_len)) {
+		debug("Parsing PE executable image failed\n");
+		return false;
+	}
+
+	if (!wincerts) {
+		/* The image is not signed */
+		ret = efi_image_unsigned_authenticate(regs);
+		free(regs);
+
+		return ret;
+	}
+
+	/*
+	 * verify signature using db and dbx
+	 */
+	db = efi_sigstore_parse_sigdb(L"db");
+	if (!db) {
+		debug("Getting signature database(db) failed\n");
+		goto err;
+	}
+
+	dbx = efi_sigstore_parse_sigdb(L"dbx");
+	if (!dbx) {
+		debug("Getting signature database(dbx) failed\n");
+		goto err;
+	}
+
+	/* go through WIN_CERTIFICATE list */
+	for (wincert = wincerts;
+	     (void *)wincert < (void *)wincerts + wincerts_len;
+	     wincert = (void *)wincert + ALIGN(wincert->dwLength, 8)) {
+		if (wincert->dwLength < sizeof(*wincert)) {
+			debug("%s: dwLength too small: %u < %zu\n",
+			      __func__, wincert->dwLength, sizeof(*wincert));
+			goto err;
+		}
+		msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
+					  wincert->dwLength - sizeof(*wincert));
+		if (!msg) {
+			debug("Parsing image's signature failed\n");
+			goto err;
+		}
+
+		/* try black-list first */
+		if (efi_signature_verify_with_sigdb(regs, msg, dbx, NULL)) {
+			debug("Signature was rejected by \"dbx\"\n");
+			goto err;
+		}
+
+		if (!efi_signature_verify_signers(msg, dbx)) {
+			debug("Signer was rejected by \"dbx\"\n");
+			goto err;
+		} else {
+			ret = true;
+		}
+
+		/* try white-list */
+		if (!efi_signature_verify_with_sigdb(regs, msg, db, &cert)) {
+			debug("Verifying signature with \"db\" failed\n");
+			goto err;
+		} else {
+			ret = true;
+		}
+
+		if (!efi_signature_verify_cert(cert, dbx)) {
+			debug("Certificate was rejected by \"dbx\"\n");
+			goto err;
+		} else {
+			ret = true;
+		}
+	}
+
+err:
+	x509_free_certificate(cert);
+	efi_sigstore_free(db);
+	efi_sigstore_free(dbx);
+	pkcs7_free_message(msg);
+	free(regs);
+
+	return ret;
+}
+#else
+static bool efi_image_authenticate(void *efi, size_t len)
+{
+	return true;
+}
+#endif /* CONFIG_EFI_SECURE_BOOT */
+
 /**
  * efi_load_pe() - relocate EFI binary
  *
@@ -217,7 +581,8 @@  static void efi_set_code_and_data_type(
  * @loaded_image_info:	loaded image protocol
  * Return:		status code
  */
-efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
+efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
+			 void *efi, size_t efi_size,
 			 struct efi_loaded_image *loaded_image_info)
 {
 	IMAGE_NT_HEADERS32 *nt;
@@ -232,17 +597,57 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 	uint64_t image_base;
 	unsigned long virt_size = 0;
 	int supported = 0;
+	void *new_efi = NULL;
+	size_t new_efi_size;
+	efi_status_t ret;
+
+	/*
+	 * Size must be 8-byte aligned and the trailing bytes must be
+	 * zero'ed. Otherwise hash value may be incorrect.
+	 */
+	if (efi_size & 0x7) {
+		new_efi_size = (efi_size + 0x7) & ~0x7ULL;
+		new_efi = calloc(new_efi_size, 1);
+		if (!new_efi)
+			return EFI_OUT_OF_RESOURCES;
+		memcpy(new_efi, efi, efi_size);
+		efi = new_efi;
+		efi_size = new_efi_size;
+	}
+
+	/* Sanity check for a file header */
+	if (efi_size < sizeof(*dos)) {
+		printf("%s: Truncated DOS Header\n", __func__);
+		ret = EFI_LOAD_ERROR;
+		goto err;
+	}
 
 	dos = efi;
 	if (dos->e_magic != IMAGE_DOS_SIGNATURE) {
 		printf("%s: Invalid DOS Signature\n", __func__);
-		return EFI_LOAD_ERROR;
+		ret = EFI_LOAD_ERROR;
+		goto err;
+	}
+
+	/* assume sizeof(IMAGE_NT_HEADERS32) <= sizeof(IMAGE_NT_HEADERS64) */
+	if (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS32)) {
+		printf("%s: Invalid offset for Extended Header\n", __func__);
+		ret = EFI_LOAD_ERROR;
+		goto err;
 	}
 
 	nt = (void *) ((char *)efi + dos->e_lfanew);
+	if ((nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
+	    (efi_size < dos->e_lfanew + sizeof(IMAGE_NT_HEADERS64))) {
+		printf("%s: Invalid offset for Extended Header\n", __func__);
+		ret = EFI_LOAD_ERROR;
+		goto err;
+	}
+
 	if (nt->Signature != IMAGE_NT_SIGNATURE) {
 		printf("%s: Invalid NT Signature\n", __func__);
-		return EFI_LOAD_ERROR;
+		ret = EFI_LOAD_ERROR;
+		goto err;
 	}
 
 	for (i = 0; machines[i]; i++)
@@ -254,14 +659,29 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 	if (!supported) {
 		printf("%s: Machine type 0x%04x is not supported\n",
 		       __func__, nt->FileHeader.Machine);
-		return EFI_LOAD_ERROR;
+		ret = EFI_LOAD_ERROR;
+		goto err;
 	}
 
-	/* Calculate upper virtual address boundary */
 	num_sections = nt->FileHeader.NumberOfSections;
 	sections = (void *)&nt->OptionalHeader +
 			    nt->FileHeader.SizeOfOptionalHeader;
 
+	if (efi_size < ((void *)sections + sizeof(sections[0]) * num_sections
+			- efi)) {
+		printf("%s: Invalid number of sections: %d\n",
+		       __func__, num_sections);
+		ret = EFI_LOAD_ERROR;
+		goto err;
+	}
+
+	/* Authenticate an image */
+	if (efi_image_authenticate(efi, efi_size))
+		handle->auth_status = EFI_IMAGE_AUTH_PASSED;
+	else
+		handle->auth_status = EFI_IMAGE_AUTH_FAILED;
+
+	/* Calculate upper virtual address boundary */
 	for (i = num_sections - 1; i >= 0; i--) {
 		IMAGE_SECTION_HEADER *sec = &sections[i];
 		virt_size = max_t(unsigned long, virt_size,
@@ -280,7 +700,8 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 		if (!efi_reloc) {
 			printf("%s: Could not allocate %lu bytes\n",
 			       __func__, virt_size);
-			return EFI_OUT_OF_RESOURCES;
+			ret = EFI_OUT_OF_RESOURCES;
+			goto err;
 		}
 		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
 		rel_size = opt->DataDirectory[rel_idx].Size;
@@ -296,7 +717,8 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 		if (!efi_reloc) {
 			printf("%s: Could not allocate %lu bytes\n",
 			       __func__, virt_size);
-			return EFI_OUT_OF_RESOURCES;
+			ret = EFI_OUT_OF_RESOURCES;
+			goto err;
 		}
 		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
 		rel_size = opt->DataDirectory[rel_idx].Size;
@@ -305,13 +727,16 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 	} else {
 		printf("%s: Invalid optional header magic %x\n", __func__,
 		       nt->OptionalHeader.Magic);
-		return EFI_LOAD_ERROR;
+		ret = EFI_LOAD_ERROR;
+		goto err;
 	}
 
 	/* Copy PE headers */
-	memcpy(efi_reloc, efi, sizeof(*dos) + sizeof(*nt)
-	       + nt->FileHeader.SizeOfOptionalHeader
-	       + num_sections * sizeof(IMAGE_SECTION_HEADER));
+	memcpy(efi_reloc, efi,
+	       sizeof(*dos)
+		 + sizeof(*nt)
+		 + nt->FileHeader.SizeOfOptionalHeader
+		 + num_sections * sizeof(IMAGE_SECTION_HEADER));
 
 	/* Load sections into RAM */
 	for (i = num_sections - 1; i >= 0; i--) {
@@ -328,7 +753,8 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 				(unsigned long)image_base) != EFI_SUCCESS) {
 		efi_free_pages((uintptr_t) efi_reloc,
 			       (virt_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT);
-		return EFI_LOAD_ERROR;
+		ret = EFI_LOAD_ERROR;
+		goto err;
 	}
 
 	/* Flush cache */
@@ -340,5 +766,13 @@  efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
 	loaded_image_info->image_base = efi_reloc;
 	loaded_image_info->image_size = virt_size;
 
-	return EFI_SUCCESS;
+	if (handle->auth_status == EFI_IMAGE_AUTH_PASSED)
+		return EFI_SUCCESS;
+	else
+		return EFI_SECURITY_VIOLATION;
+
+err:
+	free(new_efi);
+
+	return ret;
 }