[ima-evm-utils PATCH v2 00/13] Address non concurrency-safe libimaevm global variables

[ima-evm-utils PATCH v2 00/13] Address non concurrency-safe libimaevm global variables

[Mimi Zohar]

The libimaevm global variables are not concurrency-safe.  Instead of
relying on global variables, define new functions with these variables
as parameters, update static functions definitions with these variables,
and deprecate existing functions.  The change is limited to public keys,
hash algorithm, and key password.

To avoid library incompatability, make the existing functions wrappers
for the new function versions and deprecate existing functions.

Changelog v2:
Addressed Stefan Berger's comments:
- Prefix the new library functions to avoid namespace pollution.
- Define internal library function 'public_keys' as struct
  public_key_entry, not void.
- Prefix new static global variables with 'g_'.
- Annotate deprecated functions with "__attribute__(deprecated)".
- init_public_keys() function was defined as void. The new
  imaevm_init_public_keys() function is defined as an int. Check the
  return value.
- Update sign_{ima,evm} function definitions to include the hash
  algorithm as a paramater.
- Added Stefan's "Reviewed-by" tag.

Mimi Zohar (13):
  Rename "public_keys" to "g_public_keys"
  Free public keys list
  Update library function definitions to include a "public_keys"
    parameter
  Update imaevm_verify_hash() definition to include "hash_algo"
    parameter
  Update cmd_verify_ima() to define and use a local list of public keys
  Update cmd_verify_evm to define and use a local list of public keys
  Update ima_measurements to define and use a local list of public keys
  Define library ima_calc_hash2() function with a hash algorithm
    parameter
  Use a local hash algorithm variable when verifying file signatures
  Update EVM signature verification to use a local hash algorithm
    variable
  Use a file specific hash algorithm variable for signing files
  Update sign_hash_v*() definition to include the key password
  Define and use a file specific "keypass" variable

 src/evmctl.c    | 135 ++++++++++++++++++++++++++----------------
 src/imaevm.h    |   9 +++
 src/libimaevm.c | 153 +++++++++++++++++++++++++++++++++++-------------
 3 files changed, 205 insertions(+), 92 deletions(-)

-- 
2.39.3

[ima-evm-utils PATCH v2 01/13] Rename "public_keys" to "g_public_keys"

[Mimi Zohar]

In preparation for replacing the library global public_keys variable,
which is not concurrency-safe, with a local variable, rename public_keys
to g_public_keys.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/libimaevm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 5b224625644e..244774d01805 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -370,14 +370,14 @@ struct public_key_entry {
 	char name[9];
 	EVP_PKEY *key;
 };
-static struct public_key_entry *public_keys = NULL;
+static struct public_key_entry *g_public_keys = NULL;
 
 static EVP_PKEY *find_keyid(uint32_t keyid)
 {
-	struct public_key_entry *entry, *tail = public_keys;
+	struct public_key_entry *entry, *tail = g_public_keys;
 	int i = 1;
 
-	for (entry = public_keys; entry != NULL; entry = entry->next) {
+	for (entry = g_public_keys; entry; entry = entry->next) {
 		if (entry->keyid == keyid)
 			return entry->key;
 		i++;
@@ -394,7 +394,7 @@ static EVP_PKEY *find_keyid(uint32_t keyid)
 	if (tail)
 		tail->next = entry;
 	else
-		public_keys = entry;
+		g_public_keys = entry;
 	log_err("key %d: %x (unknown keyid)\n", i, __be32_to_cpup(&keyid));
 	return 0;
 }
@@ -429,8 +429,8 @@ void init_public_keys(const char *keyfiles)
 		calc_keyid_v2(&entry->keyid, entry->name, entry->key);
 		sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid));
 		log_info("key %d: %s %s\n", i++, entry->name, keyfile);
-		entry->next = public_keys;
-		public_keys = entry;
+		entry->next = g_public_keys;
+		g_public_keys = entry;
 	}
 	free(keyfiles_free);
 }
-- 
2.39.3

[ima-evm-utils PATCH v2 02/13] Free public keys list

[Mimi Zohar]

On failure to allocate memory, free the public keys list.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/imaevm.h    |  1 +
 src/libimaevm.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/imaevm.h b/src/imaevm.h
index 18d7b0e447e1..470e8376f2fb 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -249,6 +249,7 @@ uint32_t imaevm_read_keyid(const char *certfile);
 int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
 int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);
 int ima_verify_signature(const char *file, unsigned char *sig, int siglen, unsigned char *digest, int digestlen);
+void imaevm_free_public_keys(void *public_keys);
 void init_public_keys(const char *keyfiles);
 int imaevm_hash_algo_from_sig(unsigned char *sig);
 const char *imaevm_hash_algo_by_id(int algo);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 244774d01805..534468fe99ca 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -399,11 +399,25 @@ static EVP_PKEY *find_keyid(uint32_t keyid)
 	return 0;
 }
 
+void imaevm_free_public_keys(void *public_keys)
+{
+	struct public_key_entry *entry = public_keys, *next;
+
+	while (entry) {
+		next = entry->next;
+		if (entry->key)
+			free(entry->key);
+		free(entry);
+		entry = next;
+	}
+}
+
 void init_public_keys(const char *keyfiles)
 {
 	struct public_key_entry *entry;
 	char *tmp_keyfiles, *keyfiles_free;
 	char *keyfile;
+	int err = 0;
 	int i = 1;
 
 	tmp_keyfiles = strdup(keyfiles);
@@ -417,6 +431,7 @@ void init_public_keys(const char *keyfiles)
 		entry = malloc(sizeof(struct public_key_entry));
 		if (!entry) {
 			perror("malloc");
+			err = -ENOMEM;
 			break;
 		}
 
@@ -433,6 +448,8 @@ void init_public_keys(const char *keyfiles)
 		g_public_keys = entry;
 	}
 	free(keyfiles_free);
+	if (err < 0)
+		imaevm_free_public_keys(g_public_keys);
 }
 
 /*
-- 
2.39.3

[ima-evm-utils PATCH v2 03/13] Update library function definitions to include a "public_keys" parameter

[Mimi Zohar]

Instead of relying on a global static "public_keys" variable, which is
not concurrency-safe, update static library function definitions to
include it as a parameter, define new library functions with it as
a parameter, and deprecate existing functions.

Define imaevm_init_public_keys(), imaevm_verify_hash(), and
ima_verify_signature2() functions. Update static function definitions
to include "public_keys".

To avoid library incompatibility, make the existing functions -
init_public_keys(), verify_hash(), ima_verify_signature() - wrappers
for the new function versions.

Deprecate init_public_keys(), verify_hash(), ima_verify_signature()
functions.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/imaevm.h    |  4 +++
 src/libimaevm.c | 85 ++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 67 insertions(+), 22 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index 470e8376f2fb..b29a4745fc77 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -249,8 +249,12 @@ uint32_t imaevm_read_keyid(const char *certfile);
 int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
 int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);
 int ima_verify_signature(const char *file, unsigned char *sig, int siglen, unsigned char *digest, int digestlen);
+int ima_verify_signature2(void *public_keys, const char *file,
+			  unsigned char *sig, int siglen,
+			  unsigned char *digest, int digestlen);
 void imaevm_free_public_keys(void *public_keys);
 void init_public_keys(const char *keyfiles);
+int imaevm_init_public_keys(const char *keyfiles, void **public_keys);
 int imaevm_hash_algo_from_sig(unsigned char *sig);
 const char *imaevm_hash_algo_by_id(int algo);
 int calc_hash_sigv3(enum evm_ima_xattr_type type, const char *algo, const unsigned char *in_hash, unsigned char *out_hash);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 534468fe99ca..6fecb2ffd139 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -372,12 +372,13 @@ struct public_key_entry {
 };
 static struct public_key_entry *g_public_keys = NULL;
 
-static EVP_PKEY *find_keyid(uint32_t keyid)
+static EVP_PKEY *find_keyid(struct public_key_entry *public_keys,
+			    uint32_t keyid)
 {
-	struct public_key_entry *entry, *tail = g_public_keys;
+	struct public_key_entry *entry, *tail = public_keys;
 	int i = 1;
 
-	for (entry = g_public_keys; entry; entry = entry->next) {
+	for (entry = public_keys; entry; entry = entry->next) {
 		if (entry->keyid == keyid)
 			return entry->key;
 		i++;
@@ -394,7 +395,7 @@ static EVP_PKEY *find_keyid(uint32_t keyid)
 	if (tail)
 		tail->next = entry;
 	else
-		g_public_keys = entry;
+		public_keys = (void *)entry;
 	log_err("key %d: %x (unknown keyid)\n", i, __be32_to_cpup(&keyid));
 	return 0;
 }
@@ -412,7 +413,7 @@ void imaevm_free_public_keys(void *public_keys)
 	}
 }
 
-void init_public_keys(const char *keyfiles)
+int imaevm_init_public_keys(const char *keyfiles, void **public_keys)
 {
 	struct public_key_entry *entry;
 	char *tmp_keyfiles, *keyfiles_free;
@@ -420,6 +421,11 @@ void init_public_keys(const char *keyfiles)
 	int err = 0;
 	int i = 1;
 
+	if (!public_keys)
+		return -EINVAL;
+
+	*public_keys = NULL;
+
 	tmp_keyfiles = strdup(keyfiles);
 	keyfiles_free = tmp_keyfiles;
 
@@ -444,12 +450,19 @@ void init_public_keys(const char *keyfiles)
 		calc_keyid_v2(&entry->keyid, entry->name, entry->key);
 		sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid));
 		log_info("key %d: %s %s\n", i++, entry->name, keyfile);
-		entry->next = g_public_keys;
-		g_public_keys = entry;
+		entry->next = (struct public_key_entry *)*public_keys;
+		*public_keys = (void *)entry;
 	}
+
 	free(keyfiles_free);
 	if (err < 0)
-		imaevm_free_public_keys(g_public_keys);
+		imaevm_free_public_keys(public_keys);
+	return err;
+}
+
+__attribute__((deprecated)) void init_public_keys(const char *keyfiles)
+{
+	imaevm_init_public_keys(keyfiles, (void **)&g_public_keys);
 }
 
 /*
@@ -466,7 +479,8 @@ void init_public_keys(const char *keyfiles)
  *
  * (Note: signature_v2_hdr struct does not contain the 'type'.)
  */
