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

[ima-evm-utils PATCH v3 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.

The deprecated function warnings can be disabled by specifying
'-Wno-deprecated-declarations'.  Allow suppressing just the libimevm
warnings by enabling IMAEVM_SUPPRESS_DEPRECATED.

e.g. configure CFLAGS="-DIMAEVM_SUPPRESS_DEPRECATED"

Changelog v3:
Addressed Stefan Berger's comments:
- Forward declare "struct public_key_entry" in order to replace
  "void *public_key" usage.
- Move the "__attribute__((deprecated))" from the functions to the function
  definitions.
- Allow suppressing just the libimevm deprecated warngings by defining
  and enabling "-DIMAEVM_SUPPRESS_DEPRECATED".
- Add "Reviewed-by" tags.

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    | 136 +++++++++++++++++++++++++++-----------------
 src/imaevm.h    |  29 ++++++++--
 src/libimaevm.c | 148 +++++++++++++++++++++++++++++++++++-------------
 3 files changed, 218 insertions(+), 95 deletions(-)

-- 
2.39.3

[ima-evm-utils PATCH v3 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 v3 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    |  2 ++
 src/libimaevm.c | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/src/imaevm.h b/src/imaevm.h
index 18d7b0e447e1..64f7db79b33a 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -233,6 +233,7 @@ struct RSA_ASN1_template {
 #define DEFAULT_PCR 10
 
 extern struct libimaevm_params imaevm_params;
+struct public_key_entry;
 
 void imaevm_do_hexdump(FILE *fp, const void *ptr, int len, bool cr);
 void imaevm_hexdump(const void *ptr, int len);
@@ -250,6 +251,7 @@ int sign_hash(const char *algo, const unsigned char *hash, int size, const char
 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 init_public_keys(const char *keyfiles);
+void imaevm_free_public_keys(struct public_key_entry *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 244774d01805..61f91df02460 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(struct public_key_entry *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 v3 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.

Allow suppressing just the libimevm deprecate warnings by enabling
IMAEVM_SUPPRESS_DEPRECATED.
    e.g. configure CFLAGS="-DIMAEVM_SUPPRESS_DEPRECATED"

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 src/imaevm.h    | 21 +++++++++++--
 src/libimaevm.c | 82 ++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 78 insertions(+), 25 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index 64f7db79b33a..4fd421f5cd1d 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -232,6 +232,12 @@ struct RSA_ASN1_template {
 #define	NUM_PCRS 24
 #define DEFAULT_PCR 10
 
+#ifdef IMAEVM_SUPPRESS_DEPRECATED
+#define IMAEVM_DEPRECATED
+#else
+#define IMAEVM_DEPRECATED __attribute__ ((deprecated))
+#endif
+
 extern struct libimaevm_params imaevm_params;
 struct public_key_entry;
 
@@ -248,9 +254,18 @@ int key2bin(RSA *key, unsigned char *pub);
 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 init_public_keys(const char *keyfiles);
+IMAEVM_DEPRECATED int verify_hash(const char *file, const unsigned char *hash,
+				  int size, unsigned char *sig, int siglen);
+IMAEVM_DEPRECATED int ima_verify_signature(const char *file, unsigned char *sig,
+					   int siglen, unsigned char *digest,
+					   int digestlen);
+IMAEVM_DEPRECATED void init_public_keys(const char *keyfiles);
+
+int ima_verify_signature2(struct public_key_entry *public_keys, const char *file,
+			  unsigned char *sig, int siglen,
+			  unsigned char *digest, int digestlen);
+int imaevm_init_public_keys(const char *keyfiles,
+			    struct public_key_entry **public_keys);
 void imaevm_free_public_keys(struct public_key_entry *public_keys);
 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 61f91df02460..9cc83e071610 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 = entry;
 	log_err("key %d: %x (unknown keyid)\n", i, __be32_to_cpup(&keyid));
 	return 0;
 }
@@ -412,7 +413,8 @@ void imaevm_free_public_keys(struct public_key_entry *public_keys)
 	}
 }
 
-void init_public_keys(const char *keyfiles)
+int imaevm_init_public_keys(const char *keyfiles,
+			    struct public_key_entry **public_keys)
 {
 	struct public_key_entry *entry;
 	char *tmp_keyfiles, *keyfiles_free;
@@ -420,6 +422,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 +451,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 = *public_keys;
+		*public_keys = entry;
 	}
+
 	free(keyfiles_free);
 	if (err < 0)
-		imaevm_free_public_keys(g_public_keys);
+		imaevm_free_public_keys(*public_keys);
+	return err;
+}
+
+void init_public_keys(const char *keyfiles)
+{
+	imaevm_init_public_keys(keyfiles, &g_public_keys);
 }
 
 /*
@@ -466,7 +480,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 +496,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 +558,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 +573,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 +585,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 +729,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 +750,24 @@ 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)
+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(struct public_key_entry *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,23 @@ 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);
+}
+
+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 v3 03/13] Update library function definitions to include a "public_keys" parameter

[Stefan Berger]

On 1/4/24 14:05, 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.
> 
> Allow suppressing just the libimevm deprecate warnings by enabling
> IMAEVM_SUPPRESS_DEPRECATED.
>      e.g. configure CFLAGS="-DIMAEVM_SUPPRESS_DEPRECATED"
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   src/imaevm.h    | 21 +++++++++++--
>   src/libimaevm.c | 82 ++++++++++++++++++++++++++++++++++++-------------
>   2 files changed, 78 insertions(+), 25 deletions(-)
> 


> @@ -710,8 +729,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,

Replace void with struct public_key_entry.

With this nit fixed:

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

[ima-evm-utils PATCH v3 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 | 15 ++++++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index 4fd421f5cd1d..0b86d28944b3 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -261,6 +261,9 @@ IMAEVM_DEPRECATED int ima_verify_signature(const char *file, unsigned char *sig,
 					   int digestlen);
 IMAEVM_DEPRECATED void init_public_keys(const char *keyfiles);
 
+int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
+		       const char *hash_algo, const unsigned char *hash,
+		       int size, unsigned char *sig, int siglen);
 int ima_verify_signature2(struct public_key_entry *public_keys, const char *file,
 			  unsigned char *sig, int siglen,
 			  unsigned char *digest, int digestlen);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 9cc83e071610..a5e9fd5080ac 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -729,9 +729,9 @@ int imaevm_hash_algo_from_sig(unsigned char *sig)
 		return -1;
 }
 
-int imaevm_verify_hash(void *public_keys, const char *file,
-		       const unsigned char *hash, int size,
-		       unsigned char *sig, int siglen)
+int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
+		       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 @@ int imaevm_verify_hash(void *public_keys, const char *file,
 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(struct public_key_entry *public_keys, const char *file,
@@ -795,15 +796,15 @@ int ima_verify_signature2(struct public_key_entry *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

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

[Stefan Berger]

On 1/4/24 14:05, Mimi Zohar wrote:
> 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 | 15 ++++++++-------
>   2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/imaevm.h b/src/imaevm.h
> index 4fd421f5cd1d..0b86d28944b3 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -261,6 +261,9 @@ IMAEVM_DEPRECATED int ima_verify_signature(const char *file, unsigned char *sig,
>   					   int digestlen);
>   IMAEVM_DEPRECATED void init_public_keys(const char *keyfiles);
>   
> +int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
> +		       const char *hash_algo, const unsigned char *hash,
> +		       int size, unsigned char *sig, int siglen);
>   int ima_verify_signature2(struct public_key_entry *public_keys, const char *file,
>   			  unsigned char *sig, int siglen,
>   			  unsigned char *digest, int digestlen);
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index 9cc83e071610..a5e9fd5080ac 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -729,9 +729,9 @@ int imaevm_hash_algo_from_sig(unsigned char *sig)
>   		return -1;
>   }
>   
> -int imaevm_verify_hash(void *public_keys, const char *file,
> -		       const unsigned char *hash, int size,
> -		       unsigned char *sig, int siglen)
> +int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
> +		       const char *hash_algo, const unsigned char *hash,
> +		       int size, unsigned char *sig, int siglen)

With patch 3 fixed, this will disappear.

>   {
>   	/* Get signature type from sig header */
>   	if (sig[1] == DIGSIG_VERSION_1) {
> @@ -762,7 +762,8 @@ int imaevm_verify_hash(void *public_keys, const char *file,
>   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(struct public_key_entry *public_keys, const char *file,
> @@ -795,15 +796,15 @@ int ima_verify_signature2(struct public_key_entry *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);
>   }
>   

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

[ima-evm-utils PATCH v3 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..5d84a41a0762 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(struct public_key_entry *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)
 {
+	struct public_key_entry *public_keys = NULL;
 	char *file = g_argv[optind++];
 	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 v3 05/13] Update cmd_verify_ima() to define and use a local list of public keys

[Stefan Berger]

On 1/4/24 14:05, 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>

Reviewed-by: Stefan Berger <stefanb@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..5d84a41a0762 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(struct public_key_entry *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)
>   {
> +	struct public_key_entry *public_keys = NULL;
>   	char *file = g_argv[optind++];
>   	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;
>   }
>   

[ima-evm-utils PATCH v3 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 5d84a41a0762..29c4d1dc1f0d 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(struct public_key_entry *public_keys, const char *file)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
 	unsigned char sig[MAX_SIGNATURE_SIZE];
@@ -945,11 +945,13 @@ 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)
 {
+	struct public_key_entry *public_keys = NULL;
 	char *file = g_argv[optind++];
 	int err;
 
@@ -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

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

[Stefan Berger]

On 1/4/24 14:05, Mimi Zohar wrote:
> 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>

Reviewed-by: Stefan Berger <stefanb@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 5d84a41a0762..29c4d1dc1f0d 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(struct public_key_entry *public_keys, const char *file)
>   {
>   	unsigned char hash[MAX_DIGEST_SIZE];
>   	unsigned char sig[MAX_SIGNATURE_SIZE];
> @@ -945,11 +945,13 @@ 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)
>   {
> +	struct public_key_entry *public_keys = NULL;
>   	char *file = g_argv[optind++];
>   	int err;
>   
> @@ -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;
>   }
>   

[ima-evm-utils PATCH v3 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 | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 29c4d1dc1f0d..2ccaaf244aa9 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1625,7 +1625,8 @@ static int lookup_template_name_entry(char *template_name)
 	return 0;
 }
 
-void ima_ng_show(struct template_entry *entry)
+static void ima_ng_show(struct public_key_entry *public_keys,
+			struct template_entry *entry)
 {
 	uint8_t *fieldp = entry->template;
 	uint32_t field_len;
@@ -1751,10 +1752,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);
@@ -2225,6 +2228,7 @@ static int read_tpm_banks(int num_banks, struct tpm_bank_info *bank)
 
 static int ima_measurement(const char *file)
 {
+	struct public_key_entry *public_keys = NULL;
 	struct tpm_bank_info *pseudo_padded_banks;
 	struct tpm_bank_info *pseudo_banks = NULL;
 	struct tpm_bank_info *tpm_banks = NULL;
@@ -2263,10 +2267,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 +2426,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 +2485,7 @@ out_free:
 	free(pseudo_banks);
 	free(pseudo_padded_banks);
 	free(entry.template);
+	imaevm_free_public_keys(public_keys);
 
 	return err;
 }
-- 
2.39.3

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

[Stefan Berger]

On 1/4/24 14:05, Mimi Zohar wrote:
> 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>

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

[ima-evm-utils PATCH v3 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    |  3 ++-
 src/libimaevm.c | 12 ++++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index 0b86d28944b3..8e24f08bbddc 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -243,7 +243,6 @@ struct public_key_entry;
 
 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 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);
@@ -254,6 +253,7 @@ int key2bin(RSA *key, unsigned char *pub);
 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);
+IMAEVM_DEPRECATED int ima_calc_hash(const char *file, uint8_t *hash);
 IMAEVM_DEPRECATED int verify_hash(const char *file, const unsigned char *hash,
 				  int size, unsigned char *sig, int siglen);
 IMAEVM_DEPRECATED int ima_verify_signature(const char *file, unsigned char *sig,
@@ -261,6 +261,7 @@ IMAEVM_DEPRECATED int ima_verify_signature(const char *file, unsigned char *sig,
 					   int digestlen);
 IMAEVM_DEPRECATED void init_public_keys(const char *keyfiles);
 
+int ima_calc_hash2(const char *file, const char *hash_algo, uint8_t *hash);
 int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
 		       const char *hash_algo, const unsigned char *hash,
 		       int size, unsigned char *sig, int siglen);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index a5e9fd5080ac..214c656d6eba 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;
 }
 
+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 v3 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 214c656d6eba..48bce59fba43 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -485,7 +485,8 @@ 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;
@@ -496,7 +497,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);
 	}
 
@@ -527,7 +528,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))
@@ -563,11 +565,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);
 }
 
