ima-evm-utils: Add --keyid option

Allow user to specify `--keyid @/path/to/cert.pem' to extract keyid from SKID of the certificate file. PEM or DER format is auto-detected.

--keyid' option is reused instead of adding a new option (like possible --cert') to signify to the user it's only keyid extraction and nothing more.

This commit creates ABI change for libimaevm, due to adding new function ima_read_keyid(). Newer clients cannot work with older libimaevm. Together with previous commit it creates backward-incompatible ABI change, thus soname should be incremented on release.

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

---
  README                 |  1 +
  src/evmctl.c           | 22 ++++++++++---
  src/imaevm.h           |  1 +
  src/libimaevm.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
  tests/sign_verify.test |  1 +
  5 files changed, 105 insertions(+), 5 deletions(-)

README +1 -0

index 8cd66e0..0e1f6ba 100644
@@ -49,6 +49,7 @@ OPTIONS
        --rsa          use RSA key type and signing scheme v1
    -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)
        --keyid val    overwrite signature keyid with a value (for signing)
+                     val is a x509 cert file if prefixed with '@'
    -o, --portable     generate portable EVM signatures
    -p, --pass         password for encrypted signing key
    -r, --recursive    recurse into directories (sign)

src/evmctl.c +17 -5

index 8ae5488..6a60599 100644
@@ -42,6 +42,7 @@
  #include <sys/param.h>
  #include <sys/stat.h>
  #include <sys/ioctl.h>
+#include <arpa/inet.h>
  #include <fcntl.h>
  #include <unistd.h>
  #include <stdlib.h>

@@ -57,12 +58,14 @@
  #include <termios.h>
  #include <assert.h>
 
+#include <openssl/asn1.h>
  #include <openssl/sha.h>
  #include <openssl/pem.h>
  #include <openssl/hmac.h>
  #include <openssl/err.h>
  #include <openssl/rsa.h>
  #include <openssl/engine.h>
+#include <openssl/x509v3.h>
  #include "hash_info.h"
  #include "pcr.h"
  #include "utils.h"

@@ -2447,6 +2450,7 @@ static void usage(void)
  		"      --rsa          use RSA key type and signing scheme v1\n"
  		"  -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
  		"      --keyid val    overwrite signature keyid with a value (for signing)\n"
+		"                     val is a x509 cert file if prefixed with '@'\n"
  		"  -o, --portable     generate portable EVM signatures\n"
  		"  -p, --pass         password for encrypted signing key\n"
  		"  -r, --recursive    recurse into directories (sign)\n"

@@ -2572,7 +2576,6 @@ int main(int argc, char *argv[])
  	int err = 0, c, lind;
  	ENGINE *eng = NULL;
  	unsigned long keyid;
-	char *eptr;
 
  #if !(OPENSSL_VERSION_NUMBER < 0x10100000)
  	OPENSSL_init_crypto(

@@ -2718,10 +2721,19 @@ int main(int argc, char *argv[])
  			pcrfile[npcrfile++] = optarg;
  			break;
  		case 143:
-			errno = 0;
-			keyid = strtoul(optarg, &eptr, 16);
-			if (errno || eptr - optarg != strlen(optarg) ||
-			    keyid == ULONG_MAX || keyid > UINT_MAX ||
+			if (optarg[0] == '@') {
+				keyid = ima_read_keyid(optarg + 1, NULL);
+			} else {
+				char *eptr;
+
+				errno = 0;
+				keyid = strtoul(optarg, &eptr, 16);
+				if (eptr - optarg != strlen(optarg) || errno) {
+					log_err("Invalid keyid value.\n");
+					exit(1);
+				}
+			}
+			if (keyid == ULONG_MAX || keyid > UINT_MAX ||
  			    keyid == 0) {
  				log_err("Invalid keyid value.\n");
  				exit(1);

src/imaevm.h +1 -0

index 9f38059..eab7f32 100644
@@ -219,6 +219,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, EVP_PKEY *pkey);
  int key2bin(RSA *key, unsigned char *pub);
+unsigned long ima_read_keyid(const char *certfile, uint32_t *keyid);
 
  int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
  int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);

src/libimaevm.c +1 -0

index 481d29d..a22d9bb 100644
@@ -57,6 +57,7 @@
  #include <openssl/pem.h>
  #include <openssl/evp.h>
  #include <openssl/x509.h>
+#include <openssl/x509v3.h>
  #include <openssl/err.h>
 
  #include "imaevm.h"

src/libimaevm.c +43 -0

+ *           0 - this is a PEM encoded private key file with possible appended
+ *           x509 cert.
+ */
+static unsigned long _ima_read_keyid(const char *certfile, uint32_t *keyid,
+				     enum keyid_file_type file_type)
+{
+	uint32_t keyid_raw;
+	const ASN1_OCTET_STRING *skid;
+	int skid_len;
+	X509 *x = NULL;
+	FILE *fp;
+
+	if (!(fp = fopen(certfile, "r"))) {
+		log_err("read keyid: %s: Cannot open: %s\n", certfile,
+			strerror(errno));
+		return -1;
+	}
+	if (!PEM_read_X509(fp, &x, NULL, NULL)) {
+		if (ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE) {
+			ERR_clear_error();
+			if (file_type == KEYID_FILE_PEM_KEY) {
+				log_debug("%s: x509 certificate not found\n",
+					  certfile);
+				fclose(fp);
+				return -1;
+			}
+			rewind(fp);
+			d2i_X509_fp(fp, &x);
+		}
+		if (!x) {
+			ERR_print_errors_fp(stderr);
+			log_err("read keyid: %s: Error reading x509 certificate\n",
+				certfile);
+			fclose(fp);
+			return -1;
+		}
+	}
+	fclose(fp);
+
+	if (!(skid = X509_get0_subject_key_id(x))) {
+		log_err("read keyid: %s: SKID not found\n", certfile);
+		goto err_free;
+	}

src/libimaevm.c +6 -0

+
+err_free:
+	X509_free(x);
+	return -1;
+}
+

src/libimaevm.c +6 -0

+ */
+unsigned long ima_read_keyid(const char *certfile, uint32_t *keyid)
+{
+	return _ima_read_keyid(certfile, keyid, KEYID_FILE_UNK_CERT);
+}
+
  static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
  {
  	FILE *fp;

tests/sign_verify.test +1 -0

index 2c21812..52ea33a 100755
@@ -360,6 +360,7 @@ sign_verify  rsa1024  md5     0x030201:K:0080
  sign_verify  rsa1024  sha1    0x030202:K:0080
  sign_verify  rsa1024  sha224  0x030207:K:0080
  expect_pass check_sign TYPE=ima KEY=rsa1024 ALG=sha256 PREFIX=0x030204aabbccdd0080 OPTS=--keyid=aabbccdd
+expect_pass check_sign TYPE=ima KEY=rsa1024 ALG=sha256 PREFIX=0x030204:K:0080 [email protected]
  sign_verify  rsa1024  sha256  0x030204:K:0080
    try_different_keys
    try_different_sigs

Quoted 124 lines · Vitaly Chikunov

Allow user to specify `--keyid @/path/to/cert.pem' to extract keyid from SKID of the certificate file. PEM or DER format is auto-detected.

