diff mbox series

[v4,16/18] secvar/backend: add edk2 derived key updates processing

Message ID 20200511213152.24952-17-erichte@linux.ibm.com
State Changes Requested
Headers show
Series Add initial secure variable storage and backend drivers | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch
snowpatch_ozlabs/apply_patch warning Failed to apply on branch master (0f1937ef40fca0c3212a9dff1010b832a24fb063)

Commit Message

Eric Richter May 11, 2020, 9:31 p.m. UTC
From: Nayna Jain <nayna@linux.ibm.com>

As part of secureboot key management, the scheme for handling key updates
is derived from tianocore reference implementation[1]. The wrappers for
holding the signed update is the Authentication Header and for holding
the public key certificate is ESL (EFI Signature List), both derived from
tianocore reference implementation[1].

This patch adds the support to process update queue. This involves:
1. Verification of the update signature using the key authorized as per the
key hierarchy
2. Handling addition/deletion of the keys
3. Support for dbx (blacklisting of hashes)
4. Validation checks for the updates
5. Supporting multiple ESLs for single variable both for update/verification
6. Timestamp check
7. Allowing only single PK
8. Failure Handling
9. Resetting keystore if the hardware key hash changes

[1] https://github.com/tianocore/edk2-staging.git

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
V4:
 - fixed a typo in the hw-key-hash mismatch prlog
 - replace PRIORITY with PROTECTED
 - adjust most lines to 80 columns, some exceptions and prlog statements
     may still run over

 doc/secvar/edk2.rst                         |  49 ++
 include/secvar.h                            |   1 +
 libstb/secvar/backend/Makefile.inc          |   4 +-
 libstb/secvar/backend/edk2-compat-process.c | 717 ++++++++++++++++++++
 libstb/secvar/backend/edk2-compat-process.h |  61 ++
 libstb/secvar/backend/edk2-compat-reset.c   | 115 ++++
 libstb/secvar/backend/edk2-compat-reset.h   |  24 +
 libstb/secvar/backend/edk2-compat.c         | 262 +++++++
 libstb/secvar/backend/edk2.h                | 243 +++++++
 9 files changed, 1474 insertions(+), 2 deletions(-)
 create mode 100644 doc/secvar/edk2.rst
 create mode 100644 libstb/secvar/backend/edk2-compat-process.c
 create mode 100644 libstb/secvar/backend/edk2-compat-process.h
 create mode 100644 libstb/secvar/backend/edk2-compat-reset.c
 create mode 100644 libstb/secvar/backend/edk2-compat-reset.h
 create mode 100644 libstb/secvar/backend/edk2-compat.c
 create mode 100644 libstb/secvar/backend/edk2.h

Comments

Oliver O'Halloran May 14, 2020, 2:11 p.m. UTC | #1
On Mon, 2020-05-11 at 16:31 -0500, Eric Richter wrote:
> From: Nayna Jain <nayna@linux.ibm.com>
> 
> As part of secureboot key management, the scheme for handling key updates
> is derived from tianocore reference implementation[1]. The wrappers for
> holding the signed update is the Authentication Header and for holding
> the public key certificate is ESL (EFI Signature List), both derived from
> tianocore reference implementation[1].
> 
> This patch adds the support to process update queue. This involves:
> 1. Verification of the update signature using the key authorized as per the
> key hierarchy
> 2. Handling addition/deletion of the keys
> 3. Support for dbx (blacklisting of hashes)
> 4. Validation checks for the updates
> 5. Supporting multiple ESLs for single variable both for update/verification
> 6. Timestamp check
> 7. Allowing only single PK
> 8. Failure Handling
> 9. Resetting keystore if the hardware key hash changes
> 
> [1] https://github.com/tianocore/edk2-staging.git
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Signed-off-by: Eric Richter <erichte@linux.ibm.com>
> ---
> V4:
>  - fixed a typo in the hw-key-hash mismatch prlog
>  - replace PRIORITY with PROTECTED
>  - adjust most lines to 80 columns, some exceptions and prlog statements
>      may still run over
> 
>  doc/secvar/edk2.rst                         |  49 ++
>  include/secvar.h                            |   1 +
>  libstb/secvar/backend/Makefile.inc          |   4 +-
>  libstb/secvar/backend/edk2-compat-process.c | 717 ++++++++++++++++++++
>  libstb/secvar/backend/edk2-compat-process.h |  61 ++
>  libstb/secvar/backend/edk2-compat-reset.c   | 115 ++++
>  libstb/secvar/backend/edk2-compat-reset.h   |  24 +
>  libstb/secvar/backend/edk2-compat.c         | 262 +++++++
>  libstb/secvar/backend/edk2.h                | 243 +++++++
>  9 files changed, 1474 insertions(+), 2 deletions(-)
>  create mode 100644 doc/secvar/edk2.rst
>  create mode 100644 libstb/secvar/backend/edk2-compat-process.c
>  create mode 100644 libstb/secvar/backend/edk2-compat-process.h
>  create mode 100644 libstb/secvar/backend/edk2-compat-reset.c
>  create mode 100644 libstb/secvar/backend/edk2-compat-reset.h
>  create mode 100644 libstb/secvar/backend/edk2-compat.c
>  create mode 100644 libstb/secvar/backend/edk2.h
> 
> diff --git a/doc/secvar/edk2.rst b/doc/secvar/edk2.rst
> new file mode 100644
> index 00000000..1e4cc9e3
> --- /dev/null
> +++ b/doc/secvar/edk2.rst
> @@ -0,0 +1,49 @@
> +.. _secvar/edk2:
> +
> +Skiboot edk2-compatible Secure Variable Backend
> +===============================================
> +
> +Overview
> +--------
> +
> +The edk2 secure variable backend for skiboot borrows from edk2 concepts
> +such as the three key hierarchy (PK, KEK, and db), and a similar
> +structure. In general, variable updates must be signed with a key
> +of a higher level. So, updates to the db must be signed with a key stored
> +in the KEK; updates to the KEK must be signed with the PK. Updates to the
> +PK must be signed with the previous PK (if any).
> +
> +Variables are stored in the efi signature list format, and updates are a
> +signed variant that includes an authentication header.
> +
> +If no PK is currently enrolled, the system is considered to be in "Setup
> +Mode". Any key can be enrolled without signature checks. However, once a
> +PK is enrolled, the system switches to "User Mode", and each update must
> +now be signed according to the hierarchy. Furthermore, when in "User
> +Mode", the backend initialized the ``os-secure-mode`` device tree flag,
> +signaling to the kernel that we are in secure mode.
> +
> +Updates are processed sequentially, in the order that they were provided
> +in the update queue. If any update fails to validate, appears to be
> +malformed, or any other error occurs, NO updates will not be applied.
> +This includes updates that may have successfully applied prior to the
> +error. The system will continue in an error state, reporting the error
> +reason via the ``update-status`` device tree property.
> +
> +P9 Special Case for the Platform Key
> +------------------------------------
> +
> +Due to the powerful nature of the platform key and the lack of lockable
> +flash, the edk2 backend will store the PK in TPM NV rather than PNOR on
> +P9 systems. (TODO expand on this)
> +
> +Update Status Return Codes
> +--------------------------
> +
> +TODO, edk2 driver needs to actually return these properly first
> +
> +
> +Device Tree Bindings
> +--------------------
> +
> +TODO
> diff --git a/include/secvar.h b/include/secvar.h
> index 7a45db2b..4f40c115 100644
> --- a/include/secvar.h
> +++ b/include/secvar.h
> @@ -28,6 +28,7 @@ struct secvar_backend_driver {
>  };
>  
>  extern struct secvar_storage_driver secboot_tpm_driver;
> +extern struct secvar_backend_driver edk2_compatible_v1;
>  
>  int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
>  
> diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
> index 6f491a63..bc987f69 100644
> --- a/libstb/secvar/backend/Makefile.inc
> +++ b/libstb/secvar/backend/Makefile.inc
> @@ -1,11 +1,11 @@
>  # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
>  # -*-Makefile-*-
>  
> -SECVAR_BACKEND_DIR = libstb/secvar/backend
> +SECVAR_BACKEND_DIR = $(SRC)/libstb/secvar/backend
>  
>  SUBDIRS += $(SECVAR_BACKEND_DIR)
>  
> -SECVAR_BACKEND_SRCS =
> +SECVAR_BACKEND_SRCS = edk2-compat.c edk2-compat-process.c edk2-compat-reset.c
>  SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
>  SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
>  
> diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
> new file mode 100644
> index 00000000..60ebb0b2
> --- /dev/null
> +++ b/libstb/secvar/backend/edk2-compat-process.c
> @@ -0,0 +1,717 @@
> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +/* Copyright 2020 IBM Corp. */
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
> +#endif
> +
> +#include <opal.h>
> +#include <string.h>
> +#include <time.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <ccan/endian/endian.h>
> +#include <mbedtls/error.h>
> +#include <device.h>
> +#include "libstb/crypto/pkcs7/pkcs7.h"
> +#include "edk2.h"
> +#include "../secvar.h"
> +#include "edk2-compat-process.h"
> +
> +bool setup_mode;
> +
> +int update_variable_in_bank(struct secvar *secvar, const char *data,
> +			    uint64_t dsize, struct list_head *bank)
> +{
> +	struct secvar_node *node;
> +
> +	node = find_secvar(secvar->key, secvar->key_len, bank);
> +	if (!node)
> +		return OPAL_EMPTY;
> +
> +        /* Reallocate the data memory, if there is change in data size */
> +	if (node->size < dsize)
> +		if (realloc_secvar(node, dsize))
> +			return OPAL_NO_MEM;
> +
> +	if (dsize && data)
> +		memcpy(node->var->data, data, dsize);
> +	node->var->data_size = dsize;
> +
> +        /* Clear the volatile bit only if updated with positive data size */
> +	if (dsize)
> +		node->flags &= ~SECVAR_FLAG_VOLATILE;
> +	else
> +		node->flags |= SECVAR_FLAG_VOLATILE;
> +
> +        /* Is it required to be set everytime ? */
you tell me

> +	if ((!strncmp(secvar->key, "PK", 3))
> +	     || (!strncmp(secvar->key, "HWKH", 5)))
> +		node->flags |= SECVAR_FLAG_PROTECTED;
> +
> +	return 0;
> +}
> +

> +/* Converts utf8 string to ucs2 */
> +static char *utf8_to_ucs2(const char *key, size_t keylen)
> +{
> +	int i;
> +	char *str;
> +
> +	str = zalloc(keylen * 2);
> +	if (!str)
> +		return NULL;
> +
> +	for (i = 0; i < keylen*2; key++) {
> +		str[i++] = *key;
> +		str[i++] = '\0';
> +	}
> +
> +	return str;
> +}

This is completely broken for anything that doesn't fall into the ASCII
subset of UTF-8.

> +/* Returns the authority that can sign the given key update */
> +static void get_key_authority(const char *ret[3], const char *key)
> +{
> +	int i = 0;
> +
> +	if (key_equals(key, "PK")) {
> +		ret[i++] = "PK";
> +	} else if (key_equals(key, "KEK")) {
> +		ret[i++] = "PK";
> +	} else if (key_equals(key, "db") || key_equals(key, "dbx")) {
> +		ret[i++] = "KEK";
> +		ret[i++] = "PK";
> +	}
> +
> +	ret[i] = NULL;
> +}
> +
> +/* Returns the size of the complete ESL. */
> +static int get_esl_signature_list_size(char *buf, size_t buflen)
buf should be const
> +{
> +	EFI_SIGNATURE_LIST list;
> +
> +	if (buflen < sizeof(EFI_SIGNATURE_LIST))
> +		return OPAL_PARAMETER;
> +
> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
> +
> +	prlog(PR_DEBUG, "size of signature list size is %u\n",
> +			le32_to_cpu(list.SignatureListSize));
> +
> +	return le32_to_cpu(list.SignatureListSize);
> +}
> +
> +/* Copies the certificate from the ESL into cert buffer and returns the size
> + * of the certificate
> + */
> +static int get_esl_cert(char *buf, size_t buflen, char **cert)
buf should be const
> +{
> +	size_t sig_data_offset;
> +	size_t size;
> +	EFI_SIGNATURE_LIST list;
> +
> +	if (buflen < sizeof(EFI_SIGNATURE_LIST))
> +		return OPAL_PARAMETER;
> +
> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));

This looks familiar

> +
> +	size = le32_to_cpu(list.SignatureSize) - sizeof(uuid_t);
> +	/* No certificate in the ESL */
> +	if (size <= 0)
> +		return OPAL_PERMISSION;

size_t is unsigned so checking for less than zero doesn't do anything.
If list.SignatureSize is too small the calculation will underflow and
probably lead to a memory allocation failure later on.

> +	if (!cert)
> +		return OPAL_PARAMETER;
Passing a null pointer here is a programming error, use an assert.

> +	*cert = zalloc(size);
> +	if (!(*cert))
> +		return OPAL_NO_MEM;
> +
> +	prlog(PR_DEBUG,"size of signature list size is %u\n",
> +			le32_to_cpu(list.SignatureListSize));
> +	prlog(PR_DEBUG, "size of signature header size is %u\n",
> +			le32_to_cpu(list.SignatureHeaderSize));
> +	prlog(PR_DEBUG, "size of signature size is %u\n",
> +			le32_to_cpu(list.SignatureSize));
> +
> +	sig_data_offset = sizeof(list) + le32_to_cpu(list.SignatureHeaderSize)
> +		+ 16 * sizeof(uint8_t);
> +	if (sig_data_offset > buflen) {
> +		free(*cert);
> +		return OPAL_PARAMETER;
> +	}
The free() here is only required because you're performing the
allocation before verifying that the input buffer is long enough.
Reverse the order, it's less code and less error prone since you don't
have to unwind the allocation.

> +	memcpy(*cert, buf + sig_data_offset, size);
> +
> +	return size;
> +}
> +
> +/* Extracts size of the PKCS7 signed data embedded in the
> + * struct Authentication 2 Descriptor Header.
> + */
> +static int get_pkcs7_len(struct efi_variable_authentication_2 *auth)
auth should be const

> +{
> +	uint32_t dw_length;
> +	size_t size;
> +
> +	if (!auth)
> +		return OPAL_PARAMETER;
assert

> +
> +	dw_length = le32_to_cpu(auth->auth_info.hdr.dw_length);
> +	size = dw_length - (sizeof(auth->auth_info.hdr.dw_length)
> +			+ sizeof(auth->auth_info.hdr.w_revision)
> +			+ sizeof(auth->auth_info.hdr.w_certificate_type)
> +			+ sizeof(auth->auth_info.cert_type));
> +
> +	return size;
> +}
> +
> +int get_auth_descriptor2(void *buf, size_t buflen, char **auth_buffer)
buf should be const

> +{
> +	struct efi_variable_authentication_2 *auth = NULL;
> +	size_t auth_buffer_size;
> +	int len;
> +
> +	if (buflen < sizeof(struct efi_variable_authentication_2))
> +			return OPAL_PARAMETER;
> +
> +	auth = buf;
You can initalise auth to buf in the declaration above.

> +
> +	len = get_pkcs7_len(auth);
> +
> +	/* We need PKCS7 data else there is no signature */
> +	if (len <= 0)
> +		return OPAL_PARAMETER;
> +
> +	if (!auth_buffer)
> +		return OPAL_PARAMETER;

should be an assert. Also, do your parameter validation before you
start doing any actual work.

> +
> +	auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
> +			   + sizeof(auth->auth_info.cert_type) + len;
> +
> +	if (auth_buffer_size <= 0)
> +		return OPAL_PARAMETER;
> +
> +	*auth_buffer = zalloc(auth_buffer_size);
> +	if (!(*auth_buffer))
> +		return OPAL_NO_MEM;
> +
> +	memcpy(*auth_buffer, buf, auth_buffer_size);
why are we making a copy of this?

> +
> +	return auth_buffer_size;
> +}
> +
> +int validate_esl_list(char *key, char *esl, size_t size)
> +{
> +	int count = 0;
> +	int signing_cert_size;
> +	char *signing_cert = NULL;
> +	mbedtls_x509_crt x509;
> +	char *x509_buf = NULL;
> +	int eslvarsize = size;
> +	int rc = OPAL_SUCCESS;
> +	int eslsize;
> +	int offset = 0;
> +
> +	while (eslvarsize > 0) {
> +		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
> +		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
> +			break;
> +
> +		/* Calculate the size of the ESL */
> +		eslsize = get_esl_signature_list_size(esl, eslvarsize);
> +		/* If could not extract the size */
> +		if (eslsize <= 0) {
> +			prlog(PR_ERR, "Invalid size of the ESL\n");
> +			rc = OPAL_PARAMETER;
> +			break;
> +		}
> +
> +		/* Extract the certificate from the ESL */
> +		signing_cert_size = get_esl_cert(esl,
> +						 eslvarsize,
> +						 &signing_cert);
> +		if (signing_cert_size < 0) {
> +			rc = signing_cert_size;
> +			break;
> +		}
> +
> +		mbedtls_x509_crt_init(&x509);
> +		rc = mbedtls_x509_crt_parse(&x509,
> +					    signing_cert,
> +					    signing_cert_size);
> +
> +		/* If failure in parsing the certificate, exit */
> +		if(rc) {
> +			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
> +			rc = OPAL_PARAMETER;
> +			break;
> +		}
> +
> +		x509_buf = zalloc(CERT_BUFFER_SIZE);
> +		rc = mbedtls_x509_crt_info(x509_buf,
> +					   CERT_BUFFER_SIZE,
> +					   "CRT:",
> +					   &x509);
> +		prlog(PR_INFO, "%s ", x509_buf);
> +
> +		/* If failure in reading the certificate, exit */
> +		if (rc < 0) {
> +			prlog(PR_INFO, "Failed to show X509 certificate info %04x\n", rc);
> +			rc = OPAL_PARAMETER;
> +			free(x509_buf);
> +			break;
> +		}
> +		rc = 0;
> +
> +		free(x509_buf);
> +		x509_buf = NULL;
> +		count++;
> +
> +		/* Look for the next ESL */
> +		offset = offset + eslsize;
> +		eslvarsize = eslvarsize - eslsize;
> +		mbedtls_x509_crt_free(&x509);
> +		free(signing_cert);
> +		/* Since we are going to allocate again in the next iteration */
> +		signing_cert = NULL;
> +	}
> +
> +	if (rc == OPAL_SUCCESS) {
> +		if (key_equals(key, "PK") && (count > 1)) {
> +			prlog(PR_ERR, "PK can only be one\n");
> +			rc = OPAL_PARAMETER;
> +		} else {
> +			rc = count;
> +		}
> +	}
> +
> +	prlog(PR_INFO, "Total ESLs are %d\n", rc);
> +	return rc;
> +}
> +

