On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

The index (IMAGE_DIRECTORY_ENTRY_CERTTABLE) in a table points to a region containing authentication information (image's signature) in PE format.

WIN_CERTIFICATE structure defines an embedded signature format.

Those definitions will be used in my UEFI secure boot patch.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/pe.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)

diff --git a/include/pe.h b/include/pe.h index bff3b0aa7a6c..650478ca78af 100644 --- a/include/pe.h +++ b/include/pe.h @@ -155,6 +155,7 @@ typedef struct _IMAGE_SECTION_HEADER { uint32_t Characteristics; } IMAGE_SECTION_HEADER, *PIMAGE_SECTION_HEADER;

Please, add a comment here:

/* Indices for Optional Header Data Directories */

+#define IMAGE_DIRECTORY_ENTRY_CERTTABLE 4

Yes, 4 is the index of the certificate table. But EDK II calls this EFI_IMAGE_DIRECTORY_ENTRY_SECURITY. Should we use the same name to avoid confusion?

#define IMAGE_DIRECTORY_ENTRY_BASERELOC 5

typedef struct _IMAGE_BASE_RELOCATION @@ -252,4 +253,19 @@ typedef struct _IMAGE_RELOCATION #define IMAGE_REL_AMD64_PAIR 0x000F #define IMAGE_REL_AMD64_SSPAN32 0x0010

+/* certificate appended to PE image */ +typedef struct _WIN_CERTIFICATE {

We do not capitalize structs. Please use 'win_certificate' (like Linux does).

• uint32_t dwLength;
• uint16_t wRevision;
• uint16_t wCertificateType;
• uint8_t bCertificate[];

We do not use camelcase and type prefixes. Please, use the same component names as Linux (plus 'certificate' for your extra field).

+} WIN_CERTIFICATE, *LPWIN_CERTIFICATE;

Please, always make use of scripts/checkpatch.pl before submitting patches. It says 'WARNING: do not add new typedefs'.

Please, avoid the typedefs.

The following defines are verbatim copies from Linux. Please, also copy the comment:

/* Definitions for the contents of the certs data block */

+#define WIN_CERT_TYPE_PKCS_SIGNED_DATA 0x0002 +#define WIN_CERT_TYPE_EFI_OKCS115 0x0EF0 +#define WIN_CERT_TYPE_EFI_GUID 0x0EF1

+#define WIN_CERT_REVISION_1_0 0x0100 +#define WIN_CERT_REVISION_2_0 0x0200

• #endif /* _PE_H */

Except for the style issues this looks ok.

Best regards

Heinrich

On 11/18/19 6:44 AM, AKASHI Takahiro wrote:

Heinrich,

On Sat, Nov 16, 2019 at 06:42:11PM +0100, Heinrich Schuchardt wrote:

...

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

...

The index (IMAGE_DIRECTORY_ENTRY_CERTTABLE) in a table points to a region containing authentication information (image's signature) in PE format.

WIN_CERTIFICATE structure defines an embedded signature format.

Those definitions will be used in my UEFI secure boot patch.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/pe.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)

diff --git a/include/pe.h b/include/pe.h index bff3b0aa7a6c..650478ca78af 100644 --- a/include/pe.h +++ b/include/pe.h @@ -155,6 +155,7 @@ typedef struct _IMAGE_SECTION_HEADER { uint32_t Characteristics; } IMAGE_SECTION_HEADER, *PIMAGE_SECTION_HEADER;

Please, add a comment here:

/* Indices for Optional Header Data Directories */

Okay.

...

...

+#define IMAGE_DIRECTORY_ENTRY_CERTTABLE 4

Yes, 4 is the index of the certificate table. But EDK II calls this EFI_IMAGE_DIRECTORY_ENTRY_SECURITY. Should we use the same name to avoid confusion?

No. Because the naming is consistent with

...

...

#define IMAGE_DIRECTORY_ENTRY_BASERELOC 5

this existing one.

IMAGE_DIRECTORY_ENTRY_BASERELOC is a constant name used by EDK2 in MdePkg/Include/IndustryStandard/PeImage.h. Why do you want to deviate for EFI_IMAGE_DIRECTORY_ENTRY_SECURITY?

...

...

typedef struct _IMAGE_BASE_RELOCATION @@ -252,4 +253,19 @@ typedef struct _IMAGE_RELOCATION #define IMAGE_REL_AMD64_PAIR 0x000F #define IMAGE_REL_AMD64_SSPAN32 0x0010

+/* certificate appended to PE image */ +typedef struct _WIN_CERTIFICATE {

We do not capitalize structs. Please use 'win_certificate' (like Linux does).

...

• uint32_t dwLength;
• uint16_t wRevision;
• uint16_t wCertificateType;
• uint8_t bCertificate[];

We do not use camelcase and type prefixes. Please, use the same component names as Linux (plus 'certificate' for your extra field).

nak. Have you looked at 'pe.h' at all? There are already bunch of use of camelcase's and capital names since this file was first introduced by Alex.

This shouldn't have happened in the first place.

Best regards

Heinrich

...

...

+} WIN_CERTIFICATE, *LPWIN_CERTIFICATE;

Please, always make use of scripts/checkpatch.pl before submitting patches. It says 'WARNING: do not add new typedefs'.

ditto. I have run scripts/cehckpatch.pl and dare to leave them unchanged.

...

Please, avoid the typedefs.

No. this is also consistent with other typedef's.

...

...

The following defines are verbatim copies from Linux. Please, also copy the comment:

/* Definitions for the contents of the certs data block */

Okay.

...

...

+#define WIN_CERT_TYPE_PKCS_SIGNED_DATA 0x0002 +#define WIN_CERT_TYPE_EFI_OKCS115 0x0EF0 +#define WIN_CERT_TYPE_EFI_GUID 0x0EF1

+#define WIN_CERT_REVISION_1_0 0x0100 +#define WIN_CERT_REVISION_2_0 0x0200

• #endif /* _PE_H */

Except for the style issues this looks ok.

Thank you for your comments. -Takahiro Akashi

...

Best regards

Heinrich

Heinrich,

On Sat, Nov 16, 2019 at 06:59:31PM +0100, Heinrich Schuchardt wrote:

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

...

This commit allows us to use common/image-sig.c even if CONFIG_FIT is disabled but CONFIG_EFI_LOADER is enabled.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/image.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/image.h b/include/image.h index bff87f51f01b..b79424a39c22 100644 --- a/include/image.h +++ b/include/image.h @@ -53,7 +53,7 @@ struct fdt_region;

#endif /* USE_HOSTCC */

-#if IMAGE_ENABLE_FIT +#if IMAGE_ENABLE_FIT || defined(CONFIG_EFI_SECURE_BOOT)

Is this #if needed at all?

Did you run Travis to check that defining CONFIG_EFI_SECURE_BOOT does not lead to build failures if IMAGE_ENABLE_FIT is not defined?

Before posting, I have confirmed that the following combination of config options did not cause any build error, at least, on qemu_arm64: 1. CONFIG_EFI_SECURE_BOOT only 2. CONFIG_FIT (with/without CONFIG_FIT_SIGNATURE) only 3. CONFIG_EFI_SECURE_BOOT and CONFIG_FIT (with/without CONFIG_FIT_SIGNATURE)

I also successfully ran Travis although CONFI_EFI_SECURE_BOOT is not enabled by default.

...

#include <hash.h> #include <linux/libfdt.h> #include <fdt_support.h> @@ -86,13 +86,14 @@ struct fdt_region; #endif

#if defined(CONFIG_FIT_ENABLE_SHA256_SUPPORT) || \

• defined(CONFIG_SPL_SHA256_SUPPORT)

• defined(CONFIG_SPL_SHA256_SUPPORT) || \
• defined(CONFIG_EFI_SECURE_BOOT)

IMAGE_ENABLE_SHA256 is only used in common/image-fit.c. So why would you change anything here?

Okay, I have forgot to remove it. At some point, I used to use both calculate_hash() and hash_calculate(), then dropped the former.

...

#define IMAGE_ENABLE_SHA256 1 #else #define IMAGE_ENABLE_SHA256 0 #endif

-#endif /* IMAGE_ENABLE_FIT */ +#endif /* IMAGE_ENABLE_FIT || defined(CONFIG_EFI_SECURE_BOOT) */

#ifdef CONFIG_SYS_BOOT_GET_CMDLINE # define IMAGE_BOOT_GET_CMDLINE 1 @@ -1261,7 +1262,6 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name); struct padding_algo *image_get_padding_algo(const char *name);

#if IMAGE_ENABLE_FIT

This change is unrelated. It should be in a separate patch. Or at least mention in the commit message that you cleaned up other parts of the code.

I don't want to get bothered with minor clean-up patches, and will drop this hunk.

-Takahiro Akashi

...

/**

• fit_image_verify_required_sigs() - Verify signatures marked as 'required'

@@ -1337,7 +1337,7 @@ static inline int fit_image_check_target_arch(const void *fdt, int node) #define fit_unsupported(msg) #define fit_unsupported_reset(msg) #endif /* CONFIG_FIT_VERBOSE */ -#endif /* CONFIG_FIT */ +#endif /* IMAGE_ENABLE_FIT */

Same here.

Best regards

Heinrich

...

#if defined(CONFIG_ANDROID_BOOT_IMAGE) struct andr_img_hdr;

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

In this commit, implemented are a couple of helper functions which will be used to materialize variable authentication as well as image authentication in later patches.

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

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/efi_api.h | 47 +++ include/efi_loader.h | 46 +++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_signature.c | 600 +++++++++++++++++++++++++++++++++ 4 files changed, 694 insertions(+) create mode 100644 lib/efi_loader/efi_signature.c

diff --git a/include/efi_api.h b/include/efi_api.h index 22396172e15f..24d034246927 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -18,6 +18,7 @@

#include <efi.h> #include <charset.h> +#include <pe.h>

#ifdef CONFIG_EFI_LOADER #include <asm/setjmp.h> @@ -307,6 +308,10 @@ struct efi_runtime_services { EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, \ 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c)

+#define EFI_IMAGE_SECURITY_DATABASE_GUID \

• EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, \

•  0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
  

• #define EFI_FDT_GUID \ EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \ 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0)

@@ -1616,4 +1621,46 @@ struct efi_unicode_collation_protocol { #define LOAD_OPTION_CATEGORY_BOOT 0x00000000 #define LOAD_OPTION_CATEGORY_APP 0x00000100

+/* Secure boot */

Please, add a comment for each GUID.

+#define EFI_CERT_SHA256_GUID \

• EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, \

•  0x41, 0xf9, 0x36, 0x93, 0x43, 0x28)
  

