diff mbox series

[v4,01/11] libstb/secvar: add secure variable internal abstraction

Message ID 20191026094553.26635-2-erichte@linux.ibm.com
State Superseded
Headers show
Series Add Secure Variable Support | expand

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (d75e82dbfbb9443efeb3f9a5921ac23605aab469)

Commit Message

Eric Richter Oct. 26, 2019, 9:45 a.m. UTC
This patch implements a platform-independent abstraction for storing and
retrieving secure variables, as required for host OS secure boot. This
serves as the main entry point for initializing the in-memory cache of the
secure variables, which also kicks off any platform-specific logic that may
be needed. This patch also provides core functions for the subsequent
patches in this series to utilize.

The base secure variable implementation makes use of two types of
drivers, to be selected by the platform: "storage" drivers, and
"backend" drivers. The storage driver implements the hooks required to
write the secure variables to some form of non-volatile memory, and load
the variables on boot. The backend driver defines how the variables
should be interpreted, and processed.

Secure variables are stored in two types of banks, the "variable" bank
and the "update" bank. Variables that have been validated and processed
are stored in the variable bank. This bank is effectively read-only
after the base secvar initialization. Any proposed variable updates are
instead stored in the update bank. During secvar initialization, the
backend driver processes variables from the update bank, and if valid,
adds the new variable to the variable bank.

V2:
 - added secvar device tree node as child of ibm,secureboot
 - added version and compatible properties to backend driver struct
 - added secvar_ready flag for the API to detect if secvar
    initialized successfully
 - moved pre-process step to after initial variable load
 - moved flags field from secvar struct to secvar node

V3:
 - remove the metadata secvar field
 - add probe_secvar() to bump compatible flag
 - add device tree property for backend-agnostic secure mode setting
 - remove backend minor version field
 - remove static data allocation in secvar struct

V4:
 - add alloc_secvar helpers
 - removed ibm,secureboot version bump to v3
 - secvars now store their allocated size seperate from the
   data size (to permit overallocating)
 - split device tree functions into their own file
 - device tree changes:
   - secvar now a child of ibm,opal
   - compatible is "ibm,secvar-v1", backend creates its own node
   - secure-mode is now a boolean os-secure-enforcing property
   - storage and backends now have their own nodes

Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
 core/init.c                        |   4 +
 include/platform.h                 |   2 +
 include/secvar.h                   |  43 +++++++++
 libstb/Makefile.inc                |   3 +-
 libstb/secvar/Makefile.inc         |  14 +++
 libstb/secvar/backend/Makefile.inc |  11 +++
 libstb/secvar/secvar.h             |  60 +++++++++++++
 libstb/secvar/secvar_devtree.c     | 136 +++++++++++++++++++++++++++++
 libstb/secvar/secvar_devtree.h     |  15 ++++
 libstb/secvar/secvar_main.c        |  89 +++++++++++++++++++
 libstb/secvar/secvar_util.c        | 106 ++++++++++++++++++++++
 libstb/secvar/storage/Makefile.inc |  11 +++
 12 files changed, 493 insertions(+), 1 deletion(-)
 create mode 100644 include/secvar.h
 create mode 100644 libstb/secvar/Makefile.inc
 create mode 100644 libstb/secvar/backend/Makefile.inc
 create mode 100644 libstb/secvar/secvar.h
 create mode 100644 libstb/secvar/secvar_devtree.c
 create mode 100644 libstb/secvar/secvar_devtree.h
 create mode 100644 libstb/secvar/secvar_main.c
 create mode 100644 libstb/secvar/secvar_util.c
 create mode 100644 libstb/secvar/storage/Makefile.inc

Comments

