[3/7] libstb/secvar: add secvar api implementation

This patch provides the OPAL runtime service frontend for the host OS to
retrieve secure variables, and append new ones for processing on the 
next reboot. These calls operate on the internal abstraction or utilize
the platform-provided driver hooks, and therefore this API should not
need to be updated to support changes in storage or backend drivers.

Included are the following functions:
 - opal_secvar_get()
 - opal_secvar_get_size()
 - opal_secvar_get_next()
 - opal_secvar_enqueue_update()
 - opal_secvar_backend()

opal_secvar_get() retrieves the data blob and metadata blob associated
with a given key. Either the data blob or the metadata blob are
optional. This runtime service only operates on the variable bank.

opal_secvar_get_size() returns the size of the data and/or metadata blob
associated with a given key. This is a convenience function to retrieve
only the size values, and ignore any data blob. This runtime service
only operates on the variable bank.

opal_secvar_get_next() can be used to iterate through the list of
variable keys in the variable bank. Supplying an empty key (or zero key
length) returns the key of the first variable in the variable bank.
Supplying a valid key returns the key of the next variable in sequence.

opal_secvar_enqueue_update() provides a method for the host OS to submit
a new variable for processing on next boot, by appending it to the
update bank. As this does not affect the variable bank, appending a
variable via this runtime service will not affect the output of the
previous set of functions. The update queue is only processed during
secvar initialization.

opal_secvar_backend() returns an enum value for which processing backend
is in use. As the backend determines the format of the variables and how
they should be updated, the host OS should query this function prior to
any other secvar-related operations. The enum value returned from this
function will not change at runtime.

Signed-off-by: Eric Richter <erichte@linux.ibm.com>
---
 ccan/list/list.h           |  38 ++++++
 include/opal-api.h         |   7 +-
 libstb/secvar/Makefile.inc |   2 +-
 libstb/secvar/secvar_api.c | 248 +++++++++++++++++++++++++++++++++++++
 4 files changed, 293 insertions(+), 2 deletions(-)
 create mode 100644 libstb/secvar/secvar_api.c

Eric Richter <erichte@linux.ibm.com> writes:
> This patch provides the OPAL runtime service frontend for the host OS to
> retrieve secure variables, and append new ones for processing on the 
> next reboot. These calls operate on the internal abstraction or utilize
> the platform-provided driver hooks, and therefore this API should not
> need to be updated to support changes in storage or backend drivers.
>
> Included are the following functions:
>  - opal_secvar_get()
>  - opal_secvar_get_size()
>  - opal_secvar_get_next()
>  - opal_secvar_enqueue_update()
>  - opal_secvar_backend()

These all need additions to doc/ to document the calls and the concepts
behind them.

>
> opal_secvar_get() retrieves the data blob and metadata blob associated
> with a given key. Either the data blob or the metadata blob are
> optional. This runtime service only operates on the variable bank.
>
> opal_secvar_get_size() returns the size of the data and/or metadata blob
> associated with a given key. This is a convenience function to retrieve
> only the size values, and ignore any data blob. This runtime service
> only operates on the variable bank.
>
> opal_secvar_get_next() can be used to iterate through the list of
> variable keys in the variable bank. Supplying an empty key (or zero key
> length) returns the key of the first variable in the variable bank.
> Supplying a valid key returns the key of the next variable in sequence.
>
> opal_secvar_enqueue_update() provides a method for the host OS to submit
> a new variable for processing on next boot, by appending it to the
> update bank. As this does not affect the variable bank, appending a
> variable via this runtime service will not affect the output of the
> previous set of functions. The update queue is only processed during
> secvar initialization.

There should probably also be an API for getting enqueued updates?

Think about adding things to a queue in petitboot, and then wanting to
check if there's things queued once booted to an OS.

Is there a reason for having this be separate calls to the calls to
iterate through variables?

> opal_secvar_backend() returns an enum value for which processing backend
> is in use. As the backend determines the format of the variables and how
> they should be updated, the host OS should query this function prior to
> any other secvar-related operations. The enum value returned from this
> function will not change at runtime.

As discussed on the phone this morning, this seems to be more of a
schema version to ensure that everybody agrees on the format of the
secvars that are parsed by skiboot, so is probably more suited to being
in the device tree.

