ima-evm-utils: Convert v2 signatures from RSA to EVP_PKEY API

PATCH In review Vitaly Chikunov 2 participants 11 messages Jul 3, 2019 – Jul 8, 2019 Keys-trusted

Share & tools

Vitaly Chikunov

author

2019-07-03 15:50 UTC #0 lore raw

v8 cover ima-evm-utils: Convert v2 signatures from RSA to EVP_PKEY API

Convert sign v2 from RSA API (with manual formatting PKCS1) to more generic EVP_PKEY API, allowing to generate more types of OpenSSL supported signatures. This is done to enable EC-RDSA signatures, which are already supported in the Kernel. With some small fixes.

All patches tested on x86_64 to not break anything.

Changes since v7:
- Fix freeing pkey returned by find_keyid() in verify_hash_v2.
- Fix assert after ima_calc_hash() before its return status check.
- More verbose error log for sign and verify hash v2.
- Diff from v7 is below.

Changes since v6:
- Remove "Make sure sig buffer is always MAX_SIGNATURE_SIZE" commit. Instead, change assumption of sign_hash_v2() about @sig size.
- Add "Log hash_algo with hash value in verbose mode".
- Diff from v6 is below.

Changes since v5:
- Squash calc keyid v2 with cmd_import patch.
- Add log_err messages to verify_hash_v2 and sign_hash_v2.
- Fix sign and hash generation error processing to show errors instead of assert failures.

Changes since v4:
- Split conversion into more patches, as suggested by Mimi Zohar.
- Small fixes suggested by Mimi Zohar.

Changes since v3:
- As suggested by Mimi Zohar this is v3 splitted into several patches to simplify review. No code changes.

Changes since v2:
- Just rebase over newer commits.

Changes since v1:
- More key neutral code in calc_keyid_v1().
- Fix uninitialized sigsize for EVP_PKEY_sign().
- Fix memory leaks for openssl types.

Vitaly Chikunov (9):
ima-evm-utils: Convert read_pub_key to EVP_PKEY API
ima-evm-utils: Convert read_priv_key to EVP_PKEY API
ima-evm-utils: Convert cmd_import and calc keyid v2 to EVP_PKEY API
ima-evm-utils: Convert verify_hash_v2 and find_keyid to EVP_PKEY API
ima-evm-utils: Convert sign_hash_v2 to EVP_PKEY API
ima-evm-utils: Replace calc_keyid_v2 with calc_pkeyid_v2
ima-evm-utils: Remove RSA_ASN1_templates
ima-evm-utils: Pass status codes from sign and hash functions to the
callers
ima-evm-utils: Log hash_algo with hash value in verbose mode

 src/evmctl.c    |  41 ++++----
 src/imaevm.h    |   4 +-
 src/libimaevm.c | 301 ++++++++++++++++++++++++++++----------------------------
 3 files changed, 176 insertions(+), 170 deletions(-)

Signature · 1 line

2.11.0

src/libimaevm.c +21 -7

index 5bff414..51d6c33 100644
@@ -453,10 +453,11 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 		   unsigned char *sig, int siglen, const char *keyfile)
 {
 	int ret = -1;
-	EVP_PKEY *pkey;
+	EVP_PKEY *pkey, *pkey_free = NULL;
 	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
 	EVP_PKEY_CTX *ctx;
 	const EVP_MD *md;
+	const char *st;
 
 	if (params.verbose > LOG_INFO) {
 		log_info("hash(%s): ", params.hash_algo);

@@ -474,16 +475,22 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 		pkey = read_pub_pkey(keyfile, 1);
 		if (!pkey)
 			return -1;
+		pkey_free = pkey;
 	}
 
+	st = "EVP_PKEY_CTX_new";
 	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
 		goto err;
+	st = "EVP_PKEY_verify_init";
 	if (!EVP_PKEY_verify_init(ctx))
 		goto err;
+	st = "EVP_get_digestbyname";
 	if (!(md = EVP_get_digestbyname(params.hash_algo)))
 		goto err;
+	st = "EVP_PKEY_CTX_set_signature_md";
 	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
 		goto err;
+	st = "EVP_PKEY_verify";
 	ret = EVP_PKEY_verify(ctx, sig + sizeof(*hdr),
 			      siglen - sizeof(*hdr), hash, size);
 	if (ret == 1)

@@ -495,12 +502,13 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	}
 err:
 	if (ret < 0 || ret > 1) {
-		log_err("%s: verification failed: %d (%s)\n",
-			file, ret, ERR_reason_error_string(ERR_peek_error()));
+		log_err("%s: verification failed: %d (%s) in %s\n",
+			file, ret, ERR_reason_error_string(ERR_peek_error()),
+			st);
 		ret = -1;
 	}
 	EVP_PKEY_CTX_free(ctx);
