[PATCH 09/14] efi_loader: Make the pkcs7 header parsing function an extern

[Heinrich Schuchardt]

On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> The pkcs7 header parsing functionality is pretty generic, and can be
> used by other features like capsule authentication. Make the function
> an extern, also changing it's name to efi_parse_pkcs7_header
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
>   include/efi_loader.h           |  4 ++
>   lib/efi_loader/efi_signature.c | 85 +++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_variable.c  | 93 ++--------------------------------
>   3 files changed, 93 insertions(+), 89 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 76cd2b36f2..b9226208f5 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -810,6 +810,10 @@ 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,
> +					     u8 **tmpbuf);
> +
>   /* runtime implementation of memcpy() */
>   void efi_memcpy_runtime(void *dest, const void *src, size_t n);
>
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index 79dee27421..9ab071b611 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -27,6 +27,91 @@ const efi_guid_t efi_guid_cert_x509_sha256 = EFI_CERT_X509_SHA256_GUID;
>   const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_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,

Dear Takahiro,

 From where are these magic numbers taken? Could you, please, provide a
reference that we can add as a comment.

Best regards

Heinrich

> +};
> +
> +/**
> + * efi_parse_pkcs7_header - parse a signature in payload
> + * @buf:	Pointer to payload's value
> + * @buflen:	Length of @buf
> + * @tmpbuf:	Pointer to temporary buffer
> + *
> + * Parse a signature embedded in payload'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
> + * A temporary buffer will be allocated if needed, and it should be
> + * kept valid during the authentication because some data in the buffer
> + * will be referenced by efi_signature_verify().
> + *
> + * 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 **tmpbuf)
> +{
> +	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);
> +		if (IS_ERR(msg))
> +			return NULL;
> +		return msg;
> +	}
> +
> +	/*
> +	 * 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.
> +	 */
> +	EFI_PRINT("Makeshift prefix added to authentication data\n");
> +	ebuflen = sizeof(pkcs7_hdr) + buflen;
> +	if (ebuflen <= 0x7f) {
> +		EFI_PRINT("Data is too short\n");
> +		return NULL;
> +	}
> +
> +	ebuf = malloc(ebuflen);
> +	if (!ebuf) {
> +		EFI_PRINT("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);
> +
> +	if (IS_ERR(msg)) {
> +		free(ebuf);
> +		return NULL;
> +	}
> +
> +	*tmpbuf = ebuf;
> +	return msg;
> +}
>
>   /**
>    * efi_hash_regions - calculate a hash value
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 0c689cfb47..ba0874e9e7 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -24,91 +24,6 @@
>   #include <asm/sections.h>
>
>   #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
> - * @tmpbuf:	Pointer to temporary buffer
> - *
> - * 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.
> - * A temporary buffer will be allocated if needed, and it should be
> - * kept valid during the authentication because some data in the buffer
> - * will be referenced by efi_signature_verify().
> - *
> - * 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 **tmpbuf)
> -{
> -	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);
> -		if (IS_ERR(msg))
> -			return NULL;
> -		return msg;
> -	}
> -
> -	/*
> -	 * 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.
> -	 */
> -	EFI_PRINT("Makeshift prefix added to authentication data\n");
> -	ebuflen = sizeof(pkcs7_hdr) + buflen;
> -	if (ebuflen <= 0x7f) {
> -		EFI_PRINT("Data is too short\n");
> -		return NULL;
> -	}
> -
> -	ebuf = malloc(ebuflen);
> -	if (!ebuf) {
> -		EFI_PRINT("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);
> -
> -	if (IS_ERR(msg)) {
> -		free(ebuf);
> -		return NULL;
> -	}
> -
> -	*tmpbuf = ebuf;
> -	return msg;
> -}
>
>   /**
>    * efi_variable_authenticate - authenticate a variable
> @@ -215,10 +130,10 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
>   		goto err;
>
>   	/* ebuf should be kept valid during the authentication */
> -	var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
> -					       auth->auth_info.hdr.dwLength
> -						   - sizeof(auth->auth_info),
> -					       &ebuf);
> +	var_sig = efi_parse_pkcs7_header(auth->auth_info.cert_data,
> +					 auth->auth_info.hdr.dwLength
> +					 - sizeof(auth->auth_info),
> +					 &ebuf);
>   	if (!var_sig) {
>   		EFI_PRINT("Parsing variable's signature failed\n");
>   		goto err;
>