[PATCH v5 04/16] efi_loader: variable: support variable authentication

AKASHI Takahiro takahiro.akashi at linaro.org
Tue Feb 25 06:10:51 CET 2020


On Sun, Feb 23, 2020 at 12:20:16PM +0100, Heinrich Schuchardt wrote:
> On 1/28/20 9:25 AM, AKASHI Takahiro wrote:
> > With this commit, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
> > is supported for authenticated variables and the system secure state
> > will transfer between setup mode and user mode as UEFI specification
> > section 32.3 describes.
> > 
> > Internally, authentication data is stored as part of authenticated
> > variable's value. It is nothing but a pkcs7 message (but we need some
> > wrapper, see efi_variable_parse_signature()) and will be validated by
> > efi_variable_authenticate(), hence efi_signature_verify_with_db().
> > 
> > Associated time value will be encoded in "{...,time=...}" along with
> > other UEFI variable's attributes.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > ---
> >   include/efi_loader.h          |   3 +
> >   lib/efi_loader/efi_variable.c | 665 ++++++++++++++++++++++++++++------
> >   2 files changed, 564 insertions(+), 104 deletions(-)
> > 
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 34f7b8eec8cd..f461c6195834 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -184,6 +184,7 @@ extern const efi_guid_t efi_guid_image_security_database;
> >   extern const efi_guid_t efi_guid_sha256;
> >   extern const efi_guid_t efi_guid_cert_x509;
> >   extern const efi_guid_t efi_guid_cert_x509_sha256;
> > +extern const efi_guid_t efi_guid_cert_type_pkcs7;
> > 
> >   /* GUID of RNG protocol */
> >   extern const efi_guid_t efi_guid_rng_protocol;
> > @@ -738,6 +739,8 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs,
> > 
> >   void efi_sigstore_free(struct efi_signature_store *sigstore);
> >   struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
> > +
> > +bool efi_secure_boot_enabled(void);
> >   #endif /* CONFIG_EFI_SECURE_BOOT */
> > 
> >   #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index c316bdfec0e4..2ae8222b1a94 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -10,8 +10,14 @@
> >   #include <env_internal.h>
> >   #include <hexdump.h>
> >   #include <malloc.h>
> > +#include <rtc.h>
> >   #include <search.h>
> > +#include <linux/compat.h>
> >   #include <u-boot/crc.h>
> > +#include "../lib/crypto/pkcs7_parser.h"
> > +
> > +const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > +static bool efi_secure_boot;
> > 
> >   #define READ_ONLY BIT(31)
> > 
> > @@ -108,7 +114,7 @@ static const char *prefix(const char *str, const char *prefix)
> >    * @attrp:	pointer to UEFI attributes
> >    * Return:	pointer to remainder of U-Boot variable value
> >    */
> > -static const char *parse_attr(const char *str, u32 *attrp)
> > +static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
> >   {
> >   	u32 attr = 0;
> >   	char sep = '{';
> > @@ -131,6 +137,12 @@ static const char *parse_attr(const char *str, u32 *attrp)
> >   			attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS;
> >   		} else if ((s = prefix(str, "run"))) {
> >   			attr |= EFI_VARIABLE_RUNTIME_ACCESS;
> > +		} else if ((s = prefix(str, "time="))) {
> > +			attr |= EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> > +			hex2bin((u8 *)timep, s, sizeof(*timep));
> > +			s += sizeof(*timep) * 2;
> > +		} else if (*str == '}') {
> > +			break;
> >   		} else {
> >   			printf("invalid attribute: %s\n", str);
> >   			break;
> > @@ -148,48 +160,291 @@ static const char *parse_attr(const char *str, u32 *attrp)
> >   }
> > 
> >   /**
> > - * efi_get_variable() - retrieve value of a UEFI variable
> > + * efi_secure_boot_enabled - return if secure boot is enabled or not
> >    *
> > - * This function implements the GetVariable runtime service.
> > + * Return:	true if enabled, false if disabled
> > + */
> > +bool efi_secure_boot_enabled(void)
> > +{
> > +	return efi_secure_boot;
> > +}
> > +
> > +#ifdef CONFIG_EFI_SECURE_BOOT
> > +static u8 pkcs7_hdr[] = {
> > +	/* SEQUENCE */
> > +	0x30, 0x82, 0x05, 0xc7,
> > +	/* OID: pkcs7-signedData */
> > +	0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02,
> > +	/* Context Structured? */
> > +	0xa0, 0x82, 0x05, 0xb8,
> > +};
> > +
> > +/**
> > + * efi_variable_parse_signature - parse a signature in variable
> > + * @buf:	Pointer to variable's value
> > + * @buflen:	Length of @buf
> >    *
> > - * See the Unified Extensible Firmware Interface (UEFI) specification for
> > - * details.
> > + * Parse a signature embedded in variable's value and instantiate
> > + * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
> > + * pkcs7's signedData, some header needed be prepended for correctly
> > + * parsing authentication data, particularly for variable's.
> >    *
> > - * @variable_name:	name of the variable
> > - * @vendor:		vendor GUID
> > - * @attributes:		attributes of the variable
> > - * @data_size:		size of the buffer to which the variable value is copied
> > - * @data:		buffer to which the variable value is copied
> > - * Return:		status code
> > + * Return:	Pointer to pkcs7_message structure on success, NULL on error
> >    */
> > -efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > -				     const efi_guid_t *vendor, u32 *attributes,
> > -				     efi_uintn_t *data_size, void *data)
> > +static struct pkcs7_message *efi_variable_parse_signature(const void *buf,
> > +							  size_t buflen)
> > +{
> > +	u8 *ebuf;
> > +	size_t ebuflen, len;
> > +	struct pkcs7_message *msg;
> > +
> > +	/*
> > +	 * This is the best assumption to check if the binary is
> > +	 * already in a form of pkcs7's signedData.
> > +	 */
> > +	if (buflen > sizeof(pkcs7_hdr) &&
> > +	    !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
> > +		msg = pkcs7_parse_message(buf, buflen);
> > +		goto out;
> > +	}
> > +
> > +	/*
> > +	 * Otherwise, we should add a dummy prefix sequence for pkcs7
> > +	 * message parser to be able to process.
> > +	 * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> > +	 * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > +	 * TODO:
> > +	 * The header should be composed in a more refined manner.
> > +	 */
> > +	debug("Makeshift prefix added to authentication data\n");
> > +	ebuflen = sizeof(pkcs7_hdr) + buflen;
> > +	if (ebuflen <= 0x7f) {
> > +		debug("Data is too short\n");
> > +		return NULL;
> > +	}
> > +
> > +	ebuf = malloc(ebuflen);
> > +	if (!ebuf) {
> > +		debug("Out of memory\n");
> > +		return NULL;
> > +	}
> > +
> > +	memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> > +	memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> > +	len = ebuflen - 4;
> > +	ebuf[2] = (len >> 8) & 0xff;
> > +	ebuf[3] = len & 0xff;
> > +	len = ebuflen - 0x13;
> > +	ebuf[0x11] = (len >> 8) & 0xff;
> > +	ebuf[0x12] = len & 0xff;
> > +
> > +	msg = pkcs7_parse_message(ebuf, ebuflen);
> > +
> > +	free(ebuf);
> > +
> > +out:
> > +	if (IS_ERR(msg))
> > +		return NULL;
> > +
> > +	return msg;
> > +}
> > +
> > +/**
> > + * efi_variable_authenticate - authenticate a variable
> > + * @variable:	Variable name in u16
> > + * @vendor:	Guid of variable
> > + * @data_size:	Size of @data
> > + * @data:	Pointer to variable's value
> > + * @given_attr:	Attributes to be given at SetVariable()
> > + * @env_attr:	Attributes that an existing variable holds
> > + * @time:	signed time that an existing variable holds
> > + *
> > + * Called by efi_set_variable() to verify that the input is correct.
> > + * Will replace the given data pointer with another that points to
> > + * the actual data to store in the internal memory.
> > + * On success, @data and @data_size will be replaced with variable's
> > + * actual data, excluding authentication data, and its size, and variable's
> > + * attributes and signed time will also be returned in @env_attr and @time,
> > + * respectively.
> > + *
> > + * Return:	EFI_SUCCESS on success, status code (negative) on error
> > + */
> > +static efi_status_t efi_variable_authenticate(u16 *variable,
> > +					      const efi_guid_t *vendor,
> > +					      efi_uintn_t *data_size,
> > +					      const void **data, u32 given_attr,
> > +					      u32 *env_attr, u64 *time)
> > +{
> > +	const struct efi_variable_authentication_2 *auth;
> > +	struct efi_signature_store *truststore, *truststore2;
> > +	struct pkcs7_message *var_sig;
> > +	struct efi_image_regions *regs;
> > +	struct efi_time timestamp;
> > +	struct rtc_time tm;
> > +	u64 new_time;
> > +	efi_status_t ret;
> > +
> > +	var_sig = NULL;
> > +	truststore = NULL;
> > +	truststore2 = NULL;
> > +	regs = NULL;
> > +	ret = EFI_SECURITY_VIOLATION;
> > +
> > +	if (*data_size < sizeof(struct efi_variable_authentication_2))
> > +		goto err;
> > +
> > +	/* authentication data */
> > +	auth = *data;
> > +	if (*data_size < (sizeof(auth->time_stamp)
> > +				+ auth->auth_info.hdr.dwLength))
> > +		goto err;
> > +
> > +	if (guidcmp(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7))
> > +		goto err;
> > +
> > +	*data += sizeof(auth->time_stamp) + auth->auth_info.hdr.dwLength;
> > +	*data_size -= (sizeof(auth->time_stamp)
> > +				+ auth->auth_info.hdr.dwLength);
> > +
> > +	memcpy(&timestamp, &auth->time_stamp, sizeof(timestamp));
> > +	memset(&tm, 0, sizeof(tm));
> > +	tm.tm_year = timestamp.year;
> > +	tm.tm_mon = timestamp.month;
> > +	tm.tm_mday = timestamp.day;
> > +	tm.tm_hour = timestamp.hour;
> > +	tm.tm_min = timestamp.minute;
> > +	tm.tm_sec = timestamp.second;
> > +	new_time = rtc_mktime(&tm);
> > +
> > +	if (!efi_secure_boot_enabled()) {
> > +		/* finished checking */
> > +		*time = new_time;
> > +		return EFI_SUCCESS;
> > +	}
> > +
> > +	if (new_time <= *time)
> > +		goto err;
> > +
> > +	/* data to be digested */
> > +	regs = calloc(sizeof(*regs) + sizeof(struct image_region) * 5, 1);
> > +	if (!regs)
> > +		goto err;
> > +	regs->max = 5;
> > +	efi_image_region_add(regs, (uint8_t *)variable,
> > +			     (uint8_t *)variable
> > +				+ u16_strlen(variable) * sizeof(u16), 1);
> > +	efi_image_region_add(regs, (uint8_t *)vendor,
> > +			     (uint8_t *)vendor + sizeof(*vendor), 1);
> > +	efi_image_region_add(regs, (uint8_t *)&given_attr,
> > +			     (uint8_t *)&given_attr + sizeof(given_attr), 1);
> > +	efi_image_region_add(regs, (uint8_t *)&timestamp,
> > +			     (uint8_t *)&timestamp + sizeof(timestamp), 1);
> > +	efi_image_region_add(regs, (uint8_t *)*data,
> > +			     (uint8_t *)*data + *data_size, 1);
> > +
> > +	/* variable's signature list */
> > +	if (auth->auth_info.hdr.dwLength < sizeof(auth->auth_info))
> > +		goto err;
> > +	var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
> > +					       auth->auth_info.hdr.dwLength
> > +						   - sizeof(auth->auth_info));
> > +	if (IS_ERR(var_sig)) {
> > +		debug("Parsing variable's signature failed\n");
> > +		var_sig = NULL;
> > +		goto err;
> > +	}
> > +
> > +	/* signature database used for authentication */
> > +	if (u16_strcmp(variable, L"PK") == 0 ||
> > +	    u16_strcmp(variable, L"KEK") == 0) {
> > +		/* with PK */
> > +		truststore = efi_sigstore_parse_sigdb(L"PK");
> > +		if (!truststore)
> > +			goto err;
> > +	} else if (u16_strcmp(variable, L"db") == 0 ||
> > +		   u16_strcmp(variable, L"dbx") == 0) {
> > +		/* with PK and KEK */
> > +		truststore = efi_sigstore_parse_sigdb(L"KEK");
> > +		truststore2 = efi_sigstore_parse_sigdb(L"PK");
> > +
> > +		if (!truststore) {
> > +			if (!truststore2)
> > +				goto err;
> > +
> > +			truststore = truststore2;
> > +			truststore2 = NULL;
> > +		}
> > +	} else {
> > +		/* TODO: support private authenticated variables */
> > +		goto err;
> > +	}
> > +
> > +	/* verify signature */
> > +	if (efi_signature_verify_with_sigdb(regs, var_sig, truststore, NULL)) {
> > +		debug("Verified\n");
> > +	} else {
> > +		if (truststore2 &&
> > +		    efi_signature_verify_with_sigdb(regs, var_sig,
> > +						    truststore2, NULL)) {
> > +			debug("Verified\n");
> > +		} else {
> > +			debug("Verifying variable's signature failed\n");
> > +			goto err;
> > +		}
> > +	}
> > +
> > +	/* finished checking */
> > +	*time = rtc_mktime(&tm);
> > +	ret = EFI_SUCCESS;
> > +
> > +err:
> > +	efi_sigstore_free(truststore);
> > +	efi_sigstore_free(truststore2);
> > +	pkcs7_free_message(var_sig);
> > +	free(regs);
> > +
> > +	return ret;
> > +}
> > +#else
> > +static efi_status_t efi_variable_authenticate(u16 *variable,
> > +					      const efi_guid_t *vendor,
> > +					      efi_uintn_t *data_size,
> > +					      const void **data, u32 given_attr,
> > +					      u32 *env_attr, u64 *time)
> > +{
> > +	return EFI_SUCCESS;
> > +}
> > +#endif /* CONFIG_EFI_SECURE_BOOT */
> > +
> > +static
> > +efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name,
> > +					    const efi_guid_t *vendor,
> > +					    u32 *attributes,
> > +					    efi_uintn_t *data_size, void *data,
> > +					    bool is_non_volatile)
> >   {
> >   	char *native_name;
> >   	efi_status_t ret;
> >   	unsigned long in_size;
> > -	const char *val, *s;
> > +	const char *val = NULL, *s;
> > +	u64 time = 0;
> >   	u32 attr;
> > 
> > -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> > -		  data_size, data);
> > -
> >   	if (!variable_name || !vendor || !data_size)
> >   		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > 
> >   	ret = efi_to_native(&native_name, variable_name, vendor);
> >   	if (ret)
> > -		return EFI_EXIT(ret);
> > +		return ret;
> > 
> >   	EFI_PRINT("get '%s'\n", native_name);
> > 
> >   	val = env_get(native_name);
> >   	free(native_name);
> >   	if (!val)
> > -		return EFI_EXIT(EFI_NOT_FOUND);
> > +		return EFI_NOT_FOUND;
> > 
> > -	val = parse_attr(val, &attr);
> > +	val = parse_attr(val, &attr, &time);
> > 
> >   	in_size = *data_size;
> > 
> > @@ -198,7 +453,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > 
> >   		/* number of hexadecimal digits must be even */
> >   		if (len & 1)
> > -			return EFI_EXIT(EFI_DEVICE_ERROR);
> > +			return EFI_DEVICE_ERROR;
> > 
> >   		/* two characters per byte: */
> >   		len /= 2;
> > @@ -209,11 +464,13 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >   			goto out;
> >   		}
> > 
> > -		if (!data)
> > -			return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +		if (!data) {
> > +			debug("Variable with no data shouldn't exist.\n");
> > +			return EFI_INVALID_PARAMETER;
> > +		}
> > 
> >   		if (hex2bin(data, s, len))
> > -			return EFI_EXIT(EFI_DEVICE_ERROR);
> > +			return EFI_DEVICE_ERROR;
> > 
> >   		EFI_PRINT("got value: \"%s\"\n", s);
> >   	} else if ((s = prefix(val, "(utf8)"))) {
> > @@ -226,8 +483,10 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >   			goto out;
> >   		}
> > 
> > -		if (!data)
> > -			return EFI_EXIT(EFI_INVALID_PARAMETER);
> > +		if (!data) {
> > +			debug("Variable with no data shouldn't exist.\n");
> > +			return EFI_INVALID_PARAMETER;
> > +		}
> > 
> >   		memcpy(data, s, len);
> >   		((char *)data)[len] = '\0';
> > @@ -235,13 +494,67 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >   		EFI_PRINT("got value: \"%s\"\n", (char *)data);
> >   	} else {
> >   		EFI_PRINT("invalid value: '%s'\n", val);
> > -		return EFI_EXIT(EFI_DEVICE_ERROR);
> > +		return EFI_DEVICE_ERROR;
> >   	}
> > 
> >   out:
> >   	if (attributes)
> >   		*attributes = attr & EFI_VARIABLE_MASK;
> > 
> > +	return ret;
> > +}
> > +
> > +static
> > +efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name,
> > +					      const efi_guid_t *vendor,
> > +					      u32 *attributes,
> > +					      efi_uintn_t *data_size,
> > +					      void *data)
> > +{
> > +	return efi_get_variable_common(variable_name, vendor, attributes,
> > +				       data_size, data, false);
> > +}
> > +
> > +efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name,
> > +						 const efi_guid_t *vendor,
> > +						 u32 *attributes,
> > +						 efi_uintn_t *data_size,
> > +						 void *data)
> > +{
> > +	return efi_get_variable_common(variable_name, vendor, attributes,
> > +				       data_size, data, true);
> > +}
> > +
> > +/**
> > + * efi_efi_get_variable() - retrieve value of a UEFI variable
> > + *
> > + * This function implements the GetVariable runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
> > + *
> > + * @variable_name:	name of the variable
> > + * @vendor:		vendor GUID
> > + * @attributes:		attributes of the variable
> > + * @data_size:		size of the buffer to which the variable value is copied
> > + * @data:		buffer to which the variable value is copied
> > + * Return:		status code
> > + */
> > +efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > +				     const efi_guid_t *vendor, u32 *attributes,
> > +				     efi_uintn_t *data_size, void *data)
> > +{
> > +	efi_status_t ret;
> > +
> > +	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> > +		  data_size, data);
> > +
> > +	ret = efi_get_volatile_variable(variable_name, vendor, attributes,
> > +					data_size, data);
> > +	if (ret == EFI_NOT_FOUND)
> > +		ret = efi_get_nonvolatile_variable(variable_name, vendor,
> > +						   attributes, data_size, data);
> > +
> >   	return EFI_EXIT(ret);
> >   }
> > 
> > @@ -274,6 +587,7 @@ static efi_status_t parse_uboot_variable(char *variable,
> >   {
> >   	char *guid, *name, *end, c;
> >   	unsigned long name_len;
> > +	u64 time;
> >   	u16 *p;
> > 
> >   	guid = strchr(variable, '_');
> > @@ -308,7 +622,7 @@ static efi_status_t parse_uboot_variable(char *variable,
> >   	*(name - 1) = c;
> > 
> >   	/* attributes */
> > -	parse_attr(end, attributes);
> > +	parse_attr(end, attributes, &time);
> > 
> >   	return EFI_SUCCESS;
> >   }
> > @@ -390,7 +704,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> >   		list_len = hexport_r(&env_htab, '\n',
> >   				     H_MATCH_REGEX | H_MATCH_KEY,
> >   				     &efi_variables_list, 0, 1, regexlist);
> > -		/* 1 indicates that no match was found */
> > +
> >   		if (list_len <= 1)
> >   			return EFI_EXIT(EFI_NOT_FOUND);
> > 
> > @@ -403,143 +717,286 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> >   	return EFI_EXIT(ret);
> >   }
> > 
> > -/**
> > - * efi_set_variable() - set value of a UEFI variable
> > - *
> > - * This function implements the SetVariable runtime service.
> > - *
> > - * See the Unified Extensible Firmware Interface (UEFI) specification for
> > - * details.
> > - *
> > - * @variable_name:	name of the variable
> > - * @vendor:		vendor GUID
> > - * @attributes:		attributes of the variable
> > - * @data_size:		size of the buffer with the variable value
> > - * @data:		buffer with the variable value
> > - * Return:		status code
> > - */
> > -efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> > -				     const efi_guid_t *vendor, u32 attributes,
> > -				     efi_uintn_t data_size, const void *data)
> > +static
> > +efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,
> > +					    const efi_guid_t *vendor,
> > +					    u32 attributes,
> > +					    efi_uintn_t data_size,
> > +					    const void *data,
> > +					    bool ro_check,
> > +					    bool is_non_volatile)
> >   {
> > -	char *native_name = NULL, *val = NULL, *s;
> > -	const char *old_val;
> > -	size_t old_size;
> > -	efi_status_t ret = EFI_SUCCESS;
> > +	char *native_name = NULL, *old_data = NULL, *val = NULL, *s;
> > +	efi_uintn_t old_size;
> > +	bool append, delete;
> > +	u64 time = 0;
> >   	u32 attr;
> > +	efi_status_t ret = EFI_SUCCESS;
> > 
> > -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> > -		  data_size, data);
> > +	debug("%s: set '%s'\n", __func__, native_name);
> > 
> >   	if (!variable_name || !*variable_name || !vendor ||
> >   	    ((attributes & EFI_VARIABLE_RUNTIME_ACCESS) &&
> >   	     !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS))) {
> >   		ret = EFI_INVALID_PARAMETER;
> > -		goto out;
> > +		goto err;
> >   	}
> > 
> >   	ret = efi_to_native(&native_name, variable_name, vendor);
> >   	if (ret)
> > -		goto out;
> > +		goto err;
> > +
> > +	/* check if a variable exists */
> > +	old_size = 0;
> > +	attr = 0;
> > +	ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr,
> > +					&old_size, NULL));
> > +	if (ret == EFI_BUFFER_TOO_SMALL) {
> > +		if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) ||
> > +		    (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) {
> > +			ret = EFI_INVALID_PARAMETER;
> > +			goto err;
> > +		}
> > +	}
> > 
> > -	old_val = env_get(native_name);
> > -	if (old_val) {
> > -		old_val = parse_attr(old_val, &attr);
> > +	append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
> > +	attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
> > +	delete = !append && (!data_size || !attributes);
> > 
> > -		/* check read-only first */
> > -		if (attr & READ_ONLY) {
> > +	/* check attributes */
> > +	if (old_size) {
> > +		if (ro_check && (attr & READ_ONLY)) {
> >   			ret = EFI_WRITE_PROTECTED;
> > -			goto out;
> > -		}
> > -
> > -		if ((data_size == 0 &&
> > -		     !(attributes & EFI_VARIABLE_APPEND_WRITE)) ||
> > -		    !attributes) {
> > -			/* delete the variable: */
> > -			env_set(native_name, NULL);
> > -			ret = EFI_SUCCESS;
> > -			goto out;
> > +			goto err;
> >   		}
> > 
> >   		/* attributes won't be changed */
> > -		if (attr != (attributes & ~EFI_VARIABLE_APPEND_WRITE)) {
> > +		if (!delete &&
> > +		    ((ro_check && attr != attributes) ||
> > +		     (!ro_check && ((attr & ~(u32)READ_ONLY)
> > +				    != (attributes & ~(u32)READ_ONLY))))) {
> >   			ret = EFI_INVALID_PARAMETER;
> > -			goto out;
> > -		}
> > -
> > -		if (attributes & EFI_VARIABLE_APPEND_WRITE) {
> > -			if (!prefix(old_val, "(blob)")) {
> > -				ret = EFI_DEVICE_ERROR;
> > -				goto out;
> > -			}
> > -			old_size = strlen(old_val);
> > -		} else {
> > -			old_size = 0;
> > +			goto err;
> >   		}
> >   	} else {
> > -		if (data_size == 0 || !attributes ||
> > -		    (attributes & EFI_VARIABLE_APPEND_WRITE)) {
> > +		if (delete || append) {
> >   			/*
> >   			 * Trying to delete or to update a non-existent
> >   			 * variable.
> >   			 */
> >   			ret = EFI_NOT_FOUND;
> > -			goto out;
> > +			goto err;
> > +		}
> > +	}
> > +
> > +	if (((!u16_strcmp(variable_name, L"PK") ||
> > +	      !u16_strcmp(variable_name, L"KEK")) &&
> > +		!guidcmp(vendor, &efi_global_variable_guid)) ||
> > +	    ((!u16_strcmp(variable_name, L"db") ||
> > +	      !u16_strcmp(variable_name, L"dbx")) &&
> > +		!guidcmp(vendor, &efi_guid_image_security_database))) {
> > +		/* authentication is mandatory */
> > +		if (!(attributes &
> > +		      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
> > +			debug("%ls: AUTHENTICATED_WRITE_ACCESS required\n",
> > +			      variable_name);
> > +			ret = EFI_INVALID_PARAMETER;
> > +			goto err;
> >   		}
> > +	}
> > +
> > +	/* authenticate a variable */
> > +	if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {
> > +		if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
> > +			ret = EFI_INVALID_PARAMETER;
> > +			goto err;
> > +		}
> > +		if (attributes &
> > +		    EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> > +			ret = efi_variable_authenticate(variable_name, vendor,
> > +							&data_size, &data,
> > +							attributes, &attr,
> > +							&time);
> > +			if (ret != EFI_SUCCESS)
> > +				goto err;
> > +
> > +			/* last chance to check for delete */
> > +			if (!data_size)
> > +				delete = true;
> > +		}
> > +	} else {
> > +		if (attributes &
> > +		    (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> > +		     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
> > +			debug("Secure boot is not configured\n");
> > +			ret = EFI_INVALID_PARAMETER;
> > +			goto err;
> > +		}
> > +	}
> > +
> > +	/* delete a variable */
> > +	if (delete) {
> > +		/* !old_size case has been handled before */
> > +		val = NULL;
> > +		ret = EFI_SUCCESS;
> > +		goto out;
> > +	}
> > 
> > +	if (append) {
> > +		old_data = malloc(old_size);
> > +		if (!old_data) {
> > +			return EFI_OUT_OF_RESOURCES;
> > +			goto err;
> > +		}
> > +		ret = EFI_CALL(efi_get_variable(variable_name, vendor,
> > +						&attr, &old_size, old_data));
> > +		if (ret != EFI_SUCCESS)
> > +			goto err;
> > +	} else {
> >   		old_size = 0;
> >   	}
> > 
> > -	val = malloc(old_size + 2 * data_size
> > -		     + strlen("{ro,run,boot,nv}(blob)") + 1);
> > +	val = malloc(2 * old_size + 2 * data_size
> > +		     + strlen("{ro,run,boot,nv,time=0123456701234567}(blob)")
> > +		     + 1);
> >   	if (!val) {
> >   		ret = EFI_OUT_OF_RESOURCES;
> > -		goto out;
> > +		goto err;
> >   	}
> > 
> >   	s = val;
> > 
> > -	/* store attributes */
> > -	attributes &= (EFI_VARIABLE_NON_VOLATILE |
> > +	/*
> > +	 * store attributes
> > +	 */
> > +	attributes &= (READ_ONLY |
> > +		       EFI_VARIABLE_NON_VOLATILE |
> >   		       EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > -		       EFI_VARIABLE_RUNTIME_ACCESS);
> > +		       EFI_VARIABLE_RUNTIME_ACCESS |
> > +		       EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS);
> >   	s += sprintf(s, "{");
> >   	while (attributes) {
> > -		u32 attr = 1 << (ffs(attributes) - 1);
> > +		attr = 1 << (ffs(attributes) - 1);
> > 
> > -		if (attr == EFI_VARIABLE_NON_VOLATILE)
> > +		if (attr == READ_ONLY) {
> > +			s += sprintf(s, "ro");
> > +		} else if (attr == EFI_VARIABLE_NON_VOLATILE) {
> >   			s += sprintf(s, "nv");
> > -		else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS)
> > +		} else if (attr == EFI_VARIABLE_BOOTSERVICE_ACCESS) {
> >   			s += sprintf(s, "boot");
> > -		else if (attr == EFI_VARIABLE_RUNTIME_ACCESS)
> > +		} else if (attr == EFI_VARIABLE_RUNTIME_ACCESS) {
> >   			s += sprintf(s, "run");
> > +		} else if (attr ==
> > +			   EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> > +			s += sprintf(s, "time=");
> > +			s = bin2hex(s, (u8 *)&time, sizeof(time));
> > +		}
> > 
> >   		attributes &= ~attr;
> >   		if (attributes)
> >   			s += sprintf(s, ",");
> >   	}
> >   	s += sprintf(s, "}");
> > -
> > -	if (old_size)
> > -		/* APPEND_WRITE */
> > -		s += sprintf(s, old_val);
> > -	else
> > -		s += sprintf(s, "(blob)");
> > +	s += sprintf(s, "(blob)");
> > 
> >   	/* store payload: */
> > +	if (append)
> > +		s = bin2hex(s, old_data, old_size);
> >   	s = bin2hex(s, data, data_size);
> >   	*s = '\0';
> > 
> >   	EFI_PRINT("setting: %s=%s\n", native_name, val);
> > 
> > +out:
> >   	if (env_set(native_name, val))
> >   		ret = EFI_DEVICE_ERROR;
> > +	else
> > +		ret = EFI_SUCCESS;
> > 
> > -out:
> > +err:
> >   	free(native_name);
> > +	free(old_data);
> >   	free(val);
> > 
> > -	return EFI_EXIT(ret);
> > +	return ret;
> > +}
> > +
> > +static
> > +efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name,
> > +					      const efi_guid_t *vendor,
> > +					      u32 attributes,
> > +					      efi_uintn_t data_size,
> > +					      const void *data,
> > +					      bool ro_check)
> 
> Why would we need such a function which is only used once? Better simply
> call efi_set_variable_common() with the correct value of ro_check.

This function, as well as _nonvolatile version, does exist here
so that we will be able to take a different approach of implementing
"secure storage."

As volatile variables don't need any backend storage, we may want
to use the same function for volatile variables and yet implement
platform-specific non-volatile variables for some reason.
This will give us more flexibility, and yet we have optimal performance
with compiler optimization for the current code.

Thanks,
-Takahiro Akashi

> > +{
> > +	return efi_set_variable_common(variable_name, vendor, attributes,
> > +				       data_size, data, ro_check, false);
> > +}
> > +
> > +efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name,
> > +						 const efi_guid_t *vendor,
> > +						 u32 attributes,
> > +						 efi_uintn_t data_size,
> > +						 const void *data,
> > +						 bool ro_check)
> 
> Same here.
> 
> > +{
> > +	efi_status_t ret;
> > +
> > +	ret = efi_set_variable_common(variable_name, vendor, attributes,
> > +				      data_size, data, ro_check, true);
> > +
> > +	return ret;
> > +}
> > +
> > +static efi_status_t efi_set_variable_internal(u16 *variable_name,
> > +					      const efi_guid_t *vendor,
> > +					      u32 attributes,
> > +					      efi_uintn_t data_size,
> > +					      const void *data,
> > +					      bool ro_check)
> 
> I think this function should be exported. This will allow to avoid
> EFI_CALL in efi_bootmgr.c and efi_setup.c.
> 
> > +{
> > +	efi_status_t ret;
> 
> Your could simply write:
> 
> ret = efi_set_variable_common(variable_name, vendor, attributes,
> 			      data_size, data, ro_check,
> 			      !!(attributes & EFI_VARIABLE_NON_VOLATILE));
> 
> Best regards
> 
> Heinrich
> 
> > +
> > +	if (attributes & EFI_VARIABLE_NON_VOLATILE)
> > +		ret = efi_set_nonvolatile_variable(variable_name, vendor,
> > +						   attributes,
> > +						   data_size, data, ro_check);
> > +	else
> > +		ret = efi_set_volatile_variable(variable_name, vendor,
> > +						attributes, data_size, data,
> > +						ro_check);
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * efi_set_variable() - set value of a UEFI variable
> > + *
> > + * This function implements the SetVariable runtime service.
> > + *
> > + * See the Unified Extensible Firmware Interface (UEFI) specification for
> > + * details.
> > + *
> > + * @variable_name:	name of the variable
> > + * @vendor:		vendor GUID
> > + * @attributes:		attributes of the variable
> > + * @data_size:		size of the buffer with the variable value
> > + * @data:		buffer with the variable value
> > + * Return:		status code
> > + */
> > +efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> > +				     const efi_guid_t *vendor, u32 attributes,
> > +				     efi_uintn_t data_size, const void *data)
> > +{
> > +	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> > +		  data_size, data);
> > +
> > +	/* READ_ONLY bit is not part of API */
> > +	attributes &= ~(u32)READ_ONLY;
> > +
> > +	return EFI_EXIT(efi_set_variable_internal(variable_name, vendor,
> > +						  attributes, data_size, data,
> > +						  true));
> >   }
> > 
> >   /**
> > 
> 


More information about the U-Boot mailing list