--keyid' option is reused instead of adding a new option (like possible --cert') to signify to the user it's only keyid extraction and nothing more.

This commit creates ABI change for libimaevm, due to adding new function ima_read_keyid(). Newer clients cannot work with older libimaevm. Together with previous commit it creates backward-incompatible ABI change, thus soname should be incremented on release.

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

---
  README                 |  1 +
  src/evmctl.c           | 22 ++++++++++---
  src/imaevm.h           |  1 +
  src/libimaevm.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
  tests/sign_verify.test |  1 +
  5 files changed, 105 insertions(+), 5 deletions(-)

README +1 -0

index 8cd66e0..0e1f6ba 100644
@@ -49,6 +49,7 @@ OPTIONS
        --rsa          use RSA key type and signing scheme v1
    -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)
        --keyid val    overwrite signature keyid with a value (for signing)
+                     val is a x509 cert file if prefixed with '@'
    -o, --portable     generate portable EVM signatures
    -p, --pass         password for encrypted signing key
    -r, --recursive    recurse into directories (sign)

src/evmctl.c +17 -5

index 8ae5488..6a60599 100644
@@ -42,6 +42,7 @@
  #include <sys/param.h>
  #include <sys/stat.h>
  #include <sys/ioctl.h>
+#include <arpa/inet.h>
  #include <fcntl.h>
  #include <unistd.h>
  #include <stdlib.h>