+#define EFI_CERT_RSA2048_GUID \

• EFI_GUID(0x3c5766e8, 0x269c, 0x4e34, 0xaa, 0x14, \

•  0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6)
  

+#define EFI_CERT_X509_GUID \

• EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, \

•  0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)
  

+#define EFI_CERT_X509_SHA256_GUID \

• EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, \

•  0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)
  

+#define EFI_CERT_TYPE_PKCS7_GUID \

• EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \

•  0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
  

Please add comments in Spinx style.

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

Same for all structures.

+struct win_certificate_uefi_guid {

• WIN_CERTIFICATE hdr;

See comment to 01/16 (avoid typedefs).

• efi_guid_t cert_type;
• u8 cert_data[];

+} __attribute__((__packed__));

+struct efi_variable_authentication_2 {

• struct efi_time time_stamp;
• struct win_certificate_uefi_guid auth_info;

+} __attribute__((__packed__));

+struct efi_signature_data {

• efi_guid_t signature_owner;
• u8 signature_data[];

+} __attribute__((__packed__));

+struct efi_signature_list {

• efi_guid_t signature_type;
• u32 signature_list_size;
• u32 signature_header_size;
• u32 signature_size;

+/* u8 signature_header[signature_header_size]; */ +/* struct efi_signature_data signatures[...][signature_size]; */ +} __attribute__((__packed__));

• #endif

diff --git a/include/efi_loader.h b/include/efi_loader.h index 381da80cdce0..5a3df557e9f2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -21,6 +21,7 @@ static inline int guidcmp(const void *g1, const void *g2) #if CONFIG_IS_ENABLED(EFI_LOADER)

#include <linux/list.h> +#include <linux/oid_registry.h>

/* Maximum number of configuration tables */ #define EFI_MAX_CONFIGURATION_TABLES 16 @@ -169,6 +170,11 @@ extern const efi_guid_t efi_guid_hii_config_routing_protocol; extern const efi_guid_t efi_guid_hii_config_access_protocol; extern const efi_guid_t efi_guid_hii_database_protocol; extern const efi_guid_t efi_guid_hii_string_protocol; +/* GUID for authentication */

%s/GUID/GUIDs/g ?

+extern const efi_guid_t efi_guid_image_security_database; +extern const efi_guid_t efi_guid_sha256; +extern const efi_guid_t efi_guid_cert_x509; +extern const efi_guid_t efi_guid_cert_x509_sha256;

extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; @@ -650,6 +656,46 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); efi_status_t efi_bootmgr_load(efi_handle_t *handle);

+#ifdef CONFIG_EFI_SECURE_BOOT

Why do we need an #ifdef here?

+#include <image.h>

+#define EFI_REGS_MAX 16 /* currently good enough */

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

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

Add comments explaining what the structures are used for.

+struct efi_image_regions {

• int num;
• struct image_region reg[EFI_REGS_MAX];

+};

+struct efi_sig_data {

• struct efi_sig_data *next;
• efi_guid_t owner;
• void *data;
• size_t size;

+};

+struct efi_signature_store {

• struct efi_signature_store *next;
• efi_guid_t sig_type;
• struct efi_sig_data *sig_data_list;

+};

+struct x509_certificate; +struct pkcs7_message;

+bool efi_signature_verify_cert(struct x509_certificate *cert,

• 	       struct efi_signature_store *dbx);
  

+bool efi_signature_verify_signers(struct pkcs7_message *msg,

• 		  struct efi_signature_store *dbx);
  

+bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,

• 		     struct pkcs7_message *msg,
  

• 		  struct efi_signature_store *db,
  

• 		  struct x509_certificate **cert);
  

+efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck);
  

+#endif /* CONFIG_EFI_SECURE_BOOT */

• #else /* CONFIG_IS_ENABLED(EFI_LOADER) */

  /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */

diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 01769ea58ba6..49c996c89052 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -39,3 +39,4 @@ obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o +obj-y += efi_signature.o diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c new file mode 100644 index 000000000000..04f43e47d1a3 --- /dev/null +++ b/lib/efi_loader/efi_signature.c @@ -0,0 +1,600 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2018 Patrick Wildt patrick@blueri.se

From which upstream did you take the code?

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

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

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

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

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

+#include <u-boot/sha256.h> +/*

• • avoid duplicated inclusion:
• • #include "../lib/crypto/x509_parser.h"
• */

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

+const efi_guid_t efi_guid_image_security_database =

• EFI_IMAGE_SECURITY_DATABASE_GUID;
  

+const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; +const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; +const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID; +const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;

+#ifdef CONFIG_EFI_SECURE_BOOT +static const unsigned char WinIndirectSha256[] = {

We don't use camel case.

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

+};

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

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

+/**

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

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

• • Return: true on success, false on error
• */

+static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,

• 	     size_t *size)
  

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

+{

• *size = 0;
• *hash = calloc(1, SHA256_SUM_LEN);
• if (!*hash) {

• debug("Out of memory\n");
  

• return false;
  

• }
• *size = SHA256_SUM_LEN;
• hash_calculate("sha256", regs->reg, regs->num, *hash);

+#ifdef DEBUG

• debug("hash calculated:\n");

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

• print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,

•        *hash, SHA256_SUM_LEN, false);
  

This will fail to compile if CONFIG_HEXDUMP is not defined?

+#endif

• return true;

+}

+/**

• • efi_hash_regions_in_der - calculate a hash value
• • @regs: List of regions
• • @hash: Pointer to a pointer to buffer holding a hash value
• • @size: Size of buffer to be returned
• • Calculate a sha256 value of @regs and return a value in @hash.
• • This function is a variant of efi_hash_regions(), and the data will
• • be wrapped with some header in DER format before hash calculation.
• • Message digest in signature of PE format will be calculated this way.
• • Return: true on success, false on error
• */

+static bool efi_hash_regions_in_der(struct efi_image_regions *regs, void **hash,

• 		    size_t *size)
  

+{

• void *msg;
• size_t msg_size;
• struct image_region regtmp[2];
• if (!efi_hash_regions(regs, &msg, &msg_size)) {

• debug("Hash calculation failed\n");
  

• return false;
  

• ;
  

• }
• *size = 0;
• *hash = calloc(1, SHA256_SUM_LEN);
• if (!*hash) {

• debug("Out of memory\n");
  

• free(msg);
  

• return false;
  

• }
• *size = SHA256_SUM_LEN;
• /* File image hash is digested with some DER wrapper. */
• regtmp[0].data = WinIndirectSha256;
• regtmp[0].size = sizeof(WinIndirectSha256);
• regtmp[1].data = msg;
• regtmp[1].size = msg_size;
• hash_calculate("sha256", regtmp, 2, *hash);

+#ifdef DEBUG

• debug("hash calculated with extra header:\n");
• print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,

•        *hash, SHA256_SUM_LEN, false);
  

+#endif

• free(msg);
• return true;

+}

+/**

• • efi_signature_verify - verify a signature with a certificate
• • @regs: List of regions to be authenticated
• • @signed_info: Pointer to PKCS7's signed_info
• • @cert: x509 certificate
• • Signature pointed to by @signed_info against image pointed to by @regs
• • is verified by a certificate pointed to by @cert.
• • @signed_info holds a signature, including a message digest which is to be
• • compared with a hash value calculated from @regs.
• • Return: true if signature is verified, false if not
• */

+static bool efi_signature_verify(struct efi_image_regions *regs,

• 		 struct pkcs7_signed_info *ps_info,
  

• 		 struct x509_certificate *cert)
  