@@ -578,19 +581,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 */
@@ -633,8 +637,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);
@@ -754,10 +760,10 @@ int imaevm_verify_hash(struct public_key_entry *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 @@ int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
 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(struct public_key_entry *public_keys, const char *file,
@@ -776,6 +782,7 @@ int ima_verify_signature2(struct public_key_entry *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(struct public_key_entry *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

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

[Stefan Berger]

On 1/4/24 14:05, Mimi Zohar wrote:
> 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>

Reviewed-by: Stefan Berger <stefanb@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 214c656d6eba..48bce59fba43 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -485,7 +485,8 @@ 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;
> @@ -496,7 +497,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);
>   	}
>   
> @@ -527,7 +528,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))
> @@ -563,11 +565,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);
>   }
>   
> @@ -578,19 +581,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 */
> @@ -633,8 +637,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);
> @@ -754,10 +760,10 @@ int imaevm_verify_hash(struct public_key_entry *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 @@ int imaevm_verify_hash(struct public_key_entry *public_keys, const char *file,
>   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(struct public_key_entry *public_keys, const char *file,
> @@ -776,6 +782,7 @@ int ima_verify_signature2(struct public_key_entry *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(struct public_key_entry *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);
>   }
>   

[ima-evm-utils PATCH v3 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 2ccaaf244aa9..3eb995031722 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(struct public_key_entry *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(struct public_key_entry *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 v3 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().

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
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 3eb995031722..37441b1b77ea 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;
 	}
@@ -3088,7 +3088,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

[ima-evm-utils PATCH v3 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.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
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 48bce59fba43..ce4f6f73097d 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -1112,7 +1112,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;
@@ -1146,7 +1147,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;
 
@@ -1199,7 +1200,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;
@@ -1234,7 +1236,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;
 
@@ -1304,14 +1306,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

[ima-evm-utils PATCH v3 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.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
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 37441b1b77ea..d050b5e36765 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;
 		}
 
@@ -3092,9 +3093,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;
@@ -3236,8 +3237,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