diff mbox series

[v5,13/20] secvar/storage: add secvar storage driver for pnor-based p9

Message ID 20200612202514.15032-14-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 success Successfully applied on branch master (fe70fbb78d33abea788a3221bc409a7c50c019c3)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot fail Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Eric Richter June 12, 2020, 8:25 p.m. UTC
This patch implements the platform specific logic for persisting the
secure variable storage banks across reboots via the SECBOOT PNOR
partition.

For POWER 9, all secure variables and updates are stored in the
in the SECBOOT PNOR partition. The partition is split into three
sections: two variable bank sections, and a section for storing
updates. The driver alternates writes between the two variable
sections, so that the final switch from one set of variables to
the next can be as atomic as possible by flipping an "active bit"
stored in TPM NV.

PNOR space provides no lock protection, so prior to writing the
variable bank, a sha256 hash is calculated and stored in TPM NV.
This hash is compared against the hash of the variables loaded from
PNOR to ensure consistency -- otherwise a failure is reported, no keys
are loaded (which should cause skiroot to refuse to boot if secure boot
support is enabled).

Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
V5:
 - overhauled serialization/deserialization:
   - no longer memcpy's full structs
   - always uses the tighter packing scheme
   - cleaned up/merged helper functions
   - writes all numeric values as BE
 - change some programming errors to asserts
 - moved the public name hashes back into the .c file

 doc/secvar/secboot_tpm.rst          | 175 ++++++++
 include/secvar.h                    |   1 +
 libstb/secvar/secvar.h              |   4 +-
 libstb/secvar/storage/Makefile.inc  |   5 +-
 libstb/secvar/storage/secboot_tpm.c | 657 ++++++++++++++++++++++++++++
 libstb/secvar/storage/secboot_tpm.h |  61 +++
 libstb/secvar/storage/tpmnv_ops.c   |  15 +
 7 files changed, 914 insertions(+), 4 deletions(-)
 create mode 100644 doc/secvar/secboot_tpm.rst
 create mode 100644 libstb/secvar/storage/secboot_tpm.c
 create mode 100644 libstb/secvar/storage/secboot_tpm.h
 create mode 100644 libstb/secvar/storage/tpmnv_ops.c
diff mbox series

Patch

