[PATCH v3 0/9] ima-evm-utils: Add support for signing with pkcs11 URIs

[PATCH v3 0/9] ima-evm-utils: Add support for signing with pkcs11 URIs

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

This series of patches adds support for signing with pkcs11 URIs so that
pkcs11-enabled devices can also be used for file signing.

A test program is provided setting up SoftHSM for test cases to use. I added
two test cases for testing with SoftHSM.

  Stefan

v3:
  - Used commit messages Mimi suggested
  - 7/9: Split off imported script into own patch
  - 8/9: Added missing EVMCTL_ENGINE variable that allowed to enable Debian
         and Ubuntu testing in 9/9; improvements on setup and teardown
         functions
  - 9/9: Installation of required packages on Debian and Ubuntu

Stefan Berger (9):
  evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable
  evmctl: Handle failure to initialize the openssl engine
  evmctl: Implement function for setting up an OpenSSL engine
  evmctl: Define and use an ENGINE field in libimaevm_params
  evmctl: use the pkcs11 engine for pkcs11: prefixed URIs
  libimaevm: Add support for pkcs11 private keys for signing a v2 hash
  tests: Import softhsm_setup script to enable pkcs11 test case
  tests: Extend sign_verify test with pkcs11-specific test
  tests: Get the packages for pkcs11 testing on the CI/CD system

 README                 |   5 +
 ci/alt.sh              |   3 +
 ci/debian.sh           |   3 +-
 ci/fedora.sh           |   8 ++
 ci/tumbleweed.sh       |   3 +
 src/evmctl.c           |  54 +++++---
 src/imaevm.h           |   2 +
 src/libimaevm.c        |  47 +++++--
 tests/functions.sh     |  45 +++++++
 tests/sign_verify.test |  52 ++++++--
 tests/softhsm_setup    | 297 +++++++++++++++++++++++++++++++++++++++++
 11 files changed, 478 insertions(+), 41 deletions(-)
 create mode 100755 tests/softhsm_setup

-- 
2.31.1

[PATCH v3 1/9] evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

If the user did not use the --pass option to provide a key password,
get the key password from the EVMCTL_KEY_PASSWORD environment variable.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 README       | 4 ++++
 src/evmctl.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/README b/README
index 87cd3b5..1cc027f 100644
--- a/README
+++ b/README
@@ -70,6 +70,10 @@ OPTIONS
   -v                 increase verbosity level
   -h, --help         display this help and exit
 
+Environment variables:
+
+EVMCTL_KEY_PASSWORD  : Private key password to use; do not use --pass option
+
 
 INTRODUCTION
 ------------
diff --git a/src/evmctl.c b/src/evmctl.c
index a8065bb..58f8e66 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2530,6 +2530,9 @@ static void usage(void)
 		"      --ignore-violations ignore ToMToU measurement violations\n"
 		"  -v                 increase verbosity level\n"
 		"  -h, --help         display this help and exit\n"
+		"\n"
+		"Environment variables:\n\n"
+		"EVMCTL_KEY_PASSWORD  : Private key password to use; do not use --pass option\n"
 		"\n");
 }
 
@@ -2813,6 +2816,9 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (!imaevm_params.keypass)
+		imaevm_params.keypass = getenv("EVMCTL_KEY_PASSWORD");
+
 	if (argv[optind] == NULL)
 		usage();
 	else
-- 
2.31.1

[PATCH v3 2/9] evmctl: Handle failure to initialize the openssl engine

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Handle failure to initialize the openssl engine. For example,

$ ./src/evmctl --engine foo
engine foo isn't available
140322992015168:error:25066067:DSO support routines:dlfcn_load:could not load the shared library:crypto/dso/dso_dlfcn.c:118:filename(/usr/lib64/engines-1.1/foo.so): /usr/lib64/engines-1.1/foo.so: cannot open shared object file: No such file or directory
140322992015168:error:25070067:DSO support routines:DSO_load:could not load the shared library:crypto/dso/dso_lib.c:162:
140322992015168:error:260B6084:engine routines:dynamic_load:dso not found:crypto/engine/eng_dyn.c:414:
140322992015168:error:2606A074:engine routines:ENGINE_by_id:no such engine:crypto/engine/eng_list.c:334:id=foo
Segmentation fault (core dumped)

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/evmctl.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 58f8e66..ed0ece3 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2765,7 +2765,10 @@ int main(int argc, char *argv[])
 				ENGINE_free(eng);
 				eng = NULL;
 			}
-			ENGINE_set_default(eng, ENGINE_METHOD_ALL);
+			if (eng)
+				ENGINE_set_default(eng, ENGINE_METHOD_ALL);
+			else
+				goto error;
 			break;
 		case 140: /* --xattr-user */
 			xattr_ima = "user.ima";
@@ -2839,6 +2842,7 @@ int main(int argc, char *argv[])
 			err = 125;
 	}
 
