mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
* [ptxdist] [PATCH v2 1/4] ptxd_lib_code_signing: take PKCS#11 PIN from the environment
@ 2021-08-10  9:59 Roland Hieber
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 2/4] ptxd_lib_code_signing: refactor cs_check_env for SoftHSM workflow Roland Hieber
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Roland Hieber @ 2021-08-10  9:59 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2: no changes

PATCH v1: https://lore.ptxdist.org/ptxdist/20210809144030.22764-1-rhi@pengutronix.de
---
 bin/ptxdist                          |  2 +-
 doc/dev_code_signing.rst             | 12 ++++++++++++
 scripts/lib/ptxd_lib_code_signing.sh | 21 +++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/bin/ptxdist b/bin/ptxdist
index 2faaf535c1b9..6e843c26c37d 100755
--- a/bin/ptxdist
+++ b/bin/ptxdist
@@ -2163,7 +2163,7 @@ setup_env() {
 	unset $({
 		export -p  | sed -n 's/^declare -x \([^=]*\).*$/\1/p'
 		export -fp | sed -n 's/^declare -fx \([^=]*\).*$/\1/p'
-		} | egrep -v "^(PTXDIST_PTXRC|PTX_AUTOBUILD_DESTDIR|CCACHE_.*|PWD|HOME|USER|PATH|TERM|COLUMNS|LINES|DISPLAY|TMPDIR|KCONFIG_ALLCONFIG|KCONFIG_SEED|http_proxy|https_proxy|ftp_proxy|no_proxy${whitelist})$")
+		} | egrep -v "^(PTXDIST_PTXRC|PTX_AUTOBUILD_DESTDIR|PTXDIST_PKCS11_PIN|CCACHE_.*|PWD|HOME|USER|PATH|TERM|COLUMNS|LINES|DISPLAY|TMPDIR|KCONFIG_ALLCONFIG|KCONFIG_SEED|http_proxy|https_proxy|ftp_proxy|no_proxy${whitelist})$")
 
 	######## the environment is clean now ########
 
diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
index b9a7c42f2a55..8407b6a3ed3d 100644
--- a/doc/dev_code_signing.rst
+++ b/doc/dev_code_signing.rst
@@ -172,3 +172,15 @@ also via an environment variable.
   (``=``, not ``:=``).
   Otherwise the variable is expanded before a code signing provider can perform
   its setup.
+
+PIN Handling
+^^^^^^^^^^^^
+
+You can also supply the PKCS#11 PIN in the environment variable
+``PTXDIST_PKCS11_PIN`` when calling PTXdist instead of including it in the
+URI (using the parameter ``pin-value=<pin>``).
+This has the advantage that the PIN is not printed to the terminal or the
+logfile during the PTXdist run.
+The value of this variable is passed on in the environment to several programs
+that access the PKCS#11 API during the build (e.g. the kernel build system, the
+i.MX code signing tool, evmctl, mkfs, u-Boot's mkimage, rauc).
diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index 5ba1a4666af4..5579161cd5cf 100644
--- a/scripts/lib/ptxd_lib_code_signing.sh
+++ b/scripts/lib/ptxd_lib_code_signing.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 #
 # Copyright (C) 2019 Sascha Hauer <s.hauer@pengutronix.de>
+# Copyright (C) 2021 Marc Kleine-Budde <mkl@pengutronix.de>
 #
 # For further information about the PTXdist project and license conditions
 # see the README file.
@@ -11,6 +12,26 @@
 # infrastructure.
 #
 