Oliver O'Halloran Oct. 29, 2019, 2:33 p.m. UTC | #1
On Sat, 2019-10-26 at 04:45 -0500, Eric Richter wrote:
> This patch implements a platform-independent abstraction for storing and
> retrieving secure variables, as required for host OS secure boot. This
> serves as the main entry point for initializing the in-memory cache of the
> secure variables, which also kicks off any platform-specific logic that may
> be needed. This patch also provides core functions for the subsequent
> patches in this series to utilize.
> 
> The base secure variable implementation makes use of two types of
> drivers, to be selected by the platform: "storage" drivers, and
> "backend" drivers. The storage driver implements the hooks required to
> write the secure variables to some form of non-volatile memory, and load
> the variables on boot. The backend driver defines how the variables
> should be interpreted, and processed.
> 
> Secure variables are stored in two types of banks, the "variable" bank
> and the "update" bank. Variables that have been validated and processed
> are stored in the variable bank. This bank is effectively read-only
> after the base secvar initialization. Any proposed variable updates are
> instead stored in the update bank. During secvar initialization, the
> backend driver processes variables from the update bank, and if valid,
> adds the new variable to the variable bank.
> 
> V2:
>  - added secvar device tree node as child of ibm,secureboot
>  - added version and compatible properties to backend driver struct
>  - added secvar_ready flag for the API to detect if secvar
>     initialized successfully
>  - moved pre-process step to after initial variable load
>  - moved flags field from secvar struct to secvar node
> 
> V3:
>  - remove the metadata secvar field
>  - add probe_secvar() to bump compatible flag
>  - add device tree property for backend-agnostic secure mode setting
>  - remove backend minor version field
>  - remove static data allocation in secvar struct
> 
> V4:
>  - add alloc_secvar helpers
>  - removed ibm,secureboot version bump to v3
>  - secvars now store their allocated size seperate from the
>    data size (to permit overallocating)
>  - split device tree functions into their own file
>  - device tree changes:
>    - secvar now a child of ibm,opal
>    - compatible is "ibm,secvar-v1", backend creates its own node
>    - secure-mode is now a boolean os-secure-enforcing property
>    - storage and backends now have their own nodes
> 
> Signed-off-by: Eric Richter <erichte@linux.ibm.com>
> ---
>  core/init.c                        |   4 +
>  include/platform.h                 |   2 +
>  include/secvar.h                   |  43 +++++++++
>  libstb/Makefile.inc                |   3 +-
>  libstb/secvar/Makefile.inc         |  14 +++
>  libstb/secvar/backend/Makefile.inc |  11 +++
>  libstb/secvar/secvar.h             |  60 +++++++++++++
>  libstb/secvar/secvar_devtree.c     | 136 +++++++++++++++++++++++++++++
>  libstb/secvar/secvar_devtree.h     |  15 ++++
>  libstb/secvar/secvar_main.c        |  89 +++++++++++++++++++
>  libstb/secvar/secvar_util.c        | 106 ++++++++++++++++++++++
>  libstb/secvar/storage/Makefile.inc |  11 +++
>  12 files changed, 493 insertions(+), 1 deletion(-)
>  create mode 100644 include/secvar.h
>  create mode 100644 libstb/secvar/Makefile.inc
>  create mode 100644 libstb/secvar/backend/Makefile.inc
>  create mode 100644 libstb/secvar/secvar.h
>  create mode 100644 libstb/secvar/secvar_devtree.c
>  create mode 100644 libstb/secvar/secvar_devtree.h
>  create mode 100644 libstb/secvar/secvar_main.c
>  create mode 100644 libstb/secvar/secvar_util.c
>  create mode 100644 libstb/secvar/storage/Makefile.inc
> 
> diff --git a/core/init.c b/core/init.c
> index cc1fdbc4..e6efb600 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -1211,6 +1211,10 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
>  	secureboot_init();
>  	trustedboot_init();
>  
> +	/* Secure variables init, handled by platform */
> +	if (platform.secvar_init)
> +		platform.secvar_init();
> +
>  	/*
>  	 * BMC platforms load version information from flash after
>  	 * secure/trustedboot init.
> diff --git a/include/platform.h b/include/platform.h
> index 0b043856..412f8fc8 100644
> --- a/include/platform.h
> +++ b/include/platform.h
> @@ -210,6 +210,8 @@ struct platform {
>  					    uint32_t len);
>  	int		(*nvram_write)(uint32_t dst, void *src, uint32_t len);
>  
> +	int (*secvar_init)(void);
> +
>  	/*
>  	 * OCC timeout. This return how long we should wait for the OCC
>  	 * before timing out. This lets us use a high value on larger FSP
> diff --git a/include/secvar.h b/include/secvar.h
> new file mode 100644
> index 00000000..ae262616
> --- /dev/null
> +++ b/include/secvar.h
> @@ -0,0 +1,43 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */

Can you make this an SPDX tag?


> +
> +#ifndef _SECVAR_DRIVER_
> +#define _SECVAR_DRIVER_
> +
> +#include <stdint.h>
> +
> +struct secvar;
> +
> +struct secvar_storage_driver {
> +        int (*load_bank)(struct list_head *bank, int section);
> +        int (*write_bank)(struct list_head *bank, int section);
> +        int (*store_init)(void);
> +	char compatible[32];

Why isn't this a pointer to a const string?
> +	uint64_t max_var_size;
> +};
> +
> +struct secvar_backend_driver {
> +        int (*pre_process)(void);               // Perform any pre-processing stuff (e.g. determine secure boot state)
> +        int (*process)(void);                   // Process all updates
> +        int (*post_process)(void);              // Perform any post-processing stuff (e.g. derive/update variables)
> +        int (*validate)(struct secvar *var);    // Validate a single variable, return boolean
> +        char compatible[32];			// String to use for compatible in secvar node
Same quesiton
> +};
> +
> +
> +int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
> +
> +#endif
> diff --git a/libstb/Makefile.inc b/libstb/Makefile.inc
> index 6d54c5cd..d3f68496 100644
> --- a/libstb/Makefile.inc
> +++ b/libstb/Makefile.inc
> @@ -8,11 +8,12 @@ LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c
>  LIBSTB_OBJS = $(LIBSTB_SRCS:%.c=%.o)
>  LIBSTB = $(LIBSTB_DIR)/built-in.a
>  
> +include $(SRC)/$(LIBSTB_DIR)/secvar/Makefile.inc
>  include $(SRC)/$(LIBSTB_DIR)/mbedtls/Makefile.inc
>  include $(SRC)/$(LIBSTB_DIR)/drivers/Makefile.inc
>  include $(SRC)/$(LIBSTB_DIR)/tss/Makefile.inc
>  
> -$(LIBSTB): $(LIBSTB_OBJS:%=$(LIBSTB_DIR)/%) $(DRIVERS) $(TSS) $(MBEDTLS)
> +$(LIBSTB): $(LIBSTB_OBJS:%=$(LIBSTB_DIR)/%) $(DRIVERS) $(TSS) $(SECVAR) $(MBEDTLS)
>  
>  libstb/create-container: libstb/create-container.c libstb/container-utils.c
>  	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) \
> diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
> new file mode 100644
> index 00000000..e1e6e5c7
> --- /dev/null
> +++ b/libstb/secvar/Makefile.inc
> @@ -0,0 +1,14 @@
> +# -*-Makefile-*-
> +
> +SECVAR_DIR = libstb/secvar
> +
> +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_OBJS = $(SECVAR_SRCS:%.c=%.o)
> +SECVAR = $(SECVAR_DIR)/built-in.a
> +
> +$(SECVAR): $(SECVAR_OBJS:%=$(SECVAR_DIR)/%) $(SECVAR_STORAGE) $(SECVAR_BACKEND)
> diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
> new file mode 100644
> index 00000000..7a7ca1f7
> --- /dev/null
> +++ b/libstb/secvar/backend/Makefile.inc
> @@ -0,0 +1,11 @@
> +# -*-Makefile-*-
> +
> +SECVAR_BACKEND_DIR = libstb/secvar/backend
> +
> +SUBDIRS += $(SECVAR_BACKEND_DIR)
> +
> +SECVAR_BACKEND_SRCS =
> +SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
> +SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
> +
> +$(SECVAR_BACKEND): $(SECVAR_BACKEND_OBJS:%=$(SECVAR_BACKEND_DIR)/%)
> diff --git a/libstb/secvar/secvar.h b/libstb/secvar/secvar.h
> new file mode 100644
> index 00000000..cddd6d45
> --- /dev/null
> +++ b/libstb/secvar/secvar.h
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2013-2019 IBM Corp. */
> +
> +#ifndef _SECVAR_H_
> +#define _SECVAR_H_
> +
> +#include <ccan/list/list.h>
> +#include <stdint.h>
> +#include <secvar.h>
> +
> 