-	EVP_PKEY_free(pkey);
+	EVP_PKEY_free(pkey_free);
 	return ret;
 }

@@ -610,9 +618,9 @@ 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;
+	assert(hashlen <= sizeof(hash));
 
 	return verify_hash(file, hash, hashlen, sig + 1, siglen - 1);
 }

@@ -850,6 +858,7 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 	EVP_PKEY_CTX *ctx = NULL;
 	const EVP_MD *md;
 	size_t sigsize;
+	const char *st;
 
 	if (!hash) {
 		log_err("sign_hash_v2: hash is null\n");

@@ -885,14 +894,19 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 
 	calc_keyid_v2(&hdr->keyid, name, pkey);
 
+	st = "EVP_PKEY_CTX_new";
 	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
 		goto err;
+	st = "EVP_PKEY_sign_init";
 	if (!EVP_PKEY_sign_init(ctx))
 		goto err;
+	st = "EVP_get_digestbyname";
 	if (!(md = EVP_get_digestbyname(params.hash_algo)))
 		goto err;
+	st = "EVP_PKEY_CTX_set_signature_md";
 	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
 		goto err;
+	st = "EVP_PKEY_sign";
 	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
 	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
 		goto err;

@@ -905,8 +919,8 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 
 err:
 	if (len == -1)
-		log_err("sign_hash_v2: signing failed: (%s)\n",
-			ERR_reason_error_string(ERR_peek_error()));
+		log_err("sign_hash_v2: signing failed: (%s) in %s\n",
+			ERR_reason_error_string(ERR_peek_error()), st);
 	EVP_PKEY_CTX_free(ctx);
 	EVP_PKEY_free(pkey);
 	return len;

Vitaly Chikunov

author

2019-07-03 15:50 UTC #1 lore raw

PATCH v8 1/9 ima-evm-utils: Convert read_pub_key to EVP_PKEY API

Introduce read_pub_pkey() to read keys using EVP_PKEY, and change read_pub_key() to be wrapper for it.

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

---
 src/imaevm.h    |  1 +
 src/libimaevm.c | 33 ++++++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 11 deletions(-)

src/imaevm.h +1 -0

index c81bf21..6d5eabd 100644
@@ -216,6 +216,7 @@ int get_filesize(const char *filename);
 int ima_calc_hash(const char *file, uint8_t *hash);
 int get_hash_algo(const char *algo);
 RSA *read_pub_key(const char *keyfile, int x509);
+EVP_PKEY *read_pub_pkey(const char *keyfile, int x509);
 
 void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len);
 void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key);

src/libimaevm.c +22 ~~-11~~

index 3a9ab63..da0f422 100644
@@ -355,10 +355,9 @@ int ima_calc_hash(const char *file, uint8_t *hash)
 	return mdlen;
 }
 
-RSA *read_pub_key(const char *keyfile, int x509)
+EVP_PKEY *read_pub_pkey(const char *keyfile, int x509)
 {
 	FILE *fp;
-	RSA *key = NULL;
 	X509 *crt = NULL;
 	EVP_PKEY *pkey = NULL;

@@ -375,24 +374,36 @@ RSA *read_pub_key(const char *keyfile, int x509)
 			goto out;
 		}
 		pkey = X509_extract_key(crt);
+		X509_free(crt);
 		if (!pkey) {
 			log_err("X509_extract_key() failed\n");
 			goto out;
 		}
-		key = EVP_PKEY_get1_RSA(pkey);
 	} else {
-		key = PEM_read_RSA_PUBKEY(fp, NULL, NULL, NULL);
+		pkey = PEM_read_PUBKEY(fp, NULL, NULL, NULL);
+		if (!pkey)
+			log_err("PEM_read_PUBKEY() failed\n");
 	}
 
-	if (!key)
-		log_err("PEM_read_RSA_PUBKEY() failed\n");
-
 out:
-	if (pkey)
-		EVP_PKEY_free(pkey);
-	if (crt)
-		X509_free(crt);
 	fclose(fp);
+	return pkey;
+}
+
+RSA *read_pub_key(const char *keyfile, int x509)
+{
+	EVP_PKEY *pkey;
+	RSA *key;
+
+	pkey = read_pub_pkey(keyfile, x509);
+	if (!pkey)
+		return NULL;
+	key = EVP_PKEY_get1_RSA(pkey);
+	EVP_PKEY_free(pkey);
+	if (!key) {
+		log_err("read_pub_key: unsupported key type\n");
+		return NULL;
+	}
 	return key;
 }

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:50 UTC #2 lore raw

PATCH v8 2/9 ima-evm-utils: Convert read_priv_key to EVP_PKEY API

In reply to Vitaly Chikunov

Introduce read_priv_pkey() to read keys using EVP_PKEY, and change read_priv_key() to be wrapper for it.

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

---
 src/libimaevm.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

src/libimaevm.c +22 -5

index da0f422..23fa804 100644
@@ -753,10 +753,10 @@ void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key)
 	free(pkey);
 }
 
-static RSA *read_priv_key(const char *keyfile, const char *keypass)
+static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
 {
 	FILE *fp;
-	RSA *key;
+	EVP_PKEY *pkey;
 
 	fp = fopen(keyfile, "r");
 	if (!fp) {

@@ -764,15 +764,32 @@ static RSA *read_priv_key(const char *keyfile, const char *keypass)
 		return NULL;
 	}
 	ERR_load_crypto_strings();
-	key = PEM_read_RSAPrivateKey(fp, NULL, NULL, (void *)keypass);
-	if (!key) {
+	pkey = PEM_read_PrivateKey(fp, NULL, NULL, (void *)keypass);
+	if (!pkey) {
 		char str[256];
 
 		ERR_error_string(ERR_get_error(), str);
-		log_err("PEM_read_RSAPrivateKey() failed: %s\n", str);
+		log_err("PEM_read_PrivateKey() failed: %s\n", str);
 	}
 
 	fclose(fp);
+	return pkey;
+}
+
+static RSA *read_priv_key(const char *keyfile, const char *keypass)
+{
+	EVP_PKEY *pkey;
+	RSA *key;
+
+	pkey = read_priv_pkey(keyfile, keypass);
+	if (!pkey)
+		return NULL;
+	key = EVP_PKEY_get1_RSA(pkey);
+	EVP_PKEY_free(pkey);
+	if (!key) {
+		log_err("read_priv_key: unsupported key type\n");
+		return NULL;
+	}
 	return key;
 }

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:50 UTC #3 lore raw

PATCH v8 3/9 ima-evm-utils: Convert cmd_import and calc keyid v2 to EVP_PKEY API

In reply to Vitaly Chikunov

Introduce calc_pkeyid_v2() (which accepts EVP_PKEY) to replace calc_keyid_v2() (which accepts RSA) in the future and use it in cmd_import().

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

---
 src/evmctl.c    | 25 +++++++++++++++----------
 src/imaevm.h    |  1 +
 src/libimaevm.c | 30 ++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 10 deletions(-)

src/evmctl.c +15 ~~-10~~

index 15a7226..eed8f9a 100644
@@ -891,7 +891,6 @@ static int cmd_import(struct command *cmd)
 	int id, len, err = 0;
 	char name[20];
 	uint8_t keyid[8];
-	RSA *key;
 
 	inkey = g_argv[optind++];
 	if (!inkey) {

@@ -925,18 +924,26 @@ static int cmd_import(struct command *cmd)
 		}
 	}
 