+#
+# cs_export_pin
+#
+# Called at startup to export the PKCS#11 PIN to environment variables that are
+# used by the individual signing programs
+#
+cs_export_pin() {
+    if [ -z ${PTXDIST_PKCS11_PIN} ]; then
+        return
+    fi
+
+    export CST_SIGN_PIN=${PTXDIST_PKCS11_PIN}
+    export EVMCTL_SIGN_PIN=${PTXDIST_PKCS11_PIN}
+    export KBUILD_SIGN_PIN=${PTXDIST_PKCS11_PIN}
+    export MKFS_UBIFS_SIGN_PIN=${PTXDIST_PKCS11_PIN}
+    export MKIMAGE_SIGN_PIN=${PTXDIST_PKCS11_PIN}
+    export RAUC_PKCS11_PIN=${PTXDIST_PKCS11_PIN}
+}
+cs_export_pin
+
 cs_check_env() {
     if [ -z "${SOFTHSM2_CONF}" ]; then
 	ptxd_bailout "SOFTHSM2_CONF is not defined. Maybe \$(CODE_SIGNING_ENV) is not used."
-- 
2.30.2


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de


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

* [ptxdist] [PATCH v2 2/4] ptxd_lib_code_signing: refactor cs_check_env for SoftHSM workflow
  2021-08-10  9:59 [ptxdist] [PATCH v2 1/4] ptxd_lib_code_signing: take PKCS#11 PIN from the environment Roland Hieber
@ 2021-08-10  9:59 ` Roland Hieber
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files Roland Hieber
  2021-08-10 10:00 ` [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init Roland Hieber
  2 siblings, 0 replies; 8+ messages in thread
From: Roland Hieber @ 2021-08-10  9:59 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

Checking for PKCS11_MODULE_PATH etc. is also useful for the non-SoftHSM
workflow, but the other variables are specific to SoftHSM. Split off the
SoftHSM checks up into a separate function.

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2: no changes

PATCH v1: https://lore.ptxdist.org/ptxdist/20210809144030.22764-2-rhi@pengutronix.de
---
 scripts/lib/ptxd_lib_code_signing.sh | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index 5579161cd5cf..f012f8e194c7 100644
--- a/scripts/lib/ptxd_lib_code_signing.sh
+++ b/scripts/lib/ptxd_lib_code_signing.sh
@@ -32,13 +32,8 @@ cs_export_pin() {
 }
 cs_export_pin
 
+# internal
 cs_check_env() {
-    if [ -z "${SOFTHSM2_CONF}" ]; then
-	ptxd_bailout "SOFTHSM2_CONF is not defined. Maybe \$(CODE_SIGNING_ENV) is not used."
-    fi
-    if [ ! -e "${SOFTHSM2_CONF}" ]; then
-	ptxd_bailout "'${SOFTHSM2_CONF}' is missing."
-    fi
     if [ -z "${PKCS11_MODULE_PATH}" ]; then
 	ptxd_bailout "PKCS11_MODULE_PATH is not defined. Maybe \$(CODE_SIGNING_ENV) is not used."
     fi
@@ -48,6 +43,18 @@ cs_check_env() {
 }
 export -f cs_check_env
 
+# internal
+cs_check_env_softhsm() {
+    cs_check_env
+    if [ -z "${SOFTHSM2_CONF}" ]; then
+	ptxd_bailout "SOFTHSM2_CONF is not defined. Maybe \$(CODE_SIGNING_ENV) is not used."
+    fi
+    if [ ! -e "${SOFTHSM2_CONF}" ]; then
+	ptxd_bailout "'${SOFTHSM2_CONF}' is missing."
+    fi
+}
+export -f cs_check_env_softhsm
+
 #
 # softhsm_pkcs11_tool_init <args>
 #
@@ -88,7 +95,7 @@ export -f cs_init_variables
 # Initialize SoftHSM and set the initial pin
 #
 cs_init_softhsm() {
-    cs_check_env
+    cs_check_env_softhsm
     cs_init_variables
     local shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
 
-- 
2.30.2


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de


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

* [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files
  2021-08-10  9:59 [ptxdist] [PATCH v2 1/4] ptxd_lib_code_signing: take PKCS#11 PIN from the environment Roland Hieber
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 2/4] ptxd_lib_code_signing: refactor cs_check_env for SoftHSM workflow Roland Hieber
@ 2021-08-10  9:59 ` Roland Hieber
  2021-08-24 14:54   ` Bastian Krause
  2021-08-10 10:00 ` [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init Roland Hieber
  2 siblings, 1 reply; 8+ messages in thread
From: Roland Hieber @ 2021-08-10  9:59 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber, Bastian Krause

Currently, sysroot-host/var/lib/keys/${keyprovider} is left over even
when the provider package is cleaned, which could lead to
inconsistencies and leaked key material in the SoftHSM use case.
Introduce cs_clean and cs_clean_softhsm shell functions to clean up
those files. Call the cleanup functions in the clean stage of the
providers.

Reported-by: Bastian Krause <bst@pengutronix.de>
Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 - spell Bastian's last name correctly (sorry!) (feedback from Bastian
   Krause)
 - split off and extend cs_init stuff into next patch

PATCH v1: https://lore.ptxdist.org/ptxdist/20210809144030.22764-3-rhi@pengutronix.de
---
 doc/ref_code_signing_helpers.rst              | 29 ++++++++++++++++
 rules/host-ptx-code-signing-dev.make          |  6 ++++
 .../template-code-signing-provider-make       |  6 ++++
 scripts/lib/ptxd_lib_code_signing.sh          | 34 ++++++++++++++++---
 4 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
index fd16ca763557..e1ea5d981a89 100644
--- a/doc/ref_code_signing_helpers.rst
+++ b/doc/ref_code_signing_helpers.rst
@@ -29,6 +29,20 @@ Usage:
 
 Initialize SoftHSM, and set the initial pins.
 
+.. _cs_clean_softhsm:
+
+cs_clean_softhsm
+^^^^^^^^^^^^^^^^
+
+Usage:
+
+.. code-block:: bash
+
+    cs_clean_softhsm
+
+Clean up everything that was installed into the host sysroot.
+This function should be called by the provider during the ``clean`` stage.
+
 .. _cs_import_cert_from_der:
 
 cs_import_cert_from_der
@@ -125,6 +139,21 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
 authorities (CAs).
 HSM as well as SoftHSM code signing providers should use them.
 
+.. _cs_clean:
+
+cs_clean
+^^^^^^^^
+
+Usage:
+
+.. code-block:: bash
+
+    cs_clean
+
+Clean up everything that was installed into the host sysroot.
+This function should be called by the provider during the ``clean`` stage,
+For the SoftHSM workflow, call :ref:`cs_clean_softhsm` instead.
+
 .. _cs_define_role:
 
 cs_define_role
diff --git a/rules/host-ptx-code-signing-dev.make b/rules/host-ptx-code-signing-dev.make
index b242d65fc1be..d09049eaa71b 100644
--- a/rules/host-ptx-code-signing-dev.make
+++ b/rules/host-ptx-code-signing-dev.make
@@ -44,4 +44,10 @@ $(STATEDIR)/host-ptx-code-signing-dev.install:
 	@$(call targetinfo)
 	@$(call touch)
 
+$(STATEDIR)/host-ptx-code-signing-dev.clean:
+	@$(call targetinfo)
+	@$(call clean_pkg, HOST_PTX_CODE_SIGNING_DEV)
+	@$(HOST_PTX_CODE_SIGNING_DEV_MAKE_ENV) \
+		cs_clean_softhsm
+
 # vim: syntax=make
diff --git a/rules/templates/template-code-signing-provider-make b/rules/templates/template-code-signing-provider-make
index 4cf9cac358cf..a4bd4a1e74c5 100644
--- a/rules/templates/template-code-signing-provider-make
+++ b/rules/templates/template-code-signing-provider-make
@@ -39,4 +39,10 @@ $(STATEDIR)/host-@package@-code-signing.install:
 	@$(call targetinfo)
 	@$(call touch)
 
+$(STATEDIR)/host-@package@-code-signing.clean:
+	@$(call targetinfo)
+	@$(call clean_pkg, HOST_@PACKAGE@_CODE_SIGNING)
+	@$(HOST_@PACKAGE@_CODE_SIGNING_MAKE_ENV) \
+		cs_clean # FIXME: alternatively, call cs_clean_softhsm
+
 # vim: syntax=make
diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index f012f8e194c7..b0d54f47f832 100644
--- a/scripts/lib/ptxd_lib_code_signing.sh
+++ b/scripts/lib/ptxd_lib_code_signing.sh
@@ -86,6 +86,8 @@ cs_init_variables() {
     sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
     keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
     keydir="${sysroot}/var/lib/keys/${keyprovider}"
+
+    shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
 }
 export -f cs_init_variables
 
@@ -97,10 +99,7 @@ export -f cs_init_variables
 cs_init_softhsm() {
     cs_check_env_softhsm
     cs_init_variables
-    local shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
-
-    rm -rf "${shsm_keys}" &&
-    rm -rf "${keydir}" &&
+    cs_clean_softhsm &&
 
     sed -i "s^directories.tokendir =.*^directories.tokendir = ${shsm_keys}^" \
 	${SOFTHSM2_CONF} &&
@@ -112,6 +111,33 @@ cs_init_softhsm() {
 }
 export -f cs_init_softhsm
 
+#
+# cs_clean
+#
+# Clean up all files that were installed to the sysroot (generic variant)
+#
+cs_clean() {
+    cs_check_env &&
+    cs_init_variables &&
+    echo "Cleaning up ${keydir}" &&
+    rm -rf "${keydir}"
+}
+export -f cs_clean
+
+#
+# cs_clean
+#
+# Clean up all files that were installed to the sysroot (SoftHSM variant).
+#
+cs_clean_softhsm() {
+    cs_check_env_softhsm &&
+    cs_init_variables &&
+    cs_clean &&
+    echo "Cleaning up ${shsm_keys}" &&
+    rm -rf "${shsm_keys}"
+}
+export -f cs_clean_softhsm
+
 #
 # cs_define_role <role>
 #
-- 
2.30.2


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de


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

* [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init
  2021-08-10  9:59 [ptxdist] [PATCH v2 1/4] ptxd_lib_code_signing: take PKCS#11 PIN from the environment Roland Hieber
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 2/4] ptxd_lib_code_signing: refactor cs_check_env for SoftHSM workflow Roland Hieber
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files Roland Hieber
@ 2021-08-10 10:00 ` Roland Hieber
  2021-08-24 14:54   ` Bastian Krause
  2021-09-03 14:01   ` Michael Olbrich
  2 siblings, 2 replies; 8+ messages in thread
From: Roland Hieber @ 2021-08-10 10:00 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber, Bastian Krause

Similarly to cs_init_softhsm, introduce cs_init for non-SoftHSM use
cases. In both cases, clean up any left-over files from previous
installations to ensure a clean state, and enforce their use for
existing providers.

Reported-by: Bastian Krause <bst@pengutronix.de>
Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2: new in v2, split off from previous patch
 - enforce calling cs_init* at start of the provider (feedback from
   Bastian Krause)
 - slight fixes to the docs
---
 doc/ref_code_signing_helpers.rst              | 31 +++++++++++++++++++
 .../ptxdist-set-keys-hsm.sh                   |  1 +
 scripts/lib/ptxd_lib_code_signing.sh          | 28 +++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
index e1ea5d981a89..bb577c496c5b 100644
--- a/doc/ref_code_signing_helpers.rst
+++ b/doc/ref_code_signing_helpers.rst
@@ -28,6 +28,10 @@ Usage:
     cs_init_softhsm
 
 Initialize SoftHSM, and set the initial pins.
+This function must be called by the provider at the start of the ``compile`` stage.
+For non-SoftHSM workflows, call :ref:`cs_init` instead.
+
+This function also calls :ref:`cs_clean_softhsm`.
 
 .. _cs_clean_softhsm:
 
@@ -139,6 +143,23 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
 authorities (CAs).
 HSM as well as SoftHSM code signing providers should use them.
 
+.. _cs_init:
+
+cs_init
+^^^^^^^
+
+Usage:
+
+.. code-block:: bash
+
+    cs_init
+
+Initialize the provider.
+This function must be called by the provider at the start of the ``compile`` stage.
+For the SoftHSM workflow, call :ref:`cs_init_softhsm` instead.
+
+This function also calls :ref:`cs_clean`.
+
 .. _cs_clean:
 
 cs_clean
@@ -169,6 +190,10 @@ Define new key role.
 
 A default PKCS#11 URI is set implicitly as convenience for SoftHSM use cases.
 
+Preconditions:
+
+- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
+
 .. _cs_set_uri:
 
 cs_set_uri
@@ -259,6 +284,10 @@ Define a new role group.
 
 See :ref:`cs_group_add_roles` for an example.
 
+Preconditions:
+
+- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
+
 .. _cs_group_add_roles:
 
 cs_group_add_roles
@@ -281,6 +310,8 @@ Example:
 
 .. code-block:: bash
 
+   cs_init
+
    # define two roles named imx-habv4-srk1 and imx-habv4-srk2
    r="imx-habv4-srk1"
    cs_define_role "${r}"
diff --git a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
index b94eff049eac..b627541e30c1 100755
--- a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
+++ b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
@@ -43,6 +43,7 @@ set_imx_habv4_keys() {
 
 
 # HSM use case
+cs_init
 set_fit_keys
 set_rauc_keys
 set_imx_habv4_keys
diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index b0d54f47f832..a0c53f7f05fb 100644
--- a/scripts/lib/ptxd_lib_code_signing.sh
+++ b/scripts/lib/ptxd_lib_code_signing.sh
@@ -91,6 +91,26 @@ cs_init_variables() {
 }
 export -f cs_init_variables
 
+# internal check that tells us if cs_init was called
+cs_initialised=
+cs_check_initialised() {
+	if [ -z "${cs_initialised}" ]; then
+		echo ERROR_NOT_INITIALISED
+		ptxd_bailout "Not initialised – call cs_init or cs_init_softhsm first."
+	fi
+}
+
+#
+# cs_init
+#
+# Initialize the provider
+#
+cs_init() {
+    cs_clean
+    cs_initialised=1
+}
+export -f cs_init
+
 #
 # cs_init_softhsm
 #
@@ -108,6 +128,8 @@ cs_init_softhsm() {
 
     softhsm_pkcs11_tool_init --init-token --label "${keyprovider}" --so-pin 0000 &&
     softhsm_pkcs11_tool_init -l --so-pin 0000 --new-pin 1111 --init-pin
+
+    cs_initialised=1
 }
 export -f cs_init_softhsm
 
@@ -145,6 +167,7 @@ export -f cs_clean_softhsm
 #
 cs_define_role() {
     local role="${1}"
+    cs_check_initialised
     cs_init_variables
 
     mkdir -p "${keydir}/${role}" &&
@@ -160,6 +183,7 @@ export -f cs_define_role
 #
 cs_define_group() {
     local group="${1}"
+    cs_check_initialised
     cs_init_variables
 
     mkdir -p "${keydir}/${group}.group" &&
@@ -246,6 +270,7 @@ export -f cs_get_uri
 cs_import_cert_from_der() {
     local role="${1}"
     local der="${2}"
+    cs_check_initialised
     cs_init_variables
 
     softhsm_pkcs11_tool --type cert --write-object "${der}" --label "${role}"
@@ -261,6 +286,7 @@ export -f cs_import_cert_from_der
 cs_import_cert_from_pem() {
     local role="${1}"
     local pem="${2}"
+    cs_check_initialised
     cs_init_variables
 
     openssl x509 \
@@ -280,6 +306,7 @@ cs_import_pubkey_from_pem() {
     local -a openssl_keyopt
     local role="${1}"
     local pem="${2}"
+    cs_check_initialised
     cs_init_variables
 
     if [ -n "${OPENSSL_KEYPASS}" ]; then
@@ -304,6 +331,7 @@ cs_import_privkey_from_pem() {
     local -a openssl_keyopt
     local role="${1}"
     local pem="${2}"
+    cs_check_initialised
     cs_init_variables
 
     if [ -n "${OPENSSL_KEYPASS}" ]; then
-- 
2.30.2


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

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

* Re: [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files
  2021-08-10  9:59 ` [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files Roland Hieber
@ 2021-08-24 14:54   ` Bastian Krause
  2021-09-03 13:53     ` Michael Olbrich
  0 siblings, 1 reply; 8+ messages in thread
From: Bastian Krause @ 2021-08-24 14:54 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On 8/10/21 11:59 AM, Roland Hieber wrote:
> Currently, sysroot-host/var/lib/keys/${keyprovider} is left over even
> when the provider package is cleaned, which could lead to
> inconsistencies and leaked key material in the SoftHSM use case.
> Introduce cs_clean and cs_clean_softhsm shell functions to clean up
> those files. Call the cleanup functions in the clean stage of the
> providers.
> 
> Reported-by: Bastian Krause <bst@pengutronix.de>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>  - spell Bastian's last name correctly (sorry!) (feedback from Bastian
>    Krause)
>  - split off and extend cs_init stuff into next patch
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/20210809144030.22764-3-rhi@pengutronix.de
> ---
>  doc/ref_code_signing_helpers.rst              | 29 ++++++++++++++++
>  rules/host-ptx-code-signing-dev.make          |  6 ++++
>  .../template-code-signing-provider-make       |  6 ++++
>  scripts/lib/ptxd_lib_code_signing.sh          | 34 ++++++++++++++++---
>  4 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> index fd16ca763557..e1ea5d981a89 100644
> --- a/doc/ref_code_signing_helpers.rst
> +++ b/doc/ref_code_signing_helpers.rst
> @@ -29,6 +29,20 @@ Usage:
>  
>  Initialize SoftHSM, and set the initial pins.
>  
> +.. _cs_clean_softhsm:
> +
> +cs_clean_softhsm
> +^^^^^^^^^^^^^^^^
> +
> +Usage:
> +
> +.. code-block:: bash
> +
> +    cs_clean_softhsm
> +
> +Clean up everything that was installed into the host sysroot.
> +This function should be called by the provider during the ``clean`` stage.
> +
>  .. _cs_import_cert_from_der:
>  
>  cs_import_cert_from_der
> @@ -125,6 +139,21 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
>  authorities (CAs).
>  HSM as well as SoftHSM code signing providers should use them.
>  
> +.. _cs_clean:
> +
> +cs_clean
> +^^^^^^^^
> +
> +Usage:
> +
> +.. code-block:: bash
> +
> +    cs_clean
> +
> +Clean up everything that was installed into the host sysroot.
> +This function should be called by the provider during the ``clean`` stage,
> +For the SoftHSM workflow, call :ref:`cs_clean_softhsm` instead.
> +
>  .. _cs_define_role:
>  
>  cs_define_role
> diff --git a/rules/host-ptx-code-signing-dev.make b/rules/host-ptx-code-signing-dev.make
> index b242d65fc1be..d09049eaa71b 100644
> --- a/rules/host-ptx-code-signing-dev.make
> +++ b/rules/host-ptx-code-signing-dev.make
> @@ -44,4 +44,10 @@ $(STATEDIR)/host-ptx-code-signing-dev.install:
>  	@$(call targetinfo)
>  	@$(call touch)
>  
> +$(STATEDIR)/host-ptx-code-signing-dev.clean:
> +	@$(call targetinfo)
> +	@$(call clean_pkg, HOST_PTX_CODE_SIGNING_DEV)
> +	@$(HOST_PTX_CODE_SIGNING_DEV_MAKE_ENV) \
> +		cs_clean_softhsm
> +
>  # vim: syntax=make
> diff --git a/rules/templates/template-code-signing-provider-make b/rules/templates/template-code-signing-provider-make
> index 4cf9cac358cf..a4bd4a1e74c5 100644
> --- a/rules/templates/template-code-signing-provider-make
> +++ b/rules/templates/template-code-signing-provider-make
> @@ -39,4 +39,10 @@ $(STATEDIR)/host-@package@-code-signing.install:
>  	@$(call targetinfo)
>  	@$(call touch)
>  
> +$(STATEDIR)/host-@package@-code-signing.clean:
> +	@$(call targetinfo)
> +	@$(call clean_pkg, HOST_@PACKAGE@_CODE_SIGNING)
> +	@$(HOST_@PACKAGE@_CODE_SIGNING_MAKE_ENV) \
> +		cs_clean # FIXME: alternatively, call cs_clean_softhsm

We can distinguish the HSM from the SoftHSM case, see
scripts/lib/ptxd_lib_template.sh . We should be able to set the correct
function here.

> +
>  # vim: syntax=make
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index f012f8e194c7..b0d54f47f832 100644
> --- a/scripts/lib/ptxd_lib_code_signing.sh
> +++ b/scripts/lib/ptxd_lib_code_signing.sh
> @@ -86,6 +86,8 @@ cs_init_variables() {
>      sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
>      keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
>      keydir="${sysroot}/var/lib/keys/${keyprovider}"
> +
> +    shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
>  }
>  export -f cs_init_variables
>  
> @@ -97,10 +99,7 @@ export -f cs_init_variables
>  cs_init_softhsm() {
>      cs_check_env_softhsm
>      cs_init_variables
> -    local shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
> -
> -    rm -rf "${shsm_keys}" &&
> -    rm -rf "${keydir}" &&
> +    cs_clean_softhsm &&
>  
>      sed -i "s^directories.tokendir =.*^directories.tokendir = ${shsm_keys}^" \
>  	${SOFTHSM2_CONF} &&
> @@ -112,6 +111,33 @@ cs_init_softhsm() {
>  }
>  export -f cs_init_softhsm
>  
> +#
> +# cs_clean
> +#
> +# Clean up all files that were installed to the sysroot (generic variant)
> +#
> +cs_clean() {
> +    cs_check_env &&
> +    cs_init_variables &&
> +    echo "Cleaning up ${keydir}" &&
> +    rm -rf "${keydir}"
> +}
> +export -f cs_clean
> +
> +#
> +# cs_clean

Shouldn't this be "cs_clean_softhsm"?

Regards,
Bastian

> +#
> +# Clean up all files that were installed to the sysroot (SoftHSM variant).
> +#
> +cs_clean_softhsm() {
> +    cs_check_env_softhsm &&
> +    cs_init_variables &&
> +    cs_clean &&
> +    echo "Cleaning up ${shsm_keys}" &&
> +    rm -rf "${shsm_keys}"
> +}
> +export -f cs_clean_softhsm
> +
>  #
>  # cs_define_role <role>
>  #
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de


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

* Re: [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init
  2021-08-10 10:00 ` [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init Roland Hieber
@ 2021-08-24 14:54   ` Bastian Krause
  2021-09-03 14:01   ` Michael Olbrich
  1 sibling, 0 replies; 8+ messages in thread
From: Bastian Krause @ 2021-08-24 14:54 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On 8/10/21 12:00 PM, Roland Hieber wrote:
> Similarly to cs_init_softhsm, introduce cs_init for non-SoftHSM use
> cases. In both cases, clean up any left-over files from previous
> installations to ensure a clean state, and enforce their use for
> existing providers.
> 
> Reported-by: Bastian Krause <bst@pengutronix.de>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2: new in v2, split off from previous patch
>  - enforce calling cs_init* at start of the provider (feedback from
>    Bastian Krause)
>  - slight fixes to the docs
> ---
>  doc/ref_code_signing_helpers.rst              | 31 +++++++++++++++++++
>  .../ptxdist-set-keys-hsm.sh                   |  1 +
>  scripts/lib/ptxd_lib_code_signing.sh          | 28 +++++++++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> index e1ea5d981a89..bb577c496c5b 100644
> --- a/doc/ref_code_signing_helpers.rst
> +++ b/doc/ref_code_signing_helpers.rst
> @@ -28,6 +28,10 @@ Usage:
>      cs_init_softhsm
>  
>  Initialize SoftHSM, and set the initial pins.
> +This function must be called by the provider at the start of the ``compile`` stage.
> +For non-SoftHSM workflows, call :ref:`cs_init` instead.
> +
> +This function also calls :ref:`cs_clean_softhsm`.
>  
>  .. _cs_clean_softhsm:
>  
> @@ -139,6 +143,23 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
>  authorities (CAs).
>  HSM as well as SoftHSM code signing providers should use them.
>  
> +.. _cs_init:
> +
> +cs_init
> +^^^^^^^
> +
> +Usage:
> +
> +.. code-block:: bash
> +
> +    cs_init
> +
> +Initialize the provider.
> +This function must be called by the provider at the start of the ``compile`` stage.
> +For the SoftHSM workflow, call :ref:`cs_init_softhsm` instead.
> +
> +This function also calls :ref:`cs_clean`.
> +
>  .. _cs_clean:
>  
>  cs_clean
> @@ -169,6 +190,10 @@ Define new key role.
>  
>  A default PKCS#11 URI is set implicitly as convenience for SoftHSM use cases.
>  
> +Preconditions:
> +
> +- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
> +
>  .. _cs_set_uri:
>  
>  cs_set_uri
> @@ -259,6 +284,10 @@ Define a new role group.
>  
>  See :ref:`cs_group_add_roles` for an example.
>  
> +Preconditions:
> +
> +- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
> +
>  .. _cs_group_add_roles:
>  
>  cs_group_add_roles
> @@ -281,6 +310,8 @@ Example:
>  
>  .. code-block:: bash
>  
> +   cs_init
> +
>     # define two roles named imx-habv4-srk1 and imx-habv4-srk2
>     r="imx-habv4-srk1"
>     cs_define_role "${r}"
> diff --git a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> index b94eff049eac..b627541e30c1 100755
> --- a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> +++ b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> @@ -43,6 +43,7 @@ set_imx_habv4_keys() {
>  
>  
>  # HSM use case
> +cs_init
>  set_fit_keys
>  set_rauc_keys
>  set_imx_habv4_keys
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index b0d54f47f832..a0c53f7f05fb 100644
> --- a/scripts/lib/ptxd_lib_code_signing.sh
> +++ b/scripts/lib/ptxd_lib_code_signing.sh
> @@ -91,6 +91,26 @@ cs_init_variables() {
>  }
>  export -f cs_init_variables
>  
> +# internal check that tells us if cs_init was called
> +cs_initialised=
> +cs_check_initialised() {
> +	if [ -z "${cs_initialised}" ]; then
> +		echo ERROR_NOT_INITIALISED
> +		ptxd_bailout "Not initialised – call cs_init or cs_init_softhsm first."
> +	fi
> +}
> +
> +#
> +# cs_init
> +#
> +# Initialize the provider
> +#
> +cs_init() {
> +    cs_clean
> +    cs_initialised=1

Should we reset this in cs_clean?

> +}
> +export -f cs_init
> +
>  #
>  # cs_init_softhsm
>  #
> @@ -108,6 +128,8 @@ cs_init_softhsm() {
>  
>      softhsm_pkcs11_tool_init --init-token --label "${keyprovider}" --so-pin 0000 &&
>      softhsm_pkcs11_tool_init -l --so-pin 0000 --new-pin 1111 --init-pin
> +
> +    cs_initialised=1
>  }
>  export -f cs_init_softhsm
>  
> @@ -145,6 +167,7 @@ export -f cs_clean_softhsm
>  #
>  cs_define_role() {
>      local role="${1}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      mkdir -p "${keydir}/${role}" &&
> @@ -160,6 +183,7 @@ export -f cs_define_role
>  #
>  cs_define_group() {
>      local group="${1}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      mkdir -p "${keydir}/${group}.group" &&
> @@ -246,6 +270,7 @@ export -f cs_get_uri
>  cs_import_cert_from_der() {
>      local role="${1}"
>      local der="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      softhsm_pkcs11_tool --type cert --write-object "${der}" --label "${role}"
> @@ -261,6 +286,7 @@ export -f cs_import_cert_from_der
>  cs_import_cert_from_pem() {
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      openssl x509 \
> @@ -280,6 +306,7 @@ cs_import_pubkey_from_pem() {
>      local -a openssl_keyopt
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      if [ -n "${OPENSSL_KEYPASS}" ]; then
> @@ -304,6 +331,7 @@ cs_import_privkey_from_pem() {
>      local -a openssl_keyopt
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      if [ -n "${OPENSSL_KEYPASS}" ]; then
> 

You introduce a mix-up of initialised/initialized here. Better stick to
one variant.

Regards,
Bastian

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

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

* Re: [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files
  2021-08-24 14:54   ` Bastian Krause
@ 2021-09-03 13:53     ` Michael Olbrich
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Olbrich @ 2021-09-03 13:53 UTC (permalink / raw)
  To: Bastian Krause; +Cc: ptxdist, Roland Hieber

On Tue, Aug 24, 2021 at 04:54:57PM +0200, Bastian Krause wrote:
> On 8/10/21 11:59 AM, Roland Hieber wrote:
> > Currently, sysroot-host/var/lib/keys/${keyprovider} is left over even
> > when the provider package is cleaned, which could lead to
> > inconsistencies and leaked key material in the SoftHSM use case.
> > Introduce cs_clean and cs_clean_softhsm shell functions to clean up
> > those files. Call the cleanup functions in the clean stage of the
> > providers.
> > 
> > Reported-by: Bastian Krause <bst@pengutronix.de>
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > PATCH v2:
> >  - spell Bastian's last name correctly (sorry!) (feedback from Bastian
> >    Krause)
> >  - split off and extend cs_init stuff into next patch
> > 
> > PATCH v1: https://lore.ptxdist.org/ptxdist/20210809144030.22764-3-rhi@pengutronix.de
> > ---
> >  doc/ref_code_signing_helpers.rst              | 29 ++++++++++++++++
> >  rules/host-ptx-code-signing-dev.make          |  6 ++++
> >  .../template-code-signing-provider-make       |  6 ++++
> >  scripts/lib/ptxd_lib_code_signing.sh          | 34 ++++++++++++++++---
> >  4 files changed, 71 insertions(+), 4 deletions(-)
> > 
> > diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> > index fd16ca763557..e1ea5d981a89 100644
> > --- a/doc/ref_code_signing_helpers.rst
> > +++ b/doc/ref_code_signing_helpers.rst
> > @@ -29,6 +29,20 @@ Usage:
> >  
> >  Initialize SoftHSM, and set the initial pins.
> >  
> > +.. _cs_clean_softhsm:
> > +
> > +cs_clean_softhsm
> > +^^^^^^^^^^^^^^^^
> > +
> > +Usage:
> > +
> > +.. code-block:: bash
> > +
> > +    cs_clean_softhsm
> > +
> > +Clean up everything that was installed into the host sysroot.
> > +This function should be called by the provider during the ``clean`` stage.
> > +
> >  .. _cs_import_cert_from_der:
> >  
> >  cs_import_cert_from_der
> > @@ -125,6 +139,21 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
> >  authorities (CAs).
> >  HSM as well as SoftHSM code signing providers should use them.
> >  
> > +.. _cs_clean:
> > +
> > +cs_clean
> > +^^^^^^^^
> > +
> > +Usage:
> > +
> > +.. code-block:: bash
> > +
> > +    cs_clean
> > +
> > +Clean up everything that was installed into the host sysroot.
> > +This function should be called by the provider during the ``clean`` stage,
> > +For the SoftHSM workflow, call :ref:`cs_clean_softhsm` instead.
> > +
> >  .. _cs_define_role:
> >  
> >  cs_define_role
> > diff --git a/rules/host-ptx-code-signing-dev.make b/rules/host-ptx-code-signing-dev.make
> > index b242d65fc1be..d09049eaa71b 100644
> > --- a/rules/host-ptx-code-signing-dev.make
> > +++ b/rules/host-ptx-code-signing-dev.make
> > @@ -44,4 +44,10 @@ $(STATEDIR)/host-ptx-code-signing-dev.install:
> >  	@$(call targetinfo)
> >  	@$(call touch)
> >  
> > +$(STATEDIR)/host-ptx-code-signing-dev.clean:
> > +	@$(call targetinfo)
> > +	@$(call clean_pkg, HOST_PTX_CODE_SIGNING_DEV)
> > +	@$(HOST_PTX_CODE_SIGNING_DEV_MAKE_ENV) \
> > +		cs_clean_softhsm
> > +
> >  # vim: syntax=make
> > diff --git a/rules/templates/template-code-signing-provider-make b/rules/templates/template-code-signing-provider-make
> > index 4cf9cac358cf..a4bd4a1e74c5 100644
> > --- a/rules/templates/template-code-signing-provider-make
> > +++ b/rules/templates/template-code-signing-provider-make
> > @@ -39,4 +39,10 @@ $(STATEDIR)/host-@package@-code-signing.install:
> >  	@$(call targetinfo)
> >  	@$(call touch)
> >  
> > +$(STATEDIR)/host-@package@-code-signing.clean:
> > +	@$(call targetinfo)
> > +	@$(call clean_pkg, HOST_@PACKAGE@_CODE_SIGNING)
> > +	@$(HOST_@PACKAGE@_CODE_SIGNING_MAKE_ENV) \
> > +		cs_clean # FIXME: alternatively, call cs_clean_softhsm
> 
> We can distinguish the HSM from the SoftHSM case, see
> scripts/lib/ptxd_lib_template.sh . We should be able to set the correct
> function here.
> 
> > +
> >  # vim: syntax=make
> > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > index f012f8e194c7..b0d54f47f832 100644
> > --- a/scripts/lib/ptxd_lib_code_signing.sh
> > +++ b/scripts/lib/ptxd_lib_code_signing.sh
> > @@ -86,6 +86,8 @@ cs_init_variables() {
> >      sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
> >      keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
> >      keydir="${sysroot}/var/lib/keys/${keyprovider}"
> > +
> > +    shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"

This path is unique to each provider...

> >  }
> >  export -f cs_init_variables
> >  
> > @@ -97,10 +99,7 @@ export -f cs_init_variables
> >  cs_init_softhsm() {
> >      cs_check_env_softhsm
> >      cs_init_variables
> > -    local shsm_keys="${sysroot}/var/cache/softhsm/${keyprovider}"
> > -
> > -    rm -rf "${shsm_keys}" &&
> > -    rm -rf "${keydir}" &&
> > +    cs_clean_softhsm &&
> >  
> >      sed -i "s^directories.tokendir =.*^directories.tokendir = ${shsm_keys}^" \
> >  	${SOFTHSM2_CONF} &&
> > @@ -112,6 +111,33 @@ cs_init_softhsm() {
> >  }
> >  export -f cs_init_softhsm
> >  
> > +#
> > +# cs_clean
> > +#
> > +# Clean up all files that were installed to the sysroot (generic variant)
> > +#
> > +cs_clean() {
> > +    cs_check_env &&
> > +    cs_init_variables &&
> > +    echo "Cleaning up ${keydir}" &&
> > +    rm -rf "${keydir}"

... so we can just check here if ${shsm_keys} exists and remove it.

No need for a separate function.

Or maybe we can create all this stuff in pkgdir and let install.post copy
it to sysroot. Then the regular package cleanup handling will remove it.
But I'm not sure how to handle the softshm storage in this case.

Michael

> > +}
> > +export -f cs_clean
> > +
> > +#
> > +# cs_clean
> 
> Shouldn't this be "cs_clean_softhsm"?
> 
> Regards,
> Bastian
> 
> > +#
> > +# Clean up all files that were installed to the sysroot (SoftHSM variant).
> > +#
> > +cs_clean_softhsm() {
> > +    cs_check_env_softhsm &&
> > +    cs_init_variables &&
> > +    cs_clean &&
> > +    echo "Cleaning up ${shsm_keys}" &&
> > +    rm -rf "${shsm_keys}"
> > +}
> > +export -f cs_clean_softhsm
> > +
> >  #
> >  # cs_define_role <role>
> >  #
> > 
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de


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

* Re: [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init
  2021-08-10 10:00 ` [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init Roland Hieber
  2021-08-24 14:54   ` Bastian Krause
@ 2021-09-03 14:01   ` Michael Olbrich
  1 sibling, 0 replies; 8+ messages in thread
From: Michael Olbrich @ 2021-09-03 14:01 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On Tue, Aug 10, 2021 at 12:00:00PM +0200, Roland Hieber wrote:
> Similarly to cs_init_softhsm, introduce cs_init for non-SoftHSM use
> cases. In both cases, clean up any left-over files from previous
> installations to ensure a clean state, and enforce their use for
> existing providers.
> 
> Reported-by: Bastian Krause <bst@pengutronix.de>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2: new in v2, split off from previous patch
>  - enforce calling cs_init* at start of the provider (feedback from
>    Bastian Krause)
>  - slight fixes to the docs
> ---
>  doc/ref_code_signing_helpers.rst              | 31 +++++++++++++++++++
>  .../ptxdist-set-keys-hsm.sh                   |  1 +
>  scripts/lib/ptxd_lib_code_signing.sh          | 28 +++++++++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> index e1ea5d981a89..bb577c496c5b 100644
> --- a/doc/ref_code_signing_helpers.rst
> +++ b/doc/ref_code_signing_helpers.rst
> @@ -28,6 +28,10 @@ Usage:
>      cs_init_softhsm
>  
>  Initialize SoftHSM, and set the initial pins.
> +This function must be called by the provider at the start of the ``compile`` stage.
> +For non-SoftHSM workflows, call :ref:`cs_init` instead.
> +
> +This function also calls :ref:`cs_clean_softhsm`.
>  
>  .. _cs_clean_softhsm:
>  
> @@ -139,6 +143,23 @@ These helpers allow to define roles, set PKCS#11 URIs and handle certificate
>  authorities (CAs).
>  HSM as well as SoftHSM code signing providers should use them.
>  
> +.. _cs_init:
> +
> +cs_init
> +^^^^^^^
> +
> +Usage:
> +
> +.. code-block:: bash
> +
> +    cs_init
> +
> +Initialize the provider.
> +This function must be called by the provider at the start of the ``compile`` stage.
> +For the SoftHSM workflow, call :ref:`cs_init_softhsm` instead.
> +
> +This function also calls :ref:`cs_clean`.
> +
>  .. _cs_clean:
>  
>  cs_clean
> @@ -169,6 +190,10 @@ Define new key role.
>  
>  A default PKCS#11 URI is set implicitly as convenience for SoftHSM use cases.
>  
> +Preconditions:
> +
> +- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
> +
>  .. _cs_set_uri:
>  
>  cs_set_uri
> @@ -259,6 +284,10 @@ Define a new role group.
>  
>  See :ref:`cs_group_add_roles` for an example.
>  
> +Preconditions:
> +
> +- the provider must have been initialised (see :ref:`cs_init` or :ref:`cs_init_softhsm`)
> +
>  .. _cs_group_add_roles:
>  
>  cs_group_add_roles
> @@ -281,6 +310,8 @@ Example:
>  
>  .. code-block:: bash
>  
> +   cs_init
> +
>     # define two roles named imx-habv4-srk1 and imx-habv4-srk2
>     r="imx-habv4-srk1"
>     cs_define_role "${r}"
> diff --git a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> index b94eff049eac..b627541e30c1 100755
> --- a/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> +++ b/rules/templates/code-signing-provider/ptxdist-set-keys-hsm.sh
> @@ -43,6 +43,7 @@ set_imx_habv4_keys() {
>  
>  
>  # HSM use case
> +cs_init
>  set_fit_keys
>  set_rauc_keys
>  set_imx_habv4_keys
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index b0d54f47f832..a0c53f7f05fb 100644
> --- a/scripts/lib/ptxd_lib_code_signing.sh
> +++ b/scripts/lib/ptxd_lib_code_signing.sh
> @@ -91,6 +91,26 @@ cs_init_variables() {
>  }
>  export -f cs_init_variables
>  
> +# internal check that tells us if cs_init was called
> +cs_initialised=
> +cs_check_initialised() {
> +	if [ -z "${cs_initialised}" ]; then

Hmm, this assumes that, during initialisation, all cs_* functions are
called within the same shell. Can we really assume this?

Maybe put ${PTXDIST_TEMPDIR} or something like that into ${keydir}/.stamp
and compare it. This path remains the same during on PTXdist call but will
be different between two calls.

Michael

> +		echo ERROR_NOT_INITIALISED
> +		ptxd_bailout "Not initialised – call cs_init or cs_init_softhsm first."
> +	fi
> +}
> +
> +#
> +# cs_init
> +#
> +# Initialize the provider
> +#
> +cs_init() {
> +    cs_clean
> +    cs_initialised=1
> +}
> +export -f cs_init
> +
>  #
>  # cs_init_softhsm
>  #
> @@ -108,6 +128,8 @@ cs_init_softhsm() {
>  
>      softhsm_pkcs11_tool_init --init-token --label "${keyprovider}" --so-pin 0000 &&
>      softhsm_pkcs11_tool_init -l --so-pin 0000 --new-pin 1111 --init-pin
> +
> +    cs_initialised=1
>  }
>  export -f cs_init_softhsm
>  
> @@ -145,6 +167,7 @@ export -f cs_clean_softhsm
>  #
>  cs_define_role() {
>      local role="${1}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      mkdir -p "${keydir}/${role}" &&
> @@ -160,6 +183,7 @@ export -f cs_define_role
>  #
>  cs_define_group() {
>      local group="${1}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      mkdir -p "${keydir}/${group}.group" &&
> @@ -246,6 +270,7 @@ export -f cs_get_uri
>  cs_import_cert_from_der() {
>      local role="${1}"
>      local der="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      softhsm_pkcs11_tool --type cert --write-object "${der}" --label "${role}"
> @@ -261,6 +286,7 @@ export -f cs_import_cert_from_der
>  cs_import_cert_from_pem() {
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      openssl x509 \
> @@ -280,6 +306,7 @@ cs_import_pubkey_from_pem() {
>      local -a openssl_keyopt
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      if [ -n "${OPENSSL_KEYPASS}" ]; then
> @@ -304,6 +331,7 @@ cs_import_privkey_from_pem() {
>      local -a openssl_keyopt
>      local role="${1}"
>      local pem="${2}"
> +    cs_check_initialised
>      cs_init_variables
>  
>      if [ -n "${OPENSSL_KEYPASS}" ]; then
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

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

end of thread, other threads:[~2021-09-03 14:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10  9:59 [ptxdist] [PATCH v2 1/4] ptxd_lib_code_signing: take PKCS#11 PIN from the environment Roland Hieber
2021-08-10  9:59 ` [ptxdist] [PATCH v2 2/4] ptxd_lib_code_signing: refactor cs_check_env for SoftHSM workflow Roland Hieber
2021-08-10  9:59 ` [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files Roland Hieber
2021-08-24 14:54   ` Bastian Krause
2021-09-03 13:53     ` Michael Olbrich
2021-08-10 10:00 ` [ptxdist] [PATCH v2 4/4] ptxd_lib_code_signing: enforce cleaning up on init Roland Hieber
2021-08-24 14:54   ` Bastian Krause
2021-09-03 14:01   ` Michael Olbrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox