Hi Johannes, On 2026-10-29T20:13:40+11:00, Eddie Kovsky <[email protected]> wrote: > rsa: load a pkcs11 token key through OSSL_STORE in the provider path > > The OpenSSL Provider API support routes FIT signing-key loading through > OSSL_STORE on OpenSSL >= 3 (USE_PKCS11_PROVIDER), but only for key files: > rsa_pem_get_priv_key() fopen()s the path before OSSL_STORE_open(), so a > pkcs11: > token URI fails with -ENOENT. The legacy engine path that handled tokens is > compiled out on OpenSSL >= 3, so FIT signing with a token key silently > produces > an unsigned FIT (a configuration signature node with no value or > hashed-strings).
So as I understand it, this describes a regression from patch 1, so token signing is broken from patch 1 until this patch. Please can you fold this into patch 1, or put it before it, so the series stays bisectable? > Load a pkcs11: key URI in the provider path directly through OSSL_STORE and > the > pkcs11 provider: skip the fopen, append ";type=private" if absent. The PIN > travels in the URI (pin-value=); MKIMAGE_SIGN_PIN is unaffected. The patch also changes rsa_pem_get_pub_key() (used by -K), so please mention the public-key side too. 'MKIMAGE_SIGN_PIN is unaffected' reads as if the variable still works. With the provider, rsa_engine_init() is compiled out, so the variable is ignored and the PIN has to go on the command line, where it shows up in the process list and build logs. doc/usage/fit/signature.rst still documents MKIMAGE_SIGN_PIN and the engine key-id rules. Please can you update it to describe how this works with the provider? > > Load a pkcs11: key URI in the provider path directly through OSSL_STORE and > the > pkcs11 provider: skip the fopen, append ";type=private" if absent. The PIN > travels in the URI (pin-value=); MKIMAGE_SIGN_PIN is unaffected. > > Assisted-by: Claude:claude-opus-4-8 > Signed-off-by: Johannes Schneider <[email protected]> > Co-developed-by: Eddie Kovsky <[email protected]> > Signed-off-by: Eddie Kovsky <[email protected]> > > lib/rsa/rsa-sign.c | 92 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c > @@ -92,6 +92,52 @@ static int rsa_pem_get_pub_key(const char *keydir, const > char *name, EVP_PKEY ** > + if (keydir && !strncmp(keydir, "pkcs11:", 7)) { > + char uri[1200]; > + OSSL_STORE_CTX *p11_store; > + EVP_PKEY *p11_key = NULL; > + > + if (strstr(keydir, "type=")) > + snprintf(uri, sizeof(uri), "%s", keydir); > + else > + snprintf(uri, sizeof(uri), "%s;type=public", keydir); This ignores 'name' (the key-name-hint). When keydir has no 'object=' or 'id=', the engine path appends ';object=<key-name-hint>', as signature.rst documents. With this patch, the documented usage (-k pkcs11:token=... with key-name-hint set in the .its) loads the first key OpenSSL finds on the token. On a token with several keys that is likely the wrong one, and the image is signed with no error. Please can you build the URI the same way as rsa_engine_get_pub_key() and rsa_engine_get_priv_key() do? It would also be worth handling a keydir without the 'pkcs11:' prefix when -N pkcs11 is given. At present that case falls through to fopen() and gets -ENOENT. image-host.c treats that as an allowed missing key, which gives the same unsigned FIT that the commit message describes. > diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c > @@ -237,6 +283,52 @@ static int rsa_pem_get_priv_key(const char *keydir, > const char *name, > +#ifdef USE_PKCS11_PROVIDER > + /* > + * A pkcs11: token URI is not a file: load it via OSSL_STORE + the > pkcs11 > + * provider instead of fopen (PIN in the URI, pin-value=). > + */ > + if (keydir && !strncmp(keydir, "pkcs11:", 7)) { > + char uri[1200]; > + OSSL_STORE_CTX *p11_store; > + EVP_PKEY *p11_key = NULL; This block is a copy of the public-key one, apart from 'type=' and the OSSL_STORE_INFO type. Patch 1 also has an OSSL_STORE_open()/load loop for files a few lines below. Please can you put this in a helper and use it in all three places? That would also keep the provider-loading order consistent: here 'default' is loaded before 'pkcs11', but patch 1 loads them the other way round. Regards, Simon
