On 25-08-06, Sascha Hauer wrote:
> On Tue, Aug 05, 2025 at 10:07:54PM +0200, Marco Felsch wrote:
> > On 25-08-05, Sascha Hauer wrote:
> > > +Cc Basti
> > >
> > > On Thu, Jul 24, 2025 at 03:50:56PM +0200, Marco Felsch wrote:
> > > > Currently the env provided keyspec is resolved during the gen_key() step
> > > > by making use of the try_resolve_env(). This is wrong because it will
> > > > set the complete <hint>:<key> keyspec string for the 'keyname' and
> > > > 'path'.
> > > >
> > > > To fix this the resolve step must happen during the main-loop as first
> > > > step
> > > > because the main-loop is doing the 'keyname' and 'path' split already.
> > > >
> > > > Signed-off-by: Marco Felsch <m.fel...@pengutronix.de>
> > > > ---
> > > > scripts/keytoc.c | 13 +++++--------
> > > > 1 file changed, 5 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/scripts/keytoc.c b/scripts/keytoc.c
> > > > index 617317d6607e..105fa4b5413d 100644
> > > > --- a/scripts/keytoc.c
> > > > +++ b/scripts/keytoc.c
> > > > @@ -615,10 +615,6 @@ static int gen_key(const char *keyname, const char
> > > > *path)
> > > > char *tmp, *key_name_c;
> > > >
> > > > /* key name handling */
> > > > - keyname = try_resolve_env(keyname);
> > > > - if (!keyname)
> > > > - exit(1);
> > > > -
> > > > tmp = key_name_c = strdup(keyname);
> > > >
> > > > while (*tmp) {
> > > > @@ -628,10 +624,6 @@ static int gen_key(const char *keyname, const char
> > > > *path)
> > > > }
> > > >
> > > > /* path/URI handling */
> > > > - path = try_resolve_env(path);
> > > > - if (!path)
> > > > - exit(1);
> > > > -
> > >
> > > This was introduced by Basti in 685cc602e0ad ("keytoc: allow __ENV__
> > > lookup for keyname hint") to support:
> > >
> > > CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__FIT_KEY_NAME_HINT:__ENV__FIT_KEY"
> >
> > Right, albeit I don't see why there should be two separate environment
> > variables for specifying one keyspec entry.
> >
> > > I believe this patch breaks this usecase.
> >
> > Of course :-/ but at the moment the simple use-case:
> >
> > __ENV__FITKEY -> FITKEY="hint:<key>"
> >
> > is broken. IMHO having two separate ENV variables is rather counter
> > intuitive. I also found myself in the following situation:
> >
> > __ENV__FITKEYS -> FITKEYS="hint:<key> hint-2:<key-2> <key-3>"
> >
> > This is not supported as well. So we should really define and document
> > the ENV behavior.
>
> Let's recap the different possibilities. Plain without __ENV__
> substitution we would have:
>
> CONFIG_CRYPTO_PUBLIC_KEYS="pkcs11:object=foo"
> CONFIG_CRYPTO_PUBLIC_KEYS="myhint:pkcs11:object=foo"
> CONFIG_CRYPTO_PUBLIC_KEYS="myhint:pkcs11:object=foo /foobar/baz.der"
> CONFIG_CRYPTO_PUBLIC_KEYS="myhint:pkcs11:object=foo
> myotherhint:/foobar/baz.der"
>
> With Bastis patch we also support:
>
> myhint="myhint"
> myname="pkcs11:object=foo"
> CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__myhint:__ENV__myname"
>
> Now you want to support:
>
> mykeys="myhint:pkcs11:object=foo"
> CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__mykeys"
>
> This would be easy to do by adding the call to try_resolve_env() in the
> loop like you did. You could keep the calls to try_resolve_env() in
> gen_key() in place to still support Bastis usecase.
Sure, too easy that I didn't thought about keeping the code. Albeit I
still find the use-case of having it separate not really intuitive. The
integration could still have two variables <hint> and <key> which is
afterwards combined to <hintkey>.
Anyway, I will keep Bastis use-case :)
> The other thing you like to support is:
>
> mykeys="myhint:pkcs11:object=foo myotherhint:/foobar/baz.der"
> CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__mykeys"
>
> This is more complicated. Right now we assume one single key in each
> argv[] element and consequently call gen_key() for each element. With
> the above an argv[] element can expand to multiple keys and we would
> have to split this up first.
>
> One way would be to do all __ENV__ substitutions upfront.
>
> Not sure if it's worth the hassle though.
This is a use-case I would like to see implemented. Imagine the
following:
* generic defconfig set CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__FITKEYS"
* platform-a: export FITKEYS="hint:keyspec"
* platform-b: export FITKEYS="hint1:keyspec1 keyspec2 hint3:keyspec3"
This would make the integration easier since no adaption of the .config
is required. Without the support for the above, platform-b would need to
adapt the .config first:
* platform-b: CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__FITKEYS __ENV__FITKEY2
__ENV__FITKEY3"
* platform-b: export FITKEYS="hint1:keyspec1"
export FITKEYS2="keyspec2"
export FITKEYS3="hint3:keyspec3"
Regards,
Marco
>
> Sascha
>
> --
> 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 |
>
