mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
* [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts
@ 2021-08-09  8:06 Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 2/5] libptxdist: introduce ptxd_exec_silent_stderr Roland Hieber
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  8:06 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

Variables named ${pkg} are already widely used throughout the code base
for different purposes, so name it ${pkg_name} instead.

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
Turned out I didn't need it for this series, but I still think it's
useful in general.

PATCH v2: no changes

PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-1-rhi@pengutronix.de
---
 rules/post/ptxd_make_world_common.make | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rules/post/ptxd_make_world_common.make b/rules/post/ptxd_make_world_common.make
index e5cf50214e9c..c9d22c6884a3 100644
--- a/rules/post/ptxd_make_world_common.make
+++ b/rules/post/ptxd_make_world_common.make
@@ -69,6 +69,7 @@ world/env/impl = \
 	pkg_makefile="$(call ptx/escape,$($(1)_MAKEFILE))"			\
 	pkg_infile="$(call ptx/escape,$($(1)_INFILE))"				\
 										\
+	pkg_name="$(call ptx/escape,$(PTX_MAP_TO_package_$(1)))"                \
 	pkg_pkg="$(call ptx/escape,$($(1)))"					\
 	pkg_version="$(call ptx/escape,$($(1)_VERSION))"			\
 	pkg_config="$(call ptx/escape,$($(1)_CONFIG))"				\
-- 
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] 19+ messages in thread

* [ptxdist] [PATCH v2 2/5] libptxdist: introduce ptxd_exec_silent_stderr
  2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
@ 2021-08-09  8:06 ` Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs Roland Hieber
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  8:06 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

Some programs  print stuff to stderr that are not errors and are
therefore not relevant to the usual build runs (e.g. openssl when
loading the PKCS#11 libraries), but they may still be useful for
debugging. When called with ptxd_exec_silent_stderr, stderr won't make
it to the terminal except with 'ptxdist -v', but the messages are still
available in the logfile.

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

PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-2-rhi@pengutronix.de
---
 scripts/libptxdist.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/libptxdist.sh b/scripts/libptxdist.sh
index ee0ba39d3ea3..bb508798cb6f 100644
--- a/scripts/libptxdist.sh
+++ b/scripts/libptxdist.sh
@@ -776,6 +776,24 @@ ptxd_exec() {
 }
 export -f ptxd_exec
 
+#
+# execute command with silenced stderr, except when verbose building is enabled.
+# the stderr output of the command will always be written to the logfile.
+#
+ptxd_exec_silent_stderr() {
+	exec 8>&2
+	if [ "${PTXDIST_VERBOSE}" == "1" ]; then
+		:
+	elif [ -n "${PTXDIST_FD_LOGFILE}" ]; then
+		exec 2>&9
+	else
+		exec 2>/dev/null
+	fi
+	"${@}"
+	exec 2>&8 8>&-
+}
+export -f ptxd_exec_silent_stderr
+
 #
 # check if a previously executed pipe returned an error
 #
-- 
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] 19+ messages in thread

* [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs
  2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 2/5] libptxdist: introduce ptxd_exec_silent_stderr Roland Hieber
@ 2021-08-09  8:06 ` Roland Hieber
  2021-09-03 12:46   ` Michael Olbrich
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment Roland Hieber
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  8:06 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

We'll need this type of function more often later.

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

PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-3-rhi@pengutronix.de
---
 scripts/lib/ptxd_lib_code_signing.sh | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index 5ba1a4666af4..66a2cab81395 100644
--- a/scripts/lib/ptxd_lib_code_signing.sh
+++ b/scripts/lib/ptxd_lib_code_signing.sh
@@ -49,6 +49,17 @@ softhsm_pkcs11_tool() {
 }
 export -f softhsm_pkcs11_tool
 
+#
+# softhsm_pkcs11_uri <uri>
+#
+# Add the SoftHSM PIN to the given URI.
+#
+softhsm_pkcs11_uri() {
+    local role="$1"
+    printf "pkcs11:token=%s;object=%s;pin-value=1111\n" "${keyprovider}" "${role}"
+}
+export -f softhsm_pkcs11_uri
+
 #
 # cs_init_variables
 #
@@ -95,7 +106,8 @@ cs_define_role() {
 
     mkdir -p "${keydir}/${role}" &&
     # default for SoftHSM
-    cs_set_uri "${role}" "pkcs11:token=${keyprovider};object=${role};pin-value=1111"
+    local uri=$(softhsm_pkcs11_uri "${role}")
+    cs_set_uri "${role}" "${uri}"
 }
 export -f cs_define_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] 19+ messages in thread

* [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment
  2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 2/5] libptxdist: introduce ptxd_exec_silent_stderr Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs Roland Hieber
@ 2021-08-09  8:06 ` Roland Hieber
  2021-09-03 12:54   ` Michael Olbrich
  2021-09-08 20:53   ` Roland Hieber
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks Roland Hieber
  2021-09-03 12:48 ` [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Michael Olbrich
  4 siblings, 2 replies; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  8:06 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

The code signing consumer functions should be able to retrieve some
information about the recipe in which they were called in order to make
additional checks if needed. Refactor the (shell cs_get_*, …) calls into
macro calls of the form $(call ptx/cs-get-*, <PKG>, …). Let these
macros look up the package name (for now) from PTX_MAP_TO_package_<PKG>
before passing it to the shell functions. Using $(call world/env) here
would be practical, but would also cause make to complain about
recursive variable dependencies. Therefore variables must be added
to ptx/cs-consumer-env manually, but additional information can be added
later if needed.

Refactor the existing consumers in the code base too, and add an error
message in case anyone else that still uses the old API.

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 - define multiline macros using "define"

PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-4-rhi@pengutronix.de
---
 doc/dev_code_signing.rst                      |  2 +-
 doc/ref_code_signing_helpers.rst              | 25 ++++++-----
 rules/barebox.make                            |  2 +-
 rules/image-rauc.make                         |  6 +--
 rules/kernel.make                             |  6 +--
 rules/pre/030-code-signing-consumers.make     | 44 +++++++++++++++++++
 rules/rauc.make                               |  2 +-
 .../templates/template-barebox-imx-habv4-make |  6 +--
 scripts/lib/ptxd_lib_code_signing.sh          | 13 ++++++
 9 files changed, 83 insertions(+), 23 deletions(-)
 create mode 100644 rules/pre/030-code-signing-consumers.make

diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
index b9a7c42f2a55..413f694980eb 100644
--- a/doc/dev_code_signing.rst
+++ b/doc/dev_code_signing.rst
@@ -164,7 +164,7 @@ also via an environment variable.
 .. code-block:: none
 
     $(call install_copy, rauc, 0, 0, 0644, \
-      $(shell cs_get_ca update), \
+      $(call ptx/cs-get-ca, RAUC, update), \
       /etc/rauc/ca.cert.pem)
 
 .. note:: When code signing helper functions are used in make variables (e.g.
diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
index fd16ca763557..d3429778d94d 100644
--- a/doc/ref_code_signing_helpers.rst
+++ b/doc/ref_code_signing_helpers.rst
@@ -297,19 +297,21 @@ In the example given in :ref:`cs_group_add_roles` above, this would print::
 Consumer Functions
 ~~~~~~~~~~~~~~~~~~
 
+The consumer functions are implemented as make macros.
 Packages that want to sign something or need access to keys/CAs can retrieve
 PKCS#11 URIs and CA keyrings with these helpers.
 
+.. _ptx/cs-get-uri:
 .. _cs_get_uri:
 
-cs_get_uri
-^^^^^^^^^^
+ptx/cs-get-uri
+^^^^^^^^^^^^^^
 
 Usage:
 
-.. code-block:: bash
+.. code-block:: make
 
-    cs_get_uri <role>
+    $(call ptx/cs-get-uri, <PKG>, <role>)
 
 Get PKCS#11 URI for role.
 
@@ -317,16 +319,17 @@ Preconditions:
 
 - the URI must have been set (see :ref:`cs_set_uri`)
 
+.. _ptx/cs-get-ca:
 .. _cs_get_ca:
 
-cs_get_ca
-^^^^^^^^^
+ptx/cs-get-ca
+^^^^^^^^^^^^^
 
 Usage:
 
-.. code-block:: bash
+.. code-block:: make
 
-    cs_get_ca <role>
+    $(call ptx/cs-get-ca, <PKG>, <role>)
 
 Get path to the CA keyring in PEM format for role.
 
@@ -347,7 +350,7 @@ Example:
 
    # set up kernel module signing, and add a trusted CA if the provider set one
    KERNEL_SIGN_OPT =
-   	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"' \
+   	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"' \
    	CONFIG_MODULE_SIG_ALL=y \
-   	$(if $(shell cs_get_ca kernel-trusted), \
-   		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
+   	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
+   		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
diff --git a/rules/barebox.make b/rules/barebox.make
index bea9f3adcbf8..983d34032e0d 100644
--- a/rules/barebox.make
+++ b/rules/barebox.make
@@ -103,7 +103,7 @@ endif
 ifdef PTXCONF_CODE_SIGNING
 BAREBOX_MAKE_ENV = \
 	$(CODE_SIGNING_ENV) \
-	IMAGE_KERNEL_FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
+	IMAGE_KERNEL_FIT_KEY="$(call ptx/cs-get-uri, BAREBOX, image-kernel-fit)"
 endif
 
 $(STATEDIR)/barebox.compile:
diff --git a/rules/image-rauc.make b/rules/image-rauc.make
index fe1b0e89be7c..c8747231f8f1 100644
--- a/rules/image-rauc.make
+++ b/rules/image-rauc.make
@@ -32,9 +32,9 @@ IMAGE_RAUC_ENV	= \
 	RAUC_BUNDLE_VERSION="$(call remove_quotes, $(PTXCONF_RAUC_BUNDLE_VERSION))" \
 	RAUC_BUNDLE_BUILD=$(call ptx/sh, date +%FT%T%z) \
 	RAUC_BUNDLE_DESCRIPTION=$(PTXCONF_IMAGE_RAUC_DESCRIPTION) \
-	RAUC_KEY="$(shell cs_get_uri update)" \
-	RAUC_CERT="$(shell cs_get_uri update)" \
-	RAUC_KEYRING="$(shell cs_get_ca update)"
+	RAUC_KEY="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
+	RAUC_CERT="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
+	RAUC_KEYRING="$(call ptx/cs-get-ca, IMAGE_RAUC, update)"
 
 $(IMAGE_RAUC_IMAGE):
 	@$(call targetinfo)
diff --git a/rules/kernel.make b/rules/kernel.make
index 9caff677918e..e6faba82df38 100644
--- a/rules/kernel.make
+++ b/rules/kernel.make
@@ -73,12 +73,12 @@ KERNEL_BASE_OPT		= \
 
 ifdef PTXCONF_KERNEL_CODE_SIGNING
 KERNEL_BASE_OPT		+= \
-	$(if $(shell cs_get_ca kernel-trusted), \
-		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
+	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
+		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
 endif
 ifdef PTXCONF_KERNEL_MODULES_SIGN
 KERNEL_BASE_OPT		+= \
-	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"'
+	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"'
 endif
 
 # Intermediate option. This will be used by kernel module packages.
diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
new file mode 100644
index 000000000000..909e8ebd6936
--- /dev/null
+++ b/rules/pre/030-code-signing-consumers.make
@@ -0,0 +1,44 @@
+# -*-makefile-*-
+#
+# Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
+#
+# For further information about the PTXdist project and license conditions
+# see the README file.
+#
+#
+
+#
+# Usage: $(call ptx/cs-consumer-env, <PKG>)
+#
+# We usually want to use cs-get-* macros inside a <PKG>_MAKE_OPT etc., which is
+# referenced in world/env, so we cannot use world/env to set pkg_name without
+# running into circular variable dependencies.
+#
+define ptx/cs-consumer-env
+	pkg_name='$(PTX_MAP_TO_package_$(strip $(1)))' \
+	$(CODE_SIGNING_ENV)
+endef
+
+#
+# Usage: $(call ptx/cs-get-uri, <PKG>, <role>)
+#
+define ptx/cs-get-uri
+$(strip \
+	$(shell \
+		$(call ptx/cs-consumer-env, $(1))\
+			cs_get_uri '$(strip $(2))'\
+	)\
+)
+endef
+
+#
+# Usage: $(call ptx/cs-get-ca, <PKG>, <role>)
+#
+define ptx/cs-get-ca
+$(strip \
+	$(shell \
+		$(call ptx/cs-consumer-env, $(1))\
+			cs_get_ca '$(strip $(2))'\
+	)\
+)
+endef
diff --git a/rules/rauc.make b/rules/rauc.make
index 08df6336a7cd..3c28befcd3ff 100644
--- a/rules/rauc.make
+++ b/rules/rauc.make
@@ -78,7 +78,7 @@ ifdef PTXCONF_RAUC_CONFIGURATION
 	@$(call install_replace, rauc, /etc/rauc/system.conf, \
 		@RAUC_BUNDLE_COMPATIBLE@, \
 		"$(call remove_quotes,$(PTXCONF_RAUC_COMPATIBLE))")
-	@$(call install_copy, rauc, 0, 0, 0644, $(shell cs_get_ca update), \
+	@$(call install_copy, rauc, 0, 0, 0644, $(call ptx/cs-get-ca, RAUC, update), \
 		/etc/rauc/ca.cert.pem)
 endif
 
diff --git a/rules/templates/template-barebox-imx-habv4-make b/rules/templates/template-barebox-imx-habv4-make
index cc825dc90292..b2d5d7100fc9 100644
--- a/rules/templates/template-barebox-imx-habv4-make
+++ b/rules/templates/template-barebox-imx-habv4-make
@@ -64,9 +64,9 @@ endif
 
 BAREBOX_@PACKAGE@_MAKE_ENV	= \
 	$(CODE_SIGNING_ENV) \
-	CSF="$(shell cs_get_uri imx-habv4-csf1)" \
-	IMG="$(shell cs_get_uri imx-habv4-img1)" \
-	FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
+	CSF="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-csf1)" \
+	IMG="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-img1)" \
+	FIT_KEY="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, image-kernel-fit)"
 
 BAREBOX_@PACKAGE@_MAKE_OPT	:= $(BAREBOX_@PACKAGE@_CONF_OPT)
 
diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index 66a2cab81395..24730d3cf742 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 Roland Hieber, Pengutronix <rhi@pengutronix.de>
 #
 # For further information about the PTXdist project and license conditions
 # see the README file.
@@ -176,6 +177,12 @@ export -f cs_set_uri
 # Get the uri from a role
 #
 cs_get_uri() {
+    if [ -z "${pkg_name}" ]; then
+	    echo ERROR_UNSUPPORTED_CS_API_CALL
+	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
+		'Use $(call ptx/cs-get-uri, <PKG>, <role>) instead.'
+    fi
+
     local role="${1}"
     cs_init_variables
 
@@ -297,6 +304,12 @@ export -f cs_import_key_from_pem
 # Get the path to the CA in pem format from a role
 #
 cs_get_ca() {
+    if [ -z "${pkg_name}" ]; then
+	    echo ERROR_UNSUPPORTED_CS_API_CALL
+	    ptxd_bailout '$(shell cs_get_ca, …) is no longer supported in make files.' \
+		'Use $(call ptx/cs-get-ca, <PKG>, …) instead.'
+    fi
+
     local role="${1}"
     cs_init_variables
 
-- 
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] 19+ messages in thread

* [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
                   ` (2 preceding siblings ...)
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment Roland Hieber
@ 2021-08-09  8:06 ` Roland Hieber
  2021-08-09  9:30   ` Roland Hieber
  2021-09-03 13:17   ` Michael Olbrich
  2021-09-03 12:48 ` [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Michael Olbrich
  4 siblings, 2 replies; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  8:06 UTC (permalink / raw)
  To: ptxdist; +Cc: Roland Hieber

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 - cs_check_whitelisted: make "needle"  local variable (feedback by
   Michael Olbrich)
 - cs_check_whitelisted: error out with ERROR_KEY_NOT_WHITELISTED also
   if whitelist does not exist yet (Michael Olbrich)
 - rename cs_get_uri to cs_get_uri_unchecked and let cs_get_uri wrap it
   instead of setting cs_no_whitelist_check=1 (Michael Olbrich)
 - docs: simplify introductory example (Michael Olbrich)
 - docs: add short paragraph on how to determine fingerprints of certs

PATCH v1: https://lore.ptxdist.org/ptxdist/
---
 doc/dev_code_signing.rst                  | 80 +++++++++++++++++++++++
 platforms/code-signing.in                 | 22 +++++++
 rules/pre/030-code-signing-consumers.make |  6 ++
 scripts/lib/ptxd_lib_code_signing.sh      | 71 ++++++++++++++++++--
 4 files changed, 172 insertions(+), 7 deletions(-)

diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
index 413f694980eb..28ebe055346e 100644
--- a/doc/dev_code_signing.rst
+++ b/doc/dev_code_signing.rst
@@ -172,3 +172,83 @@ also via an environment variable.
   (``=``, not ``:=``).
   Otherwise the variable is expanded before a code signing provider can perform
   its setup.
+
+Key Whitelisting
+~~~~~~~~~~~~~~~~
+
+In some use cases, it may be feasible to do additional checks to make sure that
+a package uses the correct key material.
+For example, suppose you have a "release" code signing provider which you use
+to sign bootloaders for production,
+and a "development" code signing provider which you use to sign bootloaders
+with an extended feature set, e.g. to allow booting arbitrary kernels and
+userlands for debugging purposes.
+Your production boards are locked down in hardware so the ROM code only
+executes bootloaders signed with the "release" key.
+Now you don't want any bootloader with debugging features to be signed with a
+release key, otherwise someone could boot them on a locked-down production
+device, and use the additional debugging features to get extended access to the
+production device.
+In this case, key whitelisting can help to prevent signing bootloader packages
+with the wrong key.
+
+When the ``CODE_SIGNING_REQUIRE_WHITELIST`` kconfig symbol is enabled,
+the consumer functions :ref:`ptx/cs-get-ca` and :ref:`ptx/cs-get-uri`
+look up the triplet of package name, role name, and the pubkey's SHA256
+fingerprint in the whitelist whose file name is determined by the
+``CODE_SIGNING_WHITELIST_FILENAME`` kconfig symbol (the default path is
+``configs/platform-<name>/code-signing-whitelist``).
+If a key or a CA is not whitelisted for the package in which it is to be used,
+the functions will exit with an error message on the terminal::
+
+   $ ptxdist -v print KERNEL_MAKE_OPT
+   ptxdist: error: SPKI whitelist record 'kernel kernel-modules
+   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04' not found in
+   distrokit/configs/platform-v7a/code-signing-whitelist
+
+   …/ptxdist/rules/kernel.make:196: *** cs-get-uri: whitelist check failed, see errors above.  Stop.
+
+
+If ``CODE_SIGNING_REQUIRE_WHITELIST`` is disabled (the default),
+all keys and CAs are provided to all packages without further checks.
+
+The format of the code signing whitelist consists of one triplet per line, in
+which the elements of the triplet are separated by whitespace.
+If a CA is to be checked, the role name is prefixed with a literal ``ca:``,
+and the fingerprint refers to the public key of the certificate.
+All other unmatched lines in the file are ignored, but we suggest to use ``#``
+to start a line comment so as not to add a whitelist record accidentally.
+
+For example, here is a whitelist for use with the *devel* code provider which
+allows all provided keys to be used by their respective consumers::
+
+   # format: package-name role-name sha256-pubkey-fingerprint
+   kernel      kernel-modules   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
+   image-rauc  update           0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
+   image-rauc  ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
+   rauc        ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
+
+.. note:: The match is case-sensitive, and the fingerprints are expected
+   in uppercase.
+
+   If a CA consists of more than one certificate, all of their fingerprints
+   must be whitelisted.
+
+You can determine the key fingerprints by copying it from the error message,
+or with the `spki-hash`__ tool from the ``host-extract-cert`` package,
+or with openssl::
+
+   $ openssl pkey -in keyfile.pem -pubout -outform der \
+     | openssl sha256 \
+     | tr 'a-z' 'A-Z'
+   (STDIN)= 69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
+
+or, in the case of certificates::
+
+   $ openssl x509 -noout -in cert.pem -pubkey \
+     | openssl pkey -pubin -inform pem -pubout -outform der \
+     | openssl sha256 \
+     | tr 'a-z' 'A-Z'
+   (STDIN)= 0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
+
+__ https://git.pengutronix.de/cgit/extract-cert/tree/spki-hash.c
diff --git a/platforms/code-signing.in b/platforms/code-signing.in
index 81f9ef6f3c9e..92f555c7e965 100644
--- a/platforms/code-signing.in
+++ b/platforms/code-signing.in
@@ -20,4 +20,26 @@ source "generated/code_signing_provider.in"
 
 endchoice
 
+config CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
+	bool
+	prompt "require whitelisted keys"
+	help
+	  Every time a key from the code provider is used, check if the consumer
+	  is allowed to use it.
+
+	  Code signing consumers can depend on this option if they want to force
+	  the key whitelist check.
+
+if CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
+
+config CODE_SIGNING_WHITELIST_FILENAME
+	string
+	prompt "whitelist file name"
+	default "code-signing-whitelist"
+	help
+	  This file name is used for the key whitelist. The path is relative to
+	  PTXDIST_PLATFORMCONFIGDIR (e.g. configs/platform-nnn/).
+
+endif # CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
+
 endif
diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
index 909e8ebd6936..bca05854372d 100644
--- a/rules/pre/030-code-signing-consumers.make
+++ b/rules/pre/030-code-signing-consumers.make
@@ -28,6 +28,9 @@ $(strip \
 		$(call ptx/cs-consumer-env, $(1))\
 			cs_get_uri '$(strip $(2))'\
 	)\
+	$(if $(filter-out 0,$(.SHELLSTATUS)),\
+		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
+	)\
 )
 endef
 