-	key = read_pub_key(inkey, params.x509);
-	if (!key)
-		return 1;
-
 	if (params.x509) {
+		EVP_PKEY *pkey = read_pub_pkey(inkey, params.x509);
+
+		if (!pkey)
+			return 1;
 		pub = file2bin(inkey, NULL, &len);
-		if (!pub)
-			goto out;
-		calc_keyid_v2((uint32_t *)keyid, name, key);
+		if (!pub) {
+			EVP_PKEY_free(pkey);
+			return 1;
+		}
+		calc_pkeyid_v2((uint32_t *)keyid, name, pkey);
+		EVP_PKEY_free(pkey);
 	} else {
+		RSA *key = read_pub_key(inkey, params.x509);
+
+		if (!key)
+			return 1;
 		len = key2bin(key, pub);
 		calc_keyid_v1(keyid, name, pub, len);
+		RSA_free(key);
 	}
 
 	log_info("Importing public key %s from file %s into keyring %d\n", name, inkey, id);

@@ -951,8 +958,6 @@ static int cmd_import(struct command *cmd)
 	}
 	if (params.x509)
 		free(pub);
-out:
-	RSA_free(key);
 	return err;
 }

src/imaevm.h +1 -0

index 6d5eabd..48d2663 100644
@@ -220,6 +220,7 @@ EVP_PKEY *read_pub_pkey(const char *keyfile, int x509);
 
 void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len);
 void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key);
+void calc_pkeyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey);
 int key2bin(RSA *key, unsigned char *pub);
 
 int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);

src/libimaevm.c +30 -0

index 23fa804..707b2e9 100644
@@ -753,6 +753,36 @@ void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key)
 	free(pkey);
 }
 
+/*
+ * Calculate keyid of the public_key part of EVP_PKEY
+ */
+void calc_pkeyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey)
+{
+	X509_PUBKEY *pk = NULL;
+	const unsigned char *public_key = NULL;
+	int len;
+
+	/* This is more generic than i2d_PublicKey() */
+	if (X509_PUBKEY_set(&pk, pkey) &&
+	    X509_PUBKEY_get0_param(NULL, &public_key, &len, NULL, pk)) {
+		uint8_t sha1[SHA_DIGEST_LENGTH];
+
+		SHA1(public_key, len, sha1);
+		/* sha1[12 - 19] is exactly keyid from gpg file */
+		memcpy(keyid, sha1 + 16, 4);
+	} else
+		*keyid = 0;
+
+	log_debug("keyid: ");
+	log_debug_dump(keyid, 4);
+	sprintf(str, "%x", __be32_to_cpup(keyid));
+
+	if (params.verbose > LOG_INFO)
+		log_info("keyid: %s\n", str);
+
+	X509_PUBKEY_free(pk);
+}
+
 static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
 {
 	FILE *fp;

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:50 UTC #4 lore raw

PATCH v8 4/9 ima-evm-utils: Convert verify_hash_v2 and find_keyid to EVP_PKEY API

In reply to Vitaly Chikunov

Rely on OpenSSL API to verify v2 signatures instead of manual PKCS1 decoding. Also, convert find_keyid() to return EVP_PKEY because verify_hash_v2() is sole user of it.

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

---
 src/libimaevm.c | 94 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 52 insertions(+), 42 deletions(-)

src/libimaevm.c +52 ~~-42~~

index 707b2e9..4c98cb0 100644
@@ -452,11 +452,11 @@ struct public_key_entry {
 	struct public_key_entry *next;
 	uint32_t keyid;
 	char name[9];
-	RSA *key;
+	EVP_PKEY *key;
 };
 static struct public_key_entry *public_keys = NULL;
 
-static RSA *find_keyid(uint32_t keyid)
+static EVP_PKEY *find_keyid(uint32_t keyid)
 {
 	struct public_key_entry *entry;

@@ -489,13 +489,13 @@ void init_public_keys(const char *keyfiles)
 			break;
 		}
 
-		entry->key = read_pub_key(keyfile, 1);
+		entry->key = read_pub_pkey(keyfile, 1);
 		if (!entry->key) {
 			free(entry);
 			continue;
 		}
 
-		calc_keyid_v2(&entry->keyid, entry->name, entry->key);
+		calc_pkeyid_v2(&entry->keyid, entry->name, entry->key);
 		sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid));
 		log_info("key %d: %s %s\n", i++, entry->name, keyfile);
 		entry->next = public_keys;

