[PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm

[PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm

[Vitaly Chikunov]

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 <vt@altlinux.org>
---
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(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 1b46d58..f8035da 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -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;
 
-- 
2.11.0

[PATCH v3 2/7] ima-evm-utils: Define hash and sig buffer sizes and add asserts

[Vitaly Chikunov]

To prevent hash and sig buffers size mismatch, define their maximum
sizes and add sanity checking asserts.

Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- New patch.
Changes since v2:
- Add single comment line to sign_evm.

 src/evmctl.c    | 35 ++++++++++++++++++++++-------------
 src/imaevm.h    |  3 +++
 src/libimaevm.c |  4 +++-
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index f8035da..f019a67 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -505,15 +505,17 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
 
 static int sign_evm(const char *file, const char *key)
 {
-	unsigned char hash[64];
-	unsigned char sig[1024];
+	unsigned char hash[MAX_DIGEST_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
 	len = calc_evm_hash(file, hash);
+	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;
 
 	len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
+	assert(len < sizeof(sig));
 	if (len <= 1)
 		return len;
 
@@ -543,7 +545,7 @@ static int sign_evm(const char *file, const char *key)
 
 static int hash_ima(const char *file)
 {
-	unsigned char hash[66]; /* MAX hash size + 2 */
+	unsigned char hash[MAX_DIGEST_SIZE + 2]; /* +2 byte xattr header */
 	int len, err, offset;
 	int algo = get_hash_algo(params.hash_algo);
 
@@ -557,6 +559,7 @@ static int hash_ima(const char *file)
 	}
 
 	len = ima_calc_hash(file, hash + offset);
+	assert(len + offset <= sizeof(hash));
 	if (len <= 1)
 		return len;
 
@@ -581,15 +584,17 @@ static int hash_ima(const char *file)
 
 static int sign_ima(const char *file, const char *key)
 {
-	unsigned char hash[64];
-	unsigned char sig[1024];
+	unsigned char hash[MAX_DIGEST_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
 	len = ima_calc_hash(file, hash);
+	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;
 
 	len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
+	assert(len < sizeof(sig));
 	if (len <= 1)
 		return len;
 
@@ -695,8 +700,8 @@ static int cmd_sign_hash(struct command *cmd)
 	int hashlen = 0;
 	size_t line_len;
 	ssize_t len;
-	unsigned char hash[64];
-	unsigned char sig[1024] = "\x03";
+	unsigned char hash[MAX_DIGEST_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE] = "\x03";
 	int siglen;
 
 	key = params.keyfile ? : "/etc/keys/privkey_evm.pem";
@@ -711,9 +716,11 @@ static int cmd_sign_hash(struct command *cmd)
 		token = strpbrk(line, ", \t");
 		hashlen = token ? token - line : strlen(line);
 
-		hex2bin(hash, line, hashlen);
+		assert(hashlen / 2 <= sizeof(hash));
+		hex2bin(hash, line, hashlen / 2);
 		siglen = sign_hash(params.hash_algo, hash, hashlen/2,
 				 key, NULL, sig + 1);
+		assert(siglen < sizeof(sig));
 		if (siglen <= 1)
 			return siglen;
 
@@ -761,8 +768,8 @@ static int cmd_sign_evm(struct command *cmd)
 
 static int verify_evm(const char *file)
 {
-	unsigned char hash[64];
-	unsigned char sig[1024];
+	unsigned char hash[MAX_DIGEST_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int mdlen;
 	int len;
 
@@ -804,12 +811,13 @@ static int cmd_verify_evm(struct command *cmd)
 
 static int verify_ima(const char *file)
 {
-	unsigned char sig[1024];
+	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len;
 
 	if (sigfile) {
 		void *tmp = file2bin(file, "sig", &len);
 
+		assert(len <= sizeof(sig));
 		memcpy(sig, tmp, len);
 		free(tmp);
 	} else {
@@ -1138,8 +1146,8 @@ out:
 
 static int hmac_evm(const char *file, const char *key)
 {
-	unsigned char hash[64];
-	unsigned char sig[1024];
+	unsigned char hash[MAX_DIGEST_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE];
 	int len, err;
 
 	len = calc_evm_hmac(file, key, hash);
@@ -1149,6 +1157,7 @@ static int hmac_evm(const char *file, const char *key)
 
 	log_info("hmac: ");
 	log_dump(hash, len);
+	assert(len < sizeof(sig));
 	memcpy(sig + 1, hash, len);
 
 	if (xattr) {
diff --git a/src/imaevm.h b/src/imaevm.h
index 1bafaad..2ebe7e7 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -75,6 +75,9 @@
 #define	DATA_SIZE	4096
 #define SHA1_HASH_LEN   20
 
+#define MAX_DIGEST_SIZE		64
+#define MAX_SIGNATURE_SIZE	1024
+
 #define __packed __attribute__((packed))
 
 enum evm_ima_xattr_type {
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 6fa0ed4..80b61a2 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -49,6 +49,7 @@
 #include <dirent.h>
 #include <string.h>
 #include <stdio.h>
+#include <assert.h>
 
 #include <openssl/pem.h>
 #include <openssl/evp.h>
@@ -590,7 +591,7 @@ int verify_hash(const char *file, const unsigned char *hash, int size, unsigned
 int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
 			 unsigned char *digest, int digestlen)
 {
-	unsigned char hash[64];
+	unsigned char hash[MAX_DIGEST_SIZE];
 	int hashlen, sig_hash_algo;
 
 	if (sig[0] != 0x03) {
@@ -614,6 +615,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
 	    return verify_hash(file, digest, digestlen, sig + 1, siglen - 1);
 
 	hashlen = ima_calc_hash(file, hash);
+	assert(hashlen <= sizeof(hash));
 	if (hashlen <= 1)
 		return hashlen;
 
-- 
2.11.0

[PATCH v3 3/7] ima-evm-utils: Define the '--xattr-user' option for testing

[Vitaly Chikunov]

The IMA/EVM attributes are currently stored in the "security" namespace,
which requires root privileges. Storing the ima/evm attributes in the
"user" namespace, instead of the "security" namespace, would be useful
for debugging and testing purposes, and because "--sigfile" does not
work for evm signatures.

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- No code changes. Only the description is reworded.
Changes since v2:
- Update README.

 README          |  1 +
 src/evmctl.c    | 33 +++++++++++++++++++++------------
 src/libimaevm.c |  2 +-
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/README b/README
index 4805564..05cc2ff 100644
--- a/README
+++ b/README
@@ -44,6 +44,7 @@ OPTIONS
   -s, --imasig       make IMA signature
   -d, --imahash      make IMA hash
   -f, --sigfile      store IMA signature in .sig file instead of xattr
+      --xattr-user   store xattrs in user namespace (for testing purposes)
       --rsa          use RSA key type and signing scheme v1
   -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)
   -o, --portable     generate portable EVM signatures
diff --git a/src/evmctl.c b/src/evmctl.c
index f019a67..f4df027 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -145,6 +145,9 @@ static int find(const char *path, int dts, find_cb_t func);
 struct command cmds[];
 static void print_usage(struct command *cmd);
 
+static const char *xattr_ima = "security.ima";
+static const char *xattr_evm = "security.evm";
+
 static int bin2file(const char *file, const char *ext, const unsigned char *data, int len)
 {
 	FILE *fp;
@@ -533,7 +536,7 @@ static int sign_evm(const char *file, const char *key)
 		dump(sig, len);
 
 	if (xattr) {
-		err = lsetxattr(file, "security.evm", sig, len, 0);
+		err = lsetxattr(file, xattr_evm, sig, len, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
 			return err;
@@ -572,7 +575,7 @@ static int hash_ima(const char *file)
 		dump(hash, len);
 
 	if (xattr) {
-		err = lsetxattr(file, "security.ima", hash, len, 0);
+		err = lsetxattr(file, xattr_ima, hash, len, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
 			return err;
@@ -609,7 +612,7 @@ static int sign_ima(const char *file, const char *key)
 		bin2file(file, "sig", sig, len);
 
 	if (xattr) {
-		err = lsetxattr(file, "security.ima", sig, len, 0);
+		err = lsetxattr(file, xattr_ima, sig, len, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
 			return err;
@@ -778,14 +781,14 @@ static int verify_evm(const char *file)
 	if (mdlen <= 1)
 		return mdlen;
 
-	len = lgetxattr(file, "security.evm", sig, sizeof(sig));
+	len = lgetxattr(file, xattr_evm, sig, sizeof(sig));
 	if (len < 0) {
 		log_err("getxattr failed: %s\n", file);
 		return len;
 	}
 
 	if (sig[0] != 0x03) {
-		log_err("security.evm has no signature\n");
+		log_err("%s has no signature\n", xattr_evm);
 		return -1;
 	}
 
@@ -821,7 +824,7 @@ static int verify_ima(const char *file)
 		memcpy(sig, tmp, len);
 		free(tmp);
 	} else {
-		len = lgetxattr(file, "security.ima", sig, sizeof(sig));
+		len = lgetxattr(file, xattr_ima, sig, sizeof(sig));
 		if (len < 0) {
 			log_err("getxattr failed: %s\n", file);
 			return len;
@@ -964,7 +967,7 @@ static int setxattr_ima(const char *file, char *sig_file)
 	if (!sig)
 		return 0;
 
-	err = lsetxattr(file, "security.ima", sig, len, 0);
+	err = lsetxattr(file, xattr_ima, sig, len, 0);
 	if (err < 0)
 		log_err("setxattr failed: %s\n", file);
 	free(sig);
@@ -1162,7 +1165,7 @@ static int hmac_evm(const char *file, const char *key)
 
 	if (xattr) {
 		sig[0] = EVM_XATTR_HMAC;
-		err = lsetxattr(file, "security.evm", sig, len + 1, 0);
+		err = lsetxattr(file, xattr_evm, sig, len + 1, 0);
 		if (err < 0) {
 			log_err("setxattr failed: %s\n", file);
 			return err;
@@ -1218,9 +1221,9 @@ static int ima_fix(const char *path)
 		}
 		for (; size > 0; len++, size -= len, list += len) {
 			len = strlen(list);
-			if (!strcmp(list, "security.ima"))
+			if (!strcmp(list, xattr_ima))
 				ima = 1;
-			else if (!strcmp(list, "security.evm"))
+			else if (!strcmp(list, xattr_evm))
 				evm = 1;
 		}
 		if (ima && evm)
@@ -1297,8 +1300,8 @@ static int cmd_ima_fix(struct command *cmd)
 static int ima_clear(const char *path)
 {
 	log_info("%s\n", path);
-	lremovexattr(path, "security.ima");
-	lremovexattr(path, "security.evm");
+	lremovexattr(path, xattr_ima);
+	lremovexattr(path, xattr_evm);
 
 	return 0;
 }
@@ -1654,6 +1657,7 @@ static void usage(void)
 		"  -s, --imasig       make IMA signature\n"
 		"  -d, --imahash      make IMA hash\n"
 		"  -f, --sigfile      store IMA signature in .sig file instead of xattr\n"
+		"      --xattr-user   store xattrs in user namespace (for testing purposes)\n"
 		"      --rsa          use RSA key type and signing scheme v1\n"
 		"  -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
 		"  -o, --portable     generate portable EVM signatures\n"
@@ -1728,6 +1732,7 @@ static struct option opts[] = {
 	{"selinux", 1, 0, 136},
 	{"caps", 2, 0, 137},
 	{"list", 0, 0, 138},
+	{"xattr-user", 0, 0, 140},
 	{}
 
 };
@@ -1879,6 +1884,10 @@ int main(int argc, char *argv[])
 		case 138:
 			measurement_list = 1;
 			break;
+		case 140: /* --xattr-user */
+			xattr_ima = "user.ima";
+			xattr_evm = "user.evm";
+			break;
 		case '?':
 			exit(1);
 			break;
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 80b61a2..34501ca 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -595,7 +595,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
 	int hashlen, sig_hash_algo;
 
 	if (sig[0] != 0x03) {
-		log_err("security.ima has no signature\n");
+		log_err("xattr ima has no signature\n");
 		return -1;
 	}
 
-- 
2.11.0

[PATCH v3 4/7] ima-evm-utils: Allow using Streebog hash function

[Vitaly Chikunov]

This patch will allow using GOST algorithms from OpenSSL's
gost-engine[1] via config extension (which is the usual way).

[1] https://github.com/gost-engine/engine

Full usage example:

1. Install the gost-engine package for your distro, this could be
libengine-gost-openssl1.1, openssl-gost-engine, or openssl-engines.

2. Edit openssl.cnf appropriately. Reference INSTALL.md of gost-engine
for the detailed instructions.

3. Then GOST algorithms should work:

  $ cp /dev/null a
  $ openssl dgst -streebog256 a
  md_gost12_256(a)= 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
  $ evmctl -v ima_hash -a streebog256 --xattr-user a
  hash: 04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
  $ getfattr -d -m. -ehex a
  # file: a
  user.ima=0x04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- "--engine" option is removed into separate patch.
Changes since v2:
- Add usage example to description.

 src/evmctl.c    |  6 +++---
 src/imaevm.h    | 13 +++++++++++++
 src/libimaevm.c | 15 +++++++++++----
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index f4df027..1f6dad5 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -388,7 +388,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
 
 	md = EVP_get_digestbyname(params.hash_algo);
 	if (!md) {
-		log_err("EVP_get_digestbyname() failed\n");
+		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;
 	}
 
@@ -1064,7 +1064,7 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
 
 	md = EVP_get_digestbyname(params.hash_algo);
 	if (!md) {
-		log_err("EVP_get_digestbyname() failed\n");
+		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		goto out;
 	}
 
@@ -1653,7 +1653,7 @@ static void usage(void)
 
 	printf(
 		"\n"
-		"  -a, --hashalgo     sha1 (default), sha224, sha256, sha384, sha512\n"
+		"  -a, --hashalgo     sha1 (default), sha224, sha256, sha384, sha512, streebog256, streebog512\n"
 		"  -s, --imasig       make IMA signature\n"
 		"  -d, --imahash      make IMA hash\n"
 		"  -f, --sigfile      store IMA signature in .sig file instead of xattr\n"
diff --git a/src/imaevm.h b/src/imaevm.h
index 2ebe7e7..c81bf21 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -152,6 +152,7 @@ struct signature_hdr {
 	char mpi[0];
 } __packed;
 
+/* reflect enum hash_algo from include/uapi/linux/hash_info.h */
 enum pkey_hash_algo {
 	PKEY_HASH_MD4,
 	PKEY_HASH_MD5,
@@ -161,6 +162,18 @@ enum pkey_hash_algo {
 	PKEY_HASH_SHA384,
 	PKEY_HASH_SHA512,
 	PKEY_HASH_SHA224,
+	PKEY_HASH_RIPE_MD_128,
+	PKEY_HASH_RIPE_MD_256,
+	PKEY_HASH_RIPE_MD_320,
+	PKEY_HASH_WP_256,
+	PKEY_HASH_WP_384,
+	PKEY_HASH_WP_512,
+	PKEY_HASH_TGR_128,
+	PKEY_HASH_TGR_160,
+	PKEY_HASH_TGR_192,
+	PKEY_HASH_SM3_256,
+	PKEY_HASH_STREEBOG_256,
+	PKEY_HASH_STREEBOG_512,
 	PKEY_HASH__LAST
 };
 
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 34501ca..7b2b62c 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -51,6 +51,7 @@
 #include <stdio.h>
 #include <assert.h>
 
+#include <openssl/crypto.h>
 #include <openssl/pem.h>
 #include <openssl/evp.h>
 #include <openssl/x509.h>
@@ -67,6 +68,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",
 };
 
 /*
@@ -291,7 +294,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 
 	md = EVP_get_digestbyname(params.hash_algo);
 	if (!md) {
-		log_err("EVP_get_digestbyname() failed\n");
+		log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
 		return 1;
 	}
 
@@ -509,14 +512,16 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	asn1 = &RSA_ASN1_templates[hdr->hash_algo];
 
 	if (len < asn1->size || memcmp(out, asn1->data, asn1->size)) {
-		log_err("%s: verification failed: %d\n", file, err);
+		log_err("%s: verification failed: %d (asn1 mismatch)\n",
+			file, err);
 		return -1;
 	}
 
 	len -= asn1->size;
 
 	if (len != size || memcmp(out + asn1->size, hash, len)) {
-		log_err("%s: verification failed: %d\n", file, err);
+		log_err("%s: verification failed: %d (digest mismatch)\n",
+			file, err);
 		return -1;
 	}
 
@@ -528,7 +533,8 @@ int get_hash_algo(const char *algo)
 	int i;
 
 	for (i = 0; i < PKEY_HASH__LAST; i++)
-		if (!strcmp(algo, pkey_hash_algo[i]))
+		if (pkey_hash_algo[i] &&
+		    !strcmp(algo, pkey_hash_algo[i]))
 			return i;
 
 	return PKEY_HASH_SHA1;
@@ -901,5 +907,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c
 static void libinit()
 {
 	OpenSSL_add_all_algorithms();
+	OPENSSL_add_all_algorithms_conf();
 	ERR_load_crypto_strings();
 }
-- 
2.11.0

[PATCH v3 5/7] ima-evm-utils: Preload OpenSSL engine via '--engine' option

[Vitaly Chikunov]

Another method of using GOST algorithms (and cryptographic accelerators)
is via direct preloading of appropriate engine using '--engine' option.
For the gost-engine it should be '--engine gost'.

Usage example:

1. Install gost-engine appropriately. (No need to edit openssl.cnf).

2. Then GOST algorithms should work:

  # cp /dev/null a
  # evmctl -v ima_hash --engine gost -a streebog256 a
  hash: 04123f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- Code split from prevously combined patch.
- More verbose OpenSSL error message.
Changes since v2:
- Update README.
- Add example engine name to the --help.
- Add usage example to description.

 README       |  1 +
 src/evmctl.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/README b/README
index 05cc2ff..3603ae8 100644
--- a/README
+++ b/README
@@ -58,6 +58,7 @@ OPTIONS
       --smack        use extra SMACK xattrs for EVM
       --m32          force EVM hmac/signature for 32 bit target system
       --m64          force EVM hmac/signature for 64 bit target system
+      --engine e     preload OpenSSL engine e (such as: gost)
   -v                 increase verbosity level
   -h, --help         display this help and exit
 
diff --git a/src/evmctl.c b/src/evmctl.c
index 1f6dad5..0459798 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -62,6 +62,7 @@
 #include <openssl/hmac.h>
 #include <openssl/err.h>
 #include <openssl/rsa.h>
+#include <openssl/engine.h>
 
 #ifndef XATTR_APPAARMOR_SUFFIX
 #define XATTR_APPARMOR_SUFFIX "apparmor"
@@ -1680,6 +1681,7 @@ static void usage(void)
 		"      --selinux      use custom Selinux label for EVM\n"
 		"      --caps         use custom Capabilities for EVM(unspecified: from FS, empty: do not use)\n"
 		"      --list         measurement list verification\n"
+		"      --engine e     preload OpenSSL engine e (such as: gost)\n"
 		"  -v                 increase verbosity level\n"
 		"  -h, --help         display this help and exit\n"
 		"\n");
@@ -1732,6 +1734,7 @@ static struct option opts[] = {
 	{"selinux", 1, 0, 136},
 	{"caps", 2, 0, 137},
 	{"list", 0, 0, 138},
+	{"engine", 1, 0, 139},
 	{"xattr-user", 0, 0, 140},
 	{}
 
@@ -1774,6 +1777,7 @@ static char *get_password(void)
 int main(int argc, char *argv[])
 {
 	int err = 0, c, lind;
+	ENGINE *eng = NULL;
 
 	g_argv = argv;
 	g_argc = argc;
@@ -1884,6 +1888,18 @@ int main(int argc, char *argv[])
 		case 138:
 			measurement_list = 1;
 			break;
+		case 139: /* --engine e */
+			eng = ENGINE_by_id(optarg);
+			if (!eng) {
+				log_err("engine %s isn't available\n", optarg);
+				ERR_print_errors_fp(stderr);
+			} else if (!ENGINE_init(eng)) {
+				log_err("engine %s init failed\n", optarg);
+				ERR_print_errors_fp(stderr);
+				ENGINE_free(eng);
+				eng = NULL;
+			}
+			break;
 		case 140: /* --xattr-user */
 			xattr_ima = "user.ima";
 			xattr_evm = "user.evm";
@@ -1914,6 +1930,13 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (eng) {
+		ENGINE_finish(eng);
+		ENGINE_free(eng);
+#if OPENSSL_API_COMPAT < 0x10100000L
+		ENGINE_cleanup();
+#endif
+	}
 	ERR_free_strings();
 	EVP_cleanup();
 	BIO_free(NULL);
-- 
2.11.0

[PATCH v3 6/7] ima-evm-utils: Extract digest algorithms from hash_info.h

[Vitaly Chikunov]

If configured with "--with-kernel-headers=PATH" try to extract hash
algorithms from "hash_info.h" from the kernel source tree or
kernel-headers package located in the specified path. (Otherwise, it
will be tried to get from the installed kernel.)

This also introduces two algorithm lists, one is built-in and another is
from the kernel source. (They should never contain conflicting algorithm
IDs by their append-only nature.) If the digest is not found in the
built-in list it will be searched in the list from kernel's
"hash_info.h".

This patch will allow evmctl to be just recompiled to work with digest
algorithms introduced in the newer kernels.

Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- New patch.
Changes since v2:
- Update commit description.
- Update description of --with-kernel-headers.

 configure.ac      |  6 ++++++
 src/Makefile.am   |  6 ++++++
 src/hash_info.gen | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/libimaevm.c   | 44 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100755 src/hash_info.gen

diff --git a/configure.ac b/configure.ac
index a5b4288..60f3684 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,12 +27,18 @@ AC_HEADER_STDC
 PKG_CHECK_MODULES(OPENSSL, [ openssl >= 0.9.8 ])
 AC_SUBST(OPENSSL_CFLAGS)
 AC_SUBST(OPENSSL_LIBS)
+AC_SUBST(KERNEL_HEADERS)
 AC_CHECK_HEADER(unistd.h)
 AC_CHECK_HEADERS(openssl/conf.h)
 
 AC_CHECK_HEADERS(sys/xattr.h, , [AC_MSG_ERROR([sys/xattr.h header not found. You need the c-library development package.])])
 AC_CHECK_HEADERS(keyutils.h, , [AC_MSG_ERROR([keyutils.h header not found. You need the libkeyutils development package.])])
 
+AC_ARG_WITH(kernel_headers, [AS_HELP_STRING([--with-kernel-headers=PATH],
+	    [specifies the Linux kernel-headers package location or kernel root directory you want to use])],
+	    [KERNEL_HEADERS="$withval"],
+	    [KERNEL_HEADERS=/lib/modules/$(uname -r)/source])
+
 #debug support - yes for a while
 PKG_ARG_ENABLE(debug, "yes", DEBUG, [Enable Debug support])
 if test $pkg_cv_enable_debug = yes; then
diff --git a/src/Makefile.am b/src/Makefile.am
index deb18fb..d74fc6f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,6 +9,11 @@ libimaevm_la_LIBADD =  $(OPENSSL_LIBS)
 
 include_HEADERS = imaevm.h
 
+nodist_libimaevm_la_SOURCES = hash_info.h
+BUILT_SOURCES = hash_info.h
+hash_info.h: Makefile
+	./hash_info.gen $(KERNEL_HEADERS) >$@
+
 bin_PROGRAMS = evmctl
 
 evmctl_SOURCES = evmctl.c
@@ -18,5 +23,6 @@ evmctl_LDADD =  $(OPENSSL_LIBS) -lkeyutils libimaevm.la
 
 INCLUDES = -I$(top_srcdir) -include config.h
 
+CLEANFILES = hash_info.h
 DISTCLEANFILES = @DISTCLEANFILES@
 
diff --git a/src/hash_info.gen b/src/hash_info.gen
new file mode 100755
index 0000000..60fc750
--- /dev/null
+++ b/src/hash_info.gen
@@ -0,0 +1,43 @@
+#!/bin/sh
+#
+# Generate hash_info.h from kernel headers
+#
+# Copyright (C) 2018 <vt@altlinux.org>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+KERNEL_HEADERS=$1
+HASH_INFO_H=uapi/linux/hash_info.h
+HASH_INFO=$KERNEL_HEADERS/include/$HASH_INFO_H
+
+# Allow to specify kernel-headers past include/
+if [ ! -e $HASH_INFO ]; then
+  HASH_INFO2=$KERNEL_HEADERS/$HASH_INFO_H
+  if [ -e $HASH_INFO2 ]; then
+    HASH_INFO=$HASH_INFO2
+  fi
+fi
+
+if [ ! -e $HASH_INFO ]; then
+  echo "/* $HASH_INFO is not found */"
+  HASH_INFO=/dev/null
+else
+  echo "/* $HASH_INFO is found */"
+fi
+
+echo "enum hash_algo {"
+grep HASH_ALGO_.*, $HASH_INFO
+printf "\tHASH_ALGO__LAST\n"
+echo "};"
+
+echo "const char *const hash_algo_name[HASH_ALGO__LAST] = {"
+sed -n 's/HASH_ALGO_\(.*\),/[HASH_ALGO_\1] = "\L\1\E",/p' $HASH_INFO
+echo "};"
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 7b2b62c..cb4721b 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -50,6 +50,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <assert.h>
+#include <ctype.h>
 
 #include <openssl/crypto.h>
 #include <openssl/pem.h>
@@ -58,6 +59,7 @@
 #include <openssl/err.h>
 
 #include "imaevm.h"
+#include "hash_info.h"
 
 const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 	[PKEY_HASH_MD4]		= "md4",
@@ -153,6 +155,17 @@ void dump(const void *ptr, int len)
 	do_dump(stdout, ptr, len, true);
 }
 
+const char *get_hash_algo_by_id(int algo)
+{
+	if (algo < PKEY_HASH__LAST)
+	    return pkey_hash_algo[algo];
+	if (algo < HASH_ALGO__LAST)
+	    return hash_algo_name[algo];
+
+	log_err("digest %d not found\n", algo);
+	return "unknown";
+}
+
 int get_filesize(const char *filename)
 {
 	struct stat stats;
@@ -528,15 +541,44 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	return 0;
 }
 
+/* compare algo names case insensitively and ignoring separators */
+static int algocmp(const char *a, const char *b)
+{
+	while (*a && *b) {
+		int cha, chb;
+
+		cha = tolower((unsigned char)*a++);
+		if (!isalnum(cha))
+			continue;
+		chb = tolower((unsigned char)*b++);
+		if (!isalnum(chb)) {
+			a--;
+			continue;
+		}
+		if (cha != chb)
+			return -1;
+	}
+	return *a || *b;
+}
+
 int get_hash_algo(const char *algo)
 {
 	int i;
 
+	/* first iterate over builtin algorithms */
 	for (i = 0; i < PKEY_HASH__LAST; i++)
 		if (pkey_hash_algo[i] &&
 		    !strcmp(algo, pkey_hash_algo[i]))
 			return i;
 
+	/* iterate over algorithms provided by kernel-headers */
+	for (i = 0; i < HASH_ALGO__LAST; i++) {
+		if (hash_algo_name[i] &&
+		    !algocmp(algo, hash_algo_name[i]))
+			return i;
+	}
+
+	log_info("digest %s not found, fall back to sha1\n", algo);
 	return PKEY_HASH_SHA1;
 }
 
@@ -611,7 +653,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
 		return -1;
 	}
 	/* Use hash algorithm as retrieved from signature */
-	params.hash_algo = pkey_hash_algo[sig_hash_algo];
+	params.hash_algo = get_hash_algo_by_id(sig_hash_algo);
 
 	/*
 	 * Validate the signature based on the digest included in the
-- 
2.11.0

[PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[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.

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
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(-)

diff --git a/src/imaevm.h b/src/imaevm.h
index c81bf21..795966a 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -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
 
diff --git a/src/libimaevm.c b/src/libimaevm.c
index cb4721b..7501303 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -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 */
-- 
2.11.0

Re: [PATCH v3 1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm

[Mimi Zohar]

On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> 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 <vt@altlinux.org>

Thank you!  This patch set has now been applied and is in master.

Mimi

> ---
> 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(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 1b46d58..f8035da 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -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;
> 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Mimi Zohar]

Hi Vitaly,

On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> 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

> 
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> ---
> 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(-)
> 
> diff --git a/src/imaevm.h b/src/imaevm.h
> index c81bf21..795966a 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -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
> 
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index cb4721b..7501303 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -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 */

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

Mimi,

On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > 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.

> evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt  --xattr-user
> evmctl ima_verify -k <public key>  ./foo.txt  --xattr-user
> 
> Mimi
> 
> > 
> > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > ---
> > 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(-)
> > 
> > diff --git a/src/imaevm.h b/src/imaevm.h
> > index c81bf21..795966a 100644
> > --- a/src/imaevm.h
> > +++ b/src/imaevm.h
> > @@ -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
> > 
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index cb4721b..7501303 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -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 */

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Mimi Zohar]

On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > 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.

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

> 
> > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt  --xattr-user
> > evmctl ima_verify -k <public key>  ./foo.txt  --xattr-user
> > 
> > Mimi
> > 
> > > 
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > ---
> > > 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(-)
> > > 
> > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > index c81bf21..795966a 100644
> > > --- a/src/imaevm.h
> > > +++ b/src/imaevm.h
> > > @@ -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
> > > 
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index cb4721b..7501303 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -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 */
> 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

Mimi,

On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > 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.
> 
> 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.

> 
> Mimi
> 
> > 
> > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt  --xattr-user
> > > evmctl ima_verify -k <public key>  ./foo.txt  --xattr-user
> > > 
> > > Mimi
> > > 
> > > > 
> > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > ---
> > > > 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(-)
> > > > 
> > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > index c81bf21..795966a 100644
> > > > --- a/src/imaevm.h
> > > > +++ b/src/imaevm.h
> > > > @@ -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
> > > > 
> > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > index cb4721b..7501303 100644
> > > > --- a/src/libimaevm.c
> > > > +++ b/src/libimaevm.c
> > > > @@ -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 */
> > 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > 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.
> > 
> > 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.

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

@@ -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];

> 
> > 
> > Mimi
> > 
> > > 
> > > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt  --xattr-user
> > > > evmctl ima_verify -k <public key>  ./foo.txt  --xattr-user
> > > > 
> > > > Mimi
> > > > 
> > > > > 
> > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > > ---
> > > > > 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(-)
> > > > > 
> > > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > > index c81bf21..795966a 100644
> > > > > --- a/src/imaevm.h
> > > > > +++ b/src/imaevm.h
> > > > > @@ -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
> > > > > 
> > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > > index cb4721b..7501303 100644
> > > > > --- a/src/libimaevm.c
> > > > > +++ b/src/libimaevm.c
> > > > > @@ -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 */
> > > 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

Mimi,

On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > 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.
> > > 
> > > 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.

Thanks,

> > 
> > 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":
> 
> @@ -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];
> 
> > 
> > > 
> > > Mimi
> > > 
> > > > 
> > > > > evmctl ima_sign -k <private key> -p -a streebog256 ./foo.txt  --xattr-user
> > > > > evmctl ima_verify -k <public key>  ./foo.txt  --xattr-user
> > > > > 
> > > > > Mimi
> > > > > 
> > > > > > 
> > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > > > ---
> > > > > > 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(-)
> > > > > > 
> > > > > > diff --git a/src/imaevm.h b/src/imaevm.h
> > > > > > index c81bf21..795966a 100644
> > > > > > --- a/src/imaevm.h
> > > > > > +++ b/src/imaevm.h
> > > > > > @@ -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
> > > > > > 
> > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > > > index cb4721b..7501303 100644
> > > > > > --- a/src/libimaevm.c
> > > > > > +++ b/src/libimaevm.c
> > > > > > @@ -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 */
> > > > 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Mimi Zohar]

On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > 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.
> > > > 
> > > > 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.

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.

diff --git a/src/libimaevm.c b/src/libimaevm.c
index d9ffa13befb0..7901215da655 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -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

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

Mimi,

On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > 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.
> > > > > 
> > > > > 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.
> 
> 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.

> 
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index d9ffa13befb0..7901215da655 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -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

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Mimi Zohar]

On Mon, 2019-02-11 at 23:37 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> > On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > > 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.
> > > > > > 
> > > > > > 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.
> > 
> > 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


> 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.
> 
> > 
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index d9ffa13befb0..7901215da655 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -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
> 

Re: [PATCH v3 7/7] ima-evm-utils: Try to load digest by its alias

[Vitaly Chikunov]

On Tue, Feb 12, 2019 at 10:41:46AM -0500, Mimi Zohar wrote:
> On Mon, 2019-02-11 at 23:37 +0300, Vitaly Chikunov wrote:
> > Mimi,
> > 
> > On Mon, Feb 11, 2019 at 03:21:03PM -0500, Mimi Zohar wrote:
> > > On Mon, 2019-02-11 at 22:26 +0300, Vitaly Chikunov wrote:
> > > > On Mon, Feb 11, 2019 at 09:21:15PM +0300, Vitaly Chikunov wrote:
> > > > > On Mon, Feb 11, 2019 at 09:13:00PM +0300, Vitaly Chikunov wrote:
> > > > > > On Mon, Feb 11, 2019 at 12:59:12PM -0500, Mimi Zohar wrote:
> > > > > > > On Mon, 2019-02-11 at 20:52 +0300, Vitaly Chikunov wrote:
> > > > > > > > On Mon, Feb 11, 2019 at 12:38:58PM -0500, Mimi Zohar wrote:
> > > > > > > > > On Mon, 2018-12-03 at 06:35 +0300, Vitaly Chikunov wrote:
> > > > > > > > > > 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.
> > > > > > > 
> > > > > > > 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.
> > > 
> > > 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

What patch?

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

This problem was fixed in "ima-evm-utils: convert sign v2 from RSA to
EVP_PKEY API". This wasn't problem because Streebog should not be
used in sign/verify (which was using RSA). When sign/verify ability for
Streebog is added in "ima-evm-utils: convert sign v2 from RSA to EVP_PKEY
API" sign/verify problem is fixed in the same patch.

But, since I sent new patch which removes strmatch this becomes
somewhat irrelevant.

> but fails without the strcmp().
> 
> +       log_info("strmatch: algo %s \n", needle);
> 
> Mimi
> 
> 
> > 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.
> > 
> > > 
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index d9ffa13befb0..7901215da655 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -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
> >