[PATCH v3] Add support for OpenSSL Provider API
Mattijs Korpershoek
mkorpershoek at kernel.org
Wed Feb 25 17:16:32 CET 2026
Hi Eddie,
On Thu, Feb 19, 2026 at 09:51, Eddie Kovsky <ekovsky at redhat.com> wrote:
> On 01/29/26, Mattijs Korpershoek wrote:
>> Hi Eddie,
>>
>> Thank you for the patch.
>>
>
> Hi Mattijs
>
> Thanks for the review.
>
>> On Tue, Jan 20, 2026 at 09:45, Eddie Kovsky <ekovsky at redhat.com> wrote:
>>
>> > The Engine API has been deprecated since the release of OpenSSL 3.0. End
>> > users have been advised to migrate to the new Provider interface.
>> > Several distributions have already removed support for engines, which is
>> > preventing U-Boot from being compiled in those environments.
>> >
>> > Add support for the Provider API while continuing to support the existing
>> > Engine API on distros shipping older releases of OpenSSL.
>> >
>> > This is based on similar work contributed by Jan Stancek updating Linux
>> > to use the Provider interface.
>> >
>> > commit 558bdc45dfb2669e1741384a0c80be9c82fa052c
>> > Author: Jan Stancek <jstancek at redhat.com>
>> > Date: Fri Sep 20 19:52:48 2024 +0300
>> >
>> > sign-file,extract-cert: use pkcs11 provider for OPENSSL MAJOR >= 3
>> >
>> > The changes have been tested with the FIT signature verification vboot
>> > tests on Fedora 42 and Debian 13. All 30 tests pass with both the legacy
>> > Engine library installed and with the Provider API.
>> >
>> > Signed-off-by: Eddie Kovsky <ekovsky at redhat.com>
>>
>> As a follow-up, can we look into reverting/removing
>> commit 3a8b919932fd ("tools: avoid OpenSSL deprecation warnings") ?
>>
>
> Yes, we could certainly revert this once we've determined it's no longer
> needed for CI. But it's outside the scope of what I'm proposing for this
> patch.
Yes agreed, this can be done later on and is out of scope for this patch.
>
>> This looks much better than v2 in my opinion.
>>
>> Some additional comments below:
>>
>> > ---
>> > Changes in v3:
>> > - Removed Kconfig option
>> > - Changed macro symbol from CONFIG_OPENSSL_NO_DEPRECATED to
>> > USE_PKCS11_PROVIDER or USE_PKCS11_ENGINE
>> > v2: https://lore.kernel.org/u-boot/20251027195834.71109-1-ekovsky@redhat.com/
>> >
>> > Changes in v2:
>> > - Remove default for new Kconfig option
>> > - Use #ifdef instead of IS_ENABLED macro
>> > - Remove comment after #endif
>> > - Remove unrelated checkpatch cleanup of 'sslErr' variable name
>> > v1: https://lore.kernel.org/u-boot/20251017171329.255689-1-ekovsky@redhat.com/
>> > ---
>> > lib/aes/aes-encrypt.c | 4 +-
>> > lib/rsa/rsa-sign.c | 95 ++++++++++++++++++++++++++++++++++++++++++-
>> > 2 files changed, 97 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/lib/aes/aes-encrypt.c b/lib/aes/aes-encrypt.c
>> > index 90e1407b4f09..4fc4ce232478 100644
>> > --- a/lib/aes/aes-encrypt.c
>> > +++ b/lib/aes/aes-encrypt.c
>> > @@ -16,7 +16,9 @@
>> > #include <openssl/err.h>
>> > #include <openssl/ssl.h>
>> > #include <openssl/evp.h>
>> > -#include <openssl/engine.h>
>> > +#if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0)
>> > +# include <openssl/engine.h>
>> > +#endif
>> > #include <uboot_aes.h>
>> >
>> > #if OPENSSL_VERSION_NUMBER >= 0x10000000L
>> > diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
>> > index 0e38c9e802fd..31269db65950 100644
>> > --- a/lib/rsa/rsa-sign.c
>> > +++ b/lib/rsa/rsa-sign.c
>> > @@ -19,7 +19,47 @@
>> > #include <openssl/err.h>
>> > #include <openssl/ssl.h>
>> > #include <openssl/evp.h>
>> > -#include <openssl/engine.h>
>> > +#if OPENSSL_VERSION_MAJOR >= 3
>> > +# define USE_PKCS11_PROVIDER
>> > +# include <err.h>
>> > +# include <openssl/provider.h>
>> > +# include <openssl/store.h>
>> > +#else
>> > +# if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0)
>> > +# define USE_PKCS11_ENGINE
>> > +# include <openssl/engine.h>
>> > +# endif
>> > +#endif
>> > +
>> > +#ifdef USE_PKCS11_PROVIDER
>> > +#define ERR(cond, fmt, ...) \
>> > + do { \
>> > + bool __cond = (cond); \
>> > + drain_openssl_errors(__LINE__, 0); \
>> > + if (__cond) { \
>> > + errx(1, fmt, ## __VA_ARGS__); \
>> > + } \
>> > + } while (0)
>> > +
>> > +static void drain_openssl_errors(int l, int silent)
>> > +{
>> > + const char *file;
>> > + char buf[120];
>> > + int e, line;
>> > +
>> > + if (ERR_peek_error() == 0)
>> > + return;
>> > + if (!silent)
>> > + fprintf(stderr, "At main.c:%d:\n", l);
>> > +
>> > + while ((e = ERR_peek_error_line(&file, &line))) {
>> > + ERR_error_string(e, buf);
>> > + if (!silent)
>> > + fprintf(stderr, "- SSL %s: %s:%d\n", buf, file, line);
>> > + ERR_get_error();
>> > + }
>> > +}
>> > +#endif
>> >
>> > static int rsa_err(const char *msg)
>> > {
>> > @@ -98,6 +138,7 @@ err_cert:
>> > * @evpp Returns EVP_PKEY object, or NULL on failure
>> > * Return: 0 if ok, -ve on error (in which case *evpp will be set to NULL)
>> > */
>> > +#ifdef USE_PKCS11_ENGINE
>> > static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>> > ENGINE *engine, EVP_PKEY **evpp)
>> > {
>> > @@ -157,6 +198,7 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>> >
>> > return 0;
>> > }
>> > +#endif
>> >
>> > /**
>> > * rsa_get_pub_key() - read a public key
>> > @@ -170,8 +212,10 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>>
>> With this change, the ENGINE pointer might be NULL (or undefined).
>> Can we please update the documentation comment to reflect this?
>>
>> For example, we could reword as:
>> * @engine Engine to use or NULL when using pcks11 provider
>>
>
> Sure, I can update the comment for v4.
>> > static int rsa_get_pub_key(const char *keydir, const char *name,
>> > ENGINE *engine, EVP_PKEY **evpp)
>> > {
>> > +#ifdef USE_PKCS11_ENGINE
>> > if (engine)
>> > return rsa_engine_get_pub_key(keydir, name, engine, evpp);
>> > +#endif
>> > return rsa_pem_get_pub_key(keydir, name, evpp);
>> > }
>>
>> Actually, looking even closer at this function, it's seems to be called
>> only once.
>>
>> Why can't we drop this function alltogether and call
>> rsa_engine_get_pub_key() / rsa_pem_get_pub_key() directly in
>> rsa_add_verify_data() ?
>>
>> Reason I'm asking: in rsa_add_verify_data(), ENGINE *e is not used when
>> we use PROVIDER. It seems weird (and error prone) to pass a NULL pointer
>> to a function that does not need that argument
>>
>
> Yes, we could drop rsa_get_pub_key(). It is set up as a helper function and
> only called once from rsa_add_verify_data().
>
> But I am hesitant to make any changes to the RSA API in this file. I
> want to limit the scope of this patch so that the existing code is
> unchanged for users of the Engine API. And I think removing this
> function would require adding more #ifdefs around the error handling in
> rsa_add_verify_data().
I'm fine as well if you keep it this way.
>
>> >
>> > @@ -207,6 +251,37 @@ static int rsa_pem_get_priv_key(const char *keydir, const char *name,
>> > return -ENOENT;
>> > }
>> >
>> > +#ifdef USE_PKCS11_PROVIDER
>> > + EVP_PKEY *private_key = NULL;
>> > + OSSL_STORE_CTX *store;
>> > +
>> > + if (!OSSL_PROVIDER_try_load(NULL, "pkcs11", true))
>> > + ERR(1, "OSSL_PROVIDER_try_load(pkcs11)");
>> > + if (!OSSL_PROVIDER_try_load(NULL, "default", true))
>> > + ERR(1, "OSSL_PROVIDER_try_load(default)");
>> > +
>> > + store = OSSL_STORE_open(path, NULL, NULL, NULL, NULL);
>> > + ERR(!store, "OSSL_STORE_open");
>> > +
>> > + while (!OSSL_STORE_eof(store)) {
>> > + OSSL_STORE_INFO *info = OSSL_STORE_load(store);
>> > +
>> > + if (!info) {
>> > + drain_openssl_errors(__LINE__, 0);
>> > + continue;
>> > + }
>> > + if (OSSL_STORE_INFO_get_type(info) == OSSL_STORE_INFO_PKEY) {
>> > + private_key = OSSL_STORE_INFO_get1_PKEY(info);
>> > + ERR(!private_key, "OSSL_STORE_INFO_get1_PKEY");
>> > + }
>> > + OSSL_STORE_INFO_free(info);
>> > + if (private_key)
>> > + break;
>> > + }
>> > + OSSL_STORE_close(store);
>> > +
>> > + *evpp = private_key;
>> > +#else
>> > if (!PEM_read_PrivateKey(f, evpp, NULL, path)) {
>> > rsa_err("Failure reading private key");
>> > fclose(f);
>> > @@ -214,6 +289,7 @@ static int rsa_pem_get_priv_key(const char *keydir, const char *name,
>> > }
>> > fclose(f);
>> >
>> > +#endif
>> > return 0;
>>
>> This block should be
>>
>> fclose(f);
>> +#endif
>>
>> return 0;
>>
>> (not having a blank line between the fclose and the #endif)
>>
>> > }
>
> I'll clean that up in v4.
>> >
>> > @@ -226,6 +302,7 @@ static int rsa_pem_get_priv_key(const char *keydir, const char *name,
>> > * @evpp Returns EVP_PKEY object, or NULL on failure
>> > * Return: 0 if ok, -ve on error (in which case *evpp will be set to NULL)
>> > */
>> > +#ifdef USE_PKCS11_ENGINE
>> > static int rsa_engine_get_priv_key(const char *keydir, const char *name,
>> > const char *keyfile,
>> > ENGINE *engine, EVP_PKEY **evpp)
>> > @@ -293,6 +370,7 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
>> >
>> > return 0;
>> > }
>> > +#endif
>> >
>> > /**
>> > * rsa_get_priv_key() - read a private key
>> > @@ -306,9 +384,11 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
>> > static int rsa_get_priv_key(const char *keydir, const char *name,
>> > const char *keyfile, ENGINE *engine, EVP_PKEY **evpp)
>> > {
>> > +#ifdef USE_PKCS11_ENGINE
>> > if (engine)
>> > return rsa_engine_get_priv_key(keydir, name, keyfile, engine,
>> > evpp);
>> > +#endif
>> > return rsa_pem_get_priv_key(keydir, name, keyfile, evpp);
>>
>> Same remark as for rsa_engine_get_pub_key. Can't we drop this static
>> function? It's only called once.
>>
>> Maybe do a cleanup patch first, that gets rid of the static functions
>> and then do the provider support in a second patch of the same series?
>>
>> I think it will reduce the amount of #ifdefs, which seems a good
>> argument.
>>
>
> Again, I am trying to limit the scope of this proposal to preserve the
> existing code. If we remove the helper function rsa_get_priv_key() then
> the #ifdef guards also have to move inside the caller rsa_sign(). And
> since we would no longer be able to check the return value of
> rsa_get_priv_key() additional guards would be needed to recreate the
> return value logic that's already in the helper function.
No worries, I'm fine if you keep it this way for v4.
>
>
> Eddie
More information about the U-Boot
mailing list