@@ -57,12 +58,14 @@
  #include <termios.h>
  #include <assert.h>
+#include <openssl/asn1.h>
  #include <openssl/sha.h>
  #include <openssl/pem.h>
  #include <openssl/hmac.h>
  #include <openssl/err.h>
  #include <openssl/rsa.h>
  #include <openssl/engine.h>
+#include <openssl/x509v3.h>
  #include "hash_info.h"
  #include "pcr.h"
  #include "utils.h"

@@ -2447,6 +2450,7 @@ static void usage(void)
  		"      --rsa          use RSA key type and signing scheme v1\n"
  		"  -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
  		"      --keyid val    overwrite signature keyid with a value (for signing)\n"
+		"                     val is a x509 cert file if prefixed with '@'\n"
  		"  -o, --portable     generate portable EVM signatures\n"
  		"  -p, --pass         password for encrypted signing key\n"
  		"  -r, --recursive    recurse into directories (sign)\n"

@@ -2572,7 +2576,6 @@ int main(int argc, char *argv[])
  	int err = 0, c, lind;
  	ENGINE *eng = NULL;
  	unsigned long keyid;
-	char *eptr;
  #if !(OPENSSL_VERSION_NUMBER < 0x10100000)
  	OPENSSL_init_crypto(

@@ -2718,10 +2721,19 @@ int main(int argc, char *argv[])
  			pcrfile[npcrfile++] = optarg;
  			break;
  		case 143:
-			errno = 0;
-			keyid = strtoul(optarg, &eptr, 16);
-			if (errno || eptr - optarg != strlen(optarg) ||
-			    keyid == ULONG_MAX || keyid > UINT_MAX ||
+			if (optarg[0] == '@') {
+				keyid = ima_read_keyid(optarg + 1, NULL);
+			} else {
+				char *eptr;
+
+				errno = 0;
+				keyid = strtoul(optarg, &eptr, 16);
+				if (eptr - optarg != strlen(optarg) || errno) {
+					log_err("Invalid keyid value.\n");
+					exit(1);
+				}
+			}
+			if (keyid == ULONG_MAX || keyid > UINT_MAX ||
  			    keyid == 0) {
  				log_err("Invalid keyid value.\n");
  				exit(1);

src/imaevm.h +1 -0

index 9f38059..eab7f32 100644
@@ -219,6 +219,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, EVP_PKEY *pkey);
  int key2bin(RSA *key, unsigned char *pub);
+unsigned long ima_read_keyid(const char *certfile, uint32_t *keyid);
  int sign_hash(const char *algo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig);
  int verify_hash(const char *file, const unsigned char *hash, int size, unsigned char *sig, int siglen);

src/libimaevm.c +2 -0

index 481d29d..a22d9bb 100644
@@ -57,6 +57,7 @@
  #include <openssl/pem.h>
  #include <openssl/evp.h>
  #include <openssl/x509.h>
+#include <openssl/x509v3.h>
  #include <openssl/err.h>
  #include "imaevm.h"

@@ -748,6 +749,90 @@ void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey)
  	X509_PUBKEY_free(pk);
  }
+enum keyid_file_type {

src/libimaevm.c void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey) · line 758

Quoted 6 lines

src/libimaevm.c +6 -0