+{

• struct image_sign_info info;
• struct image_region regtmp[2];
• void *hash;
• size_t size;
• char c;
• bool verified;
• debug("%s: Enter, %p, %p, %p(issuer: %s, subject: %s)\n", __func__,

•      regs, ps_info, cert, cert->issuer, cert->subject);
  

• verified = false;
• memset(&info, '\0', sizeof(info));
• info.padding = image_get_padding_algo("pkcs-1.5");
• /*

• * Note: image_get_[checksum|crypto]_algo takes an string
  

• * argument like "<checksum>,<crypto>"
  

• * TODO: support other hash algorithms
  

• */
  

• if (!strcmp(ps_info->sig->hash_algo, "sha1")) {

• info.checksum = image_get_checksum_algo("sha1,rsa2048");
  

• info.name = "sha1,rsa2048";
  

• } else if (!strcmp(ps_info->sig->hash_algo, "sha256")) {

• info.checksum = image_get_checksum_algo("sha256,rsa2048");
  

• info.name = "sha256,rsa2048";
  

• } else {

• debug("unknown msg digest algo: %s\n", ps_info->sig->hash_algo);
  

• goto out;
  

• }
• info.crypto = image_get_crypto_algo(info.name);
• info.key = cert->pub->key;
• info.keylen = cert->pub->keylen;
• /* verify signature */
• debug("%s: crypto: %s, signature len:%x\n", __func__,

•      info.name, ps_info->sig->s_size);
  

• if (ps_info->aa_set & (1UL << sinfo_has_message_digest)) {

• debug("%s: RSA verify authentication attribute\n", __func__);
  

• /*
  

•  * NOTE: This path will be executed only for
  

•  * PE image authentication
  

•  */
  

• /* check if hash matches digest first */
  

• debug("checking msg digest first, len:0x%x\n",
  

•       ps_info->msgdigest_len);
  

+#ifdef DEBUG

• debug("hash in database:\n");
  

• print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
  

• 	       ps_info->msgdigest, ps_info->msgdigest_len,
  

• 	       false);
  

+#endif

• if (efi_hash_regions_in_der(regs, &hash, &size)) {
  

• 	if (ps_info->msgdigest_len != size ||
  

• 	    memcmp(hash, ps_info->msgdigest, size)) {
  

• 		debug("Digest doesn't match\n");
  

• 		free(hash);
  

• 		goto out;
  

• 	}
  

• 	free(hash);
  

• } else {
  

• 	debug("Digesting image failed\n");
  

• 	goto out;
  

• }
  

• /* against digest */
  

• c = 0x31;
  

• regtmp[0].data = &c;
  

• regtmp[0].size = 1;
  

• regtmp[1].data = ps_info->authattrs;
  

• regtmp[1].size = ps_info->authattrs_len;
  

• if (!rsa_verify(&info, regtmp, 2,
  

• 		ps_info->sig->s, ps_info->sig->s_size))
  

• 	verified = true;
  

• } else {

• debug("%s: RSA verify content data\n", __func__);
  

• /* against all data */
  

• if (!rsa_verify(&info, regs->reg, regs->num,
  

• 		ps_info->sig->s, ps_info->sig->s_size))
  

• 	verified = true;
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_signature_verify_with_list - verify a signature with signature list
• • @regs: List of regions to be authenticated
• • @signed_info: Pointer to PKCS7's signed_info
• • @siglist: Signature list for certificates
• • @valid_cert: x509 certificate that verifies this signature
• • Signature pointed to by @signed_info against image pointed to by @regs
• • is verified by signature list pointed to by @siglist.
• • Signature database is a simple concatenation of one or more
• • signature list(s).
• • Return: true if signature is verified, false if not
• */

+static +bool efi_signature_verify_with_list(struct efi_image_regions *regs,

• 		    struct pkcs7_signed_info *signed_info,
  

• 		    struct efi_signature_store *siglist,
  

• 		    struct x509_certificate **valid_cert)
  

+{

• struct x509_certificate *cert;
• struct efi_sig_data *sig_data;
• bool verified = false;
• debug("%s: Enter, %p, %p, %p, %p\n", __func__,

•      regs, signed_info, siglist, valid_cert);
  

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

• if (!signed_info) {

• void *hash;
  

• size_t size;
  

• debug("%s: unsigned image\n", __func__);
  

We already have indicated the function above.

• /*
  

•  * verify based on calculated hash value
  

•  * TODO: support other hash algorithms
  

•  */
  

• if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) {
  

• 	debug("Digest algorithm is not supported: %pUl\n",
  

• 	      &siglist->sig_type);
  

• 	goto out;
  

• }
  

• if (!efi_hash_regions(regs, &hash, &size)) {
  

• 	debug("Digesting unsigned image failed\n");
  

• 	goto out;
  

• }
  

• /* go through the list */
  

• for (sig_data = siglist->sig_data_list; sig_data;
  

•      sig_data = sig_data->next) {
  

+#ifdef DEBUG

• 	debug("Msg digest in database:\n");
  

• 	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
  

• 		       sig_data->data, sig_data->size, false);
  

Does this compile if CONFIG_HEXDUMP is not defined?

+#endif

• 	if ((sig_data->size == size) &&
  

• 	    !memcmp(sig_data->data, hash, size)) {
  

• 		verified = true;
  

• 		free(hash);
  

• 		goto out;
  

• 	}
  

• }
  

• free(hash);
  

• goto out;
  

• }
• debug("%s: signed image\n", __func__);
• if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) {

• debug("Signature type is not supported: %pUl\n",
  

•       &siglist->sig_type);
  

• goto out;
  

• }
• /* go through the list */
• for (sig_data = siglist->sig_data_list; sig_data;

•     sig_data = sig_data->next) {
  

• /* TODO: support owner check based on policy */
  

• cert = x509_cert_parse(sig_data->data, sig_data->size);
  

• if (IS_ERR(cert)) {
  

• 	debug("Parsing x509 certificate failed\n");
  

• 	goto out;
  

• }
  

• verified = efi_signature_verify(regs, signed_info, cert);
  

• if (verified) {
  

• 	if (valid_cert)
  

• 		*valid_cert = cert;
  

• 	else
  

• 		x509_free_certificate(cert);
  

• 	break;
  

• } else {
  

• 	x509_free_certificate(cert);
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_signature_verify_with_sigdb - verify a signature with db
• • @regs: List of regions to be authenticated
• • @msg: Signature
• • @db: Signature database for trusted certificates
• • @cert: x509 certificate that verifies this signature
• • Signature pointed to by @msg against image pointed to by @regs
• • is verified by signature database pointed to by @db.
• • Return: true if signature is verified, false if not
• */

+bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,

• 		     struct pkcs7_message *msg,
  

• 		     struct efi_signature_store *db,
  

• 		     struct x509_certificate **cert)
  

+{

• struct pkcs7_signed_info *info;
• struct efi_signature_store *siglist;
• bool verified = false;
• debug("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, cert);
• if (!db)

• goto out;
  

• if (!db->sig_data_list)

• goto out;
  

• /* for unsigned image */
• if (!msg) {

• debug("%s: Verify unsigned image with db\n", __func__);
  

• for (siglist = db; siglist; siglist = siglist->next)
  

• 	if (efi_signature_verify_with_list(regs, NULL,
  

• 					   siglist, cert)) {
  

• 		verified = true;
  

• 		goto out;
  

• 	}
  

• goto out;
  

• }
• /* for signed image or variable */
• debug("%s: Verify signed image with db\n", __func__);
• for (info = msg->signed_infos; info; info = info->next) {

• debug("Signed Info: digest algo: %s, pkey algo: %s\n",
  

•       info->sig->hash_algo, info->sig->pkey_algo);
  

• for (siglist = db; siglist; siglist = siglist->next) {
  

• 	if (efi_signature_verify_with_list(regs, info,
  

• 					   siglist, cert)) {
  

• 		verified = true;
  

• 		goto out;
  

• 	}
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_search_siglist - search signature list for a certificate
• • @cert: x509 certificate
• • @siglist: Signature list
• • @revoc_time: Pointer to buffer for revocation time
• • Search signature list pointed to by @siglist and find a certificate
• • pointed to by @cert.
• • If found, revocation time that is specified in signature database is
• • returned in @revoc_time.
• • Return: true if certificate is found, false if not
• */

+static bool efi_search_siglist(struct x509_certificate *cert,

• 	       struct efi_signature_store *siglist,
  

• 	       time64_t *revoc_time)
  

+{

• struct image_region reg[1];
• void *hash = NULL, *msg = NULL;
• struct efi_sig_data *sig_data;
• bool found = false;
• /* can be null */
• if (!siglist->sig_data_list)

• return false;
  

• if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) {

• /* TODO: other hash algos */
  

• debug("Certificate's digest type is not supported: %pUl\n",
  

•       &siglist->sig_type);
  

• goto out;
  

• }
• /* calculate hash of TBSCertificate */
• msg = calloc(1, SHA256_SUM_LEN);
• if (!msg) {

• debug("Out of memory\n");
  

• goto out;
  

• }
• hash = calloc(1, SHA256_SUM_LEN);
• if (!hash) {

• debug("Out of memory\n");
  

• goto out;
  

• }
• reg[0].data = cert->tbs;
• reg[0].size = cert->tbs_size;
• hash_calculate("sha256", reg, 1, msg);
• /* go through signature list */
• for (sig_data = siglist->sig_data_list; sig_data;

•     sig_data = sig_data->next) {
  

• /*
  

•  * struct efi_cert_x509_sha256 {
  

•  *	u8 tbs_hash[256/8];
  

•  *	time64_t revocation_time;
  

•  * };
  

•  */
  

• if ((sig_data->size == SHA256_SUM_LEN) &&
  

•     !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) {
  

• 	memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN,
  

• 	       sizeof(*revoc_time));
  

• 	found = true;
  

• 	goto out;
  

• }
  

• }

+out:

• free(hash);
• free(msg);
• return found;

+}

+/**

• • efi_signature_verify_cert - verify a certificate with dbx
• • @cert: x509 certificate
• • @dbx: Signature database
• • Search signature database pointed to by @dbx and find a certificate
• • pointed to by @cert.
• • This function is expected to be used against "dbx".
• • Return: true if a certificate is not rejected, false otherwise.
• */

+bool efi_signature_verify_cert(struct x509_certificate *cert,

• 	       struct efi_signature_store *dbx)
  

+{

• struct efi_signature_store *siglist;
• time64_t revoc_time;
• bool found = false;
• debug("%s: Enter, %p, %p\n", __func__, dbx, cert);

EFI_PRINT() ?

• if (!cert)

• return false;
  

• for (siglist = dbx; siglist; siglist = siglist->next) {

• if (efi_search_siglist(cert, siglist, &revoc_time)) {
  

• 	/* TODO */
  

• 	/* compare signing time with revocation time */
  

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

Best regards

Heinrich

• 	found = true;
  

• 	break;
  

• }
  

• }
• debug("%s: Exit, verified: %d\n", __func__, !found);
• return !found;

+}

+/**

• • efi_signature_verify_signers - verify signers' certificates with dbx
• • @msg: Signature
• • @dbx: Signature database
• • Determine if any of signers' certificates in @msg may be verified
• • by any of certificates in signature database pointed to by @dbx.
• • This function is expected to be used against "dbx".
• • Return: true if none of certificates is rejected, false otherwise.
• */

+bool efi_signature_verify_signers(struct pkcs7_message *msg,

• 		  struct efi_signature_store *dbx)
  

+{

• struct pkcs7_signed_info *info;
• bool found = false;
• debug("%s: Enter, %p, %p\n", __func__, msg, dbx);
• if (!msg)

• goto out;
  

• for (info = msg->signed_infos; info; info = info->next) {

• if (info->signer &&
  

•     !efi_signature_verify_cert(info->signer, dbx)) {
  

• 	found = true;
  

• 	goto out;
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, !found);
• return !found;

+}

+/**

• • efi_image_region_add - add an entry of region
• • @regs: Pointer to array of regions
• • @start: Start address of region
• • @end: End address of region
• • @nocheck: flag against overlapped regions
• • Take one entry of region [@start, @end] and append it to the list
• • pointed to by @regs. If @nocheck is false, overlapping among entries
• • will be checked first.
• • Return: 0 on success, status code (negative) on error
• */

+efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck)
  

+{

• struct image_region *reg;
• int i, j;
• if (regs->num >= EFI_REGS_MAX) {

• debug("%s: no more room for regions\n", __func__);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• if (end < start)

• return EFI_INVALID_PARAMETER;
  

• for (i = 0; i < regs->num; i++) {

• reg = &regs->reg[i];
  

• if (nocheck)
  

• 	continue;
  

• if (start > reg->data + reg->size)
  

• 	continue;
  

• if ((start >= reg->data && start < reg->data + reg->size) ||
  

•     (end > reg->data && end < reg->data + reg->size)) {
  

• 	debug("%s: new region already part of another\n",
  

• 	      __func__);
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  

• if (start < reg->data && end < reg->data + reg->size) {
  

• 	for (j = regs->num - 1; j >= i; j--)
  

• 		memcpy(&regs->reg[j], &regs->reg[j + 1],
  

• 		       sizeof(*reg));
  

• 	break;
  

• }
  

• }
• reg = &regs->reg[i];
• reg->data = start;
• reg->size = end - start;
• regs->num++;
• return EFI_SUCCESS;

+} +#endif /* CONFIG_EFI_SECURE_BOOT */

Heinrich,

On Sat, Nov 16, 2019 at 09:00:12PM +0100, Heinrich Schuchardt wrote:

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

...

In this commit, implemented are a couple of helper functions which will be used to materialize variable authentication as well as image authentication in later patches.

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

As I said elsewhere, signature verification will be best exercised and tested by image authentication or variable authentication (under pytest). As you may see, the interfaces take arguments of complicated structures and the code for setting up test environment and calling them properly would be almost equal to writing image authentication and variable authentication code.

...

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/efi_api.h | 47 +++ include/efi_loader.h | 46 +++ lib/efi_loader/Makefile | 1 + lib/efi_loader/efi_signature.c | 600 +++++++++++++++++++++++++++++++++ 4 files changed, 694 insertions(+) create mode 100644 lib/efi_loader/efi_signature.c

diff --git a/include/efi_api.h b/include/efi_api.h index 22396172e15f..24d034246927 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -18,6 +18,7 @@

#include <efi.h> #include <charset.h> +#include <pe.h>

#ifdef CONFIG_EFI_LOADER #include <asm/setjmp.h> @@ -307,6 +308,10 @@ struct efi_runtime_services { EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, \ 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c)

+#define EFI_IMAGE_SECURITY_DATABASE_GUID \

• EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, \

•  0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
  

#define EFI_FDT_GUID \ EFI_GUID(0xb1b621d5, 0xf19c, 0x41a5, \ 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0) @@ -1616,4 +1621,46 @@ struct efi_unicode_collation_protocol { #define LOAD_OPTION_CATEGORY_BOOT 0x00000000 #define LOAD_OPTION_CATEGORY_APP 0x00000100

+/* Secure boot */

Please, add a comment for each GUID.

There are bunch of GUID definitions here that have no description. It is not very useful to do so since all those macro's are already defined by UEFI specification.

...

+#define EFI_CERT_SHA256_GUID \

• EFI_GUID(0xc1c41626, 0x504c, 0x4092, 0xac, 0xa9, \

•  0x41, 0xf9, 0x36, 0x93, 0x43, 0x28)
  

+#define EFI_CERT_RSA2048_GUID \

• EFI_GUID(0x3c5766e8, 0x269c, 0x4e34, 0xaa, 0x14, \

•  0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6)
  

+#define EFI_CERT_X509_GUID \

• EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, \

•  0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)
  

+#define EFI_CERT_X509_SHA256_GUID \

• EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, \

•  0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)
  

+#define EFI_CERT_TYPE_PKCS7_GUID \

• EFI_GUID(0x4aafd29d, 0x68df, 0x49ee, 0x8a, 0xa9, \

•  0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7)
  

Please add comments in Spinx style.

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

Same for all structures.

Okay.

...

+struct win_certificate_uefi_guid {

• WIN_CERTIFICATE hdr;

See comment to 01/16 (avoid typedefs).

...

• efi_guid_t cert_type;
• u8 cert_data[];

+} __attribute__((__packed__));

+struct efi_variable_authentication_2 {

• struct efi_time time_stamp;
• struct win_certificate_uefi_guid auth_info;

+} __attribute__((__packed__));

+struct efi_signature_data {

• efi_guid_t signature_owner;
• u8 signature_data[];

+} __attribute__((__packed__));

+struct efi_signature_list {

• efi_guid_t signature_type;
• u32 signature_list_size;
• u32 signature_header_size;
• u32 signature_size;

+/* u8 signature_header[signature_header_size]; */ +/* struct efi_signature_data signatures[...][signature_size]; */ +} __attribute__((__packed__));

#endif diff --git a/include/efi_loader.h b/include/efi_loader.h index 381da80cdce0..5a3df557e9f2 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -21,6 +21,7 @@ static inline int guidcmp(const void *g1, const void *g2) #if CONFIG_IS_ENABLED(EFI_LOADER)

#include <linux/list.h> +#include <linux/oid_registry.h>

/* Maximum number of configuration tables */ #define EFI_MAX_CONFIGURATION_TABLES 16 @@ -169,6 +170,11 @@ extern const efi_guid_t efi_guid_hii_config_routing_protocol; extern const efi_guid_t efi_guid_hii_config_access_protocol; extern const efi_guid_t efi_guid_hii_database_protocol; extern const efi_guid_t efi_guid_hii_string_protocol; +/* GUID for authentication */

%s/GUID/GUIDs/g ?

Sure.

...

+extern const efi_guid_t efi_guid_image_security_database; +extern const efi_guid_t efi_guid_sha256; +extern const efi_guid_t efi_guid_cert_x509; +extern const efi_guid_t efi_guid_cert_x509_sha256;

extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; @@ -650,6 +656,46 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); efi_status_t efi_bootmgr_load(efi_handle_t *handle);

+#ifdef CONFIG_EFI_SECURE_BOOT

Why do we need an #ifdef here?

Simply because those functions won't be defined if !CONFIG_EFI_SECURE_BOOT.

...

+#include <image.h>

+#define EFI_REGS_MAX 16 /* currently good enough */

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

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

I will address this issue in the next version.

Add comments explaining what the structures are used for.

Okay.

...

+struct efi_image_regions {

• int num;
• struct image_region reg[EFI_REGS_MAX];

+};

+struct efi_sig_data {

• struct efi_sig_data *next;
• efi_guid_t owner;
• void *data;
• size_t size;

+};

+struct efi_signature_store {

• struct efi_signature_store *next;
• efi_guid_t sig_type;
• struct efi_sig_data *sig_data_list;

+};

+struct x509_certificate; +struct pkcs7_message;

+bool efi_signature_verify_cert(struct x509_certificate *cert,

• 	       struct efi_signature_store *dbx);
  

+bool efi_signature_verify_signers(struct pkcs7_message *msg,

• 		  struct efi_signature_store *dbx);
  

+bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,

• 		     struct pkcs7_message *msg,
  

• 		  struct efi_signature_store *db,
  

• 		  struct x509_certificate **cert);
  

+efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck);
  

