Re: [ptxdist] [PATCH v3 1/5] ptxd_lib_code_signing: cs_get_ca(): improve error handling

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

On Thu, Jul 08, 2021 at 10:39:37PM +0200, Roland Hieber wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> This patch changes cs_get_ca() to only output the CA if it actually
> exists, or print an error and return 1 instead. This makes it possible
> to use make's $(if $(filter-out, ERROR_CA_NOT_YET_SET, ...))
> conditional.
> 
> Co-authored-by: Roland Hieber <rhi@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v3:
>  - correctly check for existence of ${keydir} instead of ${ca} (feedback
>    from Michael Olbrich)
>  - drop controversial re-indentation patches 6/7 and 7/7 from the series
> 
> PATCH v2 (rhi): https://lore.ptxdist.org/ptxdist/20210627231121.28313-1-rhi@pengutronix.de
>  - reorder from PATCH 3/n to PATCH 1/n
>  - echo "ERROR_CA_NOT_YET_SET" in case of error (feedback from Michael
>    Olbrich) and also return 1
> 
> PATCH v1 (mkl): https://lore.ptxdist.org/ptxdist/20210412161900.2376802-3-mkl@pengutronix.de
> ---
>  scripts/lib/ptxd_lib_code_signing.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/ptxd_lib_code_signing.sh b/scripts/lib/ptxd_lib_code_signing.sh
> index 3e1654bb36e4..199f679ef828 100644
> --- a/scripts/lib/ptxd_lib_code_signing.sh
> +++ b/scripts/lib/ptxd_lib_code_signing.sh
> @@ -243,7 +243,13 @@ cs_get_ca() {
>      local role="${1}"
>      cs_init_variables
>  
> -    echo "${keydir}/${role}/ca.pem"
> +    local ca="${keydir}/${role}/ca.pem"
> +
> +    if [ ! -d "${keydir}" ]; then
> +	echo "ERROR_CA_NOT_YET_SET"
> +	return 1
> +    fi

So this is not what we want here. Sorry, I didn't notice this in the last
version. The idea is this:

If the keydir does not exist, then cs_get_ca)() was evaluated too early. So
the check above should be added as it is here.
What's now missing is what Marc originally intended and was part of the
first version of the patch:
If the keydir exists but no CA, then there will never be a CA and we want
to match that. So this should be there as well:

    if [ -e "${ca}" ]; then
	echo "${ca}"
    fi

instead of this:

> +    echo "${ca}"

Now we can do $(if $(shell cs_get_ca ...), ...) to do something only if the
CA exists.

Marc, that was the use-case, right?

Michael

>  }
>  export -f cs_get_ca
>  
> -- 
> 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