> +#define SECVAR_MAX_KEY_LEN		1024

Why 1K? The max key length should probably go into the DT too since you
need to know it to use the API correctly.

> +
> +enum {
> +	SECVAR_VARIABLE_BANK,
> +	SECVAR_UPDATE_BANK,
> +};
> +
> +

> +struct secvar_node {
> +	struct list_node link;
> +	struct secvar *var;
> +	uint64_t flags;		// Flag for how *var should be stored
> +	uint64_t size;		// How much space was allocated for data
> +};
> +
> +#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
> +
> +struct secvar {
> +	uint64_t key_len;
> +	uint64_t data_size;
> +	char key[SECVAR_MAX_KEY_LEN];
> +	char data[0];
> +};

secvar_node is nothing but a list_node, some flags, and a pointer to
the actual secvar data structure so why have them as seperate
structures? It makes no sense to me and extra indirection adds noise
when allocating, freeing and accessing the secvar.

> +
> +
> +enum {
> +	BACKEND_NONE = 0,
> +	BACKEND_TC_COMPAT_V1,
> +};

what is TC compat?

> +
> +extern struct list_head variable_bank;
> +extern struct list_head update_bank;
> +extern int secvar_enabled;
> +extern int secvar_ready;
> +extern struct secvar_storage_driver secvar_storage;
> +extern struct secvar_backend_driver secvar_backend;
> +
> +// Check for secvar support, update secureboot DT compatible if so
> +int probe_secvar(void);
> +
> +// Helper functions
> +void clear_bank_list(struct list_head *bank);
> +struct secvar_node *alloc_secvar(uint64_t size);
> +int realloc_secvar(struct secvar_node *node, uint64_t size);
> +struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank);
> +int is_key_empty(const char *key, uint64_t key_len);
> +int list_length(struct list_head *bank);
> +
> +#endif
> diff --git a/libstb/secvar/secvar_devtree.c b/libstb/secvar/secvar_devtree.c
> new file mode 100644
> index 00000000..9cd12a15
> --- /dev/null
> +++ b/libstb/secvar/secvar_devtree.c
> @@ -0,0 +1,136 @@
> +#include <device.h>
> +#include <string.h>
> +#include "secvar.h"
> +#include "secvar_devtree.h"
> +
> +struct dt_node *secvar_node;
> +struct dt_node *secvar_backend_node;
> +struct dt_node *secvar_storage_node;
> +

> +int secvar_set_secure_mode(void)
> +{
> +	struct dt_property *prop;
> +
> +	if (!secvar_node)
> +		return -1;
> +
> +	prop = (struct dt_property *) dt_find_property(secvar_node, "os-secure-enforcing");
> +	if (prop)
> +		return 0;
> +
> +	prop = dt_add_property(secvar_node, "os-secure-enforcing", 0, 0);
> +	if (!prop)
> +		return -2;
> +
> +	return 0;
> +}

Why would this ever be called twice?

> +
> +void secvar_init_devnode(void)
> +{
> +	struct dt_node *sb_root;
> +
> +	sb_root = dt_find_by_path(dt_root, "/ibm,opal/");
> +
> +	secvar_node = dt_new(sb_root, "secvar");
> +
> +	dt_add_property_string(secvar_node, "compatible", "ibm,secvar-v1");
> +
> +	secvar_backend_node = dt_new(secvar_node, "backend");
> +	secvar_storage_node = dt_new(secvar_node, "storage");
> +
> +	dt_add_property_string(secvar_backend_node, "compatible", secvar_backend.compatible);
> +	dt_add_property_string(secvar_storage_node, "compatible", secvar_storage.compatible);
> +
> +	dt_add_property_u64(secvar_storage_node, "max-var-size", secvar_storage.max_var_size);
> +}
> +

> +void secvar_set_status(const char *status)
> +{
> +	struct dt_property *stat_prop;
> +	if (!secvar_node)
> +		return; // Fail boot?
> +
> +	stat_prop = (struct dt_property *) dt_find_property(secvar_node, "status");

Why are you casting away const? The fact dt_find_property() returns a
const pointer should be a giant red flag that you're doing something
sketchy here.

In general you *should not* be modifying the DT at runtime, especially
when it's the part of the DT that skiboot constructs. We do modify the
DT in a few places to work around bugs in the input DT (if we have
one), or in bugs from HDAT, but as a rule: don't. Initialise your
hardware, API, whatever, then build the DT based on how the init
process went. Don't build the DT then hack it later on if something
fails.

If you *absolutely* have to, you should use dt_check_del_prop() to
remove an existing property and then re-add it using the correct
function.

> +
> +	if (stat_prop)
> +		strcpy(stat_prop->prop, status);
> +	else
> +		dt_add_property_string(secvar_node, "status", status);
> +		// Fail boot if not successful?
> +}