+	KEYID_FILE_PEM_KEY = 0,
+	KEYID_FILE_UNK_CERT,
+};
+
+/*
+ * @is_cert: 1 - this is a x509 cert file (maybe PEM, maybe DER encoded);

This probably change since you wrote it. Is this file_type now?

Quoted 49 lines

src/libimaevm.c +49 -0

+ *           0 - this is a PEM encoded private key file with possible appended
+ *           x509 cert.
+ */
+static unsigned long _ima_read_keyid(const char *certfile, uint32_t *keyid,
+				     enum keyid_file_type file_type)
+{
+	uint32_t keyid_raw;
+	const ASN1_OCTET_STRING *skid;
+	int skid_len;
+	X509 *x = NULL;
+	FILE *fp;
+
+	if (!(fp = fopen(certfile, "r"))) {
+		log_err("read keyid: %s: Cannot open: %s\n", certfile,
+			strerror(errno));
+		return -1;
+	}
+	if (!PEM_read_X509(fp, &x, NULL, NULL)) {
+		if (ERR_GET_REASON(ERR_peek_last_error()) == PEM_R_NO_START_LINE) {
+			ERR_clear_error();
+			if (file_type == KEYID_FILE_PEM_KEY) {
+				log_debug("%s: x509 certificate not found\n",
+					  certfile);
+				fclose(fp);
+				return -1;
+			}
+			rewind(fp);
+			d2i_X509_fp(fp, &x);
+		}
+		if (!x) {
+			ERR_print_errors_fp(stderr);
+			log_err("read keyid: %s: Error reading x509 certificate\n",
+				certfile);
+			fclose(fp);
+			return -1;
+		}
+	}
+	fclose(fp);
+
+	if (!(skid = X509_get0_subject_key_id(x))) {
+		log_err("read keyid: %s: SKID not found\n", certfile);
+		goto err_free;
+	}
+	skid_len = ASN1_STRING_length(skid);
+	if (skid_len < sizeof(keyid_raw)) {
+		log_err("read keyid: %s: SKID too short (len %d)\n", certfile,
+			skid_len);
+		goto err_free;
+	}

src/libimaevm.c void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey) · line 811

Quoted 4 lines

src/libimaevm.c +4 -0

+	memcpy(&keyid_raw, ASN1_STRING_get0_data(skid) + skid_len
+	       - sizeof(keyid_raw), sizeof(keyid_raw));
+	if (keyid)
+		memcpy(keyid, &keyid_raw, sizeof(*keyid));

If keyid is supposed to be in native format then you have to apply ntohl() to it..

src/libimaevm.c void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey) · line 813

Quoted 4 lines

src/libimaevm.c +2 -0

+	log_info("keyid %04x (from %s)\n", ntohl(keyid_raw), certfile);
+	return ntohl(keyid_raw);

I thought this should return 0  ?

Quoted 12 lines

src/libimaevm.c +10 -0