diff --git a/doc/secvar/secboot_tpm.rst b/doc/secvar/secboot_tpm.rst
new file mode 100644
index 00000000..8da0c2f0
--- /dev/null
+++ b/doc/secvar/secboot_tpm.rst
@@ -0,0 +1,175 @@ 
+.. _secvar/secboot_tpm:
+
+secboot_tpm secvar storage driver for P9 platforms
+==================================================
+
+Overview
+--------
+
+This storage driver utilizes the SECBOOT PNOR partition and TPM NV space to
+persist secure variables across reboots in a tamper-resistant manner. While
+writes to PNOR cannot be completely prevented, writes CAN be prevented to TPM
+NV. On the other hand, there is limited available space in TPM NV.
+
+Therefore, this driver uses both in conjunction: large variable data is written
+to SECBOOT, and a hash of the variable data is stored in TPM NV. When the
+variables are loaded from SECBOOT, this hash is recalculated and compared
+against the value stored in the TPM. If they do not match, then the variables
+must have been altered and are not loaded.
+
+See the following sections for more information on the internals of the driver.
+
+
+Storage Layouts
+---------------
+
+At a high-level, there are a few major logical components:
+
+ - (PNOR) Variable storage (split in half, active/staging)
+ - (PNOR) Update storage
+ - (TPM)  Protected variable storage
+ - (TPM)  Bank hashes & active bit
+
+Variable storage consists of two smaller banks, variable bank 0 and variable
+bank 1. Either of the banks may be designated "active" by setting the active
+bank bit to either 0 or 1, indicating that the corresponding bank is now
+"active". The other bank is then considered "staging". See the "Persisting
+Variable Bank Updates" for more on the active/staging bank logic.
+
+Protected variable storage is stored in ``VARS`` TPM NV index. Unlike the other
+variable storage, there is only one bank due to limited storage space. See the
+TPM NV Indices section for more.
+
+
+Persisting the Variable Bank
+----------------------------
+
+When writing a new variable bank to storage, this is (roughly) the procedure the
+driver will follow:
+
+1. write variables to the staging bank
+2. calculate hash of the staging bank
+3. store the staging bank hash in the TPM NV
+4. flip the active bank bit
+
+This procedure ensures that the switch-over from the old variables to the
+new variables is as atomic as possible. This should prevent any possible
+issues caused by an interruption during the writing process, such as power loss.
+
+The bank hashes are a SHA256 hash calculated over the whole region of
+storage space allocated to the bank, including unused storage. For consistency,
+unused space is always written as zeroes. Like the active/staging variable
+banks, there are also two corresponding active/staging bank hashes stored in
+the TPM.
+
+
+TPM NV Indices
+--------------
+
+The driver utilizes two TPM NV indices:
+
+.. code-block:: c
+
+  # size). datadefine SECBOOT_TPMNV_VARS_INDEX	0x01c10190
+  #define SECBOOT_TPMNV_CONTROL_INDEX	0x01c10191
+
+The ``VARS`` index stores variables flagged with ``SECVAR_FLAG_PROTECTED``.
+These variables are critical to the state of OS secure boot, and therefore
+cannot be safely stored in the SECBOOT partition. This index is defined to be
+1024 bytes in size, which is enough for the current implementation on P9. It
+is kept small by default to preserve the very limited NV index space.
+
+The ``CONTROL`` index stores the bank hashes, and the bit to determine which
+bank is active. See the Active/Staging Bank Swapping section for more.
+
+Both indices are defined on first boot with the same set of attributes. If the
+indices are already defined but not in the expected state, (different
+attributes, size, etc), then the driver will halt the boot. Asserting physical
+presence will redefine the indices in the correct state.
+
+
+Locking
+-------
+
+PNOR cannot be locked, however the TPM can be. The TPM NV indices are double
+protected via two locking mechanisms:
+
+ - The driver's ``.lock()`` hook sends the ``TSS_NV_WriteLock`` TPM command.
+This sets the ``WRITELOCKED`` attribute, which is cleared on the next
+TPM reset.
+
+ - The TPM NV indices are defined under the platform hierarchy. Skiboot will add
+a global lock to all the NV indices under this hierarchy prior to loading a
+kernel. This is also reset on the next TPM reset.
+
+NOTE: The TPM is only reset during a cold reboot. Fast reboots or kexecs will
+NOT unlock the TPM.
+
+
+Resetting Storage / Physical Presence
+-------------------------------------
+
+In the case that secure boot/secvar has been rendered unusable, (for example:
+corrupted data, lost/compromised private key, improperly defined NV indices, etc)
+this storage driver responds to physical presence assertion as a last-resort
+method to recover the system.
+
+Asserting physical presence undefines, and immediately redefines the TPM NV
+indices. Defining the NV indices then causes a cascading set of reformats for
+the remaining components of storage, similar to a first-boot scenario.
+
+This driver considers physical presence to be asserted if any of the following
+device tree nodes are present in ``ibm,secureboot``:
+ - ``clear-os-keys``
+ - ``clear-all-keys``
+ - ``clear-mfg-keys``
+
+
+Storage Formats/Layouts
+=======================
+
+SECBOOT (PNOR)
+--------------
+
+Partition Format:
+ - 8b secboot header
+   - 4b: u32. magic number, always 0x5053424b
+   - 1b: u8. version, always 1
+   - 3b: unused padding
+ - 32k: secvars. variable bank 0
+ - 32k: secvars. variable bank 1
+ - 32k: secvars. update bank
+
+Variable Format (secvar):
+ - 8b: u64. key length
+ - 8b: u64. data size
+ - 1k: string. key
+ - (data size). data
+
+TPM VARS (NV)
+-------------
+
+NV Index Format:
+ - 8b secboot header
+   - 4b: u32. magic number, always 0x5053424b
+   - 1b: u8. version, always 1
+   - 3b: unused padding
+ - 1016b: packed secvars. protected variable storage
+
+Variable Format (packed secvar):
+ - 8b: u64. key length
+ - 8b: u64. data size
+ - (key length): string. key
+ - (data size). data
+
+TPM CONTROL (NV)
+----------------
+
+ - 8b secboot header
+   - 4b: u32. magic number, always 0x5053424b
+   - 1b: u8. version, always 1
+   - 3b: unused padding
+ - 1b: u8. active bit, 0 or 1
+ - 32b: sha256 hash of variable bank 0
+ - 32b: sha256 hash of variable bank 1
+
diff --git a/include/secvar.h b/include/secvar.h
index db103953..21210277 100644
--- a/include/secvar.h
+++ b/include/secvar.h
@@ -36,6 +36,7 @@  struct secvar_backend_driver {
 	const char *compatible;
 };
 
+extern struct secvar_storage_driver secboot_tpm_driver;
 
 int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
 
diff --git a/libstb/secvar/secvar.h b/libstb/secvar/secvar.h
index fe660217..33cd0a08 100644
--- a/libstb/secvar/secvar.h
+++ b/libstb/secvar/secvar.h
@@ -16,8 +16,8 @@  enum {
 };
 
 