secvar_set_status("nice");
secvar_set_status("heap overflow");


> +void secvar_set_update_status(uint64_t val)
> +{
> +	struct dt_property *stat_prop;
> +	if (!secvar_node)
> +		return; // Fail boot?
> +
> +	stat_prop = (struct dt_property *) dt_find_property(secvar_backend_node, "update-status");
> +
> +	if (stat_prop)
> +		memcpy(stat_prop->prop, &val, sizeof(val));
> +	else
> +		dt_add_property(secvar_backend_node, "update-status", &val, sizeof(val));
> +}

same comments

> +void secvar_dt_backend_set_prop_string(const char *prop, const char *val)
> +{
> +	struct dt_property *p;
> +
> +	if (!secvar_backend_node)
> +		return;
> +
> +	p = (struct dt_property *) dt_find_property(secvar_backend_node, prop);
> +
> +	if (p)
> +		memcpy(p->prop, val, strlen(val));
> +	else
> +		dt_add_property_string(secvar_backend_node, prop, val);
> +}
> +
> +void secvar_dt_backend_set_prop_u64(const char *prop, uint64_t val)
> +{
> +	struct dt_property *p;
> +
> +	if (!secvar_backend_node)
> +		return;
> +
> +	p = (struct dt_property *) dt_find_property(secvar_backend_node, prop);
> +
> +	if (p)
> +		memcpy(p->prop, &val, sizeof(val));
> +	else
> +		dt_add_property_u64(secvar_backend_node, prop, val);
> +}
> +
> +void secvar_dt_storage_set_prop_string(const char *prop, const char *val)
> +{
> +	struct dt_property *p;
> +
> +	if (!secvar_storage_node)
> +		return;
> +
> +	p = (struct dt_property *) dt_find_property(secvar_storage_node, prop);
> +
> +	if (p)
> +		memcpy(p->prop, val, strlen(val));
> +	else
> +		dt_add_property_string(secvar_backend_node, prop, val);
> +}
> +
> +void secvar_dt_storage_set_prop_u64(const char *prop, uint64_t val)
> +{
> +	struct dt_property *p;
> +
> +	if (!secvar_storage_node)
> +		return;
> +
> +	p = (struct dt_property *) dt_find_property(secvar_storage_node, prop);
> +
> +	if (p)
> +		memcpy(p->prop, &val, sizeof(val));
> +	else
> +		dt_add_property_u64(secvar_backend_node, prop, val);
> +}

Same comments as above, also don't add code that has no users. If
you're going to need it later on then it can wait until the code that
uses it goes in.

> diff --git a/libstb/secvar/secvar_devtree.h b/libstb/secvar/secvar_devtree.h
> new file mode 100644
> index 00000000..8ba516f0
> --- /dev/null
> +++ b/libstb/secvar/secvar_devtree.h
> @@ -0,0 +1,15 @@
> +#ifndef _SECVAR_DEVTREE_H_
> +#define _SECVAR_DEVTREE_H_
> +
> +int secvar_set_secure_mode(void);
> +void secvar_init_devnode(void);
> +
> +void secvar_set_status(const char *status);
> +void secvar_set_update_status(uint64_t val);
> +
> +void secvar_dt_backend_set_prop_string(const char *prop, const char *val);
> +void secvar_dt_backend_set_prop_u64(const char *prop, uint64_t val);
> +void secvar_dt_storage_set_prop_string(const char *prop, const char *val);
> +void secvar_dt_storage_set_prop_u64(const char *prop, uint64_t val);
> +
> +#endif
> diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c
> new file mode 100644
> index 00000000..e40d10bb
> --- /dev/null
> +++ b/libstb/secvar/secvar_main.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2019 IBM Corp. */
> +
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "SECVAR: " fmt
> +#endif
> +
> +#include <stdlib.h>
> +#include <skiboot.h>
> +#include <opal.h>
> +#include "secvar.h"
> +#include "secvar_devtree.h"
> +
> +struct list_head variable_bank;
> +struct list_head update_bank;
> +
> +int secvar_enabled = 0;	// Set to 1 if secvar is supported
> +int secvar_ready = 0;	// Set to 1 when base secvar inits correctly
> +
> +// To be filled in by platform.secvar_init
> +struct secvar_storage_driver secvar_storage = {0};
> +struct secvar_backend_driver secvar_backend = {0};
> +
> +
> +int secvar_main(struct secvar_storage_driver storage_driver,
> +               struct secvar_backend_driver backend_driver)
> +{
> +	int rc = OPAL_UNSUPPORTED;
> +
> +	secvar_storage = storage_driver;
> +	secvar_backend = backend_driver;
> +
> +	secvar_init_devnode();
> +
> +	secvar_enabled = 1;
> +
> +	list_head_init(&variable_bank);
> +	list_head_init(&update_bank);
> +
> +	rc = secvar_storage.store_init();
> +	if (rc)
> +		goto fail;
> +
> +
> +	rc = secvar_storage.load_bank(&variable_bank, SECVAR_VARIABLE_BANK);
> +	if (rc)
> +		goto fail;
> +
> +	rc = secvar_storage.load_bank(&update_bank, SECVAR_UPDATE_BANK);
> +	if (rc)
> +		goto fail;
> +
> +	// At this point, base secvar is functional. Rest is up to the backend
> +	secvar_ready = 1;
> +
> +	if (secvar_backend.pre_process)
> +		rc = secvar_backend.pre_process();
> +
> +	// Process is required, error if it doesn't exist
> +	if (!secvar_backend.process)
> +		goto out;
> +
> +	rc = secvar_backend.process();
> +		secvar_set_update_status(rc);
> +	if (rc == OPAL_SUCCESS) {
> +		rc = secvar_storage.write_bank(&variable_bank, SECVAR_VARIABLE_BANK);
> +		if (rc)
> +			goto out;
> +
> +		rc = secvar_storage.write_bank(&update_bank, SECVAR_UPDATE_BANK);
> +		if (rc)
> +			goto out;
> +	}
> +
> +	// Last point of possible base secvar failure
> +	secvar_set_status("okay");
> +
> +	if (secvar_backend.post_process)
> +		rc = secvar_backend.post_process();
> +	if (rc)
> +		goto out;
> +
> +	return OPAL_SUCCESS;
> +fail:
> +	secvar_set_status("fail");
> +out:
> +	printf("Secure Variables Status %04x\n", rc);
> +	return rc;
> +}
> diff --git a/libstb/secvar/secvar_util.c b/libstb/secvar/secvar_util.c
> new file mode 100644
> index 00000000..2955a07c
> --- /dev/null
> +++ b/libstb/secvar/secvar_util.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2019 IBM Corp. */
> +
> +#ifndef pr_fmt
> +#define pr_fmt(fmt) "SECVAR: " fmt
> +#endif
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <skiboot.h>
> +#include <opal.h>
> +#include "secvar.h"
> +
> +void clear_bank_list(struct list_head *bank)
> +{
> +	struct secvar_node *node, *next;
> +
> +	if (!bank)
> +		return;
> +
> +	list_for_each_safe(bank, node, next, link) {
> +		list_del(&node->link);
> +
> +		if (node->var) {
> +			free(node->var);
> +		}
> +
> +		free(node);
> +	}
> +}
> +
> +struct secvar_node *alloc_secvar(uint64_t size)
> +{
> +	struct secvar_node *ret;
> +
> +	ret = zalloc(sizeof(struct secvar_node));
> +	if (!ret)
> +		return NULL;
> +
> +	ret->var = zalloc(sizeof(struct secvar) + size);
> +	if (!ret->var) {
> +		free(ret);
> +		return NULL;
> +	}
> +
> +	ret->size = size;
> +
> +	return ret;
> +}
> +
> +int realloc_secvar(struct secvar_node *node, uint64_t size)
> +{
> +	void *tmp;
> +
> +	if (node->size >= size)
> +		return 0;
> +
> +	tmp = zalloc(sizeof(struct secvar) + size);
> +	if (!tmp)
> +		return -1;
> +
> +	memcpy(tmp, node->var, sizeof(struct secvar) + node->size);
> +	free(node->var);
> +	node->var = tmp;
> +
> +	return 0;
> +}
> +
> +struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank)
> +{
> +	struct secvar_node *node = NULL;
> +
> +	list_for_each(bank, node, link) {
> +		// Prevent matching shorter key subsets / bail early
> +		if (key_len != node->var->key_len)
> +			continue;
> +		if (!memcmp(key, node->var->key, key_len)) {
> +			return node;
> +		}
> +	}
> +
> +	return NULL;
> +}