@@ -503,14 +503,18 @@ void init_public_keys(const char *keyfiles)
 	}
 }
 
+/*
+ * Return: 0 verification good, 1 verification bad, -1 error.
+ */
 int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 		   unsigned char *sig, int siglen, const char *keyfile)
 {
-	int err, len;
-	unsigned char out[1024];
-	RSA *key;
+	int ret = -1;
+	EVP_PKEY *pkey, *pkey_free = NULL;
 	struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
-	const struct RSA_ASN1_template *asn1;
+	EVP_PKEY_CTX *ctx;
+	const EVP_MD *md;
+	const char *st;
 
 	if (params.verbose > LOG_INFO) {
 		log_info("hash: ");

@@ -518,45 +522,51 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	}
 
 	if (public_keys) {
-		key = find_keyid(hdr->keyid);
-		if (!key) {
+		pkey = find_keyid(hdr->keyid);
+		if (!pkey) {
 			log_err("%s: unknown keyid: %x\n", file,
 				__be32_to_cpup(&hdr->keyid));
 			return -1;
 		}
 	} else {
-		key = read_pub_key(keyfile, 1);
-		if (!key)
-			return 1;
-	}
-
-
-	err = RSA_public_decrypt(siglen - sizeof(*hdr), sig + sizeof(*hdr),
-				 out, key, RSA_PKCS1_PADDING);
-	if (err < 0) {
-		log_err("%s: RSA_public_decrypt() failed: %d\n", file, err);
-		return 1;
-	}
-
-	len = err;
-
-	asn1 = &RSA_ASN1_templates[hdr->hash_algo];
-
-	if (len < asn1->size || memcmp(out, asn1->data, asn1->size)) {
-		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 (digest mismatch)\n",
-			file, err);
-		return -1;
-	}
-
-	return 0;
+		pkey = read_pub_pkey(keyfile, 1);
+		if (!pkey)
+			return -1;
+		pkey_free = pkey;
+	}
+
+	st = "EVP_PKEY_CTX_new";
+	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
+		goto err;
+	st = "EVP_PKEY_verify_init";
+	if (!EVP_PKEY_verify_init(ctx))
+		goto err;
+	st = "EVP_get_digestbyname";
+	if (!(md = EVP_get_digestbyname(params.hash_algo)))
+		goto err;
+	st = "EVP_PKEY_CTX_set_signature_md";
+	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
+		goto err;
+	st = "EVP_PKEY_verify";
+	ret = EVP_PKEY_verify(ctx, sig + sizeof(*hdr),
+			      siglen - sizeof(*hdr), hash, size);
+	if (ret == 1)
+		ret = 0;
+	else if (ret == 0) {
+		log_err("%s: verification failed: %d (%s)\n",
+			file, ret, ERR_reason_error_string(ERR_get_error()));
+		ret = 1;
+	}
+err:
+	if (ret < 0 || ret > 1) {
+		log_err("%s: verification failed: %d (%s) in %s\n",
+			file, ret, ERR_reason_error_string(ERR_peek_error()),
+			st);
+		ret = -1;
+	}
+	EVP_PKEY_CTX_free(ctx);
+	EVP_PKEY_free(pkey_free);
+	return ret;
 }
 
 int get_hash_algo(const char *algo)

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:51 UTC #5 lore raw

PATCH v8 5/9 ima-evm-utils: Convert sign_hash_v2 to EVP_PKEY API

In reply to Vitaly Chikunov

Convert sign_hash_v2() to use more generic EVP_PKEY API instead of RSA API. This enables generation of more signatures out of the box, such as EC-RDSA (GOST) and any other that OpenSSL supports. This conversion also fixes generation of MD4 signatures, because it didn't have proper RSA_ASN1_template.

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

---
 src/libimaevm.c | 60 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 24 deletions(-)

src/libimaevm.c +36 ~~-24~~

index 4c98cb0..213855c 100644
@@ -924,14 +924,20 @@ out:
 	return len;
 }
 
