* [PATCH v4] Add support for OpenSSL Provider API
@ 2026-04-29 18:02 Eddie Kovsky
2026-04-30 7:54 ` Mattijs Korpershoek
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Eddie Kovsky @ 2026-04-29 18:02 UTC (permalink / raw)
To: Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass, Jan Stancek,
Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot
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@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.
Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
---
Changes in v4:
- Add comment that @engine pointer is null when using pkcs11 provider
- Remove extra line break
- Add pkcs11-provider package to build dependencies
v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
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/
---
doc/build/gcc.rst | 4 +-
lib/aes/aes-encrypt.c | 4 +-
lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
tools/docker/Dockerfile | 1 +
4 files changed, 103 insertions(+), 8 deletions(-)
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index 1fef718ceecb..29a6a632e7e3 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
sudo apt-get install bc bison build-essential coccinelle \
device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
- libgnutls28-dev libguestfs-tools libncurses-dev \
- libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
+ libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
+ libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
pkg-config python3 python3-asteval python3-coverage python3-filelock \
python3-pkg-resources python3-pycryptodome python3-pyelftools \
python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
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..f456f3c58e65 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)
{
@@ -94,10 +134,11 @@ err_cert:
*
* @keydir: Key prefix
* @name Name of key
- * @engine Engine to use
+ * @engine Engine to use or NULL when using pkcs11 provider
* @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
return 0;
}
+#endif
/**
* rsa_get_pub_key() - read a public key
*
* @keydir: Directory containing the key (PEM file) or key prefix (engine)
* @name Name of key file (will have a .crt extension)
- * @engine Engine to use
+ * @engine Engine to use or NULL when using pkcs11 provider
* @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)
*/
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);
}
@@ -207,13 +251,44 @@ 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);
return -EPROTO;
}
fclose(f);
-
+#endif
return 0;
}
@@ -226,6 +301,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,22 +369,25 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
return 0;
}
+#endif
/**
* rsa_get_priv_key() - read a private key
*
* @keydir: Directory containing the key (PEM file) or key prefix (engine)
* @name Name of key
- * @engine Engine to use for signing
+ * @engine Engine to use or NULL when using pkcs11 provider
* @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)
*/
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);
}
@@ -325,6 +404,7 @@ static int rsa_init(void)
return 0;
}
+#ifdef USE_PKCS11_ENGINE
static int rsa_engine_init(const char *engine_id, ENGINE **pe)
{
const char *key_pass;
@@ -380,6 +460,7 @@ static void rsa_engine_remove(ENGINE *e)
ENGINE_free(e);
}
}
+#endif
static int rsa_sign_with_key(EVP_PKEY *pkey, struct padding_algo *padding_algo,
struct checksum_algo *checksum_algo,
@@ -480,11 +561,13 @@ int rsa_sign(struct image_sign_info *info,
if (ret)
return ret;
+#ifdef USE_PKCS11_ENGINE
if (info->engine_id) {
ret = rsa_engine_init(info->engine_id, &e);
if (ret)
return ret;
}
+#endif
ret = rsa_get_priv_key(info->keydir, info->keyname, info->keyfile,
e, &pkey);
@@ -496,16 +579,21 @@ int rsa_sign(struct image_sign_info *info,
goto err_sign;
EVP_PKEY_free(pkey);
+
+#ifdef USE_PKCS11_ENGINE
if (info->engine_id)
rsa_engine_remove(e);
+#endif
return ret;
err_sign:
EVP_PKEY_free(pkey);
err_priv:
+#ifdef USE_PKCS11_ENGINE
if (info->engine_id)
rsa_engine_remove(e);
+#endif
return ret;
}
@@ -645,11 +733,13 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest)
ENGINE *e = NULL;
debug("%s: Getting verification data\n", __func__);
+#ifdef USE_PKCS11_ENGINE
if (info->engine_id) {
ret = rsa_engine_init(info->engine_id, &e);
if (ret)
return ret;
}
+#endif
ret = rsa_get_pub_key(info->keydir, info->keyname, e, &pkey);
if (ret)
goto err_get_pub_key;
@@ -726,8 +816,10 @@ done:
err_get_params:
EVP_PKEY_free(pkey);
err_get_pub_key:
+#ifdef USE_PKCS11_ENGINE
if (info->engine_id)
rsa_engine_remove(e);
+#endif
if (ret)
return ret;
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 73bf6cdd2c52..50e98e83dc20 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -122,6 +122,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
openssl \
picocom \
parted \
+ pkcs11-provider \
pkg-config \
python-is-python3 \
python3 \
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-04-29 18:02 [PATCH v4] Add support for OpenSSL Provider API Eddie Kovsky
@ 2026-04-30 7:54 ` Mattijs Korpershoek
2026-05-12 10:17 ` Quentin Schulz
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Mattijs Korpershoek @ 2026-04-30 7:54 UTC (permalink / raw)
To: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot
Hi Eddie,
Thank you for the patch.
On Wed, Apr 29, 2026 at 12:02, Eddie Kovsky <ekovsky@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@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.
>
> Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
> Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
> Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> ---
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-04-29 18:02 [PATCH v4] Add support for OpenSSL Provider API Eddie Kovsky
2026-04-30 7:54 ` Mattijs Korpershoek
@ 2026-05-12 10:17 ` Quentin Schulz
2026-05-21 22:29 ` Eddie Kovsky
2026-05-20 10:28 ` Quentin Schulz
2026-05-20 11:32 ` Quentin Schulz
3 siblings, 1 reply; 9+ messages in thread
From: Quentin Schulz @ 2026-05-12 10:17 UTC (permalink / raw)
To: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot
Hi Eddie,
On 4/29/26 8:02 PM, Eddie Kovsky 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@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.
>
But does it actually use a provider or an engine to begin with? I don't
see test/py/tests/test_vboot.py calling mkimage with the -N argument.
What are the tests (or command) you ran to validate this? I briefly saw
the CI failed in v3 because a package was missing, but wasn't it simply
because the headers or provider libraries which are now necessary for
building lib/rsa/rsa-sign.c were not present? The logs aren't available
anymore unfortunately. If that's the case, then that's also an issue. We
shouldn't need to install providers if we aren't going to use any? Yes,
I know that we currently cannot compile if we don't have
openssl-devel-engine (on Fedora), but if we can improve the situation,
we should.
How did you test (locally is fine) with providers?
> Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
> Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
Please do not forget to add the colon after Tested-by so it actually
makes it a git trailer instead of just text.
> Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
> ---
> Changes in v4:
> - Add comment that @engine pointer is null when using pkcs11 provider
> - Remove extra line break
> - Add pkcs11-provider package to build dependencies
> v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
>
> 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/
> ---
> doc/build/gcc.rst | 4 +-
> lib/aes/aes-encrypt.c | 4 +-
> lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
> tools/docker/Dockerfile | 1 +
> 4 files changed, 103 insertions(+), 8 deletions(-)
>
> diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> index 1fef718ceecb..29a6a632e7e3 100644
> --- a/doc/build/gcc.rst
> +++ b/doc/build/gcc.rst
> @@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
>
> sudo apt-get install bc bison build-essential coccinelle \
> device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
> - libgnutls28-dev libguestfs-tools libncurses-dev \
> - libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
> + libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
> + libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
> pkg-config python3 python3-asteval python3-coverage python3-filelock \
> python3-pkg-resources python3-pycryptodome python3-pyelftools \
> python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
> 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
Considering there are no other changes in this file, is this include
actually needed?
> #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..f456f3c58e65 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
> +
Sorry but that's a NACK.
As far as I can tell, this effectively disables using engines when your
host openssl version is 3.0+, which we currently support (and I
currently use it). You can have this for 4.0+ as engine support has been
removed, but not for 3.x.
> +#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)
> +
Is this really related to the PKCS11 provider? I think there's a mix
between "using the provider API" and "using the pkcs11 provider".
> +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);
> +
main.c?
> + 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)
> {
> @@ -94,10 +134,11 @@ err_cert:
> *
> * @keydir: Key prefix
> * @name Name of key
> - * @engine Engine to use
> + * @engine Engine to use or NULL when using pkcs11 provider
> * @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>
> return 0;
> }
> +#endif
>
> /**
> * rsa_get_pub_key() - read a public key
> *
> * @keydir: Directory containing the key (PEM file) or key prefix (engine)
> * @name Name of key file (will have a .crt extension)
> - * @engine Engine to use
> + * @engine Engine to use or NULL when using pkcs11 provider
> * @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)
> */
> 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
We should probably at least print something when engines aren't
supported but the engine parameter is passed, or maybe even fail as
that's an incorrect configuration.
> return rsa_pem_get_pub_key(keydir, name, evpp);
> }
>
> @@ -207,13 +251,44 @@ 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)");
> +
Why are we unconditionally loading providers and specifically the pkcs11
one? As far as I could tell we should know from the URI whether a key
from a provider is requested by checking if a colon is in the URI. And
we can automatically load the requested provider based on that?
Otherwise the easiest answer to this could simply be: "let the user
configure this externally via OPENSSL_CONF environment variable".
Do we also need to update tools/mkimage.c (and doc/mkimage.1) to remove
-N engine from usage if the tool doesn't support it? Do we also need to
update doc/usage/fit/signature.rst to explain how to use pkcs11 provider?
FYI, we (my employer) are migrating from engines to providers for
signing FIT images with binman so I'm starting to have a look on how to
do this now.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-04-29 18:02 [PATCH v4] Add support for OpenSSL Provider API Eddie Kovsky
2026-04-30 7:54 ` Mattijs Korpershoek
2026-05-12 10:17 ` Quentin Schulz
@ 2026-05-20 10:28 ` Quentin Schulz
2026-05-21 12:43 ` Quentin Schulz
2026-05-20 11:32 ` Quentin Schulz
3 siblings, 1 reply; 9+ messages in thread
From: Quentin Schulz @ 2026-05-20 10:28 UTC (permalink / raw)
To: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot
Hi Eddie,
Thanks for working on this, this was a good base for me to work on
supporting pkcs11 and custom OpenSSL providers for signing FIT images
with binman.
On 4/29/26 8:02 PM, Eddie Kovsky 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@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.
>
> Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
> Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
> Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
> ---
> Changes in v4:
> - Add comment that @engine pointer is null when using pkcs11 provider
> - Remove extra line break
> - Add pkcs11-provider package to build dependencies
> v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
>
> 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/
> ---
> doc/build/gcc.rst | 4 +-
> lib/aes/aes-encrypt.c | 4 +-
> lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
> tools/docker/Dockerfile | 1 +
> 4 files changed, 103 insertions(+), 8 deletions(-)
>
> diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> index 1fef718ceecb..29a6a632e7e3 100644
> --- a/doc/build/gcc.rst
> +++ b/doc/build/gcc.rst
> @@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
>
> sudo apt-get install bc bison build-essential coccinelle \
> device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
> - libgnutls28-dev libguestfs-tools libncurses-dev \
> - libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
> + libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
> + libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
> pkg-config python3 python3-asteval python3-coverage python3-filelock \
> python3-pkg-resources python3-pycryptodome python3-pyelftools \
> python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
> 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..f456f3c58e65 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);
buf must be at least 256 bytes long, and it's 120 here. Either increase
buf[] size to 256 or use ERR_error_string_n(e, buf, 120), c.f.
https://docs.openssl.org/3.0/man3/ERR_error_string/#description.
> + if (!silent)
> + fprintf(stderr, "- SSL %s: %s:%d\n", buf, file, line);
> + ERR_get_error();
> + }
> +}
> +#endif
>
OK so this was somehow unclear to me this was imported from Linux even
though you stated that in your commit log. In the kernel, the macro and
function are stored in scripts/ssl-common.h, I'm wondering if we can
import it in a similar way (separate file) so that it's easy to keep
track of changes made to the file in Linux and backport them to U-Boot
whenever necessary. The origin is at least clear that way.
> static int rsa_err(const char *msg)
> {
> @@ -94,10 +134,11 @@ err_cert:
> *
> * @keydir: Key prefix
> * @name Name of key
> - * @engine Engine to use
> + * @engine Engine to use or NULL when using pkcs11 provider
No, because this function doesn't exist whenever provider support is
enabled. Since I would like to support both providers and engines in
parallel (see the other review), this function can actually be built
even if provider support is enabled. However, we shouldn't be calling
this function with a provider. If engine is NULL, this function isn't
called, see rsa_get_pub_key().
> * @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>
> return 0;
> }
> +#endif
>
> /**
> * rsa_get_pub_key() - read a public key
> *
> * @keydir: Directory containing the key (PEM file) or key prefix (engine)
> * @name Name of key file (will have a .crt extension)
> - * @engine Engine to use
> + * @engine Engine to use or NULL when using pkcs11 provider
Well, a provider isn't an engine, so I think it's pretty clear engine is
supposed to be NULL when you don't want to use an engine? Also, this
applies to all providers, not only pkcs11.
> * @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)
> */
> 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);
> }
>
> @@ -207,13 +251,44 @@ 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;
> +
This is sneaky here because it's not in the git context, but above here
there is:
"""
if (keydir && name)
snprintf(path, sizeof(path), "%s/%s.key", keydir, name);
else if (keyfile)
snprintf(path, sizeof(path), "%s", keyfile);
else
return -EINVAL;
f = fopen(path, "r");
if (!f) {
fprintf(stderr, "Couldn't open RSA private key: '%s':
%s\n",
path, strerror(errno));
return -ENOENT;
}
"""
This means this function will fail if path cannot be found on disk,
which I am assuming is the case for every provider. I'm wondering if you
actually tested this with a provider or only with a file? (the
OSSL_STORE_open() supports files in path but also providers (with the
scheme (e.g. pkcs11:) as prefix)) (my grep-fu may be lacking but nothing
in test/ seems to do anything with pkcs11?) If your only worry is to
build with an OpenSSL without engine support but have no real interest
in supporting providers, then just ifdef the engine support and let
provider support be done by someone else (I'm looking into it now for
signing FIT images, I've got something locally already).
Also, we have some logic for generating the proper path for pkcs11 from
keydir, name and keyfile for the pkcs11 engine, c.f.
rsa_engine_get_pub_key(), I'm assuming we may want something similar for
pkcs11 provider.
Locally, I've gone for:
"""
if (keydir && !strncmp(pkcs11_schema, keydir,
strlen(pkcs11_schema))) {
// Create the URI for the PKCS11 provider
if (strstr(keydir, "object=") || strstr(keydir, "id="))
snprintf(path, sizeof(path), "%s;type=private",
keydir);
else
snprintf(path, sizeof(path),
"%s;object=%s;type=private",
keydir, name);
} else if (keydir && strstr(keydir, ":")) {
// Create the URI for specified provider from keydir
and name
snprintf(path, sizeof(path), "%s%s", keydir,
name ?: "");
} else if (keydir && name) {
snprintf(path, sizeof(path), "%s/%s.key", keydir, name);
} else if (keyfile) {
snprintf(path, sizeof(path), "%s", keyfile);
} else {
return -EINVAL;
}
"""
> + 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)");
> +
You can ignore this with
export OPENSSL_CONF=/path/to/my/openssl.conf
"""
openssl_conf = openssl_init
[openssl_init]
providers = providers_section
[providers_section]
default = default_provider
pkcs11 = pkcs11_provider
[default_provider]
activate=1
[pkcs11_provider]
activate = 1
"""
This will allow supporting any provider, not only pkcs11 and allow to
build without pkcs11-provider installed if another provider is desired.
Note that you're modifying code used to sign with RSA, it's not
necessarily pkcs11 (unlike in the kernel from where you took inspiration
which is explicitly pkcs11-only). This also allows to pass a pin for the
pkcs11 provider without U-Boot needing to know about it.
> + 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");
> + }
I have been told that calling OSSL_STORE_expect() with the expected type
should make it easier to get the object we want directly.
Something like
OSSL_STORE_expect(store, OSSL_STORE_INFO_PKEY);
right after OSSL_STORE_open(). I'm assuming this should allow us to get
rid of the OSSL_STORE_INFO_get_type(info) == OSSL_STORE_INFO_PKEY check
since OSS_STORE_load() would only return objects of expected type.
c.f. https://docs.openssl.org/3.0/man3/OSSL_STORE_expect/#description
I've not tested this myself yet.
> + 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);
> return -EPROTO;
> }
> fclose(f);
> -
> +#endif
> return 0;
Not much is actually shared in rsa_pem_get_priv_key between provider and
no provider support so I think it may make sense to ifdef the whole
content of the function or maybe the whole function even (or move the
content into other functions).
> }
>
> @@ -226,6 +301,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,22 +369,25 @@ static int rsa_engine_get_priv_key(const char *keydir, const char *name,
>
> return 0;
> }
> +#endif
>
> /**
> * rsa_get_priv_key() - read a private key
> *
> * @keydir: Directory containing the key (PEM file) or key prefix (engine)
> * @name Name of key
> - * @engine Engine to use for signing
> + * @engine Engine to use or NULL when using pkcs11 provider
Same remark as above.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-04-29 18:02 [PATCH v4] Add support for OpenSSL Provider API Eddie Kovsky
` (2 preceding siblings ...)
2026-05-20 10:28 ` Quentin Schulz
@ 2026-05-20 11:32 ` Quentin Schulz
3 siblings, 0 replies; 9+ messages in thread
From: Quentin Schulz @ 2026-05-20 11:32 UTC (permalink / raw)
To: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot
Forgot to report something I fixed locally.
On 4/29/26 8:02 PM, Eddie Kovsky wrote:
[...]
> @@ -207,13 +251,44 @@ 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;
If we reach here without actually finding a private_key, we'll return 0
a few lines down which is definitely not what we want to do. I'm suggesting:
if (!private_key)
return -EINVAL;
Maybe it should be -ENOENT like for when we don't find the key on disk
(see first line in git context in this hunk), because for some reason
our logic in tools/image-host.c specifies that missing keys is allowed
(???????).
> +#else
> if (!PEM_read_PrivateKey(f, evpp, NULL, path)) {
> rsa_err("Failure reading private key");
> fclose(f);
> return -EPROTO;
> }
> fclose(f);
> -
> +#endif
> return 0;
> }
>
Cheers,
Quentin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-05-20 10:28 ` Quentin Schulz
@ 2026-05-21 12:43 ` Quentin Schulz
0 siblings, 0 replies; 9+ messages in thread
From: Quentin Schulz @ 2026-05-21 12:43 UTC (permalink / raw)
To: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek
Cc: u-boot, Peng Fan
Hi Eddie,
On 5/20/26 12:28 PM, Quentin Schulz wrote:
> On 4/29/26 8:02 PM, Eddie Kovsky wrote:
[...]>> + 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");
>> + }
>
> I have been told that calling OSSL_STORE_expect() with the expected type
> should make it easier to get the object we want directly.
>
> Something like
>
> OSSL_STORE_expect(store, OSSL_STORE_INFO_PKEY);
>
> right after OSSL_STORE_open(). I'm assuming this should allow us to get
> rid of the OSSL_STORE_INFO_get_type(info) == OSSL_STORE_INFO_PKEY check
> since OSS_STORE_load() would only return objects of expected type.
>
> c.f. https://eur02.safelinks.protection.outlook.com/?
> url=https%3A%2F%2Fdocs.openssl.org%2F3.0%2Fman3%2FOSSL_STORE_expect%2F%23description&data=05%7C02%7Cquentin.schulz%40cherry.de%7C02bd298ec294469f172a08deb65a8a6d%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C639148697211162781%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=v8m2m1Mi5UXizJjVHHUE2EPryifJfMTnDgrSgFZx1b8%3D&reserved=0
>
> I've not tested this myself yet.
>
This was actually required for my custom provider. When you don't set
the expected type, your provider store context's OSSL_STORE_PARAM_EXPECT
param gets initialized to zero which means "return all supported
OSSL_STORE_INFO_* objects except for OSSL_STORE_INFO_NAME". Our custom
provider didn't implement that (it does now) and would simply say that
what's requested is invalid. I'm unsure whether this means our custom
provider was broken or if we should cater for this corner case and
simply request a store its private key?
On a more general topic, I cannot build anything including the
openssl/engine.h header without openssl-devel-engine on Fedora. This
means tools/binman/test/fit/dummy-rsa-engine.c but also CST
(https://gitlab.apertis.org/pkg/imx-code-signing-tool/-/blob/debian/unstable/code/back_end-ssl/src/engine.c?ref_type=heads#L11)
which means we cannot run the binman test suite on Fedora without
openssl-devel-engine package installed, or anything that requires CST,
e.g. the nxp-imx8mcst nodes in binman. I'll try to figure out a way to
handle the dummy-rsa-engine.c and be able to skip engine/provider tests
if unsupported.
Someone will need to have a look at adding support for providers to CST.
We download and build code from apertis.org but I'm unsure whether we're
really supposed to use that or report bugs there. Debian points to
https://salsa.debian.org/collabora-team/imx-code-signing-tool instead so
maybe we should report there? They seem to be an import from NXP, so
adding Peng in Cc, maybe they know who to ask for provider support (or
at the very least, remove engine support if engine support is disabled
in OpenSSL so it can compile just fine).
Cheers,
Quentin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-05-12 10:17 ` Quentin Schulz
@ 2026-05-21 22:29 ` Eddie Kovsky
2026-05-22 14:37 ` Quentin Schulz
0 siblings, 1 reply; 9+ messages in thread
From: Eddie Kovsky @ 2026-05-21 22:29 UTC (permalink / raw)
To: Quentin Schulz
Cc: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek, u-boot
On 05/12/26, Quentin Schulz wrote:
> Hi Eddie,
>
> On 4/29/26 8:02 PM, Eddie Kovsky 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@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.
> >
>
> But does it actually use a provider or an engine to begin with? I don't see
> test/py/tests/test_vboot.py calling mkimage with the -N argument. What are
> the tests (or command) you ran to validate this? I briefly saw the CI failed
> in v3 because a package was missing, but wasn't it simply because the
> headers or provider libraries which are now necessary for building
> lib/rsa/rsa-sign.c were not present? The logs aren't available anymore
> unfortunately. If that's the case, then that's also an issue. We shouldn't
> need to install providers if we aren't going to use any? Yes, I know that we
> currently cannot compile if we don't have openssl-devel-engine (on Fedora),
> but if we can improve the situation, we should.
>
> How did you test (locally is fine) with providers?
>
The FIT signature verification tests are documented here:
https://docs.u-boot.org/en/latest/usage/fit/signature.html#u-boot-fit-signature-verification
The tests currently fail in build environments (like Fedora) that don't
have engine support. This is how we originally became aware of the API
issue last year.
❯ ./test/py/test.py --bd sandbox --build -k vboot
+make O=u-boot/build-sandbox -s sandbox_defconfig
+make O=u-boot/build-sandbox -s -j8
In file included from tools/generated/lib/aes/aes-encrypt.c:1:
../tools/../lib/aes/aes-encrypt.c:19:10: fatal error: openssl/engine.h: No such file or directory
19 | #include <openssl/engine.h>
| ^~~~~~~~~~~~~~~~~~
In file included from tools/generated/lib/rsa/rsa-sign.c:1:
../tools/../lib/rsa/rsa-sign.c:22:10: fatal error: openssl/engine.h: No such file or directory
22 | #include <openssl/engine.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
Github provides the Azure CI pipeline as a free service. I wouldn't
expect them to retain logs at that tier.
> > Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
> > Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
>
> Please do not forget to add the colon after Tested-by so it actually makes
> it a git trailer instead of just text.
>
That's an unlucky typo because I added the trailers manually.
> > Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
> > ---
> > Changes in v4:
> > - Add comment that @engine pointer is null when using pkcs11 provider
> > - Remove extra line break
> > - Add pkcs11-provider package to build dependencies
> > v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
> >
> > 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/
> > ---
> > doc/build/gcc.rst | 4 +-
> > lib/aes/aes-encrypt.c | 4 +-
> > lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
> > tools/docker/Dockerfile | 1 +
> > 4 files changed, 103 insertions(+), 8 deletions(-)
> >
> > diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> > index 1fef718ceecb..29a6a632e7e3 100644
> > --- a/doc/build/gcc.rst
> > +++ b/doc/build/gcc.rst
> > @@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
> > sudo apt-get install bc bison build-essential coccinelle \
> > device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
> > - libgnutls28-dev libguestfs-tools libncurses-dev \
> > - libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
> > + libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
> > + libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
> > pkg-config python3 python3-asteval python3-coverage python3-filelock \
> > python3-pkg-resources python3-pycryptodome python3-pyelftools \
> > python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
> > 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
>
> Considering there are no other changes in this file, is this include
> actually needed?
>
The header is still needed to maintain backward compatibility for engine
support. I maintained the #ifdef style to be consistent with its usage
elsewhere, based on previous feedback.
> > #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..f456f3c58e65 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
> > +
>
> Sorry but that's a NACK.
>
> As far as I can tell, this effectively disables using engines when your host
> openssl version is 3.0+, which we currently support (and I currently use
> it). You can have this for 4.0+ as engine support has been removed, but not
> for 3.x.
>
You don't get a choice here. OpenSSL deprecated the Engine API starting
with Version 3.0, and removed it completely starting with Version 4.0.
If the environment you're building U-Boot in has a recent version of
OpenSSL then Engines are no longer available to you.
> > +#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)
> > +
>
> Is this really related to the PKCS11 provider? I think there's a mix between
> "using the provider API" and "using the pkcs11 provider".
>
It's a helper macro used for the Provider path when OpenSSL is v3 or
later.
> > +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);
> > +
>
> main.c?
>
Not clear what you are referring to here.
> > + 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)
> > {
> > @@ -94,10 +134,11 @@ err_cert:
> > *
> > * @keydir: Key prefix
> > * @name Name of key
> > - * @engine Engine to use
> > + * @engine Engine to use or NULL when using pkcs11 provider
> > * @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
> > return 0;
> > }
> > +#endif
> > /**
> > * rsa_get_pub_key() - read a public key
> > *
> > * @keydir: Directory containing the key (PEM file) or key prefix (engine)
> > * @name Name of key file (will have a .crt extension)
> > - * @engine Engine to use
> > + * @engine Engine to use or NULL when using pkcs11 provider
> > * @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)
> > */
> > 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
>
> We should probably at least print something when engines aren't supported
> but the engine parameter is passed, or maybe even fail as that's an
> incorrect configuration.
>
We discussed this in a previous revision and decided the comment was
sufficient. The correct path will be taken depending on which version of
the API is available.
> > return rsa_pem_get_pub_key(keydir, name, evpp);
> > }
> > @@ -207,13 +251,44 @@ 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)");
> > +
>
> Why are we unconditionally loading providers and specifically the pkcs11
> one? As far as I could tell we should know from the URI whether a key from a
> provider is requested by checking if a colon is in the URI. And we can
> automatically load the requested provider based on that?
>
> Otherwise the easiest answer to this could simply be: "let the user
> configure this externally via OPENSSL_CONF environment variable".
>
> Do we also need to update tools/mkimage.c (and doc/mkimage.1) to remove -N
> engine from usage if the tool doesn't support it? Do we also need to update
> doc/usage/fit/signature.rst to explain how to use pkcs11 provider?
>
> FYI, we (my employer) are migrating from engines to providers for signing
> FIT images with binman so I'm starting to have a look on how to do this now.
>
> Cheers,
> Quentin
>
Additional changes may be needed down the road. I'm sure additional
patches will be welcome. The scope of this change is to introduce
support for the new API while continuing to support developers who are
still building in environments using the deprecated Engine API.
Eddie
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-05-21 22:29 ` Eddie Kovsky
@ 2026-05-22 14:37 ` Quentin Schulz
2026-06-04 6:22 ` Enric Balletbo i Serra
0 siblings, 1 reply; 9+ messages in thread
From: Quentin Schulz @ 2026-05-22 14:37 UTC (permalink / raw)
To: Eddie Kovsky
Cc: Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass, Jan Stancek,
Enric Balletbo i Serra, a.fatoum, mark.kettenis,
Mattijs Korpershoek, u-boot
Hi Eddie,
On 5/22/26 12:29 AM, Eddie Kovsky wrote:
> On 05/12/26, Quentin Schulz wrote:
>> Hi Eddie,
>>
>> On 4/29/26 8:02 PM, Eddie Kovsky 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@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.
>>>
>>
>> But does it actually use a provider or an engine to begin with? I don't see
>> test/py/tests/test_vboot.py calling mkimage with the -N argument. What are
>> the tests (or command) you ran to validate this? I briefly saw the CI failed
>> in v3 because a package was missing, but wasn't it simply because the
>> headers or provider libraries which are now necessary for building
>> lib/rsa/rsa-sign.c were not present? The logs aren't available anymore
>> unfortunately. If that's the case, then that's also an issue. We shouldn't
>> need to install providers if we aren't going to use any? Yes, I know that we
>> currently cannot compile if we don't have openssl-devel-engine (on Fedora),
>> but if we can improve the situation, we should.
>>
>> How did you test (locally is fine) with providers?
>>
>
> The FIT signature verification tests are documented here:
>
> https://docs.u-boot.org/en/latest/usage/fit/signature.html#u-boot-fit-signature-verification
>
> The tests currently fail in build environments (like Fedora) that don't
> have engine support. This is how we originally became aware of the API
> issue last year.
>
> ❯ ./test/py/test.py --bd sandbox --build -k vboot
> +make O=u-boot/build-sandbox -s sandbox_defconfig
> +make O=u-boot/build-sandbox -s -j8
> In file included from tools/generated/lib/aes/aes-encrypt.c:1:
> ../tools/../lib/aes/aes-encrypt.c:19:10: fatal error: openssl/engine.h: No such file or directory
> 19 | #include <openssl/engine.h>
> | ^~~~~~~~~~~~~~~~~~
> In file included from tools/generated/lib/rsa/rsa-sign.c:1:
> ../tools/../lib/rsa/rsa-sign.c:22:10: fatal error: openssl/engine.h: No such file or directory
> 22 | #include <openssl/engine.h>
> | ^~~~~~~~~~~~~~~~~~
> compilation terminated.
>
> Github provides the Azure CI pipeline as a free service. I wouldn't
> expect them to retain logs at that tier.
>
As said, I don't think we are testing OpenSSL providers in test/py.
rsa-sign.c and aes-encrypt.c can work perfectly fine without using
engines, but currently it requires the engine.h header file to compile
(and if missing, then the functions, macros, constants defined in that
header file will need to be compiled out like done in this patch) even
if it doesn't use engines at runtime.
The issue is not that OpenSSL is built without engine support, it's
rather that Fedora has decided to stop shipping openssl/engine.h by
default. For some reason, they still compile OpenSSL with engine support
(I don't understand why). You are fixing a build issue, sure, and we
must fix it, but I don't think this patch is doing it the proper way. So
I am asking again, did you test with an actual OpenSSL provider (and no,
the implicit file: scheme "provider" doesn't count)?
See
https://lore.kernel.org/u-boot/d8b15735-0b3f-4a72-a2a2-9ac6e12f4dbf@cherry.de/T/#m7454283f474aaee33736e1ae7154569da846f14a
>
>>> Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
>>> Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
>>
>> Please do not forget to add the colon after Tested-by so it actually makes
>> it a git trailer instead of just text.
>>
>
> That's an unlucky typo because I added the trailers manually.
>
It happens :)
For your info, with b4
(https://b4.docs.kernel.org/en/latest/contributor/overview.html) you can
run b4 trailers -u (if you started the series with b4, though you can
probably create a new branch from an existing series with b4 prep
--from-thread?) and it'll automatically fetch the trailers from answers
on the ML and add them to the appropriate patches.
>>> Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
>>> ---
>>> Changes in v4:
>>> - Add comment that @engine pointer is null when using pkcs11 provider
>>> - Remove extra line break
>>> - Add pkcs11-provider package to build dependencies
>>> v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
>>>
>>> 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/
>>> ---
>>> doc/build/gcc.rst | 4 +-
>>> lib/aes/aes-encrypt.c | 4 +-
>>> lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
>>> tools/docker/Dockerfile | 1 +
>>> 4 files changed, 103 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
>>> index 1fef718ceecb..29a6a632e7e3 100644
>>> --- a/doc/build/gcc.rst
>>> +++ b/doc/build/gcc.rst
>>> @@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
>>> sudo apt-get install bc bison build-essential coccinelle \
>>> device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
>>> - libgnutls28-dev libguestfs-tools libncurses-dev \
>>> - libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
>>> + libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
>>> + libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
>>> pkg-config python3 python3-asteval python3-coverage python3-filelock \
>>> python3-pkg-resources python3-pycryptodome python3-pyelftools \
>>> python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
>>> 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
>>
>> Considering there are no other changes in this file, is this include
>> actually needed?
>>
>
> The header is still needed to maintain backward compatibility for engine
> support. I maintained the #ifdef style to be consistent with its usage
> elsewhere, based on previous feedback.
>
What am I trying to say here is no symbol in the file comes from
openssl/engine.h and I can compile the file perfectly fine without this
header included, so we should probably get rid of this include.
>>> #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..f456f3c58e65 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
>>> +
>>
>> Sorry but that's a NACK.
>>
>> As far as I can tell, this effectively disables using engines when your host
>> openssl version is 3.0+, which we currently support (and I currently use
>> it). You can have this for 4.0+ as engine support has been removed, but not
>> for 3.x.
>>
>
> You don't get a choice here. OpenSSL deprecated the Engine API starting
Providers and engines can cohabit in OpenSSL 3.x, so that is possible (I
tested it locally with the binman test suite, with both providers and
engines local test suites working with the same OpenSSL version (Fedora
44 + openssl-devel-engine).
> with Version 3.0, and removed it completely starting with Version 4.0.
> If the environment you're building U-Boot in has a recent version of
> OpenSSL then Engines are no longer available to you.
>
This only happens because Fedora (and not Debian, or ArchLinux) decided
to stop shipping an engine.h header file (while still compiling OpenSSL
with engine support).
I can use engines with OpenSSL 3.x, with the current state in U-Boot, so
you cannot unilaterally decide to break that.
What you can do is either remove support for OpenSSL engines entirely
when it's factually not possible to build mkimage with support for it
(and no, checking for OpenSSL 3.x is not enough). That is, when OpenSSL
is built/shipped without engine support (that is, for Fedora, unless
openssl-devel-engine is installed). Or migrate engine support to use the
provider API (by e.g. prepending org.openssl.engine: in the OSSL_STORE
scheme) for OpenSSL 3.x. The user API must be identical though, what
works with engines on OpenSSL 1.x an OpenSSL 3.x with engine support via
the provider API must be the same. I would suggest to NOT go for the
second option as I don't think it's worth the added complexity.
>>> +#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)
>>> +
>>
>> Is this really related to the PKCS11 provider? I think there's a mix between
>> "using the provider API" and "using the pkcs11 provider".
>>
>
> It's a helper macro used for the Provider path when OpenSSL is v3 or
> later.
>
I'm arguing about the macro being called USE_PKCS11_PROVIDER while
having nothing to do with pkcs11. It's misleading.
>>> +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);
>>> +
>>
>> main.c?
>>
>
> Not clear what you are referring to here.
>
The error message would state "At main.c:" which is absolutely not
related to any file this is called from. I guess maybe you want __FILE__
here instead?
>>> + 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)
>>> {
>>> @@ -94,10 +134,11 @@ err_cert:
>>> *
>>> * @keydir: Key prefix
>>> * @name Name of key
>>> - * @engine Engine to use
>>> + * @engine Engine to use or NULL when using pkcs11 provider
>>> * @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
>>> return 0;
>>> }
>>> +#endif
>>> /**
>>> * rsa_get_pub_key() - read a public key
>>> *
>>> * @keydir: Directory containing the key (PEM file) or key prefix (engine)
>>> * @name Name of key file (will have a .crt extension)
>>> - * @engine Engine to use
>>> + * @engine Engine to use or NULL when using pkcs11 provider
>>> * @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)
>>> */
>>> 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
>>
>> We should probably at least print something when engines aren't supported
>> but the engine parameter is passed, or maybe even fail as that's an
>> incorrect configuration.
>>
>
> We discussed this in a previous revision and decided the comment was
> sufficient. The correct path will be taken depending on which version of
> the API is available.
>
mkimage will still happily accept the -N argument which sets the engine
parameter, even if engines aren't supported.
I think if you have a file key with the same name as in the engine,
you'll then sign with the wrong key.
Think
mkimage -N myengine -g key
and you have a key.key in the local directory. If you compile with
OpenSSL engines supported, you'll essentially use myengine:key, if not,
you'll use key.key, without mkimage even notifying you by using the same
command.
>>> return rsa_pem_get_pub_key(keydir, name, evpp);
>>> }
>>> @@ -207,13 +251,44 @@ 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)");
>>> +
>>
>> Why are we unconditionally loading providers and specifically the pkcs11
>> one? As far as I could tell we should know from the URI whether a key from a
>> provider is requested by checking if a colon is in the URI. And we can
>> automatically load the requested provider based on that?
>>
>> Otherwise the easiest answer to this could simply be: "let the user
>> configure this externally via OPENSSL_CONF environment variable".
>>
>> Do we also need to update tools/mkimage.c (and doc/mkimage.1) to remove -N
>> engine from usage if the tool doesn't support it? Do we also need to update
>> doc/usage/fit/signature.rst to explain how to use pkcs11 provider?
>>
>> FYI, we (my employer) are migrating from engines to providers for signing
>> FIT images with binman so I'm starting to have a look on how to do this now.
>>
>> Cheers,
>> Quentin
>>
>
> Additional changes may be needed down the road. I'm sure additional
> patches will be welcome. The scope of this change is to introduce
> support for the new API while continuing to support developers who are
> still building in environments using the deprecated Engine API.
>
But it's not doing that? This is removing support for OpenSSL engines on
all systems shipping OpenSSL 3.x? We cannot accept patches that break
things under the pretense there can be follow-up patches. I'm not asking
support for signing with OpenSSL providers in this series, I'm asking
you to not break existing users.
I'm still unsure what the goal of this patch is. It says it adds support
for OpenSSL provider API but I don't think provider support was actually
tested. It also says U-Boot cannot be built on Fedora anymore due to
engine.h missing. Both can be be separate topics. The urgent task is to
fix building on Fedora, supporting providers can then be tackled later
on (by either you, or someone else; e.g. I already have support for
providers locally, just need to write nice commit logs and wait on what
this patch will become here). I commented out everything related to
engines and ran the testSignSimple, testSignExactFIT, testSignNonFit,
testSignMissingMkimage and testSplPubkeyDtb to check whether using keys
was still supported (meaning, we aren't forced to migrate to the
OSSL_STORE interface for simple keys).
I'd be fine with guarding all engine support with
if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0)
and making sure we at least warn (or fail) if an engine is asked by the
user but we don't support it, so that you can build on Fedora again
without openssl-devel-engine.
Cheers,
Quentin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4] Add support for OpenSSL Provider API
2026-05-22 14:37 ` Quentin Schulz
@ 2026-06-04 6:22 ` Enric Balletbo i Serra
0 siblings, 0 replies; 9+ messages in thread
From: Enric Balletbo i Serra @ 2026-06-04 6:22 UTC (permalink / raw)
To: Quentin Schulz
Cc: Eddie Kovsky, Tom Rini, Tobias Olausson, Paul HENRYS, Simon Glass,
Jan Stancek, a.fatoum, mark.kettenis, Mattijs Korpershoek, u-boot
Hi Quentin and Eddie,
On Fri, May 22, 2026 at 4:38 PM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Eddie,
>
> On 5/22/26 12:29 AM, Eddie Kovsky wrote:
> > On 05/12/26, Quentin Schulz wrote:
> >> Hi Eddie,
> >>
> >> On 4/29/26 8:02 PM, Eddie Kovsky 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@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.
> >>>
> >>
> >> But does it actually use a provider or an engine to begin with? I don't see
> >> test/py/tests/test_vboot.py calling mkimage with the -N argument. What are
> >> the tests (or command) you ran to validate this? I briefly saw the CI failed
> >> in v3 because a package was missing, but wasn't it simply because the
> >> headers or provider libraries which are now necessary for building
> >> lib/rsa/rsa-sign.c were not present? The logs aren't available anymore
> >> unfortunately. If that's the case, then that's also an issue. We shouldn't
> >> need to install providers if we aren't going to use any? Yes, I know that we
> >> currently cannot compile if we don't have openssl-devel-engine (on Fedora),
> >> but if we can improve the situation, we should.
> >>
> >> How did you test (locally is fine) with providers?
> >>
> >
> > The FIT signature verification tests are documented here:
> >
> > https://docs.u-boot.org/en/latest/usage/fit/signature.html#u-boot-fit-signature-verification
> >
> > The tests currently fail in build environments (like Fedora) that don't
> > have engine support. This is how we originally became aware of the API
> > issue last year.
> >
> > ❯ ./test/py/test.py --bd sandbox --build -k vboot
> > +make O=u-boot/build-sandbox -s sandbox_defconfig
> > +make O=u-boot/build-sandbox -s -j8
> > In file included from tools/generated/lib/aes/aes-encrypt.c:1:
> > ../tools/../lib/aes/aes-encrypt.c:19:10: fatal error: openssl/engine.h: No such file or directory
> > 19 | #include <openssl/engine.h>
> > | ^~~~~~~~~~~~~~~~~~
> > In file included from tools/generated/lib/rsa/rsa-sign.c:1:
> > ../tools/../lib/rsa/rsa-sign.c:22:10: fatal error: openssl/engine.h: No such file or directory
> > 22 | #include <openssl/engine.h>
> > | ^~~~~~~~~~~~~~~~~~
> > compilation terminated.
> >
> > Github provides the Azure CI pipeline as a free service. I wouldn't
> > expect them to retain logs at that tier.
> >
>
> As said, I don't think we are testing OpenSSL providers in test/py.
> rsa-sign.c and aes-encrypt.c can work perfectly fine without using
> engines, but currently it requires the engine.h header file to compile
> (and if missing, then the functions, macros, constants defined in that
> header file will need to be compiled out like done in this patch) even
> if it doesn't use engines at runtime.
>
> The issue is not that OpenSSL is built without engine support, it's
> rather that Fedora has decided to stop shipping openssl/engine.h by
> default. For some reason, they still compile OpenSSL with engine support
> (I don't understand why). You are fixing a build issue, sure, and we
> must fix it, but I don't think this patch is doing it the proper way. So
> I am asking again, did you test with an actual OpenSSL provider (and no,
> the implicit file: scheme "provider" doesn't count)?
>
> See
> https://lore.kernel.org/u-boot/d8b15735-0b3f-4a72-a2a2-9ac6e12f4dbf@cherry.de/T/#m7454283f474aaee33736e1ae7154569da846f14a
>
> >
> >>> Tested-by Enric Balletbo i Serra <eballetb@redhat.com>
Chiming in here as one of the folks testing this series. I'll admit I
got completely lost in the mailing list jungle trying to follow this
massive wall of messages, but I finally dug into the logs and ran some
more hands-on tests.
I think the scope of this patch was strictly a backend API migration
to unblock U-Boot compilation on distros like Fedora, not supporting
real providers out of the gate.
To verify that Eddie's patch successfully switches the underlying
architecture to the OpenSSL 3.x Provider API, I ran a local system
trace using a file-based path (-k .). The strace confirmed that the
legacy engine hooks are bypassed and the provider module is used
instead. The underlying provider plumbing itself is completely solid
for standard file-based keys [1].
With that, plus the tests passing in CI, I assumed the new backend
plumbing worked as intended. From a functional standpoint, the outcome
is identical: your images get signed securely just like before. The
only difference is the internal machinery OpenSSL uses under the hood.
But you are entirely right. I didn't test against a real hardware
engine initially, and looking closer, I agree the current patch does
break backward compatibility for anyone using a real Engine on OpenSSL
3.x.
I did a run using SoftHSM to see exactly why:
Before this patch: Passing -N pkcs11 successfully triggered the legacy
engine. The signing succeeded.
With this patch: The OpenSSL 3.x version check unconditionally hijacks
the flow, ignores the -N engine flag, and forces execution down the
Provider path. The strict OSSL_STORE_open parser receives the mangled
string and instantly fails with ENOENT because is unconditionally
appending /[key-name-hint].key.
My understanding is that to fix the Fedora build issues without
breaking existing workflows, Eddie just needs to adjust the control
flow for v4 like you suggested:
If the user explicitly commands an engine via -N, prioritize the
legacy engine path—guarding the engine headers and blocks with #ifndef
OPENSSL_NO_ENGINE instead of a blanket OpenSSL version check.
If no engine flag is passed, route seamlessly into the new Provider API backend.
This fixes the compilation bottleneck on some distros, completely
preserves legacy engine setups, and allows real providers URI
string-mangling issue to be handled as clean, independent follow-up
patches.
Does it makes sense?
Cheers,
Enric Balletbo
[1]
$ strace -f -e trace=openat ./tools/mkimage -F -k . -K mock.dtb -r
fitImage 2>&1 | grep -iE "pkcs11|ossl-modules|\.key"
openat(AT_FDCWD, "/etc/pki/tls/openssl.d/pkcs11-provider.conf", O_RDONLY) = 7
openat(AT_FDCWD, "/usr/lib64/ossl-modules/pkcs11.so", O_RDONLY|O_CLOEXEC) = 5
openat(AT_FDCWD, "./dev.key", O_RDONLY) = 5
FIT description: Dummy FIT image for testing providers
Created: Wed Jun 3 19:26:03 2026
Image 0 (kernel-1)
Description: Dummy Kernel
Created: Wed Jun 3 19:26:03 2026
Type: Kernel Image
Compression: uncompressed
Data Size: 100 Bytes = 0.10 KiB = 0.00 MiB
Architecture: ARM
OS: Linux
Load Address: 0x80008000
Entry Point: 0x80008000
Hash algo: sha256
Hash value: cd00e292c5970d3c5e2f0ffa5171e555bc46bfc4faddfb4a418b6840b86e79a3
Default Configuration: 'config-1'
Configuration 0 (config-1)
Description: Boot configuration
Kernel: kernel-1
Sign algo: sha256,rsa2048:dev
Sign value:
4b88172afa850fb98d4c81662b043fedfebd7db55be8011aa464570e58289cf58939ea3adfafd9dd808362cfbd830fccaf806bbdc3d221922ec4e59a478a526904b1f39afa28d770b2808ed727825953eb8e4f1387faf3396e6f0e8d1a5be1c4b698e94e383d466371b8469638a3d3e1e1a3385eb4752c7554d09289face527d60262f274c8ab12ae28069a51dbce19505578e391e2fc25a9df915b6917cf057b5b2ecd9ede7db81a0be1cb106db801f75500562902f45fbed9df5b87c61cabc854cf429d25f5977813bdb239b38e27ba461b752c612068ae1dc2566d3456d2396828bfaea1a946b77d0479ee720574d6068409353e4554d0dd5c56f1a0a6228
Timestamp: Wed Jun 3 19:26:16 2026
Signature written to 'fitImage', node '/configurations/config-1/signature-1'
Public key written to 'mock.dtb', node '/signature/key-dev'
./tools/fit_check_sign -f fitImage -k mock.dtb
Verifying Hash Integrity for node 'config-1'... sha256,rsa2048:dev+
Verified OK, loading images
## Loading kernel (any) from FIT Image at 7f0d23b65000 ...
Using 'config-1' configuration
Verifying Hash Integrity ...
sha256,rsa2048:dev+
OK
Trying 'kernel-1' kernel subimage
Description: Dummy Kernel
Created: Wed Jun 3 19:26:03 2026
Type: Kernel Image
Compression: uncompressed
Data Size: 100 Bytes = 0.10 KiB = 0.00 MiB
Architecture: ARM
OS: Linux
Load Address: 0x80008000
Entry Point: 0x80008000
Hash algo: sha256
Hash value:
cd00e292c5970d3c5e2f0ffa5171e555bc46bfc4faddfb4a418b6840b86e79a3
Verifying Hash Integrity ...
sha256+
OK
Decrypting Data ...
OK
Loading Kernel Image to 0
## Loading fdt (any) from FIT Image at 7f0d23b65000 ...
Using 'config-1' configuration
Verifying Hash Integrity ...
sha256,rsa2048:dev+
OK
Could not find subimage node type 'fdt'
## Loading ramdisk (any) from FIT Image at 7f0d23b65000 ...
Using 'config-1' configuration
Verifying Hash Integrity ...
sha256,rsa2048:dev+
OK
Could not find subimage node type 'ramdisk'
Signature check OK
> >>> Tested-by Mark Kettenis <mark.kettenis@xs4all.nl>
> >>
> >> Please do not forget to add the colon after Tested-by so it actually makes
> >> it a git trailer instead of just text.
> >>
> >
> > That's an unlucky typo because I added the trailers manually.
> >
>
> It happens :)
>
> For your info, with b4
> (https://b4.docs.kernel.org/en/latest/contributor/overview.html) you can
> run b4 trailers -u (if you started the series with b4, though you can
> probably create a new branch from an existing series with b4 prep
> --from-thread?) and it'll automatically fetch the trailers from answers
> on the ML and add them to the appropriate patches.
>
> >>> Signed-off-by: Eddie Kovsky <ekovsky@redhat.com>
> >>> ---
> >>> Changes in v4:
> >>> - Add comment that @engine pointer is null when using pkcs11 provider
> >>> - Remove extra line break
> >>> - Add pkcs11-provider package to build dependencies
> >>> v3: https://lore.kernel.org/u-boot/20260120164524.253188-1-ekovsky@redhat.com/
> >>>
> >>> 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/
> >>> ---
> >>> doc/build/gcc.rst | 4 +-
> >>> lib/aes/aes-encrypt.c | 4 +-
> >>> lib/rsa/rsa-sign.c | 102 ++++++++++++++++++++++++++++++++++++++--
> >>> tools/docker/Dockerfile | 1 +
> >>> 4 files changed, 103 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> >>> index 1fef718ceecb..29a6a632e7e3 100644
> >>> --- a/doc/build/gcc.rst
> >>> +++ b/doc/build/gcc.rst
> >>> @@ -25,8 +25,8 @@ Depending on the build targets further packages maybe needed
> >>> sudo apt-get install bc bison build-essential coccinelle \
> >>> device-tree-compiler dfu-util efitools flex gdisk graphviz imagemagick \
> >>> - libgnutls28-dev libguestfs-tools libncurses-dev \
> >>> - libpython3-dev libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl \
> >>> + libgnutls28-dev libguestfs-tools libncurses-dev libpython3-dev \
> >>> + libsdl2-dev libssl-dev lz4 lzma lzma-alone openssl pkcs11-provider \
> >>> pkg-config python3 python3-asteval python3-coverage python3-filelock \
> >>> python3-pkg-resources python3-pycryptodome python3-pyelftools \
> >>> python3-pytest python3-pytest-xdist python3-sphinxcontrib.apidoc \
> >>> 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
> >>
> >> Considering there are no other changes in this file, is this include
> >> actually needed?
> >>
> >
> > The header is still needed to maintain backward compatibility for engine
> > support. I maintained the #ifdef style to be consistent with its usage
> > elsewhere, based on previous feedback.
> >
>
> What am I trying to say here is no symbol in the file comes from
> openssl/engine.h and I can compile the file perfectly fine without this
> header included, so we should probably get rid of this include.
>
> >>> #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..f456f3c58e65 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
> >>> +
> >>
> >> Sorry but that's a NACK.
> >>
> >> As far as I can tell, this effectively disables using engines when your host
> >> openssl version is 3.0+, which we currently support (and I currently use
> >> it). You can have this for 4.0+ as engine support has been removed, but not
> >> for 3.x.
> >>
> >
> > You don't get a choice here. OpenSSL deprecated the Engine API starting
>
> Providers and engines can cohabit in OpenSSL 3.x, so that is possible (I
> tested it locally with the binman test suite, with both providers and
> engines local test suites working with the same OpenSSL version (Fedora
> 44 + openssl-devel-engine).
>
> > with Version 3.0, and removed it completely starting with Version 4.0.
> > If the environment you're building U-Boot in has a recent version of
> > OpenSSL then Engines are no longer available to you.
> >
>
> This only happens because Fedora (and not Debian, or ArchLinux) decided
> to stop shipping an engine.h header file (while still compiling OpenSSL
> with engine support).
>
> I can use engines with OpenSSL 3.x, with the current state in U-Boot, so
> you cannot unilaterally decide to break that.
>
> What you can do is either remove support for OpenSSL engines entirely
> when it's factually not possible to build mkimage with support for it
> (and no, checking for OpenSSL 3.x is not enough). That is, when OpenSSL
> is built/shipped without engine support (that is, for Fedora, unless
> openssl-devel-engine is installed). Or migrate engine support to use the
> provider API (by e.g. prepending org.openssl.engine: in the OSSL_STORE
> scheme) for OpenSSL 3.x. The user API must be identical though, what
> works with engines on OpenSSL 1.x an OpenSSL 3.x with engine support via
> the provider API must be the same. I would suggest to NOT go for the
> second option as I don't think it's worth the added complexity.
>
> >>> +#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)
> >>> +
> >>
> >> Is this really related to the PKCS11 provider? I think there's a mix between
> >> "using the provider API" and "using the pkcs11 provider".
> >>
> >
> > It's a helper macro used for the Provider path when OpenSSL is v3 or
> > later.
> >
>
> I'm arguing about the macro being called USE_PKCS11_PROVIDER while
> having nothing to do with pkcs11. It's misleading.
>
> >>> +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);
> >>> +
> >>
> >> main.c?
> >>
> >
> > Not clear what you are referring to here.
> >
>
> The error message would state "At main.c:" which is absolutely not
> related to any file this is called from. I guess maybe you want __FILE__
> here instead?
>
> >>> + 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)
> >>> {
> >>> @@ -94,10 +134,11 @@ err_cert:
> >>> *
> >>> * @keydir: Key prefix
> >>> * @name Name of key
> >>> - * @engine Engine to use
> >>> + * @engine Engine to use or NULL when using pkcs11 provider
> >>> * @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,21 +198,24 @@ static int rsa_engine_get_pub_key(const char *keydir, const char *name,
> >>> return 0;
> >>> }
> >>> +#endif
> >>> /**
> >>> * rsa_get_pub_key() - read a public key
> >>> *
> >>> * @keydir: Directory containing the key (PEM file) or key prefix (engine)
> >>> * @name Name of key file (will have a .crt extension)
> >>> - * @engine Engine to use
> >>> + * @engine Engine to use or NULL when using pkcs11 provider
> >>> * @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)
> >>> */
> >>> 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
> >>
> >> We should probably at least print something when engines aren't supported
> >> but the engine parameter is passed, or maybe even fail as that's an
> >> incorrect configuration.
> >>
> >
> > We discussed this in a previous revision and decided the comment was
> > sufficient. The correct path will be taken depending on which version of
> > the API is available.
> >
>
> mkimage will still happily accept the -N argument which sets the engine
> parameter, even if engines aren't supported.
>
> I think if you have a file key with the same name as in the engine,
> you'll then sign with the wrong key.
>
> Think
>
> mkimage -N myengine -g key
>
> and you have a key.key in the local directory. If you compile with
> OpenSSL engines supported, you'll essentially use myengine:key, if not,
> you'll use key.key, without mkimage even notifying you by using the same
> command.
>
> >>> return rsa_pem_get_pub_key(keydir, name, evpp);
> >>> }
> >>> @@ -207,13 +251,44 @@ 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)");
> >>> +
> >>
> >> Why are we unconditionally loading providers and specifically the pkcs11
> >> one? As far as I could tell we should know from the URI whether a key from a
> >> provider is requested by checking if a colon is in the URI. And we can
> >> automatically load the requested provider based on that?
> >>
> >> Otherwise the easiest answer to this could simply be: "let the user
> >> configure this externally via OPENSSL_CONF environment variable".
> >>
> >> Do we also need to update tools/mkimage.c (and doc/mkimage.1) to remove -N
> >> engine from usage if the tool doesn't support it? Do we also need to update
> >> doc/usage/fit/signature.rst to explain how to use pkcs11 provider?
> >>
> >> FYI, we (my employer) are migrating from engines to providers for signing
> >> FIT images with binman so I'm starting to have a look on how to do this now.
> >>
> >> Cheers,
> >> Quentin
> >>
> >
> > Additional changes may be needed down the road. I'm sure additional
> > patches will be welcome. The scope of this change is to introduce
> > support for the new API while continuing to support developers who are
> > still building in environments using the deprecated Engine API.
> >
>
> But it's not doing that? This is removing support for OpenSSL engines on
> all systems shipping OpenSSL 3.x? We cannot accept patches that break
> things under the pretense there can be follow-up patches. I'm not asking
> support for signing with OpenSSL providers in this series, I'm asking
> you to not break existing users.
>
> I'm still unsure what the goal of this patch is. It says it adds support
> for OpenSSL provider API but I don't think provider support was actually
> tested. It also says U-Boot cannot be built on Fedora anymore due to
> engine.h missing. Both can be be separate topics. The urgent task is to
> fix building on Fedora, supporting providers can then be tackled later
> on (by either you, or someone else; e.g. I already have support for
> providers locally, just need to write nice commit logs and wait on what
> this patch will become here). I commented out everything related to
> engines and ran the testSignSimple, testSignExactFIT, testSignNonFit,
> testSignMissingMkimage and testSplPubkeyDtb to check whether using keys
> was still supported (meaning, we aren't forced to migrate to the
> OSSL_STORE interface for simple keys).
>
> I'd be fine with guarding all engine support with
> if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0)
> and making sure we at least warn (or fail) if an engine is asked by the
> user but we don't support it, so that you can build on Fedora again
> without openssl-devel-engine.
>
> Cheers,
> Quentin
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-06-04 6:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29 18:02 [PATCH v4] Add support for OpenSSL Provider API Eddie Kovsky
2026-04-30 7:54 ` Mattijs Korpershoek
2026-05-12 10:17 ` Quentin Schulz
2026-05-21 22:29 ` Eddie Kovsky
2026-05-22 14:37 ` Quentin Schulz
2026-06-04 6:22 ` Enric Balletbo i Serra
2026-05-20 10:28 ` Quentin Schulz
2026-05-21 12:43 ` Quentin Schulz
2026-05-20 11:32 ` Quentin Schulz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.