diff mbox series

[v2,09/12] secvar/backend: add edk2 derived key updates processing

Message ID 20200120023700.5373-10-erichte@linux.ibm.com
State Superseded
Headers show
Series Add initial secure variable storage and backend drivers | expand

Checks

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

Commit Message

Eric Richter Jan. 20, 2020, 2:36 a.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

[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>
---

V5:
 - Finalizes the previous version to a complete version taking care
of validation, multiple ESLs, single PK, dbx support, timestamp checks and
failure handling.

 doc/secvar/edk2.rst                 |  49 ++
 include/secvar.h                    |   1 +
 libstb/secvar/backend/Makefile.inc  |   4 +-
 libstb/secvar/backend/edk2-compat.c | 877 ++++++++++++++++++++++++++++
 libstb/secvar/backend/edk2.h        | 243 ++++++++
 5 files changed, 1172 insertions(+), 2 deletions(-)
 create mode 100644 doc/secvar/edk2.rst
 create mode 100644 libstb/secvar/backend/edk2-compat.c
 create mode 100644 libstb/secvar/backend/edk2.h

Comments

Stefan Berger Jan. 23, 2020, 1:55 a.m. UTC | #1
On 1/19/20 9:36 PM, 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
>
> [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>
> ---
>
> V5:
>   - Finalizes the previous version to a complete version taking care
> of validation, multiple ESLs, single PK, dbx support, timestamp checks and
> failure handling.
>
>   doc/secvar/edk2.rst                 |  49 ++
>   include/secvar.h                    |   1 +
>   libstb/secvar/backend/Makefile.inc  |   4 +-
>   libstb/secvar/backend/edk2-compat.c | 877 ++++++++++++++++++++++++++++
>   libstb/secvar/backend/edk2.h        | 243 ++++++++
>   5 files changed, 1172 insertions(+), 2 deletions(-)
>   create mode 100644 doc/secvar/edk2.rst
>   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..e0c29457
> --- /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


You intend to fill these ?


> diff --git a/include/secvar.h b/include/secvar.h
> index 2875c700..8b701e00 100644
> --- a/include/secvar.h
> +++ b/include/secvar.h
> @@ -24,6 +24,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 cc1a49fa..1c1896ab 100644
> --- a/libstb/secvar/backend/Makefile.inc
> +++ b/libstb/secvar/backend/Makefile.inc
> @@ -1,11 +1,11 @@
>   # SPDX-License-Identifier: Apache-2.0
>   # -*-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
>   SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
>   SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
>   
> diff --git a/libstb/secvar/backend/edk2-compat.c b/libstb/secvar/backend/edk2-compat.c
> new file mode 100644
> index 00000000..b99738b1
> --- /dev/null
> +++ b/libstb/secvar/backend/edk2-compat.c
> @@ -0,0 +1,877 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2019 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 "libstb/crypto/pkcs7/pkcs7.h"
> +#include "edk2.h"
> +#include "opal-api.h"
> +#include "../secvar.h"
> +#include "../secvar_devtree.h"
> +#include "../secvar_tpmnv.h"
> +#include <mbedtls/error.h>
> +
> +#define TPMNV_ID_EDK2_PK	0x4532504b // E2PK
> +
> +static bool setup_mode;
> +
> +//struct efi_time *timestamp_list;


remove if not needed


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

return !strcmp(key1, key2);


> +	if (memcmp(key1, key2, strlen(key2)+1) == 0)
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + * 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;
> +
> +	memset(ret, 0, sizeof(char *) * 3);


Shouldn't be necessary since you are NULL-terminating the array and 
presumably all callers stop once they see the NULL.


> +	if (key_equals(key, "PK"))
> +		ret[i++] = "PK";
> +	if (key_equals(key, "KEK"))
> +		ret[i++] = "PK";
> +	if (key_equals(key, "db") || key_equals(key, "dbx")) {
> +		ret[i++] = "KEK";
> +		ret[i++] = "PK";
> +	}
> +	ret[i] = NULL;
> +}
> +
> +/*
> + * PK needs to be stored in the TPMNV space if on p9

> + * We store it using the form <u64:esl size><esl data>, the
> + * extra secvar headers are unnecessary
> + */
> +static int edk2_p9_load_pk(void)
> +{
> +	struct secvar_node *pkvar;
> +	uint64_t size;
> +	int rc;
> +
> +	// Ensure it exists
> +	rc = secvar_tpmnv_alloc(TPMNV_ID_EDK2_PK, -1);


check rc now that you have it


> +
> +	// Peek to get the size
> +	rc = secvar_tpmnv_read(TPMNV_ID_EDK2_PK, &size, sizeof(size), 0);
> +	if (rc == OPAL_EMPTY)
> +		return 0;
Is this OPAL_SUCCESS?
> +	else if (rc)
> +		return -1;
Isn't there a better return code for 0 and -1. Below you return OPAL_* 
return codes.
> +
> +	if (size > secvar_storage.max_var_size)
> +		return OPAL_RESOURCE;
> +
> +	pkvar = alloc_secvar(size);

NULL ptr check


> +	memcpy(pkvar->var->key, "PK", 3);
> +	pkvar->var->key_len = 3;
> +	pkvar->var->data_size = size;
> +	pkvar->flags |= SECVAR_FLAG_VOLATILE;
> +
> +	rc = secvar_tpmnv_read(TPMNV_ID_EDK2_PK, pkvar->var->data, pkvar->var->data_size, sizeof(pkvar->var->data_size));
> +	if (rc)
> +		return rc;
this returns an OPAL_ error code following what I see in secvar_tpmnv_read
> +
> +	list_add_tail(&variable_bank, &pkvar->link);
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +/*
> + * Writes the PK to the TPM.
> + */
> +static int edk2_p9_write_pk(void)
> +{
> +	char *tmp;
> +	int32_t tmpsize;
> +	struct secvar_node *pkvar;
> +	int rc;
> +
> +	pkvar = find_secvar("PK", 3, &variable_bank);
find_secvar cannot do a strlen() + 1 on the name? All callers seem to 
have to pass the length of the string + 1.
> +
> +	// Should not happen
> +	if (!pkvar)
> +		return OPAL_INTERNAL_ERROR;
> +
> +	// Reset the pk flag to volatile on p9
> +	pkvar->flags |= SECVAR_FLAG_VOLATILE;
> +
> +	tmpsize = secvar_tpmnv_size(TPMNV_ID_EDK2_PK);
> +	if (tmpsize < 0) {
> +		prlog(PR_ERR, "TPMNV space for PK was not allocated properly\n");
> +		return OPAL_RESOURCE;
> +	}
> +	if (tmpsize < pkvar->var->data_size + sizeof(pkvar->var->data_size)) {
> +		prlog(PR_ERR, "TPMNV PK space is insufficient, %d < %llu\n", tmpsize,
> +			// Cast needed because x86 compiler complains building the test
> +			(long long unsigned) pkvar->var->data_size + sizeof(pkvar->var->data_size));
> +		return OPAL_RESOURCE;
> +	}
> +
> +	tmp = zalloc(tmpsize);
a rare NULL pointer check; good
> +	if (!tmp)
> +		return OPAL_NO_MEM;
> +
> +	memcpy(tmp, &pkvar->var->data_size, sizeof(pkvar->var->data_size));
> +	memcpy(tmp + sizeof(pkvar->var->data_size),
> +		pkvar->var->data,
> +		pkvar->var->data_size);
> +
> +	tmpsize = pkvar->var->data_size + sizeof(pkvar->var->data_size);
> +
> +	rc = secvar_tpmnv_write(TPMNV_ID_EDK2_PK, tmp, tmpsize, 0);
> +
> +	free(tmp);
> +
> +	return rc;
> +}
> +
> +/*
> + * Returns the size of the ESL.
> + */
> +static int get_esl_signature_list_size(char *buf)
should probably have a size_t buflen ...
> +{
> +	EFI_SIGNATURE_LIST list;
> +
> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
... so you know you enough bytes to copy ...
> +
> +	prlog(PR_DEBUG, "size of signature list size is %u\n", le32_to_cpu(list.SignatureListSize));
> +
> +	return le32_to_cpu(list.SignatureListSize);
... and access this field here
> +}
> +
> +/*
> + * Returns the size of the certificate contained in the ESL.
> + */
> +static int get_esl_cert_size(char *buf)
Same thing here: size_t buflen
> +{
> +	EFI_SIGNATURE_LIST list;
> +	uint32_t sigsize;
> +
> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
> +
> +	sigsize = le32_to_cpu(list.SignatureListSize) - sizeof(list)
> +		- le32_to_cpu(list.SignatureHeaderSize) - sizeof(uuid_t);
> +
> +	prlog(PR_DEBUG, "sig size is %u\n", sigsize);
> +	return sigsize;
> +}
> +
> +/*
> + * Copies the certificate from the ESL into cert buffer.
> + */
> +static int get_esl_cert(char *buf, char **cert)
Also here...
> +{
> +	int sig_data_offset;
> +	int size;
> +	EFI_SIGNATURE_LIST list;
> +
> +	memset(&list, 0, sizeof(EFI_SIGNATURE_LIST));
> +	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));

Memset above seems useless ; just heats up memory :-)