+#endif /* CONFIG_EFI_SECURE_BOOT */

#else /* CONFIG_IS_ENABLED(EFI_LOADER) */

/* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */ diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index 01769ea58ba6..49c996c89052 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -39,3 +39,4 @@ obj-$(CONFIG_PARTITIONS) += efi_disk.o obj-$(CONFIG_NET) += efi_net.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o +obj-y += efi_signature.o diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c new file mode 100644 index 000000000000..04f43e47d1a3 --- /dev/null +++ b/lib/efi_loader/efi_signature.c @@ -0,0 +1,600 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (c) 2018 Patrick Wildt patrick@blueri.se

From which upstream did you take the code?

...

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

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

Sure.

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

...

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

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

On which part do you specifically have concerns? The following code is UEFI specific, and I don't think that either crypto/ or lib/crypto, which I created for x509/pkcs7 parsers, is a right place.

...

+#include <u-boot/sha256.h> +/*

• • avoid duplicated inclusion:
• • #include "../lib/crypto/x509_parser.h"
• */

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

+const efi_guid_t efi_guid_image_security_database =

• EFI_IMAGE_SECURITY_DATABASE_GUID;
  

+const efi_guid_t efi_guid_sha256 = EFI_CERT_SHA256_GUID; +const efi_guid_t efi_guid_cert_rsa2048 = EFI_CERT_RSA2048_GUID; +const efi_guid_t efi_guid_cert_x509 = EFI_CERT_X509_GUID; +const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;

+#ifdef CONFIG_EFI_SECURE_BOOT +static const unsigned char WinIndirectSha256[] = {

We don't use camel case.

Okay, this one is a local variable. Fix it.

...

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

+};

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

A similar technique is also used in EDK2. I will add "decoded" version of data in a comment.

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

Because it contains OID for sha256.

...

+/**

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

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

...

• • Return: true on success, false on error
• */

+static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,

• 	     size_t *size)
  

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

...

+{

• *size = 0;
• *hash = calloc(1, SHA256_SUM_LEN);
• if (!*hash) {

• debug("Out of memory\n");
  

• return false;
  

• }
• *size = SHA256_SUM_LEN;
• hash_calculate("sha256", regs->reg, regs->num, *hash);

+#ifdef DEBUG

• debug("hash calculated:\n");

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

It will be fixed once "common.h" (hence "log.h") is included.

...

• print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,

•        *hash, SHA256_SUM_LEN, false);
  

This will fail to compile if CONFIG_HEXDUMP is not defined?

No. If CONFIG_HEXDUMP is not defined, print_hex_dump() will be nullified. See lib/hexdump.c.

...

+#endif

• return true;

+}

+/**

• • efi_hash_regions_in_der - calculate a hash value
• • @regs: List of regions
• • @hash: Pointer to a pointer to buffer holding a hash value
• • @size: Size of buffer to be returned
• • Calculate a sha256 value of @regs and return a value in @hash.
• • This function is a variant of efi_hash_regions(), and the data will
• • be wrapped with some header in DER format before hash calculation.
• • Message digest in signature of PE format will be calculated this way.
• • Return: true on success, false on error
• */

+static bool efi_hash_regions_in_der(struct efi_image_regions *regs, void **hash,

• 		    size_t *size)
  

+{

• void *msg;
• size_t msg_size;
• struct image_region regtmp[2];
• if (!efi_hash_regions(regs, &msg, &msg_size)) {

• debug("Hash calculation failed\n");
  

• return false;
  

• ;
  

• }
• *size = 0;
• *hash = calloc(1, SHA256_SUM_LEN);
• if (!*hash) {

• debug("Out of memory\n");
  

• free(msg);
  

• return false;
  

• }
• *size = SHA256_SUM_LEN;
• /* File image hash is digested with some DER wrapper. */
• regtmp[0].data = WinIndirectSha256;
• regtmp[0].size = sizeof(WinIndirectSha256);
• regtmp[1].data = msg;
• regtmp[1].size = msg_size;
• hash_calculate("sha256", regtmp, 2, *hash);

+#ifdef DEBUG

• debug("hash calculated with extra header:\n");
• print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,

•        *hash, SHA256_SUM_LEN, false);
  

+#endif

• free(msg);
• return true;

+}

+/**

• • efi_signature_verify - verify a signature with a certificate
• • @regs: List of regions to be authenticated
• • @signed_info: Pointer to PKCS7's signed_info
• • @cert: x509 certificate
• • Signature pointed to by @signed_info against image pointed to by @regs
• • is verified by a certificate pointed to by @cert.
• • @signed_info holds a signature, including a message digest which is to be
• • compared with a hash value calculated from @regs.
• • Return: true if signature is verified, false if not
• */