+
+err_free:
+	X509_free(x);
+	return -1;
+}
+
+/**
+ * ima_read_keyid() - Read 32-bit keyid from the cert file.
+ * @certfile:	File possibly containing certificate in DER/PEM format.
+ * @keyid:	Output keyid in network order.

src/libimaevm.c void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey) · line 829

Quoted 6 lines

src/libimaevm.c +6 -0

+ *
+ * Try to read keyid from Subject Key Identifier (SKID) of certificate.
+ * Autodetect if cert is in PEM or DER encoding.
+ *
+ * Return: -1 (ULONG_MAX) on error;
+ *         32-bit keyid as unsigned integer in host order.

That's confusing, two times the same result, one time in host order, on time in network order. Why not just one return value in host order?

Quoted 21 lines

src/libimaevm.c +6 -0

+ */
+unsigned long ima_read_keyid(const char *certfile, uint32_t *keyid)
+{
+	return _ima_read_keyid(certfile, keyid, KEYID_FILE_UNK_CERT);
+}
+
  static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
  {
  	FILE *fp;

tests/sign_verify.test +1 -0

index 2c21812..52ea33a 100755
@@ -360,6 +360,7 @@ sign_verify  rsa1024  md5     0x030201:K:0080
  sign_verify  rsa1024  sha1    0x030202:K:0080
  sign_verify  rsa1024  sha224  0x030207:K:0080
  expect_pass check_sign TYPE=ima KEY=rsa1024 ALG=sha256 PREFIX=0x030204aabbccdd0080 OPTS=--keyid=aabbccdd
+expect_pass check_sign TYPE=ima KEY=rsa1024 ALG=sha256 PREFIX=0x030204:K:0080 [email protected]
  sign_verify  rsa1024  sha256  0x030204:K:0080
    try_different_keys
    try_different_sigs

Quoted 32 lines · Stefan Berger

Quoted 21 lines · Vitaly Chikunov

Allow user to specify `--keyid @/path/to/cert.pem' to extract keyid from SKID of the certificate file. PEM or DER format is auto-detected.

--keyid' option is reused instead of adding a new option (like possible --cert') to signify to the user it's only keyid extraction and nothing more.

This commit creates ABI change for libimaevm, due to adding new function ima_read_keyid(). Newer clients cannot work with older libimaevm. Together with previous commit it creates backward-incompatible ABI change, thus soname should be incremented on release.

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

---
  README                 |  1 +
  src/evmctl.c           | 22 ++++++++++---
  src/imaevm.h           |  1 +
  src/libimaevm.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
  tests/sign_verify.test |  1 +
  5 files changed, 105 insertions(+), 5 deletions(-)

Quoted 10 lines

+/**
+ * ima_read_keyid() - Read 32-bit keyid from the cert file.
+ * @certfile: File possibly containing certificate in DER/PEM format.
+ * @keyid: Output keyid in network order.
+ *
+ * Try to read keyid from Subject Key Identifier (SKID) of certificate.
+ * Autodetect if cert is in PEM or DER encoding.
+ *
+ * Return: -1 (ULONG_MAX) on error;
+ * 32-bit keyid as unsigned integer in host order.

Quoted 2 lines

That's confusing, two times the same result, one time in host order, on time in network order. Why not just one return value in host order?

Pointer API is similar to calc_keyid_v2().

Do you sugegst to change calc_keyid_v2() API too?

To introduce non-confusing API that contradict other parts of API would be more confusing than it already is.

Stefan, Mimi,

Quoted 33 lines · Vitaly Chikunov

Quoted 32 lines · Stefan Berger

Quoted 31 lines · Vitaly Chikunov

Allow user to specify `--keyid @/path/to/cert.pem' to extract keyid from SKID of the certificate file. PEM or DER format is auto-detected.

--keyid' option is reused instead of adding a new option (like possible --cert') to signify to the user it's only keyid extraction and nothing more.

This commit creates ABI change for libimaevm, due to adding new function ima_read_keyid(). Newer clients cannot work with older libimaevm. Together with previous commit it creates backward-incompatible ABI change, thus soname should be incremented on release.

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

---
   README                 |  1 +
   src/evmctl.c           | 22 ++++++++++---
   src/imaevm.h           |  1 +
   src/libimaevm.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
   tests/sign_verify.test |  1 +
   5 files changed, 105 insertions(+), 5 deletions(-)

