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

[PATCH v2 0/8] 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 and the
keyid explicitly provided via a command line option.
A test program is provided setting up softhsm for test cases to use. Tests
are passing on gitub actions with some distros not enabled (Debian, Ubuntu)
where I could not reproduce the github actions failure on the same type of
container locally (debian:stable).

   Stefan
   
v2:
  - Use global imaevm_params.eng field for the ENGINE
  - Fix bug on engine initialization in existing code

Stefan Berger (8):
  evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable
  evmctl: Handle engine initialization properly
  evmctl: Move code setting up engine to own funtion
  evmctl: Extend libimaevm_params with ENGINE field and use it
  evmctl: Setup the pkcs11 engine if key has pkcs11: prefix
  libimaevm: Add support for pkcs11 private keys for signing a v2 hash
  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/fedora.sh           |   8 ++
 ci/tumbleweed.sh       |   3 +
 src/evmctl.c           |  54 +++++---
 src/imaevm.h           |   2 +
 src/libimaevm.c        |  47 +++++--
 tests/functions.sh     |  26 ++++
 tests/sign_verify.test |  50 +++++--
 tests/softhsm_setup    | 290 +++++++++++++++++++++++++++++++++++++++++
 10 files changed, 448 insertions(+), 40 deletions(-)
 create mode 100755 tests/softhsm_setup

-- 
2.31.1

[PATCH v2 1/8] 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 v2 2/8] evmctl: Handle engine initialization properly

[Stefan Berger]

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

Fix the following issue when passing a not available engine:

$ ./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)

Also, jump to the exit when the setup of the engine failed.

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 v2 3/8] evmctl: Move code setting up engine to own funtion

[Stefan Berger]

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

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 v2 4/8] evmctl: Extend libimaevm_params with ENGINE field and use it

[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 v2 5/8] evmctl: Setup the pkcs11 engine if key has pkcs11: prefix

[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 v2 6/8] 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 v2 7/8] 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.
Import softhsm_setup script from my swtpm project and contribute
it to this porject under dual license BSD 3-clause and GLP 2.0.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/functions.sh     |  26 ++++
 tests/sign_verify.test |  50 +++++--
 tests/softhsm_setup    | 290 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 352 insertions(+), 14 deletions(-)
 create mode 100755 tests/softhsm_setup

diff --git a/tests/functions.sh b/tests/functions.sh
index 91cd5d9..cbb7ea4 100755
--- a/tests/functions.sh
+++ b/tests/functions.sh
@@ -272,3 +272,29 @@ _report_exit() {
   fi
 }
 
+_at_exit() {
+  _report_exit
+  if [ -n "${WORKDIR}" ]; then
+    rm -f "${WORKDIR}"
+  fi
+}
+
+_softhsm_setup() {
+  local workdir="$1"
+
+  local msg
+
+  export SOFTHSM_SETUP_CONFIGDIR="${workdir}"
+  export SOFTHSM2_CONF="${workdir}/softhsm2.conf"
+
+  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 OPENSSL_ENGINE="-engine pkcs11"
+    export OPENSSL_KEYFORM="-keyform engine"
+  else
+    echo "softhsm_setup setup failed: ${msg}"
+  fi
+}
diff --git a/tests/sign_verify.test b/tests/sign_verify.test
index 3b42eec..369765e 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,13 @@ expect_fail \
 expect_fail \
   check_sign TYPE=ima KEY=gost2012_256-B ALG=md_gost12_512 PREFIX=0x0302 OPTS=
 
+_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
+  # skip these two tests
+  __skip() { echo "pkcs11 test is skipped: could not setup softhsm"; return $SKIP; }
+  expect_pass __skip
+  expect_pass __skip
+fi
diff --git a/tests/softhsm_setup b/tests/softhsm_setup
new file mode 100755
index 0000000..f89a2e6
--- /dev/null
+++ b/tests/softhsm_setup
@@ -0,0 +1,290 @@
+#!/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
+
+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 v2 8/8] 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: debian:stable: evmctl is not able to find the pkcs11 module but
          preceeding openssl command line tests with the pkcs11 URI succeeded;
          cannot recreate the issue locally in the debian:stable container
          --> disabled on Ubuntu and Debian
- CentOS7: tests with pkcs11 URI fail on openssl command line level
- 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/fedora.sh     | 8 ++++++++
 ci/tumbleweed.sh | 3 +++
 3 files changed, 14 insertions(+)

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/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 v2 1/8] evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable

[Mimi Zohar]

[Cc: Vitaly]

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> 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>

Thanks, Stefan.