+static bool efi_signature_verify(struct efi_image_regions *regs,

• 		 struct pkcs7_signed_info *ps_info,
  

• 		 struct x509_certificate *cert)
  

+{

• struct image_sign_info info;
• struct image_region regtmp[2];
• void *hash;
• size_t size;
• char c;
• bool verified;
• debug("%s: Enter, %p, %p, %p(issuer: %s, subject: %s)\n", __func__,

•      regs, ps_info, cert, cert->issuer, cert->subject);
  

• verified = false;
• memset(&info, '\0', sizeof(info));
• info.padding = image_get_padding_algo("pkcs-1.5");
• /*

• * Note: image_get_[checksum|crypto]_algo takes an string
  

• * argument like "<checksum>,<crypto>"
  

• * TODO: support other hash algorithms
  

• */
  

• if (!strcmp(ps_info->sig->hash_algo, "sha1")) {

• info.checksum = image_get_checksum_algo("sha1,rsa2048");
  

• info.name = "sha1,rsa2048";
  

• } else if (!strcmp(ps_info->sig->hash_algo, "sha256")) {

• info.checksum = image_get_checksum_algo("sha256,rsa2048");
  

• info.name = "sha256,rsa2048";
  

• } else {

• debug("unknown msg digest algo: %s\n", ps_info->sig->hash_algo);
  

• goto out;
  

• }
• info.crypto = image_get_crypto_algo(info.name);
• info.key = cert->pub->key;
• info.keylen = cert->pub->keylen;
• /* verify signature */
• debug("%s: crypto: %s, signature len:%x\n", __func__,

•      info.name, ps_info->sig->s_size);
  

• if (ps_info->aa_set & (1UL << sinfo_has_message_digest)) {

• debug("%s: RSA verify authentication attribute\n", __func__);
  

• /*
  

•  * NOTE: This path will be executed only for
  

•  * PE image authentication
  

•  */
  

• /* check if hash matches digest first */
  

• debug("checking msg digest first, len:0x%x\n",
  

•       ps_info->msgdigest_len);
  

+#ifdef DEBUG

• debug("hash in database:\n");
  

• print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
  

• 	       ps_info->msgdigest, ps_info->msgdigest_len,
  

• 	       false);
  

+#endif

• if (efi_hash_regions_in_der(regs, &hash, &size)) {
  

• 	if (ps_info->msgdigest_len != size ||
  

• 	    memcmp(hash, ps_info->msgdigest, size)) {
  

• 		debug("Digest doesn't match\n");
  

• 		free(hash);
  

• 		goto out;
  

• 	}
  

• 	free(hash);
  

• } else {
  

• 	debug("Digesting image failed\n");
  

• 	goto out;
  

• }
  

• /* against digest */
  

• c = 0x31;
  

• regtmp[0].data = &c;
  

• regtmp[0].size = 1;
  

• regtmp[1].data = ps_info->authattrs;
  

• regtmp[1].size = ps_info->authattrs_len;
  

• if (!rsa_verify(&info, regtmp, 2,
  

• 		ps_info->sig->s, ps_info->sig->s_size))
  

• 	verified = true;
  

• } else {

• debug("%s: RSA verify content data\n", __func__);
  

• /* against all data */
  

• if (!rsa_verify(&info, regs->reg, regs->num,
  

• 		ps_info->sig->s, ps_info->sig->s_size))
  

• 	verified = true;
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_signature_verify_with_list - verify a signature with signature list
• • @regs: List of regions to be authenticated
• • @signed_info: Pointer to PKCS7's signed_info
• • @siglist: Signature list for certificates
• • @valid_cert: x509 certificate that verifies this signature
• • Signature pointed to by @signed_info against image pointed to by @regs
• • is verified by signature list pointed to by @siglist.
• • Signature database is a simple concatenation of one or more
• • signature list(s).
• • Return: true if signature is verified, false if not
• */

+static +bool efi_signature_verify_with_list(struct efi_image_regions *regs,

• 		    struct pkcs7_signed_info *signed_info,
  

• 		    struct efi_signature_store *siglist,
  

• 		    struct x509_certificate **valid_cert)
  

+{

• struct x509_certificate *cert;
• struct efi_sig_data *sig_data;
• bool verified = false;
• debug("%s: Enter, %p, %p, %p, %p\n", __func__,

•      regs, signed_info, siglist, valid_cert);
  

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

...

• if (!signed_info) {

• void *hash;
  

• size_t size;
  

• debug("%s: unsigned image\n", __func__);
  

We already have indicated the function above.

...

• /*
  

•  * verify based on calculated hash value
  

•  * TODO: support other hash algorithms
  

•  */
  

• if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) {
  

• 	debug("Digest algorithm is not supported: %pUl\n",
  

• 	      &siglist->sig_type);
  

• 	goto out;
  

• }
  

• if (!efi_hash_regions(regs, &hash, &size)) {
  

• 	debug("Digesting unsigned image failed\n");
  

• 	goto out;
  

• }
  

• /* go through the list */
  

• for (sig_data = siglist->sig_data_list; sig_data;
  

•      sig_data = sig_data->next) {
  

+#ifdef DEBUG

• 	debug("Msg digest in database:\n");
  

• 	print_hex_dump("    ", DUMP_PREFIX_OFFSET, 16, 1,
  

• 		       sig_data->data, sig_data->size, false);
  

Does this compile if CONFIG_HEXDUMP is not defined?

...

+#endif

• 	if ((sig_data->size == size) &&
  

• 	    !memcmp(sig_data->data, hash, size)) {
  

• 		verified = true;
  

• 		free(hash);
  

• 		goto out;
  

• 	}
  

• }
  

• free(hash);
  

• goto out;
  

• }
• debug("%s: signed image\n", __func__);
• if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) {

• debug("Signature type is not supported: %pUl\n",
  

•       &siglist->sig_type);
  

• goto out;
  

• }
• /* go through the list */
• for (sig_data = siglist->sig_data_list; sig_data;

•     sig_data = sig_data->next) {
  

• /* TODO: support owner check based on policy */
  

• cert = x509_cert_parse(sig_data->data, sig_data->size);
  

• if (IS_ERR(cert)) {
  

• 	debug("Parsing x509 certificate failed\n");
  

• 	goto out;
  

• }
  

• verified = efi_signature_verify(regs, signed_info, cert);
  

• if (verified) {
  

• 	if (valid_cert)
  

• 		*valid_cert = cert;
  

• 	else
  

• 		x509_free_certificate(cert);
  

• 	break;
  

• } else {
  

• 	x509_free_certificate(cert);
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_signature_verify_with_sigdb - verify a signature with db
• • @regs: List of regions to be authenticated
• • @msg: Signature
• • @db: Signature database for trusted certificates
• • @cert: x509 certificate that verifies this signature
• • Signature pointed to by @msg against image pointed to by @regs
• • is verified by signature database pointed to by @db.
• • Return: true if signature is verified, false if not
• */

+bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,

• 		     struct pkcs7_message *msg,
  

• 		     struct efi_signature_store *db,
  

• 		     struct x509_certificate **cert)
  

+{

• struct pkcs7_signed_info *info;
• struct efi_signature_store *siglist;
• bool verified = false;
• debug("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, cert);
• if (!db)

• goto out;
  

• if (!db->sig_data_list)

• goto out;
  

• /* for unsigned image */
• if (!msg) {

• debug("%s: Verify unsigned image with db\n", __func__);
  

• for (siglist = db; siglist; siglist = siglist->next)
  

• 	if (efi_signature_verify_with_list(regs, NULL,
  

• 					   siglist, cert)) {
  

• 		verified = true;
  

• 		goto out;
  

• 	}
  

• goto out;
  

• }
• /* for signed image or variable */
• debug("%s: Verify signed image with db\n", __func__);
• for (info = msg->signed_infos; info; info = info->next) {

• debug("Signed Info: digest algo: %s, pkey algo: %s\n",
  

•       info->sig->hash_algo, info->sig->pkey_algo);
  

• for (siglist = db; siglist; siglist = siglist->next) {
  

• 	if (efi_signature_verify_with_list(regs, info,
  

• 					   siglist, cert)) {
  

• 		verified = true;
  

• 		goto out;
  

• 	}
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, verified);
• return verified;

+}

+/**

• • efi_search_siglist - search signature list for a certificate
• • @cert: x509 certificate
• • @siglist: Signature list
• • @revoc_time: Pointer to buffer for revocation time
• • Search signature list pointed to by @siglist and find a certificate
• • pointed to by @cert.
• • If found, revocation time that is specified in signature database is
• • returned in @revoc_time.
• • Return: true if certificate is found, false if not
• */

+static bool efi_search_siglist(struct x509_certificate *cert,

• 	       struct efi_signature_store *siglist,
  

• 	       time64_t *revoc_time)
  

+{

• struct image_region reg[1];
• void *hash = NULL, *msg = NULL;
• struct efi_sig_data *sig_data;
• bool found = false;
• /* can be null */
• if (!siglist->sig_data_list)

• return false;
  

• if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509_sha256)) {

• /* TODO: other hash algos */
  

• debug("Certificate's digest type is not supported: %pUl\n",
  

•       &siglist->sig_type);
  

• goto out;
  

• }
• /* calculate hash of TBSCertificate */
• msg = calloc(1, SHA256_SUM_LEN);
• if (!msg) {

• debug("Out of memory\n");
  

• goto out;
  

• }
• hash = calloc(1, SHA256_SUM_LEN);
• if (!hash) {

• debug("Out of memory\n");
  

• goto out;
  

• }
• reg[0].data = cert->tbs;
• reg[0].size = cert->tbs_size;
• hash_calculate("sha256", reg, 1, msg);
• /* go through signature list */
• for (sig_data = siglist->sig_data_list; sig_data;

•     sig_data = sig_data->next) {
  

• /*
  

•  * struct efi_cert_x509_sha256 {
  

•  *	u8 tbs_hash[256/8];
  

•  *	time64_t revocation_time;
  

•  * };
  

•  */
  

• if ((sig_data->size == SHA256_SUM_LEN) &&
  

•     !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) {
  

• 	memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN,
  

• 	       sizeof(*revoc_time));
  

• 	found = true;
  

• 	goto out;
  

• }
  

• }

+out:

• free(hash);
• free(msg);
• return found;

+}