+/**
+ * ima_read_keyid() - Read 32-bit keyid from the cert file.
+ * @certfile: File possibly containing certificate in DER/PEM format.
+ * @keyid: Output keyid in network order.
+ *
+ * Try to read keyid from Subject Key Identifier (SKID) of certificate.
+ * Autodetect if cert is in PEM or DER encoding.
+ *
+ * Return: -1 (ULONG_MAX) on error;
+ * 32-bit keyid as unsigned integer in host order.

Quoted 8 lines

Quoted 2 lines

That's confusing, two times the same result, one time in host order, on time in network order. Why not just one return value in host order?

Pointer API is similar to calc_keyid_v2().

Do you sugegst to change calc_keyid_v2() API too?

To introduce non-confusing API that contradict other parts of API would be more confusing than it already is.

Maybe we could change this libimaevm API:

   void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey);

to

   void calc_keyid_v2(uint8_t *keyid, char *str, EVP_PKEY *pkey);

To signal to the user that there it's not just uint32_t, but some byte array (possible in network order). This would not even break ABI, only API. (But, we breaking ABI with this patch set anyway.)

Thanks,

Quoted 44 lines · Vitaly Chikunov

Quoted 33 lines · Vitaly Chikunov

Quoted 32 lines · Stefan Berger

Quoted 31 lines · Vitaly Chikunov

Allow user to specify `--keyid @/path/to/cert.pem' to extract keyid from SKID of the certificate file. PEM or DER format is auto-detected.

--keyid' option is reused instead of adding a new option (like possible --cert') to signify to the user it's only keyid extraction and nothing more.

This commit creates ABI change for libimaevm, due to adding new function ima_read_keyid(). Newer clients cannot work with older libimaevm. Together with previous commit it creates backward-incompatible ABI change, thus soname should be incremented on release.

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

---
   README                 |  1 +
   src/evmctl.c           | 22 ++++++++++---
   src/imaevm.h           |  1 +
   src/libimaevm.c        | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
   tests/sign_verify.test |  1 +
   5 files changed, 105 insertions(+), 5 deletions(-)

+/**
+ * ima_read_keyid() - Read 32-bit keyid from the cert file.
+ * @certfile: File possibly containing certificate in DER/PEM format.
+ * @keyid: Output keyid in network order.
+ *
+ * Try to read keyid from Subject Key Identifier (SKID) of certificate.
+ * Autodetect if cert is in PEM or DER encoding.
+ *
+ * Return: -1 (ULONG_MAX) on error;
+ * 32-bit keyid as unsigned integer in host order.

Quoted 8 lines

Quoted 2 lines

That's confusing, two times the same result, one time in host order, on time in network order. Why not just one return value in host order?

Pointer API is similar to calc_keyid_v2().

Do you sugegst to change calc_keyid_v2() API too?

To introduce non-confusing API that contradict other parts of API would be more confusing than it already is.

Maybe we could change this libimaevm API:

Quoted 5 lines

   void calc_keyid_v2(uint32_t *keyid, char *str, EVP_PKEY *pkey);

to

   void calc_keyid_v2(uint8_t *keyid, char *str, EVP_PKEY *pkey);

I think it's better to leave it... :-(

Quoted 5 lines

To signal to the user that there it's not just uint32_t, but some byte array (possible in network order). This would not even break ABI, only API. (But, we breaking ABI with this patch set anyway.)

You mean we are breaking it by introducing this extensions here?

@ -196,6 +196,7 @@ struct libimaevm_params {

(quoted diff) +1 -0

 	const char *hash_algo;
 	const char *keyfile;
 	const char *keypass;
+	uint32_t keyid;		/* keyid overriding value, unless 0. */
 };

Quoted 4 lines

Thanks,

Quoted 3 lines

That's confusing, two times the same result, one time in host order, on time in network order. Why not just one return value in host order?

Pointer API is similar to calc_keyid_v2().

Do you sugegst to change calc_keyid_v2() API too?

To introduce non-confusing API that contradict other parts of API would be more confusing than it already is.

Allow to have certificate appended to the private key of `--key' specified (PEM) file (for v2 signing) to facilitate reading of keyid from the associated cert. This will allow users to have private and public key as a single file. There is no check that public key form the cert matches associated private key.

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

---
  README          | 3 +++
  src/libimaevm.c | 8 +++++---
  2 files changed, 8 insertions(+), 3 deletions(-)

README +3 -0

index 0e1f6ba..ea11bde 100644
@@ -127,6 +127,9 @@ for signing and importing the key.
  Second key format uses X509 DER encoded public key certificates and uses asymmetric key support
  in the kernel (since kernel 3.9). CONFIG_INTEGRITY_ASYMMETRIC_KEYS must be enabled (default).
 
+For v2 signatures x509 certificate with the public key could be appended to the private
+key (both are in PEM format) to properly determine its Subject Key Identifier (SKID).
+
 
  Integrity keyrings
  ----------------

src/libimaevm.c +0 -0

index a22d9bb..ac4bb46 100644
@@ -1017,10 +1017,12 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
  		return -1;
  	}

src/libimaevm.c +0 -0

  	st = "EVP_PKEY_CTX_new";
  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))

Quoted 38 lines · Vitaly Chikunov

Allow to have certificate appended to the private key of `--key' specified (PEM) file (for v2 signing) to facilitate reading of keyid from the associated cert. This will allow users to have private and public key as a single file. There is no check that public key form the cert matches associated private key.

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