> +
> +	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.SignatureType)
> +		+ sizeof(list.SignatureListSize)
> +		+ sizeof(list.SignatureHeaderSize)
> +		+ sizeof(list.SignatureSize)
> +		+ le32_to_cpu(list.SignatureHeaderSize)
> +		+ 16 * sizeof(uint8_t);
> +
> +	size = le32_to_cpu(list.SignatureSize) - sizeof(uuid_t);
> +	
> +	memcpy(*cert, buf + sig_data_offset, size);
Why do you need **cert here is you don't allocated the memory for it? Do 
you know *cert is big enough for this 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 = le32_to_cpu(auth->auth_info.hdr.dw_length);
> +	int size;

ssize_t here or size_t ?


> +
> +	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;
Better not be <0 ?
> +}
> +
> +/*
> + * Return the timestamp from the Authentication 2 Descriptor.
> + */
> +static int get_timestamp_from_auth(char *data, struct efi_time **timestamp)
> +{
> +	*timestamp = (struct efi_time *) data;

I trust you can just cast it like this. Without any checks it that it 
does it could just be

struct efi_time *get_timestamp_from_auth(char *data)

But this is just a simple cast you could do where it appears. So I doubt 
you need the function unless you wanted to do some sanity checks here.

> +
> +	return 0;
> +}
> +
> +/*
> + * This function outputs the Authentication 2 Descriptor in the
> + * auth_buffer and returns the size of the buffer.
> + */
> +static int get_auth_descriptor2(void *data, char **auth_buffer)
size_t datalen ??
> +{
> +	struct efi_variable_authentication_2 *auth = data;
> +	uint64_t auth_buffer_size;
> +	int len;
ssize_t len
> +
> +	if (!auth_buffer)
> +		return OPAL_PARAMETER;
> +
> +	len = get_pkcs7_len(auth);
> +	if (len < 0)
> +		return OPAL_NO_MEM;
> +
> +	auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
> +			   + sizeof(auth->auth_info.cert_type) + len;
> +
> +	*auth_buffer = zalloc(auth_buffer_size);
> +	if (!(*auth_buffer))
> +		return OPAL_NO_MEM;
> +
> +	memcpy(*auth_buffer, data, auth_buffer_size);
Are you sure you have enough bytes in data?
> +
> +	return auth_buffer_size;
> +}
> +
> +/* Check that PK has single ESL */
> +static bool is_single_pk(char *data, uint64_t data_size)
> +{
> +	char *auth_buffer = NULL;
> +	uint64_t auth_buffer_size = 0;
> +	char *newesl = NULL;
> +	uint64_t new_data_size = 0;
> +	int esllistsize;
> +
> +	auth_buffer_size = get_auth_descriptor2(data, &auth_buffer);
> +	printf("auth buffer size is %d\n", (int)auth_buffer_size);
this is probably debug-only output
> +	free(auth_buffer);
> +	if (auth_buffer_size <= 0)
> +		return false;
> +
> +	/* Calculate the size of new ESL data */
> +	new_data_size = data_size - auth_buffer_size;
> +	printf("new data size is %d\n", (int)new_data_size);
> +
> +	if (!new_data_size)
> +		return true;
> +
> +	newesl = zalloc(new_data_size);
NULL check
> +	memcpy(newesl, data + auth_buffer_size, new_data_size);
> +
> +	esllistsize = get_esl_signature_list_size(newesl);
> +	printf("esl list size is %d\n", esllistsize);
> +	free(newesl);
> +	if (new_data_size > esllistsize)
> +		return false;
> +
> +	return true;
> +}
> +
> +/*
> + * Initializes supported variables as empty if not loaded from
> + * storage. Variables are initialized as volatile if not found.
> + * Updates should clear this flag.
> +ec*
> + * Returns OPAL Error if anything fails in initialization
> + */
> +static int edk2_compat_pre_process(void)
> +{
> +	struct secvar_node *pkvar;
> +	struct secvar_node *kekvar;
> +	struct secvar_node *dbvar;
> +	struct secvar_node *dbxvar;
> +	struct secvar_node *tsvar;
> +
> +	// If we are on p9, we need to store the PK in write-lockable
> +	//  TPMNV space, as we determine our secure mode based on if this
> +	//  variable exists.
> +	// NOTE: Activation of this behavior is subject to change in a later
> +	//  patch version, ideally the platform should be able to configure
> +	//  whether it wants this extra protection, or to instead store
> +	//  everything via the storage driver.
> +	if (proc_gen == proc_gen_p9)
> +		edk2_p9_load_pk();
> +
> +	pkvar = find_secvar("PK", 3, &variable_bank);
> +	if (!pkvar) {
> +		pkvar = alloc_secvar(0);
> +		if (!pkvar)
> +			return OPAL_NO_MEM;
> +
> +		memcpy(pkvar->var->key, "PK", 3);
> +		pkvar->var->key_len = 3;

a helper function like secvar_new(0, "PK", 3, SECVAR_FLAG_VOLATILE) 
would be good ...

better yet: don't pass '3' but let alloc_secvar run strlen(). Is there a 
reason why pkvar->var->key_len is needed? It looks like key always has a 
string and key_len is strlen() + 1. Seems redundant to have to set key_len.

> +		pkvar->flags |= SECVAR_FLAG_VOLATILE;
> +		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 = alloc_secvar(0);
> +		if (!kekvar)
> +			return OPAL_NO_MEM;
> +
> +		memcpy(kekvar->var->key, "KEK", 4);
> +		kekvar->var->key_len = 4;
> +		kekvar->flags |= SECVAR_FLAG_VOLATILE;
... would save some code
> +		list_add_tail(&variable_bank, &kekvar->link);
> +	}
> +
> +	dbvar = find_secvar("db", 3, &variable_bank);
> +	if (!dbvar) {
> +		dbvar = alloc_secvar(0);
> +		if (!dbvar)
> +			return OPAL_NO_MEM;
> +
> +		memcpy(dbvar->var->key, "db", 3);
> +		dbvar->var->key_len = 3;
> +		dbvar->flags |= SECVAR_FLAG_VOLATILE;
again
> +		list_add_tail(&variable_bank, &dbvar->link);
> +	}
> +
> +	dbxvar = find_secvar("dbx", 4, &variable_bank);
> +	if (!dbxvar) {
> +		dbxvar = alloc_secvar(0);
> +		if (!dbxvar)
> +			return OPAL_NO_MEM;
> +
> +		memcpy(dbxvar->var->key, "dbx", 4);
> +		dbxvar->var->key_len = 4;
> +		dbxvar->flags |= SECVAR_FLAG_VOLATILE;
and again
> +		list_add_tail(&variable_bank, &dbxvar->link);
> +	}
> +
> +	tsvar = find_secvar("TS", 3, &variable_bank);
> +	// Should only ever happen on first boot
> +	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;
yet again
> +		tsvar->var->data_size = sizeof(struct efi_time) * 4;
> +		memset(tsvar->var->data, 0, tsvar->var->data_size);
also allow passing data_size and data to this secvar_new()
> +		//tsvar->flags |= SECVAR_FLAG_VOLATILE;
commented ?
> +		list_add_tail(&variable_bank, &tsvar->link);
> +	}
> +
> +	return OPAL_SUCCESS;
> +};
> +
> +/**
> + * Returns true if we are in Setup Mode
> + *
> + * Setup Mode is active if we have no PK.
> + * Otherwise, we are in user mode.
> + */
> +/**
> +static int is_setup_mode(void)
bool would be better because you say you want to return 'true'
> +{
> +	struct secvar_node *setup;
> +
> +	setup = find_secvar((char *)"PK", 3, &variable_bank);
why cast ?
> +
> +	// Not sure why this wouldn't exist
> +	if (!setup)
> +		return 1;
true ?
> +
> +	return !setup->var->data_size;
> +}
> +**/
> +
> +/**
> + * Update the variable with the new value.
> + */
> +static int add_to_variable_bank(struct secvar *secvar, void *data, uint64_t dsize)
> +{
> +	struct secvar_node *node;
> +
> +	node = find_secvar(secvar->key, secvar->key_len, &variable_bank);
> +	if (!node)
> +		return OPAL_INTERNAL_ERROR;
> +
> +	// Expand the secvar allocated memory if needed
> +	if (node->size < dsize)
> +		if (realloc_secvar(node, dsize))


Is this call correct? Shouldn't it be node = realloc_secvar(node, dsize) 
or  realloc_secvar(&node, dsize)?


> +			return OPAL_NO_MEM;
> +
> +	node->var->data_size = dsize;
> +	memcpy(node->var->data, data, dsize);
> +	node->flags &= ~SECVAR_FLAG_VOLATILE; // Clear the volatile bit when updated
> +
> +	return 0;
OPAL_SUCCESS?
> +}
> +
> +static struct efi_time *get_last_timestamp(char *key)