> +/* Get the timestamp for the last update of the give key */
> +static struct efi_time *get_last_timestamp(const char *key, char *last_timestamp)
> +{
> +	u8 off;
> +
> +	if (!last_timestamp)
> +		return NULL;
> +
> +	if (!strncmp(key, "PK", 3))
> +		off = 0;
> +	else if (!strncmp(key, "KEK", 4))
> +		off = 1;
> +	else if (!strncmp(key, "db", 3))
> +		off = 2;
> +	else if (!strncmp(key, "dbx", 4))
> +		off = 3;
> +	else
> +		return NULL;
> +
> +	return &((struct efi_time *)last_timestamp)[off];
> +}

I don't know what you're doing here, but I don't like it.


> +int update_timestamp(char *key, struct efi_time *timestamp, char *last_timestamp)
> +{
> +	struct efi_time *prev;
> +
> +	prev = get_last_timestamp(key, last_timestamp);
> +	if (prev == NULL)
> +		return OPAL_INTERNAL_ERROR;
> +
> +	memcpy(prev, timestamp, sizeof(struct efi_time));
> +
> +	prlog(PR_DEBUG, "updated prev year is %d month %d day %d\n",
> +			le16_to_cpu(prev->year), prev->month, prev->day);
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +int check_timestamp(char *key, struct efi_time *timestamp,
> +		    char *last_timestamp)
> +{
> +	struct efi_time *prev;
> +
> +	prev = get_last_timestamp(key, last_timestamp);
> +	if (prev == NULL)
> +		return OPAL_INTERNAL_ERROR;
> +
> +	prlog(PR_DEBUG, "timestamp year is %d month %d day %d\n",
> +			le16_to_cpu(timestamp->year), timestamp->month,
> +			timestamp->day);
> +	prlog(PR_DEBUG, "prev year is %d month %d day %d\n",
> +			le16_to_cpu(prev->year), prev->month, prev->day);
> +	if (le16_to_cpu(timestamp->year) > le16_to_cpu(prev->year))
> +		return OPAL_SUCCESS;
> +	if (le16_to_cpu(timestamp->year) < le16_to_cpu(prev->year))
> +		return OPAL_PERMISSION;
> +
> +	if (timestamp->month > prev->month)
> +		return OPAL_SUCCESS;
> +	if (timestamp->month < prev->month)
> +		return OPAL_PERMISSION;
> +
> +	if (timestamp->day > prev->day)
> +		return OPAL_SUCCESS;
> +	if (timestamp->day < prev->day)
> +		return OPAL_PERMISSION;
> +
> +	if (timestamp->hour > prev->hour)
> +		return OPAL_SUCCESS;
> +	if (timestamp->hour < prev->hour)
> +		return OPAL_PERMISSION;
> +
> +	if (timestamp->minute > prev->minute)
> +		return OPAL_SUCCESS;
> +	if (timestamp->minute < prev->minute)
> +		return OPAL_PERMISSION;
> +
> +	if (timestamp->second > prev->second)
> +		return OPAL_SUCCESS;
> +
> +	/* Time less than or equal to is considered as replay*/
> +	if (timestamp->second <= prev->second)
> +		return OPAL_PERMISSION;

You could simplify this a lot by unpacking the timestamp bytes into a
uint64 with the following format: 0xYYYYMMDDHHMMSS, then the two can be
compared with one numeric test.

> +	/* nanosecond, timezone, daylight and pad2 are meant to be zero */

I'd verify that, but that's just me.

> +
> +	return OPAL_SUCCESS;
> +}
> +

> +/* Extract PKCS7 from the authentication header */
> +static int get_pkcs7(struct efi_variable_authentication_2 *auth,
> +		     mbedtls_pkcs7 **pkcs7)
auth should be const

> +{
> +	char *checkpkcs7cert = NULL;
> +	int len;
> +	int rc;
> +
> +	len = get_pkcs7_len(auth);
> +	if (len <= 0)
> +		return OPAL_PARAMETER;
> +
> +	if (!pkcs7)
> +		return OPAL_PARAMETER;
> +
> +	*pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
> +	if (!(*pkcs7))
> +		return OPAL_NO_MEM;
> +
> +	mbedtls_pkcs7_init(*pkcs7);
> +	rc = mbedtls_pkcs7_parse_der(
> +			(const unsigned char *)auth->auth_info.cert_data,
> +			(const unsigned int)len, *pkcs7);
Why are those casts necessary?

> +	if (rc) {
> +		prlog(PR_ERR, "Parsing pkcs7 failed %04x\n", rc);
> +		mbedtls_pkcs7_free(*pkcs7);
> +		return rc;
> +	}
> +
> +	checkpkcs7cert = zalloc(CERT_BUFFER_SIZE);
> +	if (!checkpkcs7cert) {
> +		mbedtls_pkcs7_free(*pkcs7);
> +		return OPAL_NO_MEM;
> +	}
> +
> +	rc = mbedtls_x509_crt_info(checkpkcs7cert, CERT_BUFFER_SIZE, "CRT:",
> +			&((*pkcs7)->signed_data.certs));
> +	if (rc < 0) {
> +		prlog(PR_ERR, "Failed to parse the certificate in PKCS7 structure\n");
> +		rc = OPAL_PARAMETER;
> +	} else {
> +		rc = OPAL_SUCCESS;
> +		prlog(PR_DEBUG, "%s \n", checkpkcs7cert);
> +	}
> +
> +	free(checkpkcs7cert);
> +	mbedtls_pkcs7_free(*pkcs7);
> +
> +	return rc;
> +}
> +
> +/* Verify the PKCS7 signature on the signed data. */
> +static int verify_signature(struct efi_variable_authentication_2 *auth,
> +			    char *newcert, size_t new_data_size,
> +			    struct secvar *avar)
const where necessary

> +{
> +	mbedtls_pkcs7 *pkcs7 = NULL;
> +	mbedtls_x509_crt x509;
> +	char *signing_cert = NULL;
> +	char *x509_buf = NULL;
> +	int signing_cert_size;
> +	int rc;
> +	char *errbuf;
> +	int eslvarsize;
> +	int eslsize;
> +	int offset = 0;
> +
> +	if (!auth)
> +		return OPAL_PARAMETER;
> +
> +	/* Extract the pkcs7 from the auth structure */
> +	rc  = get_pkcs7(auth, &pkcs7);
> +	/* Failure to parse pkcs7 implies bad input. */
> +	if (rc != OPAL_SUCCESS)
> +		return OPAL_PARAMETER;
> +
> +	prlog(PR_INFO, "Load the signing certificate from the keystore");
> +
> +	eslvarsize = avar->data_size;
> +
> +	/* Variable is not empty */
> +	while (eslvarsize > 0) {
> +		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
> +		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
> +			break;
> +
> +		/* Calculate the size of the ESL */
> +		eslsize = get_esl_signature_list_size(avar->data + offset,
> +						      eslvarsize);
> +		/* If could not extract the size */
> +		if (eslsize <= 0) {
> +			rc = OPAL_PARAMETER;
> +			break;
> +		}
> +
> +		/* Extract the certificate from the ESL */
> +		signing_cert_size = get_esl_cert(avar->data + offset,
> +						 eslvarsize, &signing_cert);
> +		if (signing_cert_size < 0) {
> +			rc = signing_cert_size;
> +			break;
> +		}
> +
> +		mbedtls_x509_crt_init(&x509);
> +		rc = mbedtls_x509_crt_parse(&x509,
> +					    signing_cert,
> +					    signing_cert_size);
> +
> +		/* This should not happen, unless something corrupted in PNOR */
> +		if(rc) {
> +			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
> +			rc = OPAL_INTERNAL_ERROR;
> +			break;
> +		}
> +
> +		x509_buf = zalloc(CERT_BUFFER_SIZE);
> +		rc = mbedtls_x509_crt_info(x509_buf,
> +					   CERT_BUFFER_SIZE,
> +					   "CRT:",
> +					   &x509);
> +
> +		/* This should not happen, unless something corrupted in PNOR */
> +		if (rc < 0) {
> +			free(x509_buf);
> +			rc = OPAL_INTERNAL_ERROR;
> +			break;
> +		}
> +
> +		prlog(PR_INFO, "%s \n", x509_buf);
> +		free(x509_buf);
> +		x509_buf = NULL;
> +
> +		/* Verify the signature */
> +		rc = mbedtls_pkcs7_signed_data_verify(pkcs7, &x509, newcert,
> +						      new_data_size);
> +
> +		/* If you find a signing certificate, you are done */
> +		if (rc == 0) {
> +			prlog(PR_INFO, "Signature Verification passed\n");
> +			mbedtls_x509_crt_free(&x509);
> +			break;
> +		}
> +
> +		errbuf = zalloc(MBEDTLS_ERR_BUFFER_SIZE);
> +		mbedtls_strerror(rc, errbuf, MBEDTLS_ERR_BUFFER_SIZE);
> +		prlog(PR_INFO, "Signature Verification failed %02x %s\n",
> +				rc, errbuf);
> +		free(errbuf);
> +
> +		/* Look for the next ESL */
> +		offset = offset + eslsize;
> +		eslvarsize = eslvarsize - eslsize;
> +		mbedtls_x509_crt_free(&x509);
> +		free(signing_cert);
> +		/* Since we are going to allocate again in the next iteration */
> +		signing_cert = NULL;
> +
> +	}
> +
> +	free(signing_cert);
> +	mbedtls_pkcs7_free(pkcs7);
> +	free(pkcs7);
> +
> +	return rc;
> +}
> +
> +/* Create the single buffer
> + * name || vendor guid || attributes || timestamp || newcontent
> + * which is submitted as signed by the user.
> + * Returns number of bytes in the new buffer, else negative error
> + * code.
> + */

I'd be happier if you incrementally computed the hash on each field
rather than copying everything constantly.

> +static int get_data_to_verify(char *key, char *new_data, size_t new_data_size,
> +			      char **buffer, size_t *buffer_size,
> +			      struct efi_time *timestamp)
consts
> +{
> +	le32 attr = cpu_to_le32(SECVAR_ATTRIBUTES);
> +	size_t offset = 0;
> +	size_t varlen;
> +	char *wkey;
> +	uuid_t guid;
> +
> +	if (key_equals(key, "PK")
> +	    || key_equals(key, "KEK"))
> +		guid = EFI_GLOBAL_VARIABLE_GUID;
> +	else if (key_equals(key, "db")
> +	    || key_equals(key, "dbx"))
> +		guid = EFI_IMAGE_SECURITY_DATABASE_GUID;
> +	else
> +		return OPAL_INTERNAL_ERROR;
> +
> +	/* Convert utf8 name to ucs2 width */
> +	varlen = strlen(key) * 2;
> +	wkey = utf8_to_ucs2(key, strlen(key));
> +
> +	/* Prepare the single buffer */
> +	*buffer_size = varlen + UUID_SIZE + sizeof(attr)
> +		       + sizeof(struct efi_time) + new_data_size;
> +	*buffer = zalloc(*buffer_size);
> +	if (!*buffer)
> +		return OPAL_NO_MEM;
> +
> +	memcpy(*buffer + offset, wkey, varlen);
> +	offset = offset + varlen;
> +	memcpy(*buffer + offset, &guid, sizeof(guid));
> +	offset = offset + sizeof(guid);
> +	memcpy(*buffer + offset, &attr, sizeof(attr));
> +	offset = offset + sizeof(attr);
> +	memcpy(*buffer + offset, timestamp , sizeof(struct efi_time));
> +	offset = offset + sizeof(struct efi_time);
> +
> +	memcpy(*buffer + offset, new_data, new_data_size);
> +	offset = offset + new_data_size;
> +
> +	free(wkey);
> +
> +	return offset;
> +}
> +

> +bool is_pkcs7_sig_format(void *data)
const
> +{
> +	struct efi_variable_authentication_2 *auth = data;
> +	uuid_t pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID;
> +
> +	if(!(memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) == 0))
> +		return false;
> +	return true;

That's one way to do it. Alternatively:

	if (memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) != 0)
		return false;
	return true;

Or:
	if (memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16))
		return false;
	return true;

Or even:

	return !memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16);


> +int process_update(struct secvar_node *update, char **newesl,
> +		   int *new_data_size, struct efi_time *timestamp,
> +		   struct list_head *bank, char *last_timestamp)
> +{
> +	struct efi_variable_authentication_2 *auth = NULL;
> +	char *auth_buffer = NULL;

If you want a pointer to an arbitrary buffer use a void pointer rather
than a char. Voids will automaticly coerce to another pointer type so
you don't have to manually cast it. Also, this should be const.

> +	int auth_buffer_size = 0;
> +	const char *key_authority[3];
> +	char *tbhbuffer = NULL;
> +	size_t tbhbuffersize = 0;
> +	struct secvar_node *anode = NULL;
> +	int rc = 0;
> +	int i;
> +
> +	auth_buffer_size = get_auth_descriptor2(update->var->data,
> +						update->var->data_size,
> +						&auth_buffer);
> +	if ((auth_buffer_size < 0)
> +	     || (update->var->data_size < auth_buffer_size)) {
> +		prlog(PR_ERR, "Invalid auth buffer size\n");
> +		rc = auth_buffer_size;
> +		goto out;
> +	}
> +
> +	auth = (struct efi_variable_authentication_2 *)auth_buffer;
> +
> +	if (!timestamp) {
> +		rc = OPAL_INTERNAL_ERROR;
> +		goto out;
> +	}
> +
> +	memcpy(timestamp, auth_buffer, sizeof(struct efi_time));
*timestamp = auth->timestamp?

> +
> +	rc = check_timestamp(update->var->key, timestamp, last_timestamp);
> +	/* Failure implies probably an older command being resubmitted */
> +	if (rc != OPAL_SUCCESS) {
> +		prlog(PR_INFO, "Timestamp verification failed for key %s\n", update->var->key);
> +		goto out;
> +	}
> +
> +	/* Calculate the size of new ESL data */
> +	*new_data_size = update->var->data_size - auth_buffer_size;
> +	if (*new_data_size < 0) {
> +		prlog(PR_ERR, "Invalid new ESL (new data content) size\n");
> +		rc = OPAL_PARAMETER;
> +		goto out;
> +	}
> +	*newesl = zalloc(*new_data_size);
> +	if (!(*newesl)) {
> +		rc = OPAL_NO_MEM;
> +		goto out;
> +	}
> +	memcpy(*newesl, update->var->data + auth_buffer_size, *new_data_size);
> +
> +	/* Validate the new ESL is in right format */
> +	rc = validate_esl_list(update->var->key, *newesl, *new_data_size);
> +	if (rc < 0) {
> +		prlog(PR_ERR, "ESL validation failed for key %s with error %04x\n",
> +		      update->var->key, rc);
> +		goto out;
> +	}
> +
> +	if (setup_mode) {
> +		rc = OPAL_SUCCESS;
> +		goto out;
> +	}
> +
> +	/* Prepare the data to be verified */
> +	rc = get_data_to_verify(update->var->key, *newesl, *new_data_size,
> +				&tbhbuffer, &tbhbuffersize, timestamp);
> +
> +	/* Get the authority to verify the signature */
> +	get_key_authority(key_authority, update->var->key);

> +	i = 0;
> +
> +	/* Try for all the authorities that are allowed to sign.
> +	 * For eg. db/dbx can be signed by both PK or KEK
> +	 */
> +	while (key_authority[i] != NULL) {
use a for loop

> +		prlog(PR_DEBUG, "key is %s\n", update->var->key);
> +		prlog(PR_DEBUG, "key authority is %s\n", key_authority[i]);
> +		anode = find_secvar(key_authority[i],
> +				    strlen(key_authority[i]) + 1,
> +				    bank);
considering most of the calls to find_secvar() use a string literal and
hardcode the key length you could clean things up by moving the
strlen() into find_secvar() so the prototype is just find_secvar(key,
bank).

For the few cases that need to provide the explicit length they can use
a helper that allows an explicit length to be passed in.

> +		if (!anode || !anode->var->data_size) {
> +			i++;
> +			continue;
> +		}
> +
> +		/* Verify the signature */
> +		rc = verify_signature(auth, tbhbuffer, tbhbuffersize,
> +				      anode->var);
> +
> +		/* Break if signature verification is successful */
> +		if (rc == OPAL_SUCCESS)
> +			break;
> +		i++;
> +	}

> +
> +out:
> +	free(auth_buffer);
> +	free(tbhbuffer);

So... are either of these copies actually modified? Just return const
pointer to the input buffer.

> +
> +	return rc;
> +}