+error:
 	if (eng) {
 		ENGINE_finish(eng);
 		ENGINE_free(eng);
-- 
2.31.1

[PATCH v3 3/9] evmctl: Implement function for setting up an OpenSSL engine

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Implement setup_engine for setting up an OpenSSL engine.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/evmctl.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index ed0ece3..4b6f3fb 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2631,6 +2631,23 @@ static char *get_password(void)
 	return pwd;
 }
 
+static ENGINE *setup_engine(const char *engine_id)
+{
+	ENGINE *eng = ENGINE_by_id(engine_id);
+	if (!eng) {
+		log_err("engine %s isn't available\n", optarg);
+		ERR_print_errors_fp(stderr);
+	} else if (!ENGINE_init(eng)) {
+		log_err("engine %s init failed\n", optarg);
+		ERR_print_errors_fp(stderr);
+		ENGINE_free(eng);
+		eng = NULL;
+	}
+	if (eng)
+		ENGINE_set_default(eng, ENGINE_METHOD_ALL);
+	return eng;
+}
+
 int main(int argc, char *argv[])
 {
 	int err = 0, c, lind;
@@ -2755,19 +2772,8 @@ int main(int argc, char *argv[])
 			verify_list_sig = 1;
 			break;
 		case 139: /* --engine e */
-			eng = ENGINE_by_id(optarg);
-			if (!eng) {
-				log_err("engine %s isn't available\n", optarg);
-				ERR_print_errors_fp(stderr);
-			} else if (!ENGINE_init(eng)) {
-				log_err("engine %s init failed\n", optarg);
-				ERR_print_errors_fp(stderr);
-				ENGINE_free(eng);
-				eng = NULL;
-			}
-			if (eng)
-				ENGINE_set_default(eng, ENGINE_METHOD_ALL);
-			else
+			eng = setup_engine(optarg);
+			if (!eng)
 				goto error;
 			break;
 		case 140: /* --xattr-user */
-- 
2.31.1

[PATCH v3 4/9] evmctl: Define and use an ENGINE field in libimaevm_params

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Extend the global libimaevm_params structure with an ENGINE field 'eng'
and use it in place of the local ENGINE variable in main().

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/evmctl.c | 11 +++++------
 src/imaevm.h |  2 ++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 4b6f3fb..625a511 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2651,7 +2651,6 @@ static ENGINE *setup_engine(const char *engine_id)
 int main(int argc, char *argv[])
 {
 	int err = 0, c, lind;
-	ENGINE *eng = NULL;
 	unsigned long keyid;
 	char *eptr;
 
@@ -2772,8 +2771,8 @@ int main(int argc, char *argv[])
 			verify_list_sig = 1;
 			break;
 		case 139: /* --engine e */
-			eng = setup_engine(optarg);
-			if (!eng)
+			imaevm_params.eng = setup_engine(optarg);
+			if (!imaevm_params.eng)
 				goto error;
 			break;
 		case 140: /* --xattr-user */
@@ -2849,9 +2848,9 @@ int main(int argc, char *argv[])
 	}
 
 error:
-	if (eng) {
-		ENGINE_finish(eng);
-		ENGINE_free(eng);
+	if (imaevm_params.eng) {
+		ENGINE_finish(imaevm_params.eng);
+		ENGINE_free(imaevm_params.eng);
 #if OPENSSL_API_COMPAT < 0x10100000L
 		ENGINE_cleanup();
 #endif
diff --git a/src/imaevm.h b/src/imaevm.h
index 491f136..8792aa2 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -48,6 +48,7 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <openssl/rsa.h>
+#include <openssl/engine.h>
 
 #ifdef USE_FPRINTF
 #define do_log(level, fmt, args...)	\
@@ -197,6 +198,7 @@ struct libimaevm_params {
 	const char *keyfile;
 	const char *keypass;
 	uint32_t keyid;		/* keyid overriding value, unless 0. (Host order.) */
+	ENGINE *eng;
 };
 
 struct RSA_ASN1_template {
-- 
2.31.1

[PATCH v3 5/9] evmctl: use the pkcs11 engine for pkcs11: prefixed URIs

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

If the key has the pkcs11: URI prefix then setup the pkcs11 engine
if the user hasn't chosen a specific engine already.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/evmctl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/evmctl.c b/src/evmctl.c
index 625a511..5178643 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2827,6 +2827,14 @@ int main(int argc, char *argv[])
 	if (!imaevm_params.keypass)
 		imaevm_params.keypass = getenv("EVMCTL_KEY_PASSWORD");
 
+	if (imaevm_params.keyfile != NULL &&
+	    imaevm_params.eng == NULL &&
+	    !strncmp(imaevm_params.keyfile, "pkcs11:", 7)) {
+		imaevm_params.eng = setup_engine("pkcs11");
+		if (!imaevm_params.eng)
+			goto error;
+	}
+
 	if (argv[optind] == NULL)
 		usage();
 	else
-- 
2.31.1

[PATCH v3 6/9] libimaevm: Add support for pkcs11 private keys for signing a v2 hash

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Add support for pkcs11 private keys for signing a v2 hash.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 README          |  1 +
 src/evmctl.c    |  1 +
 src/libimaevm.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 1cc027f..2bb363c 100644
--- a/README
+++ b/README
@@ -48,6 +48,7 @@ OPTIONS
       --xattr-user   store xattrs in user namespace (for testing purposes)
       --rsa          use RSA key type and signing scheme v1
   -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)
+                     or a pkcs11 URI
       --keyid n      overwrite signature keyid with a 32-bit value in hex (for signing)
       --keyid-from-cert file
                      read keyid value from SKID of a x509 cert file
diff --git a/src/evmctl.c b/src/evmctl.c
index 5178643..0a54ac3 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2503,6 +2503,7 @@ static void usage(void)
 		"      --xattr-user   store xattrs in user namespace (for testing purposes)\n"
 		"      --rsa          use RSA key type and signing scheme v1\n"
 		"  -k, --key          path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
+		"                     or a pkcs11 URI\n"
 		"      --keyid n      overwrite signature keyid with a 32-bit value in hex (for signing)\n"
 		"      --keyid-from-cert file\n"
 		"                     read keyid value from SKID of a x509 cert file\n"
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 8e96157..6855184 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -60,6 +60,7 @@
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 #include <openssl/err.h>
+#include <openssl/engine.h>
 
 #include "imaevm.h"
 #include "hash_info.h"
@@ -804,20 +805,44 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
 	FILE *fp;
 	EVP_PKEY *pkey;
 
-	fp = fopen(keyfile, "r");
-	if (!fp) {
-		log_err("Failed to open keyfile: %s\n", keyfile);
-		return NULL;
-	}
-	pkey = PEM_read_PrivateKey(fp, NULL, NULL, (void *)keypass);
-	if (!pkey) {
-		log_err("Failed to PEM_read_PrivateKey key file: %s\n",
-			keyfile);
-		output_openssl_errors();
+	if (!strncmp(keyfile, "pkcs11:", 7)) {
+		if (!imaevm_params.keyid) {
+			log_err("When using a pkcs11 URI you must provide the keyid with an option\n");
+			return NULL;
+		}
+
+		if (keypass) {
+			if (!ENGINE_ctrl_cmd_string(imaevm_params.eng, "PIN", keypass, 0)) {
+				log_err("Failed to set the PIN for the private key\n");
+				goto err_engine;
+			}
+		}
+		pkey = ENGINE_load_private_key(imaevm_params.eng, keyfile, NULL, NULL);
+		if (!pkey) {
+			log_err("Failed to load private key %s\n", keyfile);
+			goto err_engine;
+		}
+	} else {
+		fp = fopen(keyfile, "r");
+		if (!fp) {
+			log_err("Failed to open keyfile: %s\n", keyfile);
+			return NULL;
+		}
+		pkey = PEM_read_PrivateKey(fp, NULL, NULL, (void *)keypass);
+		if (!pkey) {
+			log_err("Failed to PEM_read_PrivateKey key file: %s\n",
+				keyfile);
+			output_openssl_errors();
+		}
+
+		fclose(fp);
 	}
 
-	fclose(fp);
 	return pkey;
+
+err_engine:
+	output_openssl_errors();
+	return NULL;
 }
 
 static RSA *read_priv_key(const char *keyfile, const char *keypass)
-- 
2.31.1

[PATCH v3 7/9] tests: Import softhsm_setup script to enable pkcs11 test case

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Import softhsm_setup script from my swtpm project and contribute
it to this project under dual license BSD 3-clause and GLP 2.0.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/softhsm_setup | 297 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 297 insertions(+)
 create mode 100755 tests/softhsm_setup

diff --git a/tests/softhsm_setup b/tests/softhsm_setup
new file mode 100755
index 0000000..fe69a6e
--- /dev/null
+++ b/tests/softhsm_setup
@@ -0,0 +1,297 @@
+#!/usr/bin/env bash
+
+# This program originates from 'swtpm' project (https://github.com/stefanberger/swtpm/)
+# and is provided to ima-evm-utils under a dual license:
+# - BSD 3-clause
+# - GPL-2.0
+
+# This script may not work with softhsm2 2.0.0 but with >= 2.2.0
+
+if [ -z "$(type -P p11tool)" ]; then
+	echo "Need p11tool from gnutls"
+	exit 77
+fi
+
+if [ -z "$(type -P softhsm2-util)" ]; then
+	echo "Need softhsm2-util from softhsm2 package"
+	exit 77
+fi
+
+MAJOR=$(softhsm2-util -v | cut -d '.' -f1)
+MINOR=$(softhsm2-util -v | cut -d '.' -f2)
+if [ ${MAJOR} -lt 2 ] || [ ${MAJOR} -eq 2 -a ${MINOR} -lt 2 ]; then
+	echo "Need softhsm v2.2.0 or later"
+	exit 77
+fi
+
+NAME=swtpm-test
+PIN=${PIN:-1234}
+SO_PIN=${SO_PIN:-1234}
+SOFTHSM_SETUP_CONFIGDIR=${SOFTHSM_SETUP_CONFIGDIR:-~/.config/softhsm2}
+export SOFTHSM2_CONF=${SOFTHSM_SETUP_CONFIGDIR}/softhsm2.conf
+
+UNAME_S="$(uname -s)"
+
+case "${UNAME_S}" in
+Darwin)
+	msg=$(sudo -v -n)
+	if [ $? -ne 0 ]; then
+		echo "Need password-less sudo rights on OS X to change /etc/gnutls/pkcs11.conf"
+		exit 1
+	fi
+	;;
+esac
+
+teardown_softhsm() {
+	local configdir=${SOFTHSM_SETUP_CONFIGDIR}
+	local configfile=${SOFTHSM2_CONF}
+	local bakconfigfile=${configfile}.bak
+	local tokendir=${configdir}/tokens
+
+	softhsm2-util --token "${NAME}" --delete-token &>/dev/null
+
+	case "${UNAME_S}" in
+	Darwin*)
+		if [ -f /etc/gnutls/pkcs11.conf.bak ]; then
+			sudo rm -f /etc/gnutls/pkcs11.conf
+			sudo mv /etc/gnutls/pkcs11.conf.bak \
+			   /etc/gnutls/pkcs11.conf &>/dev/null
+		fi
+		;;
+	esac
+
+	if [ -f "$bakconfigfile" ]; then
+		mv "$bakconfigfile" "$configfile"
+	else
+		rm -f "$configfile"
+	fi
+	if [ -d "$tokendir" ]; then
+		rm -rf "${tokendir}"
+	fi
+	return 0
+}
+
+setup_softhsm() {
+	local msg tokenuri keyuri
+	local configdir=${SOFTHSM_SETUP_CONFIGDIR}
+	local configfile=${SOFTHSM2_CONF}
+	local bakconfigfile=${configfile}.bak
+	local tokendir=${configdir}/tokens
+	local rc
+
+	case "${UNAME_S}" in
+	Darwin*)
+		if [ -f /etc/gnutls/pkcs11.conf.bak ]; then
+			echo "/etc/gnutls/pkcs11.conf.bak already exists; need to 'teardown' first"
+			return 1
+		fi
+		sudo mv /etc/gnutls/pkcs11.conf \
+			/etc/gnutls/pkcs11.conf.bak &>/dev/null
+		if [ $(id -u) -eq 0 ]; then
+			SONAME="$(sudo -u nobody brew ls --verbose softhsm | \
+				  grep -E "\.so$")"
+		else
+			SONAME="$(brew ls --verbose softhsm | \
+				  grep -E "\.so$")"
+		fi
+		sudo mkdir -p /etc/gnutls &>/dev/null
+		sudo bash -c "echo "load=${SONAME}" > /etc/gnutls/pkcs11.conf"
+		;;
+	esac
+
+	if ! [ -d $configdir ]; then
+		mkdir -p $configdir
+	fi
+	mkdir -p ${tokendir}
+
+	if [ -f $configfile ]; then
+		mv "$configfile" "$bakconfigfile"
+	fi
+
+	if ! [ -f $configfile ]; then
+		cat <<_EOF_ > $configfile
+directories.tokendir = ${tokendir}
+objectstore.backend = file
+log.level = DEBUG
+slots.removable = false
+_EOF_
+	fi
+
+	msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}" | tail -n1)
+	if [ $? -ne 0 ]; then
+		echo "Could not list existing tokens"
+		echo "$msg"
+	fi
+	tokenuri=$(echo "$msg" | sed -n 's/.*URL: \([[:print:]*]\)/\1/p')
+
+	if [ -z "$tokenuri" ]; then
+		msg=$(softhsm2-util \
+			--init-token --pin ${PIN} --so-pin ${SO_PIN} \
+			--free --label ${NAME} 2>&1)
+		if [ $? -ne 0 ]; then
+			echo "Could not initialize token"
+			echo "$msg"
+			return 2
+		fi
+
+		slot=$(echo "$msg" | \
+		       sed -n 's/.* reassigned to slot \([0-9]*\)$/\1/p')
+		if [ -z "$slot" ]; then
+			slot=$(softhsm2-util --show-slots | \
+			       grep -E "^Slot " | head -n1 |
+			       sed -n 's/Slot \([0-9]*\)/\1/p')
+			if [ -z "$slot" ]; then
+				echo "Could not parse slot number from output."
+				echo "$msg"
+				return 3
+			fi
+		fi
+
+		msg=$(p11tool --list-tokens 2>&1 | \
+			grep "token=${NAME}" | tail -n1)
+		if [ $? -ne 0 ]; then
+			echo "Could not list existing tokens"
+			echo "$msg"
+		fi
+		tokenuri=$(echo "$msg" | sed -n 's/.*URL: \([[:print:]*]\)/\1/p')
+		if [ -z "${tokenuri}" ]; then
+			echo "Could not get tokenuri!"
+			return 4
+		fi
+
+		# more recent versions of p11tool have --generate-privkey ...
+		msg=$(GNUTLS_PIN=$PIN p11tool \
+			--generate-privkey=rsa --bits 2048 --label mykey --login \
+			"${tokenuri}" 2>&1)
+		if [ $? -ne 0 ]; then
+			# ... older versions have --generate-rsa
+			msg=$(GNUTLS_PIN=$PIN p11tool \
+				--generate-rsa --bits 2048 --label mykey --login \
+				"${tokenuri}" 2>&1)
+			if [ $? -ne 0 ]; then
+				echo "Could not create RSA key!"
+				echo "$msg"
+				return 5
+			fi
+		fi
+	fi
+
+	getkeyuri_softhsm $slot
+	rc=$?
+	if [ $rc -ne 0 ]; then
+		teardown_softhsm
+	fi
+
+	return $rc
+}
+
+_getkeyuri_softhsm() {
+	local msg tokenuri keyuri
+
+	msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}")
+	if [ $? -ne 0 ]; then
+		echo "Could not list existing tokens"
+		echo "$msg"
+		return 5
+	fi
+	tokenuri=$(echo "$msg" | sed -n 's/.*URL: \([[:print:]*]\)/\1/p')
+	if [ -z "$tokenuri" ]; then
+		echo "Could not get token URL"
+		echo "$msg"
+		return 6
+	fi
+	msg=$(p11tool --list-all ${tokenuri} 2>&1)
+	if [ $? -ne 0 ]; then
+		echo "Could not list object under token $tokenuri"
+		echo "$msg"
+		softhsm2-util --show-slots
+		return 7
+	fi
+
+	keyuri=$(echo "$msg" | sed -n 's/.*URL: \([[:print:]*]\)/\1/p')
+	if [ -z "$keyuri" ]; then
+		echo "Could not get key URL"
+		echo "$msg"
+		return 8
+	fi
+	echo "$keyuri"
+	return 0
+}
+
+getkeyuri_softhsm() {
+	local keyuri rc
+
+	keyuri=$(_getkeyuri_softhsm)
+	rc=$?
+	if [ $rc -ne 0 ]; then
+		return $rc
+	fi
+	echo "keyuri: $keyuri?pin-value=${PIN}" #&module-name=softhsm2"
+	return 0
+}
+
+getpubkey_softhsm() {
+	local keyuri rc
+
+	keyuri=$(_getkeyuri_softhsm)
+	rc=$?
+	if [ $rc -ne 0 ]; then
+		return $rc
+	fi
+	GNUTLS_PIN=${PIN} p11tool --export-pubkey "${keyuri}" --login 2>/dev/null
+	return $?
+}
+
+usage() {
+	cat <<_EOF_
+Usage: $0 [command]
+
+Supported commands are:
+
+setup      : Setup the user's account for softhsm and create a
+             token and key with a test configuration
+
+getkeyuri  : Get the key's URI; may only be called after setup
+
+getpubkey  : Get the public key in PEM format; may only be called after setup
+
+teardown   : Remove the temporary softhsm test configuration
+
+_EOF_
+}
+
+main() {
+	local ret
+
+	if [ $# -lt 1 ]; then
+		usage $0
+		echo -e "Missing command.\n\n"
+		return 1
+	fi
+	case "$1" in
+	setup)
+		setup_softhsm
+		ret=$?
+		;;
+	getkeyuri)
+		getkeyuri_softhsm
+		ret=$?
+		;;
+	getpubkey)
+		getpubkey_softhsm
+		ret=$?
+		;;
+	teardown)
+		teardown_softhsm
+		ret=$?
+		;;
+	*)
+		echo -e "Unsupported command: $1\n\n"
+		usage $0
+		ret=1
+	esac
+	return $ret
+}
+
+main "$@"
+exit $?
-- 
2.31.1

