Skip to contentSkip to conversation

ima-evm-utils: Implement function to only create IMA signature

PATCH In review Stefan Berger 3 participants 8 messages Apr 27, 2021 – May 10, 2021 Keys-trusted

lore
Share & tools
lore
#0 lore raw

v2 cover ima-evm-utils: Implement function to only create IMA signature

This series of patches implements imaevm_create_ima_signature, a function that only creates an IMA v2 signature but does not write it into an xattr. This API fullfills a requirement of rpm-ostree where a tool is being built to sign all files from something similar to a tar byte stream, though where files are written to temporary files for signing purpose but the signature itself should not be written into the filesystem's xattr but simply returned via the API.

Stefan

v2:
- use imaevm_calc_hash to also cover hash calculations of links, directories etc.

Stefan Berger (3):
libimaevm: Implement imaevm_create_ima_signature
tests: Add program to create IMA signature with new API call
libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key
file

 src/imaevm.h                 |   2 +
 src/libimaevm.c              | 148 ++++++++++++++++++++++++++++-------
 tests/Makefile.am            |   6 ++
 tests/create_ima_signature.c | 111 ++++++++++++++++++++++++++
 tests/sign_verify.test       |  21 ++++-
 5 files changed, 257 insertions(+), 31 deletions(-)
 create mode 100644 tests/create_ima_signature.c
Signature · 1 line
2.30.2
#1 lore raw

PATCH v2 1/3 libimaevm: Implement imaevm_create_ima_signature

Implement imaevm_create_ima_signature that creates an IMA V2 signature with a given private key and without writing the signature into an extended attribute. This allows a caller to provide a single key for creating a signature on multiple files without having to read the key every time. It also allows the caller to store the signature wherever it wants, which may not necessarily be an extended attribute.

---

v2:
- use imaevm_calc_hash to also cover hash calculations of links, directories etc.

---
 src/imaevm.h    |   2 +
 src/libimaevm.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 141 insertions(+), 4 deletions(-)
src/imaevm.h +2 -0
index 4503919..fc4536b 100644
@@ -225,5 +225,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen, unsig
 void init_public_keys(const char *keyfiles);
 int imaevm_hash_algo_from_sig(unsigned char *sig);
 const char *imaevm_hash_algo_by_id(int algo);
+int imaevm_create_ima_signature(const char *filename, EVP_PKEY *pkey, const char *algo,
+                                unsigned char *sig, size_t siglen, char **error);
 
 #endif
src/libimaevm.c +139 -4

183 lines not on this page · raw message

Signature · 1 line
2.30.2
#2 lore raw

PATCH v2 2/3 tests: Add program to create IMA signature with new API call

In reply to Stefan Berger

Since the new API call is not used by evmctl, implement a test program 'create_ima_signature' to use it. Extend _evmctl_sign to also created IMA v2 signatures with RSA keys using this test program and compare the results.

Evmctl's signature creation path is unmodified at this point, so the tests ensure that the existing sign_hash_v2 and the new sign_hash_v2_pkey create identical (RSA) signatures.

---
 tests/Makefile.am            |   6 ++
 tests/create_ima_signature.c | 111 +++++++++++++++++++++++++++++++++++
 tests/sign_verify.test       |  21 ++++++-
 3 files changed, 136 insertions(+), 2 deletions(-)
 create mode 100644 tests/create_ima_signature.c
tests/Makefile.am +6 -0
index ff928e1..5e255b1 100644
@@ -3,6 +3,12 @@ TESTS = $(check_SCRIPTS)
 
 check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test
 
+check_PROGRAMS = \
+	create_ima_signature
+
+create_ima_signature_CFLAGS = -I$(top_srcdir)/src
+create_ima_signature_LDFLAGS = -lcrypto -L$(top_builddir)/src/.libs -limaevm
+
 clean-local:
 	-rm -f *.txt *.out *.sig *.sig2
 
tests/create_ima_signature.c new file +111 -0

111 lines not on this page · raw message