-static int verify_hash_common(const char *file, const unsigned char *hash,
+static int verify_hash_common(struct public_key_entry *public_keys,
+			      const char *file, const unsigned char *hash,
 			      int size, unsigned char *sig, int siglen)
 {
 	int ret = -1;
@@ -481,7 +495,7 @@ static int verify_hash_common(const char *file, const unsigned char *hash,
 		log_dump(hash, size);
 	}
 
-	pkey = find_keyid(hdr->keyid);
+	pkey = find_keyid(public_keys, hdr->keyid);
 	if (!pkey) {
 		uint32_t keyid = hdr->keyid;
 
@@ -543,11 +557,13 @@ err:
  *
  * Return: 0 verification good, 1 verification bad, -1 error.
  */
-static int verify_hash_v2(const char *file, const unsigned char *hash,
+static int verify_hash_v2(struct public_key_entry *public_keys,
+			  const char *file, const unsigned char *hash,
 			  int size, unsigned char *sig, int siglen)
 {
 	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
-	return verify_hash_common(file, hash, size, sig + 1, siglen - 1);
+	return verify_hash_common(public_keys, file, hash, size,
+				  sig + 1, siglen - 1);
 }
 
 /*
@@ -556,7 +572,8 @@ static int verify_hash_v2(const char *file, const unsigned char *hash,
  *
  * Return: 0 verification good, 1 verification bad, -1 error.
  */
-static int verify_hash_v3(const char *file, const unsigned char *hash,
+static int verify_hash_v3(struct public_key_entry *public_keys,
+			  const char *file, const unsigned char *hash,
 			  int size, unsigned char *sig, int siglen)
 {
 	unsigned char sigv3_hash[MAX_DIGEST_SIZE];
@@ -567,7 +584,8 @@ static int verify_hash_v3(const char *file, const unsigned char *hash,
 		return ret;
 
 	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
-	return verify_hash_common(file, sigv3_hash, size, sig + 1, siglen - 1);
+	return verify_hash_common(public_keys, file, sigv3_hash, size,
+				  sig + 1, siglen - 1);
 }
 
 #define HASH_MAX_DIGESTSIZE 64	/* kernel HASH_MAX_DIGESTSIZE is 64 bytes */
@@ -710,8 +728,9 @@ int imaevm_hash_algo_from_sig(unsigned char *sig)
 		return -1;
 }
 
-int verify_hash(const char *file, const unsigned char *hash, int size,
-		unsigned char *sig, int siglen)
+int imaevm_verify_hash(void *public_keys, const char *file,
+		       const unsigned char *hash, int size,
+		       unsigned char *sig, int siglen)
 {
 	/* Get signature type from sig header */
 	if (sig[1] == DIGSIG_VERSION_1) {
@@ -730,15 +749,25 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
 		return -1;
 #endif
 	} else if (sig[1] == DIGSIG_VERSION_2) {
-		return verify_hash_v2(file, hash, size, sig, siglen);
+		return verify_hash_v2(public_keys, file, hash, size,
+				      sig, siglen);
 	} else if (sig[1] == DIGSIG_VERSION_3) {
-		return verify_hash_v3(file, hash, size, sig, siglen);
+		return verify_hash_v3(public_keys, file, hash, size,
+				      sig, siglen);
 	} else
 		return -1;
 }
 
-int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
-			 unsigned char *digest, int digestlen)
+__attribute__((deprecated)) int verify_hash(const char *file,
+					    const unsigned char *hash, int size,
+					    unsigned char *sig, int siglen)
+{
+	return imaevm_verify_hash(g_public_keys, file, hash, size, sig, siglen);
+}
+
+int ima_verify_signature2(void *public_keys, const char *file,
+			  unsigned char *sig, int siglen,
+			  unsigned char *digest, int digestlen)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	int hashlen, sig_hash_algo;
@@ -766,14 +795,26 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
 	 * measurement list, not by calculating the local file digest.
 	 */
 	if (digest && digestlen > 0)
-		return verify_hash(file, digest, digestlen, sig, siglen);
+		return imaevm_verify_hash(public_keys, file, digest, digestlen,
+					  sig, siglen);
 
 	hashlen = ima_calc_hash(file, hash);
 	if (hashlen <= 1)
 		return hashlen;
 	assert(hashlen <= sizeof(hash));
 
-	return verify_hash(file, hash, hashlen, sig, siglen);
+	return imaevm_verify_hash(public_keys, file, hash, hashlen,
+				  sig, siglen);
+}
+
+__attribute__((deprecated)) int ima_verify_signature(const char *file,
+						     unsigned char *sig,
+						     int siglen,
+						     unsigned char *digest,
+						     int digestlen)
+{
+	return ima_verify_signature2(g_public_keys, file, sig, siglen,
+				     digest, digestlen);
 }
 
 #if CONFIG_SIGV1
-- 
2.39.3

Re: [ima-evm-utils PATCH v2 03/13] Update library function definitions to include a "public_keys" parameter

[Stefan Berger]

On 12/6/23 14:27, Mimi Zohar wrote:
> Instead of relying on a global static "public_keys" variable, which is
> not concurrency-safe, update static library function definitions to
> include it as a parameter, define new library functions with it as
> a parameter, and deprecate existing functions.
> 
> Define imaevm_init_public_keys(), imaevm_verify_hash(), and
> ima_verify_signature2() functions. Update static function definitions
> to include "public_keys".
> 
> To avoid library incompatibility, make the existing functions -
> init_public_keys(), verify_hash(), ima_verify_signature() - wrappers
> for the new function versions.
> 
> Deprecate init_public_keys(), verify_hash(), ima_verify_signature()
> functions.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   src/imaevm.h    |  4 +++
>   src/libimaevm.c | 85 ++++++++++++++++++++++++++++++++++++-------------
>   2 files changed, 67 insertions(+), 22 deletions(-)
> 
> diff --git a/src/imaevm.h b/src/imaevm.h
> index 470e8376f2fb..b29a4745fc77 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -249,8 +249,12 @@ uint32_t imaevm_read_keyid(const char *certfile);
>   int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
>   int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);
>   int ima_verify_signature(const char *file, unsigned char *sig, int siglen, unsigned char *digest, int digestlen);

Forward-declare struct public_key_entry here:
struct public_key_entry;

> +int ima_verify_signature2(void *public_keys, const char *file,

And use it instead of void * here. For the user it's going to be an 
opaque type but better then a void*.

> +			  unsigned char *sig, int siglen,
> +			  unsigned char *digest, int digestlen);
>   void imaevm_free_public_keys(void *public_keys);
>   void init_public_keys(const char *keyfiles);
> +int imaevm_init_public_keys(const char *keyfiles, void **public_keys);
>   int imaevm_hash_algo_from_sig(unsigned char *sig);
>   const char *imaevm_hash_algo_by_id(int algo);
>   int calc_hash_sigv3(enum evm_ima_xattr_type type, const char *algo, const unsigned char *in_hash, unsigned char *out_hash);
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index 534468fe99ca..6fecb2ffd139 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -372,12 +372,13 @@ struct public_key_entry {
>   };
>   static struct public_key_entry *g_public_keys = NULL;
>   
> -static EVP_PKEY *find_keyid(uint32_t keyid)
> +static EVP_PKEY *find_keyid(struct public_key_entry *public_keys,
> +			    uint32_t keyid)
>   {
> -	struct public_key_entry *entry, *tail = g_public_keys;
> +	struct public_key_entry *entry, *tail = public_keys;
>   	int i = 1;
>   
> -	for (entry = g_public_keys; entry; entry = entry->next) {
> +	for (entry = public_keys; entry; entry = entry->next) {
>   		if (entry->keyid == keyid)
>   			return entry->key;
>   		i++;
> @@ -394,7 +395,7 @@ static EVP_PKEY *find_keyid(uint32_t keyid)
>   	if (tail)
>   		tail->next = entry;
>   	else
> -		g_public_keys = entry;
> +		public_keys = (void *)entry;
>   	log_err("key %d: %x (unknown keyid)\n", i, __be32_to_cpup(&keyid));
>   	return 0;
>   }
> @@ -412,7 +413,7 @@ void imaevm_free_public_keys(void *public_keys)
>   	}
>   }
>   
> -void init_public_keys(const char *keyfiles)
> +int imaevm_init_public_keys(const char *keyfiles, void **public_keys)
>   {
>   	struct public_key_entry *entry;
>   	char *tmp_keyfiles, *keyfiles_free;
> @@ -420,6 +421,11 @@ void init_public_keys(const char *keyfiles)
>   	int err = 0;
>   	int i = 1;
>   
> +	if (!public_keys)
> +		return -EINVAL;
> +
> +	*public_keys = NULL;
> +
>   	tmp_keyfiles = strdup(keyfiles);
>   	keyfiles_free = tmp_keyfiles;
>   
> @@ -444,12 +450,19 @@ void init_public_keys(const char *keyfiles)
>   		calc_keyid_v2(&entry->keyid, entry->name, entry->key);
>   		sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid));
>   		log_info("key %d: %s %s\n", i++, entry->name, keyfile);
> -		entry->next = g_public_keys;
> -		g_public_keys = entry;
> +		entry->next = (struct public_key_entry *)*public_keys;
> +		*public_keys = (void *)entry;
>   	}
> +
>   	free(keyfiles_free);
>   	if (err < 0)
> -		imaevm_free_public_keys(g_public_keys);
> +		imaevm_free_public_keys(public_keys);
> +	return err;
> +}
> +
> +__attribute__((deprecated)) void init_public_keys(const char *keyfiles)