[PATCH v3 8/9] tests: Extend sign_verify test with pkcs11-specific test

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Extend the sign_verify test with a pkcs11-specific test.

Since the openssl command line tool now needs to use a key provided by
an engine, extend some command lines with the additional parameters
'--keyform engine'. These parameters are passed using the global variable
OPENSSL_KEYFORM, which is only set when pkcs11 URIs are used.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/functions.sh     | 45 ++++++++++++++++++++++++++++++++++++
 tests/sign_verify.test | 52 ++++++++++++++++++++++++++++++------------
 2 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/tests/functions.sh b/tests/functions.sh
index 91cd5d9..dce278b 100755
--- a/tests/functions.sh
+++ b/tests/functions.sh
@@ -272,3 +272,48 @@ _report_exit() {
   fi
 }
 
+# Cleanups when exiting
+_at_exit() {
+  _report_exit
+  if [ -n "${WORKDIR}" ]; then
+    rm -f "${WORKDIR}"
+  fi
+}
+
+# Setup SoftHSM for local testing by calling the softhsm_setup script.
+# Use the provided workdir as the directory where SoftHSM will store its state
+# into.
+# Upon successfully setting up SoftHSM, this function sets the global variables
+# OPENSSL_ENGINE and OPENSSL_KEYFORM so that the openssl command line tool can
+# use SoftHSM. Also the PKCS11_KEYURI global variable is set to the test key's
+# pkcs11 URI.
+_softhsm_setup() {
+  local workdir="$1"
+
+  local msg
+
+  export SOFTHSM_SETUP_CONFIGDIR="${workdir}/softhsm"
+  export SOFTHSM2_CONF="${workdir}/softhsm/softhsm2.conf"
+
+  mkdir -p "${SOFTHSM_SETUP_CONFIGDIR}"
+
+  msg=$(./softhsm_setup setup 2>&1)
+  if [ $? -eq 0 ]; then
+    echo "softhsm_setup setup succeeded: $msg"
+    PKCS11_KEYURI=$(echo $msg | sed -n 's|^keyuri: \(.*\)|\1|p')
+
+    export EVMCTL_ENGINE="--engine pkcs11"
+    export OPENSSL_ENGINE="-engine pkcs11"
+    export OPENSSL_KEYFORM="-keyform engine"
+  else
+    echo "softhsm_setup setup failed: ${msg}"
+  fi
+}
+
+# Tear down the SoftHSM setup and clean up the environment
+_softhsm_teardown() {
+  ./softhsm_setup teardown &>/dev/null
+  rm -rf "${SOFTHSM_SETUP_CONFIGDIR}"
+  unset SOFTHSM_SETUP_CONFIGDIR SOFTHSM2_CONF PKCS11_KEYURI \
+    EVMCTL_ENGINE OPENSSL_ENGINE OPENSSL_KEYFORM
+}
\ No newline at end of file
diff --git a/tests/sign_verify.test b/tests/sign_verify.test
index 3b42eec..4741b92 100755
--- a/tests/sign_verify.test
+++ b/tests/sign_verify.test
@@ -28,7 +28,8 @@ fi
 
 ./gen-keys.sh >/dev/null 2>&1
 