+/**

• • efi_signature_verify_cert - verify a certificate with dbx
• • @cert: x509 certificate
• • @dbx: Signature database
• • Search signature database pointed to by @dbx and find a certificate
• • pointed to by @cert.
• • This function is expected to be used against "dbx".
• • Return: true if a certificate is not rejected, false otherwise.
• */

+bool efi_signature_verify_cert(struct x509_certificate *cert,

• 	       struct efi_signature_store *dbx)
  

+{

• struct efi_signature_store *siglist;
• time64_t revoc_time;
• bool found = false;
• debug("%s: Enter, %p, %p\n", __func__, dbx, cert);

EFI_PRINT() ?

...

• if (!cert)

• return false;
  

• for (siglist = dbx; siglist; siglist = siglist->next) {

• if (efi_search_siglist(cert, siglist, &revoc_time)) {
  

• 	/* TODO */
  

• 	/* compare signing time with revocation time */
  

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

Actually, I have already implemented the prototype code for revocation support. See my linaro repository described in the cover letter. But I won't include relevant patches in this patch series because: 1. The basic authentication mechanism should be upstreamed first, yet it will provide enough function set. 2. Timestamp based revocation is quite complicated to understand. So excluding it makes your reviews easier. 3. I haven't seen no strong demands for revocation support at the moment.

Thanks, -Takahiro Akashi

Best regards

Heinrich

...

• 	found = true;
  

• 	break;
  

• }
  

• }
• debug("%s: Exit, verified: %d\n", __func__, !found);
• return !found;

+}

+/**

• • efi_signature_verify_signers - verify signers' certificates with dbx
• • @msg: Signature
• • @dbx: Signature database
• • Determine if any of signers' certificates in @msg may be verified
• • by any of certificates in signature database pointed to by @dbx.
• • This function is expected to be used against "dbx".
• • Return: true if none of certificates is rejected, false otherwise.
• */

+bool efi_signature_verify_signers(struct pkcs7_message *msg,

• 		  struct efi_signature_store *dbx)
  

+{

• struct pkcs7_signed_info *info;
• bool found = false;
• debug("%s: Enter, %p, %p\n", __func__, msg, dbx);
• if (!msg)

• goto out;
  

• for (info = msg->signed_infos; info; info = info->next) {

• if (info->signer &&
  

•     !efi_signature_verify_cert(info->signer, dbx)) {
  

• 	found = true;
  

• 	goto out;
  

• }
  

• }

+out:

• debug("%s: Exit, verified: %d\n", __func__, !found);
• return !found;

+}

+/**

• • efi_image_region_add - add an entry of region
• • @regs: Pointer to array of regions
• • @start: Start address of region
• • @end: End address of region
• • @nocheck: flag against overlapped regions
• • Take one entry of region [@start, @end] and append it to the list
• • pointed to by @regs. If @nocheck is false, overlapping among entries
• • will be checked first.
• • Return: 0 on success, status code (negative) on error
• */

+efi_status_t efi_image_region_add(struct efi_image_regions *regs,

• 		  const void *start, const void *end,
  

• 		  int nocheck)
  

+{

• struct image_region *reg;
• int i, j;
• if (regs->num >= EFI_REGS_MAX) {

• debug("%s: no more room for regions\n", __func__);
  

• return EFI_OUT_OF_RESOURCES;
  

• }
• if (end < start)

• return EFI_INVALID_PARAMETER;
  

• for (i = 0; i < regs->num; i++) {

• reg = &regs->reg[i];
  

• if (nocheck)
  

• 	continue;
  

• if (start > reg->data + reg->size)
  

• 	continue;
  

• if ((start >= reg->data && start < reg->data + reg->size) ||
  

•     (end > reg->data && end < reg->data + reg->size)) {
  

• 	debug("%s: new region already part of another\n",
  

• 	      __func__);
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  

• if (start < reg->data && end < reg->data + reg->size) {
  

• 	for (j = regs->num - 1; j >= i; j--)
  

• 		memcpy(&regs->reg[j], &regs->reg[j + 1],
  

• 		       sizeof(*reg));
  

• 	break;
  

• }
  

• }
• reg = &regs->reg[i];
• reg->data = start;
• reg->size = end - start;
• regs->num++;
• return EFI_SUCCESS;

+} +#endif /* CONFIG_EFI_SECURE_BOOT */

On 11/13/19 1:52 AM, AKASHI Takahiro wrote:

With this commit, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS is supported for authenticated variables and the system secure state will transfer between setup mode and user mode as UEFI specification section 32.3 describes.

Internally, authentication data is stored as part of authenticated variable's value. It is nothing but a pkcs7 message (but we need some wrapper, see efi_variable_parse_signature()) and will be validated by efi_variable_authenticate(), hence efi_signature_verify_with_db().

Associated time value will be encoded in "{...,time=...}" along with other UEFI variable's attributes.

Please, provide a unit test testing this single feature.

Best regards

Heinrich

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/efi_loader.h | 3 + lib/efi_loader/efi_variable.c | 664 ++++++++++++++++++++++++++++------ 2 files changed, 563 insertions(+), 104 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 92b683b7653a..2922ee5089a8 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -175,6 +175,7 @@ extern const efi_guid_t efi_guid_image_security_database; extern const efi_guid_t efi_guid_sha256; extern const efi_guid_t efi_guid_cert_x509; extern const efi_guid_t efi_guid_cert_x509_sha256; +extern const efi_guid_t efi_guid_cert_type_pkcs7;

extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; @@ -697,6 +698,8 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs,

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); #endif /* CONFIG_EFI_SECURE_BOOT */

#else /* CONFIG_IS_ENABLED(EFI_LOADER) */ diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 46f35bc60ba0..7e7db8e4a10d 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -10,7 +10,13 @@ #include <env_internal.h> #include <hexdump.h> #include <malloc.h> +#include <rtc.h> #include <search.h> +#include <linux/compat.h> +#include "../lib/crypto/pkcs7_parser.h"

+const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID; +static bool efi_secure_boot;

#define READ_ONLY BIT(31)

@@ -107,7 +113,7 @@ static const char *prefix(const char *str, const char *prefix)

• @attrp: pointer to UEFI attributes
• Return: pointer to remainder of U-Boot variable value

*/ -static const char *parse_attr(const char *str, u32 *attrp) +static const char *parse_attr(const char *str, u32 *attrp, u64 *timep) { u32 attr = 0; char sep = '{'; @@ -130,6 +136,12 @@ static const char *parse_attr(const char *str, u32 *attrp) attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS; } else if ((s = prefix(str, "run"))) { attr |= EFI_VARIABLE_RUNTIME_ACCESS;

• } else if ((s = prefix(str, "time="))) {
  

• 	attr |= EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
  

• 	hex2bin((u8 *)timep, s, sizeof(*timep));
  

• 	s += sizeof(*timep) * 2;
  

• } else if (*str == '}') {
  

• 	break;
  

  } else { printf("invalid attribute: %s\n", str); break;

@@ -147,48 +159,290 @@ static const char *parse_attr(const char *str, u32 *attrp) }

/**

• • efi_get_variable() - retrieve value of a UEFI variable

• • efi_secure_boot_enabled - return if secure boot is enabled or not

• • This function implements the GetVariable runtime service.

• • Return: true if enabled, false if disabled
• */

+bool efi_secure_boot_enabled(void) +{

• return efi_secure_boot;

+}

+#ifdef CONFIG_EFI_SECURE_BOOT +static u8 pkcs7_hdr[] = {

• /* SEQUENCE */
• 0x30, 0x82, 0x05, 0xc7,
• /* OID: pkcs7-signedData */
• 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
• /* Context Structured? */
• 0xa0, 0x82, 0x05, 0xb8,

+};

+/**

• • efi_variable_parse_signature - parse a signature in variable
• • @buf: Pointer to variable's value
• • @buflen: Length of @buf

• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.

• • Parse a signature embedded in variable's value and instantiate
• • a pkcs7_message structure. Since pkcs7_parse_message() accepts only
• • pkcs7's signedData, some header needed be prepended for correctly
• • parsing authentication data, particularly for variable's.

• • @variable_name: name of the variable
• • @vendor: vendor GUID
• • @attributes: attributes of the variable
• • @data_size: size of the buffer to which the variable value is copied
• • @data: buffer to which the variable value is copied
• • Return: status code

• • Return: Pointer to pkcs7_message structure on success, NULL on error
  */

-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,

• 		     const efi_guid_t *vendor, u32 *attributes,
  

• 		     efi_uintn_t *data_size, void *data)
  

+static struct pkcs7_message *efi_variable_parse_signature(const void *buf,

• 					  size_t buflen)
  

+{

• u8 *ebuf;
• size_t ebuflen, len;
• struct pkcs7_message *msg;
• /*

• * This is the best assumption to check if the binary is
  

• * already in a form of pkcs7's signedData.
  

• */
  

• if (buflen > sizeof(pkcs7_hdr) &&

•    !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
  

• msg = pkcs7_parse_message(buf, buflen);
  

• goto out;
  

• }
• /*

• * Otherwise, we should add a dummy prefix sequence for pkcs7
  

• * message parser to be able to process.
  

• * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
  

• * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
  

• * TODO:
  

• * The header should be composed in a more refined manner.
  

• */
  

• debug("Makeshift prefix added to authentication data\n");
• ebuflen = sizeof(pkcs7_hdr) + buflen;
• if (ebuflen <= 0x7f) {

• debug("Data is too short\n");
  

• return NULL;
  

• }
• ebuf = malloc(ebuflen);
• if (!ebuf) {

• debug("Out of memory\n");
  

• return NULL;
  

• }
• memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
• memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
• len = ebuflen - 4;
• ebuf[2] = (len >> 8) & 0xff;
• ebuf[3] = len & 0xff;
• len = ebuflen - 0x13;
• ebuf[0x11] = (len >> 8) & 0xff;
• ebuf[0x12] = len & 0xff;
• msg = pkcs7_parse_message(ebuf, ebuflen);
• free(ebuf);

+out:

• if (IS_ERR(msg))

• return NULL;
  

• return msg;

+}

+/**

• • efi_variable_authenticate - authenticate a variable
• • @variable: Variable name in u16
• • @vendor: Guid of variable
• • @data_size: Size of @data
• • @data: Pointer to variable's value
• • @given_attr: Attributes to be given at SetVariable()
• • @env_attr: Attributes that an existing variable holds
• • @time: signed time that an existing variable holds
• • Called by efi_set_variable() to verify that the input is correct.
• • Will replace the given data pointer with another that points to
• • the actual data to store in the internal memory.
• • On success, @data and @data_size will be replaced with variable's
• • actual data, excluding authentication data, and its size, and variable's
• • attributes and signed time will also be returned in @env_attr and @time,
• • respectively.
• • Return: EFI_SUCCESS on success, status code (negative) on error
• */