The attribute should go into the header file.



> +{
> +	imaevm_init_public_keys(keyfiles, (void **)&g_public_keys);
>   }
>   
>   /*
> @@ -466,7 +479,8 @@ void init_public_keys(const char *keyfiles)
>    *
>    * (Note: signature_v2_hdr struct does not contain the 'type'.)
>    */
> -static int verify_hash_common(const char *file, const unsigned char *hash,
> +static int verify_hash_common(struct public_key_entry *public_keys,
> +			      const char *file, const unsigned char *hash,
>   			      int size, unsigned char *sig, int siglen)
>   {
>   	int ret = -1;
> @@ -481,7 +495,7 @@ static int verify_hash_common(const char *file, const unsigned char *hash,
>   		log_dump(hash, size);
>   	}
>   
> -	pkey = find_keyid(hdr->keyid);
> +	pkey = find_keyid(public_keys, hdr->keyid);
>   	if (!pkey) {
>   		uint32_t keyid = hdr->keyid;
>   
> @@ -543,11 +557,13 @@ err:
>    *
>    * Return: 0 verification good, 1 verification bad, -1 error.
>    */
> -static int verify_hash_v2(const char *file, const unsigned char *hash,
> +static int verify_hash_v2(struct public_key_entry *public_keys,
> +			  const char *file, const unsigned char *hash,
>   			  int size, unsigned char *sig, int siglen)
>   {
>   	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
> -	return verify_hash_common(file, hash, size, sig + 1, siglen - 1);
> +	return verify_hash_common(public_keys, file, hash, size,
> +				  sig + 1, siglen - 1);
>   }
>   
>   /*
> @@ -556,7 +572,8 @@ static int verify_hash_v2(const char *file, const unsigned char *hash,
>    *
>    * Return: 0 verification good, 1 verification bad, -1 error.
>    */
> -static int verify_hash_v3(const char *file, const unsigned char *hash,
> +static int verify_hash_v3(struct public_key_entry *public_keys,
> +			  const char *file, const unsigned char *hash,
>   			  int size, unsigned char *sig, int siglen)
>   {
>   	unsigned char sigv3_hash[MAX_DIGEST_SIZE];
> @@ -567,7 +584,8 @@ static int verify_hash_v3(const char *file, const unsigned char *hash,
>   		return ret;
>   
>   	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
> -	return verify_hash_common(file, sigv3_hash, size, sig + 1, siglen - 1);
> +	return verify_hash_common(public_keys, file, sigv3_hash, size,
> +				  sig + 1, siglen - 1);
>   }
>   
>   #define HASH_MAX_DIGESTSIZE 64	/* kernel HASH_MAX_DIGESTSIZE is 64 bytes */
> @@ -710,8 +728,9 @@ int imaevm_hash_algo_from_sig(unsigned char *sig)
>   		return -1;
>   }
>   
> -int verify_hash(const char *file, const unsigned char *hash, int size,
> -		unsigned char *sig, int siglen)
> +int imaevm_verify_hash(void *public_keys, const char *file,
> +		       const unsigned char *hash, int size,
> +		       unsigned char *sig, int siglen)
>   {
>   	/* Get signature type from sig header */
>   	if (sig[1] == DIGSIG_VERSION_1) {
> @@ -730,15 +749,25 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
>   		return -1;
>   #endif
>   	} else if (sig[1] == DIGSIG_VERSION_2) {
> -		return verify_hash_v2(file, hash, size, sig, siglen);
> +		return verify_hash_v2(public_keys, file, hash, size,
> +				      sig, siglen);
>   	} else if (sig[1] == DIGSIG_VERSION_3) {
> -		return verify_hash_v3(file, hash, size, sig, siglen);
> +		return verify_hash_v3(public_keys, file, hash, size,
> +				      sig, siglen);
>   	} else
>   		return -1;
>   }
>   
> -int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
> -			 unsigned char *digest, int digestlen)
> +__attribute__((deprecated)) int verify_hash(const char *file,
> +					    const unsigned char *hash, int size,
> +					    unsigned char *sig, int siglen)
> +{
> +	return imaevm_verify_hash(g_public_keys, file, hash, size, sig, siglen);
> +}
> +
> +int ima_verify_signature2(void *public_keys, const char *file,
> +			  unsigned char *sig, int siglen,
> +			  unsigned char *digest, int digestlen)
>   {
>   	unsigned char hash[MAX_DIGEST_SIZE];
>   	int hashlen, sig_hash_algo;
> @@ -766,14 +795,26 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
>   	 * measurement list, not by calculating the local file digest.
>   	 */
>   	if (digest && digestlen > 0)
> -		return verify_hash(file, digest, digestlen, sig, siglen);
> +		return imaevm_verify_hash(public_keys, file, digest, digestlen,
> +					  sig, siglen);
>   
>   	hashlen = ima_calc_hash(file, hash);
>   	if (hashlen <= 1)
>   		return hashlen;
>   	assert(hashlen <= sizeof(hash));
>   
> -	return verify_hash(file, hash, hashlen, sig, siglen);
> +	return imaevm_verify_hash(public_keys, file, hash, hashlen,
> +				  sig, siglen);
> +}
> +
> +__attribute__((deprecated)) int ima_verify_signature(const char *file,
> +						     unsigned char *sig,
> +						     int siglen,
> +						     unsigned char *digest,
> +						     int digestlen)
> +{
> +	return ima_verify_signature2(g_public_keys, file, sig, siglen,
> +				     digest, digestlen);
>   }
>   
>   #if CONFIG_SIGV1

[ima-evm-utils PATCH v2 04/13] Update imaevm_verify_hash() definition to include "hash_algo" parameter

[Mimi Zohar]

Instead of relying on a global static "hash_algo" variable, which is not
concurrency-safe, update the imaevm_verify_hash() function definition
and callers to include a "hash_algo" parameter as a place holder.

Now with the "hash_algo" parameter, export the imaevm_verify_hash()
definition.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/imaevm.h    |  3 +++
 src/libimaevm.c | 13 +++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index b29a4745fc77..99987140c278 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -248,6 +248,9 @@ uint32_t imaevm_read_keyid(const char *certfile);
 
 int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
 int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);
+int imaevm_verify_hash(void *public_keys, const char *file,
+		       const char *hash_algo, const unsigned char *hash,
+		       int size, unsigned char *sig, int siglen);
 int ima_verify_signature(const char *file, unsigned char *sig, int siglen, unsigned char *digest, int digestlen);
 int ima_verify_signature2(void *public_keys, const char *file,
 			  unsigned char *sig, int siglen,
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 6fecb2ffd139..eeffe97ec8ea 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -729,8 +729,8 @@ int imaevm_hash_algo_from_sig(unsigned char *sig)
 }
 
 int imaevm_verify_hash(void *public_keys, const char *file,
-		       const unsigned char *hash, int size,
-		       unsigned char *sig, int siglen)
+		       const char *hash_algo, const unsigned char *hash,
+		       int size, unsigned char *sig, int siglen)
 {
 	/* Get signature type from sig header */
 	if (sig[1] == DIGSIG_VERSION_1) {
@@ -762,7 +762,8 @@ __attribute__((deprecated)) int verify_hash(const char *file,
 					    const unsigned char *hash, int size,
 					    unsigned char *sig, int siglen)
 {
-	return imaevm_verify_hash(g_public_keys, file, hash, size, sig, siglen);
+	return imaevm_verify_hash(g_public_keys, file, NULL, hash, size,
+				  sig, siglen);
 }
 
 int ima_verify_signature2(void *public_keys, const char *file,
@@ -795,15 +796,15 @@ int ima_verify_signature2(void *public_keys, const char *file,
 	 * measurement list, not by calculating the local file digest.
 	 */
 	if (digest && digestlen > 0)
-		return imaevm_verify_hash(public_keys, file, digest, digestlen,
-					  sig, siglen);
+		return imaevm_verify_hash(public_keys, file, NULL, digest,
+					  digestlen, sig, siglen);
 
 	hashlen = ima_calc_hash(file, hash);
 	if (hashlen <= 1)
 		return hashlen;
 	assert(hashlen <= sizeof(hash));
 
-	return imaevm_verify_hash(public_keys, file, hash, hashlen,
+	return imaevm_verify_hash(public_keys, file, NULL, hash, hashlen,
 				  sig, siglen);
 }
 
-- 
2.39.3

[ima-evm-utils PATCH v2 05/13] Update cmd_verify_ima() to define and use a local list of public keys

[Mimi Zohar]

Update the static verify_ima() function definition to include
"public_keys".

Replace calling init_public_keys() with the imaevm_init_public_keys()
version.  Similarly replace ima_verify_signature() with the
ima_verify_signature2() version.

Free the local public keys list.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 2710a27cb305..3ae79262efbb 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -972,7 +972,7 @@ static int cmd_verify_evm(struct command *cmd)
 	return err;
 }
 
-static int verify_ima(const char *file)
+static int verify_ima(void *public_keys, const char *file)
 {
 	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len;
@@ -999,34 +999,43 @@ static int verify_ima(const char *file)
 		}
 	}
 
-	return ima_verify_signature(file, sig, len, NULL, 0);
+	return ima_verify_signature2(public_keys, file, sig, len, NULL, 0);
 }
 
 static int cmd_verify_ima(struct command *cmd)
 {
 	char *file = g_argv[optind++];
+	void *public_keys = NULL;
 	int err, fails = 0;
 
-	if (imaevm_params.x509) {
-		if (imaevm_params.keyfile) /* Support multiple public keys */
-			init_public_keys(imaevm_params.keyfile);
-		else			   /* assume read pubkey from x509 cert */
-			init_public_keys("/etc/keys/x509_evm.der");
-	}
-
 	if (!file) {
 		log_err("Parameters missing\n");
 		print_usage(cmd);
 		return -1;
 	}
 
+	if (imaevm_params.x509) {
+		if (imaevm_params.keyfile) /* Support multiple public keys */
+			err = imaevm_init_public_keys(imaevm_params.keyfile,
+						      &public_keys);
+		else			   /* assume read pubkey from x509 cert */
+			err = imaevm_init_public_keys("/etc/keys/x509_evm.der",
+						      &public_keys);
+		if (err < 0) {
+			log_info("Failed loading public keys");
+			return err;
+		}
+	}
+
 	do {
-		err = verify_ima(file);
+		err = verify_ima(public_keys, file);
 		if (err)
 			fails++;
 		if (!err && imaevm_params.verbose >= LOG_INFO)
 			log_info("%s: verification is OK\n", file);
 	} while ((file = g_argv[optind++]));
+
+	imaevm_free_public_keys(public_keys);
 	return fails > 0;
 }
 
-- 
2.39.3

Re: [ima-evm-utils PATCH v2 05/13] Update cmd_verify_ima() to define and use a local list of public keys

[Stefan Berger]

On 12/6/23 14:27, Mimi Zohar wrote:
> Update the static verify_ima() function definition to include
> "public_keys".
> 
> Replace calling init_public_keys() with the imaevm_init_public_keys()
> version.  Similarly replace ima_verify_signature() with the
> ima_verify_signature2() version.
> 
> Free the local public keys list.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   src/evmctl.c | 29 +++++++++++++++++++----------
>   1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 2710a27cb305..3ae79262efbb 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -972,7 +972,7 @@ static int cmd_verify_evm(struct command *cmd)
>   	return err;
>   }
>   
> -static int verify_ima(const char *file) > +static int verify_ima(void *public_keys, const char *file)

Use struct public_key_entry *.

>   {
>   	unsigned char sig[MAX_SIGNATURE_SIZE];
>   	int len;
> @@ -999,34 +999,43 @@ static int verify_ima(const char *file)
>   		}
>   	}
>   
> -	return ima_verify_signature(file, sig, len, NULL, 0);
> +	return ima_verify_signature2(public_keys, file, sig, len, NULL, 0);
>   }
>   
>   static int cmd_verify_ima(struct command *cmd)
>   {
>   	char *file = g_argv[optind++];
> +	void *public_keys = NULL;

Use struct public_key_entry *.

>   	int err, fails = 0;
>   
> -	if (imaevm_params.x509) {
> -		if (imaevm_params.keyfile) /* Support multiple public keys */
> -			init_public_keys(imaevm_params.keyfile);
> -		else			   /* assume read pubkey from x509 cert */
> -			init_public_keys("/etc/keys/x509_evm.der");
> -	}
> -
>   	if (!file) {
>   		log_err("Parameters missing\n");
>   		print_usage(cmd);
>   		return -1;
>   	}
>   
> +	if (imaevm_params.x509) {
> +		if (imaevm_params.keyfile) /* Support multiple public keys */
> +			err = imaevm_init_public_keys(imaevm_params.keyfile,
> +						      &public_keys);
> +		else			   /* assume read pubkey from x509 cert */
> +			err = imaevm_init_public_keys("/etc/keys/x509_evm.der",
> +						      &public_keys);
> +		if (err < 0) {
> +			log_info("Failed loading public keys");
> +			return err;
> +		}
> +	}
> +
>   	do {
> -		err = verify_ima(file);
> +		err = verify_ima(public_keys, file);
>   		if (err)
>   			fails++;
>   		if (!err && imaevm_params.verbose >= LOG_INFO)
>   			log_info("%s: verification is OK\n", file);
>   	} while ((file = g_argv[optind++]));
> +
> +	imaevm_free_public_keys(public_keys);
>   	return fails > 0;
>   }
>   