tests/sign_verify.test +19 -2
index 288e133..14964ec 100755
@@ -16,10 +16,10 @@
 # GNU General Public License for more details.
 
 cd "$(dirname "$0")" || exit 1
-PATH=../src:$PATH
+PATH=../src:.:$PATH
 source ./functions.sh
 
-_require cmp evmctl getfattr openssl xxd
+_require cmp evmctl getfattr openssl xxd create_ima_signature
 
 if cmp -b 2>&1 | grep -q "invalid option"; then
 	echo "cmp does not support -b (cmp from busybox?) Use cmp from diffutils"
@@ -118,7 +118,24 @@ _evmctl_sign() {
 
   if [ "$type" = ima ]; then
     _test_sigfile "$file" "$(_xattr "$type")" "$file.sig" "$file.sig2"
+    if [ $? -ne $OK ]; then
+        return "$FAIL"
+    fi
+  fi
+  # Compare evmctl IMA v2 signatures with RSA keys versus those from create_ima_signature
+  if [ "$type" = ima ] && [[ $key =~ rsa ]] && ! [[ $opts =~ --rsa ]] ; then
+    create_ima_signature --key "${key}" --hashalgo "${alg}" "${file}" |
+      sed -n 's/^03.*/\0/p' |
+      xxd -r -p > "$file.sig2"
+
+    if ! cmp -bl "$file.sig" "$file.sig2"; then
+      color_red
+      echo "evmctl vs. create_ima_signature: signatures on $file differ"
+      color_restore
+      return "$FAIL"
+    fi
   fi
+  return $OK
 }
 
 # Run and test {ima_,}sign operation
Signature · 1 line
2.30.2
#3 lore raw

Re: [PATCH v2 2/3] tests: Add program to create IMA signature with new API call

Stefan,

Quoted 5 lines · Stefan Berger

Since the new API call is not used by evmctl, implement a test program 'create_ima_signature' to use it. Extend _evmctl_sign to also created IMA v2 signatures with RSA keys using this test program and compare the results.

Quoted 8 lines

Evmctl's signature creation path is unmodified at this point, so the tests ensure that the existing sign_hash_v2 and the new sign_hash_v2_pkey create identical (RSA) signatures.

---
 tests/Makefile.am            |   6 ++
 tests/create_ima_signature.c | 111 +++++++++++++++++++++++++++++++++++

Why not add this to evmctl?

Thanks,

Quoted 3 lines
 tests/sign_verify.test       |  21 ++++++-
 3 files changed, 136 insertions(+), 2 deletions(-)
 create mode 100644 tests/create_ima_signature.c
#4 lore raw

Re: [PATCH v2 2/3] tests: Add program to create IMA signature with new API call

Quoted 12 lines · Vitaly Chikunov

Stefan,

Quoted 9 lines · Stefan Berger

Since the new API call is not used by evmctl, implement a test program 'create_ima_signature' to use it. Extend _evmctl_sign to also created IMA v2 signatures with RSA keys using this test program and compare the results.

Evmctl's signature creation path is unmodified at this point, so the tests ensure that the existing sign_hash_v2 and the new sign_hash_v2_pkey create identical (RSA) signatures.

Quoted 5 lines
Quoted 4 lines
---
  tests/Makefile.am            |   6 ++
  tests/create_ima_signature.c | 111 +++++++++++++++++++++++++++++++++++

Why not add this to evmctl?

