diff mbox series

[ima-evm-utils,v2,06/12] Replace the low level HMAC calls when calculating the EVM HMAC

Message ID 20220906195021.854090-7-zohar@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series address deprecated warnings | expand

Commit Message

Mimi Zohar Sept. 6, 2022, 7:50 p.m. UTC 
Calculating the EVM HMAC and labeling the filesystem was originally
included in ima-evm-utils for debugging purposes only.  For now,
instead of removing EVM HMAC support just replace the low level
HMAC_ calls with EVP_ calls.

The '-a, --hashalgo' specifies the IMA hash or signature algorithm.
The kernel EVM HMAC is limited to SHA1.  Fix ima-evm-utils by hard
coding the EVM HMAC algorithm to SHA1.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 57 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

Comments

Stefan Berger Sept. 6, 2022, 8:31 p.m. UTC | #1 
On 9/6/22 15:50, Mimi Zohar wrote:
> Calculating the EVM HMAC and labeling the filesystem was originally
> included in ima-evm-utils for debugging purposes only.  For now,
> instead of removing EVM HMAC support just replace the low level
> HMAC_ calls with EVP_ calls.
> 
> The '-a, --hashalgo' specifies the IMA hash or signature algorithm.
> The kernel EVM HMAC is limited to SHA1.  Fix ima-evm-utils by hard
> coding the EVM HMAC algorithm to SHA1.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   src/evmctl.c | 57 +++++++++++++++++++++++++++++-----------------------
>   1 file changed, 32 insertions(+), 25 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 641504047a36..a9b2f1040787 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1159,12 +1159,12 @@ static int cmd_setxattr_ima(struct command *cmd)
>   
>   static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
>   {
> -        const EVP_MD *md;
> +	size_t mdlen;
> +	EVP_MD_CTX *pctx;
> +	EVP_PKEY *pkey = NULL;
>   	struct stat st;
>   	int err = -1;
>   	uint32_t generation = 0;
> -	HMAC_CTX *pctx;
> -	unsigned int mdlen;
>   	char **xattrname;
>   	unsigned char xattr_value[1024];
>   	unsigned char *key;
> @@ -1175,10 +1175,8 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   	struct h_misc_64 hmac_misc;
>   	int hmac_size;
>   #if OPENSSL_VERSION_NUMBER < 0x10100000
> -	HMAC_CTX ctx;
> +	EVP_MD_CTX ctx;
>   	pctx = &ctx;
> -#else
> -	pctx = HMAC_CTX_new();
>   #endif
>   
>   	key = file2bin(keyfile, NULL, &keylen);
> @@ -1226,19 +1224,26 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   		goto out;
>   	}
>   
> -	md = EVP_get_digestbyname(imaevm_params.hash_algo);
> -	if (!md) {
> -		log_err("EVP_get_digestbyname(%s) failed\n",
> -			imaevm_params.hash_algo);


hash_algo was expected to always be sha1 before because now you seem to 
hard code it to  EVP_sha1() below? Following what I see here this seems 
to indeed be the only hash supported for HMAC:

https://elixir.bootlin.com/linux/latest/source/security/integrity/evm/evm_crypto.c#L38

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


> +#if OPENSSL_VERSION_NUMBER >= 0x10100000
> +	pctx = EVP_MD_CTX_new();
> +	if (!pctx) {
> +		log_err("EVP_MD_CTX_new failed\n");
>   		goto out;
>   	}
> +#endif
>   
> -	err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
> -	if (err) {
> +	pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, evmkey, sizeof(evmkey));
> +	if (!pkey) {
>   		log_err("HMAC_Init() failed\n");
>   		goto out;
>   	}
>   
> +	err = EVP_DigestSignInit(pctx, NULL, EVP_sha1(), NULL, pkey);
> +	if (err != 1) {
> +		log_err("EVP_DigestSignInit() failed\n");
> +		goto out;
> +	}
> +
>   	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
>   		err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
>   		if (err < 0) {
> @@ -1249,12 +1254,12 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   			log_info("skipping xattr: %s\n", *xattrname);
>   			continue;
>   		}
> -		/*log_debug("name: %s, value: %s, size: %d\n", *xattrname, xattr_value, err);*/
>   		log_info("name: %s, size: %d\n", *xattrname, err);
>   		log_debug_dump(xattr_value, err);
> -		err = !HMAC_Update(pctx, xattr_value, err);
> -		if (err) {
> -			log_err("HMAC_Update() failed\n");
> +
> +		err = EVP_DigestSignUpdate(pctx, xattr_value, err);
> +		if (err != 1) {
> +			log_err("EVP_DigestSignUpdate() failed\n");
>   			goto out_ctx_cleanup;
>   		}
>   	}
> @@ -1293,23 +1298,24 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
>   	log_debug("hmac_misc (%d): ", hmac_size);
>   	log_debug_dump(&hmac_misc, hmac_size);
>   
> -	err = !HMAC_Update(pctx, (const unsigned char *)&hmac_misc, hmac_size);
> -	if (err) {
> +	err = EVP_DigestSignUpdate(pctx, &hmac_misc, hmac_size);
> +	if (err != 1) {
>   		log_err("HMAC_Update() failed\n");
>   		goto out_ctx_cleanup;
>   	}
> -	err = !HMAC_Final(pctx, hash, &mdlen);
> -	if (err)
> +	err = EVP_DigestSignFinal(pctx, hash, &mdlen);
> +	if (err != 1)
>   		log_err("HMAC_Final() failed\n");
>   out_ctx_cleanup:
> -#if OPENSSL_VERSION_NUMBER < 0x10100000
> -	HMAC_CTX_cleanup(pctx);
> -#else
> -	HMAC_CTX_free(pctx);
> +	EVP_PKEY_free(pkey);
> +#if OPENSSL_VERSION_NUMBER >= 0x10100000
> +	EVP_MD_CTX_free(pctx);
>   #endif
>   out:
>   	free(key);
> -	return err ?: mdlen;
> +	if (err == 1)
> +		return mdlen;
> +	return err;
>   }
>   
>   static int hmac_evm(const char *file, const char *key)
> @@ -1333,6 +1339,7 @@ static int hmac_evm(const char *file, const char *key)
>   		err = lsetxattr(file, xattr_evm, sig, len + 1, 0);
>   		if (err < 0) {
>   			log_err("setxattr failed: %s\n", file);
> +			errno = 0;
>   			return err;
>   		}
>   	}
diff mbox series