Rest LGTM.

[ima-evm-utils PATCH v2 06/13] Update cmd_verify_evm to define and use a local list of public keys

[Mimi Zohar]

Replace calling init_public_keys() with the imaevm_init_public_keys()
version. Similarly replace verify_hash() with the imaevm_verify_hash()
version.

Update the static function verify_evm() definition to include a
"public_keys" parameter.

Free the local public keys list.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 3ae79262efbb..06cdffdd3755 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -905,7 +905,7 @@ static int cmd_sign_evm(struct command *cmd)
 	return do_cmd(cmd, sign_evm_path);
 }
 
-static int verify_evm(const char *file)
+static int verify_evm(void *public_keys, const char *file)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	unsigned char sig[MAX_SIGNATURE_SIZE];
@@ -945,12 +945,14 @@ static int verify_evm(const char *file)
 		return mdlen;
 	assert(mdlen <= sizeof(hash));
 
-	return verify_hash(file, hash, mdlen, sig, len);
+	return imaevm_verify_hash(public_keys, file, imaevm_params.hash_algo,
+				  hash, mdlen, sig, len);
 }
 
 static int cmd_verify_evm(struct command *cmd)
 {
 	char *file = g_argv[optind++];
+	void *public_keys = NULL;
 	int err;
 
 	if (!file) {
@@ -961,14 +963,22 @@ static int cmd_verify_evm(struct command *cmd)
 
 	if (imaevm_params.x509) {
 		if (imaevm_params.keyfile) /* Support multiple public keys */
-			init_public_keys(imaevm_params.keyfile);
+			err = imaevm_init_public_keys(imaevm_params.keyfile,
+						      &public_keys);
 		else			   /* assume read pubkey from x509 cert */
-			init_public_keys("/etc/keys/x509_evm.der");
+			err = imaevm_init_public_keys("/etc/keys/x509_evm.der",
+						      &public_keys);
+		if (err < 0) {
+			log_info("Failed loading public keys");
+			return err;
+		}
 	}
 
-	err = verify_evm(file);
+	err = verify_evm(public_keys, file);
 	if (!err && imaevm_params.verbose >= LOG_INFO)
 		log_info("%s: verification is OK\n", file);
+
+	imaevm_free_public_keys(public_keys);
 	return err;
 }
 
-- 
2.39.3

[ima-evm-utils PATCH v2 07/13] Update ima_measurements to define and use a local list of public keys

[Mimi Zohar]

Replace calling init_public_keys() with the imaevm_init_public_keys()
version.  Similarly replace ima_verify_signature() with the
ima_verify_signature2() version.

Update the static ima_ng_show() function definition to include a
"public_keys" parameter.

Free the local public keys list.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 06cdffdd3755..5aea3652c80f 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1625,7 +1625,7 @@ static int lookup_template_name_entry(char *template_name)
 	return 0;
 }
 
-void ima_ng_show(struct template_entry *entry)
+static void ima_ng_show(void *public_keys, struct template_entry *entry)
 {
 	uint8_t *fieldp = entry->template;
 	uint32_t field_len;
@@ -1751,10 +1751,12 @@ void ima_ng_show(struct template_entry *entry)
 		 * the measurement list or calculate the hash.
 		 */
 		if (verify_list_sig)
-			err = ima_verify_signature(path, sig, sig_len,
-						   digest, digest_len);
+			err = ima_verify_signature2(public_keys, path,
+						    sig, sig_len,
+						    digest, digest_len);
 		else
-			err = ima_verify_signature(path, sig, sig_len, NULL, 0);
+			err = ima_verify_signature2(public_keys, path,
+						    sig, sig_len, NULL, 0);
 
 		if (!err && imaevm_params.verbose > LOG_INFO)
 			log_info("%s: verification is OK\n", path);
@@ -2234,6 +2236,7 @@ static int ima_measurement(const char *file)
 	int first_record = 1;
 	unsigned int pseudo_padded_banks_mask, pseudo_banks_mask;
 	unsigned long entry_num = 0;
+	void *public_keys = NULL;
 	int c;
 
 	struct template_entry entry = { .template = NULL };
@@ -2263,10 +2266,16 @@ static int ima_measurement(const char *file)
 	}
 
 	if (imaevm_params.keyfile)	/* Support multiple public keys */