+static efi_status_t efi_variable_authenticate(u16 *variable,

• 			      const efi_guid_t *vendor,
  

• 			      efi_uintn_t *data_size,
  

• 			      const void **data, u32 given_attr,
  

• 			      u32 *env_attr, u64 *time)
  

+{

• const struct efi_variable_authentication_2 *auth;
• struct efi_signature_store *truststore, *truststore2;
• struct pkcs7_message *var_sig;
• struct efi_image_regions *regs;
• const struct efi_time *timestamp;
• struct rtc_time tm;
• u64 new_time;
• efi_status_t ret;
• var_sig = NULL;
• truststore = NULL;
• truststore2 = NULL;
• regs = NULL;
• ret = EFI_SECURITY_VIOLATION;
• if (*data_size < sizeof(struct efi_variable_authentication_2))

• goto err;
  

• /* authentication data */
• auth = *data;
• if (*data_size < (sizeof(auth->time_stamp)

• 		+ auth->auth_info.hdr.dwLength))
  

• goto err;
  

• if (guidcmp(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7))

• goto err;
  

• *data += sizeof(auth->time_stamp) + auth->auth_info.hdr.dwLength;
• *data_size -= (sizeof(auth->time_stamp)

• 		+ auth->auth_info.hdr.dwLength);
  

• timestamp = &auth->time_stamp;
• memset(&tm, 0, sizeof(tm));
• tm.tm_year = timestamp->year;
• tm.tm_mon = timestamp->month;
• tm.tm_mday = timestamp->day;
• tm.tm_hour = timestamp->hour;
• tm.tm_min = timestamp->minute;
• tm.tm_sec = timestamp->second;
• new_time = rtc_mktime(&tm);
• if (!efi_secure_boot_enabled()) {

• /* finished checking */
  

• *time = new_time;
  

• return EFI_SUCCESS;
  

• }
• if (new_time <= *time)

• goto err;
  

• /* data to be digested */
• regs = calloc(1, sizeof(*regs));
• if (!regs)

• goto err;
  

• efi_image_region_add(regs, (uint8_t *)variable,

• 	     (uint8_t *)variable
  

• 		+ u16_strlen(variable) * sizeof(u16), 1);
  

• efi_image_region_add(regs, (uint8_t *)vendor,

• 	     (uint8_t *)vendor + sizeof(*vendor), 1);
  

• efi_image_region_add(regs, (uint8_t *)&given_attr,

• 	     (uint8_t *)&given_attr + sizeof(given_attr), 1);
  

• efi_image_region_add(regs, (uint8_t *)timestamp,

• 	     (uint8_t *)timestamp + sizeof(*timestamp), 1);
  

• efi_image_region_add(regs, (uint8_t *)*data,

• 	     (uint8_t *)*data + *data_size, 1);
  

• /* variable's signature list */
• if (auth->auth_info.hdr.dwLength < sizeof(auth->auth_info))

• goto err;
  

• var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,

• 			       auth->auth_info.hdr.dwLength
  

• 				   - sizeof(auth->auth_info));
  

• if (IS_ERR(var_sig)) {

• debug("Parsing variable's signature failed\n");
  

• var_sig = NULL;
  

• goto err;
  

• }
• /* signature database used for authentication */
• if (u16_strcmp(variable, L"PK") == 0 ||

•    u16_strcmp(variable, L"KEK") == 0) {
  

• /* with PK */
  

• truststore = efi_sigstore_parse_sigdb(L"PK");
  

• if (!truststore)
  

• 	goto err;
  

• } else if (u16_strcmp(variable, L"db") == 0 ||

•    u16_strcmp(variable, L"dbx") == 0) {
  

• /* with PK and KEK */
  

• truststore = efi_sigstore_parse_sigdb(L"KEK");
  

• truststore2 = efi_sigstore_parse_sigdb(L"PK");
  

• if (!truststore) {
  

• 	if (!truststore2)
  

• 		goto err;
  

• 	truststore = truststore2;
  

• 	truststore2 = NULL;
  

• }
  

• } else {

• /* TODO: support private authenticated variables */
  

• goto err;
  

• }
• /* verify signature */
• if (efi_signature_verify_with_sigdb(regs, var_sig, truststore, NULL)) {

• debug("Verified\n");
  

• } else {

• if (truststore2 &&
  

•     efi_signature_verify_with_sigdb(regs, var_sig,
  

• 				    truststore2, NULL)) {
  

• 	debug("Verified\n");
  

• } else {
  

• 	debug("Verifying variable's signature failed\n");
  

• 	goto err;
  

• }
  

• }
• /* finished checking */
• *time = rtc_mktime(&tm);
• ret = EFI_SUCCESS;

+err:

• efi_sigstore_free(truststore);
• efi_sigstore_free(truststore2);
• pkcs7_free_message(var_sig);
• free(regs);
• return ret;

+} +#else +static efi_status_t efi_variable_authenticate(u16 *variable,

• 			      const efi_guid_t *vendor,
  

• 			      efi_uintn_t *data_size,
  

• 			      const void **data, u32 given_attr,
  

• 			      u32 *env_attr, u64 *time)
  

+{

• return EFI_SUCCESS;

+} +#endif /* CONFIG_EFI_SECURE_BOOT */

+static +efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name,

• 			    const efi_guid_t *vendor,
  

• 			    u32 *attributes,
  

• 			    efi_uintn_t *data_size, void *data,
  

• 			    bool is_non_volatile)
  

  { char *native_name; efi_status_t ret; unsigned long in_size;

• const char *val, *s;

• const char *val = NULL, *s;
• u64 time = 0; u32 attr;

• EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes,

•   data_size, data);
  

• if (!variable_name || !vendor || !data_size) return EFI_EXIT(EFI_INVALID_PARAMETER);

  ret = efi_to_native(&native_name, variable_name, vendor); if (ret)

• return EFI_EXIT(ret);
  

• return ret;
  
  

  EFI_PRINT("get '%s'\n", native_name);

  val = env_get(native_name); free(native_name); if (!val)

• return EFI_EXIT(EFI_NOT_FOUND);
  

• return EFI_NOT_FOUND;
  
  

• val = parse_attr(val, &attr);

• val = parse_attr(val, &attr, &time);

  in_size = *data_size;

@@ -197,7 +451,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,

/* number of hexadecimal digits must be even */
if (len & 1)

• 	return EFI_EXIT(EFI_DEVICE_ERROR);
  

• 	return EFI_DEVICE_ERROR;
  
  

  /* two characters per byte: */ len /= 2;

@@ -208,11 +462,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, goto out; }

• if (!data)
  

• 	return EFI_EXIT(EFI_INVALID_PARAMETER);
  

• if (!data) {
  

• 	debug("Variable with no data shouldn't exist.\n");
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  
  

  if (hex2bin(data, s, len))

• 	return EFI_EXIT(EFI_DEVICE_ERROR);
  

• 	return EFI_DEVICE_ERROR;
  
  

  EFI_PRINT("got value: "%s"\n", s); } else if ((s = prefix(val, "(utf8)"))) {

@@ -225,8 +481,10 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, goto out; }

• if (!data)
  

• 	return EFI_EXIT(EFI_INVALID_PARAMETER);
  

• if (!data) {
  

• 	debug("Variable with no data shouldn't exist.\n");
  

• 	return EFI_INVALID_PARAMETER;
  

• }
  
  

  memcpy(data, s, len); ((char *)data)[len] = '\0';

@@ -234,13 +492,67 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, EFI_PRINT("got value: "%s"\n", (char *)data); } else { EFI_PRINT("invalid value: '%s'\n", val);

• return EFI_EXIT(EFI_DEVICE_ERROR);
  

• return EFI_DEVICE_ERROR;
  

  }

  out: if (attributes) *attributes = attr & EFI_VARIABLE_MASK;

• return ret;

+}

+static +efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name,

• 			      const efi_guid_t *vendor,
  

• 			      u32 *attributes,
  

• 			      efi_uintn_t *data_size,
  

• 			      void *data)
  

+{

• return efi_get_variable_common(variable_name, vendor, attributes,

• 		       data_size, data, false);
  

+}

+efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name,

• 				 const efi_guid_t *vendor,
  

• 				 u32 *attributes,
  

• 				 efi_uintn_t *data_size,
  

• 				 void *data)
  

+{

• return efi_get_variable_common(variable_name, vendor, attributes,

• 		       data_size, data, true);
  

+}

+/**

• • efi_efi_get_variable() - retrieve value of a UEFI variable
• • This function implements the GetVariable runtime service.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @variable_name: name of the variable
• • @vendor: vendor GUID
• • @attributes: attributes of the variable
• • @data_size: size of the buffer to which the variable value is copied
• • @data: buffer to which the variable value is copied
• • Return: status code
• */

+efi_status_t EFIAPI efi_get_variable(u16 *variable_name,

• 		     const efi_guid_t *vendor, u32 *attributes,
  

• 		     efi_uintn_t *data_size, void *data)
  

+{

• efi_status_t ret;
• EFI_ENTRY(""%ls" %pUl %p %p %p", variable_name, vendor, attributes,

•   data_size, data);
  

• ret = efi_get_volatile_variable(variable_name, vendor, attributes,

• 			data_size, data);
  

• if (ret == EFI_NOT_FOUND)

• ret = efi_get_nonvolatile_variable(variable_name, vendor,
  

• 				   attributes, data_size, data);
  

• return EFI_EXIT(ret); }

@@ -273,6 +585,7 @@ static efi_status_t parse_uboot_variable(char *variable, { char *guid, *name, *end, c; unsigned long name_len;

• u64 time; u16 *p;

  guid = strchr(variable, '_');

@@ -307,7 +620,7 @@ static efi_status_t parse_uboot_variable(char *variable, *(name - 1) = c;

/* attributes */

• parse_attr(end, attributes);

• parse_attr(end, attributes, &time);

  return EFI_SUCCESS; }

@@ -389,7 +702,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, list_len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY, &efi_variables_list, 0, 1, regexlist);

• /* 1 indicates that no match was found */
  

• if (list_len <= 1) return EFI_EXIT(EFI_NOT_FOUND);

@@ -402,143 +715,286 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, return EFI_EXIT(ret); }