> +int list_length(struct list_head *bank)
> +{
> +	int ret = 0;
> +	struct secvar_node *node;
> +
> +	list_for_each(bank, node, link) {
> +		ret++;
> +	}

Single line blocks should omit the braces. The only exception to this
rule is when there's a multi-arm if statement with a multi-line block. 

e.g:

if (blah) {
	one();
	two();
} else {
	three();
}

> +
> +	return ret;
> +}
> diff --git a/libstb/secvar/storage/Makefile.inc b/libstb/secvar/storage/Makefile.inc
> new file mode 100644
> index 00000000..c107736e
> --- /dev/null
> +++ b/libstb/secvar/storage/Makefile.inc
> @@ -0,0 +1,11 @@
> +# -*-Makefile-*-
> +
> +SECVAR_STORAGE_DIR = libstb/secvar/storage
> +
> +SUBDIRS += $(SECVAR_STORAGE_DIR)
> +
> +SECVAR_STORAGE_SRCS =
> +SECVAR_STORAGE_OBJS = $(SECVAR_STORAGE_SRCS:%.c=%.o)
> +SECVAR_STORAGE = $(SECVAR_STORAGE_DIR)/built-in.a
> +
> +$(SECVAR_STORAGE): $(SECVAR_STORAGE_OBJS:%=$(SECVAR_STORAGE_DIR)/%)
diff mbox series

Patch

diff --git a/core/init.c b/core/init.c
index cc1fdbc4..e6efb600 100644
--- a/core/init.c
+++ b/core/init.c
@@ -1211,6 +1211,10 @@  void __noreturn __nomcount main_cpu_entry(const void *fdt)
 	secureboot_init();
 	trustedboot_init();
 
