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

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

Cc: Mikhail Efremov <sem@altlinux.org>
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

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

 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..f53c684 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];
 	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 v2 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.

 src/evmctl.c    | 32 ++++++++++++++++++++------------
 src/libimaevm.c |  2 +-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index f53c684..9cbc2cb 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;
 }
@@ -1728,6 +1731,7 @@ static struct option opts[] = {
 	{"selinux", 1, 0, 136},
 	{"caps", 2, 0, 137},
 	{"list", 0, 0, 138},
+	{"xattr-user", 0, 0, 140},
 	{}
 
 };
@@ -1879,6 +1883,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 v2 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

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

 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 9cbc2cb..f4b2e7d 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 v2 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.

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changes since v1:
- Code split from prevously combined patch.
- More verbose OpenSSL error message.

 src/evmctl.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/evmctl.c b/src/evmctl.c
index f4b2e7d..03b6132 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"
@@ -1679,6 +1680,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\n"
 		"  -v                 increase verbosity level\n"
 		"  -h, --help         display this help and exit\n"
 		"\n");
@@ -1731,6 +1733,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},
 	{}
 
@@ -1773,6 +1776,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;
@@ -1883,6 +1887,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";
@@ -1913,6 +1929,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 v2 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. (From the specified PATH or 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.

 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..715babc 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[[=ARG]]],
+	    [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 v2 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.

 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 v2 3/7] ima-evm-utils: Define the '--xattr-user' option for testing

[Mimi Zohar]

On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:
> 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.
> 
>  src/evmctl.c    | 32 ++++++++++++++++++++------------
>  src/libimaevm.c |  2 +-
>  2 files changed, 21 insertions(+), 13 deletions(-)

Missing is the manpage change, which is created based on the README.
 Perhaps it is in a later patch.

Mimi

> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index f53c684..9cbc2cb 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;
>  }
> @@ -1728,6 +1731,7 @@ static struct option opts[] = {
>  	{"selinux", 1, 0, 136},
>  	{"caps", 2, 0, 137},
>  	{"list", 0, 0, 138},
> +	{"xattr-user", 0, 0, 140},
>  	{}
> 
>  };
> @@ -1879,6 +1883,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;
>  	}
> 

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

[Mimi Zohar]

On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:
> 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
> 
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> ---
> Changes since v1:
> - "--engine" option is removed into separate patch.

Thanks!

> 
>  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 9cbc2cb..f4b2e7d 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();
>  }

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

[Mimi Zohar]

On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:

> @@ -1773,6 +1776,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;
> @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[])
>  		case 138:
>  			measurement_list = 1;
>  			break;
> +		case 139: /* --engine e */
> +			eng = ENGINE_by_id(optarg);

The usage is only adding "--engine e" support.  Either change the
usage or add a test to verify the argument.


> +			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";

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

[Mimi Zohar]

On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:
> 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>

Thanks! 

> ---


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

Let's comment this as /* +2 byte xattr header */

>  	int len, err, offset;
>  	int algo = get_hash_algo(params.hash_algo);
> 

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

[Vitaly Chikunov]

On Fri, Nov 30, 2018 at 02:21:34PM -0500, Mimi Zohar wrote:
> On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:
> 
> > @@ -1773,6 +1776,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;
> > @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[])
> >  		case 138:
> >  			measurement_list = 1;
> >  			break;
> > +		case 139: /* --engine e */
> > +			eng = ENGINE_by_id(optarg);
> 
> The usage is only adding "--engine e" support.  Either change the
> usage or add a test to verify the argument.

Could you elaborate what I should do? I didn't understand your
suggestion.  User should be able to specify anything as engine name and
it is tested by ENGINE_by_id call. Also, usage implies that it would
load engine with the name e.


> 
> 
> > +			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";

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

[Mimi Zohar]

On Sat, 2018-12-01 at 06:01 +0300, Vitaly Chikunov wrote:
> On Fri, Nov 30, 2018 at 02:21:34PM -0500, Mimi Zohar wrote:
> > On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote:
> > 
> > > @@ -1773,6 +1776,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;
> > > @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[])
> > >  		case 138:
> > >  			measurement_list = 1;
> > >  			break;
> > > +		case 139: /* --engine e */
> > > +			eng = ENGINE_by_id(optarg);
> > 
> > The usage is only adding "--engine e" support.  Either change the
> > usage or add a test to verify the argument.
> 
> Could you elaborate what I should do? I didn't understand your
> suggestion.  User should be able to specify anything as engine name and
> it is tested by ENGINE_by_id call. Also, usage implies that it would
> load engine with the name e.

My mistake.  Could you update the patch description with example
usage?

Mimi