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

[PATCH 0/4] 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 explicityly provided via a command line option.
A test program is provided setting up softhsm for test cases to use.

   Stefan

Stefan Berger (4):
  evmctl: Implement support for EVMCTL_KEY_PASSWORD environment variable
  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           |   7 +
 src/libimaevm.c        |  59 +++++++--
 tests/functions.sh     |  26 ++++
 tests/sign_verify.test |  50 +++++--
 tests/softhsm_setup    | 290 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 426 insertions(+), 25 deletions(-)
 create mode 100755 tests/softhsm_setup

-- 
2.31.1

[PATCH 1/4] 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 2/4] 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 | 59 ++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 50 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 58f8e66..2e85f8b 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..b84e5b8 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"
@@ -803,21 +804,57 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
 {
 	FILE *fp;
 	EVP_PKEY *pkey;
+	ENGINE *e;
 
-	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;
+		}
+
+		ENGINE_load_builtin_engines();
+		e = ENGINE_by_id("pkcs11");
+		if (!e) {
+			log_err("Failed to load pkcs11 engine\n");
+			goto err_pkcs11;
+		}
+		if (!ENGINE_init(e)) {
+			log_err("Failed to initialize the pkcs11 engine\n");
+			goto err_pkcs11;
+		}
+		if (keypass) {
+			if (!ENGINE_ctrl_cmd_string(e, "PIN", keypass, 0)) {
+				log_err("Failed to set the PIN for the private key\n");
+				goto err_pkcs11;
+			}
+		}
+		pkey = ENGINE_load_private_key(e, keyfile, NULL, NULL);
+		if (!pkey) {
+			log_err("Failed to load private key %s\n", keyfile);
+			goto err_pkcs11;
+		}
+	} 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_pkcs11:
+	ENGINE_free(e);
+	output_openssl_errors();
+	return NULL;
 }
 
 static RSA *read_priv_key(const char *keyfile, const char *keypass)
-- 
2.31.1

[PATCH 3/4] tests: Extend sign_verify test with pkcs11 specific test

[Stefan Berger]

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

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 4/4] 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 2/4] libimaevm: Add support for pkcs11 private keys for signing a v2 hash

[James Bottomley]

On Mon, 2021-08-09 at 11:10 -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>
> ---
>  README          |  1 +
>  src/evmctl.c    |  1 +
>  src/libimaevm.c | 59 ++++++++++++++++++++++++++++++++++++++++-------
> --
>  3 files changed, 50 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 58f8e66..2e85f8b 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..b84e5b8 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"
> @@ -803,21 +804,57 @@ static EVP_PKEY *read_priv_pkey(const char
> *keyfile, const char *keypass)
>  {
>  	FILE *fp;
>  	EVP_PKEY *pkey;
> +	ENGINE *e;
>  
> -	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;
> +		}
> +
> +		ENGINE_load_builtin_engines();
> +		e = ENGINE_by_id("pkcs11");
> +		if (!e) {
> +			log_err("Failed to load pkcs11 engine\n");
> +			goto err_pkcs11;
> +		}
> +		if (!ENGINE_init(e)) {
> +			log_err("Failed to initialize the pkcs11
> engine\n");
> +			goto err_pkcs11;
> +		}
> +		if (keypass) {
> +			if (!ENGINE_ctrl_cmd_string(e, "PIN", keypass,
> 0)) {
> +				log_err("Failed to set the PIN for the
> private key\n");
> +				goto err_pkcs11;
> +			}
> +		}
> +		pkey = ENGINE_load_private_key(e, keyfile, NULL, NULL);
> +		if (!pkey) {
> +			log_err("Failed to load private key %s\n",
> keyfile);
> +			goto err_pkcs11;
> +		}
> +	} 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);

This looks a bit narrow.  Isn't the problem that *any* engine key might
be a specifier not a file?  In which case the generic fix is not to
validate file existence if an engine is present on the command line. 
That way you can specify any engine key and the pkcs11 key simply
becomes a special case of this.

If you insist on not having to specify --engine for pkcs11 keys then
you do the prefix check earlier on and set the engine to pkcs11 (if
it's not already set to something else).

James

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

[Stefan Berger]

On 8/9/21 11:27 AM, James Bottomley wrote:

> On Mon, 2021-08-09 at 11:10 -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>
>> ---
>>   README          |  1 +
>>   src/evmctl.c    |  1 +
>>   src/libimaevm.c | 59 ++++++++++++++++++++++++++++++++++++++++-------
>> --
>>   3 files changed, 50 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 58f8e66..2e85f8b 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..b84e5b8 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"
>> @@ -803,21 +804,57 @@ static EVP_PKEY *read_priv_pkey(const char
>> *keyfile, const char *keypass)
>>   {
>>   	FILE *fp;
>>   	EVP_PKEY *pkey;
>> +	ENGINE *e;
>>   
>> -	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;
>> +		}
>> +
>> +		ENGINE_load_builtin_engines();
>> +		e = ENGINE_by_id("pkcs11");
>> +		if (!e) {
>> +			log_err("Failed to load pkcs11 engine\n");
>> +			goto err_pkcs11;
>> +		}
>> +		if (!ENGINE_init(e)) {
>> +			log_err("Failed to initialize the pkcs11
>> engine\n");
>> +			goto err_pkcs11;
>> +		}
>> +		if (keypass) {
>> +			if (!ENGINE_ctrl_cmd_string(e, "PIN", keypass,
>> 0)) {
>> +				log_err("Failed to set the PIN for the
>> private key\n");
>> +				goto err_pkcs11;
>> +			}
>> +		}
>> +		pkey = ENGINE_load_private_key(e, keyfile, NULL, NULL);
>> +		if (!pkey) {
>> +			log_err("Failed to load private key %s\n",
>> keyfile);
>> +			goto err_pkcs11;
>> +		}
>> +	} 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);
> This looks a bit narrow.  Isn't the problem that *any* engine key might
> be a specifier not a file?  In which case the generic fix is not to
> validate file existence if an engine is present on the command line.
> That way you can specify any engine key and the pkcs11 key simply
> becomes a special case of this.
>
> If you insist on not having to specify --engine for pkcs11 keys then
> you do the prefix check earlier on and set the engine to pkcs11 (if
> it's not already set to something else).

I had thought about it but what kept me from wanting to use the engine 
in evmctl is the crossing into the library and having to extend the 
library API there just for this purpose or extending the global variable 
'imaevm_params', which is less than ideal as well.


    Stefan


>
> James
>
>