> diff --git a/ccan/list/list.h b/ccan/list/list.h
> index 7cd3a83e..fdeddeb4 100644
> --- a/ccan/list/list.h
> +++ b/ccan/list/list.h
> @@ -523,4 +523,42 @@ static inline struct list_node *list_node_from_off_(void *ptr, size_t off)
>  	(container_off_var(var, member) +		\
>  	 check_type(var->member, struct list_node))
>  
> +
> +#if HAVE_TYPEOF
> +#define list_typeof(var) typeof(var)
> +#else
> +#define list_typeof(var) void *
> +#endif
> +
> +
> +/* Returns member, or NULL if at end of list. */
> +static inline void *list_entry_or_null(const struct list_head *h,
> +                                      const struct list_node *n,
> +                                      size_t off)
> +{
> +       if (n == &h->n)
> +               return NULL;
> +       return (char *)n - off;
> +}
> +
> +/**
> + * list_next - get the next entry in a list
> + * @h: the list_head
> + * @i: a pointer to an entry in the list.
> + * @member: the list_node member of the structure
> + *
> + * If @i was the last entry in the list, returns NULL.
> + *
> + * Example:
> + *     struct child *second;
> + *     second = list_next(&parent->children, first, list);
> + *     if (!second)
> + *             printf("No second child!\n");
> + */
> +#define list_next(h, i, member)                                                \
> +       ((list_typeof(i))list_entry_or_null(list_debug(h),              \
> +                                           (i)->member.next,           \
> +                                           list_off_var_((i), member)))
> +
> +
>  #endif /* CCAN_LIST_H */

Are these upstream? We don't want to carry patches against CCAN modules
that aren't upsteam. I'm not sure this is really needed though, we have
the next pointer, so why not use it?