-/**

• • efi_set_variable() - set value of a UEFI variable
• • This function implements the SetVariable runtime service.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @variable_name: name of the variable
• • @vendor: vendor GUID
• • @attributes: attributes of the variable
• • @data_size: size of the buffer with the variable value
• • @data: buffer with the variable value
• • Return: status code
• */

-efi_status_t EFIAPI efi_set_variable(u16 *variable_name,

• 		     const efi_guid_t *vendor, u32 attributes,
  

• 		     efi_uintn_t data_size, const void *data)
  

+static +efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,

• 			    const efi_guid_t *vendor,
  

• 			    u32 attributes,
  

• 			    efi_uintn_t data_size,
  

• 			    const void *data,
  

• 			    bool ro_check,
  

• 			    bool is_non_volatile)
  

  {

• char *native_name = NULL, *val = NULL, *s;
• const char *old_val;
• size_t old_size;
• efi_status_t ret = EFI_SUCCESS;

• char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
• efi_uintn_t old_size;
• bool append, delete;
• u64 time = 0; u32 attr;
• efi_status_t ret = EFI_SUCCESS;

• EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes,

•   data_size, data);
  

• debug("%s: set '%s'\n", __func__, native_name);

  if (!variable_name || !*variable_name || !vendor || ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) && !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) { ret = EFI_INVALID_PARAMETER;

• goto out;
  

• goto err;
  

  }

  ret = efi_to_native(&native_name, variable_name, vendor); if (ret)

• goto out;
  

• goto err;
  

• /* check if a variable exists */
• old_size = 0;
• attr = 0;
• ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr,

• 			&old_size, NULL));
  

• if (ret == EFI_BUFFER_TOO_SMALL) {

• if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) ||
  

•     (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) {
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto err;
  

• }
  

• }

• old_val = env_get(native_name);
• if (old_val) {

• old_val = parse_attr(old_val, &attr);
  

• append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
• attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
• delete = !append && (!data_size || !attributes);

• /* check read-only first */
  

• if (attr & READ_ONLY) {
  

• /* check attributes */
• if (old_size) {

• if (ro_check && (attr & READ_ONLY)) {
  ret = EFI_WRITE_PROTECTED;
  

• 	goto out;
  

• }
  

• if ((data_size == 0 &&
  

•      !(attributes & EFI_VARIABLE_APPEND_WRITE)) ||
  

•     !attributes) {
  

• 	/* delete the variable: */
  

• 	env_set(native_name, NULL);
  

• 	ret = EFI_SUCCESS;
  

• 	goto out;
  

• 	goto err;
  

  }

  /* attributes won't be changed */

• if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) {
  

• if (!delete &&
  

•     ((ro_check && attr != attributes) ||
  

•      (!ro_check && ((attr & ~(u32)READ_ONLY)
  

• 		    != (attributes & ~(u32)READ_ONLY))))) {
  ret = EFI_INVALID_PARAMETER;
  

• 	goto out;
  

• }
  

• if (attributes & EFI_VARIABLE_APPEND_WRITE) {
  

• 	if (!prefix(old_val, "(blob)")) {
  

• 		ret = EFI_DEVICE_ERROR;
  

• 		goto out;
  

• 	}
  

• 	old_size = strlen(old_val);
  

• } else {
  

• 	old_size = 0;
  

• 	goto err;
  

  } } else {

• if (data_size == 0 || !attributes ||
  

•     (attributes & EFI_VARIABLE_APPEND_WRITE)) {
  

• if (delete || append) {
  /*
   * Trying to delete or to update a non-existent
   * variable.
   */
  ret = EFI_NOT_FOUND;
  

• 	goto out;
  

• 	goto err;
  

• }
  

• }

• if (((!u16_strcmp(variable_name, L"PK") ||

•      !u16_strcmp(variable_name, L"KEK")) &&
  

• !guidcmp(vendor, &efi_global_variable_guid)) ||
  

•    ((!u16_strcmp(variable_name, L"db") ||
  

•      !u16_strcmp(variable_name, L"dbx")) &&
  

• !guidcmp(vendor, &efi_guid_image_security_database))) {
  

• /* authentication is mandatory */
  

• if (!(attributes &
  

•       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
  

• 	debug("%ls: AUTHENTICATED_WRITE_ACCESS required\n",
  

• 	      variable_name);
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto err;
  

  }

• }

• /* authenticate a variable */

• if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {

• if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto err;
  

• }
  

• if (attributes &
  

•     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
  

• 	ret = efi_variable_authenticate(variable_name, vendor,
  

• 					&data_size, &data,
  

• 					attributes, &attr,
  

• 					&time);
  

• 	if (ret != EFI_SUCCESS)
  

• 		goto err;
  

• 	/* last chance to check for delete */
  

• 	if (!data_size)
  

• 		delete = true;
  

• }
  

• } else {

• if (attributes &
  

•     (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
  

•      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
  

• 	debug("Secure boot is not configured\n");
  

• 	ret = EFI_INVALID_PARAMETER;
  

• 	goto err;
  

• }
  

• }

• /* delete a variable */

• if (delete) {

• /* !old_size case has been handled before */
  

• val = NULL;
  

• ret = EFI_SUCCESS;
  

• goto out;
  

• }

• if (append) {

• old_data = malloc(old_size);
  

• if (!old_data) {
  

• 	return EFI_OUT_OF_RESOURCES;
  

• 	goto err;
  

• }
  

• ret = EFI_CALL(efi_get_variable(variable_name, vendor,
  

• 				&attr, &old_size, old_data));
  

• if (ret != EFI_SUCCESS)
  

• 	goto err;
  

• } else { old_size = 0; }

• val = malloc(old_size + 2 * data_size

•      + strlen("{ro,run,boot,nv}(blob)") + 1);
  

• val = malloc(2 * old_size + 2 * data_size

•      + strlen("{ro,run,boot,nv,time=0123456701234567}(blob)")
  

•      + 1);
  

  if (!val) { ret = EFI_OUT_OF_RESOURCES;

• goto out;
  

• goto err;
  

  }

  s = val;

• /* store attributes */
• attributes &= (EFI_VARIABLE_NON_VOLATILE |

• /*

• * store attributes
  

• */
  

• attributes &= (READ_ONLY |

•        EFI_VARIABLE_NON_VOLATILE |
    EFI_VARIABLE_BOOTSERVICE_ACCESS |
  

•        EFI_VARIABLE_RUNTIME_ACCESS);
  

•        EFI_VARIABLE_RUNTIME_ACCESS |
  

•        EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS);
  

  s += sprintf(s, "{"); while (attributes) {

• u32 attr = 1 << (ffs(attributes) - 1);
  

• attr = 1 << (ffs(attributes) - 1);
  
  

• if (attr == EFI_VARIABLE_NON_VOLATILE)
  

• if (attr == READ_ONLY) {
  

• 	s += sprintf(s, "ro");
  

• } else if (attr == EFI_VARIABLE_NON_VOLATILE) {
  s += sprintf(s, "nv");
  

• else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS)
  

• } else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS) {
  s += sprintf(s, "boot");
  

• else if (attr == EFI_VARIABLE_RUNTIME_ACCESS)
  

• } else if (attr == EFI_VARIABLE_RUNTIME_ACCESS) {
  s += sprintf(s, "run");
  

• } else if (attr ==
  

• 	   EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
  

• 	s += sprintf(s, "time=");
  

• 	s = bin2hex(s, (u8 *)&time, sizeof(time));
  

• }
  
  

  attributes &= ~attr; if (attributes) s += sprintf(s, ","); } s += sprintf(s, "}");

• if (old_size)

• /* APPEND_WRITE */
  

• s += sprintf(s, old_val);
  

• else

• s += sprintf(s, "(blob)");
  

• s += sprintf(s, "(blob)");

  /* store payload: */

• if (append)

• s = bin2hex(s, old_data, old_size);
  

  s = bin2hex(s, data, data_size); *s = '\0';

  EFI_PRINT("setting: %s=%s\n", native_name, val);

+out: if (env_set(native_name, val)) ret = EFI_DEVICE_ERROR;

• else

• ret = EFI_SUCCESS;
  
  

-out: +err: free(native_name);

• free(old_data); free(val);

• return EFI_EXIT(ret);

• return ret;

+}

+static +efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name,

• 			      const efi_guid_t *vendor,
  

• 			      u32 attributes,
  

• 			      efi_uintn_t data_size,
  

• 			      const void *data,
  

• 			      bool ro_check)
  

+{

• return efi_set_variable_common(variable_name, vendor, attributes,

• 		       data_size, data, ro_check, false);
  

+}

+efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name,

• 				 const efi_guid_t *vendor,
  

• 				 u32 attributes,
  

• 				 efi_uintn_t data_size,
  

• 				 const void *data,
  

• 				 bool ro_check)
  

+{

• efi_status_t ret;
• ret = efi_set_variable_common(variable_name, vendor, attributes,

• 		      data_size, data, ro_check, true);
  

• return ret;

+}

+static efi_status_t efi_set_variable_internal(u16 *variable_name,

• 			      const efi_guid_t *vendor,
  

• 			      u32 attributes,
  

• 			      efi_uintn_t data_size,
  

• 			      const void *data,
  

• 			      bool ro_check)
  

+{

• efi_status_t ret;
• if (attributes & EFI_VARIABLE_NON_VOLATILE)

• ret = efi_set_nonvolatile_variable(variable_name, vendor,
  

• 				   attributes,
  

• 				   data_size, data, ro_check);
  

• else

• ret = efi_set_volatile_variable(variable_name, vendor,
  

• 				attributes, data_size, data,
  

• 				ro_check);
  

• return ret;

+}

+/**

• • efi_set_variable() - set value of a UEFI variable
• • This function implements the SetVariable runtime service.
• • See the Unified Extensible Firmware Interface (UEFI) specification for
• • details.
• • @variable_name: name of the variable
• • @vendor: vendor GUID
• • @attributes: attributes of the variable
• • @data_size: size of the buffer with the variable value
• • @data: buffer with the variable value
• • Return: status code
• */

+efi_status_t EFIAPI efi_set_variable(u16 *variable_name,

• 		     const efi_guid_t *vendor, u32 attributes,
  

• 		     efi_uintn_t data_size, const void *data)
  

+{

• EFI_ENTRY(""%ls" %pUl %x %zu %p", variable_name, vendor, attributes,

•   data_size, data);
  

• /* READ_ONLY bit is not part of API */

• attributes &= ~(u32)READ_ONLY;

• return EFI_EXIT(efi_set_variable_internal(variable_name, vendor,

• 				  attributes, data_size, data,
  

• 				  true));
  

  }

  /**