+/*
+ * @sig is assumed to be of (MAX_SIGNATURE_SIZE - 1) size
+ * Return: -1 signing error, >0 length of signature
+ */
 int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const char *keyfile, unsigned char *sig)
 {
 	struct signature_v2_hdr *hdr;
 	int len = -1;
-	RSA *key;
+	EVP_PKEY *pkey;
 	char name[20];
-	unsigned char *buf;
-	const struct RSA_ASN1_template *asn1;
+	EVP_PKEY_CTX *ctx = NULL;
+	const EVP_MD *md;
+	size_t sigsize;
+	const char *st;
 
 	if (!hash) {
 		log_err("sign_hash_v2: hash is null\n");

@@ -956,8 +962,8 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 	log_info("hash: ");
 	log_dump(hash, size);
 
-	key = read_priv_key(keyfile, params.keypass);
-	if (!key)
+	pkey = read_priv_pkey(keyfile, params.keypass);
+	if (!pkey)
 		return -1;
 
 	hdr = (struct signature_v2_hdr *)sig;

@@ -965,31 +971,37 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 
 	hdr->hash_algo = get_hash_algo(algo);
 
-	calc_keyid_v2(&hdr->keyid, name, key);
+	calc_pkeyid_v2(&hdr->keyid, name, pkey);
 
-	asn1 = &RSA_ASN1_templates[hdr->hash_algo];
-
-	buf = malloc(size + asn1->size);
-	if (!buf)
-		goto out;
-
-	memcpy(buf, asn1->data, asn1->size);
-	memcpy(buf + asn1->size, hash, size);
-	len = RSA_private_encrypt(size + asn1->size, buf, hdr->sig,
-				  key, RSA_PKCS1_PADDING);
-	if (len < 0) {
-		log_err("RSA_private_encrypt() failed: %d\n", len);
-		goto out;
-	}
+	st = "EVP_PKEY_CTX_new";
+	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
+		goto err;
+	st = "EVP_PKEY_sign_init";
+	if (!EVP_PKEY_sign_init(ctx))
+		goto err;
+	st = "EVP_get_digestbyname";
+	if (!(md = EVP_get_digestbyname(params.hash_algo)))
+		goto err;
+	st = "EVP_PKEY_CTX_set_signature_md";
+	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
+		goto err;
+	st = "EVP_PKEY_sign";
+	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
+	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
+		goto err;
+	len = (int)sigsize;
 
 	/* we add bit length of the signature to make it gnupg compatible */
 	hdr->sig_size = __cpu_to_be16(len);
 	len += sizeof(*hdr);
 	log_info("evm/ima signature: %d bytes\n", len);
-out:
-	if (buf)
-		free(buf);
-	RSA_free(key);
+
+err:
+	if (len == -1)
+		log_err("sign_hash_v2: signing failed: (%s) in %s\n",
+			ERR_reason_error_string(ERR_peek_error()), st);
+	EVP_PKEY_CTX_free(ctx);
+	EVP_PKEY_free(pkey);
 	return len;
 }

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:51 UTC #6 lore raw

PATCH v8 6/9 ima-evm-utils: Replace calc_keyid_v2 with calc_pkeyid_v2

In reply to Vitaly Chikunov

Finish conversion of calc keyid v2 to EVP_PKEY API. After sign_hash_v2() is switched to EVP_PKEY API (in previous commit), older RSA-specific calc_keyid_v2() does not needed anymore and can be replaced with calc_pkeyid_v2().

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

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

src/evmctl.c +1 -1

index eed8f9a..354d731 100644
@@ -934,7 +934,7 @@ static int cmd_import(struct command *cmd)
 			EVP_PKEY_free(pkey);
 			return 1;
 		}
-		calc_pkeyid_v2((uint32_t *)keyid, name, pkey);
+		calc_keyid_v2((uint32_t *)keyid, name, pkey);
 		EVP_PKEY_free(pkey);
 	} else {
 		RSA *key = read_pub_key(inkey, params.x509);

src/imaevm.h +1 -2

index 48d2663..9af43a2 100644
@@ -219,8 +219,7 @@ RSA *read_pub_key(const char *keyfile, int x509);
 EVP_PKEY *read_pub_pkey(const char *keyfile, int x509);
 
 void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len);
