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

PATCH Superseded Vitaly Chikunov 2 participants 6 messages Mar 23, 2019 – Jun 12, 2019 Keys-trusted

Share & tools

Vitaly Chikunov

author

2019-03-23 02:56 UTC #0 lore raw

PATCH v3 1/1 ima-evm-utils: Convert sign v2 from RSA to EVP_PKEY API

Convert sign_v2 and related to using EVP_PKEY API instead of RSA API. This enables more signatures to work out of the box.

Remove RSA_ASN1_templates[] as it does not needed anymore. OpenSSL sign is doing proper PKCS1 padding automatically (tested to be compatible with previous version, except for MD4). This also fixes bug with MD4 which produced wrong signature because of absence of the appropriate RSA_ASN1_template.

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

---
Changes since v2:
- Rebase after:
[PATCH v3] ima-evm-utils: Rework openssl init
[PATCH 2/2] ima-evm-utils: try to load digest by its alias [PATCH 1/2] ima-evm-utils: Extract digest algorithms from hash_info.h

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

 src/evmctl.c    |  29 +++---
 src/imaevm.h    |   4 +-
 src/libimaevm.c | 269 ++++++++++++++++++++++++++------------------------------
 3 files changed, 146 insertions(+), 156 deletions(-)

src/evmctl.c +17 ~~-12~~

index 15a7226..7c398f8 100644
@@ -510,7 +510,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
 static int sign_evm(const char *file, const char *key)
 {
 	unsigned char hash[MAX_DIGEST_SIZE];
-	unsigned char sig[MAX_SIGNATURE_SIZE];
+	unsigned char sig[MAX_SIGNATURE_SIZE + 1];
 	int len, err;
 
 	len = calc_evm_hash(file, hash);

@@ -519,7 +519,7 @@ static int sign_evm(const char *file, const char *key)
 		return len;
 
 	len = sign_hash(params.hash_algo, hash, len, key, NULL, sig + 1);
-	assert(len < sizeof(sig));
+	assert(len < MAX_SIGNATURE_SIZE);
 	if (len <= 1)
 		return len;

@@ -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_keyid_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 +2 -2

index c81bf21..31d05ae 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);

@@ -216,9 +215,10 @@ 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);
+void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *key);
 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 +127 ~~-142~~

399 lines not on this page · raw message

Signature · 1 line

2.11.0

Mimi Zohar

maintainer

2019-05-28 18:58 UTC #1 lore raw

Hi Vitaly,

Quoted 8 lines · Vitaly Chikunov

Convert sign_v2 and related to using EVP_PKEY API instead of RSA API. This enables more signatures to work out of the box.
Remove RSA_ASN1_templates[] as it does not needed anymore. OpenSSL sign is doing proper PKCS1 padding automatically (tested to be compatible with previous version, except for MD4). This also fixes bug with MD4 which produced wrong signature because of absence of the appropriate RSA_ASN1_template.

Is there any way of breaking this patch up to simplify review?

Mimi

Vitaly Chikunov

author

2019-05-28 22:47 UTC #2 lore raw

Mimi,

Quoted 11 lines · Mimi Zohar

Quoted 8 lines · Vitaly Chikunov
Convert sign_v2 and related to using EVP_PKEY API instead of RSA API. This enables more signatures to work out of the box.
Remove RSA_ASN1_templates[] as it does not needed anymore. OpenSSL sign is doing proper PKCS1 padding automatically (tested to be compatible with previous version, except for MD4). This also fixes bug with MD4 which produced wrong signature because of absence of the appropriate RSA_ASN1_template.
Is there any way of breaking this patch up to simplify review?

Hm. The main change is to replace key type from RSA with more abstract EVP_PKEY. All other changes are a consequence of it.

And because keys are now EVP_PKEY the templates are removed too, now that we are not dealing with keys on the too low level anymore.

I already tried to leave RSA handling as is for v1 signatures, because they are RSA specific anyway.

Also, I tried to leave most (external) API the same, except calc_keyid_v2 which now gets EVP_PKEY instead of RSA. Internally, find_keyid now returns EVP_PKEY too.

read_pub_key now extracts RSA from EVP_PKEY from read_pub_pkey.

And calc_keyid_v2 now works internally slightly differently (and generally) to handle all possible key types.