diff --git a/libstb/secvar/backend/edk2-compat.c
b/libstb/secvar/backend/edk2-compat.c
> new file mode 100644
> index 00000000..334aca3e
> --- /dev/null
> +++ b/libstb/secvar/backend/edk2-compat.c
> @@ -0,0 +1,262 @@
> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
> +/* Copyright 2020 IBM Corp. */
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
> +#endif
> +
> +#include <opal.h>
> +#include <string.h>
> +#include <time.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <skiboot.h>
> +#include <ccan/endian/endian.h>
> +#include <mbedtls/error.h>
> +#include "libstb/crypto/pkcs7/pkcs7.h"
> +#include "edk2.h"
> +#include "../secvar.h"
> +#include "edk2-compat-process.h"
> +#include "edk2-compat-reset.h"
> +
> +struct list_head staging_bank;
> +
> +/*
> + * Initializes supported variables as empty if not loaded from
> + * storage. Variables are initialized as volatile if not found.
> + * Updates should clear this flag.
> + * Returns OPAL Error if anything fails in initialization
> + */
> +static int edk2_compat_pre_process(struct list_head *variable_bank,
> +				   struct list_head *update_bank __unused)
> +{
> +	struct secvar_node *pkvar;
> +	struct secvar_node *kekvar;
> +	struct secvar_node *dbvar;
> +	struct secvar_node *dbxvar;
> +	struct secvar_node *tsvar;
> +
> +	pkvar = find_secvar("PK", 3, variable_bank);
> +	if (!pkvar) {
> +		pkvar = new_secvar("PK", 3, NULL, 0, SECVAR_FLAG_VOLATILE
> +				| SECVAR_FLAG_PROTECTED);
> +		if (!pkvar)
> +			return OPAL_NO_MEM;
> +
> +		list_add_tail(variable_bank, &pkvar->link);
> +	}
> +	if (pkvar->var->data_size == 0)
> +		setup_mode = true;
> +	else
> +		setup_mode = false;
> +
> +	kekvar = find_secvar("KEK", 4, variable_bank);
> +	if (!kekvar) {
> +		kekvar = new_secvar("KEK", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
> +		if (!kekvar)
> +			return OPAL_NO_MEM;
> +
> +		list_add_tail(variable_bank, &kekvar->link);
> +	}
> +
> +	dbvar = find_secvar("db", 3, variable_bank);
> +	if (!dbvar) {
> +		dbvar = new_secvar("db", 3, NULL, 0, SECVAR_FLAG_VOLATILE);
> +		if (!dbvar)
> +			return OPAL_NO_MEM;
> +
> +		list_add_tail(variable_bank, &dbvar->link);
> +	}
> +
> +	dbxvar = find_secvar("dbx", 4, variable_bank);
> +	if (!dbxvar) {
> +		dbxvar = new_secvar("dbx", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
> +		if (!dbxvar)
> +			return OPAL_NO_MEM;
> +
> +		list_add_tail(variable_bank, &dbxvar->link);
> +	}
> +
> +	/* Should only ever happen on first boot. Timestamp is
> +	 * initialized with all zeroes. */
> +	tsvar = find_secvar("TS", 3, variable_bank);
> +	if (!tsvar) {
> +		tsvar = alloc_secvar(sizeof(struct efi_time) * 4);
> +		if (!tsvar)
> +			return OPAL_NO_MEM;
> +
> +		memcpy(tsvar->var->key, "TS", 3);
> +		tsvar->var->key_len = 3;
> +		tsvar->var->data_size = sizeof(struct efi_time) * 4;
> +		memset(tsvar->var->data, 0, tsvar->var->data_size);
> +		list_add_tail(variable_bank, &tsvar->link);
> +	}
> +
> +	return OPAL_SUCCESS;
> +};

What's the point of these null variables? If you're doing it to ensure
that find_secvar() or whatever doesn't return null pointers it's not
really buying you anything since you still have to check for zero
length data.

> +
> +static int edk2_compat_process(struct list_head *variable_bank,
> +			       struct list_head *update_bank)
> +{
> +	struct secvar_node *node = NULL;
> +	struct secvar_node *tsvar = NULL;
> +	struct efi_time timestamp;
> +	char *newesl = NULL;
> +	int neweslsize;
> +	int rc = 0;
> +
> +	prlog(PR_INFO, "Setup mode = %d\n", setup_mode);
> +
> +	/* Check HW-KEY-HASH */
> +	if (!setup_mode) {
> +		rc = verify_hw_key_hash();
> +		if (rc != OPAL_SUCCESS) {
> +			prlog(PR_ERR, "Hardware key hash verification mismatch\n");
> +			rc = reset_keystore(variable_bank);
> +			if (rc)
> +				goto cleanup;
> +			setup_mode = true;
> +			goto cleanup;
> +		}
> +	}
> +
> +	/* Return early if we have no updates to process */
> +	if (list_empty(update_bank)) {
> +		return OPAL_EMPTY;
> +	}

> +
> +	/* Make a working copy of variable bank that is updated
> +	 * during process */
> +	list_head_init(&staging_bank);
> +	copy_bank_list(&staging_bank, variable_bank);
> +
> +	/* Loop through each command in the update bank.
> +	 * If any command fails, it just loops out of the update bank.
> +	 * It should also clear the update bank.
> +	 */
> +
> +	list_for_each(update_bank, node, link) {
> +
> +		/* Submitted data is auth_2 descriptor + new ESL data
> +		 * Extract the auth_2 2 descriptor
> +		 */
> +		prlog(PR_INFO, "Update for %s\n", node->var->key);
> +
> +		tsvar = find_secvar("TS", 3, &staging_bank);
Is looking this up each loop iteration necessary?

> +
> +		/* We cannot find timestamp variable, did someone tamper it? */
> +	        if (!tsvar) {
> +			rc = OPAL_PERMISSION;
> +			break;
> +		}
> +
> +		rc = process_update(node, &newesl,
> +				    &neweslsize, &timestamp,
> +				    &staging_bank,
> +				    tsvar->var->data);
> +		if (rc) {
> +			prlog(PR_ERR, "Update processing failed with rc %04x\n", rc);
> +			break;
> +		}
> +
> +		/*
> +		 * If reached here means, signature is verified so update the
> +		 * value in the variable bank
> +		 */
> +		rc = update_variable_in_bank(node->var,
> +					     newesl,
> +					     neweslsize,
> +					     &staging_bank);
> +		if (rc) {
> +			prlog(PR_ERR, "Updating the variable data failed %04x\n", rc);
> +			break;
> +		}
> +
> +		free(newesl);
> +		/* Update the TS variable with the new timestamp */
> +		rc = update_timestamp(node->var->key,
> +				      &timestamp,
> +				      tsvar->var->data);

Is having all the timestamps in a single secvar an actual requirement
or is that just how you've decided to implement the timestamp tracking?

Seem like it'd be easier all around if you tracked it on a per-variable 
basis and stored it as a part of the serialised data format rather than
wedging them together into a variable. Wedging them also creates the
awkward data flow we're seeing here where process_update() needs to
return the timestamp via out pointers.

> +		if (rc) {
> +			prlog (PR_ERR, "Variable updated, but timestamp updated failed %04x\n", rc);
> +			break;
> +		}
> +
> +		/* If the PK is updated, update the secure boot state of the
> +		 * system at the end of processing */
> +		if (key_equals(node->var->key, "PK")) {
> +			/* PK is tied to a particular firmware image by mapping
> +			 * it with hw-key-hash of that firmware. When PK is
> +			 * updated, hw-key-hash is updated. And when PK is
> +			 * deleted, delete hw-key-hash as well */
> +			if(neweslsize == 0) {
> +				setup_mode = true;
> +				delete_hw_key_hash(&staging_bank);
> +			} else  {
> +				setup_mode = false;
> +				add_hw_key_hash(&staging_bank);
> +			}
> +			prlog(PR_DEBUG, "setup mode is %d\n", setup_mode);
> +		}
> +	}
> +
> +	if (rc == 0) {
> +		/* Update the variable bank with updated working copy */
> +		clear_bank_list(variable_bank);
> +		copy_bank_list(variable_bank, &staging_bank);

Isn't this leaking all the variables in staging bank?

> +	}
> +
> +cleanup:
> +	/* For any failure in processing update queue, we clear the update bank
> +	 * and return failure */
> +	clear_bank_list(update_bank);
> +
> +	return rc;
> +}
> +
> +static int edk2_compat_post_process(struct list_head *variable_bank,
> +				    struct list_head *update_bank __unused)
> +{
> +	struct secvar_node *hwvar;
> +	if (!setup_mode) {
> +		secvar_set_secure_mode();
> +		prlog(PR_INFO, "Enforcing OS secure mode\n");
> +		/* HW KEY HASH is no more needed after this point. It is already
> +		 * visible to userspace via device-tree, so exposing via sysfs is
> +		 * just a duplication. Remove it from in-memory copy. */
> +		hwvar = find_secvar("HWKH", 5, variable_bank);
> +		if (!hwvar) {
> +			prlog(PR_ERR, "cannot find hw-key-hash, should not happen\n");
> +			return OPAL_INTERNAL_ERROR;
> +		}
> +		list_del(&hwvar->link);
> +		dealloc_secvar(hwvar);
> +	}
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +static int edk2_compat_validate(struct secvar *var)
> +{
> +
> +	/* Checks if the update is for supported
> +	 * Non-volatile secure variables */
> +	if (!key_equals(var->key, "PK")
> +			&& !key_equals(var->key, "KEK")
> +			&& !key_equals(var->key, "db")
> +			&& !key_equals(var->key, "dbx"))
> +		return OPAL_PARAMETER;

Being unable to support user variables is a bit crap. oh well.

> +	/* Check that signature type is PKCS7 */
> +	if (!is_pkcs7_sig_format(var->data))
> +		return OPAL_PARAMETER;
> +
> +	return OPAL_SUCCESS;
> +};
> +
> +struct secvar_backend_driver edk2_compatible_v1 = {
> +	.pre_process = edk2_compat_pre_process,
> +	.process = edk2_compat_process,
> +	.post_process = edk2_compat_post_process,
> +	.validate = edk2_compat_validate,
> +	.compatible = "ibm,edk2-compat-v1",
> +};

> diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h
> new file mode 100644
> index 00000000..fb0121af
> --- /dev/null
> +++ b/libstb/secvar/backend/edk2.h
> *snip*
> +/*
> + * Certificate which encapsulates a GUID-specific digital signature
> + */
> +struct win_certificate_uefi_guid {
> +	/*
> +	 * This is the standard win_certificate header, where w_certificate_type
> +	 * is set to WIN_CERT_TYPE_EFI_GUID.
> +	 */
> +	struct win_certificate hdr;
> +	/*
> +	 * This is the unique id which determines the format of the cert_data.
> +	 */
> +	uuid_t cert_type;
> +	/*
> +	 * The following is the certificate data. The format of the data is
> +	 * determined by the @cert_type. If @cert_type is
> +	 * EFI_CERT_TYPE_RSA2048_SHA256_GUID, the @cert_data will be
> +	 * EFI_CERT_BLOCK_RSA_2048_SHA256 structure.
> +	 */
> +	u8 cert_data[1];

I'm guessing this is TianoCore-ism, but using a length-one array at the
end of a struct to indicate trailing data is something that went out of
fashion in the 90s. GCC has an extension to allow zero-length arrays at
the end of a struct for this purpose that was added decades ago. That
extension was deprecated by C99 which added a no-length array syntax
for the same purpose:

struct a {
	int start;
	char buffer[];
};

I'd suggest you use that since it allows you to do a sizeof() on the
struct variable to get the size of the structure header without the
trailing element screwing with it.
Nayna May 23, 2020, 1:47 a.m. UTC | #2
On 5/14/20 10:11 AM, Oliver O'Halloran wrote:
> On Mon, 2020-05-11 at 16:31 -0500, Eric Richter wrote:
>> From: Nayna Jain <nayna@linux.ibm.com>
>>
>> As part of secureboot key management, the scheme for handling key updates
>> is derived from tianocore reference implementation[1]. The wrappers for
>> holding the signed update is the Authentication Header and for holding
>> the public key certificate is ESL (EFI Signature List), both derived from
>> tianocore reference implementation[1].
>>
>> This patch adds the support to process update queue. This involves:
>> 1. Verification of the update signature using the key authorized as per the
>> key hierarchy
>> 2. Handling addition/deletion of the keys
>> 3. Support for dbx (blacklisting of hashes)
>> 4. Validation checks for the updates
>> 5. Supporting multiple ESLs for single variable both for update/verification
>> 6. Timestamp check
>> 7. Allowing only single PK
>> 8. Failure Handling
>> 9. Resetting keystore if the hardware key hash changes
>>
>> [1] https://github.com/tianocore/edk2-staging.git
>>
>> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> Signed-off-by: Eric Richter <erichte@linux.ibm.com>
>> ---
>> V4:
>>   - fixed a typo in the hw-key-hash mismatch prlog
>>   - replace PRIORITY with PROTECTED
>>   - adjust most lines to 80 columns, some exceptions and prlog statements
>>       may still run over
>>
>>   doc/secvar/edk2.rst                         |  49 ++
>>   include/secvar.h                            |   1 +
>>   libstb/secvar/backend/Makefile.inc          |   4 +-
>>   libstb/secvar/backend/edk2-compat-process.c | 717 ++++++++++++++++++++
>>   libstb/secvar/backend/edk2-compat-process.h |  61 ++
>>   libstb/secvar/backend/edk2-compat-reset.c   | 115 ++++
>>   libstb/secvar/backend/edk2-compat-reset.h   |  24 +
>>   libstb/secvar/backend/edk2-compat.c         | 262 +++++++
>>   libstb/secvar/backend/edk2.h                | 243 +++++++
>>   9 files changed, 1474 insertions(+), 2 deletions(-)
>>   create mode 100644 doc/secvar/edk2.rst
>>   create mode 100644 libstb/secvar/backend/edk2-compat-process.c
>>   create mode 100644 libstb/secvar/backend/edk2-compat-process.h
>>   create mode 100644 libstb/secvar/backend/edk2-compat-reset.c
>>   create mode 100644 libstb/secvar/backend/edk2-compat-reset.h
>>   create mode 100644 libstb/secvar/backend/edk2-compat.c
>>   create mode 100644 libstb/secvar/backend/edk2.h
>>
>> diff --git a/doc/secvar/edk2.rst b/doc/secvar/edk2.rst
>> new file mode 100644
>> index 00000000..1e4cc9e3
>> --- /dev/null
>> +++ b/doc/secvar/edk2.rst
>> @@ -0,0 +1,49 @@
>> +.. _secvar/edk2:
>> +
>> +Skiboot edk2-compatible Secure Variable Backend
>> +===============================================
>> +
>> +Overview
>> +--------
>> +
>> +The edk2 secure variable backend for skiboot borrows from edk2 concepts
>> +such as the three key hierarchy (PK, KEK, and db), and a similar
>> +structure. In general, variable updates must be signed with a key
>> +of a higher level. So, updates to the db must be signed with a key stored
>> +in the KEK; updates to the KEK must be signed with the PK. Updates to the
>> +PK must be signed with the previous PK (if any).
>> +
>> +Variables are stored in the efi signature list format, and updates are a
>> +signed variant that includes an authentication header.
>> +
>> +If no PK is currently enrolled, the system is considered to be in "Setup
>> +Mode". Any key can be enrolled without signature checks. However, once a
>> +PK is enrolled, the system switches to "User Mode", and each update must
>> +now be signed according to the hierarchy. Furthermore, when in "User
>> +Mode", the backend initialized the ``os-secure-mode`` device tree flag,
>> +signaling to the kernel that we are in secure mode.
>> +
>> +Updates are processed sequentially, in the order that they were provided
>> +in the update queue. If any update fails to validate, appears to be
>> +malformed, or any other error occurs, NO updates will not be applied.
>> +This includes updates that may have successfully applied prior to the
>> +error. The system will continue in an error state, reporting the error
>> +reason via the ``update-status`` device tree property.
>> +
>> +P9 Special Case for the Platform Key
>> +------------------------------------
>> +
>> +Due to the powerful nature of the platform key and the lack of lockable
>> +flash, the edk2 backend will store the PK in TPM NV rather than PNOR on
>> +P9 systems. (TODO expand on this)
>> +
>> +Update Status Return Codes
>> +--------------------------
>> +
>> +TODO, edk2 driver needs to actually return these properly first
>> +
>> +
>> +Device Tree Bindings
>> +--------------------
>> +
>> +TODO
>> diff --git a/include/secvar.h b/include/secvar.h
>> index 7a45db2b..4f40c115 100644
>> --- a/include/secvar.h
>> +++ b/include/secvar.h
>> @@ -28,6 +28,7 @@ struct secvar_backend_driver {
>>   };
>>   
>>   extern struct secvar_storage_driver secboot_tpm_driver;
>> +extern struct secvar_backend_driver edk2_compatible_v1;
>>   
>>   int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
>>   
>> diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
>> index 6f491a63..bc987f69 100644
>> --- a/libstb/secvar/backend/Makefile.inc
>> +++ b/libstb/secvar/backend/Makefile.inc
>> @@ -1,11 +1,11 @@
>>   # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
>>   # -*-Makefile-*-
>>   
>> -SECVAR_BACKEND_DIR = libstb/secvar/backend
>> +SECVAR_BACKEND_DIR = $(SRC)/libstb/secvar/backend
>>   
>>   SUBDIRS += $(SECVAR_BACKEND_DIR)
>>   
>> -SECVAR_BACKEND_SRCS =
>> +SECVAR_BACKEND_SRCS = edk2-compat.c edk2-compat-process.c edk2-compat-reset.c
>>   SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
>>   SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
>>   
>> diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
>> new file mode 100644
>> index 00000000..60ebb0b2
>> --- /dev/null
>> +++ b/libstb/secvar/backend/edk2-compat-process.c
>> @@ -0,0 +1,717 @@
>> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
>> +/* Copyright 2020 IBM Corp. */
>> +#ifndef pr_fmt
>> +#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
>> +#endif
>> +
>> +#include <opal.h>
>> +#include <string.h>
>> +#include <time.h>
>> +#include <unistd.h>
>> +#include <stdint.h>
>> +#include <ccan/endian/endian.h>
>> +#include <mbedtls/error.h>
>> +#include <device.h>
>> +#include "libstb/crypto/pkcs7/pkcs7.h"
>> +#include "edk2.h"
>> +#include "../secvar.h"
>> +#include "edk2-compat-process.h"
>> +
>> +bool setup_mode;
>> +
>> +int update_variable_in_bank(struct secvar *secvar, const char *data,
>> +			    uint64_t dsize, struct list_head *bank)
>> +{
>> +	struct secvar_node *node;
>> +
>> +	node = find_secvar(secvar->key, secvar->key_len, bank);
>> +	if (!node)
>> +		return OPAL_EMPTY;
>> +
>> +        /* Reallocate the data memory, if there is change in data size */
>> +	if (node->size < dsize)
>> +		if (realloc_secvar(node, dsize))
>> +			return OPAL_NO_MEM;
>> +
>> +	if (dsize && data)
>> +		memcpy(node->var->data, data, dsize);
>> +	node->var->data_size = dsize;
>> +
>> +        /* Clear the volatile bit only if updated with positive data size */
>> +	if (dsize)
>> +		node->flags &= ~SECVAR_FLAG_VOLATILE;
>> +	else
>> +		node->flags |= SECVAR_FLAG_VOLATILE;
>> +
>> +        /* Is it required to be set everytime ? */
> you tell me
>
>> +	if ((!strncmp(secvar->key, "PK", 3))
>> +	     || (!strncmp(secvar->key, "HWKH", 5)))
>> +		node->flags |= SECVAR_FLAG_PROTECTED;
>> +
>> +	return 0;
>> +}
>> +
>> +/* Converts utf8 string to ucs2 */
>> +static char *utf8_to_ucs2(const char *key, size_t keylen)
>> +{
>> +	int i;
>> +	char *str;
>> +
>> +	str = zalloc(keylen * 2);
>> +	if (!str)
>> +		return NULL;
>> +
>> +	for (i = 0; i < keylen*2; key++) {
>> +		str[i++] = *key;
>> +		str[i++] = '\0';
>> +	}
>> +
>> +	return str;
>> +}
> This is completely broken for anything that doesn't fall into the ASCII
> subset of UTF-8.

Yes, it is the minimal implementation to convert ASCII keys into wide 
characters for the purpose of efitools compatibility. Currently, we are 
supporting only ASCII key names and we also do not allow user created 
variables in this version.

>
>> +/* Returns the authority that can sign the given key update */
>> +static void get_key_authority(const char *ret[3], const char *key)
>> +{
>> +	int i = 0;
>> +
>> +	if (key_equals(key, "PK")) {
>> +		ret[i++] = "PK";
>> +	} else if (key_equals(key, "KEK")) {
>> +		ret[i++] = "PK";
>> +	} else if (key_equals(key, "db") || key_equals(key, "dbx")) {
>> +		ret[i++] = "KEK";
>> +		ret[i++] = "PK";
>> +	}
>> +
>> +	ret[i] = NULL;
>> +}
>> +
>> +/* Returns the size of the complete ESL. */
>> +static int get_esl_signature_list_size(char *buf, size_t buflen)
> buf should be const
>> +{
>> +	EFI_SIGNATURE_LIST list;
>> +
>> +	if (buflen < sizeof(EFI_SIGNATURE_LIST))
>> +		return OPAL_PARAMETER;
>> +
>> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
>> +
>> +	prlog(PR_DEBUG, "size of signature list size is %u\n",
>> +			le32_to_cpu(list.SignatureListSize));
>> +
>> +	return le32_to_cpu(list.SignatureListSize);
>> +}
>> +
>> +/* Copies the certificate from the ESL into cert buffer and returns the size
>> + * of the certificate
>> + */
>> +static int get_esl_cert(char *buf, size_t buflen, char **cert)
> buf should be const
>> +{
>> +	size_t sig_data_offset;
>> +	size_t size;
>> +	EFI_SIGNATURE_LIST list;
>> +
>> +	if (buflen < sizeof(EFI_SIGNATURE_LIST))
>> +		return OPAL_PARAMETER;
>> +
>> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
> This looks familiar
>
>> +
>> +	size = le32_to_cpu(list.SignatureSize) - sizeof(uuid_t);
>> +	/* No certificate in the ESL */
>> +	if (size <= 0)
>> +		return OPAL_PERMISSION;
> size_t is unsigned so checking for less than zero doesn't do anything.
> If list.SignatureSize is too small the calculation will underflow and
> probably lead to a memory allocation failure later on.
>
>> +	if (!cert)
>> +		return OPAL_PARAMETER;
> Passing a null pointer here is a programming error, use an assert.
>
>> +	*cert = zalloc(size);
>> +	if (!(*cert))
>> +		return OPAL_NO_MEM;
>> +
>> +	prlog(PR_DEBUG,"size of signature list size is %u\n",
>> +			le32_to_cpu(list.SignatureListSize));
>> +	prlog(PR_DEBUG, "size of signature header size is %u\n",
>> +			le32_to_cpu(list.SignatureHeaderSize));
>> +	prlog(PR_DEBUG, "size of signature size is %u\n",
>> +			le32_to_cpu(list.SignatureSize));
>> +
>> +	sig_data_offset = sizeof(list) + le32_to_cpu(list.SignatureHeaderSize)
>> +		+ 16 * sizeof(uint8_t);
>> +	if (sig_data_offset > buflen) {
>> +		free(*cert);
>> +		return OPAL_PARAMETER;
>> +	}
> The free() here is only required because you're performing the
> allocation before verifying that the input buffer is long enough.
> Reverse the order, it's less code and less error prone since you don't
> have to unwind the allocation.
>
>> +	memcpy(*cert, buf + sig_data_offset, size);
>> +
>> +	return size;
>> +}
>> +
>> +/* Extracts size of the PKCS7 signed data embedded in the
>> + * struct Authentication 2 Descriptor Header.
>> + */
>> +static int get_pkcs7_len(struct efi_variable_authentication_2 *auth)
> auth should be const
>
>> +{
>> +	uint32_t dw_length;
>> +	size_t size;
>> +
>> +	if (!auth)
>> +		return OPAL_PARAMETER;
> assert
>
>> +
>> +	dw_length = le32_to_cpu(auth->auth_info.hdr.dw_length);
>> +	size = dw_length - (sizeof(auth->auth_info.hdr.dw_length)
>> +			+ sizeof(auth->auth_info.hdr.w_revision)
>> +			+ sizeof(auth->auth_info.hdr.w_certificate_type)
>> +			+ sizeof(auth->auth_info.cert_type));
>> +
>> +	return size;
>> +}
>> +
>> +int get_auth_descriptor2(void *buf, size_t buflen, char **auth_buffer)
> buf should be const
>
>> +{
>> +	struct efi_variable_authentication_2 *auth = NULL;
>> +	size_t auth_buffer_size;
>> +	int len;
>> +
>> +	if (buflen < sizeof(struct efi_variable_authentication_2))
>> +			return OPAL_PARAMETER;
>> +
>> +	auth = buf;
> You can initalise auth to buf in the declaration above.
>
>> +
>> +	len = get_pkcs7_len(auth);
>> +
>> +	/* We need PKCS7 data else there is no signature */
>> +	if (len <= 0)
>> +		return OPAL_PARAMETER;
>> +
>> +	if (!auth_buffer)
>> +		return OPAL_PARAMETER;
> should be an assert. Also, do your parameter validation before you
> start doing any actual work.
>
>> +
>> +	auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
>> +			   + sizeof(auth->auth_info.cert_type) + len;
>> +
>> +	if (auth_buffer_size <= 0)
>> +		return OPAL_PARAMETER;
>> +
>> +	*auth_buffer = zalloc(auth_buffer_size);
>> +	if (!(*auth_buffer))
>> +		return OPAL_NO_MEM;
>> +
>> +	memcpy(*auth_buffer, buf, auth_buffer_size);
> why are we making a copy of this?
>
>> +
>> +	return auth_buffer_size;
>> +}
>> +
>> +int validate_esl_list(char *key, char *esl, size_t size)
>> +{
>> +	int count = 0;
>> +	int signing_cert_size;
>> +	char *signing_cert = NULL;
>> +	mbedtls_x509_crt x509;
>> +	char *x509_buf = NULL;
>> +	int eslvarsize = size;
>> +	int rc = OPAL_SUCCESS;
>> +	int eslsize;
>> +	int offset = 0;
>> +
>> +	while (eslvarsize > 0) {
>> +		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
>> +		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
>> +			break;
>> +
>> +		/* Calculate the size of the ESL */
>> +		eslsize = get_esl_signature_list_size(esl, eslvarsize);
>> +		/* If could not extract the size */
>> +		if (eslsize <= 0) {
>> +			prlog(PR_ERR, "Invalid size of the ESL\n");
>> +			rc = OPAL_PARAMETER;
>> +			break;
>> +		}
>> +
>> +		/* Extract the certificate from the ESL */
>> +		signing_cert_size = get_esl_cert(esl,
>> +						 eslvarsize,
>> +						 &signing_cert);
>> +		if (signing_cert_size < 0) {
>> +			rc = signing_cert_size;
>> +			break;
>> +		}
>> +
>> +		mbedtls_x509_crt_init(&x509);
>> +		rc = mbedtls_x509_crt_parse(&x509,
>> +					    signing_cert,
>> +					    signing_cert_size);
>> +
>> +		/* If failure in parsing the certificate, exit */
>> +		if(rc) {
>> +			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
>> +			rc = OPAL_PARAMETER;
>> +			break;
>> +		}
>> +
>> +		x509_buf = zalloc(CERT_BUFFER_SIZE);
>> +		rc = mbedtls_x509_crt_info(x509_buf,
>> +					   CERT_BUFFER_SIZE,
>> +					   "CRT:",
>> +					   &x509);
>> +		prlog(PR_INFO, "%s ", x509_buf);
>> +
>> +		/* If failure in reading the certificate, exit */
>> +		if (rc < 0) {
>> +			prlog(PR_INFO, "Failed to show X509 certificate info %04x\n", rc);
>> +			rc = OPAL_PARAMETER;
>> +			free(x509_buf);
>> +			break;
>> +		}
>> +		rc = 0;
>> +
>> +		free(x509_buf);
>> +		x509_buf = NULL;
>> +		count++;
>> +
>> +		/* Look for the next ESL */
>> +		offset = offset + eslsize;
>> +		eslvarsize = eslvarsize - eslsize;
>> +		mbedtls_x509_crt_free(&x509);
>> +		free(signing_cert);
>> +		/* Since we are going to allocate again in the next iteration */
>> +		signing_cert = NULL;
>> +	}
>> +
>> +	if (rc == OPAL_SUCCESS) {
>> +		if (key_equals(key, "PK") && (count > 1)) {
>> +			prlog(PR_ERR, "PK can only be one\n");
>> +			rc = OPAL_PARAMETER;
>> +		} else {
>> +			rc = count;
>> +		}
>> +	}
>> +
>> +	prlog(PR_INFO, "Total ESLs are %d\n", rc);
>> +	return rc;
>> +}
>> +
>> +/* Get the timestamp for the last update of the give key */
>> +static struct efi_time *get_last_timestamp(const char *key, char *last_timestamp)
>> +{
>> +	u8 off;
>> +
>> +	if (!last_timestamp)
>> +		return NULL;
>> +
>> +	if (!strncmp(key, "PK", 3))
>> +		off = 0;
>> +	else if (!strncmp(key, "KEK", 4))
>> +		off = 1;
>> +	else if (!strncmp(key, "db", 3))
>> +		off = 2;
>> +	else if (!strncmp(key, "dbx", 4))
>> +		off = 3;
>> +	else
>> +		return NULL;
>> +
>> +	return &((struct efi_time *)last_timestamp)[off];
>> +}
> I don't know what you're doing here, but I don't like it.
>
>
>> +int update_timestamp(char *key, struct efi_time *timestamp, char *last_timestamp)
>> +{
>> +	struct efi_time *prev;
>> +
>> +	prev = get_last_timestamp(key, last_timestamp);
>> +	if (prev == NULL)
>> +		return OPAL_INTERNAL_ERROR;
>> +
>> +	memcpy(prev, timestamp, sizeof(struct efi_time));
>> +
>> +	prlog(PR_DEBUG, "updated prev year is %d month %d day %d\n",
>> +			le16_to_cpu(prev->year), prev->month, prev->day);
>> +
>> +	return OPAL_SUCCESS;
>> +}
>> +
>> +int check_timestamp(char *key, struct efi_time *timestamp,
>> +		    char *last_timestamp)
>> +{
>> +	struct efi_time *prev;
>> +
>> +	prev = get_last_timestamp(key, last_timestamp);
>> +	if (prev == NULL)
>> +		return OPAL_INTERNAL_ERROR;
>> +
>> +	prlog(PR_DEBUG, "timestamp year is %d month %d day %d\n",
>> +			le16_to_cpu(timestamp->year), timestamp->month,
>> +			timestamp->day);
>> +	prlog(PR_DEBUG, "prev year is %d month %d day %d\n",
>> +			le16_to_cpu(prev->year), prev->month, prev->day);
>> +	if (le16_to_cpu(timestamp->year) > le16_to_cpu(prev->year))
>> +		return OPAL_SUCCESS;
>> +	if (le16_to_cpu(timestamp->year) < le16_to_cpu(prev->year))
>> +		return OPAL_PERMISSION;
>> +
>> +	if (timestamp->month > prev->month)
>> +		return OPAL_SUCCESS;
>> +	if (timestamp->month < prev->month)
>> +		return OPAL_PERMISSION;
>> +
>> +	if (timestamp->day > prev->day)
>> +		return OPAL_SUCCESS;
>> +	if (timestamp->day < prev->day)
>> +		return OPAL_PERMISSION;
>> +
>> +	if (timestamp->hour > prev->hour)
>> +		return OPAL_SUCCESS;
>> +	if (timestamp->hour < prev->hour)
>> +		return OPAL_PERMISSION;
>> +
>> +	if (timestamp->minute > prev->minute)
>> +		return OPAL_SUCCESS;
>> +	if (timestamp->minute < prev->minute)
>> +		return OPAL_PERMISSION;
>> +
>> +	if (timestamp->second > prev->second)
>> +		return OPAL_SUCCESS;
>> +
>> +	/* Time less than or equal to is considered as replay*/
>> +	if (timestamp->second <= prev->second)
>> +		return OPAL_PERMISSION;
> You could simplify this a lot by unpacking the timestamp bytes into a
> uint64 with the following format: 0xYYYYMMDDHHMMSS, then the two can be
> compared with one numeric test.
>
>> +	/* nanosecond, timezone, daylight and pad2 are meant to be zero */
> I'd verify that, but that's just me.

It is taken from here - 
https://kernel.googlesource.com/pub/scm/linux/kernel/git/jejb/efitools/+/refs/heads/master/include/efiauthenticated.h#249, 
since we also use the same tool.

Looks like I missed to add "pad1" as well in the comment. :-)


>
>> +
>> +	return OPAL_SUCCESS;
>> +}
>> +
>> +/* Extract PKCS7 from the authentication header */
>> +static int get_pkcs7(struct efi_variable_authentication_2 *auth,
>> +		     mbedtls_pkcs7 **pkcs7)
> auth should be const
>
>> +{
>> +	char *checkpkcs7cert = NULL;
>> +	int len;
>> +	int rc;
>> +
>> +	len = get_pkcs7_len(auth);
>> +	if (len <= 0)
>> +		return OPAL_PARAMETER;
>> +
>> +	if (!pkcs7)
>> +		return OPAL_PARAMETER;
>> +
>> +	*pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
>> +	if (!(*pkcs7))
>> +		return OPAL_NO_MEM;
>> +
>> +	mbedtls_pkcs7_init(*pkcs7);
>> +	rc = mbedtls_pkcs7_parse_der(
>> +			(const unsigned char *)auth->auth_info.cert_data,
>> +			(const unsigned int)len, *pkcs7);
> Why are those casts necessary?
>
>> +	if (rc) {
>> +		prlog(PR_ERR, "Parsing pkcs7 failed %04x\n", rc);
>> +		mbedtls_pkcs7_free(*pkcs7);
>> +		return rc;
>> +	}
>> +
>> +	checkpkcs7cert = zalloc(CERT_BUFFER_SIZE);
>> +	if (!checkpkcs7cert) {
>> +		mbedtls_pkcs7_free(*pkcs7);
>> +		return OPAL_NO_MEM;
>> +	}
>> +
>> +	rc = mbedtls_x509_crt_info(checkpkcs7cert, CERT_BUFFER_SIZE, "CRT:",
>> +			&((*pkcs7)->signed_data.certs));
>> +	if (rc < 0) {
>> +		prlog(PR_ERR, "Failed to parse the certificate in PKCS7 structure\n");
>> +		rc = OPAL_PARAMETER;
>> +	} else {
>> +		rc = OPAL_SUCCESS;
>> +		prlog(PR_DEBUG, "%s \n", checkpkcs7cert);
>> +	}
>> +
>> +	free(checkpkcs7cert);
>> +	mbedtls_pkcs7_free(*pkcs7);
>> +
>> +	return rc;
>> +}
>> +
>> +/* Verify the PKCS7 signature on the signed data. */
>> +static int verify_signature(struct efi_variable_authentication_2 *auth,
>> +			    char *newcert, size_t new_data_size,
>> +			    struct secvar *avar)
> const where necessary
>
>> +{
>> +	mbedtls_pkcs7 *pkcs7 = NULL;
>> +	mbedtls_x509_crt x509;
>> +	char *signing_cert = NULL;
>> +	char *x509_buf = NULL;
>> +	int signing_cert_size;
>> +	int rc;
>> +	char *errbuf;
>> +	int eslvarsize;
>> +	int eslsize;
>> +	int offset = 0;
>> +
>> +	if (!auth)
>> +		return OPAL_PARAMETER;
>> +
>> +	/* Extract the pkcs7 from the auth structure */
>> +	rc  = get_pkcs7(auth, &pkcs7);
>> +	/* Failure to parse pkcs7 implies bad input. */
>> +	if (rc != OPAL_SUCCESS)
>> +		return OPAL_PARAMETER;
>> +
>> +	prlog(PR_INFO, "Load the signing certificate from the keystore");
>> +
>> +	eslvarsize = avar->data_size;
>> +
>> +	/* Variable is not empty */
>> +	while (eslvarsize > 0) {
>> +		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
>> +		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
>> +			break;
>> +
>> +		/* Calculate the size of the ESL */
>> +		eslsize = get_esl_signature_list_size(avar->data + offset,
>> +						      eslvarsize);
>> +		/* If could not extract the size */
>> +		if (eslsize <= 0) {
>> +			rc = OPAL_PARAMETER;
>> +			break;
>> +		}
>> +
>> +		/* Extract the certificate from the ESL */
>> +		signing_cert_size = get_esl_cert(avar->data + offset,
>> +						 eslvarsize, &signing_cert);
>> +		if (signing_cert_size < 0) {
>> +			rc = signing_cert_size;
>> +			break;
>> +		}
>> +
>> +		mbedtls_x509_crt_init(&x509);
>> +		rc = mbedtls_x509_crt_parse(&x509,
>> +					    signing_cert,
>> +					    signing_cert_size);
>> +
>> +		/* This should not happen, unless something corrupted in PNOR */
>> +		if(rc) {
>> +			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
>> +			rc = OPAL_INTERNAL_ERROR;
>> +			break;
>> +		}
>> +
>> +		x509_buf = zalloc(CERT_BUFFER_SIZE);
>> +		rc = mbedtls_x509_crt_info(x509_buf,
>> +					   CERT_BUFFER_SIZE,
>> +					   "CRT:",
>> +					   &x509);
>> +
>> +		/* This should not happen, unless something corrupted in PNOR */
>> +		if (rc < 0) {
>> +			free(x509_buf);
>> +			rc = OPAL_INTERNAL_ERROR;
>> +			break;
>> +		}
>> +
>> +		prlog(PR_INFO, "%s \n", x509_buf);
>> +		free(x509_buf);
>> +		x509_buf = NULL;
>> +
>> +		/* Verify the signature */
>> +		rc = mbedtls_pkcs7_signed_data_verify(pkcs7, &x509, newcert,
>> +						      new_data_size);
>> +
>> +		/* If you find a signing certificate, you are done */
>> +		if (rc == 0) {
>> +			prlog(PR_INFO, "Signature Verification passed\n");
>> +			mbedtls_x509_crt_free(&x509);
>> +			break;
>> +		}
>> +
>> +		errbuf = zalloc(MBEDTLS_ERR_BUFFER_SIZE);
>> +		mbedtls_strerror(rc, errbuf, MBEDTLS_ERR_BUFFER_SIZE);
>> +		prlog(PR_INFO, "Signature Verification failed %02x %s\n",
>> +				rc, errbuf);
>> +		free(errbuf);
>> +
>> +		/* Look for the next ESL */
>> +		offset = offset + eslsize;
>> +		eslvarsize = eslvarsize - eslsize;
>> +		mbedtls_x509_crt_free(&x509);
>> +		free(signing_cert);
>> +		/* Since we are going to allocate again in the next iteration */
>> +		signing_cert = NULL;
>> +
>> +	}
>> +
>> +	free(signing_cert);
>> +	mbedtls_pkcs7_free(pkcs7);
>> +	free(pkcs7);
>> +
>> +	return rc;
>> +}
>> +
>> +/* Create the single buffer
>> + * name || vendor guid || attributes || timestamp || newcontent
>> + * which is submitted as signed by the user.
>> + * Returns number of bytes in the new buffer, else negative error
>> + * code.
>> + */
> I'd be happier if you incrementally computed the hash on each field
> rather than copying everything constantly.
>
>> +static int get_data_to_verify(char *key, char *new_data, size_t new_data_size,
>> +			      char **buffer, size_t *buffer_size,
>> +			      struct efi_time *timestamp)
> consts
>> +{
>> +	le32 attr = cpu_to_le32(SECVAR_ATTRIBUTES);
>> +	size_t offset = 0;
>> +	size_t varlen;
>> +	char *wkey;
>> +	uuid_t guid;
>> +
>> +	if (key_equals(key, "PK")
>> +	    || key_equals(key, "KEK"))
>> +		guid = EFI_GLOBAL_VARIABLE_GUID;
>> +	else if (key_equals(key, "db")
>> +	    || key_equals(key, "dbx"))
>> +		guid = EFI_IMAGE_SECURITY_DATABASE_GUID;
>> +	else
>> +		return OPAL_INTERNAL_ERROR;
>> +
>> +	/* Convert utf8 name to ucs2 width */
>> +	varlen = strlen(key) * 2;
>> +	wkey = utf8_to_ucs2(key, strlen(key));
>> +
>> +	/* Prepare the single buffer */
>> +	*buffer_size = varlen + UUID_SIZE + sizeof(attr)
>> +		       + sizeof(struct efi_time) + new_data_size;
>> +	*buffer = zalloc(*buffer_size);
>> +	if (!*buffer)
>> +		return OPAL_NO_MEM;
>> +
>> +	memcpy(*buffer + offset, wkey, varlen);
>> +	offset = offset + varlen;
>> +	memcpy(*buffer + offset, &guid, sizeof(guid));
>> +	offset = offset + sizeof(guid);
>> +	memcpy(*buffer + offset, &attr, sizeof(attr));
>> +	offset = offset + sizeof(attr);
>> +	memcpy(*buffer + offset, timestamp , sizeof(struct efi_time));
>> +	offset = offset + sizeof(struct efi_time);
>> +
>> +	memcpy(*buffer + offset, new_data, new_data_size);
>> +	offset = offset + new_data_size;
>> +
>> +	free(wkey);
>> +
>> +	return offset;
>> +}
>> +
>> +bool is_pkcs7_sig_format(void *data)
> const
>> +{
>> +	struct efi_variable_authentication_2 *auth = data;
>> +	uuid_t pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID;
>> +
>> +	if(!(memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) == 0))
>> +		return false;
>> +	return true;
> That's one way to do it. Alternatively:
>
> 	if (memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) != 0)
> 		return false;
> 	return true;
>
> Or:
> 	if (memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16))
> 		return false;
> 	return true;
>
> Or even:
>
> 	return !memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16);
>
>
>> +int process_update(struct secvar_node *update, char **newesl,
>> +		   int *new_data_size, struct efi_time *timestamp,
>> +		   struct list_head *bank, char *last_timestamp)
>> +{
>> +	struct efi_variable_authentication_2 *auth = NULL;
>> +	char *auth_buffer = NULL;
> If you want a pointer to an arbitrary buffer use a void pointer rather
> than a char. Voids will automaticly coerce to another pointer type so
> you don't have to manually cast it. Also, this should be const.
>
>> +	int auth_buffer_size = 0;
>> +	const char *key_authority[3];
>> +	char *tbhbuffer = NULL;
>> +	size_t tbhbuffersize = 0;
>> +	struct secvar_node *anode = NULL;
>> +	int rc = 0;
>> +	int i;
>> +
>> +	auth_buffer_size = get_auth_descriptor2(update->var->data,
>> +						update->var->data_size,
>> +						&auth_buffer);
>> +	if ((auth_buffer_size < 0)
>> +	     || (update->var->data_size < auth_buffer_size)) {
>> +		prlog(PR_ERR, "Invalid auth buffer size\n");
>> +		rc = auth_buffer_size;
>> +		goto out;
>> +	}
>> +
>> +	auth = (struct efi_variable_authentication_2 *)auth_buffer;
>> +
>> +	if (!timestamp) {
>> +		rc = OPAL_INTERNAL_ERROR;
>> +		goto out;
>> +	}
>> +
>> +	memcpy(timestamp, auth_buffer, sizeof(struct efi_time));
> *timestamp = auth->timestamp?
>
>> +
>> +	rc = check_timestamp(update->var->key, timestamp, last_timestamp);
>> +	/* Failure implies probably an older command being resubmitted */
>> +	if (rc != OPAL_SUCCESS) {
>> +		prlog(PR_INFO, "Timestamp verification failed for key %s\n", update->var->key);
>> +		goto out;
>> +	}
>> +
>> +	/* Calculate the size of new ESL data */
>> +	*new_data_size = update->var->data_size - auth_buffer_size;
>> +	if (*new_data_size < 0) {
>> +		prlog(PR_ERR, "Invalid new ESL (new data content) size\n");
>> +		rc = OPAL_PARAMETER;
>> +		goto out;
>> +	}
>> +	*newesl = zalloc(*new_data_size);
>> +	if (!(*newesl)) {
>> +		rc = OPAL_NO_MEM;
>> +		goto out;
>> +	}
>> +	memcpy(*newesl, update->var->data + auth_buffer_size, *new_data_size);
>> +
>> +	/* Validate the new ESL is in right format */
>> +	rc = validate_esl_list(update->var->key, *newesl, *new_data_size);
>> +	if (rc < 0) {
>> +		prlog(PR_ERR, "ESL validation failed for key %s with error %04x\n",
>> +		      update->var->key, rc);
>> +		goto out;
>> +	}
>> +
>> +	if (setup_mode) {
>> +		rc = OPAL_SUCCESS;
>> +		goto out;
>> +	}
>> +
>> +	/* Prepare the data to be verified */
>> +	rc = get_data_to_verify(update->var->key, *newesl, *new_data_size,
>> +				&tbhbuffer, &tbhbuffersize, timestamp);
>> +
>> +	/* Get the authority to verify the signature */
>> +	get_key_authority(key_authority, update->var->key);
>> +	i = 0;
>> +
>> +	/* Try for all the authorities that are allowed to sign.
>> +	 * For eg. db/dbx can be signed by both PK or KEK
>> +	 */
>> +	while (key_authority[i] != NULL) {
> use a for loop
>
>> +		prlog(PR_DEBUG, "key is %s\n", update->var->key);
>> +		prlog(PR_DEBUG, "key authority is %s\n", key_authority[i]);
>> +		anode = find_secvar(key_authority[i],
>> +				    strlen(key_authority[i]) + 1,
>> +				    bank);
> considering most of the calls to find_secvar() use a string literal and
> hardcode the key length you could clean things up by moving the
> strlen() into find_secvar() so the prototype is just find_secvar(key,
> bank).
>
> For the few cases that need to provide the explicit length they can use
> a helper that allows an explicit length to be passed in.

find_secvar() is a generic call, but key format is defined by backend, 
so the reason to do this is to keep the provision for non-null 
terminated key names as well. In any case, if this is more preferred, 
then we are ok to change.

>
>> +		if (!anode || !anode->var->data_size) {
>> +			i++;
>> +			continue;
>> +		}
>> +
>> +		/* Verify the signature */
>> +		rc = verify_signature(auth, tbhbuffer, tbhbuffersize,
>> +				      anode->var);
>> +
>> +		/* Break if signature verification is successful */
>> +		if (rc == OPAL_SUCCESS)
>> +			break;
>> +		i++;
>> +	}
>> +
>> +out:
>> +	free(auth_buffer);
>> +	free(tbhbuffer);
> So... are either of these copies actually modified? Just return const
> pointer to the input buffer.
>
>> +
>> +	return rc;
>> +}
>
>
> diff --git a/libstb/secvar/backend/edk2-compat.c
> b/libstb/secvar/backend/edk2-compat.c
>> new file mode 100644
>> index 00000000..334aca3e
>> --- /dev/null
>> +++ b/libstb/secvar/backend/edk2-compat.c
>> @@ -0,0 +1,262 @@
>> +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
>> +/* Copyright 2020 IBM Corp. */
>> +#ifndef pr_fmt
>> +#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
>> +#endif
>> +
>> +#include <opal.h>
>> +#include <string.h>
>> +#include <time.h>
>> +#include <unistd.h>
>> +#include <stdint.h>
>> +#include <skiboot.h>
>> +#include <ccan/endian/endian.h>
>> +#include <mbedtls/error.h>
>> +#include "libstb/crypto/pkcs7/pkcs7.h"
>> +#include "edk2.h"
>> +#include "../secvar.h"
>> +#include "edk2-compat-process.h"
>> +#include "edk2-compat-reset.h"
>> +
>> +struct list_head staging_bank;
>> +
>> +/*
>> + * Initializes supported variables as empty if not loaded from
>> + * storage. Variables are initialized as volatile if not found.
>> + * Updates should clear this flag.
>> + * Returns OPAL Error if anything fails in initialization
>> + */
>> +static int edk2_compat_pre_process(struct list_head *variable_bank,
>> +				   struct list_head *update_bank __unused)
>> +{
>> +	struct secvar_node *pkvar;
>> +	struct secvar_node *kekvar;
>> +	struct secvar_node *dbvar;
>> +	struct secvar_node *dbxvar;
>> +	struct secvar_node *tsvar;
>> +
>> +	pkvar = find_secvar("PK", 3, variable_bank);
>> +	if (!pkvar) {
>> +		pkvar = new_secvar("PK", 3, NULL, 0, SECVAR_FLAG_VOLATILE
>> +				| SECVAR_FLAG_PROTECTED);
>> +		if (!pkvar)
>> +			return OPAL_NO_MEM;
>> +
>> +		list_add_tail(variable_bank, &pkvar->link);
>> +	}
>> +	if (pkvar->var->data_size == 0)
>> +		setup_mode = true;
>> +	else
>> +		setup_mode = false;
>> +
>> +	kekvar = find_secvar("KEK", 4, variable_bank);
>> +	if (!kekvar) {
>> +		kekvar = new_secvar("KEK", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
>> +		if (!kekvar)
>> +			return OPAL_NO_MEM;
>> +
>> +		list_add_tail(variable_bank, &kekvar->link);
>> +	}
>> +
>> +	dbvar = find_secvar("db", 3, variable_bank);
>> +	if (!dbvar) {
>> +		dbvar = new_secvar("db", 3, NULL, 0, SECVAR_FLAG_VOLATILE);
>> +		if (!dbvar)
>> +			return OPAL_NO_MEM;
>> +
>> +		list_add_tail(variable_bank, &dbvar->link);
>> +	}
>> +
>> +	dbxvar = find_secvar("dbx", 4, variable_bank);
>> +	if (!dbxvar) {
>> +		dbxvar = new_secvar("dbx", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
>> +		if (!dbxvar)
>> +			return OPAL_NO_MEM;
>> +
>> +		list_add_tail(variable_bank, &dbxvar->link);
>> +	}
>> +
>> +	/* Should only ever happen on first boot. Timestamp is
>> +	 * initialized with all zeroes. */
>> +	tsvar = find_secvar("TS", 3, variable_bank);
>> +	if (!tsvar) {
>> +		tsvar = alloc_secvar(sizeof(struct efi_time) * 4);
>> +		if (!tsvar)
>> +			return OPAL_NO_MEM;
>> +
>> +		memcpy(tsvar->var->key, "TS", 3);
>> +		tsvar->var->key_len = 3;
>> +		tsvar->var->data_size = sizeof(struct efi_time) * 4;
>> +		memset(tsvar->var->data, 0, tsvar->var->data_size);
>> +		list_add_tail(variable_bank, &tsvar->link);
>> +	}
>> +
>> +	return OPAL_SUCCESS;
>> +};
> What's the point of these null variables? If you're doing it to ensure
> that find_secvar() or whatever doesn't return null pointers it's not
> really buying you anything since you still have to check for zero
> length data.

They are actually empty variables (zero length data) and not null 
variables. Empty variables are not persisted to the storage. So, these 
are created to expose empty variables to the userspace via sysfs so that 
user can update. We need this as we do not allow user to create 
variables in this version.

>
>> +
>> +static int edk2_compat_process(struct list_head *variable_bank,
>> +			       struct list_head *update_bank)
>> +{
>> +	struct secvar_node *node = NULL;
>> +	struct secvar_node *tsvar = NULL;
>> +	struct efi_time timestamp;
>> +	char *newesl = NULL;
>> +	int neweslsize;
>> +	int rc = 0;
>> +
>> +	prlog(PR_INFO, "Setup mode = %d\n", setup_mode);
>> +
>> +	/* Check HW-KEY-HASH */
>> +	if (!setup_mode) {
>> +		rc = verify_hw_key_hash();
>> +		if (rc != OPAL_SUCCESS) {
>> +			prlog(PR_ERR, "Hardware key hash verification mismatch\n");
>> +			rc = reset_keystore(variable_bank);
>> +			if (rc)
>> +				goto cleanup;
>> +			setup_mode = true;
>> +			goto cleanup;
>> +		}
>> +	}
>> +
>> +	/* Return early if we have no updates to process */
>> +	if (list_empty(update_bank)) {
>> +		return OPAL_EMPTY;
>> +	}
>> +
>> +	/* Make a working copy of variable bank that is updated
>> +	 * during process */
>> +	list_head_init(&staging_bank);
>> +	copy_bank_list(&staging_bank, variable_bank);
>> +
>> +	/* Loop through each command in the update bank.
>> +	 * If any command fails, it just loops out of the update bank.
>> +	 * It should also clear the update bank.
>> +	 */
>> +
>> +	list_for_each(update_bank, node, link) {
>> +
>> +		/* Submitted data is auth_2 descriptor + new ESL data
>> +		 * Extract the auth_2 2 descriptor
>> +		 */
>> +		prlog(PR_INFO, "Update for %s\n", node->var->key);
>> +
>> +		tsvar = find_secvar("TS", 3, &staging_bank);
> Is looking this up each loop iteration necessary?
>
>> +
>> +		/* We cannot find timestamp variable, did someone tamper it? */
>> +	        if (!tsvar) {
>> +			rc = OPAL_PERMISSION;
>> +			break;
>> +		}
>> +
>> +		rc = process_update(node, &newesl,
>> +				    &neweslsize, &timestamp,
>> +				    &staging_bank,
>> +				    tsvar->var->data);
>> +		if (rc) {
>> +			prlog(PR_ERR, "Update processing failed with rc %04x\n", rc);
>> +			break;
>> +		}
>> +
>> +		/*
>> +		 * If reached here means, signature is verified so update the
>> +		 * value in the variable bank
>> +		 */
>> +		rc = update_variable_in_bank(node->var,
>> +					     newesl,
>> +					     neweslsize,
>> +					     &staging_bank);
>> +		if (rc) {
>> +			prlog(PR_ERR, "Updating the variable data failed %04x\n", rc);
>> +			break;
>> +		}
>> +
>> +		free(newesl);
>> +		/* Update the TS variable with the new timestamp */
>> +		rc = update_timestamp(node->var->key,
>> +				      &timestamp,
>> +				      tsvar->var->data);
> Is having all the timestamps in a single secvar an actual requirement
> or is that just how you've decided to implement the timestamp tracking?
>
> Seem like it'd be easier all around if you tracked it on a per-variable
> basis and stored it as a part of the serialised data format rather than
> wedging them together into a variable. Wedging them also creates the
> awkward data flow we're seeing here where process_update() needs to
> return the timestamp via out pointers.

The main reason is to expose the timestamps to the userspace via sysfs. 
It should be ok to make them per-variable basis, but might involve 
parsing either at API level to return data or user level to get ESLs. We 
think it would be simpler to keep it separate.


>
>> +		if (rc) {
>> +			prlog (PR_ERR, "Variable updated, but timestamp updated failed %04x\n", rc);
>> +			break;
>> +		}
>> +
>> +		/* If the PK is updated, update the secure boot state of the
>> +		 * system at the end of processing */
>> +		if (key_equals(node->var->key, "PK")) {
>> +			/* PK is tied to a particular firmware image by mapping
>> +			 * it with hw-key-hash of that firmware. When PK is
>> +			 * updated, hw-key-hash is updated. And when PK is
>> +			 * deleted, delete hw-key-hash as well */
>> +			if(neweslsize == 0) {
>> +				setup_mode = true;
>> +				delete_hw_key_hash(&staging_bank);
>> +			} else  {
>> +				setup_mode = false;
>> +				add_hw_key_hash(&staging_bank);
>> +			}
>> +			prlog(PR_DEBUG, "setup mode is %d\n", setup_mode);
>> +		}
>> +	}
>> +
>> +	if (rc == 0) {
>> +		/* Update the variable bank with updated working copy */
>> +		clear_bank_list(variable_bank);
>> +		copy_bank_list(variable_bank, &staging_bank);
> Isn't this leaking all the variables in staging bank?
>
>> +	}
>> +
>> +cleanup:
>> +	/* For any failure in processing update queue, we clear the update bank
>> +	 * and return failure */
>> +	clear_bank_list(update_bank);
>> +
>> +	return rc;
>> +}
>> +
>> +static int edk2_compat_post_process(struct list_head *variable_bank,
>> +				    struct list_head *update_bank __unused)
>> +{
>> +	struct secvar_node *hwvar;
>> +	if (!setup_mode) {
>> +		secvar_set_secure_mode();
>> +		prlog(PR_INFO, "Enforcing OS secure mode\n");
>> +		/* HW KEY HASH is no more needed after this point. It is already
>> +		 * visible to userspace via device-tree, so exposing via sysfs is
>> +		 * just a duplication. Remove it from in-memory copy. */
>> +		hwvar = find_secvar("HWKH", 5, variable_bank);
>> +		if (!hwvar) {
>> +			prlog(PR_ERR, "cannot find hw-key-hash, should not happen\n");
>> +			return OPAL_INTERNAL_ERROR;
>> +		}
>> +		list_del(&hwvar->link);
>> +		dealloc_secvar(hwvar);
>> +	}
>> +
>> +	return OPAL_SUCCESS;
>> +}
>> +
>> +static int edk2_compat_validate(struct secvar *var)
>> +{
>> +
>> +	/* Checks if the update is for supported
>> +	 * Non-volatile secure variables */
>> +	if (!key_equals(var->key, "PK")
>> +			&& !key_equals(var->key, "KEK")
>> +			&& !key_equals(var->key, "db")
>> +			&& !key_equals(var->key, "dbx"))
>> +		return OPAL_PARAMETER;
> Being unable to support user variables is a bit crap. oh well.

It is in plan for the future.


>
>> +	/* Check that signature type is PKCS7 */
>> +	if (!is_pkcs7_sig_format(var->data))
>> +		return OPAL_PARAMETER;
>> +
>> +	return OPAL_SUCCESS;
>> +};
>> +
>> +struct secvar_backend_driver edk2_compatible_v1 = {
>> +	.pre_process = edk2_compat_pre_process,
>> +	.process = edk2_compat_process,
>> +	.post_process = edk2_compat_post_process,
>> +	.validate = edk2_compat_validate,
>> +	.compatible = "ibm,edk2-compat-v1",
>> +};
>> diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h
>> new file mode 100644
>> index 00000000..fb0121af
>> --- /dev/null
>> +++ b/libstb/secvar/backend/edk2.h
>> *snip*
>> +/*
>> + * Certificate which encapsulates a GUID-specific digital signature
>> + */
>> +struct win_certificate_uefi_guid {
>> +	/*
>> +	 * This is the standard win_certificate header, where w_certificate_type
>> +	 * is set to WIN_CERT_TYPE_EFI_GUID.
>> +	 */
>> +	struct win_certificate hdr;
>> +	/*
>> +	 * This is the unique id which determines the format of the cert_data.
>> +	 */
>> +	uuid_t cert_type;
>> +	/*
>> +	 * The following is the certificate data. The format of the data is
>> +	 * determined by the @cert_type. If @cert_type is
>> +	 * EFI_CERT_TYPE_RSA2048_SHA256_GUID, the @cert_data will be
>> +	 * EFI_CERT_BLOCK_RSA_2048_SHA256 structure.
>> +	 */
>> +	u8 cert_data[1];
> I'm guessing this is TianoCore-ism, but using a length-one array at the
> end of a struct to indicate trailing data is something that went out of
> fashion in the 90s. GCC has an extension to allow zero-length arrays at
> the end of a struct for this purpose that was added decades ago. That
> extension was deprecated by C99 which added a no-length array syntax
> for the same purpose:
>
> struct a {
> 	int start;
> 	char buffer[];
> };
>
> I'd suggest you use that since it allows you to do a sizeof() on the
> struct variable to get the size of the structure header without the
> trailing element screwing with it.
>
>
Yes, agree. However, it is not exactly as part of TianoCore-ism, but 
mainly we just borrowed the structure "as is" from the Tianocore headers 
as listed in the license of this file. I think there should not be any 
issue in changing, but will check before doing that.

We will fix all other comments. Thanks for your feedback.

Thanks & Regards,

      - Nayna
diff mbox series

Patch

diff --git a/doc/secvar/edk2.rst b/doc/secvar/edk2.rst
new file mode 100644
index 00000000..1e4cc9e3
--- /dev/null
+++ b/doc/secvar/edk2.rst
@@ -0,0 +1,49 @@ 
+.. _secvar/edk2:
+
+Skiboot edk2-compatible Secure Variable Backend
+===============================================
+
+Overview
+--------
+
+The edk2 secure variable backend for skiboot borrows from edk2 concepts
+such as the three key hierarchy (PK, KEK, and db), and a similar
+structure. In general, variable updates must be signed with a key
+of a higher level. So, updates to the db must be signed with a key stored
+in the KEK; updates to the KEK must be signed with the PK. Updates to the
+PK must be signed with the previous PK (if any).
+
+Variables are stored in the efi signature list format, and updates are a
+signed variant that includes an authentication header.
+
+If no PK is currently enrolled, the system is considered to be in "Setup
+Mode". Any key can be enrolled without signature checks. However, once a
+PK is enrolled, the system switches to "User Mode", and each update must
+now be signed according to the hierarchy. Furthermore, when in "User
+Mode", the backend initialized the ``os-secure-mode`` device tree flag,
+signaling to the kernel that we are in secure mode.
+
+Updates are processed sequentially, in the order that they were provided
+in the update queue. If any update fails to validate, appears to be
+malformed, or any other error occurs, NO updates will not be applied.
+This includes updates that may have successfully applied prior to the
+error. The system will continue in an error state, reporting the error
+reason via the ``update-status`` device tree property.
+
+P9 Special Case for the Platform Key
+------------------------------------
+
+Due to the powerful nature of the platform key and the lack of lockable
+flash, the edk2 backend will store the PK in TPM NV rather than PNOR on
+P9 systems. (TODO expand on this)
+
+Update Status Return Codes
+--------------------------
+
+TODO, edk2 driver needs to actually return these properly first
+
+
+Device Tree Bindings
+--------------------
+
+TODO
diff --git a/include/secvar.h b/include/secvar.h
index 7a45db2b..4f40c115 100644
--- a/include/secvar.h
+++ b/include/secvar.h
@@ -28,6 +28,7 @@  struct secvar_backend_driver {
 };
 
 extern struct secvar_storage_driver secboot_tpm_driver;
+extern struct secvar_backend_driver edk2_compatible_v1;
 
 int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
 
diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
index 6f491a63..bc987f69 100644
--- a/libstb/secvar/backend/Makefile.inc
+++ b/libstb/secvar/backend/Makefile.inc
@@ -1,11 +1,11 @@ 
 # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
 # -*-Makefile-*-
 
-SECVAR_BACKEND_DIR = libstb/secvar/backend
+SECVAR_BACKEND_DIR = $(SRC)/libstb/secvar/backend
 
 SUBDIRS += $(SECVAR_BACKEND_DIR)
 
-SECVAR_BACKEND_SRCS =
+SECVAR_BACKEND_SRCS = edk2-compat.c edk2-compat-process.c edk2-compat-reset.c
 SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
 SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
 
diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
new file mode 100644
index 00000000..60ebb0b2
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -0,0 +1,717 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#ifndef pr_fmt
+#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
+#endif
+
+#include <opal.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <ccan/endian/endian.h>
+#include <mbedtls/error.h>
+#include <device.h>
+#include "libstb/crypto/pkcs7/pkcs7.h"
+#include "edk2.h"
+#include "../secvar.h"
+#include "edk2-compat-process.h"
+
+bool setup_mode;
+
+int update_variable_in_bank(struct secvar *secvar, const char *data,
+			    uint64_t dsize, struct list_head *bank)
+{
+	struct secvar_node *node;
+
+	node = find_secvar(secvar->key, secvar->key_len, bank);
+	if (!node)
+		return OPAL_EMPTY;
+
+        /* Reallocate the data memory, if there is change in data size */
+	if (node->size < dsize)
+		if (realloc_secvar(node, dsize))
+			return OPAL_NO_MEM;
+
+	if (dsize && data)
+		memcpy(node->var->data, data, dsize);
+	node->var->data_size = dsize;
+
+        /* Clear the volatile bit only if updated with positive data size */
+	if (dsize)
+		node->flags &= ~SECVAR_FLAG_VOLATILE;
+	else
+		node->flags |= SECVAR_FLAG_VOLATILE;
+
+        /* Is it required to be set everytime ? */
+	if ((!strncmp(secvar->key, "PK", 3))
+	     || (!strncmp(secvar->key, "HWKH", 5)))
+		node->flags |= SECVAR_FLAG_PROTECTED;
+
+	return 0;
+}
+
+/* Converts utf8 string to ucs2 */
+static char *utf8_to_ucs2(const char *key, size_t keylen)
+{
+	int i;
+	char *str;
+
+	str = zalloc(keylen * 2);
+	if (!str)
+		return NULL;
+
+	for (i = 0; i < keylen*2; key++) {
+		str[i++] = *key;
+		str[i++] = '\0';
+	}
+
+	return str;
+}
+
+/* Returns the authority that can sign the given key update */
+static void get_key_authority(const char *ret[3], const char *key)
+{
+	int i = 0;
+
+	if (key_equals(key, "PK")) {
+		ret[i++] = "PK";
+	} else if (key_equals(key, "KEK")) {
+		ret[i++] = "PK";
+	} else if (key_equals(key, "db") || key_equals(key, "dbx")) {
+		ret[i++] = "KEK";
+		ret[i++] = "PK";
+	}
+
+	ret[i] = NULL;
+}
+
+/* Returns the size of the complete ESL. */
+static int get_esl_signature_list_size(char *buf, size_t buflen)
+{
+	EFI_SIGNATURE_LIST list;
+
+	if (buflen < sizeof(EFI_SIGNATURE_LIST))
+		return OPAL_PARAMETER;
+
+	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
+
+	prlog(PR_DEBUG, "size of signature list size is %u\n",
+			le32_to_cpu(list.SignatureListSize));
+
+	return le32_to_cpu(list.SignatureListSize);
+}
+
+/* Copies the certificate from the ESL into cert buffer and returns the size
+ * of the certificate
+ */
+static int get_esl_cert(char *buf, size_t buflen, char **cert)
+{
+	size_t sig_data_offset;
+	size_t size;
+	EFI_SIGNATURE_LIST list;
+
+	if (buflen < sizeof(EFI_SIGNATURE_LIST))
+		return OPAL_PARAMETER;
+
+	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
+
+	size = le32_to_cpu(list.SignatureSize) - sizeof(uuid_t);
+	/* No certificate in the ESL */
+	if (size <= 0)
+		return OPAL_PERMISSION;
+
+	if (!cert)
+		return OPAL_PARAMETER;
+
+	*cert = zalloc(size);
+	if (!(*cert))
+		return OPAL_NO_MEM;
+
+	prlog(PR_DEBUG,"size of signature list size is %u\n",
+			le32_to_cpu(list.SignatureListSize));
+	prlog(PR_DEBUG, "size of signature header size is %u\n",
+			le32_to_cpu(list.SignatureHeaderSize));
+	prlog(PR_DEBUG, "size of signature size is %u\n",
+			le32_to_cpu(list.SignatureSize));
+
+	sig_data_offset = sizeof(list) + le32_to_cpu(list.SignatureHeaderSize)
+		+ 16 * sizeof(uint8_t);
+	if (sig_data_offset > buflen) {
+		free(*cert);
+		return OPAL_PARAMETER;
+	}
+
+	memcpy(*cert, buf + sig_data_offset, size);
+
+	return size;
+}
+
+/* Extracts size of the PKCS7 signed data embedded in the
+ * struct Authentication 2 Descriptor Header.
+ */
+static int get_pkcs7_len(struct efi_variable_authentication_2 *auth)
+{
+	uint32_t dw_length;
+	size_t size;
+
+	if (!auth)
+		return OPAL_PARAMETER;
+
+	dw_length = le32_to_cpu(auth->auth_info.hdr.dw_length);
+	size = dw_length - (sizeof(auth->auth_info.hdr.dw_length)
+			+ sizeof(auth->auth_info.hdr.w_revision)
+			+ sizeof(auth->auth_info.hdr.w_certificate_type)
+			+ sizeof(auth->auth_info.cert_type));
+
+	return size;
+}
+
+int get_auth_descriptor2(void *buf, size_t buflen, char **auth_buffer)
+{
+	struct efi_variable_authentication_2 *auth = NULL;
+	size_t auth_buffer_size;
+	int len;
+
+	if (buflen < sizeof(struct efi_variable_authentication_2))
+			return OPAL_PARAMETER;
+
+	auth = buf;
+
+	len = get_pkcs7_len(auth);
+
+	/* We need PKCS7 data else there is no signature */
+	if (len <= 0)
+		return OPAL_PARAMETER;
+
+	if (!auth_buffer)
+		return OPAL_PARAMETER;
+
+	auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
+			   + sizeof(auth->auth_info.cert_type) + len;
+
+	if (auth_buffer_size <= 0)
+		return OPAL_PARAMETER;
+
+	*auth_buffer = zalloc(auth_buffer_size);
+	if (!(*auth_buffer))
+		return OPAL_NO_MEM;
+
+	memcpy(*auth_buffer, buf, auth_buffer_size);
+
+	return auth_buffer_size;
+}
+
+int validate_esl_list(char *key, char *esl, size_t size)
+{
+	int count = 0;
+	int signing_cert_size;
+	char *signing_cert = NULL;
+	mbedtls_x509_crt x509;
+	char *x509_buf = NULL;
+	int eslvarsize = size;
+	int rc = OPAL_SUCCESS;
+	int eslsize;
+	int offset = 0;
+
+	while (eslvarsize > 0) {
+		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
+		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
+			break;
+
+		/* Calculate the size of the ESL */
+		eslsize = get_esl_signature_list_size(esl, eslvarsize);
+		/* If could not extract the size */
+		if (eslsize <= 0) {
+			prlog(PR_ERR, "Invalid size of the ESL\n");
+			rc = OPAL_PARAMETER;
+			break;
+		}
+
+		/* Extract the certificate from the ESL */
+		signing_cert_size = get_esl_cert(esl,
+						 eslvarsize,
+						 &signing_cert);
+		if (signing_cert_size < 0) {
+			rc = signing_cert_size;
+			break;
+		}
+
+		mbedtls_x509_crt_init(&x509);
+		rc = mbedtls_x509_crt_parse(&x509,
+					    signing_cert,
+					    signing_cert_size);
+
+		/* If failure in parsing the certificate, exit */
+		if(rc) {
+			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
+			rc = OPAL_PARAMETER;
+			break;
+		}
+
+		x509_buf = zalloc(CERT_BUFFER_SIZE);
+		rc = mbedtls_x509_crt_info(x509_buf,
+					   CERT_BUFFER_SIZE,
+					   "CRT:",
+					   &x509);
+		prlog(PR_INFO, "%s ", x509_buf);
+
+		/* If failure in reading the certificate, exit */
+		if (rc < 0) {
+			prlog(PR_INFO, "Failed to show X509 certificate info %04x\n", rc);
+			rc = OPAL_PARAMETER;
+			free(x509_buf);
+			break;
+		}
+		rc = 0;
+
+		free(x509_buf);
+		x509_buf = NULL;
+		count++;
+
+		/* Look for the next ESL */
+		offset = offset + eslsize;
+		eslvarsize = eslvarsize - eslsize;
+		mbedtls_x509_crt_free(&x509);
+		free(signing_cert);
+		/* Since we are going to allocate again in the next iteration */
+		signing_cert = NULL;
+	}
+
+	if (rc == OPAL_SUCCESS) {
+		if (key_equals(key, "PK") && (count > 1)) {
+			prlog(PR_ERR, "PK can only be one\n");
+			rc = OPAL_PARAMETER;
+		} else {
+			rc = count;
+		}
+	}
+
+	prlog(PR_INFO, "Total ESLs are %d\n", rc);
+	return rc;
+}
+
+/* Get the timestamp for the last update of the give key */
+static struct efi_time *get_last_timestamp(const char *key, char *last_timestamp)
+{
+	u8 off;
+
+	if (!last_timestamp)
+		return NULL;
+
+	if (!strncmp(key, "PK", 3))
+		off = 0;
+	else if (!strncmp(key, "KEK", 4))
+		off = 1;
+	else if (!strncmp(key, "db", 3))
+		off = 2;
+	else if (!strncmp(key, "dbx", 4))
+		off = 3;
+	else
+		return NULL;
+
+	return &((struct efi_time *)last_timestamp)[off];
+}
+
+int update_timestamp(char *key, struct efi_time *timestamp, char *last_timestamp)
+{
+	struct efi_time *prev;
+
+	prev = get_last_timestamp(key, last_timestamp);
+	if (prev == NULL)
+		return OPAL_INTERNAL_ERROR;
+
+	memcpy(prev, timestamp, sizeof(struct efi_time));
+
+	prlog(PR_DEBUG, "updated prev year is %d month %d day %d\n",
+			le16_to_cpu(prev->year), prev->month, prev->day);
+
+	return OPAL_SUCCESS;
+}
+
+int check_timestamp(char *key, struct efi_time *timestamp,
+		    char *last_timestamp)
+{
+	struct efi_time *prev;
+
+	prev = get_last_timestamp(key, last_timestamp);
+	if (prev == NULL)
+		return OPAL_INTERNAL_ERROR;
+
+	prlog(PR_DEBUG, "timestamp year is %d month %d day %d\n",
+			le16_to_cpu(timestamp->year), timestamp->month,
+			timestamp->day);
+	prlog(PR_DEBUG, "prev year is %d month %d day %d\n",
+			le16_to_cpu(prev->year), prev->month, prev->day);
+	if (le16_to_cpu(timestamp->year) > le16_to_cpu(prev->year))
+		return OPAL_SUCCESS;
+	if (le16_to_cpu(timestamp->year) < le16_to_cpu(prev->year))
+		return OPAL_PERMISSION;
+
+	if (timestamp->month > prev->month)
+		return OPAL_SUCCESS;
+	if (timestamp->month < prev->month)
+		return OPAL_PERMISSION;
+
+	if (timestamp->day > prev->day)
+		return OPAL_SUCCESS;
+	if (timestamp->day < prev->day)
+		return OPAL_PERMISSION;
+
+	if (timestamp->hour > prev->hour)
+		return OPAL_SUCCESS;
+	if (timestamp->hour < prev->hour)
+		return OPAL_PERMISSION;
+
+	if (timestamp->minute > prev->minute)
+		return OPAL_SUCCESS;
+	if (timestamp->minute < prev->minute)
+		return OPAL_PERMISSION;
+
+	if (timestamp->second > prev->second)
+		return OPAL_SUCCESS;
+
+	/* Time less than or equal to is considered as replay*/
+	if (timestamp->second <= prev->second)
+		return OPAL_PERMISSION;
+
+	/* nanosecond, timezone, daylight and pad2 are meant to be zero */
+
+	return OPAL_SUCCESS;
+}
+
+/* Extract PKCS7 from the authentication header */
+static int get_pkcs7(struct efi_variable_authentication_2 *auth,
+		     mbedtls_pkcs7 **pkcs7)
+{
+	char *checkpkcs7cert = NULL;
+	int len;
+	int rc;
+
+	len = get_pkcs7_len(auth);
+	if (len <= 0)
+		return OPAL_PARAMETER;
+
+	if (!pkcs7)
+		return OPAL_PARAMETER;
+
+	*pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
+	if (!(*pkcs7))
+		return OPAL_NO_MEM;
+
+	mbedtls_pkcs7_init(*pkcs7);
+	rc = mbedtls_pkcs7_parse_der(
+			(const unsigned char *)auth->auth_info.cert_data,
+			(const unsigned int)len, *pkcs7);
+	if (rc) {
+		prlog(PR_ERR, "Parsing pkcs7 failed %04x\n", rc);
+		mbedtls_pkcs7_free(*pkcs7);
+		return rc;
+	}
+
+	checkpkcs7cert = zalloc(CERT_BUFFER_SIZE);
+	if (!checkpkcs7cert) {
+		mbedtls_pkcs7_free(*pkcs7);
+		return OPAL_NO_MEM;
+	}
+
+	rc = mbedtls_x509_crt_info(checkpkcs7cert, CERT_BUFFER_SIZE, "CRT:",
+			&((*pkcs7)->signed_data.certs));
+	if (rc < 0) {
+		prlog(PR_ERR, "Failed to parse the certificate in PKCS7 structure\n");
+		rc = OPAL_PARAMETER;
+	} else {
+		rc = OPAL_SUCCESS;
+		prlog(PR_DEBUG, "%s \n", checkpkcs7cert);
+	}
+
+	free(checkpkcs7cert);
+	mbedtls_pkcs7_free(*pkcs7);
+
+	return rc;
+}
+
+/* Verify the PKCS7 signature on the signed data. */
+static int verify_signature(struct efi_variable_authentication_2 *auth,
+			    char *newcert, size_t new_data_size,
+			    struct secvar *avar)
+{
+	mbedtls_pkcs7 *pkcs7 = NULL;
+	mbedtls_x509_crt x509;
+	char *signing_cert = NULL;
+	char *x509_buf = NULL;
+	int signing_cert_size;
+	int rc;
+	char *errbuf;
+	int eslvarsize;
+	int eslsize;
+	int offset = 0;
+
+	if (!auth)
+		return OPAL_PARAMETER;
+
+	/* Extract the pkcs7 from the auth structure */
+	rc  = get_pkcs7(auth, &pkcs7);
+	/* Failure to parse pkcs7 implies bad input. */
+	if (rc != OPAL_SUCCESS)
+		return OPAL_PARAMETER;
+
+	prlog(PR_INFO, "Load the signing certificate from the keystore");
+
+	eslvarsize = avar->data_size;
+
+	/* Variable is not empty */
+	while (eslvarsize > 0) {
+		prlog(PR_DEBUG, "esl var size size is %d offset is %d\n", eslvarsize, offset);
+		if (eslvarsize < sizeof(EFI_SIGNATURE_LIST))
+			break;
+
+		/* Calculate the size of the ESL */
+		eslsize = get_esl_signature_list_size(avar->data + offset,
+						      eslvarsize);
+		/* If could not extract the size */
+		if (eslsize <= 0) {
+			rc = OPAL_PARAMETER;
+			break;
+		}
+
+		/* Extract the certificate from the ESL */
+		signing_cert_size = get_esl_cert(avar->data + offset,
+						 eslvarsize, &signing_cert);
+		if (signing_cert_size < 0) {
+			rc = signing_cert_size;
+			break;
+		}
+
+		mbedtls_x509_crt_init(&x509);
+		rc = mbedtls_x509_crt_parse(&x509,
+					    signing_cert,
+					    signing_cert_size);
+
+		/* This should not happen, unless something corrupted in PNOR */
+		if(rc) {
+			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
+			rc = OPAL_INTERNAL_ERROR;
+			break;
+		}
+
+		x509_buf = zalloc(CERT_BUFFER_SIZE);
+		rc = mbedtls_x509_crt_info(x509_buf,
+					   CERT_BUFFER_SIZE,
+					   "CRT:",
+					   &x509);
+
+		/* This should not happen, unless something corrupted in PNOR */
+		if (rc < 0) {
+			free(x509_buf);
+			rc = OPAL_INTERNAL_ERROR;
+			break;
+		}
+
+		prlog(PR_INFO, "%s \n", x509_buf);
+		free(x509_buf);
+		x509_buf = NULL;
+
+		/* Verify the signature */
+		rc = mbedtls_pkcs7_signed_data_verify(pkcs7, &x509, newcert,
+						      new_data_size);
+
+		/* If you find a signing certificate, you are done */
+		if (rc == 0) {
+			prlog(PR_INFO, "Signature Verification passed\n");
+			mbedtls_x509_crt_free(&x509);
+			break;
+		}
+
+		errbuf = zalloc(MBEDTLS_ERR_BUFFER_SIZE);
+		mbedtls_strerror(rc, errbuf, MBEDTLS_ERR_BUFFER_SIZE);
+		prlog(PR_INFO, "Signature Verification failed %02x %s\n",
+				rc, errbuf);
+		free(errbuf);
+
+		/* Look for the next ESL */
+		offset = offset + eslsize;
+		eslvarsize = eslvarsize - eslsize;
+		mbedtls_x509_crt_free(&x509);
+		free(signing_cert);
+		/* Since we are going to allocate again in the next iteration */
+		signing_cert = NULL;
+
+	}
+
+	free(signing_cert);
+	mbedtls_pkcs7_free(pkcs7);
+	free(pkcs7);
+
+	return rc;
+}
+
+/* Create the single buffer
+ * name || vendor guid || attributes || timestamp || newcontent
+ * which is submitted as signed by the user.
+ * Returns number of bytes in the new buffer, else negative error
+ * code.
+ */
+static int get_data_to_verify(char *key, char *new_data, size_t new_data_size,
+			      char **buffer, size_t *buffer_size,
+			      struct efi_time *timestamp)
+{
+	le32 attr = cpu_to_le32(SECVAR_ATTRIBUTES);
+	size_t offset = 0;
+	size_t varlen;
+	char *wkey;
+	uuid_t guid;
+
+	if (key_equals(key, "PK")
+	    || key_equals(key, "KEK"))
+		guid = EFI_GLOBAL_VARIABLE_GUID;
+	else if (key_equals(key, "db")
+	    || key_equals(key, "dbx"))
+		guid = EFI_IMAGE_SECURITY_DATABASE_GUID;
+	else
+		return OPAL_INTERNAL_ERROR;
+
+	/* Convert utf8 name to ucs2 width */
+	varlen = strlen(key) * 2;
+	wkey = utf8_to_ucs2(key, strlen(key));
+
+	/* Prepare the single buffer */
+	*buffer_size = varlen + UUID_SIZE + sizeof(attr)
+		       + sizeof(struct efi_time) + new_data_size;
+	*buffer = zalloc(*buffer_size);
+	if (!*buffer)
+		return OPAL_NO_MEM;
+
+	memcpy(*buffer + offset, wkey, varlen);
+	offset = offset + varlen;
+	memcpy(*buffer + offset, &guid, sizeof(guid));
+	offset = offset + sizeof(guid);
+	memcpy(*buffer + offset, &attr, sizeof(attr));
+	offset = offset + sizeof(attr);
+	memcpy(*buffer + offset, timestamp , sizeof(struct efi_time));
+	offset = offset + sizeof(struct efi_time);
+
+	memcpy(*buffer + offset, new_data, new_data_size);
+	offset = offset + new_data_size;
+
+	free(wkey);
+
+	return offset;
+}
+
+bool is_pkcs7_sig_format(void *data)
+{
+	struct efi_variable_authentication_2 *auth = data;
+	uuid_t pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID;
+
+	if(!(memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) == 0))
+		return false;
+
+	return true;
+}
+
+int process_update(struct secvar_node *update, char **newesl,
+		   int *new_data_size, struct efi_time *timestamp,
+		   struct list_head *bank, char *last_timestamp)
+{
+	struct efi_variable_authentication_2 *auth = NULL;
+	char *auth_buffer = NULL;
+	int auth_buffer_size = 0;
+	const char *key_authority[3];
+	char *tbhbuffer = NULL;
+	size_t tbhbuffersize = 0;
+	struct secvar_node *anode = NULL;
+	int rc = 0;
+	int i;
+
+	auth_buffer_size = get_auth_descriptor2(update->var->data,
+						update->var->data_size,
+						&auth_buffer);
+	if ((auth_buffer_size < 0)
+	     || (update->var->data_size < auth_buffer_size)) {
+		prlog(PR_ERR, "Invalid auth buffer size\n");
+		rc = auth_buffer_size;
+		goto out;
+	}
+
+	auth = (struct efi_variable_authentication_2 *)auth_buffer;
+
+	if (!timestamp) {
+		rc = OPAL_INTERNAL_ERROR;
+		goto out;
+	}
+
+	memcpy(timestamp, auth_buffer, sizeof(struct efi_time));
+
+	rc = check_timestamp(update->var->key, timestamp, last_timestamp);
+	/* Failure implies probably an older command being resubmitted */
+	if (rc != OPAL_SUCCESS) {
+		prlog(PR_INFO, "Timestamp verification failed for key %s\n", update->var->key);
+		goto out;
+	}
+
+	/* Calculate the size of new ESL data */
+	*new_data_size = update->var->data_size - auth_buffer_size;
+	if (*new_data_size < 0) {
+		prlog(PR_ERR, "Invalid new ESL (new data content) size\n");
+		rc = OPAL_PARAMETER;
+		goto out;
+	}
+	*newesl = zalloc(*new_data_size);
+	if (!(*newesl)) {
+		rc = OPAL_NO_MEM;
+		goto out;
+	}
+	memcpy(*newesl, update->var->data + auth_buffer_size, *new_data_size);
+
+	/* Validate the new ESL is in right format */
+	rc = validate_esl_list(update->var->key, *newesl, *new_data_size);
+	if (rc < 0) {
+		prlog(PR_ERR, "ESL validation failed for key %s with error %04x\n",
+		      update->var->key, rc);
+		goto out;
+	}
+
+	if (setup_mode) {
+		rc = OPAL_SUCCESS;
+		goto out;
+	}
+
+	/* Prepare the data to be verified */
+	rc = get_data_to_verify(update->var->key, *newesl, *new_data_size,
+				&tbhbuffer, &tbhbuffersize, timestamp);
+
+	/* Get the authority to verify the signature */
+	get_key_authority(key_authority, update->var->key);
+	i = 0;
+
+	/* Try for all the authorities that are allowed to sign.
+	 * For eg. db/dbx can be signed by both PK or KEK
+	 */
+	while (key_authority[i] != NULL) {
+		prlog(PR_DEBUG, "key is %s\n", update->var->key);
+		prlog(PR_DEBUG, "key authority is %s\n", key_authority[i]);
+		anode = find_secvar(key_authority[i],
+				    strlen(key_authority[i]) + 1,
+				    bank);
+		if (!anode || !anode->var->data_size) {
+			i++;
+			continue;
+		}
+
+		/* Verify the signature */
+		rc = verify_signature(auth, tbhbuffer, tbhbuffersize,
+				      anode->var);
+
+		/* Break if signature verification is successful */
+		if (rc == OPAL_SUCCESS)
+			break;
+		i++;
+	}
+
+out:
+	free(auth_buffer);
+	free(tbhbuffer);
+
+	return rc;
+}
diff --git a/libstb/secvar/backend/edk2-compat-process.h b/libstb/secvar/backend/edk2-compat-process.h
new file mode 100644
index 00000000..94059ee3
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat-process.h
@@ -0,0 +1,61 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+
+#ifndef __SECVAR_EDK2_COMPAT_PROCESS__
+#define __SECVAR_EDK2_COMPAT_PROCESS__
+
+#ifndef pr_fmt
+#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
+#endif
+
+#include <opal.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <ccan/endian/endian.h>
+#include <mbedtls/error.h>
+#include <device.h>
+#include "libstb/crypto/pkcs7/pkcs7.h"
+#include "edk2.h"
+#include "opal-api.h"
+#include "../secvar.h"
+#include "../secvar_devtree.h"
+
+#define CERT_BUFFER_SIZE        2048
+#define MBEDTLS_ERR_BUFFER_SIZE 1024
+
+#define EDK2_MAX_KEY_LEN        SECVAR_MAX_KEY_LEN
+#define key_equals(a,b) (!strncmp(a, b, EDK2_MAX_KEY_LEN))
+
+extern bool setup_mode;
+extern struct list_head staging_bank;
+
+/* Update the variable in the variable bank with the new value. */
+int update_variable_in_bank(struct secvar *secvar, const char *data,
+			    uint64_t dsize, struct list_head *bank);
+
+/* This function outputs the Authentication 2 Descriptor in the
+ * auth_buffer and returns the size of the buffer. Please refer to
+ * edk2.h for details on Authentication 2 Descriptor
+ */
+int get_auth_descriptor2(void *buf, size_t buflen, char **auth_buffer);
+
+/* Check the format of the ESL */
+int validate_esl_list(char *key, char *esl, size_t size);
+
+/* Update the TS variable with the new timestamp */
+int update_timestamp(char *key, struct efi_time *timestamp, char *last_timestamp);
+
+/* Check the new timestamp against the timestamp last update was done */
+int check_timestamp(char *key, struct efi_time *timestamp, char *last_timestamp);
+
+/* Check the GUID of the data type */
+bool is_pkcs7_sig_format(void *data);
+
+/* Process the update */
+int process_update(struct secvar_node *update, char **newesl, int *neweslsize,
+		   struct efi_time *timestamp, struct list_head *bank,
+		   char *last_timestamp);
+
+#endif
diff --git a/libstb/secvar/backend/edk2-compat-reset.c b/libstb/secvar/backend/edk2-compat-reset.c
new file mode 100644
index 00000000..c23235b2
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat-reset.c
@@ -0,0 +1,115 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#include <opal.h>
+#include <device.h>
+#include "edk2-compat-process.h"
+#include "edk2-compat-reset.h"
+#include "../secvar.h"
+
+int reset_keystore(struct list_head *bank)
+{
+	struct secvar_node *node;
+	int rc = 0;
+
+	node = find_secvar("PK", 3, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	if (rc)
+		return rc;
+
+	node = find_secvar("KEK", 4, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	if (rc)
+		return rc;
+
+	node = find_secvar("db", 3, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	if (rc)
+		return rc;
+
+	node = find_secvar("dbx", 4, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	if (rc)
+		return rc;
+
+	node = find_secvar("TS", 3, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	if (rc)
+		return rc;
+
+	node = find_secvar("HWKH", 5, bank);
+	if (node)
+		rc = update_variable_in_bank(node->var, NULL, 0, bank);
+
+	return rc;
+}
+
+
+int add_hw_key_hash(struct list_head *bank)
+{
+	struct secvar_node *node;
+	uint32_t hw_key_hash_size;
+	const char *hw_key_hash;
+	struct dt_node *secureboot;
+
+	secureboot = dt_find_by_path(dt_root, "ibm,secureboot");
+	if (!secureboot)
+		return false;
+
+	hw_key_hash_size = dt_prop_get_u32(secureboot, "hw-key-hash-size");
+
+	hw_key_hash = dt_prop_get(secureboot, "hw-key-hash");
+
+	if (!hw_key_hash)
+		return OPAL_PERMISSION;
+
+	node = new_secvar("HWKH", 5, hw_key_hash,
+			hw_key_hash_size, SECVAR_FLAG_PROTECTED);
+	list_add_tail(bank, &node->link);
+
+	return OPAL_SUCCESS;
+}
+
+int delete_hw_key_hash(struct list_head *bank)
+{
+	struct secvar_node *node;
+	int rc;
+
+	node = find_secvar("HWKH", 5, bank);
+	if (!node)
+		return OPAL_SUCCESS;
+
+	rc = update_variable_in_bank(node->var, NULL, 0, bank);
+	return rc;
+}
+
+int verify_hw_key_hash(void)
+{
+	const char *hw_key_hash;
+	struct dt_node *secureboot;
+	struct secvar_node *node;
+
+	secureboot = dt_find_by_path(dt_root, "ibm,secureboot");
+	if (!secureboot)
+		return OPAL_INTERNAL_ERROR;
+
+	hw_key_hash = dt_prop_get(secureboot, "hw-key-hash");
+
+	if (!hw_key_hash)
+		return OPAL_INTERNAL_ERROR;
+
+	/* This value is from the protected storage */
+	node = find_secvar("HWKH", 5, &variable_bank);
+	if (!node)
+		return OPAL_PERMISSION;
+
+	if (memcmp(hw_key_hash, node->var->data, node->var->data_size) != 0)
+		return OPAL_PERMISSION;
+
+	return OPAL_SUCCESS;
+}
+
diff --git a/libstb/secvar/backend/edk2-compat-reset.h b/libstb/secvar/backend/edk2-compat-reset.h
new file mode 100644
index 00000000..bede9c9d
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat-reset.h
@@ -0,0 +1,24 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+
+#ifndef __SECVAR_EDK2_COMPAT_CLEAR_KEYS__
+#define __SECVAR_EDK2_COMPAT_CLEAR_KEYS__
+
+#ifndef pr_fmt
+#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
+#endif
+
+/* clear all os keys and the timestamp*/
+int reset_keystore(struct list_head *bank);
+
+/* Compares the hw-key-hash from device tree to the value stored in
+ * the protected storage to ensure it is not modified */
+int verify_hw_key_hash(void);
+
+/* Adds hw-key-hash */
+int add_hw_key_hash(struct list_head *bank);
+
+/* Delete hw-key-hash */
+int delete_hw_key_hash(struct list_head *bank);
+
+#endif
diff --git a/libstb/secvar/backend/edk2-compat.c b/libstb/secvar/backend/edk2-compat.c
new file mode 100644
index 00000000..334aca3e
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat.c
@@ -0,0 +1,262 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#ifndef pr_fmt
+#define pr_fmt(fmt) "EDK2_COMPAT: " fmt
+#endif
+
+#include <opal.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <skiboot.h>
+#include <ccan/endian/endian.h>
+#include <mbedtls/error.h>
+#include "libstb/crypto/pkcs7/pkcs7.h"
+#include "edk2.h"
+#include "../secvar.h"
+#include "edk2-compat-process.h"
+#include "edk2-compat-reset.h"
+
+struct list_head staging_bank;
+
+/*
+ * Initializes supported variables as empty if not loaded from
+ * storage. Variables are initialized as volatile if not found.
+ * Updates should clear this flag.
+ * Returns OPAL Error if anything fails in initialization
+ */
+static int edk2_compat_pre_process(struct list_head *variable_bank,
+				   struct list_head *update_bank __unused)
+{
+	struct secvar_node *pkvar;
+	struct secvar_node *kekvar;
+	struct secvar_node *dbvar;
+	struct secvar_node *dbxvar;
+	struct secvar_node *tsvar;
+
+	pkvar = find_secvar("PK", 3, variable_bank);
+	if (!pkvar) {
+		pkvar = new_secvar("PK", 3, NULL, 0, SECVAR_FLAG_VOLATILE
+				| SECVAR_FLAG_PROTECTED);
+		if (!pkvar)
+			return OPAL_NO_MEM;
+
+		list_add_tail(variable_bank, &pkvar->link);
+	}
+	if (pkvar->var->data_size == 0)
+		setup_mode = true;
+	else
+		setup_mode = false;
+
+	kekvar = find_secvar("KEK", 4, variable_bank);
+	if (!kekvar) {
+		kekvar = new_secvar("KEK", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
+		if (!kekvar)
+			return OPAL_NO_MEM;
+
+		list_add_tail(variable_bank, &kekvar->link);
+	}
+
+	dbvar = find_secvar("db", 3, variable_bank);
+	if (!dbvar) {
+		dbvar = new_secvar("db", 3, NULL, 0, SECVAR_FLAG_VOLATILE);
+		if (!dbvar)
+			return OPAL_NO_MEM;
+
+		list_add_tail(variable_bank, &dbvar->link);
+	}
+
+	dbxvar = find_secvar("dbx", 4, variable_bank);
+	if (!dbxvar) {
+		dbxvar = new_secvar("dbx", 4, NULL, 0, SECVAR_FLAG_VOLATILE);
+		if (!dbxvar)
+			return OPAL_NO_MEM;
+
+		list_add_tail(variable_bank, &dbxvar->link);
+	}
+
+	/* Should only ever happen on first boot. Timestamp is
+	 * initialized with all zeroes. */
+	tsvar = find_secvar("TS", 3, variable_bank);
+	if (!tsvar) {
+		tsvar = alloc_secvar(sizeof(struct efi_time) * 4);
+		if (!tsvar)
+			return OPAL_NO_MEM;
+
+		memcpy(tsvar->var->key, "TS", 3);
+		tsvar->var->key_len = 3;
+		tsvar->var->data_size = sizeof(struct efi_time) * 4;
+		memset(tsvar->var->data, 0, tsvar->var->data_size);
+		list_add_tail(variable_bank, &tsvar->link);
+	}
+
+	return OPAL_SUCCESS;
+};
+
+static int edk2_compat_process(struct list_head *variable_bank,
+			       struct list_head *update_bank)
+{
+	struct secvar_node *node = NULL;
+	struct secvar_node *tsvar = NULL;
+	struct efi_time timestamp;
+	char *newesl = NULL;
+	int neweslsize;
+	int rc = 0;
+
+	prlog(PR_INFO, "Setup mode = %d\n", setup_mode);
+
+	/* Check HW-KEY-HASH */
+	if (!setup_mode) {
+		rc = verify_hw_key_hash();
+		if (rc != OPAL_SUCCESS) {
+			prlog(PR_ERR, "Hardware key hash verification mismatch\n");
+			rc = reset_keystore(variable_bank);
+			if (rc)
+				goto cleanup;
+			setup_mode = true;
+			goto cleanup;
+		}
+	}
+
+	/* Return early if we have no updates to process */
+	if (list_empty(update_bank)) {
+		return OPAL_EMPTY;
+	}
+
+	/* Make a working copy of variable bank that is updated
+	 * during process */
+	list_head_init(&staging_bank);
+	copy_bank_list(&staging_bank, variable_bank);
+
+	/* Loop through each command in the update bank.
+	 * If any command fails, it just loops out of the update bank.
+	 * It should also clear the update bank.
+	 */
+
+	list_for_each(update_bank, node, link) {
+
+		/* Submitted data is auth_2 descriptor + new ESL data
+		 * Extract the auth_2 2 descriptor
+		 */
+		prlog(PR_INFO, "Update for %s\n", node->var->key);
+
+		tsvar = find_secvar("TS", 3, &staging_bank);
+
+		/* We cannot find timestamp variable, did someone tamper it? */
+	        if (!tsvar) {
+			rc = OPAL_PERMISSION;
+			break;
+		}
+
+		rc = process_update(node, &newesl,
+				    &neweslsize, &timestamp,
+				    &staging_bank,
+				    tsvar->var->data);
+		if (rc) {
+			prlog(PR_ERR, "Update processing failed with rc %04x\n", rc);
+			break;
+		}
+
+		/*
+		 * If reached here means, signature is verified so update the
+		 * value in the variable bank
+		 */
+		rc = update_variable_in_bank(node->var,
+					     newesl,
+					     neweslsize,
+					     &staging_bank);
+		if (rc) {
+			prlog(PR_ERR, "Updating the variable data failed %04x\n", rc);
+			break;
+		}
+
+		free(newesl);
+		/* Update the TS variable with the new timestamp */
+		rc = update_timestamp(node->var->key,
+				      &timestamp,
+				      tsvar->var->data);
+		if (rc) {
+			prlog (PR_ERR, "Variable updated, but timestamp updated failed %04x\n", rc);
+			break;
+		}
+
+		/* If the PK is updated, update the secure boot state of the
+		 * system at the end of processing */
+		if (key_equals(node->var->key, "PK")) {
+			/* PK is tied to a particular firmware image by mapping
+			 * it with hw-key-hash of that firmware. When PK is
+			 * updated, hw-key-hash is updated. And when PK is
+			 * deleted, delete hw-key-hash as well */
+			if(neweslsize == 0) {
+				setup_mode = true;
+				delete_hw_key_hash(&staging_bank);
+			} else  {
+				setup_mode = false;
+				add_hw_key_hash(&staging_bank);
+			}
+			prlog(PR_DEBUG, "setup mode is %d\n", setup_mode);
+		}
+	}
+
+	if (rc == 0) {
+		/* Update the variable bank with updated working copy */
+		clear_bank_list(variable_bank);
+		copy_bank_list(variable_bank, &staging_bank);
+	}
+
+cleanup:
+	/* For any failure in processing update queue, we clear the update bank
+	 * and return failure */
+	clear_bank_list(update_bank);
+
+	return rc;
+}
+
+static int edk2_compat_post_process(struct list_head *variable_bank,
+				    struct list_head *update_bank __unused)
+{
+	struct secvar_node *hwvar;
+	if (!setup_mode) {
+		secvar_set_secure_mode();
+		prlog(PR_INFO, "Enforcing OS secure mode\n");
+		/* HW KEY HASH is no more needed after this point. It is already
+		 * visible to userspace via device-tree, so exposing via sysfs is
+		 * just a duplication. Remove it from in-memory copy. */
+		hwvar = find_secvar("HWKH", 5, variable_bank);
+		if (!hwvar) {
+			prlog(PR_ERR, "cannot find hw-key-hash, should not happen\n");
+			return OPAL_INTERNAL_ERROR;
+		}
+		list_del(&hwvar->link);
+		dealloc_secvar(hwvar);
+	}
+
+	return OPAL_SUCCESS;
+}
+
+static int edk2_compat_validate(struct secvar *var)
+{
+
+	/* Checks if the update is for supported
+	 * Non-volatile secure variables */
+	if (!key_equals(var->key, "PK")
+			&& !key_equals(var->key, "KEK")
+			&& !key_equals(var->key, "db")
+			&& !key_equals(var->key, "dbx"))
+		return OPAL_PARAMETER;
+
+	/* Check that signature type is PKCS7 */
+	if (!is_pkcs7_sig_format(var->data))
+		return OPAL_PARAMETER;
+
+	return OPAL_SUCCESS;
+};
+
+struct secvar_backend_driver edk2_compatible_v1 = {
+	.pre_process = edk2_compat_pre_process,
+	.process = edk2_compat_process,
+	.post_process = edk2_compat_post_process,
+	.validate = edk2_compat_validate,
+	.compatible = "ibm,edk2-compat-v1",
+};
diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h
new file mode 100644
index 00000000..fb0121af
--- /dev/null
+++ b/libstb/secvar/backend/edk2.h
@@ -0,0 +1,243 @@ 
+/* Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved. This
+ * program and the accompanying materials are licensed and made available
+ * under the terms and conditions of the 2-Clause BSD License which
+ * accompanies this distribution.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * This file is derived from the following files referred from edk2-staging[1] repo
+ * of tianocore
+ *
+ * MdePkg/Include/Guid/GlobalVariable.h
+ * MdePkg/Include/Guid/WinCertificate.h
+ * MdePkg/Include/Uefi/UefiMultiPhase.h
+ * MdePkg/Include/Uefi/UefiBaseType.h
+ * MdePkg/Include/Guid/ImageAuthentication.h
+ *
+ * [1] https://github.com/tianocore/edk2-staging
+ *
+ * Copyright 2020 IBM Corp.
+ */
+
+#ifndef __EDK2_H__
+#define __EDK2_H__
+
+#define UUID_SIZE 16
+
+typedef struct {
+        u8 b[UUID_SIZE];
+} uuid_t;
+
+#define EFI_GLOBAL_VARIABLE_GUID (uuid_t){{0x61, 0xDF, 0xe4, 0x8b, 0xca, 0x93, 0xd2, 0x11, 0xaa, \
+			 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c}}
+
+#define EFI_IMAGE_SECURITY_DATABASE_GUID (uuid_t){{0xcb, 0xb2, 0x19, 0xd7, 0x3a, 0x3d, 0x96, 0x45, \
+					   0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f}}
+
+#define SECVAR_ATTRIBUTES	39
+
+///
+/// This identifies a signature based on an X.509 certificate. If the signature is an X.509
+/// certificate then verification of the signature of an image should validate the public
+/// key certificate in the image using certificate path verification, up to this X.509
+/// certificate as a trusted root.  The SignatureHeader size shall always be 0. The
+/// SignatureSize may vary but shall always be 16 (size of the SignatureOwner component) +
+/// the size of the certificate itself.
+/// Note: This means that each certificate will normally be in a separate EFI_SIGNATURE_LIST.
+///
+
+#define EFI_CERT_RSA2048_GUID \
+  (UUID_INIT) (0x3c5766e8, 0x269c, 0x4e34, 0xaa, 0x14, 0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6)
+
+#define EFI_CERT_TYPE_PKCS7_GUID (uuid_t){{0x9d, 0xd2, 0xaf, 0x4a, 0xdf, 0x68, 0xee, 0x49, \
+					   0x8a, 0xa9, 0x34, 0x7d, 0x37, 0x56, 0x65, 0xa7}}
+
+#define EFI_VARIABLE_NON_VOLATILE				0x00000001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS				0x00000002
+#define EFI_VARIABLE_RUNTIME_ACCESS				0x00000004
+
+/*
+ * Attributes of Authenticated Variable
+ */
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS	0x00000020
+#define EFI_VARIABLE_APPEND_WRITE				0x00000040
+/*
+ * NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be
+ * considered reserved.
+ */
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS			0x00000010
+
+/*
+ * win_certificate.w_certificate_type
+ */
+#define WIN_CERT_TYPE_PKCS_SIGNED_DATA	0x0002
+
+#define SECURE_BOOT_MODE_ENABLE           1
+#define SECURE_BOOT_MODE_DISABLE          0
+///
+/// Depricated value definition for SetupMode variable
+///
+#define SETUP_MODE                        1
+#define USER_MODE                         0
+
+/*
+ * EFI Time Abstraction:
+ *   Year:       1900 - 9999
+ *   Month:      1 - 12
+ *   Day:        1 - 31
+ *   Hour:       0 - 23
+ *   Minute:     0 - 59
+ *   Second:     0 - 59
+ *   Nanosecond: 0 - 999,999,999
+ *   TimeZone:   -1440 to 1440 or 2047
+ */
+struct efi_time {
+	u16 year;
+	u8 month;
+	u8 day;
+	u8 hour;
+	u8 minute;
+	u8 second;
+	u8 pad1;
+	u32 nanosecond;
+	s16 timezone;
+	u8 daylight;
+	u8 pad2;
+};
+//***********************************************************************
+// Signature Database
+//***********************************************************************
+///
+/// The format of a signature database.
+///
+#pragma pack(1)
+
+typedef struct {
+  ///
+  /// An identifier which identifies the agent which added the signature to the list.
+  ///
+  uuid_t SignatureOwner;
+  ///
+  /// The format of the signature is defined by the SignatureType.
+  ///
+  unsigned char SignatureData[0];
+} EFI_SIGNATURE_DATA;
+
+typedef struct {
+  ///
+  /// Type of the signature. GUID signature types are defined in below.
+  ///
+  uuid_t SignatureType;
+  ///
+  /// Total size of the signature list, including this header.
+  ///
+  uint32_t	SignatureListSize;
+  ///
+  /// Size of the signature header which precedes the array of signatures.
+  ///
+  uint32_t	SignatureHeaderSize;
+  ///
+  /// Size of each signature.
+  ///
+  uint32_t	SignatureSize;
+  ///
+  /// Header before the array of signatures. The format of this header is specified
+  /// by the SignatureType.
+  /// UINT8           SignatureHeader[SignatureHeaderSize];
+  ///
+  /// An array of signatures. Each signature is SignatureSize bytes in length.
+  /// EFI_SIGNATURE_DATA Signatures[][SignatureSize];
+  ///
+} EFI_SIGNATURE_LIST;
+
+
+/*
+ * The win_certificate structure is part of the PE/COFF specification.
+ */
+struct win_certificate {
+	/*
+	 * The length of the entire certificate, including the length of the
+	 * header, in bytes.
+	 */
+	u32  dw_length;
+	/*
+	 * The revision level of the WIN_CERTIFICATE structure. The current
+	 * revision level is 0x0200.
+	 */
+	u16  w_revision;
+	/*
+	 * The certificate type. See WIN_CERT_TYPE_xxx for the UEFI certificate
+	 * types. The UEFI specification reserves the range of certificate type
+	 * values from 0x0EF0 to 0x0EFF.
+	 */
+	u16  w_certificate_type;
+	/*
+	 * The following is the actual certificate. The format of
+	 * the certificate depends on wCertificateType.
+	 */
+	/// UINT8 bCertificate[ANYSIZE_ARRAY];
+};
+
+/*
+ * Certificate which encapsulates a GUID-specific digital signature
+ */
+struct win_certificate_uefi_guid {
+	/*
+	 * This is the standard win_certificate header, where w_certificate_type
+	 * is set to WIN_CERT_TYPE_EFI_GUID.
+	 */
+	struct win_certificate hdr;
+	/*
+	 * This is the unique id which determines the format of the cert_data.
+	 */
+	uuid_t cert_type;
+	/*
+	 * The following is the certificate data. The format of the data is
+	 * determined by the @cert_type. If @cert_type is
+	 * EFI_CERT_TYPE_RSA2048_SHA256_GUID, the @cert_data will be
+	 * EFI_CERT_BLOCK_RSA_2048_SHA256 structure.
+	 */
+	u8 cert_data[1];
+};
+/*
+ * When the attribute EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS is set,
+ * then the Data buffer shall begin with an instance of a complete (and
+ * serialized) EFI_VARIABLE_AUTHENTICATION_2 descriptor. The descriptor shall be
+ * followed by the new variable value and DataSize shall reflect the combined
+ * size of the descriptor and the new variable value. The authentication
+ * descriptor is not part of the variable data and is not returned by subsequent
+ * calls to GetVariable().
+ */
+struct efi_variable_authentication_2 {
+	/*
+	 * For the TimeStamp value, components Pad1, Nanosecond, TimeZone, Daylight and
+	 * Pad2 shall be set to 0. This means that the time shall always be expressed in GMT.
+	 */
+	struct efi_time timestamp;
+	/*
+	 * Only a CertType of  EFI_CERT_TYPE_PKCS7_GUID is accepted.
+	 */
+	struct win_certificate_uefi_guid auth_info;
+};
+
+#endif