-		init_public_keys(imaevm_params.keyfile);
+		err = imaevm_init_public_keys(imaevm_params.keyfile,
+					      &public_keys);
 	else				/* assume read pubkey from x509 cert */
-		init_public_keys("/etc/keys/x509_evm.der");
-	if (errno)
+		err = imaevm_init_public_keys("/etc/keys/x509_evm.der",
+					      &public_keys);
+	/*
+	 * Without public keys, cannot validate signatures, but can
+	 * still calculate and verify the measurement list against TPM PCRs.
+	 */
+	if (errno || err < 0)
 		log_errno_reset(LOG_DEBUG,
 				"Failure in initializing public keys");
 
@@ -2416,7 +2425,7 @@ static int ima_measurement(const char *file)
 		if (is_ima_template)
 			ima_show(&entry);
 		else
-			ima_ng_show(&entry);
+			ima_ng_show(public_keys, &entry);
 
 		if (!tpmbanks)
 			continue;
@@ -2475,6 +2484,7 @@ out_free:
 	free(pseudo_banks);
 	free(pseudo_padded_banks);
 	free(entry.template);
+	imaevm_free_public_keys(public_keys);
 
 	return err;
 }
-- 
2.39.3

[ima-evm-utils PATCH v2 08/13] Define library ima_calc_hash2() function with a hash algorithm parameter

[Mimi Zohar]

Instead of relying on the "imaevm_params.algo" global variable, which
is not concurrency-safe, define a new library ima_calc_hash2() function
with the hash algorithm as a parameter.

To avoid library incompatibility, make the existing ima_calc_hash()
function a wrapper for ima_calc_hash2().

Deprecate ima_calc_hash().

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/imaevm.h    |  1 +
 src/libimaevm.c | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index 99987140c278..69b6b42806d4 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -237,6 +237,7 @@ extern struct libimaevm_params imaevm_params;
 void imaevm_do_hexdump(FILE *fp, const void *ptr, int len, bool cr);
 void imaevm_hexdump(const void *ptr, int len);
 int ima_calc_hash(const char *file, uint8_t *hash);
+int ima_calc_hash2(const char *file, const char *hash_algo, uint8_t *hash);
 int imaevm_get_hash_algo(const char *algo);
 RSA *read_pub_key(const char *keyfile, int x509);
 EVP_PKEY *read_pub_pkey(const char *keyfile, int x509);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index eeffe97ec8ea..c14ba7632b61 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -181,7 +181,7 @@ out:
 	return err;
 }
 
-int ima_calc_hash(const char *file, uint8_t *hash)
+int ima_calc_hash2(const char *file, const char *hash_algo, uint8_t *hash)
 {
 	const EVP_MD *md;
 	struct stat st;
@@ -202,10 +202,9 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		goto err;
 	}
 
-	md = EVP_get_digestbyname(imaevm_params.hash_algo);
+	md = EVP_get_digestbyname(hash_algo);
 	if (!md) {
-		log_err("EVP_get_digestbyname(%s) failed\n",
-			imaevm_params.hash_algo);
+		log_err("EVP_get_digestbyname(%s) failed\n", hash_algo);
 		err = 1;
 		goto err;
 	}
@@ -246,6 +245,11 @@ err:
 	return err;
 }
 
