From patchwork Wed Sep 22 03:11:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Richter X-Patchwork-Id: 1531001 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Cc9K9sR2; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4HDjyK20rPz9sRK for ; Wed, 22 Sep 2021 13:12:09 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4HDjyK0mzQz2yw9 for ; Wed, 22 Sep 2021 13:12:09 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Cc9K9sR2; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linux.ibm.com (client-ip=148.163.156.1; helo=mx0a-001b2d01.pphosted.com; envelope-from=erichte@linux.ibm.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Cc9K9sR2; dkim-atps=neutral Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4HDjxp6D6tz2yL9 for ; Wed, 22 Sep 2021 13:11:42 +1000 (AEST) Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 18M2dEJL018825 for ; Tue, 21 Sep 2021 23:11:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=ld6GTwhWoP6vl4EGQosnky9ZHSCzCEPSnd7U96e5esQ=; b=Cc9K9sR2gZZ97aQiGyessHdPxm/Kvtm6cQ4jMOGl8OHXSy0z9t14EZcaRIFVWlMNtntR bT9rWUHK+oBXbEvd5GkQQEnnN7idYRG9QWwKppx4KCazeZaYQUKQna5Ag6za2e2xGwmc qGpyNdtq1AL0MRkTJcu943jNCWSc14S6s24bNYkwUHmGW0Np8/VsUiR2UofZHynk++v1 qzX7nXb8YdKj+WRQTL9EPvnn5s6t4p0jTJ3dVrpi19CC8KzU9vo7yvMGO4bvlyFZ8iFs 3ZftDFMBLtZqjfkzzp1o0ZGc6NNbRQZloHe8A21yyqFWpyAcaTSwBpHwZcv8z+S3DfMS bA== Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0a-001b2d01.pphosted.com with ESMTP id 3b7t09a7u8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 21 Sep 2021 23:11:40 -0400 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 18M380pb030034 for ; Wed, 22 Sep 2021 03:11:38 GMT Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by ppma04ams.nl.ibm.com with ESMTP id 3b7q6qsjan-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 22 Sep 2021 03:11:38 +0000 Received: from d06av23.portsmouth.uk.ibm.com (d06av23.portsmouth.uk.ibm.com [9.149.105.59]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 18M3BYGm983764 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 22 Sep 2021 03:11:34 GMT Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A1972A405D; Wed, 22 Sep 2021 03:11:34 +0000 (GMT) Received: from d06av23.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D7D44A404D; Wed, 22 Sep 2021 03:11:33 +0000 (GMT) Received: from ceres.ibmuc.com (unknown [9.65.202.213]) by d06av23.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 22 Sep 2021 03:11:33 +0000 (GMT) From: Eric Richter To: skiboot@lists.ozlabs.org Date: Tue, 21 Sep 2021 22:11:24 -0500 Message-Id: <20210922031129.4188386-4-erichte@linux.ibm.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210922031129.4188386-1-erichte@linux.ibm.com> References: <20210922031129.4188386-1-erichte@linux.ibm.com> MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: XxPRrX8-vrfKzW_31vb-kvQ-yLxhhnfT X-Proofpoint-ORIG-GUID: XxPRrX8-vrfKzW_31vb-kvQ-yLxhhnfT X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.391,FMLib:17.0.607.475 definitions=2021-09-22_01,2021-09-20_01,2020-04-07_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 adultscore=0 spamscore=0 suspectscore=0 phishscore=0 bulkscore=0 impostorscore=0 priorityscore=1501 mlxscore=0 mlxlogscore=999 clxscore=1015 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109200000 definitions=main-2109220019 Subject: [Skiboot] [RFC 3/8] secvar/drivers: add mode-switchable storage driver for secboot_tpm X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" 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 --- 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 +#include 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 +#include +#include +#include "../secvar.h" +#include "../secvar_devtree.h" +#include "secboot_tpm.h" +#include +#include +#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 + +#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