> diff --git a/include/opal-api.h b/include/opal-api.h
> index 0b0ae196..1451cb00 100644
> --- a/include/opal-api.h
> +++ b/include/opal-api.h
> @@ -232,7 +232,12 @@
>  #define OPAL_XIVE_GET_VP_STATE			170 /* Get NVT state */
>  #define OPAL_NPU_MEM_ALLOC			171
>  #define OPAL_NPU_MEM_RELEASE			172
> -#define OPAL_LAST				172
> +#define OPAL_SECVAR_GET				173
> +#define OPAL_SECVAR_GET_SIZE			174
> +#define OPAL_SECVAR_GET_NEXT			175
> +#define OPAL_SECVAR_ENQUEUE_UPDATE		176
> +#define OPAL_SECVAR_BACKEND			177
> +#define OPAL_LAST				177
>  
>  #define QUIESCE_HOLD			1 /* Spin all calls at entry */
>  #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
> diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
> index 75870910..50316b48 100644
> --- a/libstb/secvar/Makefile.inc
> +++ b/libstb/secvar/Makefile.inc
> @@ -7,7 +7,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_SRCS = secvar_main.c secvar_util.c secvar_api.c
>  SECVAR_OBJS = $(SECVAR_SRCS:%.c=%.o)
>  SECVAR = $(SECVAR_DIR)/built-in.a
>  
> diff --git a/libstb/secvar/secvar_api.c b/libstb/secvar/secvar_api.c
> new file mode 100644
> index 00000000..88275401
> --- /dev/null
> +++ b/libstb/secvar/secvar_api.c
> @@ -0,0 +1,248 @@
> +/* 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 pr_fmt
> +#define pr_fmt(fmt) "SECVAR_API: " fmt
> +#endif
> +
> +#include <opal.h>
> +#include "secvar.h"
> +
> +
> +static int64_t opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
> uint64_t k_metadata, uint64_t k_metadata_size, uint64_t k_data,
> uint64_t k_data_size)

you can use pointers in the arguments here, you don't need to cast
things below.

> +{
> +	struct secvar_node *node;
> +
> +	char *key = (char *) k_key;
> +	uint64_t key_len = (uint64_t) k_key_len;
> +	void *metadata = (void*) k_metadata;
> +	uint64_t *metadata_size = (uint64_t *) k_metadata_size;
> +	void *data = (void *) k_data;
> +	uint64_t *data_size = (uint64_t *) k_data_size;
> +
> +	int64_t rc = OPAL_SUCCESS;
> +
> +	if (!secvar_enabled)
> +		return OPAL_UNSUPPORTED;
> +	if (!key)
> +		return OPAL_PARAMETER;
> +	if (key_len == 0)
> +		return OPAL_PARAMETER;
> +	// buffer and size must both be valid OR both must be NULL
> +	if ((!!metadata) != (!!metadata_size))
> +		return OPAL_PARAMETER;
> +	if ((!!data) != (!!data_size))
> +		return OPAL_PARAMETER;
> +
> +	node = find_secvar(key, key_len, &variable_bank);
> +
> +	if (!node)
> +		return OPAL_EMPTY; // Variable not found, bail early
> +
> +	if (metadata) {
> +		if (*metadata_size < node->var->metadata_size)
> +			rc = OPAL_PARTIAL;
> +		else
> +			memcpy(metadata, node->var->metadata, node->var->metadata_size);
> +		*metadata_size = node->var->metadata_size;
> +	}
> +
> +	if (data) {
> +		if (*data_size < node->var->data_size)
> +			rc = OPAL_PARTIAL;
> +		else
> +			memcpy(data, node->var->data, node->var->data_size);
> +		*data_size = node->var->data_size;
> +	}
> +
> +	return rc;
> +}
> +opal_call(OPAL_SECVAR_GET, opal_secvar_get, 6);
> +
> +
> +static int64_t opal_secvar_get_size(uint64_t k_key, uint64_t k_key_len, uint64_t k_metadata_size, uint64_t k_data_size)
> +{
> +	struct secvar_node *node;
> +
> +	char *key = (char *) k_key;
> +	uint64_t *metadata_size = (uint64_t *) k_metadata_size;
> +	uint64_t *data_size = (uint64_t *) k_data_size;
> +
> +	if (!secvar_enabled)
> +		return OPAL_UNSUPPORTED;
> +	if (!key)
> +		return OPAL_PARAMETER;
> +	if (k_key_len == 0)
> +		return OPAL_PARAMETER;
> +	if ((!metadata_size) && (!data_size)) // Should return at least one of them
> +		return OPAL_PARAMETER;
> +
> +	node = find_secvar(key, k_key_len, &variable_bank);
> +	if (!node)
> +		return OPAL_EMPTY;
> +
> +	if (metadata_size)
> +		*metadata_size = node->var->metadata_size;
> +	if (data_size)
> +		*data_size = node->var->data_size;
> +
> +	return OPAL_SUCCESS;
> +}
> +opal_call(OPAL_SECVAR_GET_SIZE, opal_secvar_get_size, 4);

Why couldn't this be satisfied by passing NULL for the data parameter
ofr OPAL_SECVAR_GET ?

> +static int64_t opal_secvar_get_next(uint64_t k_key, uint64_t
> k_key_len, uint64_t k_key_size)

What's the difference between k_key_len and k_key_size?

I'm also not sure what's up with the k_ prefixes, but that isn't really
part of any coding style for skiboot.

On 6/13/19 7:53 PM, Stewart Smith wrote:
> Eric Richter <erichte@linux.ibm.com> writes:
>> This patch provides the OPAL runtime service frontend for the host OS to
>> retrieve secure variables, and append new ones for processing on the 
>> next reboot. These calls operate on the internal abstraction or utilize
>> the platform-provided driver hooks, and therefore this API should not
>> need to be updated to support changes in storage or backend drivers.
>>
>> Included are the following functions:
>>  - opal_secvar_get()
>>  - opal_secvar_get_size()
>>  - opal_secvar_get_next()
>>  - opal_secvar_enqueue_update()
>>  - opal_secvar_backend()
> 
> These all need additions to doc/ to document the calls and the concepts
> behind them.
> 
>>
>> opal_secvar_get() retrieves the data blob and metadata blob associated
>> with a given key. Either the data blob or the metadata blob are
>> optional. This runtime service only operates on the variable bank.
>>
>> opal_secvar_get_size() returns the size of the data and/or metadata blob
>> associated with a given key. This is a convenience function to retrieve
>> only the size values, and ignore any data blob. This runtime service
>> only operates on the variable bank.
>>
>> opal_secvar_get_next() can be used to iterate through the list of
>> variable keys in the variable bank. Supplying an empty key (or zero key
>> length) returns the key of the first variable in the variable bank.
>> Supplying a valid key returns the key of the next variable in sequence.
>>
>> opal_secvar_enqueue_update() provides a method for the host OS to submit
>> a new variable for processing on next boot, by appending it to the
>> update bank. As this does not affect the variable bank, appending a
>> variable via this runtime service will not affect the output of the
>> previous set of functions. The update queue is only processed during
>> secvar initialization.
> 
> There should probably also be an API for getting enqueued updates?
> 
> Think about adding things to a queue in petitboot, and then wanting to
> check if there's things queued once booted to an OS.

There will need to be one yes, however we have not yet figured out the
kernel/userspace way that this will be expressed. With a pseudo-fs, we
can display each variable as a file, but how are updates to be displayed?
Variable per update? All updates in one variable?

> 
> Is there a reason for having this be separate calls to the calls to
> iterate through variables?
> 

Not sure I follow the question, are you asking about why sequential calls
are needed to _get_next() to iterate all variables, or why we don't return
the data with _get_next()? 

>> opal_secvar_backend() returns an enum value for which processing backend
>> is in use. As the backend determines the format of the variables and how
>> they should be updated, the host OS should query this function prior to
>> any other secvar-related operations. The enum value returned from this
>> function will not change at runtime.
> 
> As discussed on the phone this morning, this seems to be more of a
> schema version to ensure that everybody agrees on the format of the
> secvars that are parsed by skiboot, so is probably more suited to being
> in the device tree.
> 
>> diff --git a/ccan/list/list.h b/ccan/list/list.h
>> index 7cd3a83e..fdeddeb4 100644
>> --- a/ccan/list/list.h
>> +++ b/ccan/list/list.h
>> @@ -523,4 +523,42 @@ static inline struct list_node *list_node_from_off_(void *ptr, size_t off)
>>  	(container_off_var(var, member) +		\
>>  	 check_type(var->member, struct list_node))
>>  
>> +
>> +#if HAVE_TYPEOF
>> +#define list_typeof(var) typeof(var)
>> +#else
>> +#define list_typeof(var) void *
>> +#endif
>> +
>> +
>> +/* Returns member, or NULL if at end of list. */
>> +static inline void *list_entry_or_null(const struct list_head *h,
>> +                                      const struct list_node *n,
>> +                                      size_t off)
>> +{
>> +       if (n == &h->n)
>> +               return NULL;
>> +       return (char *)n - off;
>> +}
>> +
>> +/**
>> + * list_next - get the next entry in a list
>> + * @h: the list_head
>> + * @i: a pointer to an entry in the list.
>> + * @member: the list_node member of the structure
>> + *
>> + * If @i was the last entry in the list, returns NULL.
>> + *
>> + * Example:
>> + *     struct child *second;
>> + *     second = list_next(&parent->children, first, list);
>> + *     if (!second)
>> + *             printf("No second child!\n");
>> + */
>> +#define list_next(h, i, member)                                                \
>> +       ((list_typeof(i))list_entry_or_null(list_debug(h),              \
>> +                                           (i)->member.next,           \
>> +                                           list_off_var_((i), member)))
>> +
>> +
>>  #endif /* CCAN_LIST_H */
> 
> Are these upstream? We don't want to carry patches against CCAN modules
> that aren't upsteam. I'm not sure this is really needed though, we have
> the next pointer, so why not use it?
> 