Also, I run some tests with ASan.

Thanks,

Quoted 2 lines

Mimi

Mimi Zohar

maintainer

2019-05-28 23:31 UTC #3 lore raw

Quoted 13 lines · Vitaly Chikunov

Mimi,
Quoted 10 lines · Mimi Zohar
Quoted 8 lines · Vitaly Chikunov
Convert sign_v2 and related to using EVP_PKEY API instead of RSA API. This enables more signatures to work out of the box.
Remove RSA_ASN1_templates[] as it does not needed anymore. OpenSSL sign is doing proper PKCS1 padding automatically (tested to be compatible with previous version, except for MD4). This also fixes bug with MD4 which produced wrong signature because of absence of the appropriate RSA_ASN1_template.

Quoted 4 lines

Quoted 1 line
Is there any way of breaking this patch up to simplify review?
Hm. The main change is to replace key type from RSA with more abstract EVP_PKEY. All other changes are a consequence of it.

Yes, I understand.

Quoted 3 lines

And because keys are now EVP_PKEY the templates are removed too, now that we are not dealing with keys on the too low level anymore.

There's no reason that removing RSA_ASN1_templates[] needs to be in the same patch as the pkey change, nor does the MAX_SIGNATURE_SIZE changes in sign_evm().

Quoted 9 lines

I already tried to leave RSA handling as is for v1 signatures, because they are RSA specific anyway.
Also, I tried to leave most (external) API the same, except calc_keyid_v2 which now gets EVP_PKEY instead of RSA. Internally, find_keyid now returns EVP_PKEY too.
read_pub_key now extracts RSA from EVP_PKEY from read_pub_pkey.

Right. So why couldn't the first patch define read_pub_pkey(), but only call it from read_pub_key(). Then subsequent patches could call read_pub_pkey() directly.

Mimi

Quoted 6 lines

And calc_keyid_v2 now works internally slightly differently (and generally) to handle all possible key types.
Also, I run some tests with ASan.
Thanks,

Mimi Zohar

maintainer

2019-06-12 14:30 UTC #4 lore raw

Hi Vitaly,

Quoted 10 lines · Mimi Zohar

Quoted 7 lines · Vitaly Chikunov
I already tried to leave RSA handling as is for v1 signatures, because they are RSA specific anyway.
Also, I tried to leave most (external) API the same, except calc_keyid_v2 which now gets EVP_PKEY instead of RSA. Internally, find_keyid now returns EVP_PKEY too.

Quoted 11 lines

Quoted 1 line
read_pub_key now extracts RSA from EVP_PKEY from read_pub_pkey.
Right. So why couldn't the first patch define read_pub_pkey(), but only call it from read_pub_key(). Then subsequent patches could call read_pub_pkey() directly.
Quoted 5 lines
And calc_keyid_v2 now works internally slightly differently (and generally) to handle all possible key types.
Also, I run some tests with ASan.

Releasing a new version of ima-evm-utils is way over due. I'd really like to release a new version of ima-evm-utils soon. Are you planning on breaking this patch up?

Mimi

Vitaly Chikunov

author

2019-06-12 14:46 UTC #5 lore raw

Mimi,

Quoted 18 lines · Mimi Zohar

Quoted 17 lines · Mimi Zohar
Quoted 8 lines · Vitaly Chikunov
I already tried to leave RSA handling as is for v1 signatures, because they are RSA specific anyway.
Also, I tried to leave most (external) API the same, except calc_keyid_v2 which now gets EVP_PKEY instead of RSA. Internally, find_keyid now returns EVP_PKEY too.
read_pub_key now extracts RSA from EVP_PKEY from read_pub_pkey.
Right. So why couldn't the first patch define read_pub_pkey(), but only call it from read_pub_key(). Then subsequent patches could call read_pub_pkey() directly.
Quoted 1 line

Quoted 8 lines

Quoted 4 lines
Quoted 4 lines
And calc_keyid_v2 now works internally slightly differently (and generally) to handle all possible key types.
Also, I run some tests with ASan.
Releasing a new version of ima-evm-utils is way over due. I'd really like to release a new version of ima-evm-utils soon. Are you planning on breaking this patch up?

Yes. Sorry for a delay. I will sent it soon.

Thanks,