[Skiboot] [RFC 4/6] libstb: add opal runtime services for secure boot key management

Eric Richter erichte at linux.vnet.ibm.com
Tue Apr 2 05:37:54 AEDT 2019



On 3/28/19 9:24 PM, Oliver wrote:
> On Fri, Mar 29, 2019 at 9:19 AM Eric Richter <erichte at linux.ibm.com> wrote:
>>
>> 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.
>>
>> Included are the four minimum required services for basic manipulation:
>>   - opal_secvar_read()
>>   - opal_secvar_read_next()
>>   - opal_secvar_enqueue()
>>   - opal_secvar_info()
>>

I just noticed I failed to update this patch description from a previous 
version, should be s/read/get/

>> opal_secvar_read() takes in an name string and a vendor buffer (likely a
>> GUID), and returns the corresponding blob of data if there is a matching
>> name + vendor pair in the variable bank. This runtime service only
>> operates on the variable bank.
>>
>> opal_secvar_read_next() provides a method to iterate through the
>> variables in the variable bank. If provided a name and vendor that match a
>> variable in the variable bank, the function will return the name and
>> vendor of the next variable in the bank in sequence. When provided an
>> empty string, it will begin from the top of the list. This runtime service
>> only operates on the variable bank.
>>
>> opal_secvar_enqueue() provides a method for the host OS to submit a new
>> secure variable for processing on next boot, by appending it to the
>> update bank, or "update queue". As this does not affect the variable bank,
>> appending a variable via this runtime service will not affect the output
>> of the previous two. The update queue will only ever be processed during
>> the initialization of skiboot, and is part of a forthcoming patch
>> series.
>>
>> opal_secvar_info() has not been implemented yet. It will return
>> status on the size of, and remaining storage space for secure variables.
>>
>> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
>> ---
>>   include/opal-api.h  |   6 +-
>>   libstb/Makefile.inc |   2 +-
>>   libstb/secvar_api.c | 203 ++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 209 insertions(+), 2 deletions(-)
>>   create mode 100644 libstb/secvar_api.c
>>
>> diff --git a/include/opal-api.h b/include/opal-api.h
>> index 73f86f9a..8e398073 100644
>> --- a/include/opal-api.h
>> +++ b/include/opal-api.h
>> @@ -226,7 +226,11 @@
>>   #define OPAL_NX_COPROC_INIT                    167
>>   #define OPAL_NPU_SET_RELAXED_ORDER             168
>>   #define OPAL_NPU_GET_RELAXED_ORDER             169
>> -#define OPAL_LAST                              169
>> +#define OPAL_SECVAR_GET                                170
>> +#define OPAL_SECVAR_GET_NEXT                   171
> 
>> +#define OPAL_SECVAR_ENQUEUE                    172
> 
> Call it ENQUEUE_UPDATE or something. This is a bit vague. >
>> +#define OPAL_SECVAR_INFO                       173
>> +#define OPAL_LAST                              173
>>
>>   #define QUIESCE_HOLD                   1 /* Spin all calls at entry */
>>   #define QUIESCE_REJECT                 2 /* Fail all calls with OPAL_BUSY */
>> diff --git a/libstb/Makefile.inc b/libstb/Makefile.inc
>> index 2bc9e91b..f3a7e716 100644
>> --- a/libstb/Makefile.inc
>> +++ b/libstb/Makefile.inc
>> @@ -4,7 +4,7 @@ LIBSTB_DIR = libstb
>>
>>   SUBDIRS += $(LIBSTB_DIR)
>>
>> -LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c
>> +LIBSTB_SRCS = container.c tpm_chip.c cvc.c secureboot.c trustedboot.c secvar.c secvar_api.c
>>   LIBSTB_OBJS = $(LIBSTB_SRCS:%.c=%.o)
>>   LIBSTB = $(LIBSTB_DIR)/built-in.a
>>
>> diff --git a/libstb/secvar_api.c b/libstb/secvar_api.c
>> new file mode 100644
>> index 00000000..c5de3bdd
>> --- /dev/null
>> +++ b/libstb/secvar_api.c
>> @@ -0,0 +1,203 @@
>> +/* 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_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
>> +{
>> +       struct secvar *var;
>> +
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +       uint32_t *attributes = (uint32_t *) k_attributes;       // TODO: make sure this size is correct
>> +       size_t *data_size = (size_t *) k_data_size;
>> +       void *data = (void *) k_data;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +       if (!data_size)
>> +               return OPAL_PARAMETER;
>> +       if ((*data_size == 0) && (data))        // Data should be NULL when size is zero
>> +               return OPAL_PARAMETER;
>> +       if ((*data_size) && (!data))
>> +               return OPAL_PARAMETER;          // Data can't be NULL with nonzero size
>> +       if (str16nlen(name, SECVAR_MAX_NAME_SIZE) <= 0)
>> +               return OPAL_PARAMETER;          // Empty or erroneous name is useless
>> +
>> +       var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
>> +
>> +       if (!var)
>> +               return OPAL_EMPTY;
>> +
>> +       if (!data) {
>> +               *data_size = var->data_size;
>> +               return OPAL_SUCCESS; // Size check
>> +       }
>> +
>> +       if (var->data_size > *data_size) {
>> +               *data_size = var->data_size;
>> +               return OPAL_PARTIAL;
>> +       }
>> +
>> +       memcpy(data, var->data, var->data_size);
>> +       *data_size = var->data_size;
>> +
> 
>> +       if (attributes)
>> +               *attributes = var->attributes;
> 
> Moving this above the !data check might be a good idea. That would
> allow the kernel to query the size and attributes simultaneously.
> 
> 
>> +
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_GET, opal_secvar_get, 5);
>> +
>> +
>> +static int64_t opal_secvar_get_next(uint64_t k_name_size, uint64_t k_name, uint64_t k_vendor)
>> +{
>> +       struct secvar *var;
>> +       int namelen;
>> +
>> +       size_t *name_size = (size_t *) k_name_size;
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name_size || (*name_size == 0))
>> +               return OPAL_PARAMETER;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +
>> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen < 0)
>> +               return OPAL_PARAMETER;  // Erroneous name buffer
>> +
>> +       if (namelen != 0) {
>> +               // Non-empty string
>> +               var = find_secvar_by_name_vendor(name, vendor, &variable_bank);
>> +               if (!var)
>> +                       return OPAL_PARAMETER;
>> +
>> +               var = list_next(&variable_bank, var, link);
>> +       }
>> +       else {
>> +               // Empty string was found, returning first entry
>> +               var = list_top(&variable_bank, struct secvar, link);
>> +       }
>> +
>> +       if (!var)
>> +               return OPAL_EMPTY;
>> +
>> +       // Return name buffer is too small
>> +       namelen = str16nlen(var->name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen <= 0)
>> +               return OPAL_HARDWARE; // This should not be reached
>> +
>> +       if ((namelen*2) > *name_size) {
>> +               *name_size = namelen*2;
>> +               return OPAL_PARTIAL;
>> +       }
>> +
>> +
>> +       memcpy(name, var->name, namelen*2);
>> +       memcpy(vendor, var->vendor, SECVAR_VENDOR_SIZE);
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_GET_NEXT, opal_secvar_get_next, 3);
>> +
>> +static int64_t opal_secvar_enqueue(uint64_t k_name, uint64_t k_vendor, uint64_t k_attributes, uint64_t k_data_size, uint64_t k_data)
>> +{
>> +       struct secvar *var;
>> +       int namelen;
>> +
>> +       char16_t *name = (char16_t *) k_name;
>> +       guid_t *vendor = (guid_t *) k_vendor;
>> +       uint32_t attributes = (uint32_t) k_attributes;
>> +       size_t data_size = (size_t) k_data_size;
>> +       void *data = (void *) k_data;
>> +
>> +       if (!secvar_enabled)
>> +               return OPAL_HARDWARE;
>> +       if (!name)
>> +               return OPAL_PARAMETER;
>> +       if (!vendor)
>> +               return OPAL_PARAMETER;
>> +       if (!data)
>> +               return OPAL_PARAMETER;
> 
>> +       if (data_size == 0)
>> +               return OPAL_UNSUPPORTED;        // we don't support variable deletion this way
> 
> How does variable deletion work?
> 

At the moment, we don't support that. In order for a variable to be 
deleted, it would have to pass through the update queue, and thus would 
need to be signed. Signing nothing, or some fixed value would be 
vulnerable to a replay attack, so for now we don't support this.

The kernel however, expects a zero data_size as a deletion operation. 
This is included as we explicitly don't support that method.

>> +       if (data_size > SECVAR_MAX_DATA_SIZE)
>> +               return OPAL_PARAMETER;
>> +
>> +       namelen = str16nlen(name, SECVAR_MAX_NAME_SIZE);
>> +       if (namelen <= 0)
>> +               return OPAL_PARAMETER;          // empty or erroneous name is useless
>> +
> 
>> +       var = zalloc(sizeof(struct secvar));
>> +       if (!var)
>> +               return OPAL_NO_MEM;
>> +
>> +       var->data = malloc(data_size);
>> +       if (!var->data) {
>> +               free(var);
>> +               return OPAL_NO_MEM;
>> +       }
> 
> You can use a trailing struct member to do this in a single
> allocation. Not a big deal, but it makes the code a bit cleaner.
> 
>> +
>> +
>> +       memcpy(var->name, name, namelen*2);
>> +       memcpy(var->vendor, vendor, SECVAR_VENDOR_SIZE);
>> +       var->attributes = attributes;
>> +       var->data_size = data_size;
>> +       memcpy(var->data, data, data_size);
>> +
> 
>> +       // TODO: Prevalidate here?
> 
> What does pre-validation involve?
> 

We can only process variable updates on initial boot, so ideally we 
should catch bad variable updates here before going for a full reboot 
(invalid signature, etc).

This would not actually accept the new variable into the variable bank, 
but attempt to filter bad ones that will fail anyway on boot.

>> +
>> +       list_add_tail(&update_bank, &var->link);
>> +
>> +       if (!secvar_write_update_bank)
>> +               return OPAL_HARDWARE;
>> +
>> +       secvar_write_update_bank(&update_bank);
>> +
>> +       return OPAL_SUCCESS;
>> +}
>> +opal_call(OPAL_SECVAR_ENQUEUE, opal_secvar_enqueue, 5);
>> +
>> +
>> +// TODO: not implemented yet, out of scope for now
>> +static int64_t opal_secvar_info(uint64_t attributes, uint64_t *storage_space, uint64_t *remaining_space, uint64_t *max_variable_size)
>> +{
>> +       (void) attributes;
>> +       (void) storage_space;
>> +       (void) remaining_space;
> 
>> +       (void) max_variable_size;
> 
> Is the max variable size a static value or does it depend on how much
> space we have left? If it's static then this should be put in the
> device-tree rather than being queried with an OPAL call.
> 
It is (likely) a static size, however this is a call the kernel 
implementation is expecting.

>> +
>> +       return OPAL_UNSUPPORTED;
>> +}
>> +opal_call(OPAL_SECVAR_INFO, opal_secvar_info, 4);
>> --
>> 2.17.2
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot
> 



More information about the Skiboot mailing list