Vitaly, I'm not sure that there's any benefit of using secure heap for
a password stored as an environment variable, but it needs to at least
be documented.

thanks,

Mimi

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

[Mimi Zohar]

Hi Stefan,

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> This series of patches adds support for signing with pkcs11 URIs and the
> keyid explicitly provided via a command line option.

Before explaining "what" , please provide the motivation for adding
pkcs11 support.

Other than updating the cover letter and the patch descriptions,
patches 1 - 6 look good.  (Still looking at 7/8 and 8/8.)

thanks,

Mimi

Re: [PATCH v2 2/8] evmctl: Handle engine initialization properly

[Mimi Zohar]

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Fix the following issue when passing a not available engine:

First describe the problem and then include details and/or an example. 
For example, "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)
> 
> Also, jump to the exit when the setup of the engine failed.

Patch descriptions should not be explaining the details of the code,
but providing the motivation for the patch.  Please remove this line.

> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Thanks,

Mimi

> ---
>  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);

Re: [PATCH v2 3/8] evmctl: Move code setting up engine to own funtion

[Mimi Zohar]

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>

Missing patch description.

For example, the subject line could be "evmctl: define a function to
setup the openssl engine".  Provide the motivation before saying, "move
the existing code setting up engine to own function."

thanks,

Mimi

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

[Mimi Zohar]

Hi Stefan,

Perhaps reword the subject line as, "libimaevm: add pkcs11 private key
support for signing a v2 hash".

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> 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>

thanks,

Mimi

Re: [PATCH v2 4/8] evmctl: Extend libimaevm_params with ENGINE field and use it

[Mimi Zohar]

Perhaps update the subject line like, "Define and use an ENGINE field
in libimaevm_params".

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> 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>

thanks,

Mimi

Re: [PATCH v2 5/8] evmctl: Setup the pkcs11 engine if key has pkcs11: prefix

[Mimi Zohar]

Hi Stefan,

Simplify the subject line.  Perhaps something like,  "evmctl: use the
pkcs11 engine for pkcs11 prefixed URIs"?

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> 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>

thanks

Mimi

Re: [PATCH v2 7/8] tests: Extend sign_verify test with pkcs11-specific test

[Mimi Zohar]

Hi Stefan,

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Extend the sign_verify test with a pkcs11-specific test.
> Import softhsm_setup script from my swtpm project and contribute
> it to this porject under dual license BSD 3-clause and GLP 2.0.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Up to here, the patches were nicely split up.  Just from reading the
patch description, this patch needs to be split up.

> ---
>  tests/functions.sh     |  26 ++++
>  tests/sign_verify.test |  50 +++++--
>  tests/softhsm_setup    | 290 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 352 insertions(+), 14 deletions(-)
>  create mode 100755 tests/softhsm_setup
> 
> diff --git a/tests/functions.sh b/tests/functions.sh
> index 91cd5d9..cbb7ea4 100755
> --- a/tests/functions.sh
> +++ b/tests/functions.sh
> @@ -272,3 +272,29 @@ _report_exit() {
>    fi
>  }
>  
> +_at_exit() {
> +  _report_exit
> +  if [ -n "${WORKDIR}" ]; then
> +    rm -f "${WORKDIR}"
> +  fi
> +}
> +

It would be nice to have a function comment here.

> +_softhsm_setup() {
> +  local workdir="$1"
> +

${WORKDIR} is being passed as a parameter.  Why is a local environment
variable needed?

> +  local msg
> +
> +  export SOFTHSM_SETUP_CONFIGDIR="${workdir}"
> +  export SOFTHSM2_CONF="${workdir}/softhsm2.conf"
> +
> +  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 OPENSSL_ENGINE="-engine pkcs11"
> +    export OPENSSL_KEYFORM="-keyform engine"
> +  else
> +    echo "softhsm_setup setup failed: ${msg}"
> +  fi

Should there be a test checking that softhsm_setup is installed before
using it?   If it's not installed, then the test is "skipped".

> +}
> diff --git a/tests/sign_verify.test b/tests/sign_verify.test
> index 3b42eec..369765e 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

Agreed, don't modify the global variable, use a local one.  Making this
a separate patch, would simplify review.

thanks,

Mimi

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

[Mimi Zohar]

On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> 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: debian:stable: evmctl is not able to find the pkcs11 module but
>           preceeding openssl command line tests with the pkcs11 URI succeeded;
>           cannot recreate the issue locally in the debian:stable container
>           --> disabled on Ubuntu and Debian
> - CentOS7: tests with pkcs11 URI fail on openssl command line level
> - CentOS: works
> - Fedora: works
> - OpenSuSE Leap: package not available in main repo
> - OpenSuSE Tumbleweed: works

