diff mbox series

[ima-evm-utils,v2,07/12] Add missing EVP_MD_CTX_free() call in calc_evm_hash()

Message ID 20220906195021.854090-8-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 
When EVP_MD_CTX_new() call was added, the corresponding EVP_MD_CTX_free()
was never called.  Properly free it.

Fixes: 81010f0d87ef ("ima-evm-utils: Add backward compatible support for openssl 1.1")
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 57 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 17 deletions(-)

Comments

Stefan Berger Sept. 6, 2022, 8:36 p.m. UTC | #1 
On 9/6/22 15:50, Mimi Zohar wrote:
> When EVP_MD_CTX_new() call was added, the corresponding EVP_MD_CTX_free()
> was never called.  Properly free it.
> 
> Fixes: 81010f0d87ef ("ima-evm-utils: Add backward compatible support for openssl 1.1")
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   src/evmctl.c | 57 ++++++++++++++++++++++++++++++++++++----------------
>   1 file changed, 40 insertions(+), 17 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index a9b2f1040787..b89e74e06c3d 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -331,11 +331,17 @@ err:
>   	return -1;
>   }
>   
> +/*
> + * calc_evm_hash - calculate the file metadata hash
> + *
> + * Returns 0 for EVP_ function failures. Return -1 for other failures.
> + * Return hash algorithm size on success.
> + */
>   static int calc_evm_hash(const char *file, unsigned char *hash)
>   {
>           const EVP_MD *md;
>   	struct stat st;
> -	int err;
> +	int err = -1;
>   	uint32_t generation = 0;
>   	EVP_MD_CTX *pctx;
>   	unsigned int mdlen;
> @@ -349,12 +355,11 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   #if OPENSSL_VERSION_NUMBER < 0x10100000
>   	EVP_MD_CTX ctx;
>   	pctx = &ctx;
> -#else
> -	pctx = EVP_MD_CTX_new();
>   #endif
>   
>   	if (lstat(file, &st)) {
>   		log_err("Failed to stat: %s\n", file);
> +		errno = 0;
>   		return -1;
>   	}
>   
> @@ -391,20 +396,30 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   	list_size = llistxattr(file, list, sizeof(list));
>   	if (list_size < 0) {
>   		log_err("llistxattr() failed\n");
> +		errno = 0;
>   		return -1;
>   	}
>   
> +#if OPENSSL_VERSION_NUMBER >= 0x10100000
> +	pctx = EVP_MD_CTX_new();
> +	if (!pctx) {
> +		log_err("EVP_MD_CTX_new() failed\n");
> +		return 0;
> +	}
> +#endif
> +
>   	md = EVP_get_digestbyname(imaevm_params.hash_algo);
>   	if (!md) {
>   		log_err("EVP_get_digestbyname(%s) failed\n",
>   			imaevm_params.hash_algo);
> -		return 1;
> +		err = 0;
> +		goto out;
>   	}
>   
>   	err = EVP_DigestInit(pctx, md);
>   	if (!err) {
>   		log_err("EVP_DigestInit() failed\n");
> -		return 1;
> +		goto out;
>   	}
>   
>   	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> @@ -415,7 +430,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   			if (err > sizeof(xattr_value)) {
>   				log_err("selinux[%u] value is too long to fit into xattr[%zu]\n",
>   					err, sizeof(xattr_value));
> -				return -1;
> +				err = -1;
> +				goto out;
>   			}
>   			strcpy(xattr_value, selinux_str);
>   		} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && ima_str) {
> @@ -423,7 +439,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   			if (err > sizeof(xattr_value)) {
>   				log_err("ima[%u] value is too long to fit into xattr[%zu]\n",
>   					err, sizeof(xattr_value));
> -				return -1;
> +				err = -1;
> +				goto out;
>   			}
>   			hex2bin(xattr_value, ima_str, err);
>   		} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && evm_portable){
> @@ -432,7 +449,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   			if (err < 0) {
>   				log_err("EVM portable sig: %s required\n",
>   					xattr_ima);
> -				return -1;
> +				goto out;
>   			}
>   			use_xattr_ima = 1;
>   		} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
> @@ -442,7 +459,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   			if (err >= sizeof(xattr_value)) {
>   				log_err("caps[%u] value is too long to fit into xattr[%zu]\n",
>   					err + 1, sizeof(xattr_value));
> -				return -1;
> +				err = -1;
> +				goto out;
>   			}
>   			strcpy(xattr_value, caps_str);
>   		} else {
> @@ -463,7 +481,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   		err = EVP_DigestUpdate(pctx, xattr_value, err);
>   		if (!err) {
>   			log_err("EVP_DigestUpdate() failed\n");
> -			return 1;
> +			goto out;
>   		}
>   	}
>   
> @@ -517,29 +535,33 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
>   	err = EVP_DigestUpdate(pctx, &hmac_misc, hmac_size);
>   	if (!err) {
>   		log_err("EVP_DigestUpdate() failed\n");
> -		return 1;
> +		goto out;
>   	}
>   
>   	if (!evm_immutable && !evm_portable &&
>   	    !(hmac_flags & HMAC_FLAG_NO_UUID)) {
>   		err = get_uuid(&st, uuid);
>   		if (err)
> -			return -1;
> +			goto out;
>   
>   		err = EVP_DigestUpdate(pctx, (const unsigned char *)uuid, sizeof(uuid));
>   		if (!err) {
>   			log_err("EVP_DigestUpdate() failed\n");
> -			return 1;
> +			goto out;
>   		}
>   	}
>   
>   	err = EVP_DigestFinal(pctx, hash, &mdlen);
> -	if (!err) {
> +	if (!err)
>   		log_err("EVP_DigestFinal() failed\n");
> -		return 1
> -	}
>   
> -	return mdlen;
> +out:
> +#if OPENSSL_VERSION_NUMBER >= 0x10100000
> +	EVP_MD_CTX_free(pctx);
> +#endif
> +	if (err == 1)
> +		return mdlen;
> +	return err;
>   }
>   
>   static int sign_evm(const char *file, const char *key)
> @@ -575,6 +597,7 @@ static int sign_evm(const char *file, const char *key)
>   		err = lsetxattr(file, xattr_evm, sig, len, 0);
>   		if (err < 0) {
>   			log_err("setxattr failed: %s\n", file);
> +			errno = 0;
>   			return err;
>   		}
>   	}

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