+__attribute__((deprecated)) int ima_calc_hash(const char *file, uint8_t *hash)
+{
+	return ima_calc_hash2(file, imaevm_params.hash_algo, hash);
+}
+
 EVP_PKEY *read_pub_pkey(const char *keyfile, int x509)
 {
 	FILE *fp;
-- 
2.39.3

[ima-evm-utils PATCH v2 09/13] Use a local hash algorithm variable when verifying file signatures

[Mimi Zohar]

Instead of relying on the "imaevm_params.algo" global variable, which
is not concurrency-safe, define and use a local variable.

Update static verify_hash_v2(), verify_hash_v3(), and verify_hash_common()
function definitions to include a hash algorithm argument.

Similarly update ima_verify_signature2() and ima_calc_hash2() to define
and use a local hash algorithm variable.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/libimaevm.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index c14ba7632b61..10e1ed3eab4d 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -484,7 +484,8 @@ __attribute__((deprecated)) void init_public_keys(const char *keyfiles)
  * (Note: signature_v2_hdr struct does not contain the 'type'.)
  */
 static int verify_hash_common(struct public_key_entry *public_keys,
-			      const char *file, const unsigned char *hash,
+			      const char *file, const char *hash_algo,
+			      const unsigned char *hash,
 			      int size, unsigned char *sig, int siglen)
 {
 	int ret = -1;
@@ -495,7 +496,7 @@ static int verify_hash_common(struct public_key_entry *public_keys,
 	const char *st;
 
 	if (imaevm_params.verbose > LOG_INFO) {
-		log_info("hash(%s): ", imaevm_params.hash_algo);
+		log_info("hash(%s): ", hash_algo);
 		log_dump(hash, size);
 	}
 
@@ -526,7 +527,8 @@ static int verify_hash_common(struct public_key_entry *public_keys,
 	if (!EVP_PKEY_verify_init(ctx))
 		goto err;
 	st = "EVP_get_digestbyname";
-	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
+	md = EVP_get_digestbyname(hash_algo);
+	if (!md)
 		goto err;
 	st = "EVP_PKEY_CTX_set_signature_md";
 	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
@@ -562,11 +564,12 @@ err:
  * Return: 0 verification good, 1 verification bad, -1 error.
  */
 static int verify_hash_v2(struct public_key_entry *public_keys,
-			  const char *file, const unsigned char *hash,
+			  const char *file, const char *hash_algo,
+			  const unsigned char *hash,
 			  int size, unsigned char *sig, int siglen)
 {
 	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
-	return verify_hash_common(public_keys, file, hash, size,
+	return verify_hash_common(public_keys, file, hash_algo, hash, size,
 				  sig + 1, siglen - 1);
 }
 
@@ -577,19 +580,20 @@ static int verify_hash_v2(struct public_key_entry *public_keys,
  * Return: 0 verification good, 1 verification bad, -1 error.
  */
 static int verify_hash_v3(struct public_key_entry *public_keys,
-			  const char *file, const unsigned char *hash,
+			  const char *file, const char *hash_algo,
+			  const unsigned char *hash,
 			  int size, unsigned char *sig, int siglen)
 {
 	unsigned char sigv3_hash[MAX_DIGEST_SIZE];
 	int ret;
 
-	ret = calc_hash_sigv3(sig[0], NULL, hash, sigv3_hash);
+	ret = calc_hash_sigv3(sig[0], hash_algo, hash, sigv3_hash);
 	if (ret < 0)
 		return ret;
 
 	/* note: signature_v2_hdr does not contain 'type', use sig + 1 */
-	return verify_hash_common(public_keys, file, sigv3_hash, size,
-				  sig + 1, siglen - 1);
+	return verify_hash_common(public_keys, file, hash_algo, sigv3_hash,
+				  size, sig + 1, siglen - 1);
 }
 
 #define HASH_MAX_DIGESTSIZE 64	/* kernel HASH_MAX_DIGESTSIZE is 64 bytes */
@@ -632,8 +636,10 @@ int calc_hash_sigv3(enum evm_ima_xattr_type type, const char *algo,
 		return -EINVAL;
 	}
 
-	if (!algo)
-		algo = imaevm_params.hash_algo;
+	if (!algo) {
+		log_err("Hash algorithm unspecified\n");
+		return -EINVAL;
+	}
 
 	if ((hash_algo = imaevm_get_hash_algo(algo)) < 0) {
 		log_err("Hash algorithm %s not supported\n", algo);
@@ -753,10 +759,10 @@ int imaevm_verify_hash(void *public_keys, const char *file,
 		return -1;
 #endif
 	} else if (sig[1] == DIGSIG_VERSION_2) {
-		return verify_hash_v2(public_keys, file, hash, size,
+		return verify_hash_v2(public_keys, file, hash_algo, hash, size,
 				      sig, siglen);
 	} else if (sig[1] == DIGSIG_VERSION_3) {
-		return verify_hash_v3(public_keys, file, hash, size,
+		return verify_hash_v3(public_keys, file, hash_algo, hash, size,
 				      sig, siglen);
 	} else
 		return -1;
@@ -766,8 +772,8 @@ __attribute__((deprecated)) int verify_hash(const char *file,
 					    const unsigned char *hash, int size,
 					    unsigned char *sig, int siglen)
 {
-	return imaevm_verify_hash(g_public_keys, file, NULL, hash, size,
-				  sig, siglen);
+	return imaevm_verify_hash(g_public_keys, file, imaevm_params.hash_algo,
+				  hash, size, sig, siglen);
 }
 
 int ima_verify_signature2(void *public_keys, const char *file,
@@ -776,6 +782,7 @@ int ima_verify_signature2(void *public_keys, const char *file,
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	int hashlen, sig_hash_algo;
+	const char *hash_algo;
 
 	if (sig[0] != EVM_IMA_XATTR_DIGSIG && sig[0] != IMA_VERITY_DIGSIG) {
 		log_err("%s: xattr ima has no signature\n", file);
@@ -793,22 +800,23 @@ int ima_verify_signature2(void *public_keys, const char *file,
 		return -1;
 	}
 	/* Use hash algorithm as retrieved from signature */
-	imaevm_params.hash_algo = imaevm_hash_algo_by_id(sig_hash_algo);
+	hash_algo = imaevm_hash_algo_by_id(sig_hash_algo);
 
 	/*
 	 * Validate the signature based on the digest included in the
 	 * measurement list, not by calculating the local file digest.
 	 */
 	if (digest && digestlen > 0)
-		return imaevm_verify_hash(public_keys, file, NULL, digest,
-					  digestlen, sig, siglen);
+		return imaevm_verify_hash(public_keys, file,
+					  hash_algo, digest, digestlen,
+					  sig, siglen);
 
-	hashlen = ima_calc_hash(file, hash);
+	hashlen = ima_calc_hash2(file, hash_algo, hash);
 	if (hashlen <= 1)
 		return hashlen;
 	assert(hashlen <= sizeof(hash));
 
-	return imaevm_verify_hash(public_keys, file, NULL, hash, hashlen,
+	return imaevm_verify_hash(public_keys, file, hash_algo, hash, hashlen,
 				  sig, siglen);
 }
 
-- 
2.39.3

[ima-evm-utils PATCH v2 10/13] Update EVM signature verification to use a local hash algorithm variable

[Mimi Zohar]

Instead of relying on the "imaevm_params.algo" global variable, which
is not concurrency-safe, define and use a local file hash algorithm
variable.

Update calc_evm_hash(), imaevm_verify_hash().

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 5aea3652c80f..9e1f4e33bc01 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -340,7 +340,8 @@ err:
  * 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)
+static int calc_evm_hash(const char *file, const char *hash_algo,
+			 unsigned char *hash)
 {
         const EVP_MD *md;
 	struct stat st;
@@ -408,10 +409,9 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
 	}
 #endif
 
-	md = EVP_get_digestbyname(imaevm_params.hash_algo);
+	md = EVP_get_digestbyname(hash_algo);
 	if (!md) {
-		log_err("EVP_get_digestbyname(%s) failed\n",
-			imaevm_params.hash_algo);
+		log_err("EVP_get_digestbyname(%s) failed\n", hash_algo);
 		err = 0;
 		goto out;
 	}
@@ -570,7 +570,7 @@ static int sign_evm(const char *file, const char *key)
 	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
-	len = calc_evm_hash(file, hash);
+	len = calc_evm_hash(file, imaevm_params.hash_algo, hash);
 	if (len <= 1)
 		return len;
 	assert(len <= sizeof(hash));
@@ -909,6 +909,7 @@ static int verify_evm(void *public_keys, const char *file)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	unsigned char sig[MAX_SIGNATURE_SIZE];
+	const char *hash_algo = NULL;
 	int sig_hash_algo;
 	int mdlen;
 	int len;
@@ -938,15 +939,15 @@ static int verify_evm(void *public_keys, const char *file)
 		log_err("unknown hash algo: %s\n", file);
 		return -1;
 	}
-	imaevm_params.hash_algo = imaevm_hash_algo_by_id(sig_hash_algo);
+	hash_algo = imaevm_hash_algo_by_id(sig_hash_algo);
 
-	mdlen = calc_evm_hash(file, hash);
+	mdlen = calc_evm_hash(file, hash_algo, hash);
 	if (mdlen <= 1)
 		return mdlen;
 	assert(mdlen <= sizeof(hash));
 
-	return imaevm_verify_hash(public_keys, file, imaevm_params.hash_algo,
-				  hash, mdlen, sig, len);
+	return imaevm_verify_hash(public_keys, file, hash_algo, hash,
+				  mdlen, sig, len);
 }
 
 static int cmd_verify_evm(struct command *cmd)
-- 
2.39.3

[ima-evm-utils PATCH v2 11/13] Use a file specific hash algorithm variable for signing files

[Mimi Zohar]

Instead of relying on the library "imaevm_params.algo" global variable,
which is not concurrency-safe, define and use an evmctl file specific
hash algorithm variable.

Propagate using the file specific hash algorithm variable in sign_evm(),
sign_ima(), hash_ima(), and sign_hash() function.

Replace using the library function ima_calc_hash() with ima_calc_hash2().

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 9e1f4e33bc01..8eb2c46fbff0 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -140,6 +140,7 @@ static bool evm_immutable;
 static bool evm_portable;
 static bool veritysig;
 static bool hwtpm;
+static char *g_hash_algo = DEFAULT_HASH_ALGO;
 
 #define HMAC_FLAG_NO_UUID	0x0001
 #define HMAC_FLAG_CAPS_SET	0x0002
@@ -564,18 +565,18 @@ out:
 	return err;
 }
 
-static int sign_evm(const char *file, const char *key)
+static int sign_evm(const char *file, char *hash_algo, const char *key)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
-	len = calc_evm_hash(file, imaevm_params.hash_algo, hash);
+	len = calc_evm_hash(file, hash_algo, hash);
 	if (len <= 1)
 		return len;
 	assert(len <= sizeof(hash));
 
-	len = sign_hash(imaevm_params.hash_algo, hash, len, key, NULL, sig + 1);
+	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
 	if (len <= 1)
 		return len;
 	assert(len < sizeof(sig));
@@ -609,10 +610,10 @@ static int hash_ima(const char *file)
 {
 	unsigned char hash[MAX_DIGEST_SIZE + 2]; /* +2 byte xattr header */
 	int len, err, offset;
-	int algo = imaevm_get_hash_algo(imaevm_params.hash_algo);
+	int algo = imaevm_get_hash_algo(g_hash_algo);
 
 	if (algo < 0) {
-		log_err("Unknown hash algo: %s\n", imaevm_params.hash_algo);
+		log_err("Unknown hash algo: %s\n", g_hash_algo);
 		return -1;
 	}
 	if (algo > PKEY_HASH_SHA1) {
@@ -624,7 +625,7 @@ static int hash_ima(const char *file)
 		offset = 1;
 	}
 
-	len = ima_calc_hash(file, hash + offset);
+	len = ima_calc_hash2(file, g_hash_algo, hash + offset);
 	if (len <= 1)
 		return len;
 	assert(len + offset <= sizeof(hash));
@@ -632,7 +633,7 @@ static int hash_ima(const char *file)
 	len += offset;
 
 	if (imaevm_params.verbose >= LOG_INFO)
-		log_info("hash(%s): ", imaevm_params.hash_algo);
+		log_info("hash(%s): ", g_hash_algo);
 
 	if (sigdump || imaevm_params.verbose >= LOG_INFO)
 		imaevm_hexdump(hash, len);
@@ -650,18 +651,18 @@ static int hash_ima(const char *file)
 	return 0;
 }
 
-static int sign_ima(const char *file, const char *key)
+static int sign_ima(const char *file, char *hash_algo, const char *key)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
-	len = ima_calc_hash(file, hash);
+	len = ima_calc_hash2(file, hash_algo, hash);
 	if (len <= 1)
 		return len;
 	assert(len <= sizeof(hash));
 
-	len = sign_hash(imaevm_params.hash_algo, hash, len, key, NULL, sig + 1);
+	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
 	if (len <= 1)
 		return len;
 	assert(len < sizeof(sig));
@@ -751,7 +752,7 @@ static int sign_ima_file(const char *file)
 
 	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
 
-	return sign_ima(file, key);
+	return sign_ima(file, g_hash_algo, key);
 }
 
 static int cmd_sign_ima(struct command *cmd)
@@ -854,7 +855,7 @@ static int cmd_sign_hash(struct command *cmd)
 			assert(hashlen / 2 <= sizeof(hash));
 			hex2bin(hash, line, hashlen / 2);
 
-			siglen = sign_hash(imaevm_params.hash_algo, hash,
+			siglen = sign_hash(g_hash_algo, hash,
 					   hashlen / 2, key, NULL, sig + 1);
 			sig[0] = EVM_IMA_XATTR_DIGSIG;
 		}
@@ -874,7 +875,6 @@ static int cmd_sign_hash(struct command *cmd)
 		print_usage(cmd);
 		return -1;
 	}
-
 	return 0;
 }
 
@@ -886,7 +886,7 @@ static int sign_evm_path(const char *file)
 	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
 
 	if (digsig) {
-		err = sign_ima(file, key);
+		err = sign_ima(file, g_hash_algo, key);
 		if (err)
 			return err;
 	}
@@ -897,7 +897,7 @@ static int sign_evm_path(const char *file)
 			return err;
 	}
 
-	return sign_evm(file, key);
+	return sign_evm(file, g_hash_algo, key);
 }
 
 static int cmd_sign_evm(struct command *cmd)
@@ -1426,7 +1426,7 @@ static int cmd_hmac_evm(struct command *cmd)
 	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
 
 	if (digsig) {
-		err = sign_ima(file, key);
+		err = sign_ima(file, g_hash_algo, key);
 		if (err)
 			return err;
 	}
@@ -3087,7 +3087,7 @@ int main(int argc, char *argv[])
 			sigdump = 1;
 			break;
 		case 'a':
-			imaevm_params.hash_algo = optarg;
+			g_hash_algo = optarg;
 			break;
 		case 'p':
 			if (optarg)
-- 
2.39.3

Re: [ima-evm-utils PATCH v2 11/13] Use a file specific hash algorithm variable for signing files

[Stefan Berger]

On 12/6/23 14:27, Mimi Zohar wrote:
> Instead of relying on the library "imaevm_params.algo" global variable,
> which is not concurrency-safe, define and use an evmctl file specific
> hash algorithm variable.
> 
> Propagate using the file specific hash algorithm variable in sign_evm(),
> sign_ima(), hash_ima(), and sign_hash() function.
> 
> Replace using the library function ima_calc_hash() with ima_calc_hash2().
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

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

> ---
>   src/evmctl.c | 34 +++++++++++++++++-----------------
>   1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 9e1f4e33bc01..8eb2c46fbff0 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -140,6 +140,7 @@ static bool evm_immutable;
>   static bool evm_portable;
>   static bool veritysig;
>   static bool hwtpm;
> +static char *g_hash_algo = DEFAULT_HASH_ALGO;
>   
>   #define HMAC_FLAG_NO_UUID	0x0001
>   #define HMAC_FLAG_CAPS_SET	0x0002
> @@ -564,18 +565,18 @@ out:
>   	return err;
>   }
>   
> -static int sign_evm(const char *file, const char *key)
> +static int sign_evm(const char *file, char *hash_algo, const char *key)
>   {
>   	unsigned char hash[MAX_DIGEST_SIZE];
>   	unsigned char sig[MAX_SIGNATURE_SIZE];
>   	int len, err;
>   
> -	len = calc_evm_hash(file, imaevm_params.hash_algo, hash);
> +	len = calc_evm_hash(file, hash_algo, hash);
>   	if (len <= 1)
>   		return len;
>   	assert(len <= sizeof(hash));
>   
> -	len = sign_hash(imaevm_params.hash_algo, hash, len, key, NULL, sig + 1);
> +	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
>   	if (len <= 1)
>   		return len;
>   	assert(len < sizeof(sig));
> @@ -609,10 +610,10 @@ static int hash_ima(const char *file)
>   {
>   	unsigned char hash[MAX_DIGEST_SIZE + 2]; /* +2 byte xattr header */
>   	int len, err, offset;
> -	int algo = imaevm_get_hash_algo(imaevm_params.hash_algo);
> +	int algo = imaevm_get_hash_algo(g_hash_algo);
>   
>   	if (algo < 0) {
> -		log_err("Unknown hash algo: %s\n", imaevm_params.hash_algo);
> +		log_err("Unknown hash algo: %s\n", g_hash_algo);
>   		return -1;
>   	}
>   	if (algo > PKEY_HASH_SHA1) {
> @@ -624,7 +625,7 @@ static int hash_ima(const char *file)
>   		offset = 1;
>   	}
>   
> -	len = ima_calc_hash(file, hash + offset);
> +	len = ima_calc_hash2(file, g_hash_algo, hash + offset);
>   	if (len <= 1)
>   		return len;
>   	assert(len + offset <= sizeof(hash));
> @@ -632,7 +633,7 @@ static int hash_ima(const char *file)
>   	len += offset;
>   
>   	if (imaevm_params.verbose >= LOG_INFO)
> -		log_info("hash(%s): ", imaevm_params.hash_algo);
> +		log_info("hash(%s): ", g_hash_algo);
>   
>   	if (sigdump || imaevm_params.verbose >= LOG_INFO)
>   		imaevm_hexdump(hash, len);
> @@ -650,18 +651,18 @@ static int hash_ima(const char *file)
>   	return 0;
>   }
>   
> -static int sign_ima(const char *file, const char *key)
> +static int sign_ima(const char *file, char *hash_algo, const char *key)
>   {
>   	unsigned char hash[MAX_DIGEST_SIZE];
>   	unsigned char sig[MAX_SIGNATURE_SIZE];
>   	int len, err;
>   
> -	len = ima_calc_hash(file, hash);
> +	len = ima_calc_hash2(file, hash_algo, hash);
>   	if (len <= 1)
>   		return len;
>   	assert(len <= sizeof(hash));
>   
> -	len = sign_hash(imaevm_params.hash_algo, hash, len, key, NULL, sig + 1);
> +	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
>   	if (len <= 1)
>   		return len;
>   	assert(len < sizeof(sig));
> @@ -751,7 +752,7 @@ static int sign_ima_file(const char *file)
>   
>   	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
>   
> -	return sign_ima(file, key);
> +	return sign_ima(file, g_hash_algo, key);
>   }
>   
>   static int cmd_sign_ima(struct command *cmd)
> @@ -854,7 +855,7 @@ static int cmd_sign_hash(struct command *cmd)
>   			assert(hashlen / 2 <= sizeof(hash));
>   			hex2bin(hash, line, hashlen / 2);
>   
> -			siglen = sign_hash(imaevm_params.hash_algo, hash,
> +			siglen = sign_hash(g_hash_algo, hash,
>   					   hashlen / 2, key, NULL, sig + 1);
>   			sig[0] = EVM_IMA_XATTR_DIGSIG;
>   		}
> @@ -874,7 +875,6 @@ static int cmd_sign_hash(struct command *cmd)
>   		print_usage(cmd);
>   		return -1;
>   	}
> -
>   	return 0;
>   }
>   
> @@ -886,7 +886,7 @@ static int sign_evm_path(const char *file)
>   	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
>   
>   	if (digsig) {
> -		err = sign_ima(file, key);
> +		err = sign_ima(file, g_hash_algo, key);
>   		if (err)
>   			return err;
>   	}
> @@ -897,7 +897,7 @@ static int sign_evm_path(const char *file)
>   			return err;
>   	}
>   
> -	return sign_evm(file, key);
> +	return sign_evm(file, g_hash_algo, key);
>   }
>   
>   static int cmd_sign_evm(struct command *cmd)
> @@ -1426,7 +1426,7 @@ static int cmd_hmac_evm(struct command *cmd)
>   	key = imaevm_params.keyfile ? : "/etc/keys/privkey_evm.pem";
>   
>   	if (digsig) {
> -		err = sign_ima(file, key);
> +		err = sign_ima(file, g_hash_algo, key);
>   		if (err)
>   			return err;
>   	}
> @@ -3087,7 +3087,7 @@ int main(int argc, char *argv[])
>   			sigdump = 1;
>   			break;
>   		case 'a':
> -			imaevm_params.hash_algo = optarg;
> +			g_hash_algo = optarg;
>   			break;
>   		case 'p':
>   			if (optarg)