@@ -40,5 +43,8 @@ $(strip \
 		$(call ptx/cs-consumer-env, $(1))\
 			cs_get_ca '$(strip $(2))'\
 	)\
+	$(if $(filter-out 0,$(.SHELLSTATUS)),\
+		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
+	)\
 )
 endef
diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
index 24730d3cf742..c855821bd039 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) 2020 Jan Luebbe <j.luebbe@pengutronix.de>
 # Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
 #
 # For further information about the PTXdist project and license conditions
@@ -70,6 +71,7 @@ cs_init_variables() {
     sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
     keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
     keydir="${sysroot}/var/lib/keys/${keyprovider}"
+    whitelist="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_WHITELIST_FILENAME || true)"
 }
 export -f cs_init_variables
 
@@ -157,6 +159,42 @@ cs_group_get_roles() {
 }
 export -f cs_group_get_roles
 
+#
+# cs_check_whitelisted <role> <uri/pem or fingerprint prefixed with "sha256:">
+#
+# Checks if the SPKI (Subject Public Key Info) Hash is in the whitelist
+#
+cs_check_whitelisted() {
+    local role="${1:-ERROR_ROLE_IS_EMPTY}"
+    local src="${2}"
+    cs_init_variables
+
+    if [ "$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_REQUIRE_WHITELISTED_KEYS)" != "y" ]; then
+	return
+    fi
+
+    if ! ptxd_in_path PTXDIST_PATH_PLATFORMCONFIGDIR "${whitelist}"; then
+	echo ERROR_KEY_NOT_WHITELISTED
+	ptxd_bailout "${pkg_name}: SPKI hash whitelist check for ${role} (${src}) is required, but configs/<platform>/${whitelist} is missing."
+    fi
+    local whitelist_path="${ptxd_reply}"
+
+    local hash
+    if [ "${src#sha256:}" != "${src}" ]; then
+	hash="${src#sha256:}"
+    else
+	hash=$(ptxd_exec_silent_stderr spki-hash "${src}")
+    fi
+    hash=${hash:-ERROR_HASH_IS_EMPTY}
+    local needle="${pkg_name}\s\+${role}\s\+${hash}"
+
+    if ! grep -q --line-regexp "${needle}" "${whitelist_path}"; then
+	echo ERROR_KEY_NOT_WHITELISTED
+	ptxd_bailout "SPKI whitelist record '${pkg_name} ${role} ${hash}' not found in $(ptxd_print_path "${whitelist_path}")"
+    fi
+}
+export -f cs_check_whitelisted
+
 #
 # cs_set_uri <role> <uri>
 #
@@ -171,12 +209,8 @@ cs_set_uri() {
 }
 export -f cs_set_uri
 