Patch

diff --git a/src/evmctl.c b/src/evmctl.c
index 641504047a36..a9b2f1040787 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1159,12 +1159,12 @@  static int cmd_setxattr_ima(struct command *cmd)
 
 static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
 {
-        const EVP_MD *md;
+	size_t mdlen;
+	EVP_MD_CTX *pctx;
+	EVP_PKEY *pkey = NULL;
 	struct stat st;
 	int err = -1;
 	uint32_t generation = 0;
-	HMAC_CTX *pctx;
-	unsigned int mdlen;
 	char **xattrname;
 	unsigned char xattr_value[1024];
 	unsigned char *key;
@@ -1175,10 +1175,8 @@  static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
 	struct h_misc_64 hmac_misc;
 	int hmac_size;
 #if OPENSSL_VERSION_NUMBER < 0x10100000
-	HMAC_CTX ctx;
+	EVP_MD_CTX ctx;
 	pctx = &ctx;
-#else
-	pctx = HMAC_CTX_new();
 #endif
 
 	key = file2bin(keyfile, NULL, &keylen);
@@ -1226,19 +1224,26 @@  static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
 		goto out;
 	}
 
-	md = EVP_get_digestbyname(imaevm_params.hash_algo);
-	if (!md) {
-		log_err("EVP_get_digestbyname(%s) failed\n",
-			imaevm_params.hash_algo);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+	pctx = EVP_MD_CTX_new();
+	if (!pctx) {
+		log_err("EVP_MD_CTX_new failed\n");
 		goto out;
 	}
+#endif
 
-	err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
-	if (err) {
+	pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, evmkey, sizeof(evmkey));
+	if (!pkey) {
 		log_err("HMAC_Init() failed\n");
 		goto out;
 	}
 
+	err = EVP_DigestSignInit(pctx, NULL, EVP_sha1(), NULL, pkey);
+	if (err != 1) {
+		log_err("EVP_DigestSignInit() failed\n");
+		goto out;
+	}
+
 	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
 		err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
 		if (err < 0) {
@@ -1249,12 +1254,12 @@  static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
 			log_info("skipping xattr: %s\n", *xattrname);
 			continue;
 		}
-		/*log_debug("name: %s, value: %s, size: %d\n", *xattrname, xattr_value, err);*/
 		log_info("name: %s, size: %d\n", *xattrname, err);
 		log_debug_dump(xattr_value, err);
-		err = !HMAC_Update(pctx, xattr_value, err);
-		if (err) {
-			log_err("HMAC_Update() failed\n");
+
+		err = EVP_DigestSignUpdate(pctx, xattr_value, err);
+		if (err != 1) {
+			log_err("EVP_DigestSignUpdate() failed\n");
 			goto out_ctx_cleanup;
 		}
 	}
@@ -1293,23 +1298,24 @@  static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
 	log_debug("hmac_misc (%d): ", hmac_size);
 	log_debug_dump(&hmac_misc, hmac_size);
 
-	err = !HMAC_Update(pctx, (const unsigned char *)&hmac_misc, hmac_size);
-	if (err) {
+	err = EVP_DigestSignUpdate(pctx, &hmac_misc, hmac_size);
+	if (err != 1) {
 		log_err("HMAC_Update() failed\n");
 		goto out_ctx_cleanup;
 	}
-	err = !HMAC_Final(pctx, hash, &mdlen);
-	if (err)
+	err = EVP_DigestSignFinal(pctx, hash, &mdlen);
+	if (err != 1)
 		log_err("HMAC_Final() failed\n");
 out_ctx_cleanup:
-#if OPENSSL_VERSION_NUMBER < 0x10100000
-	HMAC_CTX_cleanup(pctx);
-#else
-	HMAC_CTX_free(pctx);
+	EVP_PKEY_free(pkey);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+	EVP_MD_CTX_free(pctx);
 #endif
 out:
 	free(key);
-	return err ?: mdlen;
+	if (err == 1)
+		return mdlen;
+	return err;
 }
 
 static int hmac_evm(const char *file, const char *key)
@@ -1333,6 +1339,7 @@  static int hmac_evm(const char *file, const char *key)
 		err = lsetxattr(file, xattr_evm, sig, len + 1, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
+			errno = 0;
 			return err;
 		}
 	}