[ima-evm-utils PATCH v2 12/13] Update sign_hash_v*() definition to include the key password

[Mimi Zohar]

The library sign_hash() definition already includes a key password as a
parameter, but it isn't passed on to sign_hash_v*() functions.  Update
the sign_hash_v*() function definitions and callers.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/libimaevm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index 10e1ed3eab4d..9d8f419ae64d 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -1115,7 +1115,8 @@ static int get_hash_algo_v1(const char *algo)
 }
 
 static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
-			int size, const char *keyfile, unsigned char *sig)
+			int size, const char *keyfile, const char *keypass,
+			unsigned char *sig)
 {
 	int len = -1, hashalgo_idx;
 	SHA_CTX ctx;
@@ -1149,7 +1150,7 @@ static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
 	log_info("hash(%s): ", hashalgo);
 	log_dump(hash, size);
 
-	key = read_priv_key(keyfile, imaevm_params.keypass);
+	key = read_priv_key(keyfile, keypass);
 	if (!key)
 		return -1;
 
@@ -1202,7 +1203,8 @@ out:
  * Return: -1 signing error, >0 length of signature
  */
 static int sign_hash_v2(const char *algo, const unsigned char *hash,
-			int size, const char *keyfile, unsigned char *sig)
+			int size, const char *keyfile, const char *keypass,
+			unsigned char *sig)
 {
 	struct signature_v2_hdr *hdr;
 	int len = -1;
@@ -1237,7 +1239,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
 	log_info("hash(%s): ", algo);
 	log_dump(hash, size);
 
-	pkey = read_priv_pkey(keyfile, imaevm_params.keypass);
+	pkey = read_priv_pkey(keyfile, keypass);
 	if (!pkey)
 		return -1;
 
@@ -1307,14 +1309,14 @@ err:
 
 int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig)
 {
-	if (keypass)
-		imaevm_params.keypass = keypass;
+	if (!keypass)	/* Avoid breaking existing libimaevm usage */
+		keypass = imaevm_params.keypass;
 
 	if (imaevm_params.x509)
-		return sign_hash_v2(hashalgo, hash, size, keyfile, sig);
+		return sign_hash_v2(hashalgo, hash, size, keyfile, keypass, sig);
 #if CONFIG_SIGV1
 	else
-		return sign_hash_v1(hashalgo, hash, size, keyfile, sig);
+		return sign_hash_v1(hashalgo, hash, size, keyfile, keypass, sig);
 #endif
 	log_info("Signature version 1 deprecated.");
 	return -1;
-- 
2.39.3

Re: [ima-evm-utils PATCH v2 12/13] Update sign_hash_v*() definition to include the key password

[Stefan Berger]

On 12/6/23 14:27, Mimi Zohar wrote:
> The library sign_hash() definition already includes a key password as a
> parameter, but it isn't passed on to sign_hash_v*() functions.  Update
> the sign_hash_v*() function definitions and callers.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

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

> ---
>   src/libimaevm.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index 10e1ed3eab4d..9d8f419ae64d 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -1115,7 +1115,8 @@ static int get_hash_algo_v1(const char *algo)
>   }
>   
>   static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
> -			int size, const char *keyfile, unsigned char *sig)
> +			int size, const char *keyfile, const char *keypass,
> +			unsigned char *sig)
>   {
>   	int len = -1, hashalgo_idx;
>   	SHA_CTX ctx;
> @@ -1149,7 +1150,7 @@ static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
>   	log_info("hash(%s): ", hashalgo);
>   	log_dump(hash, size);
>   
> -	key = read_priv_key(keyfile, imaevm_params.keypass);
> +	key = read_priv_key(keyfile, keypass);
>   	if (!key)
>   		return -1;
>   
> @@ -1202,7 +1203,8 @@ out:
>    * Return: -1 signing error, >0 length of signature
>    */
>   static int sign_hash_v2(const char *algo, const unsigned char *hash,
> -			int size, const char *keyfile, unsigned char *sig)
> +			int size, const char *keyfile, const char *keypass,
> +			unsigned char *sig)
>   {
>   	struct signature_v2_hdr *hdr;
>   	int len = -1;
> @@ -1237,7 +1239,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>   	log_info("hash(%s): ", algo);
>   	log_dump(hash, size);
>   
> -	pkey = read_priv_pkey(keyfile, imaevm_params.keypass);
> +	pkey = read_priv_pkey(keyfile, keypass);
>   	if (!pkey)
>   		return -1;
>   
> @@ -1307,14 +1309,14 @@ err:
>   
>   int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig)
>   {
> -	if (keypass)
> -		imaevm_params.keypass = keypass;
> +	if (!keypass)	/* Avoid breaking existing libimaevm usage */
> +		keypass = imaevm_params.keypass;
>   
>   	if (imaevm_params.x509)
> -		return sign_hash_v2(hashalgo, hash, size, keyfile, sig);
> +		return sign_hash_v2(hashalgo, hash, size, keyfile, keypass, sig);
>   #if CONFIG_SIGV1
>   	else
> -		return sign_hash_v1(hashalgo, hash, size, keyfile, sig);
> +		return sign_hash_v1(hashalgo, hash, size, keyfile, keypass, sig);
>   #endif
>   	log_info("Signature version 1 deprecated.");
>   	return -1;

[ima-evm-utils PATCH v2 13/13] Define and use a file specific "keypass" variable

[Mimi Zohar]

Instead of relying on the "imaevm_parrams.keypass" global variable,
which is not concurrency-safe, define and use a file specific variable.

To avoid library incompatibility, don't remove imaevm_params.keypass
variable.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/evmctl.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 8eb2c46fbff0..72b800f6884c 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -141,6 +141,7 @@ static bool evm_portable;
 static bool veritysig;
 static bool hwtpm;
 static char *g_hash_algo = DEFAULT_HASH_ALGO;
+static char *g_keypass;
 
 #define HMAC_FLAG_NO_UUID	0x0001
 #define HMAC_FLAG_CAPS_SET	0x0002
@@ -576,7 +577,7 @@ static int sign_evm(const char *file, char *hash_algo, const char *key)
 		return len;
 	assert(len <= sizeof(hash));
 
-	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
+	len = sign_hash(hash_algo, hash, len, key, g_keypass, sig + 1);
 	if (len <= 1)
 		return len;
 	assert(len < sizeof(sig));
@@ -662,7 +663,7 @@ static int sign_ima(const char *file, char *hash_algo, const char *key)
 		return len;
 	assert(len <= sizeof(hash));
 
-	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
+	len = sign_hash(hash_algo, hash, len, key, g_keypass, sig + 1);
 	if (len <= 1)
 		return len;
 	assert(len < sizeof(sig));
@@ -844,7 +845,7 @@ static int cmd_sign_hash(struct command *cmd)
 			}
 
 			siglen = sign_hash(algo, sigv3_hash, hashlen / 2,
-					   key, NULL, sig + 1);
+					   key, g_keypass, sig + 1);
 
 			sig[0] = IMA_VERITY_DIGSIG;
 			sig[1] = DIGSIG_VERSION_3;	/* sigv3 */
