linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ima-evm-utils: Implement function to only create IMA signature
@ 2021-04-20 18:30 Stefan Berger
  2021-04-20 18:30 ` [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature Stefan Berger
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Stefan Berger @ 2021-04-20 18:30 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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

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              | 191 ++++++++++++++++++++++++++++++-----
 tests/Makefile.am            |   6 ++
 tests/create_ima_signature.c | 111 ++++++++++++++++++++
 tests/sign_verify.test       |  21 +++-
 5 files changed, 304 insertions(+), 27 deletions(-)
 create mode 100644 tests/create_ima_signature.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature
  2021-04-20 18:30 [PATCH 0/3] ima-evm-utils: Implement function to only create IMA signature Stefan Berger
@ 2021-04-20 18:30 ` Stefan Berger
  2021-04-27  0:02   ` Colin Walters
  2021-04-20 18:30 ` [PATCH 2/3] tests: Add program to create IMA signature with new API call Stefan Berger
  2021-04-20 18:30 ` [PATCH 3/3] libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key file Stefan Berger
  2 siblings, 1 reply; 7+ messages in thread
From: Stefan Berger @ 2021-04-20 18:30 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, Stefan Berger

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.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/imaevm.h    |   2 +
 src/libimaevm.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 188 insertions(+)

diff --git a/src/imaevm.h b/src/imaevm.h
index 4503919..fc4536b 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -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
diff --git a/src/libimaevm.c b/src/libimaevm.c
index fa6c278..bb425af 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -44,9 +44,11 @@
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 #include <asm/byteorder.h>
 #include <unistd.h>
 #include <dirent.h>
+#include <fcntl.h>
 #include <string.h>
 #include <stdio.h>
 #include <assert.h>
@@ -879,6 +881,109 @@ out:
 	return len;
 }
 
+/* Sign a hash with the given private key.
+ *
+ * @algo: Name of the algorithm that was used to compute the hash, e.g. "sha256"
+ * @hash: The hash
+ * @size: Size of the hash
+ * @pkey: Private key to use for signing
+ * @sig: Buffer for the signature; it is assumed to be of (MAX_SIGNATURE_SIZE - 1) size
+ * @siglen: Length of the signature buffer; it must be at least (MAX_SIGNATURE_SIZE - 1)
+ *          to be accepted
+ * @error: Pointer where a buffer with the error to report can be returned
+ *
+ * Returns -1 on error, the length of the signature, including header, otherwise
+ */
+static int sign_hash_v2_pkey(const char *algo, const unsigned char *hash,
+			     int size, EVP_PKEY *pkey, unsigned char *sig, size_t siglen,
+			     char **error)
+{
+	struct signature_v2_hdr *hdr;
+	EVP_PKEY_CTX *ctx = NULL;
+	const char *st = NULL;
+	const EVP_MD *md;
+	size_t sigsize;
+	uint32_t keyid;
+	char name[20];
+	int len = -1;
+
+	if (!hash) {
+		asprintf(error, "sign_hash_v2_pkey: hash is null");
+		return -1;
+	}
+
+	if (size < 0) {
+		asprintf(error, "sign_hash_v2_pkey: size is negative: %d", size);
+		return -1;
+	}
+
+	if (!pkey) {
+		asprintf(error, "sign_hash_v2_pkey: pkey is null");
+		return -1;
+	}
+
+	if (!sig) {
+		asprintf(error, "sign_hash_v2_pkey: sig is null");
+		return -1;
+	}
+
+	if (siglen < MAX_SIGNATURE_SIZE - 1) {
+		asprintf(error, "sign_hash_v2_pkey: siglen must be at least %d bytes\n",
+		         MAX_SIGNATURE_SIZE - 1);
+		return -1;
+	}
+
+	if (!algo) {
+		asprintf(error, "sign_hash_v2_pkey: algo is null");
+		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) {
+		asprintf(error, "sign_hash_v2_pkey: hash algo is unknown: %s", 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(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;
+	st = NULL;
+
+	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);
+
+err:
+	if (len == -1 && st != NULL) {
+		asprintf(error, "sign_hash_v2_pkey: signing failed: (%s) in %s",
+			ERR_reason_error_string(ERR_peek_error()), st);
+	}
+	EVP_PKEY_CTX_free(ctx);
+
+	return len;
+}
+
 /*
  * @sig is assumed to be of (MAX_SIGNATURE_SIZE - 1) size
  * Return: -1 signing error, >0 length of signature
@@ -980,6 +1085,87 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c
 		sign_hash_v1(hashalgo, hash, size, keyfile, sig);
 }
 
+/*
+ * Create an IMA signature for a given file using a given private key for signing
+ *
+ * @filename: Name of the file to sign
+ * @pkey: Private key to use for signing
+ * @algo: Name of the algorithm to use to compute the hash, e.g. "sha256"
+ * @sig: Buffer for the signature; it is assumed to be of (MAX_SIGNATURE_SIZE - 1) size
+ * @siglen: Length of the signature buffer; it must be at least (MAX_SIGNATURE_SIZE - 1)
+ *          to be accepted
+ * @error: Pointer where a buffer with the error to report can be returned
+ *
+ * Returns -1 on error, the length of the signature, including header, otherwise
+ */
+int imaevm_create_ima_signature(const char *filename, EVP_PKEY *pkey, const char *algo,
+                                unsigned char *sig, size_t siglen, char **error)
+{
+	unsigned char hashbuf[EVP_MAX_MD_SIZE];
+	unsigned int hashbuf_size = sizeof(hashbuf);
+	struct stat statbuf;
+	EVP_MD_CTX *md_ctx;
+	const EVP_MD *md;
+	const char *st = NULL;
+	int len = -1;
+	void *addr = NULL;
+	int fd;
+	int n;
+
+	fd = open(filename, O_RDONLY);
+	if (!fd) {
+		asprintf(error, "Failed to open %s: %s", filename, strerror(errno));
+		return -1;
+	}
+
+	n = fstat(fd, &statbuf);
+	if (n != 0) {
+		asprintf(error, "Failed to stat %s: %s", filename, strerror(errno));
+		goto err_close;
+	}
+
+	if (statbuf.st_size > 0) {
+		addr = mmap(NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0);
+		if (addr == MAP_FAILED) {
+			asprintf(error, "Failed to mmap file: %s", strerror(errno));
+			goto err_close;
+		}
+	}
+
+	st = "EVP_MD_CTX_new";
+	if (!(md_ctx = EVP_MD_CTX_new()))
+		goto err;
+	st = "EVP_get_digestbyname";
+	if (!(md = EVP_get_digestbyname(algo)))
+		goto err;
+	st = "EVP_DigestInit_ex";
+	if (EVP_DigestInit_ex(md_ctx, md, NULL) != 1)
+		goto err;
+	st = "EVP_DigestUpdate";
+	if (statbuf.st_size > 0 && EVP_DigestUpdate(md_ctx, addr, statbuf.st_size) != 1)
+		goto err;
+	st = "EVP_DigestFinal_ex";
+	if (EVP_DigestFinal_ex(md_ctx, hashbuf, &hashbuf_size) != 1)
+		goto err;
+	st = NULL;
+
+	len = sign_hash_v2_pkey(algo, hashbuf, hashbuf_size, pkey, sig, siglen, error);
+
+err:
+	if (len < 0 && st != NULL) {
+		asprintf(error,
+			 "%s: signing failed: (%s) in %s\n",
+			 __func__, ERR_reason_error_string(ERR_peek_error()), st);
+	}
+
+	EVP_MD_CTX_free(md_ctx);
+	munmap(addr, statbuf.st_size);
+err_close:
+	close(fd);
+
+	return len;
+}
+
 static void libinit()
 {
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] tests: Add program to create IMA signature with new API call
  2021-04-20 18:30 [PATCH 0/3] ima-evm-utils: Implement function to only create IMA signature Stefan Berger
  2021-04-20 18:30 ` [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature Stefan Berger
@ 2021-04-20 18:30 ` Stefan Berger
  2021-04-20 18:30 ` [PATCH 3/3] libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key file Stefan Berger
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Berger @ 2021-04-20 18:30 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, 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.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 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

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ff928e1..5e255b1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -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
 
diff --git a/tests/create_ima_signature.c b/tests/create_ima_signature.c
new file mode 100644
index 0000000..649efcf
--- /dev/null
+++ b/tests/create_ima_signature.c
@@ -0,0 +1,111 @@
+/*
+ * create_ima_signature - Test program for imaevm_create_ima_signature
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * As a special exception, the copyright holders give permission to link the
+ * code of portions of this program with the OpenSSL library under certain
+ * conditions as described in each individual source file and distribute
+ * linked combinations including the program with the OpenSSL library. You
+ * must comply with the GNU General Public License in all respects
+ * for all of the code used other than as permitted herein. If you modify
+ * file(s) with this exception, you may extend this exception to your
+ * version of the file(s), but you are not obligated to do so. If you do not
+ * wish to do so, delete this exception statement from your version. If you
+ * delete this exception statement from all source files in the program,
+ * then also delete it in the license file.
+ *
+ */
+
+#include <getopt.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <openssl/pem.h>
+
+#include "imaevm.h"
+
+int main(int argc, char *argv[]) {
+	unsigned char ima_signature[MAX_SIGNATURE_SIZE];
+	static struct option long_options[] = {
+		{"key", required_argument, NULL, 'k'},
+		{"hashalgo", required_argument, NULL, 'a'},
+		{NULL, 0, NULL, 0}
+	};
+	const char *hash_algo = "sha1";
+	const char *keyfile = NULL;
+	const char *file_to_sign;
+	EVP_PKEY *pkey = NULL;
+	char *error = NULL;
+	int option_index;
+	int siglen;
+	size_t i;
+	FILE *fp;
+	int opt;
+
+	while ((opt = getopt_long_only(argc, argv, "", long_options, &option_index)) != -1) {
+		switch (opt) {
+		case 'k':
+			keyfile = optarg;
+			break;
+		case 'a':
+			hash_algo = optarg;
+			break;
+		default:
+			fprintf(stderr, "Unhandled option %d.\n", opt);
+			return 1;
+		}
+	}
+	if (keyfile == NULL) {
+		fprintf(stderr, "Missing --key option.\n");
+		return 1;
+	}
+
+	if (optind == argc) {
+		fprintf(stderr, "Missing filename for file to sign.");
+	}
+
+	file_to_sign = argv[optind];
+
+	fp = fopen(keyfile, "r");
+	if (fp == NULL) {
+		fprintf(stderr, "Could not open private key file: %s\n", strerror(errno));
+		return 1;
+	}
+
+	pkey = PEM_read_PrivateKey(fp, NULL, NULL, NULL);
+	fclose(fp);
+	if (pkey == NULL) {
+		fprintf(stderr, "Could not read private key!\n");
+		return 1;
+	}
+
+	/* the library doesn't prepend this! */
+	ima_signature[0] = EVM_IMA_XATTR_DIGSIG;
+	siglen = imaevm_create_ima_signature(file_to_sign, pkey, hash_algo, &ima_signature[1],
+	                                     sizeof(ima_signature) - 1, &error);
+	if (siglen < 0) {
+		fprintf(stderr, "Failed to created IMA signature: %s\n", error);
+	} else {
+		fprintf(stdout, "Successfully created IMA signature!\n");
+		for (i = 0; i < siglen + 1; i++)
+			fprintf(stdout, "%02x", ima_signature[i]);
+		fprintf(stdout, "\n");
+	}
+
+	free(error);
+	EVP_PKEY_free(pkey);
+
+	return siglen < 0;
+}
diff --git a/tests/sign_verify.test b/tests/sign_verify.test
index 288e133..14964ec 100755
--- a/tests/sign_verify.test
+++ b/tests/sign_verify.test
@@ -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
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key file
  2021-04-20 18:30 [PATCH 0/3] ima-evm-utils: Implement function to only create IMA signature Stefan Berger
  2021-04-20 18:30 ` [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature Stefan Berger
  2021-04-20 18:30 ` [PATCH 2/3] tests: Add program to create IMA signature with new API call Stefan Berger
@ 2021-04-20 18:30 ` Stefan Berger
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Berger @ 2021-04-20 18:30 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, dmitry.kasatkin, Stefan Berger

Have sign_hash_v2 call sign_hash_v2_pkey after reading the key file.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/libimaevm.c | 65 ++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)

diff --git a/src/libimaevm.c b/src/libimaevm.c
index bb425af..f7f070c 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -991,15 +991,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");
@@ -1011,16 +1005,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);
 
@@ -1028,49 +1012,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;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature
  2021-04-20 18:30 ` [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature Stefan Berger
@ 2021-04-27  0:02   ` Colin Walters
  2021-04-27 17:47     ` Stefan Berger
  0 siblings, 1 reply; 7+ messages in thread
From: Colin Walters @ 2021-04-27  0:02 UTC (permalink / raw)
  To: Stefan Berger, linux-integrity; +Cc: zohar, Dmitry Kasatkin



On Tue, Apr 20, 2021, at 2:30 PM, Stefan Berger wrote:

> + fd = open(filename, O_RDONLY);

Missing O_CLOEXEC.

> +int imaevm_create_ima_signature(const char *filename, EVP_PKEY *pkey, 

It'd maximize flexibility for the caller to pass a file descriptor, and not a file name.

> +	if (statbuf.st_size > 0) {
> +		addr = mmap(NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0);
> +		if (addr == MAP_FAILED) {
> +			asprintf(error, "Failed to mmap file: %s", strerror(errno));
> +			goto err_close;
> +		}
> +	}

Tangentially related to this, I think we should consider doing the same optimization here:
https://github.com/ostreedev/ostree/blob/36693f064c63dad550ebcfed33bf9b95806ddef9/src/libotutil/ot-fs-utils.c#L171

Or alternatively, just have the caller provide a (mmap'd or copied-via-read()) buffer?

Though clearly the most flexible is a streaming API.  But eh, I am not really concerned about that level of performance.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature
  2021-04-27  0:02   ` Colin Walters
@ 2021-04-27 17:47     ` Stefan Berger
  2021-04-27 19:13       ` Stefan Berger
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Berger @ 2021-04-27 17:47 UTC (permalink / raw)
  To: Colin Walters, linux-integrity; +Cc: zohar, Dmitry Kasatkin


On 4/26/21 8:02 PM, Colin Walters wrote:
>
> On Tue, Apr 20, 2021, at 2:30 PM, Stefan Berger wrote:
>
>> + fd = open(filename, O_RDONLY);
> Missing O_CLOEXEC.


Will do.

>
>> +int imaevm_create_ima_signature(const char *filename, EVP_PKEY *pkey,
> It'd maximize flexibility for the caller to pass a file descriptor, and not a file name.


We could do this with a callback where the user implements the callback 
function and providers buffer, size of buffer, and eof indicator, and 
gets called for providing the data to hash. That would maybe be even 
more flexible..


>
>> +	if (statbuf.st_size > 0) {
>> +		addr = mmap(NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0);
>> +		if (addr == MAP_FAILED) {
>> +			asprintf(error, "Failed to mmap file: %s", strerror(errno));
>> +			goto err_close;
>> +		}
>> +	}
> Tangentially related to this, I think we should consider doing the same optimization here:
> https://github.com/ostreedev/ostree/blob/36693f064c63dad550ebcfed33bf9b95806ddef9/src/libotutil/ot-fs-utils.c#L171
>
> Or alternatively, just have the caller provide a (mmap'd or copied-via-read()) buffer?
>
> Though clearly the most flexible is a streaming API.  But eh, I am not really concerned about that level of performance.


I think using a callback would allow for the streaming as well...

    Stefan



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature
  2021-04-27 17:47     ` Stefan Berger
@ 2021-04-27 19:13       ` Stefan Berger
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Berger @ 2021-04-27 19:13 UTC (permalink / raw)
  To: Colin Walters, linux-integrity; +Cc: zohar, Dmitry Kasatkin


On 4/27/21 1:47 PM, Stefan Berger wrote:
>
> On 4/26/21 8:02 PM, Colin Walters wrote:
>>
>> On Tue, Apr 20, 2021, at 2:30 PM, Stefan Berger wrote:
>>
>>> + fd = open(filename, O_RDONLY);
>> Missing O_CLOEXEC.
>
>
> Will do.
>
>>
>>> +int imaevm_create_ima_signature(const char *filename, EVP_PKEY *pkey,
>> It'd maximize flexibility for the caller to pass a file descriptor, 
>> and not a file name.
>
>
> We could do this with a callback where the user implements the 
> callback function and providers buffer, size of buffer, and eof 
> indicator, and gets called for providing the data to hash. That would 
> maybe be even more flexible..


Actually, I cannot do this. For this function we have to call 
ima_calc_hash, which hashes, files, links, directories etc.


     Stefan



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-27 19:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 18:30 [PATCH 0/3] ima-evm-utils: Implement function to only create IMA signature Stefan Berger
2021-04-20 18:30 ` [PATCH 1/3] libimaevm: Implement imaevm_create_ima_signature Stefan Berger
2021-04-27  0:02   ` Colin Walters
2021-04-27 17:47     ` Stefan Berger
2021-04-27 19:13       ` Stefan Berger
2021-04-20 18:30 ` [PATCH 2/3] tests: Add program to create IMA signature with new API call Stefan Berger
2021-04-20 18:30 ` [PATCH 3/3] libimaevm: Have sign_hash_v2 call sign_hash_v2_pkey after reading key file Stefan Berger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).