-void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key);
-void calc_pkeyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey);
+void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey);
 int key2bin(RSA *key, unsigned char *pub);
 
 int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);

src/libimaevm.c +3 ~~-25~~

index 213855c..25d5a00 100644
@@ -495,7 +495,7 @@ void init_public_keys(const char *keyfiles)
 			continue;
 		}
 
-		calc_pkeyid_v2(&entry->keyid, entry->name, entry->key);
+		calc_keyid_v2(&entry->keyid, entry->name, entry->key);
 		sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid));
 		log_info("key %d: %s %s\n", i++, entry->name, keyfile);
 		entry->next = public_keys;

@@ -741,32 +741,10 @@ void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len
 		log_info("keyid-v1: %s\n", str);
 }
 
-void calc_keyid_v2(uint32_t *keyid, char *str, RSA *key)
-{
-	uint8_t sha1[SHA_DIGEST_LENGTH];
-	unsigned char *pkey = NULL;
-	int len;
-
-	len = i2d_RSAPublicKey(key, &pkey);
-
-	SHA1(pkey, len, sha1);
-
-	/* sha1[12 - 19] is exactly keyid from gpg file */
-	memcpy(keyid, sha1 + 16, 4);
-	log_debug("keyid: ");
-	log_debug_dump(keyid, 4);
-	sprintf(str, "%x", __be32_to_cpup(keyid));
-
-	if (params.verbose > LOG_INFO)
-		log_info("keyid: %s\n", str);
-
-	free(pkey);
-}
-
 /*
  * Calculate keyid of the public_key part of EVP_PKEY
  */