Patch

diff --git a/src/evmctl.c b/src/evmctl.c
index a9b2f1040787..b89e74e06c3d 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -331,11 +331,17 @@  err:
 	return -1;
 }
 
+/*
+ * calc_evm_hash - calculate the file metadata hash
+ *
+ * Returns 0 for EVP_ function failures. Return -1 for other failures.
+ * Return hash algorithm size on success.
+ */
 static int calc_evm_hash(const char *file, unsigned char *hash)
 {
         const EVP_MD *md;
 	struct stat st;
-	int err;
+	int err = -1;
 	uint32_t generation = 0;
 	EVP_MD_CTX *pctx;
 	unsigned int mdlen;
@@ -349,12 +355,11 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 #if OPENSSL_VERSION_NUMBER < 0x10100000
 	EVP_MD_CTX ctx;
 	pctx = &ctx;
-#else
-	pctx = EVP_MD_CTX_new();
 #endif
 
 	if (lstat(file, &st)) {
 		log_err("Failed to stat: %s\n", file);
+		errno = 0;
 		return -1;
 	}
 
@@ -391,20 +396,30 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 	list_size = llistxattr(file, list, sizeof(list));
 	if (list_size < 0) {
 		log_err("llistxattr() failed\n");
+		errno = 0;
 		return -1;
 	}
 
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+	pctx = EVP_MD_CTX_new();
+	if (!pctx) {
+		log_err("EVP_MD_CTX_new() failed\n");
+		return 0;
+	}
+#endif
+
 	md = EVP_get_digestbyname(imaevm_params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n",
 			imaevm_params.hash_algo);
-		return 1;
+		err = 0;
+		goto out;
 	}
 
 	err = EVP_DigestInit(pctx, md);
 	if (!err) {
 		log_err("EVP_DigestInit() failed\n");
-		return 1;
+		goto out;
 	}
 
 	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
@@ -415,7 +430,8 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 			if (err > sizeof(xattr_value)) {
 				log_err("selinux[%u] value is too long to fit into xattr[%zu]\n",
 					err, sizeof(xattr_value));
-				return -1;
+				err = -1;
+				goto out;
 			}
 			strcpy(xattr_value, selinux_str);
 		} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && ima_str) {
@@ -423,7 +439,8 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 			if (err > sizeof(xattr_value)) {
 				log_err("ima[%u] value is too long to fit into xattr[%zu]\n",
 					err, sizeof(xattr_value));
-				return -1;
+				err = -1;
+				goto out;
 			}
 			hex2bin(xattr_value, ima_str, err);
 		} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && evm_portable){
@@ -432,7 +449,7 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 			if (err < 0) {
 				log_err("EVM portable sig: %s required\n",
 					xattr_ima);
-				return -1;
+				goto out;
 			}
 			use_xattr_ima = 1;
 		} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
@@ -442,7 +459,8 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 			if (err >= sizeof(xattr_value)) {
 				log_err("caps[%u] value is too long to fit into xattr[%zu]\n",
 					err + 1, sizeof(xattr_value));
-				return -1;
+				err = -1;
+				goto out;
 			}
 			strcpy(xattr_value, caps_str);
 		} else {
@@ -463,7 +481,7 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 		err = EVP_DigestUpdate(pctx, xattr_value, err);
 		if (!err) {
 			log_err("EVP_DigestUpdate() failed\n");
-			return 1;
+			goto out;
 		}
 	}
 
@@ -517,29 +535,33 @@  static int calc_evm_hash(const char *file, unsigned char *hash)
 	err = EVP_DigestUpdate(pctx, &hmac_misc, hmac_size);
 	if (!err) {
 		log_err("EVP_DigestUpdate() failed\n");
-		return 1;
+		goto out;
 	}
 
 	if (!evm_immutable && !evm_portable &&
 	    !(hmac_flags & HMAC_FLAG_NO_UUID)) {
 		err = get_uuid(&st, uuid);
 		if (err)
-			return -1;
+			goto out;
 
 		err = EVP_DigestUpdate(pctx, (const unsigned char *)uuid, sizeof(uuid));
 		if (!err) {
 			log_err("EVP_DigestUpdate() failed\n");
-			return 1;
+			goto out;
 		}
 	}
 
 	err = EVP_DigestFinal(pctx, hash, &mdlen);
-	if (!err) {
+	if (!err)
 		log_err("EVP_DigestFinal() failed\n");
-		return 1;
-	}
 
-	return mdlen;
+out:
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+	EVP_MD_CTX_free(pctx);
+#endif
+	if (err == 1)
+		return mdlen;
+	return err;
 }
 
 static int sign_evm(const char *file, const char *key)
@@ -575,6 +597,7 @@  static int sign_evm(const char *file, const char *key)
 		err = lsetxattr(file, xattr_evm, sig, len, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
+			errno = 0;
 			return err;
 		}
 	}