-#define SECVAR_FLAG_VOLATILE		0x1 // Instructs storage driver to ignore variable on writes
-#define SECVAR_FLAG_SECURE_STORAGE	0x2 // Hint for storage driver to select storage location
+#define SECVAR_FLAG_VOLATILE	0x1 /* Instructs storage driver to ignore variable on writes */
+#define SECVAR_FLAG_PROTECTED	0x2 /* Instructs storage driver to store in lockable flash */
 
 struct secvar {
 	struct list_node link;
diff --git a/libstb/secvar/storage/Makefile.inc b/libstb/secvar/storage/Makefile.inc
index 3fd9543a..35fba723 100644
--- a/libstb/secvar/storage/Makefile.inc
+++ b/libstb/secvar/storage/Makefile.inc
@@ -1,11 +1,12 @@ 
 # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
 # -*-Makefile-*-
 
-SECVAR_STORAGE_DIR = libstb/secvar/storage
+SECVAR_STORAGE_DIR = $(SRC)/libstb/secvar/storage
 
 SUBDIRS += $(SECVAR_STORAGE_DIR)
 
-SECVAR_STORAGE_SRCS =
+SECVAR_STORAGE_SRCS = secboot_tpm.c tpmnv_ops.c
+#SECVAR_STORAGE_SRCS = secboot_tpm.c fakenv_ops.c
 SECVAR_STORAGE_OBJS = $(SECVAR_STORAGE_SRCS:%.c=%.o)
 SECVAR_STORAGE = $(SECVAR_STORAGE_DIR)/built-in.a
 
diff --git a/libstb/secvar/storage/secboot_tpm.c b/libstb/secvar/storage/secboot_tpm.c
new file mode 100644
index 00000000..57e35e87
--- /dev/null
+++ b/libstb/secvar/storage/secboot_tpm.c
@@ -0,0 +1,657 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#ifndef pr_fmt
+#define pr_fmt(fmt) "SECBOOT_TPM: " fmt
+#endif
+
+#include <stdlib.h>
+#include <skiboot.h>
+#include <opal.h>
+#include <mbedtls/sha256.h>
+#include "../secvar.h"
+#include "../secvar_devtree.h"
+#include "secboot_tpm.h"
+#include <tssskiboot.h>
+#include <ibmtss/TPM_Types.h>
+
+#define CYCLE_BIT(b) (b^0x1)
+
+#define SECBOOT_TPM_MAX_VAR_SIZE	8192
+
+struct secboot *secboot_image = NULL;
+struct tpmnv_vars *tpmnv_vars_image = NULL;
+struct tpmnv_control *tpmnv_control_image = NULL;
+
+const size_t tpmnv_vars_size = 1024;
+
+/* Expected TPM NV index name field from NV_ReadPublic given our known
+ * set of attributes.
+ * See Part 1 Section 16, and Part 2 Section 13.5 of the TPM Specification
+ * for how this is calculated
+ */
+const uint8_t tpmnv_vars_name[] = {
+	0x00, 0x0b, 0x94, 0x64, 0x36, 0x25, 0xfc, 0xc1, 0x1d, 0xc1, 0x0e, 0x28, 0xe7,
+	0xac, 0xaf, 0xc6, 0x08, 0x8e, 0xda, 0x21, 0xd6, 0x43, 0xd2, 0x77, 0xe7, 0x2d,
+	0x83, 0x39, 0x0f, 0xa6, 0xdf, 0xc0, 0x59, 0x37,
+};
+
+const uint8_t tpmnv_control_name[] = {
+	0x00, 0x0b, 0xad, 0x47, 0x6b, 0xa5, 0xdf, 0xb1, 0xe2, 0x18, 0x50, 0xf6, 0x05,
+	0x67, 0xe8, 0x8b, 0xa9, 0x0f, 0x86, 0x1f, 0x06, 0xab, 0x43, 0x96, 0x7f, 0x6e,
+	0x85, 0x33, 0x5b, 0xa6, 0xf0, 0x63, 0x73, 0xd0,
+};
+
+
+/* Calculate a SHA256 hash over the supplied buffer */
+static int calc_bank_hash(char *target_hash, const char *source_buf, uint64_t size)
+{
+	mbedtls_sha256_context ctx;
+	int rc;
+
+	mbedtls_sha256_init(&ctx);
+
+	rc = mbedtls_sha256_update_ret(&ctx, source_buf, size);
+	if (rc)
+		goto out;
+
+	mbedtls_sha256_finish_ret(&ctx, target_hash);
+	if (rc)
+		goto out;
+
+out:
+	mbedtls_sha256_free(&ctx);
+	return rc;
+}
+
+/* Reformat the TPMNV space */
+static int tpmnv_format(void)
+{
+	int rc;
+
+	memset(tpmnv_vars_image, 0x00, tpmnv_vars_size);
+	memset(tpmnv_control_image, 0x00, sizeof(struct tpmnv_control));
+
+	tpmnv_vars_image->header.magic_number = SECBOOT_MAGIC_NUMBER;
+	tpmnv_vars_image->header.version = SECBOOT_VERSION;
+	tpmnv_control_image->header.magic_number = SECBOOT_MAGIC_NUMBER;
+	tpmnv_control_image->header.version = SECBOOT_VERSION;
+
+	/* Counts as first write to the TPM NV, which sets the
+	 *  TPMA_NVA_WRITTEN attribute */
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_VARS_INDEX,
+			     tpmnv_vars_image,
+			     tpmnv_vars_size, 0);
+	if (rc) {
+		prlog(PR_ERR, "Could not write new formatted data to VARS index, rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX,
+			     tpmnv_control_image,
+			     sizeof(struct tpmnv_control), 0);
+	if (rc)
+		prlog(PR_ERR, "Could not write new formatted data to CONTROL index, rc=%d\n", rc);
+
+	return rc;
+}
+
+/* Reformat the secboot PNOR space */
+static int secboot_format(void)
+{
+	int rc;
+
+	memset(secboot_image, 0x00, sizeof(struct secboot));
+
+	secboot_image->header.magic_number = SECBOOT_MAGIC_NUMBER;
+	secboot_image->header.version = SECBOOT_VERSION;
+
+	/* Write the hash of the empty bank to the tpm so future loads work */
+	rc = calc_bank_hash(tpmnv_control_image->bank_hash[0],
+			    secboot_image->bank[0],
+			    SECBOOT_VARIABLE_BANK_SIZE);
+	if (rc) {
+		prlog(PR_ERR, "Bank hash failed to calculate somehow\n");
+		return rc;
+	}
+
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX,
+			     tpmnv_control_image->bank_hash[0],
+			     SHA256_DIGEST_SIZE,
+			     offsetof(struct tpmnv_control,
+			     bank_hash[0]));
+	if (rc) {
+		prlog(PR_ERR, "Could not write fresh formatted bank hashes to CONTROL index, rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = flash_secboot_write(0, secboot_image, sizeof(struct secboot));
+	if (rc)
+		prlog(PR_ERR, "Could not write formatted data to PNOR, rc=%d\n", rc);
+
+	return rc;
+}
+
+
+/*
+ * Serialize one variable to a target memory location.
+ * Returns the advanced target pointer,
+ *   NULL if advanced pointer would exceed the supplied bound
+ */
+static char *secboot_serialize_secvar(char *target, const struct secvar *var, const char *end)
+{
+	if ((target + sizeof(uint64_t) + sizeof(uint64_t)
+		+ var->key_len + var->data_size) > end)
+		return NULL;
+
+	*((uint64_t*) target) = cpu_to_be64(var->key_len);
+	target += sizeof(var->key_len);
+	*((uint64_t*) target) = cpu_to_be64(var->data_size);
+	target += sizeof(var->data_size);
+	memcpy(target, var->key, var->key_len);
+	target += var->key_len;
+	memcpy(target, var->data, var->data_size);
+	target += var->data_size;
+
+	return target;
+}
+
+
+/* Flattens a linked-list bank into a contiguous buffer for writing */
+static int secboot_serialize_bank(const struct list_head *bank, char *target,
+				  size_t target_size, int flags)
+{
+	struct secvar *var;
+	char *end = target + target_size;
+
+	assert(bank);
+	assert(target);
+
+	memset(target, 0x00, target_size);
+
+	// TODO: maybe do a size check before even writing?
+	// TODO: maybe add secvar_sizeof() function to make this easier?
+
+	list_for_each(bank, var, link) {
+		if (var->flags != flags)
+			continue;
+
+		target = secboot_serialize_secvar(target, var, end);
+		if (!target) {
+			prlog(PR_ERR, "Ran out of %s space, giving up!",
+				(flags & SECVAR_FLAG_PROTECTED) ? "TPMNV" : "PNOR");
+			return OPAL_EMPTY;
+		}
+	}
+
+	return OPAL_SUCCESS;
+}
+
+/* Helper for the variable-bank specific writing logic */
+static int secboot_tpm_write_variable_bank(const struct list_head *bank)
+{
+	int rc;
+	uint64_t bit;
+
+	bit = CYCLE_BIT(tpmnv_control_image->active_bit);
+	/* Serialize TPMNV variables */
+	rc = secboot_serialize_bank(bank, tpmnv_vars_image->vars, tpmnv_vars_size - sizeof(struct tpmnv_vars), SECVAR_FLAG_PROTECTED);
+	if (rc)
+		goto out;
+
+
+	/* Write TPMNV variables to actual NV */
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_VARS_INDEX, tpmnv_vars_image, tpmnv_vars_size, 0);
+	if (rc)
+		goto out;
+
+	/* Serialize the PNOR variables, but don't write to flash until after the bank hash */
+	rc = secboot_serialize_bank(bank, secboot_image->bank[bit], SECBOOT_VARIABLE_BANK_SIZE, 0);
+	if (rc)
+		goto out;
+
+	/* Calculate the bank hash, and write to TPM NV */
+	rc = calc_bank_hash(tpmnv_control_image->bank_hash[bit], secboot_image->bank[bit], SECBOOT_VARIABLE_BANK_SIZE);
+	if (rc)
+		goto out;
+
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX, tpmnv_control_image->bank_hash[bit],
+				SHA256_DIGEST_LENGTH, offsetof(struct tpmnv_control, bank_hash[bit]));
+	if (rc)
+		goto out;
+
+	/* Write new variable bank to pnor */
+	rc = flash_secboot_write(0, secboot_image, sizeof(struct secboot));
+	if (rc)
+		goto out;
+
+	/* Flip the bit, and write to TPM NV */
+	tpmnv_control_image->active_bit = bit;
+	rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX,
+			     &tpmnv_control_image->active_bit,
+			     sizeof(tpmnv_control_image->active_bit),
+			     offsetof(struct tpmnv_control, active_bit));
+out:
+
+	return rc;
+}
+
+static int secboot_tpm_write_bank(struct list_head *bank, int section)
+{
+	int rc;
+
+	switch (section) {
+	case SECVAR_VARIABLE_BANK:
+		rc = secboot_tpm_write_variable_bank(bank);
+		break;
+	case SECVAR_UPDATE_BANK:
+		memset(secboot_image->update, 0, SECBOOT_UPDATE_BANK_SIZE);
+		rc = secboot_serialize_bank(bank, secboot_image->update,
+					    SECBOOT_UPDATE_BANK_SIZE, 0);
+		if (rc)
+			break;
+
+		rc = flash_secboot_write(0, secboot_image,
+					    sizeof(struct secboot));
+		break;
+	default:
+		rc = OPAL_HARDWARE;
+	}
+
+	return rc;
+}
+
+
+/*
+ * Deserialize a single secvar from a buffer.
+ * Returns an advanced pointer, and an allocated secvar in *var.
+ * Returns NULL if out of bounds reached, or out of memory.
+ */
+static int secboot_deserialize_secvar(struct secvar **var, char **src, const char *end)
+{
+	uint64_t key_len;
+	uint64_t data_size;
+	struct secvar *ret;
+
+	assert(var);
+
+	/* Load in the two header values */
+	key_len = be64_to_cpu(*((uint64_t *) *src));
+	*src += sizeof(uint64_t);
+	data_size = be64_to_cpu(*((uint64_t *) *src));
+	*src += sizeof(uint64_t);
+
+	/* Check if we've reached the last var to deserialize */
+	if ((key_len == 0) && (data_size == 0)) {
+		return OPAL_EMPTY;
+	}
+
+	if (key_len > SECVAR_MAX_KEY_LEN) {
+		prlog(PR_ERR, "Deserialization failed: key length exceeded maximum value"
+			"%llu > %u", key_len, SECVAR_MAX_KEY_LEN);
+		return OPAL_RESOURCE;
+	}
+	if (data_size > SECBOOT_TPM_MAX_VAR_SIZE) {
+		prlog(PR_ERR, "Deserialization failed: data size exceeded maximum value"
+			"%llu > %u", key_len, SECBOOT_TPM_MAX_VAR_SIZE);
+		return OPAL_RESOURCE;
+	}
+
+	/* Make sure these fields aren't oversized... */
+	if ((*src + key_len + data_size) > end) {
+		*var = NULL;
+		prlog(PR_ERR, "key_len or data_size exceeded the expected bounds");
+		return OPAL_RESOURCE;
+	}
+
+	ret = alloc_secvar(key_len, data_size);
+	if (!ret) {
+		*var = NULL;
+		prlog(PR_ERR, "Out of memory, could not allocate new secvar");
+		return OPAL_NO_MEM;
+	}
+
+	/* Load in variable-sized data */
+	memcpy(ret->key, *src, ret->key_len);
+	*src += ret->key_len;
+	memcpy(ret->data, *src, ret->data_size);
+	*src += ret->data_size;
+
+	*var = ret;
+
+	return OPAL_SUCCESS;
+}
+
+
+/* Load variables from a flattened buffer into a bank list */
+static int secboot_tpm_deserialize_from_buffer(struct list_head *bank, char *src,
+					       uint64_t size, uint64_t flags)
+{
+	struct secvar *var;
+	char *cur;
+	char *end;
+	int rc = 0;
+
+	cur = src;
+	end = src + size;
+
+	while (cur < end) {
+		/* Ensure there is enough space to even check for another var header */
+		if ((end - cur) < (sizeof(uint64_t) * 2))
+			break;
+
+		rc = secboot_deserialize_secvar(&var, &cur, end);
+		switch (rc) {
+			case OPAL_RESOURCE:
+			case OPAL_NO_MEM:
+				goto fail;
+			case OPAL_EMPTY:
+				goto done;
+			default: assert(1);
+		}
+
+		var->flags |= flags;
+
+		list_add_tail(bank, &var->link);
+	}
+done:
+	return OPAL_SUCCESS;
+fail:
+	clear_bank_list(bank);
+	return rc;
+}
+
+static int secboot_tpm_load_variable_bank(struct list_head *bank)
+{
+	char bank_hash[SHA256_DIGEST_LENGTH];
+	uint64_t bit = tpmnv_control_image->active_bit;
+	int rc;
+
+	/* Check the hash of the bank we loaded from PNOR
+	 *  versus the expected hash in TPM NV */
+	rc = calc_bank_hash(bank_hash,
+			    secboot_image->bank[bit],
+			    SECBOOT_VARIABLE_BANK_SIZE);
+	if (rc)
+		return rc;
+
+	if (memcmp(bank_hash,
+		   tpmnv_control_image->bank_hash[bit],
+		   SHA256_DIGEST_LENGTH))
+		/* Tampered pnor space detected, abandon ship */
+		return OPAL_PERMISSION;
+
+	rc = secboot_tpm_deserialize_from_buffer(bank, tpmnv_vars_image->vars, tpmnv_vars_size, SECVAR_FLAG_PROTECTED);
+	if (rc)
+		return rc;
+
+	return secboot_tpm_deserialize_from_buffer(bank, secboot_image->bank[bit], SECBOOT_VARIABLE_BANK_SIZE, 0);
+}
+
+
+static int secboot_tpm_load_bank(struct list_head *bank, int section)
+{
+	switch (section) {
+	case SECVAR_VARIABLE_BANK:
+		return secboot_tpm_load_variable_bank(bank);
+	case SECVAR_UPDATE_BANK:
+		return secboot_tpm_deserialize_from_buffer(bank, secboot_image->update, SECBOOT_UPDATE_BANK_SIZE, 0);
+	}
+
+	return OPAL_HARDWARE;
+}
+
+
+/* Ensure the NV indices were defined with the correct set of attributes */
+static int secboot_tpm_check_tpmnv_attrs(void)
+{
+	TPMS_NV_PUBLIC nv_public; /* Throwaway, we only want the name field */
+	TPM2B_NAME nv_vars_name;
+	TPM2B_NAME nv_control_name;
+	int rc;
+
+	rc = tpmnv_ops.readpublic(SECBOOT_TPMNV_VARS_INDEX,
+				  &nv_public,
+				  &nv_vars_name);
+	if (rc) {
+		prlog(PR_ERR, "Failed to readpublic from the VARS index, rc=%d\n", rc);
+		return rc;
+	}
+	rc = tpmnv_ops.readpublic(SECBOOT_TPMNV_CONTROL_INDEX,
+				  &nv_public,
+				  &nv_control_name);
+	if (rc) {
+		prlog(PR_ERR, "Failed to readpublic from the CONTROL index, rc=%d\n", rc);
+		return rc;
+	}
+
+	if (memcmp(tpmnv_vars_name,
+		   nv_vars_name.t.name,
+		   sizeof(tpmnv_vars_name))) {
+		prlog(PR_ERR, "VARS index not defined with the correct attributes\n");
+		return OPAL_RESOURCE;
+	}
+	if (memcmp(tpmnv_control_name,
+		   nv_control_name.t.name,
+		   sizeof(tpmnv_control_name))) {
+		prlog(PR_ERR, "CONTROL index not defined with the correct attributes\n");
+		return OPAL_RESOURCE;
+	}
+
+	return OPAL_SUCCESS;
+}
+
+
+static int secboot_tpm_define_indices(void)
+{
+	int rc = OPAL_SUCCESS;
+
+	rc = tpmnv_ops.definespace(SECBOOT_TPMNV_VARS_INDEX, tpmnv_vars_size);
+	if (rc) {
+		prlog(PR_ERR, "Failed to define the VARS index, rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = tpmnv_ops.definespace(SECBOOT_TPMNV_CONTROL_INDEX, sizeof(struct tpmnv_control));
+	if (rc) {
+		prlog(PR_ERR, "Failed to define the CONTROL index, rc=%d\n", rc);
+		return rc;
+	}
+
+	rc = tpmnv_format();
+	if (rc)
+		return rc;
+
+	/* TPM NV just got redefined, so unconditionally format the SECBOOT partition */
+	return secboot_format();
+}
+
+static int secboot_tpm_store_init(void)
+{
+	int rc;
+	unsigned int secboot_size;
+
+	TPMI_RH_NV_INDEX *indices = NULL;
+	size_t count = 0;
+	bool control_defined = false;
+	bool vars_defined = false;
+	int i;
+
+	if (secboot_image)
+		return OPAL_SUCCESS;
+
+	prlog(PR_DEBUG, "Initializing for pnor+tpm based platform\n");
+
+	/* Initialize SECBOOT first, we may need to format this later */
+	rc = flash_secboot_info(&secboot_size);
+	if (rc) {
+		prlog(PR_ERR, "error %d retrieving keystore info\n", rc);
+		goto error;
+	}
+	if (sizeof(struct secboot) > secboot_size) {
+		prlog(PR_ERR, "secboot partition %d KB too small. min=%ld\n",
+		      secboot_size >> 10, sizeof(struct secboot));
+		rc = OPAL_RESOURCE;
+		goto error;
+	}
+
+	secboot_image = memalign(0x1000, sizeof(struct secboot));
+	if (!secboot_image) {
+		prlog(PR_ERR, "Failed to allocate space for the secboot image\n");
+		rc = OPAL_NO_MEM;
+		goto error;
+	}
+
+	/* Read in the PNOR data, bank hash is checked on call to .load_bank() */
+	rc = flash_secboot_read(secboot_image, 0, sizeof(struct secboot));
+	if (rc) {
+		prlog(PR_ERR, "failed to read the secboot partition, rc=%d\n", rc);
+		goto error;
+	}
+
+	/* Allocate the tpmnv data buffers */
+	tpmnv_vars_image = zalloc(tpmnv_vars_size);
+	if (!tpmnv_vars_image)
+		return OPAL_NO_MEM;
+	tpmnv_control_image = zalloc(sizeof(struct tpmnv_control));
+	if (!tpmnv_control_image)
+		return OPAL_NO_MEM;
+
+	/* Check if the NV indices have been defined already */
+	rc = tpmnv_ops.getindices(&indices, &count);
+	if (rc) {
+		prlog(PR_ERR, "Could not load defined indicies from TPM, rc=%d\n", rc);
+		goto error;
+	}
+
+	for (i = 0; i < count; i++) {
+		if (indices[i] == SECBOOT_TPMNV_VARS_INDEX)
+			vars_defined = true;
+		else if (indices[i] == SECBOOT_TPMNV_CONTROL_INDEX)
+			control_defined = true;
+	}
+	free(indices);
+
+	/* Undefine the NV indices if physical presence has been asserted */
+	if (secvar_check_physical_presence()) {
+		prlog(PR_INFO, "Physical presence asserted, redefining NV indices, and resetting keystore\n");
+
+		if (vars_defined) {
+			rc = tpmnv_ops.undefinespace(SECBOOT_TPMNV_VARS_INDEX);
+			if (rc) {
+				prlog(PR_ERR, "Physical presence failed to undefine VARS, something is seriously wrong\n");
+				goto error;
+			}
+		}
+
+		if (control_defined) {
+			rc = tpmnv_ops.undefinespace(SECBOOT_TPMNV_CONTROL_INDEX);
+			if (rc) {
+				prlog(PR_ERR, "Physical presence failed to undefine CONTROL, something is seriously wrong\n");
+				goto error;
+			}
+		}
+
+		vars_defined = control_defined = false;
+	}
+
+	/* Determine if we need to define the indices. These should BOTH be false or true */
+	if (!vars_defined && !control_defined) {
+		rc = secboot_tpm_define_indices();
+		if (rc)
+			goto error;
+
+		/* Indicies got defined and formatted, we're done here */
+		goto done;
+	} else if (vars_defined ^ control_defined) {
+		/* This should never happen. Both indices should be defined at the same
+		 * time. Otherwise something seriously went wrong. */
+		prlog(PR_ERR, "NV indices defined with unexpected attributes. Assert physical presence to clear\n");
+		goto error;
+	}
+
+	/* Ensure the NV indices were defined with the correct set of attributes */
+	rc = secboot_tpm_check_tpmnv_attrs();
+	if (rc)
+		goto error;
+
+	/* TPMNV indices exist, are correct, and weren't just formatted, so read them in */
+	rc = tpmnv_ops.read(SECBOOT_TPMNV_VARS_INDEX,
+			    tpmnv_vars_image,
+			    tpmnv_vars_size, 0);
+	if (rc) {
+		prlog(PR_ERR, "Failed to read from the VARS index\n");
+		goto error;
+	}
+
+	rc = tpmnv_ops.read(SECBOOT_TPMNV_CONTROL_INDEX,
+			    tpmnv_control_image,
+			    sizeof(struct tpmnv_control), 0);
+	if (rc) {
+		prlog(PR_ERR, "Failed to read from the CONTROL index\n");
+		goto error;
+	}
+
+	/* Verify the header information is correct */
+	if (tpmnv_vars_image->header.magic_number != SECBOOT_MAGIC_NUMBER ||
+	    tpmnv_control_image->header.magic_number != SECBOOT_MAGIC_NUMBER ||
+	    tpmnv_vars_image->header.version != SECBOOT_VERSION ||
+	    tpmnv_control_image->header.version != SECBOOT_VERSION) {
+		prlog(PR_ERR, "TPMNV indices defined, but contain bad data. Assert physical presence to clear\n");
+		goto error;
+	}
+
+	/* Verify the secboot partition header information,
+	 *  reformat if incorrect
+	 * Note: Future variants should attempt to handle older versions safely
+	 */
+	if (secboot_image->header.magic_number != SECBOOT_MAGIC_NUMBER ||
+	    secboot_image->header.version != SECBOOT_VERSION) {
+		rc = secboot_format();
+		if (rc)
+			goto error;
+	}
+
+done:
+	return OPAL_SUCCESS;
+
+error:
+	free(secboot_image);
+	secboot_image = NULL;
+	free(tpmnv_vars_image);
+	tpmnv_vars_image = NULL;
+	free(tpmnv_control_image);
+	tpmnv_control_image = NULL;
+
+	return rc;
+}
+
+
+static void secboot_tpm_lockdown(void)
+{
+	/* Note: While write lock is called here on the two NV indices,
+	 * both indices are also defined on the platform hierarchy.
+	 * The platform hierarchy auth is set later in the skiboot
+	 * initialization process, and not by any secvar-related code.
+	 */
+	int rc;
+
+	rc = tpmnv_ops.writelock(SECBOOT_TPMNV_VARS_INDEX);
+	if (rc) {
+		prlog(PR_EMERG, "TSS Write Lock failed on VARS index, halting.\n");
+		abort();
+	}
+
+	rc = tpmnv_ops.writelock(SECBOOT_TPMNV_CONTROL_INDEX);
+	if (rc) {
+		prlog(PR_EMERG, "TSS Write Lock failed on CONTROL index, halting.\n");
+		abort();
+	}
+}
+
+struct secvar_storage_driver secboot_tpm_driver = {
+	.load_bank = secboot_tpm_load_bank,
+	.write_bank = secboot_tpm_write_bank,
+	.store_init = secboot_tpm_store_init,
+	.lockdown = secboot_tpm_lockdown,
+	.max_var_size = SECBOOT_TPM_MAX_VAR_SIZE,
+};
diff --git a/libstb/secvar/storage/secboot_tpm.h b/libstb/secvar/storage/secboot_tpm.h
new file mode 100644
index 00000000..30a747a7
--- /dev/null
+++ b/libstb/secvar/storage/secboot_tpm.h
@@ -0,0 +1,61 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#ifndef _SECBOOT_TPM_H_
+#define _SECBOOT_TPM_H_
+
+#include <ibmtss/tss.h>
+
+#define SECBOOT_VARIABLE_BANK_SIZE	32000
+#define SECBOOT_UPDATE_BANK_SIZE	32000
+
+#define SECBOOT_VARIABLE_BANK_NUM	2
+
+/* Because mbedtls doesn't define this? */
+#define SHA256_DIGEST_LENGTH	32
+
+/* 0x5053424b = "PSBK" or Power Secure Boot Keystore */
+#define SECBOOT_MAGIC_NUMBER	0x5053424b
+#define SECBOOT_VERSION		1
+
+#define SECBOOT_TPMNV_VARS_INDEX	0x01c10190
+#define SECBOOT_TPMNV_CONTROL_INDEX	0x01c10191
+
+struct secboot_header {
+	uint32_t magic_number;
+	uint8_t version;
+	uint8_t reserved[3];	/* Fix alignment */
+} __attribute__((packed));
+
+struct secboot {
+	struct secboot_header header;
+	char bank[SECBOOT_VARIABLE_BANK_NUM][SECBOOT_VARIABLE_BANK_SIZE];
+	char update[SECBOOT_UPDATE_BANK_SIZE];
+} __attribute__((packed));
+
+struct tpmnv_vars {
+	struct secboot_header header;
+	char vars[0];
+} __attribute__((packed));
+
+struct tpmnv_control {
+	struct secboot_header header;
+	uint8_t active_bit;
+	char bank_hash[SECBOOT_VARIABLE_BANK_NUM][SHA256_DIGEST_LENGTH];
+} __attribute__((packed));
+
+struct tpmnv_ops_s {
+	int (*read)(TPMI_RH_NV_INDEX nv, void*, size_t, uint16_t);
+	int (*write)(TPMI_RH_NV_INDEX nv, void*, size_t, uint16_t);
+	int (*writelock)(TPMI_RH_NV_INDEX);
+	int (*definespace)(TPMI_RH_NV_INDEX, uint16_t);
+	int (*getindices)(TPMI_RH_NV_INDEX**, size_t*);
+	int (*undefinespace)(TPMI_RH_NV_INDEX);
+	int (*readpublic)(TPMI_RH_NV_INDEX, TPMS_NV_PUBLIC*, TPM2B_NAME*);
+};
+
+extern struct tpmnv_ops_s tpmnv_ops;
+
+extern const uint8_t tpmnv_vars_name[];
+extern const uint8_t tpmnv_control_name[];
+
+#endif
diff --git a/libstb/secvar/storage/tpmnv_ops.c b/libstb/secvar/storage/tpmnv_ops.c
new file mode 100644
index 00000000..d6135c31
--- /dev/null
+++ b/libstb/secvar/storage/tpmnv_ops.c
@@ -0,0 +1,15 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
+/* Copyright 2020 IBM Corp. */
+#include <tssskiboot.h>
+#include "secboot_tpm.h"
+
+struct tpmnv_ops_s tpmnv_ops = {
+	.read = tss_nv_read,
+	.write = tss_nv_write,
+	.writelock = tss_nv_write_lock,
+	.definespace = tss_nv_define_space,
+	.getindices = tss_get_defined_nv_indices,
+	.undefinespace = tss_nv_undefine_space,
+	.readpublic = tss_nv_read_public,
+};
+