-void calc_pkeyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey)
+void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey)
 {
 	X509_PUBKEY *pk = NULL;
 	const unsigned char *public_key = NULL;

@@ -971,7 +949,7 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 
 	hdr->hash_algo = get_hash_algo(algo);
 
-	calc_pkeyid_v2(&hdr->keyid, name, pkey);
+	calc_keyid_v2(&hdr->keyid, name, pkey);
 
 	st = "EVP_PKEY_CTX_new";
 	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:51 UTC #7 lore raw

PATCH v8 7/9 ima-evm-utils: Remove RSA_ASN1_templates

In reply to Vitaly Chikunov

RSA_ASN1_templates[] are not needed anymore, because we switched to the generic EVP_PKEY OpenSSL API to generate v2 signatures instead of constructing PKCS1 ourselves.

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

---
 src/imaevm.h    |  1 -
 src/libimaevm.c | 57 ---------------------------------------------------------
 2 files changed, 58 deletions(-)

src/imaevm.h +0 -1

index 9af43a2..dc81a3a 100644
@@ -207,7 +207,6 @@ struct RSA_ASN1_template {
 #define	NUM_PCRS 20
 #define DEFAULT_PCR 10
 
-extern const struct RSA_ASN1_template RSA_ASN1_templates[PKEY_HASH__LAST];
 extern struct libevm_params params;
 
 void do_dump(FILE *fp, const void *ptr, int len, bool cr);

src/libimaevm.c +0 ~~-57~~

63 lines not on this page · raw message

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:51 UTC #8 lore raw

PATCH v8 8/9 ima-evm-utils: Pass status codes from sign and hash functions to the callers

In reply to Vitaly Chikunov

Move sign_hash()/ima_calc_hash()/calc_evm_hmac()/calc_evm_hash() status checking before assert()'ing of their return values, so it can be passed to the upper level callers. Especially useful for showing errors.

Fixes 1d9c279279 ("Define hash and sig buffer sizes and add asserts")

Fixes 9643544701 ("Fix hash buffer overflow in verify_evm and hmac_evm")

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

ima-evm-utils: Fix assert after ima_calc_hash

---
 src/evmctl.c    | 16 ++++++++--------
 src/libimaevm.c |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

src/evmctl.c +8 -8

index 354d731..4e0a831 100644
@@ -514,14 +514,14 @@ static int sign_evm(const char *file, const char *key)
 	int len, err;
 
 	len = calc_evm_hash(file, hash);
-	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;
+	assert(len <= sizeof(hash));
 
 	len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
-	assert(len < sizeof(sig));
 	if (len <= 1)
 		return len;
+	assert(len < sizeof(sig));
 
 	/* add header */
 	len++;

@@ -563,9 +563,9 @@ static int hash_ima(const char *file)
 	}
 
 	len = ima_calc_hash(file, hash + offset);
-	assert(len + offset <= sizeof(hash));
 	if (len <= 1)
 		return len;
+	assert(len + offset <= sizeof(hash));
 
 	len += offset;

@@ -593,14 +593,14 @@ static int sign_ima(const char *file, const char *key)
 	int len, err;
 
 	len = ima_calc_hash(file, hash);
-	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;
+	assert(len <= sizeof(hash));
 
 	len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
-	assert(len < sizeof(sig));
 	if (len <= 1)
 		return len;
+	assert(len < sizeof(sig));
 
 	/* add header */
 	len++;

@@ -724,9 +724,9 @@ static int cmd_sign_hash(struct command *cmd)
 		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;
+		assert(siglen < sizeof(sig));
 
 		fwrite(line, len, 1, stdout);
 		fprintf(stdout, " ");

@@ -778,9 +778,9 @@ static int verify_evm(const char *file)
 	int len;
 
 	mdlen = calc_evm_hash(file, hash);
-	assert(mdlen <= sizeof(hash));
 	if (mdlen <= 1)
 		return mdlen;
+	assert(mdlen <= sizeof(hash));
 
 	len = lgetxattr(file, xattr_evm, sig, sizeof(sig));
 	if (len < 0) {

@@ -1160,9 +1160,9 @@ static int hmac_evm(const char *file, const char *key)
 	int len, err;
 
 	len = calc_evm_hmac(file, key, hash);
-	assert(len <= sizeof(hash));
 	if (len <= 1)
 		return len;
+	assert(len <= sizeof(hash));
 
 	log_info("hmac: ");
 	log_dump(hash, len);

src/libimaevm.c +1 -1

index d8e23a3..caf1237 100644
@@ -618,9 +618,9 @@ 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;
+	assert(hashlen <= sizeof(hash));
 
 	return verify_hash(file, hash, hashlen, sig + 1, siglen - 1);
 }

Signature · 1 line

2.11.0

Vitaly Chikunov

author

2019-07-03 15:51 UTC #9 lore raw

PATCH v8 9/9 ima-evm-utils: Log hash_algo with hash value in verbose mode

In reply to Vitaly Chikunov

It's useful to know not just a hash value but also which algorithm is used.

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

---
 src/libimaevm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

src/libimaevm.c +2 -2

index caf1237..51d6c33 100644
@@ -460,7 +460,7 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
 	const char *st;
 
 	if (params.verbose > LOG_INFO) {
-		log_info("hash: ");
+		log_info("hash(%s): ", params.hash_algo);
 		log_dump(hash, size);
 	}

@@ -880,7 +880,7 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 		return -1;
 	}
 
-	log_info("hash: ");
+	log_info("hash(%s): ", params.hash_algo);
 	log_dump(hash, size);
 
 	pkey = read_priv_pkey(keyfile, params.keypass);

Signature · 1 line

2.11.0

Mimi Zohar

maintainer

2019-07-08 14:55 UTC #10 lore raw

In reply to Vitaly Chikunov

Hi Vitaly,

Quoted 5 lines · Vitaly Chikunov

Convert sign v2 from RSA API (with manual formatting PKCS1) to more generic EVP_PKEY API, allowing to generate more types of OpenSSL supported signatures. This is done to enable EC-RDSA signatures, which are already supported in the Kernel. With some small fixes.

Quoted 7 lines

All patches tested on x86_64 to not break anything.
Changes since v7:
- Fix freeing pkey returned by find_keyid() in verify_hash_v2.
- Fix assert after ima_calc_hash() before its return status check.
- More verbose error log for sign and verify hash v2.
- Diff from v7 is below.

Thank you for the updated patch set.

Mimi