Re: [PATCH ima-evm-utils v2 1/9] Fix error messages and mdlen init in calc_evm_hmac()

[Stefan Berger <stefanb@linux.ibm.com>]

On 1/12/23 07:24, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Make sure that the function name in the error message corresponds to the
> actual function called. Also, initialize mdlen to the size of 'hash'
> (MAX_DIGEST_SIZE), as this is expected by EVP_DigestSignFinal().

This code was converted from HMAC_Final() where the man page doesn't say much
about mdlen on input but required md to be of size EVP_MAX_MD_SIZE. Now it's
called sig & siglen in the man page:


        maximum necessary size of the output buffer is written to the siglen parameter. If sig is not NULL then before the
        call the siglen parameter should contain the length of the sig buffer. If the call is successful the signature is
        written to sig and the amount of data written to siglen.

It say's 'should' not 'must'. Following the last sentence abive I think evmctl will NOT have created short
HMAC signatures.
The mdlen should be called siglen and hash should defintely be renamed to sig at some point because it's not a 'hash'
and never was.

> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

> ---
>   src/evmctl.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 0ac7930da6f2..d4912d7ee891 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1186,7 +1186,7 @@ static int cmd_setxattr_ima(struct command *cmd)
>   
>   static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
>   {
> -	size_t mdlen;
> +	size_t mdlen = MAX_DIGEST_SIZE;
>   	EVP_MD_CTX *pctx;
>   	EVP_PKEY *pkey = NULL;
>   	struct stat st;
> @@ -1260,7 +1260,7 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   
>   	pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, evmkey, sizeof(evmkey));
>   	if (!pkey) {
> -		log_err("HMAC_Init() failed\n");
> +		log_err("EVP_PKEY_new_mac_key() failed\n");
>   		goto out;
>   	}
>   
> @@ -1326,12 +1326,12 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   
>   	err = EVP_DigestSignUpdate(pctx, &hmac_misc, hmac_size);
>   	if (err != 1) {
> -		log_err("HMAC_Update() failed\n");
> +		log_err("EVP_DigestSignUpdate() failed\n");
>   		goto out_ctx_cleanup;
>   	}
>   	err = EVP_DigestSignFinal(pctx, hash, &mdlen);
>   	if (err != 1)
> -		log_err("HMAC_Final() failed\n");
> +		log_err("EVP_DigestSignFinal() failed\n");
>   out_ctx_cleanup:
>   	EVP_PKEY_free(pkey);
>   #if OPENSSL_VERSION_NUMBER >= 0x10100000