They are in upstream, however pulling the latest list module managed to
break skiboot entirely last I attempted.

I preferred using this macro when developing as it was more self-documenting
on what it was doing. It is not necessary however, I'll look into a nicer
alternative.

>> diff --git a/include/opal-api.h b/include/opal-api.h
>> index 0b0ae196..1451cb00 100644
>> --- a/include/opal-api.h
>> +++ b/include/opal-api.h
>> @@ -232,7 +232,12 @@
>>  #define OPAL_XIVE_GET_VP_STATE			170 /* Get NVT state */
>>  #define OPAL_NPU_MEM_ALLOC			171
>>  #define OPAL_NPU_MEM_RELEASE			172
>> -#define OPAL_LAST				172
>> +#define OPAL_SECVAR_GET				173
>> +#define OPAL_SECVAR_GET_SIZE			174
>> +#define OPAL_SECVAR_GET_NEXT			175
>> +#define OPAL_SECVAR_ENQUEUE_UPDATE		176
>> +#define OPAL_SECVAR_BACKEND			177
>> +#define OPAL_LAST				177
>>  
>>  #define QUIESCE_HOLD			1 /* Spin all calls at entry */
>>  #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
>> diff --git a/libstb/secvar/Makefile.inc b/libstb/secvar/Makefile.inc
>> index 75870910..50316b48 100644
>> --- a/libstb/secvar/Makefile.inc
>> +++ b/libstb/secvar/Makefile.inc
>> @@ -7,7 +7,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_SRCS = secvar_main.c secvar_util.c secvar_api.c
>>  SECVAR_OBJS = $(SECVAR_SRCS:%.c=%.o)
>>  SECVAR = $(SECVAR_DIR)/built-in.a
>>  
>> diff --git a/libstb/secvar/secvar_api.c b/libstb/secvar/secvar_api.c
>> new file mode 100644
>> index 00000000..88275401
>> --- /dev/null
>> +++ b/libstb/secvar/secvar_api.c
>> @@ -0,0 +1,248 @@
>> +/* 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 pr_fmt
>> +#define pr_fmt(fmt) "SECVAR_API: " fmt
>> +#endif
>> +
>> +#include <opal.h>
>> +#include "secvar.h"
>> +
>> +
>> +static int64_t opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
>> uint64_t k_metadata, uint64_t k_metadata_size, uint64_t k_data,
>> uint64_t k_data_size)
> 
> you can use pointers in the arguments here, you don't need to cast
> things below.
> 
>> +{
>> +	struct secvar_node *node;
>> +
>> +	char *key = (char *) k_key;
>> +	uint64_t key_len = (uint64_t) k_key_len;
>> +	void *metadata = (void*) k_metadata;
>> +	uint64_t *metadata_size = (uint64_t *) k_metadata_size;
>> +	void *data = (void *) k_data;
>> +	uint64_t *data_size = (uint64_t *) k_data_size;
>> +
>> +	int64_t rc = OPAL_SUCCESS;
>> +
>> +	if (!secvar_enabled)
>> +		return OPAL_UNSUPPORTED;
>> +	if (!key)
>> +		return OPAL_PARAMETER;
>> +	if (key_len == 0)
>> +		return OPAL_PARAMETER;
>> +	// buffer and size must both be valid OR both must be NULL
>> +	if ((!!metadata) != (!!metadata_size))
>> +		return OPAL_PARAMETER;
>> +	if ((!!data) != (!!data_size))
>> +		return OPAL_PARAMETER;
>> +
>> +	node = find_secvar(key, key_len, &variable_bank);
>> +
>> +	if (!node)
>> +		return OPAL_EMPTY; // Variable not found, bail early
>> +
>> +	if (metadata) {
>> +		if (*metadata_size < node->var->metadata_size)
>> +			rc = OPAL_PARTIAL;
>> +		else
>> +			memcpy(metadata, node->var->metadata, node->var->metadata_size);
>> +		*metadata_size = node->var->metadata_size;
>> +	}
>> +
>> +	if (data) {
>> +		if (*data_size < node->var->data_size)
>> +			rc = OPAL_PARTIAL;
>> +		else
>> +			memcpy(data, node->var->data, node->var->data_size);
>> +		*data_size = node->var->data_size;
>> +	}
>> +
>> +	return rc;
>> +}
>> +opal_call(OPAL_SECVAR_GET, opal_secvar_get, 6);
>> +
>> +
>> +static int64_t opal_secvar_get_size(uint64_t k_key, uint64_t k_key_len, uint64_t k_metadata_size, uint64_t k_data_size)
>> +{
>> +	struct secvar_node *node;
>> +
>> +	char *key = (char *) k_key;
>> +	uint64_t *metadata_size = (uint64_t *) k_metadata_size;
>> +	uint64_t *data_size = (uint64_t *) k_data_size;
>> +
>> +	if (!secvar_enabled)
>> +		return OPAL_UNSUPPORTED;
>> +	if (!key)
>> +		return OPAL_PARAMETER;
>> +	if (k_key_len == 0)
>> +		return OPAL_PARAMETER;
>> +	if ((!metadata_size) && (!data_size)) // Should return at least one of them
>> +		return OPAL_PARAMETER;
>> +
>> +	node = find_secvar(key, k_key_len, &variable_bank);
>> +	if (!node)
>> +		return OPAL_EMPTY;
>> +
>> +	if (metadata_size)
>> +		*metadata_size = node->var->metadata_size;
>> +	if (data_size)
>> +		*data_size = node->var->data_size;
>> +
>> +	return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_GET_SIZE, opal_secvar_get_size, 4);
> 
> Why couldn't this be satisfied by passing NULL for the data parameter
> ofr OPAL_SECVAR_GET ?
> 

It can be. The size function was split out to keep the _get() function slightly
simpler, however I fear that complexity managed to creep back in anyway.

>> +static int64_t opal_secvar_get_next(uint64_t k_key, uint64_t
>> k_key_len, uint64_t k_key_size)
> 
> What's the difference between k_key_len and k_key_size?
> 
> I'm also not sure what's up with the k_ prefixes, but that isn't really
> part of any coding style for skiboot.
> 

To differentiate the argument from the casted local variable. Will be
removed alongside the unnecessary casts.