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

[Michael Olbrich <m.olbrich@pengutronix.de>]

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

This path is unique to each provider...

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

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

No need for a separate function.

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

Michael

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

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

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