@@ -856,7 +857,7 @@ static int cmd_sign_hash(struct command *cmd)
 			hex2bin(hash, line, hashlen / 2);
 
 			siglen = sign_hash(g_hash_algo, hash,
-					   hashlen / 2, key, NULL, sig + 1);
+					   hashlen / 2, key, g_keypass, sig + 1);
 			sig[0] = EVM_IMA_XATTR_DIGSIG;
 		}
 
@@ -3091,9 +3092,9 @@ int main(int argc, char *argv[])
 			break;
 		case 'p':
 			if (optarg)
-				imaevm_params.keypass = optarg;
+				g_keypass = optarg;
 			else
-				imaevm_params.keypass = get_password();
+				g_keypass = get_password();
 			break;
 		case 'f':
 			sigfile = 1;
@@ -3235,8 +3236,8 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	if (!imaevm_params.keypass)
-		imaevm_params.keypass = getenv("EVMCTL_KEY_PASSWORD");
+	if (!g_keypass)
+		g_keypass = getenv("EVMCTL_KEY_PASSWORD");
 
 	if (imaevm_params.keyfile != NULL &&
 	    imaevm_params.eng == NULL &&
-- 
2.39.3

Re: [ima-evm-utils PATCH v2 13/13] Define and use a file specific "keypass" variable

[Stefan Berger]

On 12/6/23 14:27, Mimi Zohar wrote:
> Instead of relying on the "imaevm_parrams.keypass" global variable,
> which is not concurrency-safe, define and use a file specific variable.
> 
> To avoid library incompatibility, don't remove imaevm_params.keypass
> variable.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>

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


> ---
>   src/evmctl.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 8eb2c46fbff0..72b800f6884c 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -141,6 +141,7 @@ static bool evm_portable;
>   static bool veritysig;
>   static bool hwtpm;
>   static char *g_hash_algo = DEFAULT_HASH_ALGO;
> +static char *g_keypass;
>   
>   #define HMAC_FLAG_NO_UUID	0x0001
>   #define HMAC_FLAG_CAPS_SET	0x0002
> @@ -576,7 +577,7 @@ static int sign_evm(const char *file, char *hash_algo, const char *key)
>   		return len;
>   	assert(len <= sizeof(hash));
>   
> -	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
> +	len = sign_hash(hash_algo, hash, len, key, g_keypass, sig + 1);
>   	if (len <= 1)
>   		return len;
>   	assert(len < sizeof(sig));
> @@ -662,7 +663,7 @@ static int sign_ima(const char *file, char *hash_algo, const char *key)
>   		return len;
>   	assert(len <= sizeof(hash));
>   
> -	len = sign_hash(hash_algo, hash, len, key, NULL, sig + 1);
> +	len = sign_hash(hash_algo, hash, len, key, g_keypass, sig + 1);
>   	if (len <= 1)
>   		return len;
>   	assert(len < sizeof(sig));
> @@ -844,7 +845,7 @@ static int cmd_sign_hash(struct command *cmd)
>   			}
>   
>   			siglen = sign_hash(algo, sigv3_hash, hashlen / 2,
> -					   key, NULL, sig + 1);
> +					   key, g_keypass, sig + 1);
>   
>   			sig[0] = IMA_VERITY_DIGSIG;
>   			sig[1] = DIGSIG_VERSION_3;	/* sigv3 */
> @@ -856,7 +857,7 @@ static int cmd_sign_hash(struct command *cmd)
>   			hex2bin(hash, line, hashlen / 2);
>   
>   			siglen = sign_hash(g_hash_algo, hash,
> -					   hashlen / 2, key, NULL, sig + 1);
> +					   hashlen / 2, key, g_keypass, sig + 1);
>   			sig[0] = EVM_IMA_XATTR_DIGSIG;
>   		}
>   
> @@ -3091,9 +3092,9 @@ int main(int argc, char *argv[])
>   			break;
>   		case 'p':
>   			if (optarg)
> -				imaevm_params.keypass = optarg;
> +				g_keypass = optarg;
>   			else
> -				imaevm_params.keypass = get_password();
> +				g_keypass = get_password();
>   			break;
>   		case 'f':
>   			sigfile = 1;
> @@ -3235,8 +3236,8 @@ int main(int argc, char *argv[])
>   		}
>   	}
>   
> -	if (!imaevm_params.keypass)
> -		imaevm_params.keypass = getenv("EVMCTL_KEY_PASSWORD");
> +	if (!g_keypass)
> +		g_keypass = getenv("EVMCTL_KEY_PASSWORD");
>   
>   	if (imaevm_params.keyfile != NULL &&
>   	    imaevm_params.eng == NULL &&