[Skiboot] [RFC 3/8] secvar/drivers: add mode-switchable storage driver for secboot_tpm
Eric Richter
erichte at linux.ibm.com
Wed Sep 22 13:11:24 AEST 2021
This patch adds a new storage driver that detects whether a system should
load static built-in variables, or load from the regular SECBOOT PNOR
partition and TPM NV.
The storage driver is responsible for maintaining the protected storage
available (in this case, TPM NV), therefore it must be also be responsible
for preserving this state across reboots in this protected storage.
Ultimately, the storage driver controls the secure boot state of the
system -- if it cannot load the variables, then there is nothing for the
backend to operate on. Furthermore, if it cannot load from the root of
trust, then we have no idea what the previous state of the system may
have been.
This new driver operates in two "modes", USER_MODE and SYSTEM_MODE.
USER_MODE operates exactly as the normal secboot_tpm driver does.
SYSTEM_MODE does not load anything from persistant storage, and passes off
the loading of default variables to the backend.
This driver utilizes a previously unused TPM NV index reserved for opal
secure boot use (0x01c10192) to store which mode it is operating in.
The driver will initialize itself into a mode based on the value stored in
the MODE index, and if undefined, on the status of the indices used by the
secboot_tpm driver. The following describes the possible states:
- MODE undefined, VARS/CONTROL undefined -> SYSTEM_MODE
- MODE undefined, VARS/CONTROL defined -> USER_MODE
- MODE index defined, value = SYSTEM_MODE -> SYSTEM_MODE
- MODE index defined, value = USER_MODE -> USER_MODE
The first option details the first-boot scenario, the system (if built
with default variables) will boot directly into SYSTEM_MODE. The second
option covers a scenario where the machine may have initialized secvar
prior (e.g. firmware upgrade occurred), and so it initializes in USER_MODE
instead, to preserve the machine's previous state.
Physical presence is used to switch between modes. Asserting with the
clear-os-keys option will set to USER_MODE, and effectively disable
secure boot. Asserting with reset-default-keys will set to SYSTEM_MODE,
and secure boot will be enabled with the default keys provided by the
backend.
RFC NOTE: This is probably the messiest part of this patch set. I am trying to
keep the secboot_tpm driver relatively untouched, so that these new modes
are purely an extension on the existing driver(s). However, as the TPM NV
space is needed to securely preserve the state, this new driver needs to
re-implement a lot of the same TPM NV initialization logic.
I am looking to refactor the existing secboot_tpm driver to be more
modular, and also better reactive to some of its components being
pre-initialized. At the moment, it should most "work" as is, it is just
very inelegant as it repeats a lot of the same initialization steps when
the init function is called in USER_MODE.
I have left a lot of TODO comments in where I would be copy-pasting even
more bits from secboot_tpm -- all candidates for a better refactor and
reuse of code from secboot_tpm.
Signed-off-by: Eric Richter <erichte at linux.ibm.com>
---
include/secvar.h | 2 +
libstb/secvar/storage/Makefile.inc | 2 +-
.../secvar/storage/secboot_tpm_switchable.c | 251 ++++++++++++++++++
.../secvar/storage/secboot_tpm_switchable.h | 15 ++
4 files changed, 269 insertions(+), 1 deletion(-)
create mode 100644 libstb/secvar/storage/secboot_tpm_switchable.c
create mode 100644 libstb/secvar/storage/secboot_tpm_switchable.h
diff --git a/include/secvar.h b/include/secvar.h
index 413d7997..259b9b63 100644
--- a/include/secvar.h
+++ b/include/secvar.h
@@ -5,6 +5,7 @@
#define _SECVAR_DRIVER_
#include <stdint.h>
+#include <ccan/list/list.h>
struct secvar;
@@ -37,6 +38,7 @@ struct secvar_backend_driver {
};
extern struct secvar_storage_driver secboot_tpm_driver;
+extern struct secvar_storage_driver secboot_tpm_switchable_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/storage/Makefile.inc b/libstb/secvar/storage/Makefile.inc
index 50156fe2..dbddd6e5 100644
--- a/libstb/secvar/storage/Makefile.inc
+++ b/libstb/secvar/storage/Makefile.inc
@@ -7,7 +7,7 @@ SUBDIRS += $(SECVAR_STORAGE_DIR)
# Swap the comment on these two lines to use the fake TPM NV
# implementation hardware without a TPM
-SECVAR_STORAGE_OBJS = secboot_tpm.o tpmnv_ops.o
+SECVAR_STORAGE_OBJS = secboot_tpm.o tpmnv_ops.o secboot_tpm_switchable.o
#SECVAR_STORAGE_OBJS = secboot_tpm.o fakenv_ops.o
SECVAR_STORAGE = $(SECVAR_STORAGE_DIR)/built-in.a
diff --git a/libstb/secvar/storage/secboot_tpm_switchable.c b/libstb/secvar/storage/secboot_tpm_switchable.c
new file mode 100644
index 00000000..0790ed68
--- /dev/null
+++ b/libstb/secvar/storage/secboot_tpm_switchable.c
@@ -0,0 +1,251 @@
+#include <stdlib.h>
+#include <skiboot.h>
+#include <opal.h>
+#include "../secvar.h"
+#include "../secvar_devtree.h"
+#include "secboot_tpm.h"
+#include <tssskiboot.h>
+#include <ibmtss/TPM_Types.h>
+#include "secboot_tpm_switchable.h"
+
+/* POTENTIAL ITEMS TO MOVE TO secboot_tpm.h */
+#define SECBOOT_TPM_MAX_VAR_SIZE 8192
+
+extern const uint8_t tpmnv_vars_name[34];
+extern const uint8_t tpmnv_vars_prov_name[34];
+extern const uint8_t tpmnv_control_name[34];
+extern const uint8_t tpmnv_control_prov_name[34];
+
+/* END POTENTIALLY HEADER STUFF */
+
+// TODO: actually get the correct mode name hash here.
+const uint8_t tpmnv_mode_name[] = {
+ 0x00, 0x0b,
+ 0x7a, 0xe8, 0x16, 0xe0, 0x15, 0xab, 0xdc, 0xd8, 0xc6, 0xe6, 0xa1, 0x6f, 0x26, 0x71, 0x65, 0x96,
+ 0x69, 0x15, 0x04, 0xd1, 0x05, 0x14, 0xfc, 0xf0, 0x82, 0x17, 0x99, 0x22, 0x4e, 0x06, 0xe5, 0x76,
+};
+
+const uint8_t tpmnv_mode_prov_name[] = {
+ 0x00, 0x0b,
+ 0x7a, 0xe8, 0x16, 0xe0, 0x15, 0xab, 0xdc, 0xd8, 0xc6, 0xe6, 0xa1, 0x6f, 0x26, 0x71, 0x65, 0x96,
+ 0x69, 0x15, 0x04, 0xd1, 0x05, 0x14, 0xfc, 0xf0, 0x82, 0x17, 0x99, 0x22, 0x4e, 0x06, 0xe5, 0x76,
+};
+
+uint64_t mode_value = 0;
+
+
+// TODO: break down into reusable functions w/ the functions in secboot_tpm_store_init
+static int pre_switchable_init(void)
+{
+ TPMI_RH_NV_INDEX *indices = NULL;
+ char nv_vars_name[sizeof(tpmnv_vars_name)];
+ char nv_control_name[sizeof(tpmnv_control_name)];
+ char nv_mode_name[sizeof(tpmnv_mode_name)];
+ size_t count = 0;
+ bool control_defined = false;
+ bool vars_defined = false;
+ bool mode_defined = false;
+ int i, rc;
+
+ /* 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;
+ else if (indices[i] == SECBOOT_TPMNV_MODE_INDEX)
+ mode_defined = true;
+ }
+ free(indices);
+
+ if (secvar_check_clear_keys() || secvar_check_set_default_keys()) {
+ /* TODO: undefine *ALL THREE* indices */
+ mode_defined = control_defined = vars_defined = false;
+ }
+
+ if (mode_defined) {
+ // TODO: Probably break a lot of the following up into a reusable function between
+ // this driver and secboot_tpm
+ TPMS_NV_PUBLIC nv_public;
+ TPM2B_NAME vars_tmp;
+
+ rc = tpmnv_ops.readpublic(SECBOOT_TPMNV_MODE_INDEX,
+ &nv_public,
+ &vars_tmp);
+ if (rc) {
+ prlog(PR_ERR, "Failed to readpublic from the MODE index, rc=%d\n", rc);
+ return rc;
+ }
+
+ memcpy(nv_mode_name, vars_tmp.t.name, MIN(sizeof(tpmnv_vars_name), vars_tmp.t.size));
+
+
+ // TODO: perhaps rework this function too, or just drop that extra function
+ if (!memcmp(tpmnv_mode_prov_name, nv_mode_name, sizeof(tpmnv_mode_prov_name))) {
+ /* Detected a provisioned mode index, claim it here and set the mode to system mode */
+ // TODO: perhaps not hardcode the space
+ rc = tpmnv_ops.definespace(SECBOOT_TPMNV_MODE_INDEX, sizeof(uint64_t));
+ if (rc) {
+ prlog(PR_ERR, "Failed to define the MODE index, rc=%d\n", rc);
+ return rc;
+ }
+
+ /* Write TPMNV variables to actual NV */
+ mode_value = 1;
+ rc = tpmnv_ops.write(SECBOOT_TPMNV_MODE_INDEX, &mode_value, sizeof(mode_value), 0);
+ if (rc)
+ return rc;
+
+ return OPAL_SUCCESS;
+ }
+
+ if (!memcmp(tpmnv_mode_name, nv_mode_name, sizeof(tpmnv_mode_name))) {
+ /* Detected an improperly defined mode index, give up? */
+ abort(); // TODO: don't actually abort, this should be in an init function
+ }
+
+
+ rc = tpmnv_ops.read(SECBOOT_TPMNV_VARS_INDEX,
+ &mode_value,
+ sizeof(mode_value), 0);
+ if (rc) {
+ prlog(PR_ERR, "Failed to read from the VARS index\n");
+ goto error;
+ }
+
+ return OPAL_SUCCESS;
+ }
+
+ /* Mode index was not defined, so we need to check the other indices first */
+
+ /* Determine if we need to define the indices. These should BOTH be false or true */
+ if (!vars_defined && !control_defined) {
+ // TODO: Define *ALL THREE* indices here, perhaps let the other init handle the formatting?
+ // or do we format here as well in secboot_tpm format?
+
+ if (secvar_check_clear_keys()) {
+ /* clear-os-keys was issued, continue in user mode (OS secure boot disabled) */
+ mode_value = USER_MODE;
+ }
+
+ /* reset-default-keys likely undefined the indices, either way default to system mode */
+ mode_value = SYSTEM_MODE;
+
+ goto define;
+ }
+ 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;
+ }
+
+ rc = secboot_tpm_get_tpmnv_names(nv_vars_name, nv_control_name);
+ if (rc)
+ goto error;
+
+ if (secboot_tpm_check_provisioned_indices(nv_vars_name, nv_control_name)) {
+ // TODO: Reclaim the NV indices here, we default to system mode (noop),
+ // so that driver will not handle it.
+
+ mode_value = SYSTEM_MODE;
+ goto define;
+ }
+
+ /* Otherwise, ensure the NV indices were defined with the correct set of attributes */
+ rc = secboot_tpm_check_tpmnv_attrs(nv_vars_name, nv_control_name);
+ if (rc)
+ goto error;
+
+ /* Reaching here, mode index was not defined, and the other NV indices were.
+ * This is (likely) a firmware update case, where we should preserve the previous
+ * secure boot state. */
+ mode_value = SYSTEM_MODE;
+
+define:
+ rc = tpmnv_ops.definespace(SECBOOT_TPMNV_MODE_INDEX, sizeof(mode_value));
+ if (rc) {
+ prlog(PR_ERR, "Failed to define the MODE index, rc=%d\n", rc);
+ return rc;
+ }
+
+ rc = tpmnv_ops.write(SECBOOT_TPMNV_MODE_INDEX,
+ &mode_value,
+ sizeof(mode_value), 0);
+ if (rc) {
+ prlog(PR_ERR, "Could not write to the MODE index, rc=%d\n", rc);
+ goto error;
+ }
+
+ return OPAL_SUCCESS;
+
+error:
+ // TODO: error handle better here
+ return rc;
+}
+
+static int switchable_store_init(void)
+{
+ int rc;
+
+ rc = pre_switchable_init();
+ if (rc) {
+ prlog(PR_ERR, "Failed to initialize/detect secure boot mode\n");
+ }
+
+ if (mode_value == USER_MODE)
+ return secboot_tpm_driver.store_init();
+ return OPAL_SUCCESS;
+}
+
+static int switchable_load_bank(struct list_head *bank, int section)
+{
+ if (mode_value == USER_MODE)
+ return secboot_tpm_driver.load_bank(bank, section);
+ return OPAL_SUCCESS;
+}
+
+static int switchable_write_bank(struct list_head *bank, int section)
+{
+ if (mode_value == USER_MODE)
+ return secboot_tpm_driver.write_bank(bank, section);
+ return OPAL_SUCCESS;
+}
+
+static void switchable_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_MODE_INDEX);
+ if (rc) {
+ prlog(PR_EMERG, "TSS Write Lock failed on CONTROL index, halting.\n");
+ abort();
+ }
+
+ /* Unconditionally call the secboot_tpm lockdown as well to lock the other
+ * two indices we care about. */
+ secboot_tpm_driver.lockdown();
+}
+
+
+struct secvar_storage_driver secboot_tpm_switchable_driver = {
+ .store_init = switchable_store_init,
+
+ /* These two hooks are no-ops, no storage actually needed */
+ .load_bank = switchable_load_bank,
+ .write_bank = switchable_write_bank,
+
+ .lockdown = switchable_lockdown,
+ .max_var_size = SECBOOT_TPM_MAX_VAR_SIZE,
+};
diff --git a/libstb/secvar/storage/secboot_tpm_switchable.h b/libstb/secvar/storage/secboot_tpm_switchable.h
new file mode 100644
index 00000000..2c95b387
--- /dev/null
+++ b/libstb/secvar/storage/secboot_tpm_switchable.h
@@ -0,0 +1,15 @@
+#ifndef _SECBOOT_TPM_SWITCHABLE_H_
+#define _SECBOOT_TPM_SWITCHABLE_H_
+
+#include <stdint.h>
+
+#define SECBOOT_TPMNV_MODE_INDEX 0x01c10192
+
+extern uint64_t mode_value;
+
+enum switchable_modes {
+ USER_MODE,
+ SYSTEM_MODE,
+};
+
+#endif
\ No newline at end of file
--
2.33.0
More information about the Skiboot
mailing list