Re: [ptxdist] [PATCH v2 3/4] ptxd_lib_code_signing: let providers clean up their installed files

[Bastian Krause <bst@pengutronix.de>]

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

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

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

Shouldn't this be "cs_clean_softhsm"?

Regards,
Bastian

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


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

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