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@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
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