---
  README          | 3 +++
  src/libimaevm.c | 8 +++++---
  2 files changed, 8 insertions(+), 3 deletions(-)

README +3 -0

index 0e1f6ba..ea11bde 100644
@@ -127,6 +127,9 @@ for signing and importing the key.
  Second key format uses X509 DER encoded public key certificates and uses asymmetric key support
  in the kernel (since kernel 3.9). CONFIG_INTEGRITY_ASYMMETRIC_KEYS must be enabled (default).
+For v2 signatures x509 certificate with the public key could be appended to the private
+key (both are in PEM format) to properly determine its Subject Key Identifier (SKID).
+
  Integrity keyrings
  ----------------

src/libimaevm.c +3 -3

index a22d9bb..ac4bb46 100644
@@ -1017,10 +1017,12 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
  		return -1;
  	}
-	if (imaevm_params.keyid)
+	if (imaevm_params.keyid) {
  		hdr->keyid = htonl(imaevm_params.keyid);
-	else
-		calc_keyid_v2(&hdr->keyid, name, pkey);
+	} else {
+		if (_ima_read_keyid(keyfile, &hdr->keyid, KEYID_FILE_PEM_KEY) == ULONG_MAX)

src/libimaevm.c static int sign_hash_v2(const char *algo, const unsigned char *hash, · line 1025

Quoted 2 lines

src/libimaevm.c +2 -0

+			calc_keyid_v2(&hdr->keyid, name, pkey);
+	}

It might be convenient here to just write the result in network byte order into the header but for a library API I find it not so nice, but then there's calc_keyid_v2 that does that already... I just wouldn't expect that these parameter are in big endian order already, I would expect them in native order.

So maybe ima_read_keyid should just return ULONG_MAX or the keyid in host order and it call _ima_read_keyid() with a NULL pointer or a dummy variable as keyid that the library API caller doesn't see.

   Stefan

Quoted 2 lines

src/libimaevm.c +0 -0

  	st = "EVP_PKEY_CTX_new";
  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))

Quoted 40 lines · Stefan Berger

Quoted 38 lines · Vitaly Chikunov