-#
-# cs_get_uri <role>
-#
-# Get the uri from a role
-#
-cs_get_uri() {
+# internal helper for cs_get_uri
+cs_get_uri_unchecked() {
     if [ -z "${pkg_name}" ]; then
 	    echo ERROR_UNSUPPORTED_CS_API_CALL
 	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
@@ -198,8 +232,24 @@ cs_get_uri() {
 	    return
 	fi
     fi
+
     cat "${keydir}/${role}/uri"
 }
+export -f cs_get_uri_unchecked
+
+#
+# cs_get_uri <role>
+#
+# Get the uri from a role
+#
+cs_get_uri() {
+    local role="${1}"
+    local uri=$(cs_get_uri_unchecked "$1")
+
+    if cs_check_whitelisted "${role}" "${uri}"; then
+	echo "${uri}"
+    fi
+}
 export -f cs_get_uri
 
 #
@@ -321,6 +371,9 @@ cs_get_ca() {
     fi
 
     if [ -e "${ca}" ]; then
+	while read fp; do
+	    cs_check_whitelisted "ca:${role}" "sha256:${fp}"
+	done < "${keydir}/${role}/ca.fingerprints"
 	echo "${ca}"
     fi
 }
@@ -339,6 +392,9 @@ cs_append_ca_from_pem() {
     cat "${pem}" >> "${keydir}/${role}/ca.pem"
     # add new line in case ${pem} does not end with an EOL
     echo >> "${keydir}/${role}/ca.pem"
+
+    openssl x509 -in "${pem}" -inform pem -noout -pubkey | \
+	spki-hash /dev/stdin >> "${keydir}/${role}/ca.fingerprints"
 }
 export -f cs_append_ca_from_pem
 
@@ -370,7 +426,8 @@ cs_append_ca_from_uri() {
     cs_init_variables
 
     if [ -z "${uri}" ]; then
-	uri=$(cs_get_uri "${role}")
+	# cs_append_ca_from_der will check this later
+	uri=$(cs_get_uri_unchecked "${role}")
     fi
 
     ptxd_exec extract-cert "${uri}" "${tmpdir}/ca.der" &&
-- 
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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks Roland Hieber
@ 2021-08-09  9:30   ` Roland Hieber
  2021-09-03 13:17   ` Michael Olbrich
  1 sibling, 0 replies; 19+ messages in thread
From: Roland Hieber @ 2021-08-09  9:30 UTC (permalink / raw)
  To: ptxdist

On Mon, Aug 09, 2021 at 10:06:08AM +0200, Roland Hieber wrote:
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>  - cs_check_whitelisted: make "needle"  local variable (feedback by
>    Michael Olbrich)
>  - cs_check_whitelisted: error out with ERROR_KEY_NOT_WHITELISTED also
>    if whitelist does not exist yet (Michael Olbrich)

Forgot to mention that the cs-get* make wrappers now use ptx/error too.

 - Roland

>  - rename cs_get_uri to cs_get_uri_unchecked and let cs_get_uri wrap it
>    instead of setting cs_no_whitelist_check=1 (Michael Olbrich)
>  - docs: simplify introductory example (Michael Olbrich)
>  - docs: add short paragraph on how to determine fingerprints of certs
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/
> ---
>  doc/dev_code_signing.rst                  | 80 +++++++++++++++++++++++
>  platforms/code-signing.in                 | 22 +++++++
>  rules/pre/030-code-signing-consumers.make |  6 ++
>  scripts/lib/ptxd_lib_code_signing.sh      | 71 ++++++++++++++++++--
>  4 files changed, 172 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> index 413f694980eb..28ebe055346e 100644
> --- a/doc/dev_code_signing.rst
> +++ b/doc/dev_code_signing.rst
> @@ -172,3 +172,83 @@ also via an environment variable.
>    (``=``, not ``:=``).
>    Otherwise the variable is expanded before a code signing provider can perform
>    its setup.
> +
> +Key Whitelisting
> +~~~~~~~~~~~~~~~~
> +
> +In some use cases, it may be feasible to do additional checks to make sure that
> +a package uses the correct key material.
> +For example, suppose you have a "release" code signing provider which you use
> +to sign bootloaders for production,
> +and a "development" code signing provider which you use to sign bootloaders
> +with an extended feature set, e.g. to allow booting arbitrary kernels and
> +userlands for debugging purposes.
> +Your production boards are locked down in hardware so the ROM code only
> +executes bootloaders signed with the "release" key.
> +Now you don't want any bootloader with debugging features to be signed with a
> +release key, otherwise someone could boot them on a locked-down production
> +device, and use the additional debugging features to get extended access to the
> +production device.
> +In this case, key whitelisting can help to prevent signing bootloader packages
> +with the wrong key.
> +
> +When the ``CODE_SIGNING_REQUIRE_WHITELIST`` kconfig symbol is enabled,
> +the consumer functions :ref:`ptx/cs-get-ca` and :ref:`ptx/cs-get-uri`
> +look up the triplet of package name, role name, and the pubkey's SHA256
> +fingerprint in the whitelist whose file name is determined by the
> +``CODE_SIGNING_WHITELIST_FILENAME`` kconfig symbol (the default path is
> +``configs/platform-<name>/code-signing-whitelist``).
> +If a key or a CA is not whitelisted for the package in which it is to be used,
> +the functions will exit with an error message on the terminal::
> +
> +   $ ptxdist -v print KERNEL_MAKE_OPT
> +   ptxdist: error: SPKI whitelist record 'kernel kernel-modules
> +   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04' not found in
> +   distrokit/configs/platform-v7a/code-signing-whitelist
> +
> +   …/ptxdist/rules/kernel.make:196: *** cs-get-uri: whitelist check failed, see errors above.  Stop.
> +
> +
> +If ``CODE_SIGNING_REQUIRE_WHITELIST`` is disabled (the default),
> +all keys and CAs are provided to all packages without further checks.
> +
> +The format of the code signing whitelist consists of one triplet per line, in
> +which the elements of the triplet are separated by whitespace.
> +If a CA is to be checked, the role name is prefixed with a literal ``ca:``,
> +and the fingerprint refers to the public key of the certificate.
> +All other unmatched lines in the file are ignored, but we suggest to use ``#``
> +to start a line comment so as not to add a whitelist record accidentally.
> +
> +For example, here is a whitelist for use with the *devel* code provider which
> +allows all provided keys to be used by their respective consumers::
> +
> +   # format: package-name role-name sha256-pubkey-fingerprint
> +   kernel      kernel-modules   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> +   image-rauc  update           0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +   image-rauc  ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +   rauc        ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +
> +.. note:: The match is case-sensitive, and the fingerprints are expected
> +   in uppercase.
> +
> +   If a CA consists of more than one certificate, all of their fingerprints
> +   must be whitelisted.
> +
> +You can determine the key fingerprints by copying it from the error message,
> +or with the `spki-hash`__ tool from the ``host-extract-cert`` package,
> +or with openssl::
> +
> +   $ openssl pkey -in keyfile.pem -pubout -outform der \
> +     | openssl sha256 \
> +     | tr 'a-z' 'A-Z'
> +   (STDIN)= 69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> +
> +or, in the case of certificates::
> +
> +   $ openssl x509 -noout -in cert.pem -pubkey \
> +     | openssl pkey -pubin -inform pem -pubout -outform der \
> +     | openssl sha256 \
> +     | tr 'a-z' 'A-Z'
> +   (STDIN)= 0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +
> +__ https://git.pengutronix.de/cgit/extract-cert/tree/spki-hash.c
> diff --git a/platforms/code-signing.in b/platforms/code-signing.in
> index 81f9ef6f3c9e..92f555c7e965 100644
> --- a/platforms/code-signing.in
> +++ b/platforms/code-signing.in
> @@ -20,4 +20,26 @@ source "generated/code_signing_provider.in"
>  
>  endchoice
>  
> +config CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +	bool
> +	prompt "require whitelisted keys"
> +	help
> +	  Every time a key from the code provider is used, check if the consumer
> +	  is allowed to use it.
> +
> +	  Code signing consumers can depend on this option if they want to force
> +	  the key whitelist check.
> +
> +if CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +
> +config CODE_SIGNING_WHITELIST_FILENAME
> +	string
> +	prompt "whitelist file name"
> +	default "code-signing-whitelist"
> +	help
> +	  This file name is used for the key whitelist. The path is relative to
> +	  PTXDIST_PLATFORMCONFIGDIR (e.g. configs/platform-nnn/).
> +
> +endif # CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +
>  endif
> diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> index 909e8ebd6936..bca05854372d 100644
> --- a/rules/pre/030-code-signing-consumers.make
> +++ b/rules/pre/030-code-signing-consumers.make
> @@ -28,6 +28,9 @@ $(strip \
>  		$(call ptx/cs-consumer-env, $(1))\
>  			cs_get_uri '$(strip $(2))'\
>  	)\
> +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> +		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
> +	)\
>  )
>  endef
>  
> @@ -40,5 +43,8 @@ $(strip \
>  		$(call ptx/cs-consumer-env, $(1))\
>  			cs_get_ca '$(strip $(2))'\
>  	)\
> +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> +		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
> +	)\
>  )
>  endef
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 24730d3cf742..c855821bd039 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) 2020 Jan Luebbe <j.luebbe@pengutronix.de>
>  # Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
>  #
>  # For further information about the PTXdist project and license conditions
> @@ -70,6 +71,7 @@ cs_init_variables() {
>      sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
>      keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
>      keydir="${sysroot}/var/lib/keys/${keyprovider}"
> +    whitelist="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_WHITELIST_FILENAME || true)"
>  }
>  export -f cs_init_variables
>  
> @@ -157,6 +159,42 @@ cs_group_get_roles() {
>  }
>  export -f cs_group_get_roles
>  
> +#
> +# cs_check_whitelisted <role> <uri/pem or fingerprint prefixed with "sha256:">
> +#
> +# Checks if the SPKI (Subject Public Key Info) Hash is in the whitelist
> +#
> +cs_check_whitelisted() {
> +    local role="${1:-ERROR_ROLE_IS_EMPTY}"
> +    local src="${2}"
> +    cs_init_variables
> +
> +    if [ "$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_REQUIRE_WHITELISTED_KEYS)" != "y" ]; then
> +	return
> +    fi
> +
> +    if ! ptxd_in_path PTXDIST_PATH_PLATFORMCONFIGDIR "${whitelist}"; then
> +	echo ERROR_KEY_NOT_WHITELISTED
> +	ptxd_bailout "${pkg_name}: SPKI hash whitelist check for ${role} (${src}) is required, but configs/<platform>/${whitelist} is missing."
> +    fi
> +    local whitelist_path="${ptxd_reply}"
> +
> +    local hash
> +    if [ "${src#sha256:}" != "${src}" ]; then
> +	hash="${src#sha256:}"
> +    else
> +	hash=$(ptxd_exec_silent_stderr spki-hash "${src}")
> +    fi
> +    hash=${hash:-ERROR_HASH_IS_EMPTY}
> +    local needle="${pkg_name}\s\+${role}\s\+${hash}"
> +
> +    if ! grep -q --line-regexp "${needle}" "${whitelist_path}"; then
> +	echo ERROR_KEY_NOT_WHITELISTED
> +	ptxd_bailout "SPKI whitelist record '${pkg_name} ${role} ${hash}' not found in $(ptxd_print_path "${whitelist_path}")"
> +    fi
> +}
> +export -f cs_check_whitelisted
> +
>  #
>  # cs_set_uri <role> <uri>
>  #
> @@ -171,12 +209,8 @@ cs_set_uri() {
>  }
>  export -f cs_set_uri
>  
> -#
> -# cs_get_uri <role>
> -#
> -# Get the uri from a role
> -#
> -cs_get_uri() {
> +# internal helper for cs_get_uri
> +cs_get_uri_unchecked() {
>      if [ -z "${pkg_name}" ]; then
>  	    echo ERROR_UNSUPPORTED_CS_API_CALL
>  	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> @@ -198,8 +232,24 @@ cs_get_uri() {
>  	    return
>  	fi
>      fi
> +
>      cat "${keydir}/${role}/uri"
>  }
> +export -f cs_get_uri_unchecked
> +
> +#
> +# cs_get_uri <role>
> +#
> +# Get the uri from a role
> +#
> +cs_get_uri() {
> +    local role="${1}"
> +    local uri=$(cs_get_uri_unchecked "$1")
> +
> +    if cs_check_whitelisted "${role}" "${uri}"; then
> +	echo "${uri}"
> +    fi
> +}
>  export -f cs_get_uri
>  
>  #
> @@ -321,6 +371,9 @@ cs_get_ca() {
>      fi
>  
>      if [ -e "${ca}" ]; then
> +	while read fp; do
> +	    cs_check_whitelisted "ca:${role}" "sha256:${fp}"
> +	done < "${keydir}/${role}/ca.fingerprints"
>  	echo "${ca}"
>      fi
>  }
> @@ -339,6 +392,9 @@ cs_append_ca_from_pem() {
>      cat "${pem}" >> "${keydir}/${role}/ca.pem"
>      # add new line in case ${pem} does not end with an EOL
>      echo >> "${keydir}/${role}/ca.pem"
> +
> +    openssl x509 -in "${pem}" -inform pem -noout -pubkey | \
> +	spki-hash /dev/stdin >> "${keydir}/${role}/ca.fingerprints"
>  }
>  export -f cs_append_ca_from_pem
>  
> @@ -370,7 +426,8 @@ cs_append_ca_from_uri() {
>      cs_init_variables
>  
>      if [ -z "${uri}" ]; then
> -	uri=$(cs_get_uri "${role}")
> +	# cs_append_ca_from_der will check this later
> +	uri=$(cs_get_uri_unchecked "${role}")
>      fi
>  
>      ptxd_exec extract-cert "${uri}" "${tmpdir}/ca.der" &&
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs Roland Hieber
@ 2021-09-03 12:46   ` Michael Olbrich
  2021-09-08 11:27     ` Roland Hieber
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Olbrich @ 2021-09-03 12:46 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On Mon, Aug 09, 2021 at 10:06:06AM +0200, Roland Hieber wrote:
> We'll need this type of function more often later.

I don't see another user of this function in the rest of the series.

> 
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2: no changes
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-3-rhi@pengutronix.de
> ---
>  scripts/lib/ptxd_lib_code_signing.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 5ba1a4666af4..66a2cab81395 100644
> --- a/scripts/lib/ptxd_lib_code_signing.sh
> +++ b/scripts/lib/ptxd_lib_code_signing.sh
> @@ -49,6 +49,17 @@ softhsm_pkcs11_tool() {
>  }
>  export -f softhsm_pkcs11_tool
>  
> +#
> +# softhsm_pkcs11_uri <uri>
> +#
> +# Add the SoftHSM PIN to the given URI.
> +#
> +softhsm_pkcs11_uri() {
> +    local role="$1"

Why is 'role' passed as argument and 'keyprovider' is not?

> +    printf "pkcs11:token=%s;object=%s;pin-value=1111\n" "${keyprovider}" "${role}"

Why not just:

    echo "pkcs11:token=${keyprovider};object=${role};pin-value=1111"

> +}
> +export -f softhsm_pkcs11_uri
> +
>  #
>  # cs_init_variables
>  #
> @@ -95,7 +106,8 @@ cs_define_role() {
>  
>      mkdir -p "${keydir}/${role}" &&
>      # default for SoftHSM
> -    cs_set_uri "${role}" "pkcs11:token=${keyprovider};object=${role};pin-value=1111"
> +    local uri=$(softhsm_pkcs11_uri "${role}")

Why the extra local variable?

Michael

> +    cs_set_uri "${role}" "${uri}"
>  }
>  export -f cs_define_role
>  
> -- 
> 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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts
  2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
                   ` (3 preceding siblings ...)
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks Roland Hieber
@ 2021-09-03 12:48 ` Michael Olbrich
  2021-09-08 10:17   ` Roland Hieber
  4 siblings, 1 reply; 19+ messages in thread
From: Michael Olbrich @ 2021-09-03 12:48 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On Mon, Aug 09, 2021 at 10:06:04AM +0200, Roland Hieber wrote:
> Variables named ${pkg} are already widely used throughout the code base
> for different purposes, so name it ${pkg_name} instead.
> 
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> Turned out I didn't need it for this series, but I still think it's
> useful in general.

I'd like to drop this. This environment will be part of a commandline and
that's long enough as it is. So I'd like to avoid adding anything that's
not strictly needed. We've had issues in the past with commandlines that
exceed the limit.

Michael

> PATCH v2: no changes
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-1-rhi@pengutronix.de
> ---
>  rules/post/ptxd_make_world_common.make | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rules/post/ptxd_make_world_common.make b/rules/post/ptxd_make_world_common.make
> index e5cf50214e9c..c9d22c6884a3 100644
> --- a/rules/post/ptxd_make_world_common.make
> +++ b/rules/post/ptxd_make_world_common.make
> @@ -69,6 +69,7 @@ world/env/impl = \
>  	pkg_makefile="$(call ptx/escape,$($(1)_MAKEFILE))"			\
>  	pkg_infile="$(call ptx/escape,$($(1)_INFILE))"				\
>  										\
> +	pkg_name="$(call ptx/escape,$(PTX_MAP_TO_package_$(1)))"                \
>  	pkg_pkg="$(call ptx/escape,$($(1)))"					\
>  	pkg_version="$(call ptx/escape,$($(1)_VERSION))"			\
>  	pkg_config="$(call ptx/escape,$($(1)_CONFIG))"				\
> -- 
> 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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment Roland Hieber
@ 2021-09-03 12:54   ` Michael Olbrich
  2021-09-08 11:30     ` Roland Hieber
  2021-09-08 20:53   ` Roland Hieber
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Olbrich @ 2021-09-03 12:54 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On Mon, Aug 09, 2021 at 10:06:07AM +0200, Roland Hieber wrote:
> The code signing consumer functions should be able to retrieve some
> information about the recipe in which they were called in order to make
> additional checks if needed. Refactor the (shell cs_get_*, …) calls into
> macro calls of the form $(call ptx/cs-get-*, <PKG>, …). Let these
> macros look up the package name (for now) from PTX_MAP_TO_package_<PKG>
> before passing it to the shell functions. Using $(call world/env) here
> would be practical, but would also cause make to complain about
> recursive variable dependencies. Therefore variables must be added
> to ptx/cs-consumer-env manually, but additional information can be added
> later if needed.
> 
> Refactor the existing consumers in the code base too, and add an error
> message in case anyone else that still uses the old API.
> 
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>  - define multiline macros using "define"
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-4-rhi@pengutronix.de
> ---
>  doc/dev_code_signing.rst                      |  2 +-
>  doc/ref_code_signing_helpers.rst              | 25 ++++++-----
>  rules/barebox.make                            |  2 +-
>  rules/image-rauc.make                         |  6 +--
>  rules/kernel.make                             |  6 +--
>  rules/pre/030-code-signing-consumers.make     | 44 +++++++++++++++++++
>  rules/rauc.make                               |  2 +-
>  .../templates/template-barebox-imx-habv4-make |  6 +--
>  scripts/lib/ptxd_lib_code_signing.sh          | 13 ++++++
>  9 files changed, 83 insertions(+), 23 deletions(-)
>  create mode 100644 rules/pre/030-code-signing-consumers.make
> 
> diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> index b9a7c42f2a55..413f694980eb 100644
> --- a/doc/dev_code_signing.rst
> +++ b/doc/dev_code_signing.rst
> @@ -164,7 +164,7 @@ also via an environment variable.
>  .. code-block:: none
>  
>      $(call install_copy, rauc, 0, 0, 0644, \
> -      $(shell cs_get_ca update), \
> +      $(call ptx/cs-get-ca, RAUC, update), \
>        /etc/rauc/ca.cert.pem)
>  
>  .. note:: When code signing helper functions are used in make variables (e.g.
> diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> index fd16ca763557..d3429778d94d 100644
> --- a/doc/ref_code_signing_helpers.rst
> +++ b/doc/ref_code_signing_helpers.rst
> @@ -297,19 +297,21 @@ In the example given in :ref:`cs_group_add_roles` above, this would print::
>  Consumer Functions
>  ~~~~~~~~~~~~~~~~~~
>  
> +The consumer functions are implemented as make macros.
>  Packages that want to sign something or need access to keys/CAs can retrieve
>  PKCS#11 URIs and CA keyrings with these helpers.
>  
> +.. _ptx/cs-get-uri:
>  .. _cs_get_uri:
>  
> -cs_get_uri
> -^^^^^^^^^^
> +ptx/cs-get-uri
> +^^^^^^^^^^^^^^
>  
>  Usage:
>  
> -.. code-block:: bash
> +.. code-block:: make
>  
> -    cs_get_uri <role>
> +    $(call ptx/cs-get-uri, <PKG>, <role>)
>  
>  Get PKCS#11 URI for role.
>  
> @@ -317,16 +319,17 @@ Preconditions:
>  
>  - the URI must have been set (see :ref:`cs_set_uri`)
>  
> +.. _ptx/cs-get-ca:
>  .. _cs_get_ca:
>  
> -cs_get_ca
> -^^^^^^^^^
> +ptx/cs-get-ca
> +^^^^^^^^^^^^^
>  
>  Usage:
>  
> -.. code-block:: bash
> +.. code-block:: make
>  
> -    cs_get_ca <role>
> +    $(call ptx/cs-get-ca, <PKG>, <role>)
>  
>  Get path to the CA keyring in PEM format for role.
>  
> @@ -347,7 +350,7 @@ Example:
>  
>     # set up kernel module signing, and add a trusted CA if the provider set one
>     KERNEL_SIGN_OPT =
> -   	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"' \
> +   	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"' \
>     	CONFIG_MODULE_SIG_ALL=y \
> -   	$(if $(shell cs_get_ca kernel-trusted), \
> -   		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> +   	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> +   		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
> diff --git a/rules/barebox.make b/rules/barebox.make
> index bea9f3adcbf8..983d34032e0d 100644
> --- a/rules/barebox.make
> +++ b/rules/barebox.make
> @@ -103,7 +103,7 @@ endif
>  ifdef PTXCONF_CODE_SIGNING
>  BAREBOX_MAKE_ENV = \
>  	$(CODE_SIGNING_ENV) \
> -	IMAGE_KERNEL_FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> +	IMAGE_KERNEL_FIT_KEY="$(call ptx/cs-get-uri, BAREBOX, image-kernel-fit)"
>  endif
>  
>  $(STATEDIR)/barebox.compile:
> diff --git a/rules/image-rauc.make b/rules/image-rauc.make
> index fe1b0e89be7c..c8747231f8f1 100644
> --- a/rules/image-rauc.make
> +++ b/rules/image-rauc.make
> @@ -32,9 +32,9 @@ IMAGE_RAUC_ENV	= \
>  	RAUC_BUNDLE_VERSION="$(call remove_quotes, $(PTXCONF_RAUC_BUNDLE_VERSION))" \
>  	RAUC_BUNDLE_BUILD=$(call ptx/sh, date +%FT%T%z) \
>  	RAUC_BUNDLE_DESCRIPTION=$(PTXCONF_IMAGE_RAUC_DESCRIPTION) \
> -	RAUC_KEY="$(shell cs_get_uri update)" \
> -	RAUC_CERT="$(shell cs_get_uri update)" \
> -	RAUC_KEYRING="$(shell cs_get_ca update)"
> +	RAUC_KEY="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> +	RAUC_CERT="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> +	RAUC_KEYRING="$(call ptx/cs-get-ca, IMAGE_RAUC, update)"
>  
>  $(IMAGE_RAUC_IMAGE):
>  	@$(call targetinfo)
> diff --git a/rules/kernel.make b/rules/kernel.make
> index 9caff677918e..e6faba82df38 100644
> --- a/rules/kernel.make
> +++ b/rules/kernel.make
> @@ -73,12 +73,12 @@ KERNEL_BASE_OPT		= \
>  
>  ifdef PTXCONF_KERNEL_CODE_SIGNING
>  KERNEL_BASE_OPT		+= \
> -	$(if $(shell cs_get_ca kernel-trusted), \
> -		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> +	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> +		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
>  endif
>  ifdef PTXCONF_KERNEL_MODULES_SIGN
>  KERNEL_BASE_OPT		+= \
> -	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"'
> +	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"'
>  endif
>  
>  # Intermediate option. This will be used by kernel module packages.
> diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> new file mode 100644
> index 000000000000..909e8ebd6936
> --- /dev/null
> +++ b/rules/pre/030-code-signing-consumers.make
> @@ -0,0 +1,44 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +#
> +
> +#
> +# Usage: $(call ptx/cs-consumer-env, <PKG>)
> +#
> +# We usually want to use cs-get-* macros inside a <PKG>_MAKE_OPT etc., which is
> +# referenced in world/env, so we cannot use world/env to set pkg_name without
> +# running into circular variable dependencies.
> +#
> +define ptx/cs-consumer-env

Use the regular 'ptx/cs-consumer-env =' for this to keep it consistent with
other env lists.

> +	pkg_name='$(PTX_MAP_TO_package_$(strip $(1)))' \
> +	$(CODE_SIGNING_ENV)
> +endef
> +
> +#
> +# Usage: $(call ptx/cs-get-uri, <PKG>, <role>)
> +#
> +define ptx/cs-get-uri
> +$(strip \
> +	$(shell \
> +		$(call ptx/cs-consumer-env, $(1))\
> +			cs_get_uri '$(strip $(2))'\
> +	)\

No \ needed with 'define'.

> +)
> +endef
> +
> +#
> +# Usage: $(call ptx/cs-get-ca, <PKG>, <role>)
> +#
> +define ptx/cs-get-ca
> +$(strip \
> +	$(shell \
> +		$(call ptx/cs-consumer-env, $(1))\
> +			cs_get_ca '$(strip $(2))'\
> +	)\

Same here.

> +)
> +endef
> diff --git a/rules/rauc.make b/rules/rauc.make
> index 08df6336a7cd..3c28befcd3ff 100644
> --- a/rules/rauc.make
> +++ b/rules/rauc.make
> @@ -78,7 +78,7 @@ ifdef PTXCONF_RAUC_CONFIGURATION
>  	@$(call install_replace, rauc, /etc/rauc/system.conf, \
>  		@RAUC_BUNDLE_COMPATIBLE@, \
>  		"$(call remove_quotes,$(PTXCONF_RAUC_COMPATIBLE))")
> -	@$(call install_copy, rauc, 0, 0, 0644, $(shell cs_get_ca update), \
> +	@$(call install_copy, rauc, 0, 0, 0644, $(call ptx/cs-get-ca, RAUC, update), \
>  		/etc/rauc/ca.cert.pem)
>  endif
>  
> diff --git a/rules/templates/template-barebox-imx-habv4-make b/rules/templates/template-barebox-imx-habv4-make
> index cc825dc90292..b2d5d7100fc9 100644
> --- a/rules/templates/template-barebox-imx-habv4-make
> +++ b/rules/templates/template-barebox-imx-habv4-make
> @@ -64,9 +64,9 @@ endif
>  
>  BAREBOX_@PACKAGE@_MAKE_ENV	= \
>  	$(CODE_SIGNING_ENV) \
> -	CSF="$(shell cs_get_uri imx-habv4-csf1)" \
> -	IMG="$(shell cs_get_uri imx-habv4-img1)" \
> -	FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> +	CSF="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-csf1)" \
> +	IMG="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-img1)" \
> +	FIT_KEY="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, image-kernel-fit)"
>  
>  BAREBOX_@PACKAGE@_MAKE_OPT	:= $(BAREBOX_@PACKAGE@_CONF_OPT)
>  
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 66a2cab81395..24730d3cf742 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 Roland Hieber, Pengutronix <rhi@pengutronix.de>
>  #
>  # For further information about the PTXdist project and license conditions
>  # see the README file.
> @@ -176,6 +177,12 @@ export -f cs_set_uri
>  # Get the uri from a role
>  #
>  cs_get_uri() {
> +    if [ -z "${pkg_name}" ]; then
> +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> +	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> +		'Use $(call ptx/cs-get-uri, <PKG>, <role>) instead.'
> +    fi
> +
>      local role="${1}"
>      cs_init_variables
>  
> @@ -297,6 +304,12 @@ export -f cs_import_key_from_pem
>  # Get the path to the CA in pem format from a role
>  #
>  cs_get_ca() {
> +    if [ -z "${pkg_name}" ]; then
> +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> +	    ptxd_bailout '$(shell cs_get_ca, …) is no longer supported in make files.' \
> +		'Use $(call ptx/cs-get-ca, <PKG>, …) instead.'
> +    fi
> +
>      local role="${1}"
>      cs_init_variables
>  
> -- 
> 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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks Roland Hieber
  2021-08-09  9:30   ` Roland Hieber
@ 2021-09-03 13:17   ` Michael Olbrich
  2021-09-08 11:43     ` Roland Hieber
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Olbrich @ 2021-09-03 13:17 UTC (permalink / raw)
  To: Roland Hieber, ptxdist

On Mon, Aug 09, 2021 at 10:06:08AM +0200, Roland Hieber wrote:
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>  - cs_check_whitelisted: make "needle"  local variable (feedback by
>    Michael Olbrich)
>  - cs_check_whitelisted: error out with ERROR_KEY_NOT_WHITELISTED also
>    if whitelist does not exist yet (Michael Olbrich)
>  - rename cs_get_uri to cs_get_uri_unchecked and let cs_get_uri wrap it
>    instead of setting cs_no_whitelist_check=1 (Michael Olbrich)
>  - docs: simplify introductory example (Michael Olbrich)
>  - docs: add short paragraph on how to determine fingerprints of certs
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/
> ---
>  doc/dev_code_signing.rst                  | 80 +++++++++++++++++++++++
>  platforms/code-signing.in                 | 22 +++++++
>  rules/pre/030-code-signing-consumers.make |  6 ++
>  scripts/lib/ptxd_lib_code_signing.sh      | 71 ++++++++++++++++++--
>  4 files changed, 172 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> index 413f694980eb..28ebe055346e 100644
> --- a/doc/dev_code_signing.rst
> +++ b/doc/dev_code_signing.rst
> @@ -172,3 +172,83 @@ also via an environment variable.
>    (``=``, not ``:=``).
>    Otherwise the variable is expanded before a code signing provider can perform
>    its setup.
> +
> +Key Whitelisting
> +~~~~~~~~~~~~~~~~
> +
> +In some use cases, it may be feasible to do additional checks to make sure that
> +a package uses the correct key material.
> +For example, suppose you have a "release" code signing provider which you use
> +to sign bootloaders for production,
> +and a "development" code signing provider which you use to sign bootloaders
> +with an extended feature set, e.g. to allow booting arbitrary kernels and
> +userlands for debugging purposes.
> +Your production boards are locked down in hardware so the ROM code only
> +executes bootloaders signed with the "release" key.
> +Now you don't want any bootloader with debugging features to be signed with a
> +release key, otherwise someone could boot them on a locked-down production
> +device, and use the additional debugging features to get extended access to the
> +production device.
> +In this case, key whitelisting can help to prevent signing bootloader packages
> +with the wrong key.
> +
> +When the ``CODE_SIGNING_REQUIRE_WHITELIST`` kconfig symbol is enabled,
> +the consumer functions :ref:`ptx/cs-get-ca` and :ref:`ptx/cs-get-uri`
> +look up the triplet of package name, role name, and the pubkey's SHA256
> +fingerprint in the whitelist whose file name is determined by the
> +``CODE_SIGNING_WHITELIST_FILENAME`` kconfig symbol (the default path is
> +``configs/platform-<name>/code-signing-whitelist``).
> +If a key or a CA is not whitelisted for the package in which it is to be used,
> +the functions will exit with an error message on the terminal::
> +
> +   $ ptxdist -v print KERNEL_MAKE_OPT
> +   ptxdist: error: SPKI whitelist record 'kernel kernel-modules
> +   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04' not found in
> +   distrokit/configs/platform-v7a/code-signing-whitelist
> +
> +   …/ptxdist/rules/kernel.make:196: *** cs-get-uri: whitelist check failed, see errors above.  Stop.
> +
> +
> +If ``CODE_SIGNING_REQUIRE_WHITELIST`` is disabled (the default),
> +all keys and CAs are provided to all packages without further checks.
> +
> +The format of the code signing whitelist consists of one triplet per line, in
> +which the elements of the triplet are separated by whitespace.
> +If a CA is to be checked, the role name is prefixed with a literal ``ca:``,
> +and the fingerprint refers to the public key of the certificate.
> +All other unmatched lines in the file are ignored, but we suggest to use ``#``
> +to start a line comment so as not to add a whitelist record accidentally.
> +
> +For example, here is a whitelist for use with the *devel* code provider which
> +allows all provided keys to be used by their respective consumers::
> +
> +   # format: package-name role-name sha256-pubkey-fingerprint
> +   kernel      kernel-modules   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> +   image-rauc  update           0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +   image-rauc  ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +   rauc        ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +
> +.. note:: The match is case-sensitive, and the fingerprints are expected
> +   in uppercase.
> +
> +   If a CA consists of more than one certificate, all of their fingerprints
> +   must be whitelisted.
> +
> +You can determine the key fingerprints by copying it from the error message,
> +or with the `spki-hash`__ tool from the ``host-extract-cert`` package,
> +or with openssl::
> +
> +   $ openssl pkey -in keyfile.pem -pubout -outform der \
> +     | openssl sha256 \
> +     | tr 'a-z' 'A-Z'
> +   (STDIN)= 69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> +
> +or, in the case of certificates::
> +
> +   $ openssl x509 -noout -in cert.pem -pubkey \
> +     | openssl pkey -pubin -inform pem -pubout -outform der \
> +     | openssl sha256 \
> +     | tr 'a-z' 'A-Z'
> +   (STDIN)= 0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> +
> +__ https://git.pengutronix.de/cgit/extract-cert/tree/spki-hash.c
> diff --git a/platforms/code-signing.in b/platforms/code-signing.in
> index 81f9ef6f3c9e..92f555c7e965 100644
> --- a/platforms/code-signing.in
> +++ b/platforms/code-signing.in
> @@ -20,4 +20,26 @@ source "generated/code_signing_provider.in"
>  
>  endchoice
>  
> +config CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +	bool
> +	prompt "require whitelisted keys"
> +	help
> +	  Every time a key from the code provider is used, check if the consumer
> +	  is allowed to use it.
> +
> +	  Code signing consumers can depend on this option if they want to force
> +	  the key whitelist check.
> +
> +if CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +
> +config CODE_SIGNING_WHITELIST_FILENAME
> +	string
> +	prompt "whitelist file name"
> +	default "code-signing-whitelist"
> +	help
> +	  This file name is used for the key whitelist. The path is relative to
> +	  PTXDIST_PLATFORMCONFIGDIR (e.g. configs/platform-nnn/).

I think, we can just hard-code this name. No need for a config option.
Or is there a use-case for this?

> +
> +endif # CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> +
>  endif
> diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> index 909e8ebd6936..bca05854372d 100644
> --- a/rules/pre/030-code-signing-consumers.make
> +++ b/rules/pre/030-code-signing-consumers.make
> @@ -28,6 +28,9 @@ $(strip \
>  		$(call ptx/cs-consumer-env, $(1))\
>  			cs_get_uri '$(strip $(2))'\
>  	)\
> +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> +		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
> +	)\

You cannot use ptx/error here. That macro can only be used for stuff that
is evaluated before the first target is executed. Otherwise the error
message will get lost.
You need to use $(error ...) here. Probably with the same $(shell :) hack
that is used in kernel.make.

>  )
>  endef
>  
> @@ -40,5 +43,8 @@ $(strip \
>  		$(call ptx/cs-consumer-env, $(1))\
>  			cs_get_ca '$(strip $(2))'\
>  	)\
> +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> +		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
> +	)\

same here.

>  )
>  endef
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 24730d3cf742..c855821bd039 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) 2020 Jan Luebbe <j.luebbe@pengutronix.de>
>  # Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
>  #
>  # For further information about the PTXdist project and license conditions
> @@ -70,6 +71,7 @@ cs_init_variables() {
>      sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
>      keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
>      keydir="${sysroot}/var/lib/keys/${keyprovider}"
> +    whitelist="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_WHITELIST_FILENAME || true)"
>  }
>  export -f cs_init_variables
>  
> @@ -157,6 +159,42 @@ cs_group_get_roles() {
>  }
>  export -f cs_group_get_roles
>  
> +#
> +# cs_check_whitelisted <role> <uri/pem or fingerprint prefixed with "sha256:">
> +#
> +# Checks if the SPKI (Subject Public Key Info) Hash is in the whitelist
> +#
> +cs_check_whitelisted() {
> +    local role="${1:-ERROR_ROLE_IS_EMPTY}"
> +    local src="${2}"
> +    cs_init_variables
> +
> +    if [ "$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_REQUIRE_WHITELISTED_KEYS)" != "y" ]; then
> +	return
> +    fi
> +
> +    if ! ptxd_in_path PTXDIST_PATH_PLATFORMCONFIGDIR "${whitelist}"; then
> +	echo ERROR_KEY_NOT_WHITELISTED
> +	ptxd_bailout "${pkg_name}: SPKI hash whitelist check for ${role} (${src}) is required, but configs/<platform>/${whitelist} is missing."
> +    fi
> +    local whitelist_path="${ptxd_reply}"
> +
> +    local hash
> +    if [ "${src#sha256:}" != "${src}" ]; then
> +	hash="${src#sha256:}"
> +    else
> +	hash=$(ptxd_exec_silent_stderr spki-hash "${src}")
> +    fi
> +    hash=${hash:-ERROR_HASH_IS_EMPTY}
> +    local needle="${pkg_name}\s\+${role}\s\+${hash}"
> +
> +    if ! grep -q --line-regexp "${needle}" "${whitelist_path}"; then
> +	echo ERROR_KEY_NOT_WHITELISTED
> +	ptxd_bailout "SPKI whitelist record '${pkg_name} ${role} ${hash}' not found in $(ptxd_print_path "${whitelist_path}")"
> +    fi
> +}
> +export -f cs_check_whitelisted
> +
>  #
>  # cs_set_uri <role> <uri>
>  #
> @@ -171,12 +209,8 @@ cs_set_uri() {
>  }
>  export -f cs_set_uri
>  
> -#
> -# cs_get_uri <role>
> -#
> -# Get the uri from a role
> -#
> -cs_get_uri() {
> +# internal helper for cs_get_uri
> +cs_get_uri_unchecked() {
>      if [ -z "${pkg_name}" ]; then
>  	    echo ERROR_UNSUPPORTED_CS_API_CALL
>  	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> @@ -198,8 +232,24 @@ cs_get_uri() {
>  	    return
>  	fi
>      fi
> +
>      cat "${keydir}/${role}/uri"
>  }
> +export -f cs_get_uri_unchecked
> +
> +#
> +# cs_get_uri <role>
> +#
> +# Get the uri from a role
> +#
> +cs_get_uri() {
> +    local role="${1}"
> +    local uri=$(cs_get_uri_unchecked "$1")
> +
> +    if cs_check_whitelisted "${role}" "${uri}"; then
> +	echo "${uri}"
> +    fi
> +}
>  export -f cs_get_uri
>  
>  #
> @@ -321,6 +371,9 @@ cs_get_ca() {
>      fi
>  
>      if [ -e "${ca}" ]; then
> +	while read fp; do
> +	    cs_check_whitelisted "ca:${role}" "sha256:${fp}"
> +	done < "${keydir}/${role}/ca.fingerprints"
>  	echo "${ca}"
>      fi
>  }
> @@ -339,6 +392,9 @@ cs_append_ca_from_pem() {
>      cat "${pem}" >> "${keydir}/${role}/ca.pem"
>      # add new line in case ${pem} does not end with an EOL
>      echo >> "${keydir}/${role}/ca.pem"
> +
> +    openssl x509 -in "${pem}" -inform pem -noout -pubkey | \
> +	spki-hash /dev/stdin >> "${keydir}/${role}/ca.fingerprints"

This should happen first, before the extending ca.pem and make sure to
abort on error. Use check_pipe_status to catch errors from openssl.

>  }
>  export -f cs_append_ca_from_pem
>  
> @@ -370,7 +426,8 @@ cs_append_ca_from_uri() {
>      cs_init_variables
>  
>      if [ -z "${uri}" ]; then
> -	uri=$(cs_get_uri "${role}")
> +	# cs_append_ca_from_der will check this later

I don't think this comment is correct. There is no checking while adding to
the CA, right?

Michael

> +	uri=$(cs_get_uri_unchecked "${role}")
>      fi
>  
>      ptxd_exec extract-cert "${uri}" "${tmpdir}/ca.der" &&
> -- 
> 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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts
  2021-09-03 12:48 ` [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Michael Olbrich
@ 2021-09-08 10:17   ` Roland Hieber
  0 siblings, 0 replies; 19+ messages in thread
From: Roland Hieber @ 2021-09-08 10:17 UTC (permalink / raw)
  To: ptxdist

On Fri, Sep 03, 2021 at 02:48:53PM +0200, Michael Olbrich wrote:
> On Mon, Aug 09, 2021 at 10:06:04AM +0200, Roland Hieber wrote:
> > Variables named ${pkg} are already widely used throughout the code base
> > for different purposes, so name it ${pkg_name} instead.
> > 
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > Turned out I didn't need it for this series, but I still think it's
> > useful in general.
> 
> I'd like to drop this. This environment will be part of a commandline and
> that's long enough as it is. So I'd like to avoid adding anything that's
> not strictly needed. We've had issues in the past with commandlines that
> exceed the limit.

Mhm, yes, makes sense.

 - Roland

> > PATCH v2: no changes
> > 
> > PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-1-rhi@pengutronix.de
> > ---
> >  rules/post/ptxd_make_world_common.make | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/rules/post/ptxd_make_world_common.make b/rules/post/ptxd_make_world_common.make
> > index e5cf50214e9c..c9d22c6884a3 100644
> > --- a/rules/post/ptxd_make_world_common.make
> > +++ b/rules/post/ptxd_make_world_common.make
> > @@ -69,6 +69,7 @@ world/env/impl = \
> >  	pkg_makefile="$(call ptx/escape,$($(1)_MAKEFILE))"			\
> >  	pkg_infile="$(call ptx/escape,$($(1)_INFILE))"				\
> >  										\
> > +	pkg_name="$(call ptx/escape,$(PTX_MAP_TO_package_$(1)))"                \
> >  	pkg_pkg="$(call ptx/escape,$($(1)))"					\
> >  	pkg_version="$(call ptx/escape,$($(1)_VERSION))"			\
> >  	pkg_config="$(call ptx/escape,$($(1)_CONFIG))"				\
> > -- 
> > 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
> 

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs
  2021-09-03 12:46   ` Michael Olbrich
@ 2021-09-08 11:27     ` Roland Hieber
  2021-09-08 14:01       ` Michael Olbrich
  0 siblings, 1 reply; 19+ messages in thread
From: Roland Hieber @ 2021-09-08 11:27 UTC (permalink / raw)
  To: ptxdist

On Fri, Sep 03, 2021 at 02:46:46PM +0200, Michael Olbrich wrote:
> On Mon, Aug 09, 2021 at 10:06:06AM +0200, Roland Hieber wrote:
> > We'll need this type of function more often later.
> 
> I don't see another user of this function in the rest of the series.

Huh yes. I think I used it multiple times in a previous version of the
series. I think this patch can be dropped.

> 
> > 
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > PATCH v2: no changes
> > 
> > PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-3-rhi@pengutronix.de
> > ---
> >  scripts/lib/ptxd_lib_code_signing.sh | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > index 5ba1a4666af4..66a2cab81395 100644
> > --- a/scripts/lib/ptxd_lib_code_signing.sh
> > +++ b/scripts/lib/ptxd_lib_code_signing.sh
> > @@ -49,6 +49,17 @@ softhsm_pkcs11_tool() {
> >  }
> >  export -f softhsm_pkcs11_tool
> >  
> > +#
> > +# softhsm_pkcs11_uri <uri>
> > +#
> > +# Add the SoftHSM PIN to the given URI.
> > +#
> > +softhsm_pkcs11_uri() {
> > +    local role="$1"
> 
> Why is 'role' passed as argument and 'keyprovider' is not?
> 
> > +    printf "pkcs11:token=%s;object=%s;pin-value=1111\n" "${keyprovider}" "${role}"
> 
> Why not just:
> 
>     echo "pkcs11:token=${keyprovider};object=${role};pin-value=1111"

Force of habit from using C and Python. And depending on the actual echo
implementation (POSIX sh, bash, or /bin/echo), there are different
behaviours regarding things like printing a literal -e, or interpolation
of \r, \t etc., and I've never encountered this with printf. So I
usually use printf instead of echo.

 - Roland

> > +}
> > +export -f softhsm_pkcs11_uri
> > +
> >  #
> >  # cs_init_variables
> >  #
> > @@ -95,7 +106,8 @@ cs_define_role() {
> >  
> >      mkdir -p "${keydir}/${role}" &&
> >      # default for SoftHSM
> > -    cs_set_uri "${role}" "pkcs11:token=${keyprovider};object=${role};pin-value=1111"
> > +    local uri=$(softhsm_pkcs11_uri "${role}")
> 
> Why the extra local variable?
> Michael
> 
> > +    cs_set_uri "${role}" "${uri}"
> >  }
> >  export -f cs_define_role
> >  
> > -- 
> > 2.30.2

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment
  2021-09-03 12:54   ` Michael Olbrich
@ 2021-09-08 11:30     ` Roland Hieber
  2021-09-08 14:08       ` Michael Olbrich
  0 siblings, 1 reply; 19+ messages in thread
From: Roland Hieber @ 2021-09-08 11:30 UTC (permalink / raw)
  To: ptxdist

On Fri, Sep 03, 2021 at 02:54:31PM +0200, Michael Olbrich wrote:
> On Mon, Aug 09, 2021 at 10:06:07AM +0200, Roland Hieber wrote:
> > The code signing consumer functions should be able to retrieve some
> > information about the recipe in which they were called in order to make
> > additional checks if needed. Refactor the (shell cs_get_*, …) calls into
> > macro calls of the form $(call ptx/cs-get-*, <PKG>, …). Let these
> > macros look up the package name (for now) from PTX_MAP_TO_package_<PKG>
> > before passing it to the shell functions. Using $(call world/env) here
> > would be practical, but would also cause make to complain about
> > recursive variable dependencies. Therefore variables must be added
> > to ptx/cs-consumer-env manually, but additional information can be added
> > later if needed.
> > 
> > Refactor the existing consumers in the code base too, and add an error
> > message in case anyone else that still uses the old API.
> > 
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > PATCH v2:
> >  - define multiline macros using "define"
> > 
> > PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-4-rhi@pengutronix.de
> > ---
> >  doc/dev_code_signing.rst                      |  2 +-
> >  doc/ref_code_signing_helpers.rst              | 25 ++++++-----
> >  rules/barebox.make                            |  2 +-
> >  rules/image-rauc.make                         |  6 +--
> >  rules/kernel.make                             |  6 +--
> >  rules/pre/030-code-signing-consumers.make     | 44 +++++++++++++++++++
> >  rules/rauc.make                               |  2 +-
> >  .../templates/template-barebox-imx-habv4-make |  6 +--
> >  scripts/lib/ptxd_lib_code_signing.sh          | 13 ++++++
> >  9 files changed, 83 insertions(+), 23 deletions(-)
> >  create mode 100644 rules/pre/030-code-signing-consumers.make
> > 
> > diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> > index b9a7c42f2a55..413f694980eb 100644
> > --- a/doc/dev_code_signing.rst
> > +++ b/doc/dev_code_signing.rst
> > @@ -164,7 +164,7 @@ also via an environment variable.
> >  .. code-block:: none
> >  
> >      $(call install_copy, rauc, 0, 0, 0644, \
> > -      $(shell cs_get_ca update), \
> > +      $(call ptx/cs-get-ca, RAUC, update), \
> >        /etc/rauc/ca.cert.pem)
> >  
> >  .. note:: When code signing helper functions are used in make variables (e.g.
> > diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> > index fd16ca763557..d3429778d94d 100644
> > --- a/doc/ref_code_signing_helpers.rst
> > +++ b/doc/ref_code_signing_helpers.rst
> > @@ -297,19 +297,21 @@ In the example given in :ref:`cs_group_add_roles` above, this would print::
> >  Consumer Functions
> >  ~~~~~~~~~~~~~~~~~~
> >  
> > +The consumer functions are implemented as make macros.
> >  Packages that want to sign something or need access to keys/CAs can retrieve
> >  PKCS#11 URIs and CA keyrings with these helpers.
> >  
> > +.. _ptx/cs-get-uri:
> >  .. _cs_get_uri:
> >  
> > -cs_get_uri
> > -^^^^^^^^^^
> > +ptx/cs-get-uri
> > +^^^^^^^^^^^^^^
> >  
> >  Usage:
> >  
> > -.. code-block:: bash
> > +.. code-block:: make
> >  
> > -    cs_get_uri <role>
> > +    $(call ptx/cs-get-uri, <PKG>, <role>)
> >  
> >  Get PKCS#11 URI for role.
> >  
> > @@ -317,16 +319,17 @@ Preconditions:
> >  
> >  - the URI must have been set (see :ref:`cs_set_uri`)
> >  
> > +.. _ptx/cs-get-ca:
> >  .. _cs_get_ca:
> >  
> > -cs_get_ca
> > -^^^^^^^^^
> > +ptx/cs-get-ca
> > +^^^^^^^^^^^^^
> >  
> >  Usage:
> >  
> > -.. code-block:: bash
> > +.. code-block:: make
> >  
> > -    cs_get_ca <role>
> > +    $(call ptx/cs-get-ca, <PKG>, <role>)
> >  
> >  Get path to the CA keyring in PEM format for role.
> >  
> > @@ -347,7 +350,7 @@ Example:
> >  
> >     # set up kernel module signing, and add a trusted CA if the provider set one
> >     KERNEL_SIGN_OPT =
> > -   	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"' \
> > +   	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"' \
> >     	CONFIG_MODULE_SIG_ALL=y \
> > -   	$(if $(shell cs_get_ca kernel-trusted), \
> > -   		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> > +   	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> > +   		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
> > diff --git a/rules/barebox.make b/rules/barebox.make
> > index bea9f3adcbf8..983d34032e0d 100644
> > --- a/rules/barebox.make
> > +++ b/rules/barebox.make
> > @@ -103,7 +103,7 @@ endif
> >  ifdef PTXCONF_CODE_SIGNING
> >  BAREBOX_MAKE_ENV = \
> >  	$(CODE_SIGNING_ENV) \
> > -	IMAGE_KERNEL_FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> > +	IMAGE_KERNEL_FIT_KEY="$(call ptx/cs-get-uri, BAREBOX, image-kernel-fit)"
> >  endif
> >  
> >  $(STATEDIR)/barebox.compile:
> > diff --git a/rules/image-rauc.make b/rules/image-rauc.make
> > index fe1b0e89be7c..c8747231f8f1 100644
> > --- a/rules/image-rauc.make
> > +++ b/rules/image-rauc.make
> > @@ -32,9 +32,9 @@ IMAGE_RAUC_ENV	= \
> >  	RAUC_BUNDLE_VERSION="$(call remove_quotes, $(PTXCONF_RAUC_BUNDLE_VERSION))" \
> >  	RAUC_BUNDLE_BUILD=$(call ptx/sh, date +%FT%T%z) \
> >  	RAUC_BUNDLE_DESCRIPTION=$(PTXCONF_IMAGE_RAUC_DESCRIPTION) \
> > -	RAUC_KEY="$(shell cs_get_uri update)" \
> > -	RAUC_CERT="$(shell cs_get_uri update)" \
> > -	RAUC_KEYRING="$(shell cs_get_ca update)"
> > +	RAUC_KEY="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> > +	RAUC_CERT="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> > +	RAUC_KEYRING="$(call ptx/cs-get-ca, IMAGE_RAUC, update)"
> >  
> >  $(IMAGE_RAUC_IMAGE):
> >  	@$(call targetinfo)
> > diff --git a/rules/kernel.make b/rules/kernel.make
> > index 9caff677918e..e6faba82df38 100644
> > --- a/rules/kernel.make
> > +++ b/rules/kernel.make
> > @@ -73,12 +73,12 @@ KERNEL_BASE_OPT		= \
> >  
> >  ifdef PTXCONF_KERNEL_CODE_SIGNING
> >  KERNEL_BASE_OPT		+= \
> > -	$(if $(shell cs_get_ca kernel-trusted), \
> > -		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> > +	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> > +		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
> >  endif
> >  ifdef PTXCONF_KERNEL_MODULES_SIGN
> >  KERNEL_BASE_OPT		+= \
> > -	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"'
> > +	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"'
> >  endif
> >  
> >  # Intermediate option. This will be used by kernel module packages.
> > diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> > new file mode 100644
> > index 000000000000..909e8ebd6936
> > --- /dev/null
> > +++ b/rules/pre/030-code-signing-consumers.make
> > @@ -0,0 +1,44 @@
> > +# -*-makefile-*-
> > +#
> > +# Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> > +#
> > +# For further information about the PTXdist project and license conditions
> > +# see the README file.
> > +#
> > +#
> > +
> > +#
> > +# Usage: $(call ptx/cs-consumer-env, <PKG>)
> > +#
> > +# We usually want to use cs-get-* macros inside a <PKG>_MAKE_OPT etc., which is
> > +# referenced in world/env, so we cannot use world/env to set pkg_name without
> > +# running into circular variable dependencies.
> > +#
> > +define ptx/cs-consumer-env
> 
> Use the regular 'ptx/cs-consumer-env =' for this to keep it consistent with
> other env lists.

I don't understand, in v1 [1] you said I should use 'define' instead of
multi-line '=' macros…?

[1]: https://lore.ptxdist.org/ptxdist/20210804152307.GS13108@pengutronix.de/

 - Roland

> > +	pkg_name='$(PTX_MAP_TO_package_$(strip $(1)))' \
> > +	$(CODE_SIGNING_ENV)
> > +endef
> > +
> > +#
> > +# Usage: $(call ptx/cs-get-uri, <PKG>, <role>)
> > +#
> > +define ptx/cs-get-uri
> > +$(strip \
> > +	$(shell \
> > +		$(call ptx/cs-consumer-env, $(1))\
> > +			cs_get_uri '$(strip $(2))'\
> > +	)\
> 
> No \ needed with 'define'.
> 
> > +)
> > +endef
> > +
> > +#
> > +# Usage: $(call ptx/cs-get-ca, <PKG>, <role>)
> > +#
> > +define ptx/cs-get-ca
> > +$(strip \
> > +	$(shell \
> > +		$(call ptx/cs-consumer-env, $(1))\
> > +			cs_get_ca '$(strip $(2))'\
> > +	)\
> 
> Same here.
> 
> > +)
> > +endef
> > diff --git a/rules/rauc.make b/rules/rauc.make
> > index 08df6336a7cd..3c28befcd3ff 100644
> > --- a/rules/rauc.make
> > +++ b/rules/rauc.make
> > @@ -78,7 +78,7 @@ ifdef PTXCONF_RAUC_CONFIGURATION
> >  	@$(call install_replace, rauc, /etc/rauc/system.conf, \
> >  		@RAUC_BUNDLE_COMPATIBLE@, \
> >  		"$(call remove_quotes,$(PTXCONF_RAUC_COMPATIBLE))")
> > -	@$(call install_copy, rauc, 0, 0, 0644, $(shell cs_get_ca update), \
> > +	@$(call install_copy, rauc, 0, 0, 0644, $(call ptx/cs-get-ca, RAUC, update), \
> >  		/etc/rauc/ca.cert.pem)
> >  endif
> >  
> > diff --git a/rules/templates/template-barebox-imx-habv4-make b/rules/templates/template-barebox-imx-habv4-make
> > index cc825dc90292..b2d5d7100fc9 100644
> > --- a/rules/templates/template-barebox-imx-habv4-make
> > +++ b/rules/templates/template-barebox-imx-habv4-make
> > @@ -64,9 +64,9 @@ endif
> >  
> >  BAREBOX_@PACKAGE@_MAKE_ENV	= \
> >  	$(CODE_SIGNING_ENV) \
> > -	CSF="$(shell cs_get_uri imx-habv4-csf1)" \
> > -	IMG="$(shell cs_get_uri imx-habv4-img1)" \
> > -	FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> > +	CSF="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-csf1)" \
> > +	IMG="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-img1)" \
> > +	FIT_KEY="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, image-kernel-fit)"
> >  
> >  BAREBOX_@PACKAGE@_MAKE_OPT	:= $(BAREBOX_@PACKAGE@_CONF_OPT)
> >  
> > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > index 66a2cab81395..24730d3cf742 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 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> >  #
> >  # For further information about the PTXdist project and license conditions
> >  # see the README file.
> > @@ -176,6 +177,12 @@ export -f cs_set_uri
> >  # Get the uri from a role
> >  #
> >  cs_get_uri() {
> > +    if [ -z "${pkg_name}" ]; then
> > +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> > +	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> > +		'Use $(call ptx/cs-get-uri, <PKG>, <role>) instead.'
> > +    fi
> > +
> >      local role="${1}"
> >      cs_init_variables
> >  
> > @@ -297,6 +304,12 @@ export -f cs_import_key_from_pem
> >  # Get the path to the CA in pem format from a role
> >  #
> >  cs_get_ca() {
> > +    if [ -z "${pkg_name}" ]; then
> > +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> > +	    ptxd_bailout '$(shell cs_get_ca, …) is no longer supported in make files.' \
> > +		'Use $(call ptx/cs-get-ca, <PKG>, …) instead.'
> > +    fi
> > +
> >      local role="${1}"
> >      cs_init_variables
> >  
> > -- 
> > 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

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-09-03 13:17   ` Michael Olbrich
@ 2021-09-08 11:43     ` Roland Hieber
  2021-09-12 20:33       ` Roland Hieber
  0 siblings, 1 reply; 19+ messages in thread
From: Roland Hieber @ 2021-09-08 11:43 UTC (permalink / raw)
  To: ptxdist

On Fri, Sep 03, 2021 at 03:17:13PM +0200, Michael Olbrich wrote:
> On Mon, Aug 09, 2021 at 10:06:08AM +0200, Roland Hieber wrote:
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > PATCH v2:
> >  - cs_check_whitelisted: make "needle"  local variable (feedback by
> >    Michael Olbrich)
> >  - cs_check_whitelisted: error out with ERROR_KEY_NOT_WHITELISTED also
> >    if whitelist does not exist yet (Michael Olbrich)
> >  - rename cs_get_uri to cs_get_uri_unchecked and let cs_get_uri wrap it
> >    instead of setting cs_no_whitelist_check=1 (Michael Olbrich)
> >  - docs: simplify introductory example (Michael Olbrich)
> >  - docs: add short paragraph on how to determine fingerprints of certs
> > 
> > PATCH v1: https://lore.ptxdist.org/ptxdist/
> > ---
> >  doc/dev_code_signing.rst                  | 80 +++++++++++++++++++++++
> >  platforms/code-signing.in                 | 22 +++++++
> >  rules/pre/030-code-signing-consumers.make |  6 ++
> >  scripts/lib/ptxd_lib_code_signing.sh      | 71 ++++++++++++++++++--
> >  4 files changed, 172 insertions(+), 7 deletions(-)
> > 
> > diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> > index 413f694980eb..28ebe055346e 100644
> > --- a/doc/dev_code_signing.rst
> > +++ b/doc/dev_code_signing.rst
> > @@ -172,3 +172,83 @@ also via an environment variable.
> >    (``=``, not ``:=``).
> >    Otherwise the variable is expanded before a code signing provider can perform
> >    its setup.
> > +
> > +Key Whitelisting
> > +~~~~~~~~~~~~~~~~
> > +
> > +In some use cases, it may be feasible to do additional checks to make sure that
> > +a package uses the correct key material.
> > +For example, suppose you have a "release" code signing provider which you use
> > +to sign bootloaders for production,
> > +and a "development" code signing provider which you use to sign bootloaders
> > +with an extended feature set, e.g. to allow booting arbitrary kernels and
> > +userlands for debugging purposes.
> > +Your production boards are locked down in hardware so the ROM code only
> > +executes bootloaders signed with the "release" key.
> > +Now you don't want any bootloader with debugging features to be signed with a
> > +release key, otherwise someone could boot them on a locked-down production
> > +device, and use the additional debugging features to get extended access to the
> > +production device.
> > +In this case, key whitelisting can help to prevent signing bootloader packages
> > +with the wrong key.
> > +
> > +When the ``CODE_SIGNING_REQUIRE_WHITELIST`` kconfig symbol is enabled,
> > +the consumer functions :ref:`ptx/cs-get-ca` and :ref:`ptx/cs-get-uri`
> > +look up the triplet of package name, role name, and the pubkey's SHA256
> > +fingerprint in the whitelist whose file name is determined by the
> > +``CODE_SIGNING_WHITELIST_FILENAME`` kconfig symbol (the default path is
> > +``configs/platform-<name>/code-signing-whitelist``).
> > +If a key or a CA is not whitelisted for the package in which it is to be used,
> > +the functions will exit with an error message on the terminal::
> > +
> > +   $ ptxdist -v print KERNEL_MAKE_OPT
> > +   ptxdist: error: SPKI whitelist record 'kernel kernel-modules
> > +   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04' not found in
> > +   distrokit/configs/platform-v7a/code-signing-whitelist
> > +
> > +   …/ptxdist/rules/kernel.make:196: *** cs-get-uri: whitelist check failed, see errors above.  Stop.
> > +
> > +
> > +If ``CODE_SIGNING_REQUIRE_WHITELIST`` is disabled (the default),
> > +all keys and CAs are provided to all packages without further checks.
> > +
> > +The format of the code signing whitelist consists of one triplet per line, in
> > +which the elements of the triplet are separated by whitespace.
> > +If a CA is to be checked, the role name is prefixed with a literal ``ca:``,
> > +and the fingerprint refers to the public key of the certificate.
> > +All other unmatched lines in the file are ignored, but we suggest to use ``#``
> > +to start a line comment so as not to add a whitelist record accidentally.
> > +
> > +For example, here is a whitelist for use with the *devel* code provider which
> > +allows all provided keys to be used by their respective consumers::
> > +
> > +   # format: package-name role-name sha256-pubkey-fingerprint
> > +   kernel      kernel-modules   69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> > +   image-rauc  update           0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> > +   image-rauc  ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> > +   rauc        ca:update        0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> > +
> > +.. note:: The match is case-sensitive, and the fingerprints are expected
> > +   in uppercase.
> > +
> > +   If a CA consists of more than one certificate, all of their fingerprints
> > +   must be whitelisted.
> > +
> > +You can determine the key fingerprints by copying it from the error message,
> > +or with the `spki-hash`__ tool from the ``host-extract-cert`` package,
> > +or with openssl::
> > +
> > +   $ openssl pkey -in keyfile.pem -pubout -outform der \
> > +     | openssl sha256 \
> > +     | tr 'a-z' 'A-Z'
> > +   (STDIN)= 69C9BBB8BB4DFAE74AB21D06DFB5F2C67066373AE545453276847340822CDF04
> > +
> > +or, in the case of certificates::
> > +
> > +   $ openssl x509 -noout -in cert.pem -pubkey \
> > +     | openssl pkey -pubin -inform pem -pubout -outform der \
> > +     | openssl sha256 \
> > +     | tr 'a-z' 'A-Z'
> > +   (STDIN)= 0034F8FE5ADC3B0DFE642407275D144DE2398C68CC9A86DD6703D7151116B44E
> > +
> > +__ https://git.pengutronix.de/cgit/extract-cert/tree/spki-hash.c
> > diff --git a/platforms/code-signing.in b/platforms/code-signing.in
> > index 81f9ef6f3c9e..92f555c7e965 100644
> > --- a/platforms/code-signing.in
> > +++ b/platforms/code-signing.in
> > @@ -20,4 +20,26 @@ source "generated/code_signing_provider.in"
> >  
> >  endchoice
> >  
> > +config CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> > +	bool
> > +	prompt "require whitelisted keys"
> > +	help
> > +	  Every time a key from the code provider is used, check if the consumer
> > +	  is allowed to use it.
> > +
> > +	  Code signing consumers can depend on this option if they want to force
> > +	  the key whitelist check.
> > +
> > +if CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> > +
> > +config CODE_SIGNING_WHITELIST_FILENAME
> > +	string
> > +	prompt "whitelist file name"
> > +	default "code-signing-whitelist"
> > +	help
> > +	  This file name is used for the key whitelist. The path is relative to
> > +	  PTXDIST_PLATFORMCONFIGDIR (e.g. configs/platform-nnn/).
> 
> I think, we can just hard-code this name. No need for a config option.
> Or is there a use-case for this?

Hmm yes. The more adequate use case now is probably to add a new layer
on top instead of using different platformconfigs with different key
whitelists for the same platform. So I'd hard-code it too.

> > +
> > +endif # CODE_SIGNING_REQUIRE_WHITELISTED_KEYS
> > +
> >  endif
> > diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> > index 909e8ebd6936..bca05854372d 100644
> > --- a/rules/pre/030-code-signing-consumers.make
> > +++ b/rules/pre/030-code-signing-consumers.make
> > @@ -28,6 +28,9 @@ $(strip \
> >  		$(call ptx/cs-consumer-env, $(1))\
> >  			cs_get_uri '$(strip $(2))'\
> >  	)\
> > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > +		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
> > +	)\
> 
> You cannot use ptx/error here. That macro can only be used for stuff that
> is evaluated before the first target is executed. Otherwise the error
> message will get lost.
> You need to use $(error ...) here. Probably with the same $(shell :) hack
> that is used in kernel.make.

Okay.

> >  )
> >  endef
> >  
> > @@ -40,5 +43,8 @@ $(strip \
> >  		$(call ptx/cs-consumer-env, $(1))\
> >  			cs_get_ca '$(strip $(2))'\
> >  	)\
> > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > +		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
> > +	)\
> 
> same here.
> 
> >  )
> >  endef
> > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > index 24730d3cf742..c855821bd039 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) 2020 Jan Luebbe <j.luebbe@pengutronix.de>
> >  # Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> >  #
> >  # For further information about the PTXdist project and license conditions
> > @@ -70,6 +71,7 @@ cs_init_variables() {
> >      sysroot="$(ptxd_get_ptxconf PTXCONF_SYSROOT_HOST)"
> >      keyprovider="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_PROVIDER)"
> >      keydir="${sysroot}/var/lib/keys/${keyprovider}"
> > +    whitelist="$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_WHITELIST_FILENAME || true)"
> >  }
> >  export -f cs_init_variables
> >  
> > @@ -157,6 +159,42 @@ cs_group_get_roles() {
> >  }
> >  export -f cs_group_get_roles
> >  
> > +#
> > +# cs_check_whitelisted <role> <uri/pem or fingerprint prefixed with "sha256:">
> > +#
> > +# Checks if the SPKI (Subject Public Key Info) Hash is in the whitelist
> > +#
> > +cs_check_whitelisted() {
> > +    local role="${1:-ERROR_ROLE_IS_EMPTY}"
> > +    local src="${2}"
> > +    cs_init_variables
> > +
> > +    if [ "$(ptxd_get_ptxconf PTXCONF_CODE_SIGNING_REQUIRE_WHITELISTED_KEYS)" != "y" ]; then
> > +	return
> > +    fi
> > +
> > +    if ! ptxd_in_path PTXDIST_PATH_PLATFORMCONFIGDIR "${whitelist}"; then
> > +	echo ERROR_KEY_NOT_WHITELISTED
> > +	ptxd_bailout "${pkg_name}: SPKI hash whitelist check for ${role} (${src}) is required, but configs/<platform>/${whitelist} is missing."
> > +    fi
> > +    local whitelist_path="${ptxd_reply}"
> > +
> > +    local hash
> > +    if [ "${src#sha256:}" != "${src}" ]; then
> > +	hash="${src#sha256:}"
> > +    else
> > +	hash=$(ptxd_exec_silent_stderr spki-hash "${src}")
> > +    fi
> > +    hash=${hash:-ERROR_HASH_IS_EMPTY}
> > +    local needle="${pkg_name}\s\+${role}\s\+${hash}"
> > +
> > +    if ! grep -q --line-regexp "${needle}" "${whitelist_path}"; then
> > +	echo ERROR_KEY_NOT_WHITELISTED
> > +	ptxd_bailout "SPKI whitelist record '${pkg_name} ${role} ${hash}' not found in $(ptxd_print_path "${whitelist_path}")"
> > +    fi
> > +}
> > +export -f cs_check_whitelisted
> > +
> >  #
> >  # cs_set_uri <role> <uri>
> >  #
> > @@ -171,12 +209,8 @@ cs_set_uri() {
> >  }
> >  export -f cs_set_uri
> >  
> > -#
> > -# cs_get_uri <role>
> > -#
> > -# Get the uri from a role
> > -#
> > -cs_get_uri() {
> > +# internal helper for cs_get_uri
> > +cs_get_uri_unchecked() {
> >      if [ -z "${pkg_name}" ]; then
> >  	    echo ERROR_UNSUPPORTED_CS_API_CALL
> >  	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> > @@ -198,8 +232,24 @@ cs_get_uri() {
> >  	    return
> >  	fi
> >      fi
> > +
> >      cat "${keydir}/${role}/uri"
> >  }
> > +export -f cs_get_uri_unchecked
> > +
> > +#
> > +# cs_get_uri <role>
> > +#
> > +# Get the uri from a role
> > +#
> > +cs_get_uri() {
> > +    local role="${1}"
> > +    local uri=$(cs_get_uri_unchecked "$1")
> > +
> > +    if cs_check_whitelisted "${role}" "${uri}"; then
> > +	echo "${uri}"
> > +    fi
> > +}
> >  export -f cs_get_uri
> >  
> >  #
> > @@ -321,6 +371,9 @@ cs_get_ca() {
> >      fi
> >  
> >      if [ -e "${ca}" ]; then
> > +	while read fp; do
> > +	    cs_check_whitelisted "ca:${role}" "sha256:${fp}"
> > +	done < "${keydir}/${role}/ca.fingerprints"
> >  	echo "${ca}"
> >      fi
> >  }
> > @@ -339,6 +392,9 @@ cs_append_ca_from_pem() {
> >      cat "${pem}" >> "${keydir}/${role}/ca.pem"
> >      # add new line in case ${pem} does not end with an EOL
> >      echo >> "${keydir}/${role}/ca.pem"
> > +
> > +    openssl x509 -in "${pem}" -inform pem -noout -pubkey | \
> > +	spki-hash /dev/stdin >> "${keydir}/${role}/ca.fingerprints"
> 
> This should happen first, before the extending ca.pem and make sure to
> abort on error. Use check_pipe_status to catch errors from openssl.

Okay.

> >  }
> >  export -f cs_append_ca_from_pem
> >  
> > @@ -370,7 +426,8 @@ cs_append_ca_from_uri() {
> >      cs_init_variables
> >  
> >      if [ -z "${uri}" ]; then
> > -	uri=$(cs_get_uri "${role}")
> > +	# cs_append_ca_from_der will check this later
> 
> I don't think this comment is correct. There is no checking while adding to
> the CA, right?

Yes, it's outdated. cs_append_ca_from_der calls cs_append_ca_from_pem,
which extracts the fingerprints, but the checking happens in cs_get_ca.

 - Roland

> Michael
> 
> > +	uri=$(cs_get_uri_unchecked "${role}")
> >      fi
> >  
> >      ptxd_exec extract-cert "${uri}" "${tmpdir}/ca.der" &&
> > -- 
> > 2.30.2

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs
  2021-09-08 11:27     ` Roland Hieber
@ 2021-09-08 14:01       ` Michael Olbrich
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Olbrich @ 2021-09-08 14:01 UTC (permalink / raw)
  To: Roland Hieber; +Cc: ptxdist

On Wed, Sep 08, 2021 at 01:27:11PM +0200, Roland Hieber wrote:
> On Fri, Sep 03, 2021 at 02:46:46PM +0200, Michael Olbrich wrote:
> > On Mon, Aug 09, 2021 at 10:06:06AM +0200, Roland Hieber wrote:
> > > We'll need this type of function more often later.
> > 
> > I don't see another user of this function in the rest of the series.
> 
> Huh yes. I think I used it multiple times in a previous version of the
> series. I think this patch can be dropped.
> 
> > 
> > > 
> > > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > > ---
> > > PATCH v2: no changes
> > > 
> > > PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-3-rhi@pengutronix.de
> > > ---
> > >  scripts/lib/ptxd_lib_code_signing.sh | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > > index 5ba1a4666af4..66a2cab81395 100644
> > > --- a/scripts/lib/ptxd_lib_code_signing.sh
> > > +++ b/scripts/lib/ptxd_lib_code_signing.sh
> > > @@ -49,6 +49,17 @@ softhsm_pkcs11_tool() {
> > >  }
> > >  export -f softhsm_pkcs11_tool
> > >  
> > > +#
> > > +# softhsm_pkcs11_uri <uri>
> > > +#
> > > +# Add the SoftHSM PIN to the given URI.
> > > +#
> > > +softhsm_pkcs11_uri() {
> > > +    local role="$1"
> > 
> > Why is 'role' passed as argument and 'keyprovider' is not?
> > 
> > > +    printf "pkcs11:token=%s;object=%s;pin-value=1111\n" "${keyprovider}" "${role}"
> > 
> > Why not just:
> > 
> >     echo "pkcs11:token=${keyprovider};object=${role};pin-value=1111"
> 
> Force of habit from using C and Python. And depending on the actual echo
> implementation (POSIX sh, bash, or /bin/echo), there are different
> behaviours regarding things like printing a literal -e, or interpolation
> of \r, \t etc., and I've never encountered this with printf. So I
> usually use printf instead of echo.

In ptxdist, the shell is always bash and we use 'echo' everywhere else, so
I prefer it here as well.

Michael

> > > +}
> > > +export -f softhsm_pkcs11_uri
> > > +
> > >  #
> > >  # cs_init_variables
> > >  #
> > > @@ -95,7 +106,8 @@ cs_define_role() {
> > >  
> > >      mkdir -p "${keydir}/${role}" &&
> > >      # default for SoftHSM
> > > -    cs_set_uri "${role}" "pkcs11:token=${keyprovider};object=${role};pin-value=1111"
> > > +    local uri=$(softhsm_pkcs11_uri "${role}")
> > 
> > Why the extra local variable?
> > Michael
> > 
> > > +    cs_set_uri "${role}" "${uri}"
> > >  }
> > >  export -f cs_define_role
> > >  
> > > -- 
> > > 2.30.2
> 
> -- 
> Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
> Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment
  2021-09-08 11:30     ` Roland Hieber
@ 2021-09-08 14:08       ` Michael Olbrich
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Olbrich @ 2021-09-08 14:08 UTC (permalink / raw)
  To: Roland Hieber; +Cc: ptxdist

On Wed, Sep 08, 2021 at 01:30:01PM +0200, Roland Hieber wrote:
> On Fri, Sep 03, 2021 at 02:54:31PM +0200, Michael Olbrich wrote:
> > On Mon, Aug 09, 2021 at 10:06:07AM +0200, Roland Hieber wrote:
> > > The code signing consumer functions should be able to retrieve some
> > > information about the recipe in which they were called in order to make
> > > additional checks if needed. Refactor the (shell cs_get_*, …) calls into
> > > macro calls of the form $(call ptx/cs-get-*, <PKG>, …). Let these
> > > macros look up the package name (for now) from PTX_MAP_TO_package_<PKG>
> > > before passing it to the shell functions. Using $(call world/env) here
> > > would be practical, but would also cause make to complain about
> > > recursive variable dependencies. Therefore variables must be added
> > > to ptx/cs-consumer-env manually, but additional information can be added
> > > later if needed.
> > > 
> > > Refactor the existing consumers in the code base too, and add an error
> > > message in case anyone else that still uses the old API.
> > > 
> > > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > > ---
> > > PATCH v2:
> > >  - define multiline macros using "define"
> > > 
> > > PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-4-rhi@pengutronix.de
> > > ---
> > >  doc/dev_code_signing.rst                      |  2 +-
> > >  doc/ref_code_signing_helpers.rst              | 25 ++++++-----
> > >  rules/barebox.make                            |  2 +-
> > >  rules/image-rauc.make                         |  6 +--
> > >  rules/kernel.make                             |  6 +--
> > >  rules/pre/030-code-signing-consumers.make     | 44 +++++++++++++++++++
> > >  rules/rauc.make                               |  2 +-
> > >  .../templates/template-barebox-imx-habv4-make |  6 +--
> > >  scripts/lib/ptxd_lib_code_signing.sh          | 13 ++++++
> > >  9 files changed, 83 insertions(+), 23 deletions(-)
> > >  create mode 100644 rules/pre/030-code-signing-consumers.make
> > > 
> > > diff --git a/doc/dev_code_signing.rst b/doc/dev_code_signing.rst
> > > index b9a7c42f2a55..413f694980eb 100644
> > > --- a/doc/dev_code_signing.rst
> > > +++ b/doc/dev_code_signing.rst
> > > @@ -164,7 +164,7 @@ also via an environment variable.
> > >  .. code-block:: none
> > >  
> > >      $(call install_copy, rauc, 0, 0, 0644, \
> > > -      $(shell cs_get_ca update), \
> > > +      $(call ptx/cs-get-ca, RAUC, update), \
> > >        /etc/rauc/ca.cert.pem)
> > >  
> > >  .. note:: When code signing helper functions are used in make variables (e.g.
> > > diff --git a/doc/ref_code_signing_helpers.rst b/doc/ref_code_signing_helpers.rst
> > > index fd16ca763557..d3429778d94d 100644
> > > --- a/doc/ref_code_signing_helpers.rst
> > > +++ b/doc/ref_code_signing_helpers.rst
> > > @@ -297,19 +297,21 @@ In the example given in :ref:`cs_group_add_roles` above, this would print::
> > >  Consumer Functions
> > >  ~~~~~~~~~~~~~~~~~~
> > >  
> > > +The consumer functions are implemented as make macros.
> > >  Packages that want to sign something or need access to keys/CAs can retrieve
> > >  PKCS#11 URIs and CA keyrings with these helpers.
> > >  
> > > +.. _ptx/cs-get-uri:
> > >  .. _cs_get_uri:
> > >  
> > > -cs_get_uri
> > > -^^^^^^^^^^
> > > +ptx/cs-get-uri
> > > +^^^^^^^^^^^^^^
> > >  
> > >  Usage:
> > >  
> > > -.. code-block:: bash
> > > +.. code-block:: make
> > >  
> > > -    cs_get_uri <role>
> > > +    $(call ptx/cs-get-uri, <PKG>, <role>)
> > >  
> > >  Get PKCS#11 URI for role.
> > >  
> > > @@ -317,16 +319,17 @@ Preconditions:
> > >  
> > >  - the URI must have been set (see :ref:`cs_set_uri`)
> > >  
> > > +.. _ptx/cs-get-ca:
> > >  .. _cs_get_ca:
> > >  
> > > -cs_get_ca
> > > -^^^^^^^^^
> > > +ptx/cs-get-ca
> > > +^^^^^^^^^^^^^
> > >  
> > >  Usage:
> > >  
> > > -.. code-block:: bash
> > > +.. code-block:: make
> > >  
> > > -    cs_get_ca <role>
> > > +    $(call ptx/cs-get-ca, <PKG>, <role>)
> > >  
> > >  Get path to the CA keyring in PEM format for role.
> > >  
> > > @@ -347,7 +350,7 @@ Example:
> > >  
> > >     # set up kernel module signing, and add a trusted CA if the provider set one
> > >     KERNEL_SIGN_OPT =
> > > -   	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"' \
> > > +   	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"' \
> > >     	CONFIG_MODULE_SIG_ALL=y \
> > > -   	$(if $(shell cs_get_ca kernel-trusted), \
> > > -   		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> > > +   	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> > > +   		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
> > > diff --git a/rules/barebox.make b/rules/barebox.make
> > > index bea9f3adcbf8..983d34032e0d 100644
> > > --- a/rules/barebox.make
> > > +++ b/rules/barebox.make
> > > @@ -103,7 +103,7 @@ endif
> > >  ifdef PTXCONF_CODE_SIGNING
> > >  BAREBOX_MAKE_ENV = \
> > >  	$(CODE_SIGNING_ENV) \
> > > -	IMAGE_KERNEL_FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> > > +	IMAGE_KERNEL_FIT_KEY="$(call ptx/cs-get-uri, BAREBOX, image-kernel-fit)"
> > >  endif
> > >  
> > >  $(STATEDIR)/barebox.compile:
> > > diff --git a/rules/image-rauc.make b/rules/image-rauc.make
> > > index fe1b0e89be7c..c8747231f8f1 100644
> > > --- a/rules/image-rauc.make
> > > +++ b/rules/image-rauc.make
> > > @@ -32,9 +32,9 @@ IMAGE_RAUC_ENV	= \
> > >  	RAUC_BUNDLE_VERSION="$(call remove_quotes, $(PTXCONF_RAUC_BUNDLE_VERSION))" \
> > >  	RAUC_BUNDLE_BUILD=$(call ptx/sh, date +%FT%T%z) \
> > >  	RAUC_BUNDLE_DESCRIPTION=$(PTXCONF_IMAGE_RAUC_DESCRIPTION) \
> > > -	RAUC_KEY="$(shell cs_get_uri update)" \
> > > -	RAUC_CERT="$(shell cs_get_uri update)" \
> > > -	RAUC_KEYRING="$(shell cs_get_ca update)"
> > > +	RAUC_KEY="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> > > +	RAUC_CERT="$(call ptx/cs-get-uri, IMAGE_RAUC, update)" \
> > > +	RAUC_KEYRING="$(call ptx/cs-get-ca, IMAGE_RAUC, update)"
> > >  
> > >  $(IMAGE_RAUC_IMAGE):
> > >  	@$(call targetinfo)
> > > diff --git a/rules/kernel.make b/rules/kernel.make
> > > index 9caff677918e..e6faba82df38 100644
> > > --- a/rules/kernel.make
> > > +++ b/rules/kernel.make
> > > @@ -73,12 +73,12 @@ KERNEL_BASE_OPT		= \
> > >  
> > >  ifdef PTXCONF_KERNEL_CODE_SIGNING
> > >  KERNEL_BASE_OPT		+= \
> > > -	$(if $(shell cs_get_ca kernel-trusted), \
> > > -		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> > > +	$(if $(call ptx/cs-get-ca, KERNEL, kernel-trusted), \
> > > +		CONFIG_SYSTEM_TRUSTED_KEYS=$(call ptx/cs-get-ca, KERNEL, kernel-trusted))
> > >  endif
> > >  ifdef PTXCONF_KERNEL_MODULES_SIGN
> > >  KERNEL_BASE_OPT		+= \
> > > -	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"'
> > > +	CONFIG_MODULE_SIG_KEY='"$(call ptx/cs-get-uri, KERNEL, kernel-modules)"'
> > >  endif
> > >  
> > >  # Intermediate option. This will be used by kernel module packages.
> > > diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> > > new file mode 100644
> > > index 000000000000..909e8ebd6936
> > > --- /dev/null
> > > +++ b/rules/pre/030-code-signing-consumers.make
> > > @@ -0,0 +1,44 @@
> > > +# -*-makefile-*-
> > > +#
> > > +# Copyright (C) 2021 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> > > +#
> > > +# For further information about the PTXdist project and license conditions
> > > +# see the README file.
> > > +#
> > > +#
> > > +
> > > +#
> > > +# Usage: $(call ptx/cs-consumer-env, <PKG>)
> > > +#
> > > +# We usually want to use cs-get-* macros inside a <PKG>_MAKE_OPT etc., which is
> > > +# referenced in world/env, so we cannot use world/env to set pkg_name without
> > > +# running into circular variable dependencies.
> > > +#
> > > +define ptx/cs-consumer-env
> > 
> > Use the regular 'ptx/cs-consumer-env =' for this to keep it consistent with
> > other env lists.
> 
> I don't understand, in v1 [1] you said I should use 'define' instead of
> multi-line '=' macros…?

I meant for ptx/cs-get-uri and ptx/cs-get-ca but not for
ptx/cs-consumer-env which is just a variable list. It's just for
consistency. We've had the variable list like that since the beginning.
And I prefer 'define' for other complex macros because the nested brackets
are complex enough without adding the '\' in each line.

Michael

> [1]: https://lore.ptxdist.org/ptxdist/20210804152307.GS13108@pengutronix.de/
> 
>  - Roland
> 
> > > +	pkg_name='$(PTX_MAP_TO_package_$(strip $(1)))' \
> > > +	$(CODE_SIGNING_ENV)
> > > +endef
> > > +
> > > +#
> > > +# Usage: $(call ptx/cs-get-uri, <PKG>, <role>)
> > > +#
> > > +define ptx/cs-get-uri
> > > +$(strip \
> > > +	$(shell \
> > > +		$(call ptx/cs-consumer-env, $(1))\
> > > +			cs_get_uri '$(strip $(2))'\
> > > +	)\
> > 
> > No \ needed with 'define'.
> > 
> > > +)
> > > +endef
> > > +
> > > +#
> > > +# Usage: $(call ptx/cs-get-ca, <PKG>, <role>)
> > > +#
> > > +define ptx/cs-get-ca
> > > +$(strip \
> > > +	$(shell \
> > > +		$(call ptx/cs-consumer-env, $(1))\
> > > +			cs_get_ca '$(strip $(2))'\
> > > +	)\
> > 
> > Same here.
> > 
> > > +)
> > > +endef
> > > diff --git a/rules/rauc.make b/rules/rauc.make
> > > index 08df6336a7cd..3c28befcd3ff 100644
> > > --- a/rules/rauc.make
> > > +++ b/rules/rauc.make
> > > @@ -78,7 +78,7 @@ ifdef PTXCONF_RAUC_CONFIGURATION
> > >  	@$(call install_replace, rauc, /etc/rauc/system.conf, \
> > >  		@RAUC_BUNDLE_COMPATIBLE@, \
> > >  		"$(call remove_quotes,$(PTXCONF_RAUC_COMPATIBLE))")
> > > -	@$(call install_copy, rauc, 0, 0, 0644, $(shell cs_get_ca update), \
> > > +	@$(call install_copy, rauc, 0, 0, 0644, $(call ptx/cs-get-ca, RAUC, update), \
> > >  		/etc/rauc/ca.cert.pem)
> > >  endif
> > >  
> > > diff --git a/rules/templates/template-barebox-imx-habv4-make b/rules/templates/template-barebox-imx-habv4-make
> > > index cc825dc90292..b2d5d7100fc9 100644
> > > --- a/rules/templates/template-barebox-imx-habv4-make
> > > +++ b/rules/templates/template-barebox-imx-habv4-make
> > > @@ -64,9 +64,9 @@ endif
> > >  
> > >  BAREBOX_@PACKAGE@_MAKE_ENV	= \
> > >  	$(CODE_SIGNING_ENV) \
> > > -	CSF="$(shell cs_get_uri imx-habv4-csf1)" \
> > > -	IMG="$(shell cs_get_uri imx-habv4-img1)" \
> > > -	FIT_KEY="$(shell cs_get_uri image-kernel-fit)"
> > > +	CSF="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-csf1)" \
> > > +	IMG="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, imx-habv4-img1)" \
> > > +	FIT_KEY="$(call ptx/cs-get-uri, BAREBOX_@PACKAGE@, image-kernel-fit)"
> > >  
> > >  BAREBOX_@PACKAGE@_MAKE_OPT	:= $(BAREBOX_@PACKAGE@_CONF_OPT)
> > >  
> > > diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> > > index 66a2cab81395..24730d3cf742 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 Roland Hieber, Pengutronix <rhi@pengutronix.de>
> > >  #
> > >  # For further information about the PTXdist project and license conditions
> > >  # see the README file.
> > > @@ -176,6 +177,12 @@ export -f cs_set_uri
> > >  # Get the uri from a role
> > >  #
> > >  cs_get_uri() {
> > > +    if [ -z "${pkg_name}" ]; then
> > > +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> > > +	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> > > +		'Use $(call ptx/cs-get-uri, <PKG>, <role>) instead.'
> > > +    fi
> > > +
> > >      local role="${1}"
> > >      cs_init_variables
> > >  
> > > @@ -297,6 +304,12 @@ export -f cs_import_key_from_pem
> > >  # Get the path to the CA in pem format from a role
> > >  #
> > >  cs_get_ca() {
> > > +    if [ -z "${pkg_name}" ]; then
> > > +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> > > +	    ptxd_bailout '$(shell cs_get_ca, …) is no longer supported in make files.' \
> > > +		'Use $(call ptx/cs-get-ca, <PKG>, …) instead.'
> > > +    fi
> > > +
> > >      local role="${1}"
> > >      cs_init_variables
> > >  
> > > -- 
> > > 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
> 
> -- 
> Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
> Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment
  2021-08-09  8:06 ` [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment Roland Hieber
  2021-09-03 12:54   ` Michael Olbrich
@ 2021-09-08 20:53   ` Roland Hieber
  1 sibling, 0 replies; 19+ messages in thread
From: Roland Hieber @ 2021-09-08 20:53 UTC (permalink / raw)
  To: ptxdist

On Mon, Aug 09, 2021 at 10:06:07AM +0200, Roland Hieber wrote:
> The code signing consumer functions should be able to retrieve some
> information about the recipe in which they were called in order to make
> additional checks if needed. Refactor the (shell cs_get_*, …) calls into
> macro calls of the form $(call ptx/cs-get-*, <PKG>, …). Let these
> macros look up the package name (for now) from PTX_MAP_TO_package_<PKG>
> before passing it to the shell functions. Using $(call world/env) here
> would be practical, but would also cause make to complain about
> recursive variable dependencies. Therefore variables must be added
> to ptx/cs-consumer-env manually, but additional information can be added
> later if needed.
> 
> Refactor the existing consumers in the code base too, and add an error
> message in case anyone else that still uses the old API.
> 
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>  - define multiline macros using "define"
> 
> PATCH v1: https://lore.ptxdist.org/ptxdist/20210804142330.32739-4-rhi@pengutronix.de
> ---
>  doc/dev_code_signing.rst                      |  2 +-
>  doc/ref_code_signing_helpers.rst              | 25 ++++++-----
>  rules/barebox.make                            |  2 +-
>  rules/image-rauc.make                         |  6 +--
>  rules/kernel.make                             |  6 +--
>  rules/pre/030-code-signing-consumers.make     | 44 +++++++++++++++++++
>  rules/rauc.make                               |  2 +-
>  .../templates/template-barebox-imx-habv4-make |  6 +--
>  scripts/lib/ptxd_lib_code_signing.sh          | 13 ++++++
>  9 files changed, 83 insertions(+), 23 deletions(-)
>  create mode 100644 rules/pre/030-code-signing-consumers.make
> 
[...]
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 66a2cab81395..24730d3cf742 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 Roland Hieber, Pengutronix <rhi@pengutronix.de>
>  #
>  # For further information about the PTXdist project and license conditions
>  # see the README file.
> @@ -176,6 +177,12 @@ export -f cs_set_uri
>  # Get the uri from a role
>  #
>  cs_get_uri() {
> +    if [ -z "${pkg_name}" ]; then
> +	    echo ERROR_UNSUPPORTED_CS_API_CALL
> +	    ptxd_bailout '$(shell cs_get_uri, <role>) is no longer supported in make files.' \
> +		'Use $(call ptx/cs-get-uri, <PKG>, <role>) instead.'
> +    fi
> +
>      local role="${1}"
>      cs_init_variables

Mmh, cs_append_ca_* calls cs_get_uri too and now fails because pkg_name
is empty… I did not catch this in my tests because I never re-installed
the code provider. This needs another workaround in v3.

 - Roland

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-09-08 11:43     ` Roland Hieber
@ 2021-09-12 20:33       ` Roland Hieber
  2021-09-29 11:51         ` Michael Olbrich
  0 siblings, 1 reply; 19+ messages in thread
From: Roland Hieber @ 2021-09-12 20:33 UTC (permalink / raw)
  To: ptxdist

On Wed, Sep 08, 2021 at 01:43:59PM +0200, Roland Hieber wrote:
> On Fri, Sep 03, 2021 at 03:17:13PM +0200, Michael Olbrich wrote:
> > On Mon, Aug 09, 2021 at 10:06:08AM +0200, Roland Hieber wrote:
> > > diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> > > index 909e8ebd6936..bca05854372d 100644
> > > --- a/rules/pre/030-code-signing-consumers.make
> > > +++ b/rules/pre/030-code-signing-consumers.make
> > > @@ -28,6 +28,9 @@ $(strip \
> > >  		$(call ptx/cs-consumer-env, $(1))\
> > >  			cs_get_uri '$(strip $(2))'\
> > >  	)\
> > > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > > +		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
> > > +	)\
> > 
> > You cannot use ptx/error here. That macro can only be used for stuff that
> > is evaluated before the first target is executed. Otherwise the error
> > message will get lost.
> > You need to use $(error ...) here. Probably with the same $(shell :) hack
> > that is used in kernel.make.
> 
> Okay.

The $(shell :) hack was apparently not necessary, but there was some
fallout; as soon as I changed it to $(error), I noticed that it errors
all over the place when building a BSP from scratch, so I had to make a
few more fixes for variables that are expanded before the code signing
provider is installed.

> > >  )
> > >  endef
> > >  
> > > @@ -40,5 +43,8 @@ $(strip \
> > >  		$(call ptx/cs-consumer-env, $(1))\
> > >  			cs_get_ca '$(strip $(2))'\
> > >  	)\
> > > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > > +		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
> > > +	)\
> > 
> > same here.

 - Roland


-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://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] 19+ messages in thread

* Re: [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks
  2021-09-12 20:33       ` Roland Hieber
@ 2021-09-29 11:51         ` Michael Olbrich
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Olbrich @ 2021-09-29 11:51 UTC (permalink / raw)
  To: Roland Hieber; +Cc: ptxdist

On Sun, Sep 12, 2021 at 10:33:29PM +0200, Roland Hieber wrote:
> On Wed, Sep 08, 2021 at 01:43:59PM +0200, Roland Hieber wrote:
> > On Fri, Sep 03, 2021 at 03:17:13PM +0200, Michael Olbrich wrote:
> > > On Mon, Aug 09, 2021 at 10:06:08AM +0200, Roland Hieber wrote:
> > > > diff --git a/rules/pre/030-code-signing-consumers.make b/rules/pre/030-code-signing-consumers.make
> > > > index 909e8ebd6936..bca05854372d 100644
> > > > --- a/rules/pre/030-code-signing-consumers.make
> > > > +++ b/rules/pre/030-code-signing-consumers.make
> > > > @@ -28,6 +28,9 @@ $(strip \
> > > >  		$(call ptx/cs-consumer-env, $(1))\
> > > >  			cs_get_uri '$(strip $(2))'\
> > > >  	)\
> > > > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > > > +		$(call ptx/error, cs-get-uri: whitelist check failed – see errors above)\
> > > > +	)\
> > > 
> > > You cannot use ptx/error here. That macro can only be used for stuff that
> > > is evaluated before the first target is executed. Otherwise the error
> > > message will get lost.
> > > You need to use $(error ...) here. Probably with the same $(shell :) hack
> > > that is used in kernel.make.
> > 
> > Okay.
> 
> The $(shell :) hack was apparently not necessary, but there was some

Are you sure? The difference is only visible, for quiet parallel builds, I
think.

Michael

> fallout; as soon as I changed it to $(error), I noticed that it errors
> all over the place when building a BSP from scratch, so I had to make a
> few more fixes for variables that are expanded before the code signing
> provider is installed.
> 
> > > >  )
> > > >  endef
> > > >  
> > > > @@ -40,5 +43,8 @@ $(strip \
> > > >  		$(call ptx/cs-consumer-env, $(1))\
> > > >  			cs_get_ca '$(strip $(2))'\
> > > >  	)\
> > > > +	$(if $(filter-out 0,$(.SHELLSTATUS)),\
> > > > +		$(call ptx/error, cs-get-ca: whitelist check failed – see errors above)\
> > > > +	)\
> > > 
> > > same here.
> 
>  - Roland
> 
> 
> -- 
> Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
> Steuerwalder Str. 21                     | https://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] 19+ messages in thread

end of thread, other threads:[~2021-09-29 11:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09  8:06 [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Roland Hieber
2021-08-09  8:06 ` [ptxdist] [PATCH v2 2/5] libptxdist: introduce ptxd_exec_silent_stderr Roland Hieber
2021-08-09  8:06 ` [ptxdist] [PATCH v2 3/5] ptxd_lib_code_signing: refactor hard-coded SoftHSM PIN in PKCS11 URIs Roland Hieber
2021-09-03 12:46   ` Michael Olbrich
2021-09-08 11:27     ` Roland Hieber
2021-09-08 14:01       ` Michael Olbrich
2021-08-09  8:06 ` [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment Roland Hieber
2021-09-03 12:54   ` Michael Olbrich
2021-09-08 11:30     ` Roland Hieber
2021-09-08 14:08       ` Michael Olbrich
2021-09-08 20:53   ` Roland Hieber
2021-08-09  8:06 ` [ptxdist] [PATCH v2 5/5] ptxd_lib_code_signing: add key whitelist checks Roland Hieber
2021-08-09  9:30   ` Roland Hieber
2021-09-03 13:17   ` Michael Olbrich
2021-09-08 11:43     ` Roland Hieber
2021-09-12 20:33       ` Roland Hieber
2021-09-29 11:51         ` Michael Olbrich
2021-09-03 12:48 ` [ptxdist] [PATCH v2 1/5] ptxd_make_world_common: make the package name available to scripts Michael Olbrich
2021-09-08 10:17   ` Roland Hieber

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