In patch 7/8 there's a comment of requiring a certain release.  Should
there be a test for a specific version?  Then only run the pkcs11 tests
if that version or later is installed.

thanks,

Mimi

Re: [PATCH v2 7/8] tests: Extend sign_verify test with pkcs11-specific test

[Stefan Berger]

On 9/3/21 3:11 PM, Mimi Zohar wrote:
> Hi Stefan,
>
> On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Extend the sign_verify test with a pkcs11-specific test.
>> Import softhsm_setup script from my swtpm project and contribute
>> it to this porject under dual license BSD 3-clause and GLP 2.0.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Up to here, the patches were nicely split up.  Just from reading the
> patch description, this patch needs to be split up.

Ok, softhsm_setup will be a separate patch then before function.sh and 
sign_verify.test are being patched.


>
>> ---
>>   tests/functions.sh     |  26 ++++
>>   tests/sign_verify.test |  50 +++++--
>>   tests/softhsm_setup    | 290 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 352 insertions(+), 14 deletions(-)
>>   create mode 100755 tests/softhsm_setup
>>
>> diff --git a/tests/functions.sh b/tests/functions.sh
>> index 91cd5d9..cbb7ea4 100755
>> --- a/tests/functions.sh
>> +++ b/tests/functions.sh
>> @@ -272,3 +272,29 @@ _report_exit() {
>>     fi
>>   }
>>   
>> +_at_exit() {
>> +  _report_exit
>> +  if [ -n "${WORKDIR}" ]; then
>> +    rm -f "${WORKDIR}"
>> +  fi
>> +}
>> +
> It would be nice to have a function comment here.

I can add this.

>
>> +_softhsm_setup() {
>> +  local workdir="$1"
>> +
> ${WORKDIR} is being passed as a parameter.  Why is a local environment
> variable needed?


I prefer to avoid accessing them when they can be passed to functions as 
parameters. I rather only use global variables at the 'top level' and 
then pass them down as parameters to all the lower level functions.


>
>> +  local msg
>> +
>> +  export SOFTHSM_SETUP_CONFIGDIR="${workdir}"
>> +  export SOFTHSM2_CONF="${workdir}/softhsm2.conf"
>> +
>> +  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 OPENSSL_ENGINE="-engine pkcs11"
>> +    export OPENSSL_KEYFORM="-keyform engine"
>> +  else
>> +    echo "softhsm_setup setup failed: ${msg}"
>> +  fi
> Should there be a test checking that softhsm_setup is installed before
> using it?   If it's not installed, then the test is "skipped".

softhsm_setup is being contributed to this project via the code above, 
so it should be available.


>
>> +}
>> diff --git a/tests/sign_verify.test b/tests/sign_verify.test
>> index 3b42eec..369765e 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
> Agreed, don't modify the global variable, use a local one.  Making this
> a separate patch, would simplify review.
> thanks,
>
> Mimi
>

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

[Stefan Berger]

On 9/3/21 3:17 PM, Mimi Zohar wrote:

> On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
>> 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: debian:stable: evmctl is not able to find the pkcs11 module but
>>            preceeding openssl command line tests with the pkcs11 URI succeeded;
>>            cannot recreate the issue locally in the debian:stable container
>>            --> disabled on Ubuntu and Debian
>> - CentOS7: tests with pkcs11 URI fail on openssl command line level
>> - CentOS: works
>> - Fedora: works
>> - OpenSuSE Leap: package not available in main repo
>> - OpenSuSE Tumbleweed: works
> In patch 7/8 there's a comment of requiring a certain release.  Should
> there be a test for a specific version?  Then only run the pkcs11 tests
> if that version or later is installed.

I'll add a test into softhsm_setup checking that the version identifier 
is at least 2.2.0.

   Stefan

> thanks,
>
> Mimi
>

Re: [PATCH v2 1/8] evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable

[Vitaly Chikunov]

Mimi, Stefan,

On Fri, Aug 27, 2021 at 05:37:35PM -0400, Mimi Zohar wrote:
> [Cc: Vitaly]
> 
> On Tue, 2021-08-10 at 09:45 -0400, Stefan Berger wrote:
> > 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>
> 
> Thanks, Stefan.
> 
> Vitaly, I'm not sure that there's any benefit of using secure heap for
> a password stored as an environment variable, but it needs to at least
> be documented.

I did not receive this patch nor it's in the git.

Thanks,

> 
> thanks,
> 
> Mimi