Re: [ptxdist] [PATCH v2 4/5] ptxd_lib_code_signing: provide consumer functions with some environment

[Roland Hieber <rhi@pengutronix.de>]

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