[PATCH 5/8] efi_loader: Make the pkcs7 header parsing function an extern

Akashi Takahiro takahiro.akashi at linaro.org
Thu May 7 09:34:16 CEST 2020


On Thu, Apr 30, 2020 at 11:06:27PM +0530, Sughosh Ganu wrote:
> The pkcs7 header parsing functionality is pretty generic, and can be
> used by other features like capsule authentication. Make the function
> as an extern, also changing it's name to efi_parse_pkcs7_header.

The patch itself is fine to me, but is "pkcs7 header" a common term?

-Takahiro Akashi

> 
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>  include/efi_loader.h           |  2 +
>  lib/efi_loader/efi_signature.c | 78 ++++++++++++++++++++++++++++++++
>  lib/efi_loader/efi_variable.c  | 82 +---------------------------------
>  3 files changed, 82 insertions(+), 80 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index b7638d5825..8d923451ce 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -781,6 +781,8 @@ bool efi_secure_boot_enabled(void);
>  
>  bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
>  		     WIN_CERTIFICATE **auth, size_t *auth_len);
> +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t buflen);
> +
>  #endif /* CONFIG_EFI_SECURE_BOOT */
>  
>  #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index bf6f39aab2..9897f5418e 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -25,6 +25,84 @@ const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;
>  
>  #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_parse_pkcs7_header - parse a signature in variable
> + * @buf:	Pointer to the payload's value
> + * @buflen:	Length of @buf
> + *
> + * 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.
> + *
> + * Return:	Pointer to pkcs7_message structure on success, NULL on error
> + */
> +struct pkcs7_message *efi_parse_pkcs7_header(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_hash_regions - calculate a hash value
>   * @regs:	List of regions
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 6c2dd82306..be34a2cadd 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -415,85 +415,7 @@ 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
> - *
> - * 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.
> - *
> - * Return:	Pointer to pkcs7_message structure on success, NULL on error
> - */
> -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;
> -}
> +#ifdef CONFIG_SECURE_BOOT
>  
>  /**
>   * efi_variable_authenticate - authenticate a variable
> @@ -591,7 +513,7 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
>  	/* 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,
> +	var_sig = efi_parse_pkcs7_header(auth->auth_info.cert_data,
>  					       auth->auth_info.hdr.dwLength
>  						   - sizeof(auth->auth_info));
>  	if (IS_ERR(var_sig)) {
> -- 
> 2.17.1
> 


More information about the U-Boot mailing list