[PATCH v2 03/14] crypto: Fix the logic to calculate hash with authattributes set

[Heinrich Schuchardt]

On 12/21/20 12:43 PM, Sughosh Ganu wrote:
> RFC 2315 Section 9.3 describes the message digesting process. The
> digest calculated depends on whether the authenticated attributes are
> present. In case of a scenario where the authenticated attributes are
> present, the message digest that gets signed and is part of the pkcs7
> message is computed from the auth attributes rather than the contents
> field.
>
> Check if the auth attributes are present, and if set, use the auth
> attributes to compute the hash that would be compared with the
> encrypted hash on the pkcs7 message.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>

Hello Takahiro,

as you provided the lib/crypto/pkcs7_verify.c file could you, please,
review this patch.

Best regards

Heinrich

> ---
>
> Changes since V1: None
>
>   lib/crypto/pkcs7_verify.c | 37 ++++++++++++++++++++++++++-----------
>   1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c
> index 320ba49f79..58683ef614 100644
> --- a/lib/crypto/pkcs7_verify.c
> +++ b/lib/crypto/pkcs7_verify.c
> @@ -50,8 +50,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>   	struct image_region regions[2];
>   	int ret = 0;
>
> -	/* The digest was calculated already. */
> -	if (sig->digest)
> +	/*
> +	 * [RFC2315 9.3]
> +	 * If the authenticated attributes are present,
> +	 * the message-digest is calculated on the
> +	 * attributes present in the
> +	 * authenticatedAttributes field and not just
> +	 * the contents field
> +	 */
> +	if (!sinfo->authattrs && sig->digest)
>   		return 0;
>
>   	if (!sinfo->sig->hash_algo)
> @@ -63,17 +70,25 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>   	else
>   		return -ENOPKG;
>
> -	sig->digest = calloc(1, sig->digest_size);
> -	if (!sig->digest) {
> -		pr_warn("Sig %u: Out of memory\n", sinfo->index);
> -		return -ENOMEM;
> -	}
> +	/*
> +	 * Calculate the hash only if the data is present.
> +	 * In case of authenticated variable and capsule,
> +	 * the hash has already been calculated on the
> +	 * efi_image_regions and populated
> +	 */
> +	if (pkcs7->data) {
> +		sig->digest = calloc(1, sig->digest_size);
> +		if (!sig->digest) {
> +			pr_warn("Sig %u: Out of memory\n", sinfo->index);
> +			return -ENOMEM;
> +		}
>
> -	regions[0].data = pkcs7->data;
> -	regions[0].size = pkcs7->data_len;
> +		regions[0].data = pkcs7->data;
> +		regions[0].size = pkcs7->data_len;
>
> -	/* Digest the message [RFC2315 9.3] */
> -	hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
> +		/* Digest the message [RFC2315 9.3] */
> +		hash_calculate(sinfo->sig->hash_algo, regions, 1, sig->digest);
> +	}
>
>   	/* However, if there are authenticated attributes, there must be a
>   	 * message digest attribute amongst them which corresponds to the
>