Allow to have certificate appended to the private key of `--key' specified (PEM) file (for v2 signing) to facilitate reading of keyid from the associated cert. This will allow users to have private and public key as a single file. There is no check that public key form the cert matches associated private key.

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

---
  README          | 3 +++
  src/libimaevm.c | 8 +++++---
  2 files changed, 8 insertions(+), 3 deletions(-)

README +3 -0

index 0e1f6ba..ea11bde 100644
@@ -127,6 +127,9 @@ for signing and importing the key.
  Second key format uses X509 DER encoded public key certificates and uses asymmetric key support
  in the kernel (since kernel 3.9). CONFIG_INTEGRITY_ASYMMETRIC_KEYS must be enabled (default).
+For v2 signatures x509 certificate with the public key could be appended to the private
+key (both are in PEM format) to properly determine its Subject Key Identifier (SKID).
+
  Integrity keyrings
  ----------------

src/libimaevm.c +3 -3

index a22d9bb..ac4bb46 100644
@@ -1017,10 +1017,12 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
  		return -1;
  	}
-	if (imaevm_params.keyid)
+	if (imaevm_params.keyid) {
  		hdr->keyid = htonl(imaevm_params.keyid);
-	else
-		calc_keyid_v2(&hdr->keyid, name, pkey);
+	} else {
+		if (_ima_read_keyid(keyfile, &hdr->keyid, KEYID_FILE_PEM_KEY) == ULONG_MAX)

Quoted 8 lines

src/libimaevm.c static int sign_hash_v2(const char *algo, const unsigned char *hash, · line 1025

Quoted 2 lines

src/libimaevm.c +2 -0

+			calc_keyid_v2(&hdr->keyid, name, pkey);
+	}

It might be convenient here to just write the result in network byte order into the header but for a library API I find it not so nice, but then there's calc_keyid_v2 that does that already... I just wouldn't expect that these parameter are in big endian order already, I would expect them in native order.

I expect them in network order, similar to how calc_keyid_v2() already writes it one line below. So, why should we mix orders? Both functions write keyids, so it's not like completely different parts of API. Also, it's documented that ima_read_keyid() writes to the pointer in network order (and returns integer in host order), so I don't see the problem. Thus, I would prefer not to follow this suggestion.

Thanks,

Quoted 3 lines

So maybe ima_read_keyid should just return ULONG_MAX or the keyid in host order and it call _ima_read_keyid() with a NULL pointer or a dummy variable as keyid that the library API caller doesn't see.

Quoted 6 lines

   Stefan

Quoted 2 lines

src/libimaevm.c +0 -0

  	st = "EVP_PKEY_CTX_new";
  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))

Stefan,

Quoted 47 lines · Vitaly Chikunov

Quoted 38 lines · Stefan Berger

Quoted 37 lines · Vitaly Chikunov

Allow to have certificate appended to the private key of `--key' specified (PEM) file (for v2 signing) to facilitate reading of keyid from the associated cert. This will allow users to have private and public key as a single file. There is no check that public key form the cert matches associated private key.

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

---
   README          | 3 +++
   src/libimaevm.c | 8 +++++---
   2 files changed, 8 insertions(+), 3 deletions(-)

README +3 -0

index 0e1f6ba..ea11bde 100644
@@ -127,6 +127,9 @@ for signing and importing the key.
   Second key format uses X509 DER encoded public key certificates and uses asymmetric key support
   in the kernel (since kernel 3.9). CONFIG_INTEGRITY_ASYMMETRIC_KEYS must be enabled (default).
+For v2 signatures x509 certificate with the public key could be appended to the private
+key (both are in PEM format) to properly determine its Subject Key Identifier (SKID).
+
   Integrity keyrings
   ----------------

src/libimaevm.c +2 -3

index a22d9bb..ac4bb46 100644
@@ -1017,10 +1017,12 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
   		return -1;
   	}
-	if (imaevm_params.keyid)
+	if (imaevm_params.keyid) {
   		hdr->keyid = htonl(imaevm_params.keyid);
-	else
-		calc_keyid_v2(&hdr->keyid, name, pkey);
+	} else {

Quoted 8 lines

src/libimaevm.c static int sign_hash_v2(const char *algo, const unsigned char *hash, · line 1025

Quoted 3 lines

src/libimaevm.c +3 -0

+		if (_ima_read_keyid(keyfile, &hdr->keyid, KEYID_FILE_PEM_KEY) == ULONG_MAX)
+			calc_keyid_v2(&hdr->keyid, name, pkey);
+	}

It might be convenient here to just write the result in network byte order into the header but for a library API I find it not so nice, but then there's calc_keyid_v2 that does that already... I just wouldn't expect that these parameter are in big endian order already, I would expect them in native order.

Quoted 12 lines

I expect them in network order, similar to how calc_keyid_v2() already writes it one line below. So, why should we mix orders? Both functions write keyids, so it's not like completely different parts of API. Also, it's documented that ima_read_keyid() writes to the pointer in network order (and returns integer in host order), so I don't see the problem. Thus, I would prefer not to follow this suggestion.

Thanks,

Quoted 3 lines

So maybe ima_read_keyid should just return ULONG_MAX or the keyid in host order and it call _ima_read_keyid() with a NULL pointer or a dummy variable as keyid that the library API caller doesn't see.

So there will be exported

    uint32_t ima_read_keyid(char *certfile);

and internal
static bool _read_keyid(char *certfile, uint32_t *keyid)

ima_read_keyid will wrap read_keyid and return keyid via intermediate variable. Right?