evmctl has 3 callers to the 'competing' function sign_hash(), all of which of course let the code path under sign_hash() read the key from the file (and access the keypassword via a global variable :(( ). I didn't want to pull up the reading of the key into evmctl, which we would have to do in case we wanted to call this new function. Also then the question is which callers to replace? All of them, or which ones?

#5 lore raw

PATCH v2 3/3 libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key file

In reply to Stefan Berger

Have sign_hash_v2 call sign_hash_v2_pkey after reading the key file.

---
 src/libimaevm.c | 65 ++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)
src/libimaevm.c +10 -55
index c4841c5..bda28a1 100644
@@ -994,15 +994,9 @@ err:
 static 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;
+	char *error = NULL;
 	EVP_PKEY *pkey;
-	char name[20];
-	EVP_PKEY_CTX *ctx = NULL;
-	const EVP_MD *md;
-	size_t sigsize;
-	const char *st;
-	uint32_t keyid;
+	int len = -1;
 
 	if (!hash) {
 		log_err("sign_hash_v2: hash is null\n");
@@ -1014,16 +1008,6 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
 		return -1;
 	}
 
-	if (!sig) {
-		log_err("sign_hash_v2: sig is null\n");
-		return -1;
-	}
-
-	if (!algo) {
-		log_err("sign_hash_v2: algo is null\n");
-		return -1;
-	}
-
 	log_info("hash(%s): ", imaevm_params.hash_algo);
 	log_dump(hash, size);
 
@@ -1031,49 +1015,20 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
 	if (!pkey)
 		return -1;
 
-	hdr = (struct signature_v2_hdr *)sig;
-	hdr->version = (uint8_t) DIGSIG_VERSION_2;
-
-	hdr->hash_algo = imaevm_get_hash_algo(algo);
-	if (hdr->hash_algo == (uint8_t)-1) {
-		log_err("sign_hash_v2: hash algo is unknown: %s\n", algo);
-		return -1;
-	}
-
-	calc_keyid_v2(&keyid, name, pkey);
-	hdr->keyid = keyid;
-
-	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(imaevm_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))
+	len = sign_hash_v2_pkey(algo, hash, size, pkey, sig, MAX_SIGNATURE_SIZE - 1,
+	                        &error);
+	if (len < 0) {
+		log_err("%s\n", error);
+		output_openssl_errors();
 		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);
 
 err:
-	if (len == -1) {
-		log_err("sign_hash_v2: signing failed: (%s) in %s\n",
-			ERR_reason_error_string(ERR_peek_error()), st);
-		output_openssl_errors();
-	}
-	EVP_PKEY_CTX_free(ctx);
+	free(error);
 	EVP_PKEY_free(pkey);
+
 	return len;
 }
 
Signature · 1 line
2.30.2
#6 lore raw
In reply to Stefan Berger

I will rebase this series of patches on Vitaly's keyid-related changes, which are good to have 'underneath'.

  Stefan

Quoted 27 lines · Stefan Berger

This series of patches implements imaevm_create_ima_signature, a function that only creates an IMA v2 signature but does not write it into an xattr. This API fullfills a requirement of rpm-ostree where a tool is being built to sign all files from something similar to a tar byte stream, though where files are written to temporary files for signing purpose but the signature itself should not be written into the filesystem's xattr but simply returned via the API.

Stefan

v2:
- use imaevm_calc_hash to also cover hash calculations of links, directories etc.

Stefan Berger (3):
libimaevm: Implement imaevm_create_ima_signature
tests: Add program to create IMA signature with new API call
libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key
file

  src/imaevm.h                 |   2 +
  src/libimaevm.c              | 148 ++++++++++++++++++++++++++++-------
  tests/Makefile.am            |   6 ++
  tests/create_ima_signature.c | 111 ++++++++++++++++++++++++++
  tests/sign_verify.test       |  21 ++++-
  5 files changed, 257 insertions(+), 31 deletions(-)

create mode 100644 tests/create_ima_signature.c

maintainer
#7 lore raw

Hi Stefan, Vitaly,

Quoted 2 lines · Stefan Berger

I will rebase this series of patches on Vitaly's keyid-related changes, which are good to have 'underneath'.

Sorry for the delay in reviewing/commenting on your patch sets. Hopefully I'll be circling back around to ima-evm-utils later this week, after staging Roberto's EVM kernel patch set. Vitaly, I noticed
that your patch set doesn't apply cleanly to next-testing. I haven't
taken the time to figure out why yet.

thanks,

Mimi