ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user evmctl ima_verify -k <public key> ./foo.txt --xattr-user

Mimi

Quoted 132 lines

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Mimi,

Quoted 7 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

Quoted 2 lines

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 138 lines

evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user evmctl ima_verify -k <public key> ./foo.txt --xattr-user

Mimi

Quoted 132 lines

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Quoted 8 lines · Vitaly Chikunov

Quoted 7 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

Quoted 8 lines

Quoted 2 lines

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

Mimi

Quoted 139 lines

Quoted 138 lines

evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user evmctl ima_verify -k <public key> ./foo.txt --xattr-user

Mimi

Quoted 132 lines

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Mimi,

Quoted 18 lines · Mimi Zohar

Quoted 8 lines · Vitaly Chikunov

Quoted 7 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

Quoted 8 lines

Quoted 2 lines

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

I was not found this mistake at the time, since Streebog should only be used in ima_hash and not in ima_sign/verify (which requires RSA support which I intentionally didn't add, planning to add EC-RDSA support later), so that code path was not tested.

Quoted 143 lines

Mimi

Quoted 139 lines

Quoted 138 lines

evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user evmctl ima_verify -k <public key> ./foo.txt --xattr-user

Mimi

Quoted 132 lines

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Quoted 19 lines · Vitaly Chikunov

Quoted 18 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

Quoted 9 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 8 lines

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

Quoted 5 lines

I was not found this mistake at the time, since Streebog should only be used in ima_hash and not in ima_sign/verify (which requires RSA support which I intentionally didn't add, planning to add EC-RDSA support later), so that code path was not tested.

This copy-paste of the fix form "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API":

(quoted diff) +6 -2

@@ -159,8 +102,12 @@ void dump(const void *ptr, int len)
 
 const char *get_hash_algo_by_id(int algo)
 {
-       if (algo < PKEY_HASH__LAST)
-           return pkey_hash_algo[algo];
+       if (algo < PKEY_HASH__LAST) {
+               const char *name = pkey_hash_algo[algo];
+               const char *last = strrchr(name, ',');
+
+           return last ? last + 1 : name;
+       }
        if (algo < HASH_ALGO__LAST)
            return hash_algo_name[algo];
 

Quoted 143 lines

Quoted 143 lines

Mimi

Quoted 139 lines

Quoted 138 lines

evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt --xattr-user evmctl ima_verify -k <public key> ./foo.txt --xattr-user

Mimi

Quoted 132 lines

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- New patch.
Changes since v2:
- No changes.

 src/imaevm.h    |  1 +
 src/libimaevm.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 4 deletions(-)

src/imaevm.h +1 -0

index c81bf21..795966a 100644
@@ -75,6 +75,7 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_NAME		32
 #define MAX_DIGEST_SIZE		64
 #define MAX_SIGNATURE_SIZE	1024
 

src/libimaevm.c +51 -4

index cb4721b..7501303 100644
@@ -41,6 +41,7 @@
 /* should we use logger instead for library? */
 #define USE_FPRINTF
 
+#define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>

@@ -56,6 +57,7 @@
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
+#include <openssl/engine.h>
 #include <openssl/err.h>
 
 #include "imaevm.h"

@@ -70,8 +72,8 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_SHA384]	= "sha384",
 	[PKEY_HASH_SHA512]	= "sha512",
 	[PKEY_HASH_SHA224]	= "sha224",
-	[PKEY_HASH_STREEBOG_256] = "streebog256",
-	[PKEY_HASH_STREEBOG_512] = "streebog512",
+	[PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
+	[PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
 };
 
 /*

@@ -284,6 +286,35 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
 	return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
 }
 
+/* Call EVP_get_digestbyname with provided name and with alias,
+ * which is first name in the comma separated list of names
+ * in pkey_hash_algo.
+ */
+static const EVP_MD *get_digestbyname(const char *name)
+{
+	const EVP_MD *md;
+
+	md = EVP_get_digestbyname(params.hash_algo);
+	if (!md) {
+		char alias[MAX_DIGEST_NAME];
+		int len;
+		int algo_id;
+		const char *algo_alias;
+
+		/* try to get algo by its alias */
+		algo_id = get_hash_algo(params.hash_algo);
+		algo_alias = get_hash_algo_by_id(algo_id);
+		len = strchrnul(algo_alias, ',') - algo_alias;
+		if (len < sizeof(alias)) {
+			memcpy(alias, algo_alias, len);
+			alias[len] = '\0';
+			if (strcmp(params.hash_algo, alias))
+				md = EVP_get_digestbyname(alias);
+		}
+	}
+	return md;
+}
+
 int ima_calc_hash(const char *file, uint8_t *hash)
 {
 	const EVP_MD *md;

@@ -305,7 +336,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 		return err;
 	}
 
-	md = EVP_get_digestbyname(params.hash_algo);
+	md = get_digestbyname(params.hash_algo);
 	if (!md) {
 		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;

@@ -561,6 +592,22 @@ static int algocmp(const char *a, const char *b)
 	return *a || *b;
 }
 
+static int strmatch(const char *needle, const char *haystack)
+{
+	int len = strlen(needle);
+	const char *p = haystack;
+	const char *t;
+
+	for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
+		if (t - p == len &&
+		    !strncmp(needle, p, len))
+			return 0;
+		if (!*t++)
+			break;
+	}
+	return 1;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;

@@ -568,7 +615,7 @@ int get_hash_algo(const char *algo)
 	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
-		    !strcmp(algo, pkey_hash_algo[i]))
+		    !strmatch(algo, pkey_hash_algo[i]))
 			return i;
 
 	/* iterate over algorithms provided by kernel-headers */

Mimi,

Quoted 20 lines · Vitaly Chikunov

Quoted 19 lines · Vitaly Chikunov

Quoted 18 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

Quoted 9 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

Or you may suggest simpler approach.

Basically, we need to resole both text strings into the same PKEY_HASH_ id, allow any of the string pass into EVP_get_digestbyname, and resolve PKEY_HASH_ id back into the correct hash name. Optionally, allow user to specify new hash name on older openssl/gost-engine. This is all implemented by that patch, it was not just overly complicated "hash algorithm comparison".

My wish is we retain support of older openssl/gost-engine.

Quoted 39 lines · Vitaly Chikunov

Quoted 20 lines · Vitaly Chikunov

Quoted 19 lines · Vitaly Chikunov

Quoted 18 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

Quoted 9 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

Or you may suggest simpler approach.

Basically, we need to resole both text strings into the same PKEY_HASH_ id, allow any of the string pass into EVP_get_digestbyname, and resolve PKEY_HASH_ id back into the correct hash name. Optionally, allow user to specify new hash name on older openssl/gost-engine. This is all implemented by that patch, it was not just overly complicated "hash algorithm comparison".

Quoted 1 line

My wish is we retain support of older openssl/gost-engine.

The following seems to fix the problem, but instead of adding it in strmatch(), as below, I would add it before the strmatch() call.  Then strmatch is only called as needed.

src/libimaevm.c +3 -0

index d9ffa13befb0..7901215da655 100644
@@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
        const char *p = haystack;
        const char *t;
 
+       if (strcmp(needle, haystack) == 0)
+               return 0;
+
        for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
                if (t - p == len &&
                    !strncmp(needle, p, len))

Mimi

Mimi,

Quoted 40 lines · Mimi Zohar

Quoted 39 lines · Vitaly Chikunov

Quoted 20 lines · Vitaly Chikunov

Quoted 19 lines · Vitaly Chikunov

Quoted 18 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

Quoted 9 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

Or you may suggest simpler approach.

Basically, we need to resole both text strings into the same PKEY_HASH_ id, allow any of the string pass into EVP_get_digestbyname, and resolve PKEY_HASH_ id back into the correct hash name. Optionally, allow user to specify new hash name on older openssl/gost-engine. This is all implemented by that patch, it was not just overly complicated "hash algorithm comparison".

Quoted 5 lines

Quoted 1 line

My wish is we retain support of older openssl/gost-engine.

The following seems to fix the problem, but instead of adding it in strmatch(), as below, I would add it before the strmatch() call.  Then strmatch is only called as needed.

IC.

I will try to conceptually simplify the code and prepare another patch. Probably, I will split pkey_hash_algop[] into two arrays, simple one where strcmp() in the loop is used, and complex one for hash names with aliases. I will try to replace strmatch() with just simple string array loop.

Quoted 17 lines

src/libimaevm.c +3 -0

index d9ffa13befb0..7901215da655 100644
@@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
        const char *p = haystack;
        const char *t;
 
+       if (strcmp(needle, haystack) == 0)
+               return 0;
+
        for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
                if (t - p == len &&
                    !strncmp(needle, p, len))

Mimi

Quoted 45 lines · Vitaly Chikunov

Mimi,

Quoted 42 lines · Mimi Zohar

Quoted 40 lines · Vitaly Chikunov

Quoted 20 lines · Vitaly Chikunov

Quoted 19 lines · Vitaly Chikunov

Quoted 18 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

Quoted 9 lines · Mimi Zohar

Quoted 5 lines · Vitaly Chikunov

For compatibility with older OpenSSL try to load digest by its alias if load by its proper name is failed.

This is configured in pkey_hash_algo by mentioning loadable alias first in the comma separated list of algo names.

After this patch, I can not verify the signature.  It's failing to find the hash algorithm.

Yes, it's fixed in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY API". I didn't consider it a problem since streebog256 should not work for sign/verify anyway (since RSA should not support it). That EVP_PKEY patch which adds ability to sign/verify for Streebog also fixes above problem.

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

I've been having second thoughts about this patch in general, as it made the hash algorithm comparison unnecessarily more complex for the simple case.  As there hasn't been a new ima-evm-utils release with patch, perhaps we should simple remove/revert it?

It was only for compatibility with older openssl/gost-engine, where "streebog256" name is not defined yet. If you don't want to allow that users to use Streebog feel free to revert it.

Or you may suggest simpler approach.

Basically, we need to resole both text strings into the same PKEY_HASH_ id, allow any of the string pass into EVP_get_digestbyname, and resolve PKEY_HASH_ id back into the correct hash name. Optionally, allow user to specify new hash name on older openssl/gost-engine. This is all implemented by that patch, it was not just overly complicated "hash algorithm comparison".

My wish is we retain support of older openssl/gost-engine.

Quoted 5 lines

Quoted 3 lines

The following seems to fix the problem, but instead of adding it in strmatch(), as below, I would add it before the strmatch() call.  Then strmatch is only called as needed.

IC.

The patch below isn't addressing old vs. new naming, but fixes

"ima_verify".  Adding the following line, results in "strmatch: algo md_gost12_256,streebog256".  It's trying to match the entire string,

but fails without the strcmp().

+       log_info("strmatch: algo %s \n", needle);

Mimi

Quoted 24 lines

I will try to conceptually simplify the code and prepare another patch. Probably, I will split pkey_hash_algop[] into two arrays, simple one where strcmp() in the loop is used, and complex one for hash names with aliases. I will try to replace strmatch() with just simple string array loop.

Quoted 17 lines

src/libimaevm.c +3 -0

index d9ffa13befb0..7901215da655 100644
@@ -598,6 +598,9 @@ static int strmatch(const char *needle, const char *haystack)
        const char *p = haystack;
        const char *t;
 
+       if (strcmp(needle, haystack) == 0)
+               return 0;
+
        for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
                if (t - p == len &&
                    !strncmp(needle, p, len))

Mimi

Commit ae1319eeabd6 ("Remove hardcoding of SHA1 in EVM signatures") introduces overflow of 20 byte buffer on the stack while calculating hash. Also, invalid hash length is passed to the underlying verification function in verify_evm. This prevents any non-SHA1 hashes from being properly validated using evmctl.

Fixes ae1319eeabd6 ("Remove hardcoding of SHA1 in EVM signatures")

Signed-off-by Vitaly Chikunov <[email protected]>

---
Changes since v1:
- Fix similar issue in hmac_evm
Changes since v2:
- No changes.

 src/evmctl.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

src/evmctl.c +10 -6

index 1b46d58..f8035da 100644
@@ -55,6 +55,7 @@
 #include <keyutils.h>
 #include <ctype.h>
 #include <termios.h>
+#include <assert.h>
 
 #include <openssl/sha.h>
 #include <openssl/pem.h>

@@ -760,13 +761,15 @@ static int cmd_sign_evm(struct command *cmd)
 
 static int verify_evm(const char *file)
 {
-	unsigned char hash[20];
+	unsigned char hash[64];
 	unsigned char sig[1024];
+	int mdlen;
 	int len;
 
-	len = calc_evm_hash(file, hash);
-	if (len <= 1)
-		return len;
+	mdlen = calc_evm_hash(file, hash);
+	assert(mdlen <= sizeof(hash));
+	if (mdlen <= 1)
+		return mdlen;
 
 	len = lgetxattr(file, "security.evm", sig, sizeof(sig));
 	if (len < 0) {

@@ -779,7 +782,7 @@ static int verify_evm(const char *file)
 		return -1;
 	}
 
-	return verify_hash(file, hash, sizeof(hash), sig + 1, len - 1);
+	return verify_hash(file, hash, mdlen, sig + 1, len - 1);
 }
 
 static int cmd_verify_evm(struct command *cmd)

@@ -1135,11 +1138,12 @@ out:
 
 static int hmac_evm(const char *file, const char *key)
 {
-	unsigned char hash[20];
+	unsigned char hash[64];
 	unsigned char sig[1024];
 	int len, err;
 
 	len = calc_evm_hmac(file, key, hash);
+	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;