+	/* Secure variables init, handled by platform */
+	if (platform.secvar_init)
+		platform.secvar_init();
+
 	/*
 	 * BMC platforms load version information from flash after
 	 * secure/trustedboot init.
diff --git a/include/platform.h b/include/platform.h
index 0b043856..412f8fc8 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -210,6 +210,8 @@  struct platform {
 					    uint32_t len);
 	int		(*nvram_write)(uint32_t dst, void *src, uint32_t len);
 
+	int (*secvar_init)(void);
+
 	/*
 	 * OCC timeout. This return how long we should wait for the OCC
 	 * before timing out. This lets us use a high value on larger FSP
diff --git a/include/secvar.h b/include/secvar.h
new file mode 100644
index 00000000..ae262616
--- /dev/null
+++ b/include/secvar.h
@@ -0,0 +1,43 @@ 
+/* Copyright 2019 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef _SECVAR_DRIVER_
+#define _SECVAR_DRIVER_
+
+#include <stdint.h>
+
+struct secvar;
+
+struct secvar_storage_driver {
+        int (*load_bank)(struct list_head *bank, int section);
+        int (*write_bank)(struct list_head *bank, int section);
+        int (*store_init)(void);
+	char compatible[32];
+	uint64_t max_var_size;
+};
+
+struct secvar_backend_driver {
+        int (*pre_process)(void);               // Perform any pre-processing stuff (e.g. determine secure boot state)
+        int (*process)(void);                   // Process all updates
+        int (*post_process)(void);              // Perform any post-processing stuff (e.g. derive/update variables)
+        int (*validate)(struct secvar *var);    // Validate a single variable, return boolean
+        char compatible[32];			// String to use for compatible in secvar node
+};
+
+
+int secvar_main(struct secvar_storage_driver, struct secvar_backend_driver);
+
+#endif
diff --git a/libstb/Makefile.inc b/libstb/Makefile.inc
index 6d54c5cd..d3f68496 100644
--- a/libstb/Makefile.inc
+++ b/libstb/Makefile.inc
@@ -8,11 +8,12 @@  LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c
 LIBSTB_OBJS = $(LIBSTB_SRCS:%.c=%.o)
 LIBSTB = $(LIBSTB_DIR)/built-in.a
 
+include $(SRC)/$(LIBSTB_DIR)/secvar/Makefile.inc
 include $(SRC)/$(LIBSTB_DIR)/mbedtls/Makefile.inc
 include $(SRC)/$(LIBSTB_DIR)/drivers/Makefile.inc
 include $(SRC)/$(LIBSTB_DIR)/tss/Makefile.inc
 
-$(LIBSTB): $(LIBSTB_OBJS:%=$(LIBSTB_DIR)/%) $(DRIVERS) $(TSS) $(MBEDTLS)
+$(LIBSTB): $(LIBSTB_OBJS:%=$(LIBSTB_DIR)/%) $(DRIVERS) $(TSS) $(SECVAR) $(MBEDTLS)
 
 libstb/create-container: libstb/create-container.c libstb/container-utils.c
 	$(call Q, HOSTCC ,$(HOSTCC) $(HOSTCFLAGS) \
diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
new file mode 100644
index 00000000..e1e6e5c7
--- /dev/null
+++ b/libstb/secvar/Makefile.inc
@@ -0,0 +1,14 @@ 
+# -*-Makefile-*-
+
+SECVAR_DIR = libstb/secvar
+
+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_OBJS = $(SECVAR_SRCS:%.c=%.o)
+SECVAR = $(SECVAR_DIR)/built-in.a
+
+$(SECVAR): $(SECVAR_OBJS:%=$(SECVAR_DIR)/%) $(SECVAR_STORAGE) $(SECVAR_BACKEND)
diff --git a/libstb/secvar/backend/Makefile.inc b/libstb/secvar/backend/Makefile.inc
new file mode 100644
index 00000000..7a7ca1f7
--- /dev/null
+++ b/libstb/secvar/backend/Makefile.inc
@@ -0,0 +1,11 @@ 
+# -*-Makefile-*-
+
+SECVAR_BACKEND_DIR = libstb/secvar/backend
+
+SUBDIRS += $(SECVAR_BACKEND_DIR)
+
+SECVAR_BACKEND_SRCS =
+SECVAR_BACKEND_OBJS = $(SECVAR_BACKEND_SRCS:%.c=%.o)
+SECVAR_BACKEND = $(SECVAR_BACKEND_DIR)/built-in.a
+
+$(SECVAR_BACKEND): $(SECVAR_BACKEND_OBJS:%=$(SECVAR_BACKEND_DIR)/%)
diff --git a/libstb/secvar/secvar.h b/libstb/secvar/secvar.h
new file mode 100644
index 00000000..cddd6d45
--- /dev/null
+++ b/libstb/secvar/secvar.h
@@ -0,0 +1,60 @@ 
+// SPDX-License-Identifier: Apache-2.0
+/* Copyright 2013-2019 IBM Corp. */
+
+#ifndef _SECVAR_H_
+#define _SECVAR_H_
+
+#include <ccan/list/list.h>
+#include <stdint.h>
+#include <secvar.h>
+
+#define SECVAR_MAX_KEY_LEN		1024
+
+enum {
+	SECVAR_VARIABLE_BANK,
+	SECVAR_UPDATE_BANK,
+};
+
+
+struct secvar_node {
+	struct list_node link;
+	struct secvar *var;
+	uint64_t flags;		// Flag for how *var should be stored
+	uint64_t size;		// How much space was allocated for data
+};
+
+#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
+
+struct secvar {
+	uint64_t key_len;
+	uint64_t data_size;
+	char key[SECVAR_MAX_KEY_LEN];
+	char data[0];
+};
+
+
+enum {
+	BACKEND_NONE = 0,
+	BACKEND_TC_COMPAT_V1,
+};
+
+extern struct list_head variable_bank;
+extern struct list_head update_bank;
+extern int secvar_enabled;
+extern int secvar_ready;
+extern struct secvar_storage_driver secvar_storage;
+extern struct secvar_backend_driver secvar_backend;
+
+// Check for secvar support, update secureboot DT compatible if so
+int probe_secvar(void);
+
+// Helper functions
+void clear_bank_list(struct list_head *bank);
+struct secvar_node *alloc_secvar(uint64_t size);
+int realloc_secvar(struct secvar_node *node, uint64_t size);
+struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank);
+int is_key_empty(const char *key, uint64_t key_len);
+int list_length(struct list_head *bank);
+
+#endif
diff --git a/libstb/secvar/secvar_devtree.c b/libstb/secvar/secvar_devtree.c
new file mode 100644
index 00000000..9cd12a15
--- /dev/null
+++ b/libstb/secvar/secvar_devtree.c
@@ -0,0 +1,136 @@ 
+#include <device.h>
+#include <string.h>
+#include "secvar.h"
+#include "secvar_devtree.h"
+
+struct dt_node *secvar_node;
+struct dt_node *secvar_backend_node;
+struct dt_node *secvar_storage_node;
+
+int secvar_set_secure_mode(void)
+{
+	struct dt_property *prop;
+
+	if (!secvar_node)
+		return -1;
+
+	prop = (struct dt_property *) dt_find_property(secvar_node, "os-secure-enforcing");
+	if (prop)
+		return 0;
+
+	prop = dt_add_property(secvar_node, "os-secure-enforcing", 0, 0);
+	if (!prop)
+		return -2;
+
+	return 0;
+}
+
+void secvar_init_devnode(void)
+{
+	struct dt_node *sb_root;
+
+	sb_root = dt_find_by_path(dt_root, "/ibm,opal/");
+
+	secvar_node = dt_new(sb_root, "secvar");
+
+	dt_add_property_string(secvar_node, "compatible", "ibm,secvar-v1");
+
+	secvar_backend_node = dt_new(secvar_node, "backend");
+	secvar_storage_node = dt_new(secvar_node, "storage");
+
+	dt_add_property_string(secvar_backend_node, "compatible", secvar_backend.compatible);
+	dt_add_property_string(secvar_storage_node, "compatible", secvar_storage.compatible);
+
+	dt_add_property_u64(secvar_storage_node, "max-var-size", secvar_storage.max_var_size);
+}
+
+void secvar_set_status(const char *status)
+{
+	struct dt_property *stat_prop;
+	if (!secvar_node)
+		return; // Fail boot?
+
+	stat_prop = (struct dt_property *) dt_find_property(secvar_node, "status");
+
+	if (stat_prop)
+		strcpy(stat_prop->prop, status);
+	else
+		dt_add_property_string(secvar_node, "status", status);
+		// Fail boot if not successful?
+}
+
+
+void secvar_set_update_status(uint64_t val)
+{
+	struct dt_property *stat_prop;
+	if (!secvar_node)
+		return; // Fail boot?
+
+	stat_prop = (struct dt_property *) dt_find_property(secvar_backend_node, "update-status");
+
+	if (stat_prop)
+		memcpy(stat_prop->prop, &val, sizeof(val));
+	else
+		dt_add_property(secvar_backend_node, "update-status", &val, sizeof(val));
+}
+
+
+void secvar_dt_backend_set_prop_string(const char *prop, const char *val)
+{
+	struct dt_property *p;
+
+	if (!secvar_backend_node)
+		return;
+
+	p = (struct dt_property *) dt_find_property(secvar_backend_node, prop);
+
+	if (p)
+		memcpy(p->prop, val, strlen(val));
+	else
+		dt_add_property_string(secvar_backend_node, prop, val);
+}
+
+void secvar_dt_backend_set_prop_u64(const char *prop, uint64_t val)
+{
+	struct dt_property *p;
+
+	if (!secvar_backend_node)
+		return;
+
+	p = (struct dt_property *) dt_find_property(secvar_backend_node, prop);
+
+	if (p)
+		memcpy(p->prop, &val, sizeof(val));
+	else
+		dt_add_property_u64(secvar_backend_node, prop, val);
+}
+
+void secvar_dt_storage_set_prop_string(const char *prop, const char *val)
+{
+	struct dt_property *p;
+
+	if (!secvar_storage_node)
+		return;
+
+	p = (struct dt_property *) dt_find_property(secvar_storage_node, prop);
+
+	if (p)
+		memcpy(p->prop, val, strlen(val));
+	else
+		dt_add_property_string(secvar_backend_node, prop, val);
+}
+
+void secvar_dt_storage_set_prop_u64(const char *prop, uint64_t val)
+{
+	struct dt_property *p;
+
+	if (!secvar_storage_node)
+		return;
+
+	p = (struct dt_property *) dt_find_property(secvar_storage_node, prop);
+
+	if (p)
+		memcpy(p->prop, &val, sizeof(val));
+	else
+		dt_add_property_u64(secvar_backend_node, prop, val);
+}
diff --git a/libstb/secvar/secvar_devtree.h b/libstb/secvar/secvar_devtree.h
new file mode 100644
index 00000000..8ba516f0
--- /dev/null
+++ b/libstb/secvar/secvar_devtree.h
@@ -0,0 +1,15 @@ 
+#ifndef _SECVAR_DEVTREE_H_
+#define _SECVAR_DEVTREE_H_
+
+int secvar_set_secure_mode(void);
+void secvar_init_devnode(void);
+
+void secvar_set_status(const char *status);
+void secvar_set_update_status(uint64_t val);
+
+void secvar_dt_backend_set_prop_string(const char *prop, const char *val);
+void secvar_dt_backend_set_prop_u64(const char *prop, uint64_t val);
+void secvar_dt_storage_set_prop_string(const char *prop, const char *val);
+void secvar_dt_storage_set_prop_u64(const char *prop, uint64_t val);
+
+#endif
diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c
new file mode 100644
index 00000000..e40d10bb
--- /dev/null
+++ b/libstb/secvar/secvar_main.c
@@ -0,0 +1,89 @@ 
+// SPDX-License-Identifier: Apache-2.0
+/* Copyright 2019 IBM Corp. */
+
+#ifndef pr_fmt
+#define pr_fmt(fmt) "SECVAR: " fmt
+#endif
+
+#include <stdlib.h>
+#include <skiboot.h>
+#include <opal.h>
+#include "secvar.h"
+#include "secvar_devtree.h"
+
+struct list_head variable_bank;
+struct list_head update_bank;
+
+int secvar_enabled = 0;	// Set to 1 if secvar is supported
+int secvar_ready = 0;	// Set to 1 when base secvar inits correctly
+
+// To be filled in by platform.secvar_init
+struct secvar_storage_driver secvar_storage = {0};
+struct secvar_backend_driver secvar_backend = {0};
+
+
+int secvar_main(struct secvar_storage_driver storage_driver,
+               struct secvar_backend_driver backend_driver)
+{
+	int rc = OPAL_UNSUPPORTED;
+
+	secvar_storage = storage_driver;
+	secvar_backend = backend_driver;
+
+	secvar_init_devnode();
+
+	secvar_enabled = 1;
+
+	list_head_init(&variable_bank);
+	list_head_init(&update_bank);
+
+	rc = secvar_storage.store_init();
+	if (rc)
+		goto fail;
+
+
+	rc = secvar_storage.load_bank(&variable_bank, SECVAR_VARIABLE_BANK);
+	if (rc)
+		goto fail;
+
+	rc = secvar_storage.load_bank(&update_bank, SECVAR_UPDATE_BANK);
+	if (rc)
+		goto fail;
+
+	// At this point, base secvar is functional. Rest is up to the backend
+	secvar_ready = 1;
+
+	if (secvar_backend.pre_process)
+		rc = secvar_backend.pre_process();
+
+	// Process is required, error if it doesn't exist
+	if (!secvar_backend.process)
+		goto out;
+
+	rc = secvar_backend.process();
+		secvar_set_update_status(rc);
+	if (rc == OPAL_SUCCESS) {
+		rc = secvar_storage.write_bank(&variable_bank, SECVAR_VARIABLE_BANK);
+		if (rc)
+			goto out;
+
+		rc = secvar_storage.write_bank(&update_bank, SECVAR_UPDATE_BANK);
+		if (rc)
+			goto out;
+	}
+
+	// Last point of possible base secvar failure
+	secvar_set_status("okay");
+
+	if (secvar_backend.post_process)
+		rc = secvar_backend.post_process();
+	if (rc)
+		goto out;
+
+	return OPAL_SUCCESS;
+fail:
+	secvar_set_status("fail");
+out:
+	printf("Secure Variables Status %04x\n", rc);
+	return rc;
+}
diff --git a/libstb/secvar/secvar_util.c b/libstb/secvar/secvar_util.c
new file mode 100644
index 00000000..2955a07c
--- /dev/null
+++ b/libstb/secvar/secvar_util.c
@@ -0,0 +1,106 @@ 
+// SPDX-License-Identifier: Apache-2.0
+/* Copyright 2019 IBM Corp. */
+
+#ifndef pr_fmt
+#define pr_fmt(fmt) "SECVAR: " fmt
+#endif
+
+#include <stdlib.h>
+#include <string.h>
+#include <skiboot.h>
+#include <opal.h>
+#include "secvar.h"
+
+void clear_bank_list(struct list_head *bank)
+{
+	struct secvar_node *node, *next;
+
+	if (!bank)
+		return;
+
+	list_for_each_safe(bank, node, next, link) {
+		list_del(&node->link);
+
+		if (node->var) {
+			free(node->var);
+		}
+
+		free(node);
+	}
+}
+
+struct secvar_node *alloc_secvar(uint64_t size)
+{
+	struct secvar_node *ret;
+
+	ret = zalloc(sizeof(struct secvar_node));
+	if (!ret)
+		return NULL;
+
+	ret->var = zalloc(sizeof(struct secvar) + size);
+	if (!ret->var) {
+		free(ret);
+		return NULL;
+	}
+
+	ret->size = size;
+
+	return ret;
+}
+
+int realloc_secvar(struct secvar_node *node, uint64_t size)
+{
+	void *tmp;
+
+	if (node->size >= size)
+		return 0;
+
+	tmp = zalloc(sizeof(struct secvar) + size);
+	if (!tmp)
+		return -1;
+
+	memcpy(tmp, node->var, sizeof(struct secvar) + node->size);
+	free(node->var);
+	node->var = tmp;
+
+	return 0;
+}
+
+struct secvar_node *find_secvar(const char *key, uint64_t key_len, struct list_head *bank)
+{
+	struct secvar_node *node = NULL;
+
+	list_for_each(bank, node, link) {
+		// Prevent matching shorter key subsets / bail early
+		if (key_len != node->var->key_len)
+			continue;
+		if (!memcmp(key, node->var->key, key_len)) {
+			return node;
+		}
+	}
+
+	return NULL;
+}
+
+int is_key_empty(const char *key, uint64_t key_len)
+{
+	int i;
+	for (i = 0; i < key_len; i++) {
+		if (key[i] != 0)
+			return 0;
+	}
+
+	return 1;
+}
+
+int list_length(struct list_head *bank)
+{
+	int ret = 0;
+	struct secvar_node *node;
+
+	list_for_each(bank, node, link) {
+		ret++;
+	}
+
+	return ret;
+}
diff --git a/libstb/secvar/storage/Makefile.inc b/libstb/secvar/storage/Makefile.inc
new file mode 100644
index 00000000..c107736e
--- /dev/null
+++ b/libstb/secvar/storage/Makefile.inc
@@ -0,0 +1,11 @@ 
+# -*-Makefile-*-
+
+SECVAR_STORAGE_DIR = libstb/secvar/storage
+
+SUBDIRS += $(SECVAR_STORAGE_DIR)
+
+SECVAR_STORAGE_SRCS =
+SECVAR_STORAGE_OBJS = $(SECVAR_STORAGE_SRCS:%.c=%.o)
+SECVAR_STORAGE = $(SECVAR_STORAGE_DIR)/built-in.a
+
+$(SECVAR_STORAGE): $(SECVAR_STORAGE_OBJS:%=$(SECVAR_STORAGE_DIR)/%)