[Skiboot] [PATCH v2 06/12] secvar_tpmnv: add high-level tpm nv index abstraction for secvar
Stefan Berger
stefanb at linux.ibm.com
Fri Jan 24 01:39:30 AEDT 2020
On 1/19/20 9:36 PM, Eric Richter wrote:
> Multiple components, like the storage driver or backend driver, may need
> to store information in the one TPM NV index assigned for secure boot.
> This abstraction provides a method for these components to share the
> index space without stomping on each other's data, and without them
> needing to understand anything about the other.
>
> This is probably an overengineered solution to the problem, but the
> intent is to keep the drivers as independent from one another as possible.
>
> NOTE: This version of the patch now includes the ability to switch between
> a "Fake TPMNV" mode simulated using PNOR, and use of an actual TPM. The
> simulated mode exists for unit test and review purposes on machines without
> a TPM. It is not intended for production use.
>
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
> libstb/secvar/Makefile.inc | 3 +-
> libstb/secvar/secvar_tpmnv.c | 265 +++++++++++++++++++++++++++++++++++
> libstb/secvar/secvar_tpmnv.h | 16 +++
> 3 files changed, 282 insertions(+), 2 deletions(-)
> create mode 100644 libstb/secvar/secvar_tpmnv.c
> create mode 100644 libstb/secvar/secvar_tpmnv.h
>
> diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
> index f4b196d9..1d68941e 100644
> --- a/libstb/secvar/Makefile.inc
> +++ b/libstb/secvar/Makefile.inc
> @@ -8,8 +8,7 @@ SUBDIRS += $(SECVAR_DIR)
> include $(SECVAR_DIR)/storage/Makefile.inc
> include $(SECVAR_DIR)/backend/Makefile.inc
>
> -SECVAR_SRCS = secvar_main.c secvar_util.c secvar_devtree.c
> -SECVAR_SRCS = secvar_main.c secvar_util.c secvar_devtree.c secvar_api.c
> +SECVAR_SRCS = secvar_main.c secvar_util.c secvar_devtree.c secvar_api.c secvar_tpmnv.c
> SECVAR_OBJS = $(SECVAR_SRCS:%.c=%.o)
> SECVAR = $(SECVAR_DIR)/built-in.a
>
> diff --git a/libstb/secvar/secvar_tpmnv.c b/libstb/secvar/secvar_tpmnv.c
> new file mode 100644
> index 00000000..2944ece4
> --- /dev/null
> +++ b/libstb/secvar/secvar_tpmnv.c
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2019 IBM Corp. */
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "SECVAR_TPMNV: " fmt
> +#endif
> +
> +#include <opal.h>
> +#include <skiboot.h>
> +#include <string.h>
> +#include <tssskiboot.h>
> +#include "secvar_tpmnv.h"
> +
> +#define TPM_SECVAR_NV_INDEX 0x01c10191
> +#define TPM_SECVAR_MAGIC_NUM 0x53544e56
> +
> +struct tpm_nv_id {
> + uint32_t id;
> + uint32_t size;
> + char data[0];
> +} __packed;
> +
> +struct tpm_nv {
> + uint32_t magic_num;
> + uint32_t version;
> + struct tpm_nv_id vars[0];
> +} __packed;
> +
> +int tpm_ready = 0;
> +int tpm_error = 0;
> +int tpm_first_init = 0;
> +struct tpm_nv *tpm_image;
> +size_t tpm_nv_size = 0;
> +
> +// Values set by a platform to enable TPMNV simulation mode
> +// NOT INTENDED FOR PRODUCTION USE
> +int tpm_fake_nv = 0; // Use fake NV mode using pnor
> +uint64_t tpm_fake_nv_offset = 0; // Offset into SECBOOT pnor to use
> +uint64_t tpm_fake_nv_max_size = 0;
> +
> +static int TSS_Fake_Read(uint32_t nvIndex, void *buf, size_t bufsize, uint64_t off)
> +{
> + (void) nvIndex;
> + (void) off;
Same comment as for other patch: use __attribute__((unused))
> + return platform.secboot_read(buf, tpm_fake_nv_offset, bufsize);
> +}
> +
> +static int TSS_Fake_Write(uint32_t nvIndex, void *buf, size_t bufsize, uint64_t off)
> +{
> + (void) nvIndex;
> + (void) off;
> + return platform.secboot_write(tpm_fake_nv_offset, buf, bufsize);
> +}
> +
> +static int TSS_Fake_Define_Space(uint32_t nvIndex, const char hierarchy,
> + const char hierarchy_authorization,
> + uint16_t dataSize)
> +{
> + (void) nvIndex;
> + (void) hierarchy;
> + (void) hierarchy_authorization;
> + (void) dataSize;
> + return 0;
> +}
> +
> +struct tpmnv_ops_s {
> + int (*tss_nv_read)(uint32_t, void*, size_t, uint64_t);
> + int (*tss_nv_write)(uint32_t, void*, size_t, uint64_t);
> + int (*tss_nv_define_space)(uint32_t, const char, const char, uint16_t);
> +};
> +
> +struct tpmnv_ops_s TSS_tpmnv_ops = {
> + .tss_nv_read = TSS_NV_Read,
> + .tss_nv_write = TSS_NV_Write,
> + .tss_nv_define_space = TSS_NV_Define_Space,
> +};
> +
> +struct tpmnv_ops_s Fake_tpmnv_ops = {
> + .tss_nv_read = TSS_Fake_Read,
> + .tss_nv_write = TSS_Fake_Write,
> + .tss_nv_define_space = TSS_Fake_Define_Space,
> +};
> +
> +struct tpmnv_ops_s *tpmnv_ops = &TSS_tpmnv_ops;
> +
> +// This function should be replaced with logic that performs the initial
> +// TPM NV Index definition, and any first-write logic
> +static int secvar_tpmnv_format(void)
> +{
> + int rc;
> +
> + memset(tpm_image, 0, sizeof(tpm_nv_size));
+size_t tpm_nv_size = 0;
sizeof() doesn't seem right...
> +
> + // TODO: Determine the proper auth
right. I doubt it will always work with physical presence protection.
What all environments do you intend to run this in? Will SLOF be there
before you run this ?
> + rc = tpmnv_ops->tss_nv_define_space(TPM_SECVAR_NV_INDEX, 'p', 'p', tpm_nv_size);
> + if (rc) {
> + prlog(PR_INFO, "Failed to define NV index, rc = %d\n", rc);
> + return rc;
> + }
> +
> + tpm_image->magic_num = TPM_SECVAR_MAGIC_NUM;
> + tpm_image->version = 1;
> +
> + tpm_first_init = 1;
> +
> + return tpmnv_ops->tss_nv_write(TPM_SECVAR_NV_INDEX, tpm_image, tpm_nv_size, 0);
> +}
> +
> +
> +static int secvar_tpmnv_init(void)
> +{
> + int rc;
> +
> + if (tpm_ready)
> + return OPAL_SUCCESS;
> + if (tpm_error)
> + return OPAL_HARDWARE;
> +
> + prlog(PR_INFO, "Initializing TPMNV space...\n");
> +
> + // Check here if TPM NV Index is defined
> + // if not, call secvar_tpmnv_format() here
> +
> + // Using the minimum defined by the spec for now
> + // This value should probably be determined by tss_get_capatibility
> + tpm_nv_size = 1024;
> +
> + tpm_image = malloc(tpm_nv_size);
> + if (!tpm_image) {
> + tpm_error = 1;
> + return OPAL_NO_MEM;
> + }
> +
> + if (tpm_fake_nv) {
> + prlog(PR_INFO, "Enabling fake TPM NV mode\n");
> + tpmnv_ops = &Fake_tpmnv_ops;
> + }
> +
> + prlog(PR_INFO, "Reading in from TPM NV...\n");
> + rc = tpmnv_ops->tss_nv_read(TPM_SECVAR_NV_INDEX, tpm_image, tpm_nv_size, 0);
> + if (rc) {
> + prlog(PR_INFO, "Failed to read from NV index, rc = %d\n", rc);
> + tpm_error = 1;
> + return OPAL_HARDWARE;
> + }
> +
Hm, will it ever get here in case TPM_SECVAR_NV_INDEX doesn't exist yet
and you need to 'format'?
> + if (tpm_image->magic_num != TPM_SECVAR_MAGIC_NUM) {
> + prlog(PR_INFO, "Magic num mismatch, reformatting NV space...\n");
> + rc = secvar_tpmnv_format();
> + if (rc) {
> + prlog(PR_INFO, "Failed to format tpmnv space, rc = %d\n", rc);
> + tpm_error = 1;
> + return OPAL_HARDWARE;
> + }
> + }
> + prlog(PR_INFO, "TPMNV space initialized successfully\n");
> + tpm_ready = 1;
> +
> + return OPAL_SUCCESS;
> +}
> +
> +
> +static struct tpm_nv_id *find_tpmnv_id(uint32_t id)
A bit description for this function would help what this id is that you
are searching for.
> +{
> + struct tpm_nv_id *tmp;
> + char *cur, *end;
> +
> + cur = (char *) tpm_image->vars;
> + end = ((char *) tpm_image) + tpm_nv_size;
> + while (cur < end) {
> + tmp = (struct tpm_nv_id *) cur;
> + if (tmp->id == 0)
> + return NULL;
> + if (tmp->id == id)
> + return tmp;
> + cur += sizeof(struct tpm_nv_id) + tmp->size;
whitespace error.
Do you know that cur will be fitting into the rest of the space?
> + }
> +
> + return NULL;
> +}
> +
> +
> +// "Allocate" space within the secvar tpm
> +int secvar_tpmnv_alloc(uint32_t id, int32_t size)
> +{
> + struct tpm_nv_id *tmp;
> + char *cur;
> + char *end;
> +
> + if (secvar_tpmnv_init())
> + return OPAL_RESOURCE;
> +
> + cur = (char *) tpm_image->vars;
> + end = ((char *) tpm_image) + tpm_nv_size;
> + while (cur < end) {
> + tmp = (struct tpm_nv_id *) cur;
> + if (tmp->id == 0)
> + goto allocate;
> + if (tmp->id == id)
> + return OPAL_SUCCESS; // Already allocated
> +
> + cur += sizeof(struct tpm_nv_id) + tmp->size;
> + }
> + // We ran out of space...
> + return OPAL_EMPTY;
> +
> +allocate:
> + tmp->id = id;
> +
> + // Special case: size of -1 gives remaining space
> + if (size == -1)
> + tmp->size = end - tmp->data;
> + else
> + tmp->size = size;
Are you sure you have size bytes left?
> +
> + return OPAL_SUCCESS;
> +}
> +
> +
> +int secvar_tpmnv_read(uint32_t id, void *buf, size_t size, size_t off)
> +{
> + struct tpm_nv_id *var;
> +
> + if (secvar_tpmnv_init())
> + return OPAL_RESOURCE;
There should be an 'earlier' location where you can call the init function.
> +
> + var = find_tpmnv_id(id);
> + if (!var)
> + return OPAL_EMPTY;
> +
> + size = MIN(size, var->size);
You should check whether off > var->size and then also do MIN(size,
var->size - off).
> + memcpy(buf, var->data + off, size);
> +
> + return 0;
> +}
> +
> +
> +int secvar_tpmnv_write(uint32_t id, void *buf, size_t size, size_t off)
> +{
> + struct tpm_nv_id *var;
> +
> + if (secvar_tpmnv_init())
> + return OPAL_RESOURCE;
Same comment as above.
> +
> + var = find_tpmnv_id(id);
> + if (!var)
> + return OPAL_EMPTY;
> +
> + size = MIN(size, var->size);
similar comment as above but off > size check and MIN(size - off,
var->size).
> + memcpy(var->data, buf + off, size);
size - off ?
> +
> + return tpmnv_ops->tss_nv_write(TPM_SECVAR_NV_INDEX, tpm_image, tpm_nv_size, 0);
> +}
> +
> +int secvar_tpmnv_size(uint32_t id)
> +{
> + struct tpm_nv_id *var;
> +
> + if (secvar_tpmnv_init())
> + return OPAL_RESOURCE;
> +
same comment
> + var = find_tpmnv_id(id);
> + if (!var)
> + return 0;
> + return var->size;
+struct tpm_nv_id {
+ uint32_t id;
+ uint32_t size;
+ char data[0];
+} __packed;
function should return uint32_t ?
> +}
> diff --git a/libstb/secvar/secvar_tpmnv.h b/libstb/secvar/secvar_tpmnv.h
> new file mode 100644
> index 00000000..697a52c2
> --- /dev/null
> +++ b/libstb/secvar/secvar_tpmnv.h
> @@ -0,0 +1,16 @@
> +#ifndef _SECVAR_TPMNV_H_
> +#define _SECVAR_TPMNV_H_
> +#include <stdint.h>
License header needed ?
> +
> +extern int tpm_first_init;
> +
> +int secvar_tpmnv_alloc(uint32_t id, int32_t size);
> +int secvar_tpmnv_read(uint32_t id, void *buf, size_t size, size_t off);
> +int secvar_tpmnv_write(uint32_t id, void *buf, size_t size, size_t off);
> +int secvar_tpmnv_size(uint32_t id);
> +
> +extern int tpm_fake_nv;
> +extern uint64_t tpm_fake_nv_offset;
> +
> +#endif
> +
More information about the Skiboot
mailing list