const char *key


> +{
> +	struct secvar_node *node;
> +	struct efi_time *prev;
> +	char *timestamp_list;
> +	u8 off;
> +
> +	node = find_secvar("TS", 3, &variable_bank);
node NULL check ?
> +	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;	// unexpected variable name?
> +
> +	timestamp_list = node->var->data;
> +	if (!timestamp_list)
> +		return NULL;
> +
> +	prev = (struct efi_time *) (timestamp_list + (off * sizeof(struct efi_time)));

Looks like you are accessing an array

return &((struct efi_time *)timestamplist)[index];



> +
> +	return prev;
> +}
> +
> +// Update the TS variable with the new timestamp
Sometimes comments are with /* */, this one is //. I guess you need to 
be more consistent.
> +static int update_timestamp(char *key, struct efi_time *timestamp)
> +{
> +	struct efi_time *prev;
> +
> +	prev = get_last_timestamp(key);
> +	if (prev == NULL)
> +		return OPAL_PARAMETER;
> +
> +	memcpy(prev, timestamp, sizeof(struct efi_time));
> +
> +	printf("updated prev year is %d month %d day %d\n", le16_to_cpu(prev->year), prev->month, prev->day);
> +//	add_to_variable_bank(node->var, timestamp_list, node->var->data_size);
remove if not needed
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +static int check_timestamp(char *key, struct efi_time *timestamp)
> +{
> +	struct efi_time *prev;
> +	char *current = NULL;
> +	char *last =NULL;
> +	int s1 = 0;
> +
> +	prev = get_last_timestamp(key);
> +	if (prev == NULL)
> +		return OPAL_PARAMETER;
> +
> +	printf("timestamp year is %d month %d day %d\n", le16_to_cpu(timestamp->year), timestamp->month, timestamp->day);
> +	printf("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;
> +
> +	current = &(timestamp->month);
> +	last = &(prev->month);
> +
> +	s1 = memcmp(current, last, 5);
there must be something better than magic '5'; some sizeof() would look 
better. What are you comparing? Put a comment there. It seems to be the 
order of month, day, hours, minutes and seconds.
> +	if (s1 <= 0) {
> +		printf("s1 is %d\n", s1);

remove debug output

Equal time is reason for OPAL_PERMISSION?

> +		return OPAL_PERMISSION;
> +	}
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +/*
> + * Verify the PKCS7 signature on the signed data.
> + */
> +static int verify_signature(void *auth_buffer, char *newcert,
> +		uint64_t new_data_size, struct secvar *avar)
> +{
> +	struct efi_variable_authentication_2 *auth;
> +	mbedtls_pkcs7 *pkcs7;
> +	mbedtls_x509_crt x509;
> +	char *checkpkcs7cert;
> +	char *signing_cert = NULL;
> +	char *x509_buf;
> +	int len;
> +	int signing_cert_size;
> +	int rc;
> +	char *errbuf;
> +	int eslvarsize;
> +	int offset = 0;
> +
> +	auth = auth_buffer;
you know you have enough bytes for this cast here ? you cannot just pass 
in struct efi_variable_..._2 * ?
> +	len  = get_pkcs7_len(auth);
above you compare if (len < 0) --- do this also here.
> +	pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
NULL pointer check
> +	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);
> +		goto pkcs7out;
> +	}
> +
> +	checkpkcs7cert = zalloc(2048);
NULL pointer check
> +	mbedtls_x509_crt_info(checkpkcs7cert, 2048, "CRT:", &(pkcs7->signed_data.certs));	
Does this have a return value?
> +	prlog(PR_DEBUG, "%s \n", checkpkcs7cert);
> +	free(checkpkcs7cert);
> +
> +	prlog(PR_INFO, "Load the signing certificate from the keystore");
> +
> +	eslvarsize = avar->data_size;
> +
> +	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;
> +
> +		signing_cert_size = get_esl_cert_size(avar->data + offset);
> +		if (!signing_cert_size) {
> +			rc = OPAL_PERMISSION;



> +			break;	
> +		}
> +
> +		signing_cert = zalloc(signing_cert_size);
NULL pointer check... also further below.
> +		get_esl_cert(avar->data + offset, &signing_cert);
> +
> +		mbedtls_x509_crt_init(&x509);
> +		rc = mbedtls_x509_crt_parse(&x509, signing_cert, signing_cert_size);
> +
> +		/* If failure in parsing the certificate, try next */
> +		if(rc) {
> +			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
> +			goto next;
> +		}
> +
> +		x509_buf = zalloc(2048);

unsigned char x509_buf[2048];


> +		mbedtls_x509_crt_info(x509_buf, 2048, "CRT:", &x509);
Check return code ?
> +		prlog(PR_INFO, "%s \n", x509_buf);
> +		free(x509_buf);
> +		rc = mbedtls_pkcs7_signed_data_verify(pkcs7, &x509, newcert, new_data_size);
> +
> +		/* If find a signing certificate, you are done */
If you find ...
> +		if (rc == 0) {
> +			if (signing_cert)
> +				free(signing_cert);
No need to check for NULL.
> +			mbedtls_x509_crt_free(&x509);
> +			prlog(PR_INFO, "Signature Verification passed\n");
> +			break;
> +		}
> +
> +		errbuf = zalloc(1024);

this one you could have allocated above

char errbuf[1024];

> +		mbedtls_strerror(rc, errbuf, 1024);
> +		prlog(PR_INFO, "Signature Verification failed %02x %s\n", rc, errbuf);
> +		free(errbuf);
> +
> +next:
> +		offset += get_esl_signature_list_size(avar->data + offset);
> +		eslvarsize = eslvarsize - offset;
> +		mbedtls_x509_crt_free(&x509);
> +		if (signing_cert)
> +			free(signing_cert);
again no need to check, just free()
> +
> +	}
> +
> +pkcs7out:
> +	mbedtls_pkcs7_free(pkcs7);
IMO, this function should also free(pkcs7).
> +	free(pkcs7);
this should not be necessary
> +
> +	return rc;
> +}
> +
> +
> +/**
> + * Create the single buffer
> + * name || vendor guid || attributes || timestamp || newcontent
> + * which is submitted as signed by the user.
> + */
> +static int get_data_to_verify(char *key, char *new_data,
Return type should be char *, returning NULL if OOM.
> +		uint64_t new_data_size,
size_t
> +		char **buffer,
> +		uint64_t *buffer_size, struct efi_time *timestamp)
size_t
> +{
> +	le32 attr = cpu_to_le32(SECVAR_ATTRIBUTES);
> +	int size = 0;
> +	int varlen;
> +	char *wkey;
> +	uuid_t guid;
> +
> +	if (key_equals(key, "PK")
> +	    || key_equals(key, "KEK"))
> +		guid = EFI_GLOBAL_VARIABLE_GUID;
> +
> +	if (key_equals(key, "db")
else if
> +	    || key_equals(key, "dbx"))
> +		guid = EFI_IMAGE_SECURITY_DATABASE_GUID;
> +		
> +	// Convert utf8 name to ucs2 width
> +	varlen = strlen(key) * 2;
varlen should be size_t
> +	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);
null ptr check
> +
> +	memcpy(*buffer + size, wkey, varlen);
rename size to offset, probably of type off_t ?
> +	size = size + varlen;
> +	memcpy(*buffer + size, &guid, sizeof(guid));
> +	size = size + sizeof(guid);
> +	memcpy(*buffer + size, &attr, sizeof(attr));
> +	size = size + sizeof(attr);
> +	memcpy(*buffer + size, timestamp , sizeof(struct efi_time));
> +	size = size + sizeof(struct efi_time);
> +
> +	memcpy(*buffer + size, new_data, new_data_size);
> +	size = size + new_data_size;
this last one isn't necessary anymore
> +
> +	free(wkey);
> +
> +	return 0;
> +}
> +
> +static int edk2_compat_process(void)
> +{
> +	char *auth_buffer = NULL;
> +	uint64_t auth_buffer_size = 0;
size_t
> +	struct efi_time *timestamp = NULL;
> +	const char *key_authority[3];
> +	char *newesl = NULL;
> +	uint64_t new_data_size = 0;
size_t
> +	char *tbhbuffer = NULL;
> +	uint64_t tbhbuffersize = 0;
> +	struct secvar_node *anode = NULL;
> +	struct secvar_node *node = NULL;
> +	int rc = 0;
> +	int pk_updated = 0;
> +	int i;
> +
> +	//setup_mode = is_setup_mode();
> +	prlog(PR_INFO, "Setup mode = %d\n", setup_mode);
> +
> +	/* 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
> +		 */
> + 		printf("setup mode is %d\n", setup_mode);		
> +		prlog(PR_INFO, "update for %s\n", node->var->key);
> +		auth_buffer_size = get_auth_descriptor2(node->var->data, &auth_buffer);
> +		if (auth_buffer_size <= 0)
> +			return OPAL_PARAMETER;
> +
> +		if (node->var->data_size < auth_buffer_size) {
> +			rc = OPAL_PARAMETER;
> +			goto out;
> +		}
> +
> +		rc = get_timestamp_from_auth(auth_buffer, &timestamp);
> +		if (rc < 0)
> +			goto out;	
> +
> +		rc = check_timestamp(node->var->key, timestamp);
> +		if (rc)
> +			goto out;
> +
> +		/* Calculate the size of new ESL data */
> +		new_data_size = node->var->data_size - auth_buffer_size;
> +		newesl = zalloc(new_data_size);
> +		memcpy(newesl, node->var->data + auth_buffer_size, new_data_size);
> +
> +		if (!setup_mode) {
> +			/* Prepare the data to be verified */
> +			rc = get_data_to_verify(node->var->key, newesl,
> +						new_data_size, &tbhbuffer,
> +						&tbhbuffersize, timestamp);
> +		
> +			/* Get the authority to verify the signature */
> +			get_key_authority(key_authority, node->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", node->var->key);
> +				prlog(PR_DEBUG, "key authority is %s\n", key_authority[i]);
> +				anode = find_secvar(key_authority[i], strlen(key_authority[i]) + 1,
> +						    &variable_bank);
> +				if (!anode) {
> +					rc = OPAL_PERMISSION;
> +					goto out;
> +				}
> +				if (anode->var->data_size == 0) {
> +					rc = OPAL_PERMISSION;
> +					goto out;
> +				}
> +
> +				/* Verify the signature */
> +				rc = verify_signature(auth_buffer, tbhbuffer,
> +						      tbhbuffersize, anode->var);
> +
> +				/* Break if signature verification is successful */
> +				if (!rc)
> +					break;
> +				i++;
> +			}
> +		}
> +
> +		if (rc)
> +			goto out;
> +
> +		/*
> +		 * If reached here means, signature is verified so update the
> +		 * value in the variable bank
> +		 */
> +		add_to_variable_bank(node->var, newesl, new_data_size);
Check return code ?
> +		// Update the TS variable with the new timestamp
> +		update_timestamp(node->var->key, timestamp);
Check return code?
> +
> +		/* 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_updated = 1;
> +			if(new_data_size == 0)
> +				setup_mode = true;
> +			else
> +				setup_mode = false;
> +			printf("setup mode is %d\n", setup_mode);
> +		}
> +	}
> +
> +	if (pk_updated) {
> +		// Store the updated pk in TPMNV on p9
> +		if (proc_gen == proc_gen_p9) {
> +			rc = edk2_p9_write_pk();
> +			prlog(PR_INFO, "edk2_p9_write rc=%d\n", rc);
> +		}
> +	}
> +
> +out:
> +	if (auth_buffer)
> +		free(auth_buffer);
> +	if (newesl)
> +		free(newesl);
> +	if (tbhbuffer)
> +		free(tbhbuffer);
no need to check
> +
> +	clear_bank_list(&update_bank);
> +
> +	return rc;
> +}
> +
> +static int edk2_compat_post_process(void)
> +{
> +	printf("setup mode is %d\n", setup_mode);
> +	if (!setup_mode) {
> +		secvar_set_secure_mode();
> +		prlog(PR_INFO, "Enforcing OS secure mode\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static bool is_pkcs7_sig_format(void *data)
> +{
> +	struct efi_variable_authentication_2 *auth = data;
Why does this function take a void as input rather than the concrete type?
> +	uuid_t pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID;
> +
> +	if(!(memcmp(&auth->auth_info.cert_type, &pkcs7_guid, 16) == 0))

16 -> sizeof(pkcs7_guid)

You first check that memcmp() returns 0 and then you invert it . Seems 
unnecessary and you could just invert the return value below.

> +		return false;
> +
> +	return true;
> +}
> +
> +static int edk2_compat_validate(struct secvar *var)

What does this validate and what does it have to do with edk2 ?

Can it return a bool?

> +{
> +
> +	/*
> +	 * Checks if the update is for supported
> +	 * Non-volatile secure variales
variables
> +	 */
> +	if (!key_equals(var->key, "PK")
> +	    && !key_equals(var->key, "KEK")
> +	    && !key_equals(var->key, "db")
> +	    && !key_equals(var->key, "dbx"))
> +		return -1;
> +
> +	/*
> +	 * PK update should contain single ESL.
> +	 */
> +	//Not sure if we need to restrict it but, am adding as of now.
> +	//Feel free to remove it if you don't it as good idea
Uh?

> +	if (key_equals(var->key, "PK")) {
> +		printf("check if single PK\n");
> +		if (!is_single_pk(var->data, var->data_size)) {
> +			printf("not single pk\n");
> +			return -1;
> +		}
> +	}
> +
> +	/*
> +	 * Check that signature type is PKCS7
> +	 */
> +	if (!is_pkcs7_sig_format(var->data))
> +		return -1;
> +	//Some more checks needs to be added:
> +	// - check guid
> +	// - check auth struct
> +	// - possibly check signature? can't add but can validate
> +
> +	return 0;
> +};
> +
> +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..29874ef7
> --- /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 2019 IBM Corp.
> + */
> +
> +#ifndef __EDK2_H__
> +#define __EDK2_H__
> +
> +#define UUID_SIZE 16
> +
> +typedef struct {
> +        u8 b[UUID_SIZE];
This may be the first time I see u8; would think uint8_t
> +} 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;