-trap _report_exit EXIT
+trap _at_exit EXIT
+WORKDIR=$(mktemp -d)
 set -f # disable globbing
 
 # Determine keyid from a cert
@@ -132,11 +133,16 @@ check_sign() {
   # OPTS (additional options for evmctl),
   # FILE (working file to sign).
   local "$@"
-  local KEY=${KEY%.*}.key
+  local key verifykey
   local FILE=${FILE:-$ALG.txt}
 
-  # Normalize key filename
-  KEY=test-${KEY#test-}
+  # Normalize key filename if it's not a pkcs11 URI
+  if [ ${KEY:0:7} != pkcs11: ]; then
+    key=${KEY%.*}.key
+    key=test-${key#test-}
+  else
+    key=${KEY}
+  fi
 
   # Append suffix to files for negative tests, because we may
   # leave only good files for verify tests.
@@ -152,33 +158,33 @@ check_sign() {
 
   if _test_expected_to_pass; then
     # Can openssl work with this digest?
-    cmd="openssl dgst $OPENSSL_ENGINE -$ALG $FILE"
+    cmd="openssl dgst $OPENSSL_ENGINE $OPENSSL_KEYFORM -$ALG $FILE"
     echo - "$cmd"
     if ! $cmd >/dev/null; then
-      echo "${CYAN}$ALG ($KEY) test is skipped (openssl is unable to digest)$NORM"
+      echo "${CYAN}$ALG ($key) test is skipped (openssl is unable to digest)$NORM"
       return "$SKIP"
     fi
 
-    if [ ! -e "$KEY" ]; then
-      echo "${CYAN}$ALG ($KEY) test is skipped (key file not found)$NORM"
+    if [ "${key:0:7}" != pkcs11: ] && [ ! -e "$key" ]; then
+      echo "${CYAN}$ALG ($key) test is skipped (key file not found)$NORM"
       return "$SKIP"
     fi
 
     # Can openssl sign with this digest and key?
-    cmd="openssl dgst $OPENSSL_ENGINE -$ALG -sign $KEY -hex $FILE"
+    cmd="openssl dgst $OPENSSL_ENGINE $OPENSSL_KEYFORM -$ALG -sign $key -hex $FILE"
     echo - "$cmd"
     if ! $cmd >/dev/null; then
-      echo "${CYAN}$ALG ($KEY) test is skipped (openssl is unable to sign)$NORM"
+      echo "${CYAN}$ALG ($key) test is skipped (openssl is unable to sign)$NORM"
       return "$SKIP"
     fi
   fi
 
   # Insert keyid from cert into PREFIX in-place of marker `:K:'
   if [[ $PREFIX =~ :K: ]]; then
-    keyid=$(_keyid_from_cert "$KEY")
+    keyid=$(_keyid_from_cert "$key")
     if [ $? -ne 0 ]; then
       color_red
-      echo "Unable to determine keyid for $KEY"
+      echo "Unable to determine keyid for $key"
       color_restore
       return "$HARDFAIL"
     fi
@@ -187,7 +193,7 @@ check_sign() {
   fi
 
   # Perform signing by evmctl
-  _evmctl_sign "$TYPE" "$KEY" "$ALG" "$FILE" "$OPTS" || return
+  _evmctl_sign "$TYPE" "$key" "$ALG" "$FILE" "$OPTS" || return
 
   # First simple pattern match the signature.
   ADD_TEXT_FOR=$ALG \
@@ -207,7 +213,13 @@ check_sign() {
   _extract_xattr "$FILE" "$(_xattr "$TYPE")" "$FILE.sig2" "$PREFIX"
 
   # Verify extracted signature with openssl
-  cmd="openssl dgst $OPENSSL_ENGINE -$ALG -verify ${KEY%.*}.pub \
+  if [ "${key:0:7}" != pkcs11: ]; then
+      verifykey=${key%.*}.pub
+  else
+      verifykey=${key}
+  fi
+
+  cmd="openssl dgst $OPENSSL_ENGINE $OPENSSL_KEYFORM -$ALG -verify ${verifykey} \
 	-signature $FILE.sig2 $FILE"
   echo - "$cmd"
   if ! $cmd; then
@@ -413,3 +425,15 @@ expect_fail \
 expect_fail \
   check_sign TYPE=ima KEY=gost2012_256-B ALG=md_gost12_512 PREFIX=0x0302 OPTS=
 
+# Test signing with key described by pkcs11 URI
+_softhsm_setup "${WORKDIR}"
+if [ -n "${PKCS11_KEYURI}" ]; then
+  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY=${PKCS11_KEYURI} ALG=sha256 PREFIX=0x030204aabbccdd0100 OPTS=--keyid=aabbccdd
+  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY=${PKCS11_KEYURI} ALG=sha1   PREFIX=0x030202aabbccdd0100 OPTS=--keyid=aabbccdd
+else
+  # to have a constant number of tests, skip these two tests
+  __skip() { echo "pkcs11 test is skipped: could not setup softhsm"; return $SKIP; }
+  expect_pass __skip
+  expect_pass __skip
+fi
+_softhsm_teardown "${WORKDIR}"
-- 
2.31.1

[PATCH v3 9/9] tests: Get the packages for pkcs11 testing on the CI/CD system

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Get the packages for pkcs11 testing on the CI/CD system.

This is the status on various distros:

- Alpine: could not find package with pkcs11 engine
- Alt Linux: works
- Debian: works
- CentOS7: softhsm 2.1.0 is too old for tests to work; tests also fail
           when trying to sign with pkcs11 URI using openssl command line tool
- CentOS: works
- Fedora: works
- OpenSuSE Leap: package not available in main repo
- OpenSuSE Tumbleweed: works

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 ci/alt.sh        | 3 +++
 ci/debian.sh     | 3 ++-
 ci/fedora.sh     | 8 ++++++++
 ci/tumbleweed.sh | 3 +++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/ci/alt.sh b/ci/alt.sh
index 884c995..65389be 100755
--- a/ci/alt.sh
+++ b/ci/alt.sh
@@ -12,12 +12,15 @@ apt-get install -y \
 		asciidoc \
 		attr \
 		docbook-style-xsl \
+		gnutls-utils \
 		libattr-devel \
 		libkeyutils-devel \
+		libp11 \
 		libssl-devel \
 		openssl \
 		openssl-gost-engine \
 		rpm-build \
+		softhsm \
 		wget \
 		xsltproc \
 		xxd \
diff --git a/ci/debian.sh b/ci/debian.sh
index ad7d2c0..ae5c9c1 100755
--- a/ci/debian.sh
+++ b/ci/debian.sh
@@ -48,7 +48,8 @@ $apt \
 	procps \
 	sudo \
 	wget \
-	xsltproc \
+	xsltproc
 
 $apt xxd || $apt vim-common
 $apt libengine-gost-openssl1.1$ARCH || true
+$apt softhsm gnutls-bin libengine-pkcs11-openssl1.1$ARCH || true
diff --git a/ci/fedora.sh b/ci/fedora.sh
index 2d80915..0993607 100755
--- a/ci/fedora.sh
+++ b/ci/fedora.sh
@@ -25,6 +25,7 @@ yum -y install \
 	automake \
 	diffutils \
 	docbook-xsl \
+	gnutls-utils \
 	gzip \
 	keyutils-libs-devel \
 	libattr-devel \
@@ -33,6 +34,7 @@ yum -y install \
 	make \
 	openssl \
 	openssl-devel \
+	openssl-pkcs11 \
 	pkg-config \
 	procps \
 	sudo \
@@ -42,3 +44,9 @@ yum -y install \
 
 yum -y install docbook5-style-xsl || true
 yum -y install swtpm || true
+
+# SoftHSM is available via EPEL on CentOS
+if [ -f /etc/centos-release ]; then
+	yum -y install epel-release
+fi
+yum -y install softhsm || true
\ No newline at end of file
diff --git a/ci/tumbleweed.sh b/ci/tumbleweed.sh
index dfc478b..4e3da0c 100755
--- a/ci/tumbleweed.sh
+++ b/ci/tumbleweed.sh
@@ -42,6 +42,9 @@ zypper --non-interactive install --force-resolution --no-recommends \
 	which \
 	xsltproc
 
+zypper --non-interactive install --force-resolution --no-recommends \
+	gnutls openssl-engine-libp11 softhsm || true
+
 if [ -f /usr/lib/ibmtss/tpm_server -a ! -e /usr/local/bin/tpm_server ]; then
 	ln -s /usr/lib/ibmtss/tpm_server /usr/local/bin
 fi
-- 
2.31.1

Re: [PATCH v3 0/9] ima-evm-utils: Add support for signing with pkcs11 URIs

[Mimi Zohar]

On Wed, 2021-09-08 at 15:23 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>

Having a "From: " line normally occurs when the original author is
being credited with the patch, but the patch is being updated/modified
by someone else.

> 
> This series of patches adds support for signing with pkcs11 URIs so that
> pkcs11-enabled devices can also be used for file signing.
> 
> A test program is provided setting up SoftHSM for test cases to use. I added
> two test cases for testing with SoftHSM.

The correlation between the first paragraph and the second is
missing.   The second paragraph also needs to be written in the 3rd
person.  Perhaps say something like, "Extend the existing
sign_verify.test with tests for the new pkcs11 URI support.  Use the
SoftHSM, when available, as the pkcs11 device."

thanks,

Mimi

Re: [PATCH v3 3/9] evmctl: Implement function for setting up an OpenSSL engine

[Mimi Zohar]

On Wed, 2021-09-08 at 15:23 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Implement setup_engine for setting up an OpenSSL engine.

Duplicating the subject line like this, isn't any different than
leaving out the patch description.  Please provide a proper patch
description (e.g. code cleanup).

thanks,

Mimi

Re: [PATCH v3 7/9] tests: Import softhsm_setup script to enable pkcs11 test case

[Mimi Zohar]

On Wed, 2021-09-08 at 15:23 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Import softhsm_setup script from my swtpm project and contribute
> it to this project under dual license BSD 3-clause and GLP 2.0.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks, Stefan, for splitting this out from the other changes, making
it a separate patch.
> ---
>  tests/softhsm_setup | 297 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 297 insertions(+)
>  create mode 100755 tests/softhsm_setup
> 
> diff --git a/tests/softhsm_setup b/tests/softhsm_setup
> new file mode 100755
> index 0000000..fe69a6e
> --- /dev/null
> +++ b/tests/softhsm_setup
> @@ -0,0 +1,297 @@
> +#!/usr/bin/env bash
> +
> +# This program originates from 'swtpm' project (https://github.com/stefanberger/swtpm/)
> +# and is provided to ima-evm-utils under a dual license:
> +# - BSD 3-clause
> +# - GPL-2.0

The newer code includes an SPDX line.  Is there a reason for not
including it/them here?

thanks,

Mimi

Re: [PATCH v3 8/9] tests: Extend sign_verify test with pkcs11-specific test

[Mimi Zohar]

On Wed, 2021-09-08 at 15:23 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Extend the sign_verify test with a pkcs11-specific test.
> 
> Since the openssl command line tool now needs to use a key provided by
> an engine, extend some command lines with the additional parameters
> '--keyform engine'. These parameters are passed using the global variable
> OPENSSL_KEYFORM, which is only set when pkcs11 URIs are used.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks, Stefan.  Much easier to review as an independent patch.
> ---
>  tests/functions.sh     | 45 ++++++++++++++++++++++++++++++++++++
>  tests/sign_verify.test | 52 ++++++++++++++++++++++++++++++------------
>  2 files changed, 83 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/functions.sh b/tests/functions.sh
> index 91cd5d9..dce278b 100755
> --- a/tests/functions.sh
> +++ b/tests/functions.sh
> @@ -272,3 +272,48 @@ _report_exit() {
>    fi
>  }
>  
> +# Cleanups when exiting
> +_at_exit() {
> +  _report_exit
> +  if [ -n "${WORKDIR}" ]; then
> +    rm -f "${WORKDIR}"
> +  fi
> +}

There's one caller of _report_exit().  I don't see a need to define a
new function.

thanks,

Mimi

Re: [PATCH v3 9/9] tests: Get the packages for pkcs11 testing on the CI/CD system

[Mimi Zohar]

On Wed, 2021-09-08 at 15:23 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Get the packages for pkcs11 testing on the CI/CD system.

How about saying, "Get the packages for pkcs11 testing on the CI/CD
system, where available.  On those system where it is not available,
skip the two tests."

Then just list those that do not have pkcs11 support or it's too old.

thanks,

Mimi

> 
> This is the status on various distros:
> 
> - Alpine: could not find package with pkcs11 engine
> - Alt Linux: works
> - Debian: works
> - CentOS7: softhsm 2.1.0 is too old for tests to work; tests also fail
>            when trying to sign with pkcs11 URI using openssl command line tool
> - CentOS: works
> - Fedora: works
> - OpenSuSE Leap: package not available in main repo
> - OpenSuSE Tumbleweed: works
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>