u32 nad uint32_t in the same file... I would go for uint(8|16|32)_t in 
this file just because I saw them most often being used.


> +  ///
> +  /// 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
diff mbox series

Patch

diff --git a/doc/secvar/edk2.rst b/doc/secvar/edk2.rst
new file mode 100644
index 00000000..e0c29457
--- /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 2875c700..8b701e00 100644
--- a/include/secvar.h
+++ b/include/secvar.h
@@ -24,6 +24,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 cc1a49fa..1c1896ab 100644
--- a/libstb/secvar/backend/Makefile.inc
+++ b/libstb/secvar/backend/Makefile.inc
@@ -1,11 +1,11 @@ 
 # SPDX-License-Identifier: Apache-2.0
 # -*-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
 SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
 SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
 
diff --git a/libstb/secvar/backend/edk2-compat.c b/libstb/secvar/backend/edk2-compat.c
new file mode 100644
index 00000000..b99738b1
--- /dev/null
+++ b/libstb/secvar/backend/edk2-compat.c
@@ -0,0 +1,877 @@ 
+// SPDX-License-Identifier: Apache-2.0
+/* Copyright 2019 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 "libstb/crypto/pkcs7/pkcs7.h"
+#include "edk2.h"
+#include "opal-api.h"
+#include "../secvar.h"
+#include "../secvar_devtree.h"
+#include "../secvar_tpmnv.h"
+#include <mbedtls/error.h>
+
+#define TPMNV_ID_EDK2_PK	0x4532504b // E2PK
+
+static bool setup_mode;
+
+//struct efi_time *timestamp_list;
+
+/*
+ * Converts utf8 string to ucs2
+ */
+static char *utf8_to_ucs2(const char *key, const char keylen)
+{
+	int i;
+	char *str;
+	str = zalloc(keylen * 2);
+
+	for (i = 0; i < keylen*2; key++) {
+		str[i++] = *key;
+		str[i++] = '\0';
+	}
+	return str;
+}
+
+/*
+ * Returns true if key1 = key2
+ */
+static bool key_equals(const char *key1, const char *key2)
+{
+	if (memcmp(key1, key2, strlen(key2)+1) == 0)
+		return true;
+
+	return false;
+}
+
+/**
+ * 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;
+
+	memset(ret, 0, sizeof(char *) * 3);
+	if (key_equals(key, "PK"))
+		ret[i++] = "PK";
+	if (key_equals(key, "KEK"))
+		ret[i++] = "PK";
+	if (key_equals(key, "db") || key_equals(key, "dbx")) {
+		ret[i++] = "KEK";
+		ret[i++] = "PK";
+	}
+	ret[i] = NULL;
+}
+
+/*
+ * PK needs to be stored in the TPMNV space if on p9
+ * We store it using the form <u64:esl size><esl data>, the
+ * extra secvar headers are unnecessary
+ */
+static int edk2_p9_load_pk(void)
+{
+	struct secvar_node *pkvar;
+	uint64_t size;
+	int rc;
+
+	// Ensure it exists
+	rc = secvar_tpmnv_alloc(TPMNV_ID_EDK2_PK, -1);
+
+	// Peek to get the size
+	rc = secvar_tpmnv_read(TPMNV_ID_EDK2_PK, &size, sizeof(size), 0);
+	if (rc == OPAL_EMPTY)
+		return 0;
+	else if (rc)
+		return -1;
+
+	if (size > secvar_storage.max_var_size)
+		return OPAL_RESOURCE;
+
+	pkvar = alloc_secvar(size);
+	memcpy(pkvar->var->key, "PK", 3);
+	pkvar->var->key_len = 3;
+	pkvar->var->data_size = size;
+	pkvar->flags |= SECVAR_FLAG_VOLATILE;
+
+	rc = secvar_tpmnv_read(TPMNV_ID_EDK2_PK, pkvar->var->data, pkvar->var->data_size, sizeof(pkvar->var->data_size));
+	if (rc)
+		return rc;
+
+	list_add_tail(&variable_bank, &pkvar->link);
+
+	return OPAL_SUCCESS;
+}
+
+/*
+ * Writes the PK to the TPM.
+ */
+static int edk2_p9_write_pk(void)
+{
+	char *tmp;
+	int32_t tmpsize;
+	struct secvar_node *pkvar;
+	int rc;
+
+	pkvar = find_secvar("PK", 3, &variable_bank);
+
+	// Should not happen
+	if (!pkvar)
+		return OPAL_INTERNAL_ERROR;
+
+	// Reset the pk flag to volatile on p9
+	pkvar->flags |= SECVAR_FLAG_VOLATILE;
+
+	tmpsize = secvar_tpmnv_size(TPMNV_ID_EDK2_PK);
+	if (tmpsize < 0) {
+		prlog(PR_ERR, "TPMNV space for PK was not allocated properly\n");
+		return OPAL_RESOURCE;
+	}
+	if (tmpsize < pkvar->var->data_size + sizeof(pkvar->var->data_size)) {
+		prlog(PR_ERR, "TPMNV PK space is insufficient, %d < %llu\n", tmpsize,
+			// Cast needed because x86 compiler complains building the test
+			(long long unsigned) pkvar->var->data_size + sizeof(pkvar->var->data_size));
+		return OPAL_RESOURCE;
+	}
+
+	tmp = zalloc(tmpsize);
+	if (!tmp)
+		return OPAL_NO_MEM;
+
+	memcpy(tmp, &pkvar->var->data_size, sizeof(pkvar->var->data_size));
+	memcpy(tmp + sizeof(pkvar->var->data_size),
+		pkvar->var->data,
+		pkvar->var->data_size);
+
+	tmpsize = pkvar->var->data_size + sizeof(pkvar->var->data_size);
+
+	rc = secvar_tpmnv_write(TPMNV_ID_EDK2_PK, tmp, tmpsize, 0);
+
+	free(tmp);
+
+	return rc;
+}
+
+/*
+ * Returns the size of the ESL.
+ */
+static int get_esl_signature_list_size(char *buf)
+{
+	EFI_SIGNATURE_LIST list;
+
+	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);
+}
+
+/*
+ * Returns the size of the certificate contained in the ESL.
+ */
+static int get_esl_cert_size(char *buf)
+{
+	EFI_SIGNATURE_LIST list;
+	uint32_t sigsize;
+
+	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
+
+	sigsize = le32_to_cpu(list.SignatureListSize) - sizeof(list)
+		- le32_to_cpu(list.SignatureHeaderSize) - sizeof(uuid_t); 
+
+	prlog(PR_DEBUG, "sig size is %u\n", sigsize);
+	return sigsize;
+}
+
+/*
+ * Copies the certificate from the ESL into cert buffer.
+ */
+static int get_esl_cert(char *buf, char **cert)
+{
+	int sig_data_offset;
+	int size;
+	EFI_SIGNATURE_LIST list;
+
+	memset(&list, 0, sizeof(EFI_SIGNATURE_LIST));
+	memcpy(&list, buf, sizeof(EFI_SIGNATURE_LIST));
+
+	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.SignatureType)
+		+ sizeof(list.SignatureListSize)
+		+ sizeof(list.SignatureHeaderSize)
+		+ sizeof(list.SignatureSize)
+		+ le32_to_cpu(list.SignatureHeaderSize)
+		+ 16 * sizeof(uint8_t);
+
+	size = le32_to_cpu(list.SignatureSize) - sizeof(uuid_t);
+	
+	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 = le32_to_cpu(auth->auth_info.hdr.dw_length);
+	int size;
+
+	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;
+}
+
+/*
+ * Return the timestamp from the Authentication 2 Descriptor.
+ */
+static int get_timestamp_from_auth(char *data, struct efi_time **timestamp)
+{
+	*timestamp = (struct efi_time *) data;
+
+	return 0;
+}
+
+/*
+ * This function outputs the Authentication 2 Descriptor in the
+ * auth_buffer and returns the size of the buffer.
+ */
+static int get_auth_descriptor2(void *data, char **auth_buffer)
+{
+	struct efi_variable_authentication_2 *auth = data;
+	uint64_t auth_buffer_size;
+	int len;
+
+	if (!auth_buffer)
+		return OPAL_PARAMETER;
+
+	len = get_pkcs7_len(auth);
+	if (len < 0)
+		return OPAL_NO_MEM;
+
+	auth_buffer_size = sizeof(auth->timestamp) + sizeof(auth->auth_info.hdr)
+			   + sizeof(auth->auth_info.cert_type) + len;
+
+	*auth_buffer = zalloc(auth_buffer_size);
+	if (!(*auth_buffer))
+		return OPAL_NO_MEM;
+
+	memcpy(*auth_buffer, data, auth_buffer_size);
+
+	return auth_buffer_size;
+}
+
+/* Check that PK has single ESL */
+static bool is_single_pk(char *data, uint64_t data_size)
+{
+	char *auth_buffer = NULL;
+	uint64_t auth_buffer_size = 0;
+	char *newesl = NULL;
+	uint64_t new_data_size = 0;
+	int esllistsize;
+
+	auth_buffer_size = get_auth_descriptor2(data, &auth_buffer);
+	printf("auth buffer size is %d\n", (int)auth_buffer_size);
+	free(auth_buffer);
+	if (auth_buffer_size <= 0)
+		return false;
+
+	/* Calculate the size of new ESL data */
+	new_data_size = data_size - auth_buffer_size;
+	printf("new data size is %d\n", (int)new_data_size);
+
+	if (!new_data_size)
+		return true;
+
+	newesl = zalloc(new_data_size);
+	memcpy(newesl, data + auth_buffer_size, new_data_size);
+
+	esllistsize = get_esl_signature_list_size(newesl);
+	printf("esl list size is %d\n", esllistsize);
+	free(newesl);
+	if (new_data_size > esllistsize)
+		return false;
+
+	return true;
+}
+
+/*
+ * Initializes supported variables as empty if not loaded from
+ * storage. Variables are initialized as volatile if not found.
+ * Updates should clear this flag.
+ec*
+ * Returns OPAL Error if anything fails in initialization
+ */
+static int edk2_compat_pre_process(void)
+{
+	struct secvar_node *pkvar;
+	struct secvar_node *kekvar;
+	struct secvar_node *dbvar;
+	struct secvar_node *dbxvar;
+	struct secvar_node *tsvar;
+
+	// If we are on p9, we need to store the PK in write-lockable
+	//  TPMNV space, as we determine our secure mode based on if this
+	//  variable exists.
+	// NOTE: Activation of this behavior is subject to change in a later
+	//  patch version, ideally the platform should be able to configure
+	//  whether it wants this extra protection, or to instead store
+	//  everything via the storage driver.
+	if (proc_gen == proc_gen_p9)
+		edk2_p9_load_pk();
+
+	pkvar = find_secvar("PK", 3, &variable_bank);
+	if (!pkvar) {
+		pkvar = alloc_secvar(0);
+		if (!pkvar)
+			return OPAL_NO_MEM;
+
+		memcpy(pkvar->var->key, "PK", 3);
+		pkvar->var->key_len = 3;
+		pkvar->flags |= SECVAR_FLAG_VOLATILE;
+		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 = alloc_secvar(0);
+		if (!kekvar)
+			return OPAL_NO_MEM;
+
+		memcpy(kekvar->var->key, "KEK", 4);
+		kekvar->var->key_len = 4;
+		kekvar->flags |= SECVAR_FLAG_VOLATILE;
+		list_add_tail(&variable_bank, &kekvar->link);
+	}
+
+	dbvar = find_secvar("db", 3, &variable_bank);
+	if (!dbvar) {
+		dbvar = alloc_secvar(0);
+		if (!dbvar)
+			return OPAL_NO_MEM;
+
+		memcpy(dbvar->var->key, "db", 3);
+		dbvar->var->key_len = 3;
+		dbvar->flags |= SECVAR_FLAG_VOLATILE;
+		list_add_tail(&variable_bank, &dbvar->link);
+	}
+
+	dbxvar = find_secvar("dbx", 4, &variable_bank);
+	if (!dbxvar) {
+		dbxvar = alloc_secvar(0);
+		if (!dbxvar)
+			return OPAL_NO_MEM;
+
+		memcpy(dbxvar->var->key, "dbx", 4);
+		dbxvar->var->key_len = 4;
+		dbxvar->flags |= SECVAR_FLAG_VOLATILE;
+		list_add_tail(&variable_bank, &dbxvar->link);
+	}
+
+	tsvar = find_secvar("TS", 3, &variable_bank);
+	// Should only ever happen on first boot
+	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);
+		//tsvar->flags |= SECVAR_FLAG_VOLATILE;
+		list_add_tail(&variable_bank, &tsvar->link);
+	}
+
+	return OPAL_SUCCESS;
+};
+
+/**
+ * Returns true if we are in Setup Mode
+ *
+ * Setup Mode is active if we have no PK.
+ * Otherwise, we are in user mode.
+ */
+/**
+static int is_setup_mode(void)
+{
+	struct secvar_node *setup;
+
+	setup = find_secvar((char *)"PK", 3, &variable_bank);
+
+	// Not sure why this wouldn't exist
+	if (!setup)
+		return 1;
+
+	return !setup->var->data_size;
+}
+**/
+
+/**
+ * Update the variable with the new value.
+ */
+static int add_to_variable_bank(struct secvar *secvar, void *data, uint64_t dsize)
+{
+	struct secvar_node *node;
+
+	node = find_secvar(secvar->key, secvar->key_len, &variable_bank);
+	if (!node)
+		return OPAL_INTERNAL_ERROR;
+
+	// Expand the secvar allocated memory if needed
+	if (node->size < dsize)
+		if (realloc_secvar(node, dsize))
+			return OPAL_NO_MEM;
+
+	node->var->data_size = dsize;
+	memcpy(node->var->data, data, dsize);
+	node->flags &= ~SECVAR_FLAG_VOLATILE; // Clear the volatile bit when updated
+
+	return 0;
+}
+
+static struct efi_time *get_last_timestamp(char *key)
+{
+	struct secvar_node *node;
+	struct efi_time *prev;
+	char *timestamp_list;
+	u8 off;
+
+	node = find_secvar("TS", 3, &variable_bank);
+	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;	// unexpected variable name?
+
+	timestamp_list = node->var->data;
+	if (!timestamp_list)
+		return NULL;
+
+	prev = (struct efi_time *) (timestamp_list + (off * sizeof(struct efi_time)));
+
+	return prev;
+}
+
+// Update the TS variable with the new timestamp
+static int update_timestamp(char *key, struct efi_time *timestamp)
+{
+	struct efi_time *prev;
+
+	prev = get_last_timestamp(key);
+	if (prev == NULL)
+		return OPAL_PARAMETER;
+
+	memcpy(prev, timestamp, sizeof(struct efi_time));
+
+	printf("updated prev year is %d month %d day %d\n", le16_to_cpu(prev->year), prev->month, prev->day);
+//	add_to_variable_bank(node->var, timestamp_list, node->var->data_size); 
+
+	return OPAL_SUCCESS;
+}
+
+static int check_timestamp(char *key, struct efi_time *timestamp)
+{
+	struct efi_time *prev;
+	char *current = NULL;
+	char *last =NULL;
+	int s1 = 0;
+
+	prev = get_last_timestamp(key);
+	if (prev == NULL)
+		return OPAL_PARAMETER;
+
+	printf("timestamp year is %d month %d day %d\n", le16_to_cpu(timestamp->year), timestamp->month, timestamp->day);
+	printf("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;
+
+	current = &(timestamp->month);
+	last = &(prev->month);
+
+	s1 = memcmp(current, last, 5);
+	if (s1 <= 0) {
+		printf("s1 is %d\n", s1);
+		return OPAL_PERMISSION;
+	}
+
+	return OPAL_SUCCESS;
+}
+
+/*
+ * Verify the PKCS7 signature on the signed data.
+ */
+static int verify_signature(void *auth_buffer, char *newcert,
+		uint64_t new_data_size, struct secvar *avar)
+{
+	struct efi_variable_authentication_2 *auth;
+	mbedtls_pkcs7 *pkcs7;
+	mbedtls_x509_crt x509;
+	char *checkpkcs7cert;
+	char *signing_cert = NULL;
+	char *x509_buf;
+	int len;
+	int signing_cert_size;
+	int rc;
+	char *errbuf;
+	int eslvarsize;
+	int offset = 0;
+
+	auth = auth_buffer;
+	len  = get_pkcs7_len(auth);
+	pkcs7 = malloc(sizeof(struct mbedtls_pkcs7));
+	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);
+		goto pkcs7out;
+	}
+
+	checkpkcs7cert = zalloc(2048);
+	mbedtls_x509_crt_info(checkpkcs7cert, 2048, "CRT:", &(pkcs7->signed_data.certs));	
+	prlog(PR_DEBUG, "%s \n", checkpkcs7cert);
+	free(checkpkcs7cert);
+
+	prlog(PR_INFO, "Load the signing certificate from the keystore");
+
+	eslvarsize = avar->data_size;
+
+	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;
+
+		signing_cert_size = get_esl_cert_size(avar->data + offset);
+		if (!signing_cert_size) {
+			rc = OPAL_PERMISSION;
+			break;	
+		}
+
+		signing_cert = zalloc(signing_cert_size);
+		get_esl_cert(avar->data + offset, &signing_cert);
+
+		mbedtls_x509_crt_init(&x509);
+		rc = mbedtls_x509_crt_parse(&x509, signing_cert, signing_cert_size);
+
+		/* If failure in parsing the certificate, try next */
+		if(rc) {
+			prlog(PR_INFO, "X509 certificate parsing failed %04x\n", rc);
+			goto next;
+		}
+
+		x509_buf = zalloc(2048);
+		mbedtls_x509_crt_info(x509_buf, 2048, "CRT:", &x509);
+		prlog(PR_INFO, "%s \n", x509_buf);
+		free(x509_buf);
+		rc = mbedtls_pkcs7_signed_data_verify(pkcs7, &x509, newcert, new_data_size);
+
+		/* If find a signing certificate, you are done */
+		if (rc == 0) {
+			if (signing_cert)
+				free(signing_cert);
+			mbedtls_x509_crt_free(&x509);
+			prlog(PR_INFO, "Signature Verification passed\n");
+			break;
+		}
+
+		errbuf = zalloc(1024);
+		mbedtls_strerror(rc, errbuf, 1024);
+		prlog(PR_INFO, "Signature Verification failed %02x %s\n", rc, errbuf);
+		free(errbuf);
+
+next:
+		offset += get_esl_signature_list_size(avar->data + offset);
+		eslvarsize = eslvarsize - offset;
+		mbedtls_x509_crt_free(&x509);
+		if (signing_cert)
+			free(signing_cert);
+
+	}
+
+pkcs7out:
+	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.
+ */
+static int get_data_to_verify(char *key, char *new_data,
+		uint64_t new_data_size,
+		char **buffer,
+		uint64_t *buffer_size, struct efi_time *timestamp)
+{
+	le32 attr = cpu_to_le32(SECVAR_ATTRIBUTES);
+	int size = 0;
+	int varlen;
+	char *wkey;
+	uuid_t guid;
+
+	if (key_equals(key, "PK")
+	    || key_equals(key, "KEK"))
+		guid = EFI_GLOBAL_VARIABLE_GUID;
+
+	if (key_equals(key, "db")
+	    || key_equals(key, "dbx"))
+		guid = EFI_IMAGE_SECURITY_DATABASE_GUID;
+		
+	// 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);
+
+	memcpy(*buffer + size, wkey, varlen);
+	size = size + varlen;
+	memcpy(*buffer + size, &guid, sizeof(guid));
+	size = size + sizeof(guid);
+	memcpy(*buffer + size, &attr, sizeof(attr));
+	size = size + sizeof(attr);
+	memcpy(*buffer + size, timestamp , sizeof(struct efi_time));
+	size = size + sizeof(struct efi_time);
+
+	memcpy(*buffer + size, new_data, new_data_size);
+	size = size + new_data_size;
+
+	free(wkey);
+
+	return 0;
+}
+
+static int edk2_compat_process(void)
+{
+	char *auth_buffer = NULL;
+	uint64_t auth_buffer_size = 0;
+	struct efi_time *timestamp = NULL;
+	const char *key_authority[3];
+	char *newesl = NULL;
+	uint64_t new_data_size = 0;
+	char *tbhbuffer = NULL;
+	uint64_t tbhbuffersize = 0;
+	struct secvar_node *anode = NULL;
+	struct secvar_node *node = NULL;
+	int rc = 0;
+	int pk_updated = 0;
+	int i;
+
+	//setup_mode = is_setup_mode();
+	prlog(PR_INFO, "Setup mode = %d\n", setup_mode);
+
+	/* 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
+		 */
+ 		printf("setup mode is %d\n", setup_mode);		
+		prlog(PR_INFO, "update for %s\n", node->var->key);
+		auth_buffer_size = get_auth_descriptor2(node->var->data, &auth_buffer);
+		if (auth_buffer_size <= 0)
+			return OPAL_PARAMETER;
+
+		if (node->var->data_size < auth_buffer_size) {
+			rc = OPAL_PARAMETER;
+			goto out;
+		}
+
+		rc = get_timestamp_from_auth(auth_buffer, &timestamp);
+		if (rc < 0)
+			goto out;	
+
+		rc = check_timestamp(node->var->key, timestamp);
+		if (rc)
+			goto out;
+
+		/* Calculate the size of new ESL data */
+		new_data_size = node->var->data_size - auth_buffer_size;
+		newesl = zalloc(new_data_size);
+		memcpy(newesl, node->var->data + auth_buffer_size, new_data_size);
+
+		if (!setup_mode) {
+			/* Prepare the data to be verified */
+			rc = get_data_to_verify(node->var->key, newesl,
+						new_data_size, &tbhbuffer,
+						&tbhbuffersize, timestamp);
+		
+			/* Get the authority to verify the signature */
+			get_key_authority(key_authority, node->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", node->var->key);
+				prlog(PR_DEBUG, "key authority is %s\n", key_authority[i]);
+				anode = find_secvar(key_authority[i], strlen(key_authority[i]) + 1,
+						    &variable_bank);
+				if (!anode) {
+					rc = OPAL_PERMISSION;
+					goto out;
+				}
+				if (anode->var->data_size == 0) {
+					rc = OPAL_PERMISSION;
+					goto out;
+				}
+
+				/* Verify the signature */
+				rc = verify_signature(auth_buffer, tbhbuffer,
+						      tbhbuffersize, anode->var);
+
+				/* Break if signature verification is successful */
+				if (!rc)
+					break;
+				i++;
+			}
+		}
+
+		if (rc)
+			goto out;
+
+		/*
+		 * If reached here means, signature is verified so update the
+		 * value in the variable bank
+		 */
+		add_to_variable_bank(node->var, newesl, new_data_size);
+		// Update the TS variable with the new timestamp
+		update_timestamp(node->var->key, timestamp);
+
+		/* 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_updated = 1;
+			if(new_data_size == 0)
+				setup_mode = true;
+			else
+				setup_mode = false;
+			printf("setup mode is %d\n", setup_mode);
+		}
+	}
+
+	if (pk_updated) {
+		// Store the updated pk in TPMNV on p9
+		if (proc_gen == proc_gen_p9) {
+			rc = edk2_p9_write_pk();
+			prlog(PR_INFO, "edk2_p9_write rc=%d\n", rc);
+		}
+	}
+
+out:
+	if (auth_buffer)
+		free(auth_buffer);
+	if (newesl)
+		free(newesl);
+	if (tbhbuffer)
+		free(tbhbuffer);
+
+	clear_bank_list(&update_bank);
+
+	return rc;
+}
+
+static int edk2_compat_post_process(void)
+{
+	printf("setup mode is %d\n", setup_mode);
+	if (!setup_mode) {
+		secvar_set_secure_mode();
+		prlog(PR_INFO, "Enforcing OS secure mode\n");
+	}
+
+	return 0;
+}
+
+static 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;
+}
+
+static int edk2_compat_validate(struct secvar *var)
+{
+
+	/*
+	 * Checks if the update is for supported
+	 * Non-volatile secure variales
+	 */
+	if (!key_equals(var->key, "PK")
+	    && !key_equals(var->key, "KEK")
+	    && !key_equals(var->key, "db")
+	    && !key_equals(var->key, "dbx"))
+		return -1;
+
+	/*
+	 * PK update should contain single ESL.
+	 */
+	//Not sure if we need to restrict it but, am adding as of now.
+	//Feel free to remove it if you don't it as good idea
+	if (key_equals(var->key, "PK")) {
+		printf("check if single PK\n");
+		if (!is_single_pk(var->data, var->data_size)) {
+			printf("not single pk\n");
+			return -1;
+		}
+	}
+
+	/*
+	 * Check that signature type is PKCS7
+	 */
+	if (!is_pkcs7_sig_format(var->data))
+		return -1;
+	//Some more checks needs to be added:
+	// - check guid
+	// - check auth struct
+	// - possibly check signature? can't add but can validate
+
+	return 0;
+};
+
+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..29874ef7
--- /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 2019 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