* [PATCH v3 0/4] fit: allow signing with an OpenSSL engine
@ 2025-11-21 17:14 Quentin Schulz
2025-11-21 17:14 ` [PATCH v3 1/4] fit: support signing with only an engine_id Quentin Schulz
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Quentin Schulz @ 2025-11-21 17:14 UTC (permalink / raw)
To: u-boot
Cc: Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass,
Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov,
Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner,
Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog,
Quentin Schulz
I have a couple of products whose U-Boot FIT is signed via a proprietary
OpenSSL engine which only expects the name of a "slot" to select the key
to sign data with.
Currently mkimage fit support expects either a key-dir (-k) or a
key-file (-G) as a toggle for signing, however this doesn't apply to our
usecase because we use an OpenSSL engine (so no key-file to provide)
which doesn't mimic a directory layout like key-dir implies. Moreover,
binman really expects private keys (.key extension) to be available in
this key-dir directory, which we of course cannot provide.
This series allows to sign a FIT image with mkimage (and binman) with
an OpenSSL engine, including PKCS11 and custom engines. If a key-dir
needs to be passed (which is typical for PKCS11), one can do so by using
fit,engine-keydir.
Note that the public key (.crt extension) still needs to be available if
one wants to embed it for signature verification (which is probably what
one wants to do :) ). It is probably possible to use the engine for
getting the public key instead of storing it on disk, but this needs to
be added to fdt_add_pubkey and then binman, through a mechanism
different from fit,engine*.
One issue though is that since binman resolves key paths absolutely and
that I don't believe an OpenSSL engine would happen to have the exact
same key_id value than a local absolute path, fit,encrypt and
fit,engine cannot cohabit. An issue for the next person who wants
an OpenSSL engine AND encrypt the same FIT image, I don't.
Note that LibreSSL supports neither engines nor providers as far as I
could tell (engine support has been explicitly removed).
Note that OpenSSL engines have been deprecated since 3.0 (Q3-2021),
however note that OpenSSL 3.5 still seems to support engines (git grep)
and is EOL end of Q1 2030.
+Cc Eddie who's working on OpenSSL provider support, maybe we can work
together on supporting both engines and providers via the same API we
expose to the user?
+Cc Wolfgang who seems interested in signing with PKCS11 engine which
should now be supported.
+Cc Peter who seems interested according to v1
If anyone has an idea on how to test PKCS11 with SOftHSMv2 with id=
passed in fit,engine-keydir, I'm all ears.
I'm also wondering if the explanation around fit,engine-keydir aren't
too much. After all, they are passed verbatim to mkimage as -k argument
and the special cases are all specific to mkimage and not binman.
This depends on https://lore.kernel.org/all/20251121-pkcs11-ci-v1-1-8ba6da56c2f5@cherry.de/T/#u
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Changes in v3:
- did NOT add T-b and R-B on patch 3 and 4 since there's been some
rework,
- added R-b on patch 2,
- reworded commit log of patch 3,
- reworded documentation of fit,engine and the special case of pkcs11 to
hopefully be easier to digest,
- put fit,engine and fit,engine-keydir extraction into a separate
function,
- fixed issues due to modification of the environment in tests failing
other tests, by using unittest.mock.patch.dict() on os.environ as
suggested by the unittest.mock doc,
- added test for fit,encrypt + fit,engine raising an issue to keep 100%
coverage for fit etype,
- fixed issue when running binman with only one process (-T or -P 1) by
renaming uniquely files and directories created in the input dir in
individual tests as it is shared among all tests run by a process,
- removed apt-get install of the lib packages in the test as
Bintool.apt_install() is racey (apt itself is...) and would fail when
running binman with multiple processes. The package is now part of the
CI container (well, in a different series). If it's not detected, the
test is skipped.
- Link to v2: https://patch.msgid.link/20251117-binman-engine-v2-0-e053f683c0b8@cherry.de
Changes in v2:
- added R-b on patch 1,
- did NOT add T-b and R-B on patch 3 since there's been some rework,
- added binman test for signing with dummy RSA engine,
- added binman test for signing with SoftHSMv2 PKCS11 engine,
- added binman test for signing with SoftHSMv2 PKCS11 engine and
specific keydir (which contains an object= to tackle that special
case)
- added dummy RSA engine for binman test,
- renamed property to fit,engine as my hunch is that it could be used
for encrypting as well (from the same engine with the same keydir but
likely a different key-name-hint so that a different keypair is used)
- added fit,engine-keydir to easily support PKCS11 with arguments other
than object= taken from key-name-hint,
- Link to v1: https://patch.msgid.link/20251031-binman-engine-v1-0-c13c1b5dac43@cherry.de
---
Quentin Schulz (4):
fit: support signing with only an engine_id
tools: binman: mkimage: add support for passing the engine
tools: binman: fit: add support for OpenSSL engines
tools: binman: fit: add tests for signing with an OpenSSL engine
tools/binman/btool/mkimage.py | 5 +-
tools/binman/btool/softhsm2_util.py | 21 ++
tools/binman/entries.rst | 54 ++++-
tools/binman/etype/fit.py | 93 ++++++++-
tools/binman/ftest.py | 223 +++++++++++++++++++++
tools/binman/test/340_dummy-rsa4096.crt | 31 +++
tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++
.../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++
.../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++
.../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++
tools/binman/test/340_openssl.conf | 10 +
tools/binman/test/340_softhsm2.conf | 16 ++
tools/binman/test/Makefile | 6 +-
tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++
tools/fit_image.c | 3 +-
tools/image-host.c | 4 +-
16 files changed, 1001 insertions(+), 12 deletions(-)
---
base-commit: a642f401d700034c82128defc2d6f9c18065de1f
change-id: 20251030-binman-engine-e349b02696d0
Best regards,
--
Quentin Schulz <quentin.schulz@cherry.de>
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v3 1/4] fit: support signing with only an engine_id 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz @ 2025-11-21 17:14 ` Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 2/4] tools: binman: mkimage: add support for passing the engine Quentin Schulz ` (3 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Quentin Schulz @ 2025-11-21 17:14 UTC (permalink / raw) To: u-boot Cc: Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz From: Quentin Schulz <quentin.schulz@cherry.de> Currently, when one wants to use an OpenSSL engine to sign a FIT image, one needs to pass a keydir (via -k) to mkimage which will then be prepended to the value of the key-name-hint before being passed as key_id argument to the OpenSSL Engine API, or pass a keyfile (via -G) to mkimage. My OpenSSL engine only has "slots" which are not mapped like directories, so using keydir is not proper, though I could simply have -k '' I guess but this won't work currently with binman anyway. Additionally, passing a keyfile (-G) when using an engine doesn't make sense as the key is stored in the engine. Let simply allow FIT images be signed if both keydir and keyfile are missing but an engine is to be used. The keyname member is already filled by looking at key-name-hint property in the FIT and passed to the engine, which is exactly what is needed here. Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com> Reviewed-by: Simon Glass <sjg@chromium.org> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> --- tools/fit_image.c | 3 ++- tools/image-host.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/fit_image.c b/tools/fit_image.c index 0306333141e..694bb927c7d 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -26,7 +26,8 @@ static struct legacy_img_hdr header; static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname) { - bool signing = IMAGE_ENABLE_SIGN && (params->keydir || params->keyfile); + bool signing = IMAGE_ENABLE_SIGN && + (params->keydir || params->keyfile || params->engine_id); struct stat sbuf; void *fdt; int fd; diff --git a/tools/image-host.c b/tools/image-host.c index 21dd7f2d922..54df86316ae 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -696,7 +696,7 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile, strlen(FIT_HASH_NODENAME))) { ret = fit_image_process_hash(fit, image_name, noffset, data, size); - } else if (IMAGE_ENABLE_SIGN && (keydir || keyfile) && + } else if (IMAGE_ENABLE_SIGN && (keydir || keyfile || engine_id) && !strncmp(node_name, FIT_SIG_NODENAME, strlen(FIT_SIG_NODENAME))) { ret = fit_image_process_sig(keydir, keyfile, keydest, @@ -1366,7 +1366,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile, } /* If there are no keys, we can't sign configurations */ - if (!IMAGE_ENABLE_SIGN || !(keydir || keyfile)) + if (!IMAGE_ENABLE_SIGN || !(keydir || keyfile || engine_id)) return 0; /* Find configurations parent node offset */ -- 2.51.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/4] tools: binman: mkimage: add support for passing the engine 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 1/4] fit: support signing with only an engine_id Quentin Schulz @ 2025-11-21 17:14 ` Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines Quentin Schulz ` (2 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Quentin Schulz @ 2025-11-21 17:14 UTC (permalink / raw) To: u-boot Cc: Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz From: Quentin Schulz <quentin.schulz@cherry.de> mkimage has support for OpenSSL engines but binman currently doesn't for direct callers of mkimage (e.g. the fit etype). This prepares for adding support for OpenSSL engines for signing elements of a FIT image, which will done in the next commit. Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com> Reviewed-by: Simon Glass <sjg@chromium.org> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> --- tools/binman/btool/mkimage.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py index 23c0a8e490d..1dbd466f438 100644 --- a/tools/binman/btool/mkimage.py +++ b/tools/binman/btool/mkimage.py @@ -22,7 +22,7 @@ class Bintoolmkimage(bintool.Bintool): # pylint: disable=R0913 def run(self, reset_timestamp=False, output_fname=None, external=False, - pad=None, align=None, keys_dir=None): + pad=None, align=None, keys_dir=None, engine=None): """Run mkimage Args: @@ -35,6 +35,7 @@ class Bintoolmkimage(bintool.Bintool): signatures align: Bytes to use for alignment of the FIT and its external data keys_dir: Path to directory containing private and encryption keys + engine: Name of the OpenSSL engine to use """ args = [] if external: @@ -49,6 +50,8 @@ class Bintoolmkimage(bintool.Bintool): args += ['-k', f'{keys_dir}'] if output_fname: args += ['-F', output_fname] + if engine: + args += ['-N', engine] return self.run_cmd(*args) def fetch(self, method): -- 2.51.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 1/4] fit: support signing with only an engine_id Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 2/4] tools: binman: mkimage: add support for passing the engine Quentin Schulz @ 2025-11-21 17:14 ` Quentin Schulz 2025-11-25 22:15 ` Simon Glass 2025-11-21 17:15 ` [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine Quentin Schulz 2025-12-07 14:04 ` [PATCH v3 0/4] fit: allow " Tom Rini 4 siblings, 1 reply; 18+ messages in thread From: Quentin Schulz @ 2025-11-21 17:14 UTC (permalink / raw) To: u-boot Cc: Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz From: Quentin Schulz <quentin.schulz@cherry.de> This adds support for using an OpenSSL engine for signing a FIT image. To use it, one should set the fit,engine property at the FIT node level with the engine to use. This will in turn call mkimage with the -N option. The -k argument to mkimage can be specified via fit,engine-keydir. If not specified, -k is not passed to mkimage. This property is especially useful for pkcs11 engine to specify slots, token label, etc... As far as I could tell, mkimage encrypts and signs a FIT in one go, thus the -k argument applies to both signing and encrypting. Considering we reuse the -k argument for two different meanings (info to pass to the engine when using an engine otherwise the directory where keys are stored), we cannot reasonably encrypt using local keys and signing with an engine, hence the enforced check. I believe it should be possible to support encrypting and signing with the same engine (using different key pairs of course, via different key-name-hint likely), but this is left for the next person to implement. This is why the property is named fit,engine and not fit,sign-engine. Ditto for fit,engine-keydir. The public key (with .crt extension) is still required if it needs to be embedded in the SPL DTB for example. We could probably support retrieving the public key from an engine, but this is a change to make to fdt_add_pubkey.c. Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> --- tools/binman/entries.rst | 54 +++++++++++++++++++++++++-- tools/binman/etype/fit.py | 93 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 140 insertions(+), 7 deletions(-) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 8922d6cd070..a81fcbd3891 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -885,9 +885,10 @@ The top-level 'fit' node supports the following special properties: fit,sign Enable signing FIT images via mkimage as described in - verified-boot.rst. If the property is found, the private keys path - is detected among binman include directories and passed to mkimage - via -k flag. All the keys required for signing FIT must be + verified-boot.rst. + If the property is found and fit,engine is not set, the private + keys path is detected among binman include directories and passed to + mkimage via -k flag. All the keys required for signing FIT must be available at time of signing and must be located in single include directory. @@ -898,6 +899,53 @@ The top-level 'fit' node supports the following special properties: required for encrypting the FIT must be available at the time of encrypting and must be located in a single include directory. + Incompatible with fit,engine. + + fit,engine + Indicates the OpenSSL engine to use for signing the FIT image. This + is passed to mkimage via the `-N` flag. Example:: + + fit,engine = "my-engine"; + + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. + + When `fit,engine` is set to `pkcs11`, the following applies: + + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is + prefixed with `pkcs11:object=` before being passed to the OpenSSL + engine API:: + + pkcs11:object=<key-name-hint> + + - If `fit,engine-keydir` contains either `object=` or `id=`, its + value is passed verbatim to the OpenSSL engine API, + + - Otherwise, the value of `fit,engine-keydir` is followed by + `;object=` and the value of `key-name-hint` before being passed + to the OpenSSL engine API:: + + <fit,engine-keydir>;object=<key-name-hint> + + If `fit,engine` is set to something different than `pkcs11`, the + value of `key-name-hint` (prefixed with the value of + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL + engine API. + + Depends on fit,sign. + + Incompatible with fit,encrypt. + + fit,engine-keydir + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine + is to be used for signing the FIT image. Example:: + + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; + + Read `fit,engine` documentation for more info on special cases when + using `pkcs11` as engine. + + Depends on fit,engine. + Substitutions ~~~~~~~~~~~~~ diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index db40479d30e..f28b1e6b4cb 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -104,9 +104,10 @@ class Entry_fit(Entry_section): fit,sign Enable signing FIT images via mkimage as described in - verified-boot.rst. If the property is found, the private keys path - is detected among binman include directories and passed to mkimage - via -k flag. All the keys required for signing FIT must be + verified-boot.rst. + If the property is found and fit,engine is not set, the private + keys path is detected among binman include directories and passed to + mkimage via -k flag. All the keys required for signing FIT must be available at time of signing and must be located in single include directory. @@ -117,6 +118,53 @@ class Entry_fit(Entry_section): required for encrypting the FIT must be available at the time of encrypting and must be located in a single include directory. + Incompatible with fit,engine. + + fit,engine + Indicates the OpenSSL engine to use for signing the FIT image. This + is passed to mkimage via the `-N` flag. Example:: + + fit,engine = "my-engine"; + + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. + + When `fit,engine` is set to `pkcs11`, the following applies: + + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is + prefixed with `pkcs11:object=` before being passed to the OpenSSL + engine API:: + + pkcs11:object=<key-name-hint> + + - If `fit,engine-keydir` contains either `object=` or `id=`, its + value is passed verbatim to the OpenSSL engine API, + + - Otherwise, the value of `fit,engine-keydir` is followed by + `;object=` and the value of `key-name-hint` before being passed to + the OpenSSL engine API:: + + <fit,engine-keydir>;object=<key-name-hint> + + If `fit,engine` is set to something different than `pkcs11`, the + value of `key-name-hint` (prefixed with the value of + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL + engine API. + + Depends on fit,sign. + + Incompatible with fit,encrypt. + + fit,engine-keydir + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine + is to be used for signing the FIT image. Example:: + + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; + + Read `fit,engine` documentation for more info on special cases when + using `pkcs11` as engine. + + Depends on fit,engine. + Substitutions ~~~~~~~~~~~~~ @@ -588,6 +636,29 @@ class Entry_fit(Entry_section): return paths[0] if len(paths) else None + def _get_fit_engine(self): + """Detect whether an OpenSSL engine is to be used for the FIT + + Returns: + Tuple(str, str): Name of the engine to use, as first element of the + Tuple. None if no engine to use. + keydir arguments to pass with the engine to the + OpenSSL API, as second element of the Tuple. None + if no keydir to pass. + """ + engine = None + engine_keydir = None + + prop = self._fit_props.get('fit,engine') + if prop is not None: + engine = prop.value + + prop = self._fit_props.get('fit,engine-keydir') + if prop is not None: + engine_keydir = prop.value + + return engine, engine_keydir + def BuildSectionData(self, required): """Build FIT entry contents @@ -620,7 +691,21 @@ class Entry_fit(Entry_section): args.update({'align': fdt_util.fdt32_to_cpu(align.value)}) if (self._fit_props.get('fit,sign') is not None or self._fit_props.get('fit,encrypt') is not None): - args.update({'keys_dir': self._get_keys_dir(data)}) + engine = None + keydir = None + + # Engine only supported for signing for now + if self._fit_props.get('fit,sign') is not None: + engine, keydir = self._get_fit_engine() + + args.update({'engine': engine}) + # If no engine, keys must exist locally, find them + if engine is None: + keydir = self._get_keys_dir(data) + elif self._fit_props.get('fit,encrypt') is not None: + self.Raise('fit,engine currently does not support encryption') + + args.update({'keys_dir': keydir}) if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, **args) is None: if not self.GetAllowMissing(): -- 2.51.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines 2025-11-21 17:14 ` [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines Quentin Schulz @ 2025-11-25 22:15 ` Simon Glass 2025-11-26 11:28 ` Quentin Schulz 0 siblings, 1 reply; 18+ messages in thread From: Simon Glass @ 2025-11-25 22:15 UTC (permalink / raw) To: Quentin Schulz Cc: u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz Hi Quentin, On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > From: Quentin Schulz <quentin.schulz@cherry.de> > > This adds support for using an OpenSSL engine for signing a FIT image. > To use it, one should set the fit,engine property at the FIT node level > with the engine to use. This will in turn call mkimage with the -N > option. > > The -k argument to mkimage can be specified via fit,engine-keydir. If > not specified, -k is not passed to mkimage. This property is especially > useful for pkcs11 engine to specify slots, token label, etc... > > As far as I could tell, mkimage encrypts and signs a FIT in one go, thus > the -k argument applies to both signing and encrypting. Considering we > reuse the -k argument for two different meanings (info to pass to the > engine when using an engine otherwise the directory where keys are > stored), we cannot reasonably encrypt using local keys and signing with > an engine, hence the enforced check. I believe it should be possible to > support encrypting and signing with the same engine (using different > key pairs of course, via different key-name-hint likely), but this is > left for the next person to implement. > This is why the property is named fit,engine and not fit,sign-engine. > Ditto for fit,engine-keydir. > > The public key (with .crt extension) is still required if it needs to be > embedded in the SPL DTB for example. We could probably support > retrieving the public key from an engine, but this is a change to make > to fdt_add_pubkey.c. > > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > --- > tools/binman/entries.rst | 54 +++++++++++++++++++++++++-- > tools/binman/etype/fit.py | 93 +++++++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 140 insertions(+), 7 deletions(-) > > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst > index 8922d6cd070..a81fcbd3891 100644 > --- a/tools/binman/entries.rst > +++ b/tools/binman/entries.rst > @@ -885,9 +885,10 @@ The top-level 'fit' node supports the following special properties: > > fit,sign > Enable signing FIT images via mkimage as described in > - verified-boot.rst. If the property is found, the private keys path > - is detected among binman include directories and passed to mkimage > - via -k flag. All the keys required for signing FIT must be > + verified-boot.rst. > + If the property is found and fit,engine is not set, the private > + keys path is detected among binman include directories and passed to > + mkimage via -k flag. All the keys required for signing FIT must be > available at time of signing and must be located in single include > directory. > > @@ -898,6 +899,53 @@ The top-level 'fit' node supports the following special properties: > required for encrypting the FIT must be available at the time of > encrypting and must be located in a single include directory. > > + Incompatible with fit,engine. > + > + fit,engine > + Indicates the OpenSSL engine to use for signing the FIT image. This > + is passed to mkimage via the `-N` flag. Example:: > + > + fit,engine = "my-engine"; > + > + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. > + > + When `fit,engine` is set to `pkcs11`, the following applies: > + > + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is > + prefixed with `pkcs11:object=` before being passed to the OpenSSL > + engine API:: > + > + pkcs11:object=<key-name-hint> > + > + - If `fit,engine-keydir` contains either `object=` or `id=`, its > + value is passed verbatim to the OpenSSL engine API, > + > + - Otherwise, the value of `fit,engine-keydir` is followed by > + `;object=` and the value of `key-name-hint` before being passed > + to the OpenSSL engine API:: > + > + <fit,engine-keydir>;object=<key-name-hint> > + > + If `fit,engine` is set to something different than `pkcs11`, the > + value of `key-name-hint` (prefixed with the value of > + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL > + engine API. > + > + Depends on fit,sign. > + > + Incompatible with fit,encrypt. > + > + fit,engine-keydir > + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine > + is to be used for signing the FIT image. Example:: > + > + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; > + > + Read `fit,engine` documentation for more info on special cases when > + using `pkcs11` as engine. > + > + Depends on fit,engine. > + > Substitutions > ~~~~~~~~~~~~~ > > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py > index db40479d30e..f28b1e6b4cb 100644 > --- a/tools/binman/etype/fit.py > +++ b/tools/binman/etype/fit.py > @@ -104,9 +104,10 @@ class Entry_fit(Entry_section): > > fit,sign > Enable signing FIT images via mkimage as described in > - verified-boot.rst. If the property is found, the private keys path > - is detected among binman include directories and passed to mkimage > - via -k flag. All the keys required for signing FIT must be > + verified-boot.rst. > + If the property is found and fit,engine is not set, the private > + keys path is detected among binman include directories and passed to > + mkimage via -k flag. All the keys required for signing FIT must be > available at time of signing and must be located in single include > directory. > > @@ -117,6 +118,53 @@ class Entry_fit(Entry_section): > required for encrypting the FIT must be available at the time of > encrypting and must be located in a single include directory. > > + Incompatible with fit,engine. > + > + fit,engine > + Indicates the OpenSSL engine to use for signing the FIT image. This > + is passed to mkimage via the `-N` flag. Example:: > + > + fit,engine = "my-engine"; > + > + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. > + > + When `fit,engine` is set to `pkcs11`, the following applies: > + > + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is > + prefixed with `pkcs11:object=` before being passed to the OpenSSL > + engine API:: > + > + pkcs11:object=<key-name-hint> > + > + - If `fit,engine-keydir` contains either `object=` or `id=`, its > + value is passed verbatim to the OpenSSL engine API, > + > + - Otherwise, the value of `fit,engine-keydir` is followed by > + `;object=` and the value of `key-name-hint` before being passed to > + the OpenSSL engine API:: > + > + <fit,engine-keydir>;object=<key-name-hint> > + > + If `fit,engine` is set to something different than `pkcs11`, the > + value of `key-name-hint` (prefixed with the value of > + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL > + engine API. > + > + Depends on fit,sign. > + > + Incompatible with fit,encrypt. > + > + fit,engine-keydir > + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine > + is to be used for signing the FIT image. Example:: > + > + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; > + > + Read `fit,engine` documentation for more info on special cases when > + using `pkcs11` as engine. > + > + Depends on fit,engine. > + > Substitutions > ~~~~~~~~~~~~~ > > @@ -588,6 +636,29 @@ class Entry_fit(Entry_section): > > return paths[0] if len(paths) else None > > + def _get_fit_engine(self): > + """Detect whether an OpenSSL engine is to be used for the FIT > + > + Returns: > + Tuple(str, str): Name of the engine to use, as first element of the > + Tuple. None if no engine to use. > + keydir arguments to pass with the engine to the > + OpenSSL API, as second element of the Tuple. None > + if no keydir to pass. > + """ > + engine = None > + engine_keydir = None > + > + prop = self._fit_props.get('fit,engine') > + if prop is not None: > + engine = prop.value > + > + prop = self._fit_props.get('fit,engine-keydir') > + if prop is not None: > + engine_keydir = prop.value > + > + return engine, engine_keydir > + > def BuildSectionData(self, required): > """Build FIT entry contents > > @@ -620,7 +691,21 @@ class Entry_fit(Entry_section): > args.update({'align': fdt_util.fdt32_to_cpu(align.value)}) > if (self._fit_props.get('fit,sign') is not None or > self._fit_props.get('fit,encrypt') is not None): > - args.update({'keys_dir': self._get_keys_dir(data)}) > + engine = None > + keydir = None > + > + # Engine only supported for signing for now > + if self._fit_props.get('fit,sign') is not None: > + engine, keydir = self._get_fit_engine() > + > + args.update({'engine': engine}) > + # If no engine, keys must exist locally, find them > + if engine is None: > + keydir = self._get_keys_dir(data) > + elif self._fit_props.get('fit,encrypt') is not None: > + self.Raise('fit,engine currently does not support encryption') > + > + args.update({'keys_dir': keydir}) > if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, > **args) is None: > if not self.GetAllowMissing(): > > -- > 2.51.1 > Were there any changes on this one? It looks OK to me. You should be able to do: if 'fit,sign' not in self._fit_props Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines 2025-11-25 22:15 ` Simon Glass @ 2025-11-26 11:28 ` Quentin Schulz 0 siblings, 0 replies; 18+ messages in thread From: Quentin Schulz @ 2025-11-26 11:28 UTC (permalink / raw) To: Simon Glass, Quentin Schulz Cc: u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Simon, On 11/25/25 11:15 PM, Simon Glass wrote: > Hi Quentin, > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: >> >> From: Quentin Schulz <quentin.schulz@cherry.de> >> >> This adds support for using an OpenSSL engine for signing a FIT image. >> To use it, one should set the fit,engine property at the FIT node level >> with the engine to use. This will in turn call mkimage with the -N >> option. >> >> The -k argument to mkimage can be specified via fit,engine-keydir. If >> not specified, -k is not passed to mkimage. This property is especially >> useful for pkcs11 engine to specify slots, token label, etc... >> >> As far as I could tell, mkimage encrypts and signs a FIT in one go, thus >> the -k argument applies to both signing and encrypting. Considering we >> reuse the -k argument for two different meanings (info to pass to the >> engine when using an engine otherwise the directory where keys are >> stored), we cannot reasonably encrypt using local keys and signing with >> an engine, hence the enforced check. I believe it should be possible to >> support encrypting and signing with the same engine (using different >> key pairs of course, via different key-name-hint likely), but this is >> left for the next person to implement. >> This is why the property is named fit,engine and not fit,sign-engine. >> Ditto for fit,engine-keydir. >> >> The public key (with .crt extension) is still required if it needs to be >> embedded in the SPL DTB for example. We could probably support >> retrieving the public key from an engine, but this is a change to make >> to fdt_add_pubkey.c. >> >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >> --- >> tools/binman/entries.rst | 54 +++++++++++++++++++++++++-- >> tools/binman/etype/fit.py | 93 +++++++++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 140 insertions(+), 7 deletions(-) >> >> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst >> index 8922d6cd070..a81fcbd3891 100644 >> --- a/tools/binman/entries.rst >> +++ b/tools/binman/entries.rst >> @@ -885,9 +885,10 @@ The top-level 'fit' node supports the following special properties: >> >> fit,sign >> Enable signing FIT images via mkimage as described in >> - verified-boot.rst. If the property is found, the private keys path >> - is detected among binman include directories and passed to mkimage >> - via -k flag. All the keys required for signing FIT must be >> + verified-boot.rst. >> + If the property is found and fit,engine is not set, the private >> + keys path is detected among binman include directories and passed to >> + mkimage via -k flag. All the keys required for signing FIT must be >> available at time of signing and must be located in single include >> directory. >> >> @@ -898,6 +899,53 @@ The top-level 'fit' node supports the following special properties: >> required for encrypting the FIT must be available at the time of >> encrypting and must be located in a single include directory. >> >> + Incompatible with fit,engine. >> + >> + fit,engine >> + Indicates the OpenSSL engine to use for signing the FIT image. This >> + is passed to mkimage via the `-N` flag. Example:: >> + >> + fit,engine = "my-engine"; >> + >> + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. >> + >> + When `fit,engine` is set to `pkcs11`, the following applies: >> + >> + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is >> + prefixed with `pkcs11:object=` before being passed to the OpenSSL >> + engine API:: >> + >> + pkcs11:object=<key-name-hint> >> + >> + - If `fit,engine-keydir` contains either `object=` or `id=`, its >> + value is passed verbatim to the OpenSSL engine API, >> + >> + - Otherwise, the value of `fit,engine-keydir` is followed by >> + `;object=` and the value of `key-name-hint` before being passed >> + to the OpenSSL engine API:: >> + >> + <fit,engine-keydir>;object=<key-name-hint> >> + >> + If `fit,engine` is set to something different than `pkcs11`, the >> + value of `key-name-hint` (prefixed with the value of >> + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL >> + engine API. >> + >> + Depends on fit,sign. >> + >> + Incompatible with fit,encrypt. >> + >> + fit,engine-keydir >> + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine >> + is to be used for signing the FIT image. Example:: >> + >> + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; >> + >> + Read `fit,engine` documentation for more info on special cases when >> + using `pkcs11` as engine. >> + >> + Depends on fit,engine. >> + >> Substitutions >> ~~~~~~~~~~~~~ >> >> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py >> index db40479d30e..f28b1e6b4cb 100644 >> --- a/tools/binman/etype/fit.py >> +++ b/tools/binman/etype/fit.py >> @@ -104,9 +104,10 @@ class Entry_fit(Entry_section): >> >> fit,sign >> Enable signing FIT images via mkimage as described in >> - verified-boot.rst. If the property is found, the private keys path >> - is detected among binman include directories and passed to mkimage >> - via -k flag. All the keys required for signing FIT must be >> + verified-boot.rst. >> + If the property is found and fit,engine is not set, the private >> + keys path is detected among binman include directories and passed to >> + mkimage via -k flag. All the keys required for signing FIT must be >> available at time of signing and must be located in single include >> directory. >> >> @@ -117,6 +118,53 @@ class Entry_fit(Entry_section): >> required for encrypting the FIT must be available at the time of >> encrypting and must be located in a single include directory. >> >> + Incompatible with fit,engine. >> + >> + fit,engine >> + Indicates the OpenSSL engine to use for signing the FIT image. This >> + is passed to mkimage via the `-N` flag. Example:: >> + >> + fit,engine = "my-engine"; >> + >> + A `-k` argument for mkimage may be passed via `fit,engine-keydir`. >> + >> + When `fit,engine` is set to `pkcs11`, the following applies: >> + >> + - If `fit,engine-keydir` is absent, the value of `key-name-hint` is >> + prefixed with `pkcs11:object=` before being passed to the OpenSSL >> + engine API:: >> + >> + pkcs11:object=<key-name-hint> >> + >> + - If `fit,engine-keydir` contains either `object=` or `id=`, its >> + value is passed verbatim to the OpenSSL engine API, >> + >> + - Otherwise, the value of `fit,engine-keydir` is followed by >> + `;object=` and the value of `key-name-hint` before being passed to >> + the OpenSSL engine API:: >> + >> + <fit,engine-keydir>;object=<key-name-hint> >> + >> + If `fit,engine` is set to something different than `pkcs11`, the >> + value of `key-name-hint` (prefixed with the value of >> + `fit,engine-keydir` if present) and passed verbatim to the OpenSSL >> + engine API. >> + >> + Depends on fit,sign. >> + >> + Incompatible with fit,encrypt. >> + >> + fit,engine-keydir >> + Indicates the `-k` argument to pass to mkimage if an OpenSSL engine >> + is to be used for signing the FIT image. Example:: >> + >> + fit,engine-keydir = "pkcs11:model=xxx;manufacturer=xxx"; >> + >> + Read `fit,engine` documentation for more info on special cases when >> + using `pkcs11` as engine. >> + >> + Depends on fit,engine. >> + >> Substitutions >> ~~~~~~~~~~~~~ >> >> @@ -588,6 +636,29 @@ class Entry_fit(Entry_section): >> >> return paths[0] if len(paths) else None >> >> + def _get_fit_engine(self): >> + """Detect whether an OpenSSL engine is to be used for the FIT >> + >> + Returns: >> + Tuple(str, str): Name of the engine to use, as first element of the >> + Tuple. None if no engine to use. >> + keydir arguments to pass with the engine to the >> + OpenSSL API, as second element of the Tuple. None >> + if no keydir to pass. >> + """ >> + engine = None >> + engine_keydir = None >> + >> + prop = self._fit_props.get('fit,engine') >> + if prop is not None: >> + engine = prop.value >> + >> + prop = self._fit_props.get('fit,engine-keydir') >> + if prop is not None: >> + engine_keydir = prop.value >> + >> + return engine, engine_keydir >> + >> def BuildSectionData(self, required): >> """Build FIT entry contents >> >> @@ -620,7 +691,21 @@ class Entry_fit(Entry_section): >> args.update({'align': fdt_util.fdt32_to_cpu(align.value)}) >> if (self._fit_props.get('fit,sign') is not None or >> self._fit_props.get('fit,encrypt') is not None): >> - args.update({'keys_dir': self._get_keys_dir(data)}) >> + engine = None >> + keydir = None >> + >> + # Engine only supported for signing for now >> + if self._fit_props.get('fit,sign') is not None: >> + engine, keydir = self._get_fit_engine() >> + >> + args.update({'engine': engine}) >> + # If no engine, keys must exist locally, find them >> + if engine is None: >> + keydir = self._get_keys_dir(data) >> + elif self._fit_props.get('fit,encrypt') is not None: >> + self.Raise('fit,engine currently does not support encryption') >> + >> + args.update({'keys_dir': keydir}) >> if self.mkimage.run(reset_timestamp=True, output_fname=output_fname, >> **args) is None: >> if not self.GetAllowMissing(): >> >> -- >> 2.51.1 >> > > Were there any changes on this one? It looks OK to me. > Yes. You can see the git-range-diff between both versions with: $ b4 diff -v 2 3 -- https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t Also see cover letter where I list the changes made to the v3 compared to v2 and why I decided to not add your and Wolfgang's trailers. > You should be able to do: if 'fit,sign' not in self._fit_props > $ git grep "in self._fit_props" tools/binman/etype/fit.py returns nothing. I would rather have the same logic to get properties within a given file. Cheers, Quentin ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz ` (2 preceding siblings ...) 2025-11-21 17:14 ` [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines Quentin Schulz @ 2025-11-21 17:15 ` Quentin Schulz 2025-11-25 22:15 ` Simon Glass 2025-12-07 14:04 ` [PATCH v3 0/4] fit: allow " Tom Rini 4 siblings, 1 reply; 18+ messages in thread From: Quentin Schulz @ 2025-11-21 17:15 UTC (permalink / raw) To: u-boot Cc: Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz From: Quentin Schulz <quentin.schulz@cherry.de> This adds a test that signs a FIT and verifies the signature with fit_check_sign. OpenSSL engines are typically for signing with external HW so it's not that straight-forward to simulate. For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA 4096 private key is made available. It can be selected by setting the OpenSSL engine argument to dummy-rsa-engine. This can only be done if the engine is detected by OpenSSL, which works by setting the OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine is properly implementing what is expected from an RSA engine, but it seems to be enough for testing. For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 without specific hardware. The keypairs and tokens are generated on the fly. The "prod" token is generated with a different PIN (1234 instead of 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. Binman will not mess with the local SoftHSMv2 setup as it will only use tokens from a per-test temporary directory enforced via the temporary configuration file set via SOFTHSM2_CONF env variable in the tests. The files created in the input dir should NOT be named the same as it is shared between all tests in the same process (which is all tests when running binman with -P 1 or with -T). Once signed, it's checked with fit_check_sign with the associated certificate. Finally, a new softhsm2_util bintool is added so that we can initialize the token and import keypairs. On Debian, the package also brings libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It is not the only package required though, as it also needs p11-kit and libengine-pkcs11-openssl (the latter bringing the former). We can detect if it's properly installed by running openssl engine dynamic -c pkcs11. If that fails, we simply skip the test. The package is installed in the CI container by default. Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> --- tools/binman/btool/softhsm2_util.py | 21 ++ tools/binman/ftest.py | 223 +++++++++++++++++++++ tools/binman/test/340_dummy-rsa4096.crt | 31 +++ tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ tools/binman/test/340_openssl.conf | 10 + tools/binman/test/340_softhsm2.conf | 16 ++ tools/binman/test/Makefile | 6 +- tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ 11 files changed, 853 insertions(+), 1 deletion(-) diff --git a/tools/binman/btool/softhsm2_util.py b/tools/binman/btool/softhsm2_util.py new file mode 100644 index 00000000000..869221d841d --- /dev/null +++ b/tools/binman/btool/softhsm2_util.py @@ -0,0 +1,21 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright 2025 Cherry Embedded Solutions GmbH +# +"""Bintool implementation for SoftHSMv2 (softhsm2-util)""" + +from binman import bintool + + +class Bintoolsofthsm2_util(bintool.Bintool): + """SoftHSMv2 -- support tool for libsofthsm2""" + def __init__(self, name): + super().__init__('softhsm2-util', + 'SoftHSMv2 support tool for libsofthsm2', + version_args='-v') + + def fetch(self, method): + """Install softhsm2-util via APT """ + if method != bintool.FETCH_BIN: + return None + + return self.apt_install('softhsm2') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 925c39a530e..21ec48d86fd 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -7952,6 +7952,229 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIsNotNone(signature) self.assertIsNotNone(signature.props.get('value')) + def testFitSignEngineSimple(self): + """Test that image with FIT and signature nodes can be signed with an + OpenSSL Engine""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + entry_args = { + 'of-list': 'test-fdt1', + 'default-dt': 'test-fdt1', + 'atf-bl31-path': 'bl31.elf', + } + + x509_pubkey = '340_dummy-rsa4096.crt' + data = tools.read_file(self.TestFile(x509_pubkey)) + self._MakeInputFile('dev.crt', data) + + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + ossl_engines_path = TestFunctional._elf_testdir + # Make OpenSSL find our dummy-rsa-engine engine + with unittest.mock.patch.dict('os.environ', + {'OPENSSL_ENGINES': ossl_engines_path}): + data = self._DoReadFileDtb( + '340_fit_signature_engine.dts', + entry_args=entry_args, + extra_indirs=[test_subdir])[0] + + dtb = fdt.Fdt.FromData(data) + dtb.Scan() + + conf = dtb.GetNode('/configurations/conf-uboot-1') + self.assertIsNotNone(conf) + signature = conf.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + images = dtb.GetNode('/images') + self.assertIsNotNone(images) + for subnode in images.subnodes: + signature = subnode.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + some_dtb = tools.get_output_filename('source.dtb') + tools.run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-k', self._indir, + '-n', 'dev', '-r', 'conf', some_dtb) + tools.run('fit_check_sign', '-k', some_dtb, + '-f', tools.get_output_filename('fit.fit')) + + def testFitSignEncryptEngine(self): + """Test that FIT image binman is requested to sign with engine does not + request to encrypt as well""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + entry_args = { + 'of-list': 'test-fdt1', + 'default-dt': 'test-fdt1', + 'atf-bl31-path': 'bl31.elf', + } + + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb( + '340_fit_signature_engine_encrypt.dts', + entry_args=entry_args, + extra_indirs=[test_subdir]) + + self.assertIn( + 'fit,engine currently does not support encryption', + str(e.exception)) + + def testFitSignPKCS11Simple(self): + """Test that image with FIT and signature nodes can be signed with a + PKCS11 OpenSSL Engine""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + softhsm2_util = bintool.Bintool.create('softhsm2_util') + self._CheckBintool(softhsm2_util) + + try: + tools.run('openssl', 'engine', 'dynamic', '-c', 'pkcs11') + except ValueError: + self.skipTest('PKCS11 engine setup not functional, ' + 'did you install libengine-pkcs11-openssl?') + + prefix = "testFitSignPKCS11Simple." + # Configure SoftHSMv2 + data = tools.read_file(self.TestFile('340_softhsm2.conf')) + softhsm2_conf = self._MakeInputFile(f'{prefix}softhsm2.conf', data) + softhsm2_tokens_dir = self._MakeInputDir(f'{prefix}softhsm2.tokens') + + with open(softhsm2_conf, 'a') as f: + f.write(f'directories.tokendir = {softhsm2_tokens_dir}\n') + + # Generate pubkey DTB with random RSA4096 key + _, _, private_key, pubkey_dtb = self._PrepareSignEnv() + + with unittest.mock.patch.dict('os.environ', + {'SOFTHSM2_CONF': softhsm2_conf}): + tools.run('softhsm2-util', '--init-token', '--free', '--label', + 'U-Boot token', '--pin', '1111', '--so-pin', + '222222') + tools.run('softhsm2-util', '--import', private_key, '--token', + 'U-Boot token', '--label', 'test_key', '--id', '999999', + '--pin', '1111') + + # Make sure the private key can only be accessed through the engine + os.remove(private_key) + + entry_args = { + 'of-list': 'test-fdt1', + 'default-dt': 'test-fdt1', + 'atf-bl31-path': 'bl31.elf', + } + + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + + # Make OpenSSL use softhsm2 engine + ossl_conf = self.TestFile('340_openssl.conf') + with unittest.mock.patch.dict('os.environ', + {'OPENSSL_CONF': ossl_conf, + 'SOFTHSM2_CONF': softhsm2_conf}): + data = self._DoReadFileDtb( + '340_fit_signature_engine_pkcs11.dts', + entry_args=entry_args, + extra_indirs=[test_subdir])[0] + + dtb = fdt.Fdt.FromData(data) + dtb.Scan() + + conf = dtb.GetNode('/configurations/conf-uboot-1') + self.assertIsNotNone(conf) + signature = conf.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + images = dtb.GetNode('/images') + self.assertIsNotNone(images) + for subnode in images.subnodes: + signature = subnode.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + tools.run('fit_check_sign', '-k', pubkey_dtb, + '-f', tools.get_output_filename('fit.fit')) + + def testFitSignPKCS11Object(self): + """Test that image with FIT and signature nodes can be signed with a + PKCS11 OpenSSL Engine with a specified object=""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') + softhsm2_util = bintool.Bintool.create('softhsm2_util') + self._CheckBintool(softhsm2_util) + + try: + tools.run('openssl', 'engine', 'dynamic', '-c', 'pkcs11') + except ValueError: + self.skipTest('PKCS11 engine setup not functional, ' + 'did you install libengine-pkcs11-openssl?') + + prefix = "testFitSignPKCS11Object." + # Configure SoftHSMv2 + data = tools.read_file(self.TestFile('340_softhsm2.conf')) + softhsm2_conf = self._MakeInputFile(f'{prefix}softhsm2.conf', data) + softhsm2_tokens_dir = self._MakeInputDir(f'{prefix}softhsm2.tokens') + + with open(softhsm2_conf, 'a') as f: + f.write(f'directories.tokendir = {softhsm2_tokens_dir}') + + # Generate pubkey DTB with random RSA4096 key + _, _, private_key, pubkey_dtb = self._PrepareSignEnv() + + with unittest.mock.patch.dict('os.environ', + {'SOFTHSM2_CONF': softhsm2_conf}): + tools.run('softhsm2-util', '--init-token', '--free', '--label', + 'U-Boot prod token', '--pin', '1234', '--so-pin', + '222222') + tools.run('softhsm2-util', '--import', private_key, '--token', + 'U-Boot prod token', '--label', 'prod', '--id', '999999', + '--pin', '1234') + + # Make sure the private key can only be accessed through the engine + os.remove(private_key) + + entry_args = { + 'of-list': 'test-fdt1', + 'default-dt': 'test-fdt1', + 'atf-bl31-path': 'bl31.elf', + } + + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR) + + # Make OpenSSL use softhsm2 engine and configure PIN for token + # The PIN is incorrect on purpose, the correct one will be passed by + # MKIMAGE_SIGN_PIN + ossl_conf = self.TestFile('340_openssl.conf') + with unittest.mock.patch.dict('os.environ', + {'OPENSSL_CONF': ossl_conf, + 'SOFTHSM2_CONF': softhsm2_conf, + 'MKIMAGE_SIGN_PIN': '1234'}): + data = self._DoReadFileDtb( + '340_fit_signature_engine_pkcs11_object.dts', + entry_args=entry_args, + extra_indirs=[test_subdir])[0] + + dtb = fdt.Fdt.FromData(data) + dtb.Scan() + + conf = dtb.GetNode('/configurations/conf-uboot-1') + self.assertIsNotNone(conf) + signature = conf.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + images = dtb.GetNode('/images') + self.assertIsNotNone(images) + for subnode in images.subnodes: + signature = subnode.FindNode('signature') + self.assertIsNotNone(signature) + self.assertIsNotNone(signature.props.get('value')) + + tools.run('fit_check_sign', '-k', pubkey_dtb, + '-f', tools.get_output_filename('fit.fit')) + def testFitSignKeyNotFound(self): """Test that missing keys raise an error""" if not elf.ELF_TOOLS: diff --git a/tools/binman/test/340_dummy-rsa4096.crt b/tools/binman/test/340_dummy-rsa4096.crt new file mode 100644 index 00000000000..c426874431b --- /dev/null +++ b/tools/binman/test/340_dummy-rsa4096.crt @@ -0,0 +1,31 @@ +-----BEGIN CERTIFICATE----- +MIIFZTCCA02gAwIBAgIUHBvjPF93z8GaT9YkFleXIvBtt+8wDQYJKoZIhvcNAQEL +BQAwQjELMAkGA1UEBhMCWFgxFTATBgNVBAcMDERlZmF1bHQgQ2l0eTEcMBoGA1UE +CgwTRGVmYXVsdCBDb21wYW55IEx0ZDAeFw0yNTExMTQxMDAxNDRaFw0yNTEyMTQx +MDAxNDRaMEIxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAa +BgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwggIiMA0GCSqGSIb3DQEBAQUAA4IC +DwAwggIKAoICAQC6cUpK5P8J7E+flOwYFI2vh4qQ2mNbDvNfLG0DYMbMuWvWjwd5 +j88UuPCGdptLUL9QGsA0ZreewA3zBh2hAei0ryieYBSq6ez9kG6pAYcI6+c9VCro +jH9Tv+w/UEtNQUsn40j5c8koAqizEqGpwThM+gm+ftWBVCQHDscI9JXac4V/69OE +Oegf8NI4FoC/EQqLDVNxlVaSwxFC9ATEEuO9++rdaHiicmGnt27UOsgM9WO+gZpk +BgicLUp05qFymCwSPsiM6ELT9QYSxAMUi5u6gX2V5d444lSicaaZ2nRQXzOBqnsN +KQksObV2MiJ//jr3/M83TE23eJun6lACvu/5G/sJzJ+aqzBdpHx1DIGYKkQU5jA7 +n/FrHNAWVdyDhLYfaLOZKq7I/pcOzsjYFAjVbPnbVXqujuYj5YXsFttIo+PXTRzY +WBsEcBojqxOhEw8SpQOoeVYgvksKrw3u5RDO7XxkogMRuyO/DuBfm0dDvDf3Anlb +BjCsHHoJAuuOUKidGMkIK37lR6N6NcdZ4g3aSXvjl785/jCzQBl9HnrrGQV5rLHn +PYQkNAXrW+bXil4m4LB07e0/4aUrV8A/5SMolzWpfSjeFuLliZzBlAORNmtfKmT+ +F1WrNNnzUa11iRKe9JA7X+dBtC/nbxB2liBwzl8KdWNZOAAP5zQdcJQFMQIDAQAB +o1MwUTAdBgNVHQ4EFgQUy7zUkFMKw/hlJjHqjOYbClVCM3AwHwYDVR0jBBgwFoAU +y7zUkFMKw/hlJjHqjOYbClVCM3AwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0B +AQsFAAOCAgEAGpsQ5klxd4itvbjVCCf9usItnwlhrf6GxwvGpDBEa8oJbkcQNI+9 +GHVVE+/RO3jx/hjBItLUruKK7mmauO1K2qPLKK8nwcGYV2oMTxTbhZD93a61YBVO +KSWKi5czN8qCU2vkCc9LJM0y2aZzTjVDZCO6b7lYxCZTZcX2Ygcmt3Zv5sv3wrAF +9h0nv8CD4soqsE31GZmfuSiZ/lfBdX7Awr9WI6t++zOKBSZmxmLhfG+Fplak10Xv +TR+yOXHmy+aLegUbrsNgH8ktDAyvM7Aq6+EvbTD6lT06T328cA+AWrKRzYcEGjE0 +wdohmfaKoOch3oZNaXR4fiXDuwTBZ6xHsDpa0RDhvu/YhBP50CxQSqQNuURhFYJB +qBq3E0tpM4cCABBDA8ci2i1ilYb1AeL1LMqYQJXdIa8X5YP1NMdNyMO+4Aizc1J6 +wNvBqItgShvLjBWoQGYWKiiqgz8UoWWMjp0VuKmv0fu/71rbBarg2JDMsC7U5yno +rsfCDgcLGhemFe2EpvlLd8j+rmzjl4Kx1UGKC0EbqBFFaGj8ZlCu20KHpPYiOzNd +p2dU6LpeQYvOfQOCMjNe6BPKCxE4FEkQpdXuF2aKq9rlbZtcVeo1W/NDn2JHaDZ2 +BhHQfv/vugdB9E6JtqoQsME4rDkE3+0AxA7qvVkWFnI0cFtHsqZD+N4= +-----END CERTIFICATE----- diff --git a/tools/binman/test/340_fit_signature_engine.dts b/tools/binman/test/340_fit_signature_engine.dts new file mode 100644 index 00000000000..fd374e0edd2 --- /dev/null +++ b/tools/binman/test/340_fit_signature_engine.dts @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + fit,sign; + fit,engine = "dummy-rsa-engine"; + + images { + u-boot { + description = "test u-boot"; + type = "standalone"; + arch = "arm64"; + os = "u-boot"; + compression = "none"; + load = <0x00000000>; + entry = <0x00000000>; + + u-boot-nodtb { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "test tf-a"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + @fdt-SEQ { + description = "test fdt"; + type = "flat_dt"; + compression = "none"; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + }; + + configurations { + default = "@conf-uboot-DEFAULT-SEQ"; + @conf-uboot-SEQ { + description = "uboot config"; + fdt = "fdt-SEQ"; + fit,firmware = "u-boot"; + fit,loadables; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + sign-images = "firmware", "loadables", "fdt"; + }; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/340_fit_signature_engine_encrypt.dts b/tools/binman/test/340_fit_signature_engine_encrypt.dts new file mode 100644 index 00000000000..fbcde342c65 --- /dev/null +++ b/tools/binman/test/340_fit_signature_engine_encrypt.dts @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + fit,sign; + fit,encrypt; + fit,engine = "dummy-rsa-engine"; + + images { + u-boot { + description = "test u-boot"; + type = "standalone"; + arch = "arm64"; + os = "u-boot"; + compression = "none"; + load = <0x00000000>; + entry = <0x00000000>; + + u-boot-nodtb { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "test tf-a"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + @fdt-SEQ { + description = "test fdt"; + type = "flat_dt"; + compression = "none"; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + }; + }; + }; + + configurations { + default = "@conf-uboot-DEFAULT-SEQ"; + @conf-uboot-SEQ { + description = "uboot config"; + fdt = "fdt-SEQ"; + fit,firmware = "u-boot"; + fit,loadables; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "dev"; + sign-images = "firmware", "loadables", "fdt"; + }; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/340_fit_signature_engine_pkcs11.dts b/tools/binman/test/340_fit_signature_engine_pkcs11.dts new file mode 100644 index 00000000000..60ba04c13d4 --- /dev/null +++ b/tools/binman/test/340_fit_signature_engine_pkcs11.dts @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + fit,sign; + fit,engine = "pkcs11"; + + images { + u-boot { + description = "test u-boot"; + type = "standalone"; + arch = "arm64"; + os = "u-boot"; + compression = "none"; + load = <0x00000000>; + entry = <0x00000000>; + + u-boot-nodtb { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "test tf-a"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + @fdt-SEQ { + description = "test fdt"; + type = "flat_dt"; + compression = "none"; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + }; + + configurations { + default = "@conf-uboot-DEFAULT-SEQ"; + @conf-uboot-SEQ { + description = "uboot config"; + fdt = "fdt-SEQ"; + fit,firmware = "u-boot"; + fit,loadables; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + sign-images = "firmware", "loadables", "fdt"; + }; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/340_fit_signature_engine_pkcs11_object.dts b/tools/binman/test/340_fit_signature_engine_pkcs11_object.dts new file mode 100644 index 00000000000..5226ab4c6cf --- /dev/null +++ b/tools/binman/test/340_fit_signature_engine_pkcs11_object.dts @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + fit { + description = "test desc"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + fit,sign; + fit,engine = "pkcs11"; + fit,engine-keydir = "pkcs11:object=prod"; + + images { + u-boot { + description = "test u-boot"; + type = "standalone"; + arch = "arm64"; + os = "u-boot"; + compression = "none"; + load = <0x00000000>; + entry = <0x00000000>; + + u-boot-nodtb { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "test tf-a"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + @fdt-SEQ { + description = "test fdt"; + type = "flat_dt"; + compression = "none"; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + }; + }; + }; + + configurations { + default = "@conf-uboot-DEFAULT-SEQ"; + @conf-uboot-SEQ { + description = "uboot config"; + fdt = "fdt-SEQ"; + fit,firmware = "u-boot"; + fit,loadables; + + hash { + algo = "sha256"; + }; + + signature { + algo = "sha256,rsa4096"; + key-name-hint = "test_key"; + sign-images = "firmware", "loadables", "fdt"; + }; + }; + }; + }; + }; +}; diff --git a/tools/binman/test/340_openssl.conf b/tools/binman/test/340_openssl.conf new file mode 100644 index 00000000000..9ab9dc739fe --- /dev/null +++ b/tools/binman/test/340_openssl.conf @@ -0,0 +1,10 @@ +openssl_conf = openssl_init + +[openssl_init] +engines = engine_section + +[engine_section] +pkcs11 = pkcs11_section + +[pkcs11_section] +PIN=1111 diff --git a/tools/binman/test/340_softhsm2.conf b/tools/binman/test/340_softhsm2.conf new file mode 100644 index 00000000000..dabaacb15a1 --- /dev/null +++ b/tools/binman/test/340_softhsm2.conf @@ -0,0 +1,16 @@ +# SoftHSM v2 configuration file + +# directories.tokendir = /path/to/binman/test/softhsm2.tokens/ +objectstore.backend = file + +# ERROR, WARNING, INFO, DEBUG +log.level = ERROR + +# If CKF_REMOVABLE_DEVICE flag should be set +slots.removable = false + +# Enable and disable PKCS#11 mechanisms using slots.mechanisms. +slots.mechanisms = ALL + +# If the library should reset the state on fork +library.reset_on_fork = false diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 4d152eee9c0..d5e4c0f556a 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -35,7 +35,8 @@ LDS_BLOB := -T $(SRC)blob_syms.lds TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data bss_data_zero \ u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \ u_boot_binman_syms_size u_boot_binman_syms_x86 embed_data \ - u_boot_binman_embed u_boot_binman_embed_sm elf_sections blob_syms.bin + u_boot_binman_embed u_boot_binman_embed_sm elf_sections blob_syms.bin \ + dummy-rsa-engine.so all: $(TARGETS) @@ -84,6 +85,9 @@ blob_syms: blob_syms.c elf_sections: CFLAGS += $(LDS_EFL_SECTIONS) elf_sections: elf_sections.c +dummy-rsa-engine.so: dummy-rsa-engine.c + $(CC) -fPIC -shared -lcrypto -lssl -o $@ $< + clean: rm -f $(TARGETS) diff --git a/tools/binman/test/dummy-rsa-engine.c b/tools/binman/test/dummy-rsa-engine.c new file mode 100644 index 00000000000..d163e917a8b --- /dev/null +++ b/tools/binman/test/dummy-rsa-engine.c @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* + * Dummy RSA OpenSSL engine to test whether engines work with mkimage + * + * OpenSSL can call it with 'dummy-rsa'. The only supported key_id is "dev". + */ + +#define OPENSSL_API_COMPAT 0x10101000L + +#include <openssl/engine.h> + +#define OPENSSL_SUPPRESS_DEPRECATED + +static const char *engine_dummy_rsa_id = "dummy-rsa-engine"; +static const char *engine_dummy_rsa_name = "Dummy RSA engine"; + +/* RSA4096 private key */ +static const char n[] = +"\x00\xba\x71\x4a\x4a\xe4\xff\x09\xec\x4f\x9f\x94\xec\x18\x14" +"\x8d\xaf\x87\x8a\x90\xda\x63\x5b\x0e\xf3\x5f\x2c\x6d\x03\x60" +"\xc6\xcc\xb9\x6b\xd6\x8f\x07\x79\x8f\xcf\x14\xb8\xf0\x86\x76" +"\x9b\x4b\x50\xbf\x50\x1a\xc0\x34\x66\xb7\x9e\xc0\x0d\xf3\x06" +"\x1d\xa1\x01\xe8\xb4\xaf\x28\x9e\x60\x14\xaa\xe9\xec\xfd\x90" +"\x6e\xa9\x01\x87\x08\xeb\xe7\x3d\x54\x2a\xe8\x8c\x7f\x53\xbf" +"\xec\x3f\x50\x4b\x4d\x41\x4b\x27\xe3\x48\xf9\x73\xc9\x28\x02" +"\xa8\xb3\x12\xa1\xa9\xc1\x38\x4c\xfa\x09\xbe\x7e\xd5\x81\x54" +"\x24\x07\x0e\xc7\x08\xf4\x95\xda\x73\x85\x7f\xeb\xd3\x84\x39" +"\xe8\x1f\xf0\xd2\x38\x16\x80\xbf\x11\x0a\x8b\x0d\x53\x71\x95" +"\x56\x92\xc3\x11\x42\xf4\x04\xc4\x12\xe3\xbd\xfb\xea\xdd\x68" +"\x78\xa2\x72\x61\xa7\xb7\x6e\xd4\x3a\xc8\x0c\xf5\x63\xbe\x81" +"\x9a\x64\x06\x08\x9c\x2d\x4a\x74\xe6\xa1\x72\x98\x2c\x12\x3e" +"\xc8\x8c\xe8\x42\xd3\xf5\x06\x12\xc4\x03\x14\x8b\x9b\xba\x81" +"\x7d\x95\xe5\xde\x38\xe2\x54\xa2\x71\xa6\x99\xda\x74\x50\x5f" +"\x33\x81\xaa\x7b\x0d\x29\x09\x2c\x39\xb5\x76\x32\x22\x7f\xfe" +"\x3a\xf7\xfc\xcf\x37\x4c\x4d\xb7\x78\x9b\xa7\xea\x50\x02\xbe" +"\xef\xf9\x1b\xfb\x09\xcc\x9f\x9a\xab\x30\x5d\xa4\x7c\x75\x0c" +"\x81\x98\x2a\x44\x14\xe6\x30\x3b\x9f\xf1\x6b\x1c\xd0\x16\x55" +"\xdc\x83\x84\xb6\x1f\x68\xb3\x99\x2a\xae\xc8\xfe\x97\x0e\xce" +"\xc8\xd8\x14\x08\xd5\x6c\xf9\xdb\x55\x7a\xae\x8e\xe6\x23\xe5" +"\x85\xec\x16\xdb\x48\xa3\xe3\xd7\x4d\x1c\xd8\x58\x1b\x04\x70" +"\x1a\x23\xab\x13\xa1\x13\x0f\x12\xa5\x03\xa8\x79\x56\x20\xbe" +"\x4b\x0a\xaf\x0d\xee\xe5\x10\xce\xed\x7c\x64\xa2\x03\x11\xbb" +"\x23\xbf\x0e\xe0\x5f\x9b\x47\x43\xbc\x37\xf7\x02\x79\x5b\x06" +"\x30\xac\x1c\x7a\x09\x02\xeb\x8e\x50\xa8\x9d\x18\xc9\x08\x2b" +"\x7e\xe5\x47\xa3\x7a\x35\xc7\x59\xe2\x0d\xda\x49\x7b\xe3\x97" +"\xbf\x39\xfe\x30\xb3\x40\x19\x7d\x1e\x7a\xeb\x19\x05\x79\xac" +"\xb1\xe7\x3d\x84\x24\x34\x05\xeb\x5b\xe6\xd7\x8a\x5e\x26\xe0" +"\xb0\x74\xed\xed\x3f\xe1\xa5\x2b\x57\xc0\x3f\xe5\x23\x28\x97" +"\x35\xa9\x7d\x28\xde\x16\xe2\xe5\x89\x9c\xc1\x94\x03\x91\x36" +"\x6b\x5f\x2a\x64\xfe\x17\x55\xab\x34\xd9\xf3\x51\xad\x75\x89" +"\x12\x9e\xf4\x90\x3b\x5f\xe7\x41\xb4\x2f\xe7\x6f\x10\x76\x96" +"\x20\x70\xce\x5f\x0a\x75\x63\x59\x38\x00\x0f\xe7\x34\x1d\x70" +"\x94\x05\x31"; + +static const char e[] = "\x01\x00\x01"; + +static const char d[] = +"\x2e\xad\xfb\xbc\x59\xae\x53\x35\x33\xd0\x50\x30\x76\x6c\xfa" +"\xf6\x76\x38\xa6\xc0\xce\xfc\x76\xf7\x4f\x1e\x67\xe2\xdf\x21" +"\x97\x13\x5b\xa1\x1a\x29\x74\x71\xa1\x96\xde\x20\xf6\x81\x8e" +"\xab\x22\x39\xec\x1b\xee\x80\x90\x31\x2c\x11\x88\xcc\x8e\x7c" +"\xef\x99\x73\x42\x7d\xd2\x6d\x28\xc0\x33\xf4\xa2\xad\xef\xb2" +"\x0d\x25\x81\x42\x26\x12\x3c\xe4\x2c\x64\x11\xfd\x35\x22\x49" +"\xcb\xa6\x56\x5c\x2e\xdb\x5a\xce\xc8\xb0\x10\x21\xce\x9f\x2f" +"\xce\xb9\xfc\xf8\xec\x14\x25\x0d\xbb\x4f\xd3\x20\xb0\xa3\x38" +"\xeb\xfd\x72\xae\xd6\xd2\x08\x22\x41\x4c\x00\x66\xf2\x65\xaf" +"\x2d\x04\x16\x16\x0d\xe8\x49\x2e\x42\x96\x03\x0d\x9f\xd4\x14" +"\x9b\x65\x34\x96\xaf\x52\xdc\x26\xa0\x97\xaa\x11\xa9\x42\xa5" +"\x65\x82\xbe\xd9\x87\x8f\x3b\x9a\xc6\x08\x9c\x8d\xcf\x1e\x52" +"\xe1\xf5\x32\xc5\xab\x7f\x47\x5a\x91\x14\x88\x6e\x0c\x59\x64" +"\xcc\x06\xc0\xe8\xa1\xa1\xd6\x23\xf6\x63\x77\xfa\xd3\x0c\xe5" +"\xbd\x18\x92\xf5\x6a\x11\x67\xa4\x8d\xe2\x5a\x74\x21\xc5\x5c" +"\x37\x7c\x50\x17\xc0\xec\xf7\x82\x31\x61\x38\xf7\x33\x55\x9b" +"\x3d\x08\xb2\x76\xf5\x67\x45\xe6\xf0\x0d\xf0\x17\xfd\x40\x02" +"\x03\xce\x82\xc4\xa5\xa9\xcb\x8b\x9b\x68\x6a\x27\xed\x3a\xb7" +"\xd2\x60\x93\x48\x55\x8c\x87\xc9\x8c\xbd\x97\x24\xd2\x2f\xd1" +"\xc2\x7c\xec\xa7\x4d\x67\x5d\xd4\xfd\xaf\x91\x15\xdd\x57\xcb" +"\x17\xea\xcf\xfc\x84\x19\xd6\x27\x08\xa3\xef\xdc\xe4\x9b\xf6" +"\xea\x72\x37\xb0\xc7\xbc\xe0\xc6\x39\xf8\x89\x1b\xde\x84\xad" +"\xb7\x4f\xf0\xef\x87\x0c\xfe\x5d\x9b\xfd\xee\x6a\x68\xdb\x1f" +"\xc7\x9b\x23\x8f\x1a\xf1\xe8\x3f\x17\xd5\x58\x5f\x0b\x8c\x92" +"\xf4\xc7\x5f\x3c\x71\xa7\xac\xdd\xa3\x3a\x5f\x4f\xc0\xf1\x4c" +"\x8f\x3f\x63\x93\x27\x1b\xbe\xc9\xc1\x5f\x04\xf9\xff\x0f\x36" +"\x62\x6c\x3b\x65\xb3\xa8\xb4\x78\xa8\xab\xae\x3e\xf5\x61\x67" +"\xc1\x9f\xcf\x41\x19\xdc\x21\x7d\x83\xfb\x1b\xcc\x92\x6e\xf8" +"\x70\xcb\xb4\xb2\xc3\x1e\xbe\xaf\x91\xf8\xc8\x32\x17\xad\x82" +"\x62\x70\x70\xe2\x31\x34\x0b\xd0\xe2\x71\xc8\x8b\x8f\xee\xcd" +"\xa1\x00\x91\x84\x18\x04\xd0\x9e\x21\xd9\x5c\xcd\xf9\x4e\x75" +"\x32\x81\x1d\xf3\xe7\x41\xfc\x22\xcd\x3b\x88\x09\xae\xb5\xc5" +"\x5f\x5c\x25\x65\x71\xfb\x61\xd0\x8c\xc8\x53\xee\xee\x83\xdf" +"\x41\xf8\x96\xda\x5f\x06\x21\x87\xf8\xe4\x07\xe7\xf5\xdb\xc0" +"\x3e\x9b"; + +static EVP_PKEY *dummy_rsa_ossl_load_privkey(ENGINE *eng, const char *key_id, + UI_METHOD *ui_method, + void *callback_data) +{ + EVP_PKEY *pkey = NULL; + RSA *rsa = NULL; + + if (strncmp(key_id, "dev", 3)) + return NULL; + + pkey = EVP_PKEY_new(); + if (!pkey) + goto pkey_err; + + rsa = RSA_new(); + if (!rsa) + goto err; + + if (!EVP_PKEY_assign_RSA(pkey, rsa)) + goto err; + + if (!RSA_set0_key(rsa, + BN_bin2bn(n, sizeof(n) - 1, NULL), + BN_bin2bn(e, sizeof(e) - 1, NULL), + BN_bin2bn(d, sizeof(d) - 1, NULL))) + goto err; + + return pkey; + +pkey_err: + EVP_PKEY_free(pkey); +err: + RSA_free(rsa); + return NULL; +} + +static int bind_helper(ENGINE *e, const char *id) +{ + const RSA_METHOD *rsa_default_meth = RSA_get_default_method(); + RSA_METHOD *dummy_rsa_meth; + + if (id && strcmp(id, engine_dummy_rsa_id)) + return 0; + + dummy_rsa_meth = RSA_meth_dup(rsa_default_meth); + + ENGINE_set_load_privkey_function(e, dummy_rsa_ossl_load_privkey); + + return ENGINE_set_id(e, engine_dummy_rsa_id) && + ENGINE_set_name(e, engine_dummy_rsa_name) && + ENGINE_set_RSA(e, dummy_rsa_meth); +} + +IMPLEMENT_DYNAMIC_CHECK_FN() +IMPLEMENT_DYNAMIC_BIND_FN(bind_helper) -- 2.51.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-11-21 17:15 ` [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine Quentin Schulz @ 2025-11-25 22:15 ` Simon Glass 2025-11-26 11:44 ` Quentin Schulz 0 siblings, 1 reply; 18+ messages in thread From: Simon Glass @ 2025-11-25 22:15 UTC (permalink / raw) To: Quentin Schulz Cc: u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz Hi Quentin, On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > From: Quentin Schulz <quentin.schulz@cherry.de> > > This adds a test that signs a FIT and verifies the signature with > fit_check_sign. > > OpenSSL engines are typically for signing with external HW so it's not > that straight-forward to simulate. > > For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > 4096 private key is made available. It can be selected by setting the > OpenSSL engine argument to dummy-rsa-engine. This can only be done if > the engine is detected by OpenSSL, which works by setting the > OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > is properly implementing what is expected from an RSA engine, but it > seems to be enough for testing. > > For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > without specific hardware. The keypairs and tokens are generated on the > fly. The "prod" token is generated with a different PIN (1234 instead of > 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > > Binman will not mess with the local SoftHSMv2 setup as it will only use > tokens from a per-test temporary directory enforced via the temporary > configuration file set via SOFTHSM2_CONF env variable in the tests. The > files created in the input dir should NOT be named the same as it is > shared between all tests in the same process (which is all tests when > running binman with -P 1 or with -T). > > Once signed, it's checked with fit_check_sign with the associated > certificate. > > Finally, a new softhsm2_util bintool is added so that we can initialize > the token and import keypairs. On Debian, the package also brings > libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > is not the only package required though, as it also needs p11-kit and > libengine-pkcs11-openssl (the latter bringing the former). We can detect > if it's properly installed by running openssl engine dynamic -c pkcs11. > If that fails, we simply skip the test. > The package is installed in the CI container by default. > > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > --- > tools/binman/btool/softhsm2_util.py | 21 ++ > tools/binman/ftest.py | 223 +++++++++++++++++++++ > tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > tools/binman/test/340_openssl.conf | 10 + > tools/binman/test/340_softhsm2.conf | 16 ++ > tools/binman/test/Makefile | 6 +- > tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > 11 files changed, 853 insertions(+), 1 deletion(-) Not sure of the changes from last time, but I assume the test coverage is finished. Not important, but I think instead of: + self.assertIsNotNone(signature) we typically do: self.assertTrue(signature) and for: + self.assertIsNotNone(signature.props.get('value')) self.assertIn('value', signature.props) Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-11-25 22:15 ` Simon Glass @ 2025-11-26 11:44 ` Quentin Schulz 2025-12-02 20:06 ` Simon Glass 0 siblings, 1 reply; 18+ messages in thread From: Quentin Schulz @ 2025-11-26 11:44 UTC (permalink / raw) To: Simon Glass, Quentin Schulz Cc: u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Simon, On 11/25/25 11:15 PM, Simon Glass wrote: > Hi Quentin, > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: >> >> From: Quentin Schulz <quentin.schulz@cherry.de> >> >> This adds a test that signs a FIT and verifies the signature with >> fit_check_sign. >> >> OpenSSL engines are typically for signing with external HW so it's not >> that straight-forward to simulate. >> >> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA >> 4096 private key is made available. It can be selected by setting the >> OpenSSL engine argument to dummy-rsa-engine. This can only be done if >> the engine is detected by OpenSSL, which works by setting the >> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine >> is properly implementing what is expected from an RSA engine, but it >> seems to be enough for testing. >> >> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 >> without specific hardware. The keypairs and tokens are generated on the >> fly. The "prod" token is generated with a different PIN (1234 instead of >> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. >> >> Binman will not mess with the local SoftHSMv2 setup as it will only use >> tokens from a per-test temporary directory enforced via the temporary >> configuration file set via SOFTHSM2_CONF env variable in the tests. The >> files created in the input dir should NOT be named the same as it is >> shared between all tests in the same process (which is all tests when >> running binman with -P 1 or with -T). >> >> Once signed, it's checked with fit_check_sign with the associated >> certificate. >> >> Finally, a new softhsm2_util bintool is added so that we can initialize >> the token and import keypairs. On Debian, the package also brings >> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It >> is not the only package required though, as it also needs p11-kit and >> libengine-pkcs11-openssl (the latter bringing the former). We can detect >> if it's properly installed by running openssl engine dynamic -c pkcs11. >> If that fails, we simply skip the test. >> The package is installed in the CI container by default. >> >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >> --- >> tools/binman/btool/softhsm2_util.py | 21 ++ >> tools/binman/ftest.py | 223 +++++++++++++++++++++ >> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ >> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ >> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ >> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ >> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ >> tools/binman/test/340_openssl.conf | 10 + >> tools/binman/test/340_softhsm2.conf | 16 ++ >> tools/binman/test/Makefile | 6 +- >> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ >> 11 files changed, 853 insertions(+), 1 deletion(-) > > Not sure of the changes from last time, but I assume the test coverage > is finished. > They are listed in the cover letter in the Changes section. $ b4 diff -v 2 3 -- https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t will show you the git-range-diff between both versions for a given commit. Yes, both the tests (parallel by default) and test coverage (single thread/process) pass now. You can verify this for yourself by fetching the code with $ b4 shazam https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t and follow the instructions from the .gitlab-ci.yml file in the job named "Run binman, buildman, dtoc, Kconfig and patman testsuites" up till "./tools/binman/binman ${TOOLPATH} test -T" from within the CI container. e.g. podman run -it --rm --userns=keep-id -w $PWD -v $PWD:$PWD --pull always docker.io/trini/u-boot-gitlab-ci-runner:noble-20251001-14Nov2025 > Not important, but I think instead of: > > + self.assertIsNotNone(signature) > > we typically do: self.assertTrue(signature) > > and for: > > + self.assertIsNotNone(signature.props.get('value')) > > self.assertIn('value', signature.props) > Both copied from testFitSignSimple() which uses self.assertIsNotNone(). I like having the "shared" code implemented the same way so that if there's an issue in one test we can hopefully easily spot the same error in others and fix them in one go. It's also seldom used on nodes in tools/binman/ftest.py, 3 out of the 29 times it appears in the file. self.assertIsNotNone() is used on nodes 17 out of the 26 times it appears in the file. Cheers, Quentin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-11-26 11:44 ` Quentin Schulz @ 2025-12-02 20:06 ` Simon Glass 2025-12-02 20:14 ` Tom Rini 2025-12-04 11:50 ` Quentin Schulz 0 siblings, 2 replies; 18+ messages in thread From: Simon Glass @ 2025-12-02 20:06 UTC (permalink / raw) To: Quentin Schulz Cc: Quentin Schulz, u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Quentin, On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > Hi Simon, > > On 11/25/25 11:15 PM, Simon Glass wrote: > > Hi Quentin, > > > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > >> > >> From: Quentin Schulz <quentin.schulz@cherry.de> > >> > >> This adds a test that signs a FIT and verifies the signature with > >> fit_check_sign. > >> > >> OpenSSL engines are typically for signing with external HW so it's not > >> that straight-forward to simulate. > >> > >> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > >> 4096 private key is made available. It can be selected by setting the > >> OpenSSL engine argument to dummy-rsa-engine. This can only be done if > >> the engine is detected by OpenSSL, which works by setting the > >> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > >> is properly implementing what is expected from an RSA engine, but it > >> seems to be enough for testing. > >> > >> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > >> without specific hardware. The keypairs and tokens are generated on the > >> fly. The "prod" token is generated with a different PIN (1234 instead of > >> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > >> > >> Binman will not mess with the local SoftHSMv2 setup as it will only use > >> tokens from a per-test temporary directory enforced via the temporary > >> configuration file set via SOFTHSM2_CONF env variable in the tests. The > >> files created in the input dir should NOT be named the same as it is > >> shared between all tests in the same process (which is all tests when > >> running binman with -P 1 or with -T). > >> > >> Once signed, it's checked with fit_check_sign with the associated > >> certificate. > >> > >> Finally, a new softhsm2_util bintool is added so that we can initialize > >> the token and import keypairs. On Debian, the package also brings > >> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > >> is not the only package required though, as it also needs p11-kit and > >> libengine-pkcs11-openssl (the latter bringing the former). We can detect > >> if it's properly installed by running openssl engine dynamic -c pkcs11. > >> If that fails, we simply skip the test. > >> The package is installed in the CI container by default. > >> > >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > >> --- > >> tools/binman/btool/softhsm2_util.py | 21 ++ > >> tools/binman/ftest.py | 223 +++++++++++++++++++++ > >> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > >> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > >> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > >> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > >> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > >> tools/binman/test/340_openssl.conf | 10 + > >> tools/binman/test/340_softhsm2.conf | 16 ++ > >> tools/binman/test/Makefile | 6 +- > >> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > >> 11 files changed, 853 insertions(+), 1 deletion(-) > > > > Not sure of the changes from last time, but I assume the test coverage > > is finished. > > > > They are listed in the cover letter in the Changes section. > > $ b4 diff -v 2 3 -- > https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t > > will show you the git-range-diff between both versions for a given commit. I normally review just in email (often on a Chromebook) so I don't have that. It is also an extra step and I don't know where your log argument comes from. It would be better to put the change log in the patch as well. > > Yes, both the tests (parallel by default) and test coverage (single > thread/process) pass now. You can verify this for yourself by fetching > the code with > OK great! > $ b4 shazam > https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t > > and follow the instructions from the .gitlab-ci.yml file in the job > named "Run binman, buildman, dtoc, Kconfig and patman testsuites" up > till "./tools/binman/binman ${TOOLPATH} test -T" from within the CI > container. e.g. > > podman run -it --rm --userns=keep-id -w $PWD -v $PWD:$PWD --pull always > docker.io/trini/u-boot-gitlab-ci-runner:noble-20251001-14Nov2025 > > > Not important, but I think instead of: > > > > + self.assertIsNotNone(signature) > > > > we typically do: self.assertTrue(signature) > > > > and for: > > > > + self.assertIsNotNone(signature.props.get('value')) > > > > self.assertIn('value', signature.props) > > > > Both copied from testFitSignSimple() which uses self.assertIsNotNone(). > I like having the "shared" code implemented the same way so that if > there's an issue in one test we can hopefully easily spot the same error > in others and fix them in one go. It's also seldom used on nodes in > tools/binman/ftest.py, 3 out of the 29 times it appears in the file. > self.assertIsNotNone() is used on nodes 17 out of the 26 times it > appears in the file. That's fine. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-02 20:06 ` Simon Glass @ 2025-12-02 20:14 ` Tom Rini 2025-12-04 11:52 ` Quentin Schulz 2025-12-10 12:32 ` Simon Glass 2025-12-04 11:50 ` Quentin Schulz 1 sibling, 2 replies; 18+ messages in thread From: Tom Rini @ 2025-12-02 20:14 UTC (permalink / raw) To: Simon Glass Cc: Quentin Schulz, Quentin Schulz, u-boot, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog [-- Attachment #1: Type: text/plain, Size: 4442 bytes --] On Tue, Dec 02, 2025 at 08:06:02PM +0000, Simon Glass wrote: > Hi Quentin, > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > > > Hi Simon, > > > > On 11/25/25 11:15 PM, Simon Glass wrote: > > > Hi Quentin, > > > > > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > >> > > >> From: Quentin Schulz <quentin.schulz@cherry.de> > > >> > > >> This adds a test that signs a FIT and verifies the signature with > > >> fit_check_sign. > > >> > > >> OpenSSL engines are typically for signing with external HW so it's not > > >> that straight-forward to simulate. > > >> > > >> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > > >> 4096 private key is made available. It can be selected by setting the > > >> OpenSSL engine argument to dummy-rsa-engine. This can only be done if > > >> the engine is detected by OpenSSL, which works by setting the > > >> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > > >> is properly implementing what is expected from an RSA engine, but it > > >> seems to be enough for testing. > > >> > > >> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > > >> without specific hardware. The keypairs and tokens are generated on the > > >> fly. The "prod" token is generated with a different PIN (1234 instead of > > >> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > > >> > > >> Binman will not mess with the local SoftHSMv2 setup as it will only use > > >> tokens from a per-test temporary directory enforced via the temporary > > >> configuration file set via SOFTHSM2_CONF env variable in the tests. The > > >> files created in the input dir should NOT be named the same as it is > > >> shared between all tests in the same process (which is all tests when > > >> running binman with -P 1 or with -T). > > >> > > >> Once signed, it's checked with fit_check_sign with the associated > > >> certificate. > > >> > > >> Finally, a new softhsm2_util bintool is added so that we can initialize > > >> the token and import keypairs. On Debian, the package also brings > > >> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > > >> is not the only package required though, as it also needs p11-kit and > > >> libengine-pkcs11-openssl (the latter bringing the former). We can detect > > >> if it's properly installed by running openssl engine dynamic -c pkcs11. > > >> If that fails, we simply skip the test. > > >> The package is installed in the CI container by default. > > >> > > >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > > >> --- > > >> tools/binman/btool/softhsm2_util.py | 21 ++ > > >> tools/binman/ftest.py | 223 +++++++++++++++++++++ > > >> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > > >> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > > >> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > > >> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > > >> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > > >> tools/binman/test/340_openssl.conf | 10 + > > >> tools/binman/test/340_softhsm2.conf | 16 ++ > > >> tools/binman/test/Makefile | 6 +- > > >> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > > >> 11 files changed, 853 insertions(+), 1 deletion(-) > > > > > > Not sure of the changes from last time, but I assume the test coverage > > > is finished. > > > > > > > They are listed in the cover letter in the Changes section. > > > > $ b4 diff -v 2 3 -- > > https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t > > > > will show you the git-range-diff between both versions for a given commit. > > I normally review just in email (often on a Chromebook) so I don't > have that. It is also an extra step and I don't know where your log > argument comes from. It would be better to put the change log in the > patch as well. The cover letter is just an email. Perhaps a handy tips bit of documentation (and external ref to the general b4 docs) would be helpful, especially since b4 is a common and widely used tool these days. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-02 20:14 ` Tom Rini @ 2025-12-04 11:52 ` Quentin Schulz 2025-12-04 14:25 ` Tom Rini 2025-12-10 12:32 ` Simon Glass 1 sibling, 1 reply; 18+ messages in thread From: Quentin Schulz @ 2025-12-04 11:52 UTC (permalink / raw) To: Tom Rini, Simon Glass Cc: Quentin Schulz, u-boot, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Tom, On 12/2/25 9:14 PM, Tom Rini wrote: > On Tue, Dec 02, 2025 at 08:06:02PM +0000, Simon Glass wrote: >> Hi Quentin, >> >> On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: >>> >>> Hi Simon, >>> >>> On 11/25/25 11:15 PM, Simon Glass wrote: >>>> Hi Quentin, >>>> >>>> On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: >>>>> >>>>> From: Quentin Schulz <quentin.schulz@cherry.de> >>>>> >>>>> This adds a test that signs a FIT and verifies the signature with >>>>> fit_check_sign. >>>>> >>>>> OpenSSL engines are typically for signing with external HW so it's not >>>>> that straight-forward to simulate. >>>>> >>>>> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA >>>>> 4096 private key is made available. It can be selected by setting the >>>>> OpenSSL engine argument to dummy-rsa-engine. This can only be done if >>>>> the engine is detected by OpenSSL, which works by setting the >>>>> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine >>>>> is properly implementing what is expected from an RSA engine, but it >>>>> seems to be enough for testing. >>>>> >>>>> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 >>>>> without specific hardware. The keypairs and tokens are generated on the >>>>> fly. The "prod" token is generated with a different PIN (1234 instead of >>>>> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. >>>>> >>>>> Binman will not mess with the local SoftHSMv2 setup as it will only use >>>>> tokens from a per-test temporary directory enforced via the temporary >>>>> configuration file set via SOFTHSM2_CONF env variable in the tests. The >>>>> files created in the input dir should NOT be named the same as it is >>>>> shared between all tests in the same process (which is all tests when >>>>> running binman with -P 1 or with -T). >>>>> >>>>> Once signed, it's checked with fit_check_sign with the associated >>>>> certificate. >>>>> >>>>> Finally, a new softhsm2_util bintool is added so that we can initialize >>>>> the token and import keypairs. On Debian, the package also brings >>>>> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It >>>>> is not the only package required though, as it also needs p11-kit and >>>>> libengine-pkcs11-openssl (the latter bringing the former). We can detect >>>>> if it's properly installed by running openssl engine dynamic -c pkcs11. >>>>> If that fails, we simply skip the test. >>>>> The package is installed in the CI container by default. >>>>> >>>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >>>>> --- >>>>> tools/binman/btool/softhsm2_util.py | 21 ++ >>>>> tools/binman/ftest.py | 223 +++++++++++++++++++++ >>>>> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ >>>>> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ >>>>> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ >>>>> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ >>>>> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ >>>>> tools/binman/test/340_openssl.conf | 10 + >>>>> tools/binman/test/340_softhsm2.conf | 16 ++ >>>>> tools/binman/test/Makefile | 6 +- >>>>> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ >>>>> 11 files changed, 853 insertions(+), 1 deletion(-) >>>> >>>> Not sure of the changes from last time, but I assume the test coverage >>>> is finished. >>>> >>> >>> They are listed in the cover letter in the Changes section. >>> >>> $ b4 diff -v 2 3 -- >>> https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t >>> >>> will show you the git-range-diff between both versions for a given commit. >> >> I normally review just in email (often on a Chromebook) so I don't >> have that. It is also an extra step and I don't know where your log >> argument comes from. It would be better to put the change log in the >> patch as well. > > The cover letter is just an email. Perhaps a handy tips bit of > documentation (and external ref to the general b4 docs) would be > helpful, especially since b4 is a common and widely used tool these > days. > I can do that, what do you have in mind? What should we add to the docs? Cheers, Quentin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-04 11:52 ` Quentin Schulz @ 2025-12-04 14:25 ` Tom Rini 0 siblings, 0 replies; 18+ messages in thread From: Tom Rini @ 2025-12-04 14:25 UTC (permalink / raw) To: Quentin Schulz Cc: Simon Glass, Quentin Schulz, u-boot, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog [-- Attachment #1: Type: text/plain, Size: 5336 bytes --] On Thu, Dec 04, 2025 at 12:52:17PM +0100, Quentin Schulz wrote: > Hi Tom, > > On 12/2/25 9:14 PM, Tom Rini wrote: > > On Tue, Dec 02, 2025 at 08:06:02PM +0000, Simon Glass wrote: > > > Hi Quentin, > > > > > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > > > > > > > Hi Simon, > > > > > > > > On 11/25/25 11:15 PM, Simon Glass wrote: > > > > > Hi Quentin, > > > > > > > > > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > > > > > > > > > > > From: Quentin Schulz <quentin.schulz@cherry.de> > > > > > > > > > > > > This adds a test that signs a FIT and verifies the signature with > > > > > > fit_check_sign. > > > > > > > > > > > > OpenSSL engines are typically for signing with external HW so it's not > > > > > > that straight-forward to simulate. > > > > > > > > > > > > For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > > > > > > 4096 private key is made available. It can be selected by setting the > > > > > > OpenSSL engine argument to dummy-rsa-engine. This can only be done if > > > > > > the engine is detected by OpenSSL, which works by setting the > > > > > > OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > > > > > > is properly implementing what is expected from an RSA engine, but it > > > > > > seems to be enough for testing. > > > > > > > > > > > > For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > > > > > > without specific hardware. The keypairs and tokens are generated on the > > > > > > fly. The "prod" token is generated with a different PIN (1234 instead of > > > > > > 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > > > > > > > > > > > > Binman will not mess with the local SoftHSMv2 setup as it will only use > > > > > > tokens from a per-test temporary directory enforced via the temporary > > > > > > configuration file set via SOFTHSM2_CONF env variable in the tests. The > > > > > > files created in the input dir should NOT be named the same as it is > > > > > > shared between all tests in the same process (which is all tests when > > > > > > running binman with -P 1 or with -T). > > > > > > > > > > > > Once signed, it's checked with fit_check_sign with the associated > > > > > > certificate. > > > > > > > > > > > > Finally, a new softhsm2_util bintool is added so that we can initialize > > > > > > the token and import keypairs. On Debian, the package also brings > > > > > > libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > > > > > > is not the only package required though, as it also needs p11-kit and > > > > > > libengine-pkcs11-openssl (the latter bringing the former). We can detect > > > > > > if it's properly installed by running openssl engine dynamic -c pkcs11. > > > > > > If that fails, we simply skip the test. > > > > > > The package is installed in the CI container by default. > > > > > > > > > > > > Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > > > > > > --- > > > > > > tools/binman/btool/softhsm2_util.py | 21 ++ > > > > > > tools/binman/ftest.py | 223 +++++++++++++++++++++ > > > > > > tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > > > > > > tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > > > > > > .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > > > > > > .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > > > > > > .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > > > > > > tools/binman/test/340_openssl.conf | 10 + > > > > > > tools/binman/test/340_softhsm2.conf | 16 ++ > > > > > > tools/binman/test/Makefile | 6 +- > > > > > > tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > > > > > > 11 files changed, 853 insertions(+), 1 deletion(-) > > > > > > > > > > Not sure of the changes from last time, but I assume the test coverage > > > > > is finished. > > > > > > > > > > > > > They are listed in the cover letter in the Changes section. > > > > > > > > $ b4 diff -v 2 3 -- > > > > https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t > > > > > > > > will show you the git-range-diff between both versions for a given commit. > > > > > > I normally review just in email (often on a Chromebook) so I don't > > > have that. It is also an extra step and I don't know where your log > > > argument comes from. It would be better to put the change log in the > > > patch as well. > > > > The cover letter is just an email. Perhaps a handy tips bit of > > documentation (and external ref to the general b4 docs) would be > > helpful, especially since b4 is a common and widely used tool these > > days. > > I can do that, what do you have in mind? What should we add to the docs? Honestly, whatever you've found useful as a contributor and reviewer. I'll follow-up with some handy things for custodians (mainly patchwork integration and that cover letters are so important because it's an automatic useful merge commit message). Thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-02 20:14 ` Tom Rini 2025-12-04 11:52 ` Quentin Schulz @ 2025-12-10 12:32 ` Simon Glass 1 sibling, 0 replies; 18+ messages in thread From: Simon Glass @ 2025-12-10 12:32 UTC (permalink / raw) To: Tom Rini Cc: Quentin Schulz, Quentin Schulz, u-boot, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Tom, On Tue, 2 Dec 2025 at 20:14, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Dec 02, 2025 at 08:06:02PM +0000, Simon Glass wrote: > > Hi Quentin, > > > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > > > > > Hi Simon, > > > > > > On 11/25/25 11:15 PM, Simon Glass wrote: > > > > Hi Quentin, > > > > > > > > On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > > >> > > > >> From: Quentin Schulz <quentin.schulz@cherry.de> > > > >> > > > >> This adds a test that signs a FIT and verifies the signature with > > > >> fit_check_sign. > > > >> > > > >> OpenSSL engines are typically for signing with external HW so it's not > > > >> that straight-forward to simulate. > > > >> > > > >> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > > > >> 4096 private key is made available. It can be selected by setting the > > > >> OpenSSL engine argument to dummy-rsa-engine. This can only be done if > > > >> the engine is detected by OpenSSL, which works by setting the > > > >> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > > > >> is properly implementing what is expected from an RSA engine, but it > > > >> seems to be enough for testing. > > > >> > > > >> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > > > >> without specific hardware. The keypairs and tokens are generated on the > > > >> fly. The "prod" token is generated with a different PIN (1234 instead of > > > >> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > > > >> > > > >> Binman will not mess with the local SoftHSMv2 setup as it will only use > > > >> tokens from a per-test temporary directory enforced via the temporary > > > >> configuration file set via SOFTHSM2_CONF env variable in the tests. The > > > >> files created in the input dir should NOT be named the same as it is > > > >> shared between all tests in the same process (which is all tests when > > > >> running binman with -P 1 or with -T). > > > >> > > > >> Once signed, it's checked with fit_check_sign with the associated > > > >> certificate. > > > >> > > > >> Finally, a new softhsm2_util bintool is added so that we can initialize > > > >> the token and import keypairs. On Debian, the package also brings > > > >> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > > > >> is not the only package required though, as it also needs p11-kit and > > > >> libengine-pkcs11-openssl (the latter bringing the former). We can detect > > > >> if it's properly installed by running openssl engine dynamic -c pkcs11. > > > >> If that fails, we simply skip the test. > > > >> The package is installed in the CI container by default. > > > >> > > > >> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > > > >> --- > > > >> tools/binman/btool/softhsm2_util.py | 21 ++ > > > >> tools/binman/ftest.py | 223 +++++++++++++++++++++ > > > >> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > > > >> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > > > >> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > > > >> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > > > >> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > > > >> tools/binman/test/340_openssl.conf | 10 + > > > >> tools/binman/test/340_softhsm2.conf | 16 ++ > > > >> tools/binman/test/Makefile | 6 +- > > > >> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > > > >> 11 files changed, 853 insertions(+), 1 deletion(-) > > > > > > > > Not sure of the changes from last time, but I assume the test coverage > > > > is finished. > > > > > > > > > > They are listed in the cover letter in the Changes section. > > > > > > $ b4 diff -v 2 3 -- > > > https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T/\#t > > > > > > will show you the git-range-diff between both versions for a given commit. > > > > I normally review just in email (often on a Chromebook) so I don't > > have that. It is also an extra step and I don't know where your log > > argument comes from. It would be better to put the change log in the > > patch as well. > > The cover letter is just an email. Perhaps a handy tips bit of > documentation (and external ref to the general b4 docs) would be > helpful, especially since b4 is a common and widely used tool these > days. Well in that case, just put the change log in each patch as well as the cover letter. It should not be difficult to support that in b4...I think patman has supported it for 10 years now. We are the humans and the tools should work for us! Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-02 20:06 ` Simon Glass 2025-12-02 20:14 ` Tom Rini @ 2025-12-04 11:50 ` Quentin Schulz 2025-12-10 12:32 ` Simon Glass 1 sibling, 1 reply; 18+ messages in thread From: Quentin Schulz @ 2025-12-04 11:50 UTC (permalink / raw) To: Simon Glass Cc: Quentin Schulz, u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Simon On 12/2/25 9:06 PM, Simon Glass wrote: > Hi Quentin, > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: >> >> Hi Simon, >> >> On 11/25/25 11:15 PM, Simon Glass wrote: >>> Hi Quentin, >>> >>> On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: >>>> >>>> From: Quentin Schulz <quentin.schulz@cherry.de> >>>> >>>> This adds a test that signs a FIT and verifies the signature with >>>> fit_check_sign. >>>> >>>> OpenSSL engines are typically for signing with external HW so it's not >>>> that straight-forward to simulate. >>>> >>>> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA >>>> 4096 private key is made available. It can be selected by setting the >>>> OpenSSL engine argument to dummy-rsa-engine. This can only be done if >>>> the engine is detected by OpenSSL, which works by setting the >>>> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine >>>> is properly implementing what is expected from an RSA engine, but it >>>> seems to be enough for testing. >>>> >>>> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 >>>> without specific hardware. The keypairs and tokens are generated on the >>>> fly. The "prod" token is generated with a different PIN (1234 instead of >>>> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. >>>> >>>> Binman will not mess with the local SoftHSMv2 setup as it will only use >>>> tokens from a per-test temporary directory enforced via the temporary >>>> configuration file set via SOFTHSM2_CONF env variable in the tests. The >>>> files created in the input dir should NOT be named the same as it is >>>> shared between all tests in the same process (which is all tests when >>>> running binman with -P 1 or with -T). >>>> >>>> Once signed, it's checked with fit_check_sign with the associated >>>> certificate. >>>> >>>> Finally, a new softhsm2_util bintool is added so that we can initialize >>>> the token and import keypairs. On Debian, the package also brings >>>> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It >>>> is not the only package required though, as it also needs p11-kit and >>>> libengine-pkcs11-openssl (the latter bringing the former). We can detect >>>> if it's properly installed by running openssl engine dynamic -c pkcs11. >>>> If that fails, we simply skip the test. >>>> The package is installed in the CI container by default. >>>> >>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> >>>> --- >>>> tools/binman/btool/softhsm2_util.py | 21 ++ >>>> tools/binman/ftest.py | 223 +++++++++++++++++++++ >>>> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ >>>> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ >>>> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ >>>> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ >>>> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ >>>> tools/binman/test/340_openssl.conf | 10 + >>>> tools/binman/test/340_softhsm2.conf | 16 ++ >>>> tools/binman/test/Makefile | 6 +- >>>> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ >>>> 11 files changed, 853 insertions(+), 1 deletion(-) >>> >>> Not sure of the changes from last time, but I assume the test coverage >>> is finished. >>> >> >> They are listed in the cover letter in the Changes section. >> >> $ b4 diff -v 2 3 -- >> https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T//#t >> >> will show you the git-range-diff between both versions for a given commit. > > I normally review just in email (often on a Chromebook) so I don't > have that. It is also an extra step and I don't know where your log > argument comes from. It would be better to put the change log in the What do you mean by "your log argument"? > patch as well. > We already had that discussion 7 months ago. See https://lore.kernel.org/u-boot/CAFLszTh49HqGC4P2=nefc8vgdWrLe_p6HoyoEZ=FkadNBLsp3Q@mail.gmail.com/ and the exchange afterwards. b4 expects you to put the changes made in the series in the cover letter. It doesn't forbid you from adding some to individual commits underneath "---" but I also don't think it syncs them with the cover letter (which is a feature Tom told you you could bring up to the b4 maintainer). Cheers, Quentin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-04 11:50 ` Quentin Schulz @ 2025-12-10 12:32 ` Simon Glass 2025-12-10 14:29 ` Tom Rini 0 siblings, 1 reply; 18+ messages in thread From: Simon Glass @ 2025-12-10 12:32 UTC (permalink / raw) To: Quentin Schulz Cc: Quentin Schulz, u-boot, Tom Rini, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog Hi Quentin, On Thu, 4 Dec 2025 at 04:50, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > Hi Simon > > On 12/2/25 9:06 PM, Simon Glass wrote: > > Hi Quentin, > > > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > >> > >> Hi Simon, > >> > >> On 11/25/25 11:15 PM, Simon Glass wrote: > >>> Hi Quentin, > >>> > >>> On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > >>>> > >>>> From: Quentin Schulz <quentin.schulz@cherry.de> > >>>> > >>>> This adds a test that signs a FIT and verifies the signature with > >>>> fit_check_sign. > >>>> > >>>> OpenSSL engines are typically for signing with external HW so it's not > >>>> that straight-forward to simulate. > >>>> > >>>> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > >>>> 4096 private key is made available. It can be selected by setting the > >>>> OpenSSL engine argument to dummy-rsa-engine. This can only be done if > >>>> the engine is detected by OpenSSL, which works by setting the > >>>> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > >>>> is properly implementing what is expected from an RSA engine, but it > >>>> seems to be enough for testing. > >>>> > >>>> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > >>>> without specific hardware. The keypairs and tokens are generated on the > >>>> fly. The "prod" token is generated with a different PIN (1234 instead of > >>>> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > >>>> > >>>> Binman will not mess with the local SoftHSMv2 setup as it will only use > >>>> tokens from a per-test temporary directory enforced via the temporary > >>>> configuration file set via SOFTHSM2_CONF env variable in the tests. The > >>>> files created in the input dir should NOT be named the same as it is > >>>> shared between all tests in the same process (which is all tests when > >>>> running binman with -P 1 or with -T). > >>>> > >>>> Once signed, it's checked with fit_check_sign with the associated > >>>> certificate. > >>>> > >>>> Finally, a new softhsm2_util bintool is added so that we can initialize > >>>> the token and import keypairs. On Debian, the package also brings > >>>> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > >>>> is not the only package required though, as it also needs p11-kit and > >>>> libengine-pkcs11-openssl (the latter bringing the former). We can detect > >>>> if it's properly installed by running openssl engine dynamic -c pkcs11. > >>>> If that fails, we simply skip the test. > >>>> The package is installed in the CI container by default. > >>>> > >>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > >>>> --- > >>>> tools/binman/btool/softhsm2_util.py | 21 ++ > >>>> tools/binman/ftest.py | 223 +++++++++++++++++++++ > >>>> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > >>>> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > >>>> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > >>>> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > >>>> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > >>>> tools/binman/test/340_openssl.conf | 10 + > >>>> tools/binman/test/340_softhsm2.conf | 16 ++ > >>>> tools/binman/test/Makefile | 6 +- > >>>> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > >>>> 11 files changed, 853 insertions(+), 1 deletion(-) > >>> > >>> Not sure of the changes from last time, but I assume the test coverage > >>> is finished. > >>> > >> > >> They are listed in the cover letter in the Changes section. > >> > >> $ b4 diff -v 2 3 -- > >> https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T//#t > >> > >> will show you the git-range-diff between both versions for a given commit. > > > > I normally review just in email (often on a Chromebook) so I don't > > have that. It is also an extra step and I don't know where your log > > argument comes from. It would be better to put the change log in the > > What do you mean by "your log argument"? Basically this is an email review flow. It is true that sometimes we apply patches to look into them in detail, but as I said I am often on a machine where I cannot. > > > patch as well. > > > > We already had that discussion 7 months ago. See > https://lore.kernel.org/u-boot/CAFLszTh49HqGC4P2=nefc8vgdWrLe_p6HoyoEZ=FkadNBLsp3Q@mail.gmail.com/ > and the exchange afterwards. b4 expects you to put the changes made in > the series in the cover letter. It doesn't forbid you from adding some > to individual commits underneath "---" but I also don't think it syncs > them with the cover letter (which is a feature Tom told you you could > bring up to the b4 maintainer). OK I see. Well in this case I just assume that the next version solves anything I might have mentioned last time. Regards, Simon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine 2025-12-10 12:32 ` Simon Glass @ 2025-12-10 14:29 ` Tom Rini 0 siblings, 0 replies; 18+ messages in thread From: Tom Rini @ 2025-12-10 14:29 UTC (permalink / raw) To: Simon Glass Cc: Quentin Schulz, Quentin Schulz, u-boot, Aristo Chen, Rasmus Villemoes, Marek Vasut, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog [-- Attachment #1: Type: text/plain, Size: 5025 bytes --] On Wed, Dec 10, 2025 at 05:32:04AM -0700, Simon Glass wrote: > Hi Quentin, > > On Thu, 4 Dec 2025 at 04:50, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > > > Hi Simon > > > > On 12/2/25 9:06 PM, Simon Glass wrote: > > > Hi Quentin, > > > > > > On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <quentin.schulz@cherry.de> wrote: > > >> > > >> Hi Simon, > > >> > > >> On 11/25/25 11:15 PM, Simon Glass wrote: > > >>> Hi Quentin, > > >>> > > >>> On Fri, 21 Nov 2025 at 10:15, Quentin Schulz <foss+uboot@0leil.net> wrote: > > >>>> > > >>>> From: Quentin Schulz <quentin.schulz@cherry.de> > > >>>> > > >>>> This adds a test that signs a FIT and verifies the signature with > > >>>> fit_check_sign. > > >>>> > > >>>> OpenSSL engines are typically for signing with external HW so it's not > > >>>> that straight-forward to simulate. > > >>>> > > >>>> For a simple RSA OpenSSL engine, a dummy engine with a hardcoded RSA > > >>>> 4096 private key is made available. It can be selected by setting the > > >>>> OpenSSL engine argument to dummy-rsa-engine. This can only be done if > > >>>> the engine is detected by OpenSSL, which works by setting the > > >>>> OPENSSL_ENGINES environment variable. I have no clue if dummy-rsa-engine > > >>>> is properly implementing what is expected from an RSA engine, but it > > >>>> seems to be enough for testing. > > >>>> > > >>>> For a simple PKCS11 engine, SoftHSMv2 is used, which allows to do PKCS11 > > >>>> without specific hardware. The keypairs and tokens are generated on the > > >>>> fly. The "prod" token is generated with a different PIN (1234 instead of > > >>>> 1111) to also test MKIMAGE_SIGN_PIN env variable while we're at it. > > >>>> > > >>>> Binman will not mess with the local SoftHSMv2 setup as it will only use > > >>>> tokens from a per-test temporary directory enforced via the temporary > > >>>> configuration file set via SOFTHSM2_CONF env variable in the tests. The > > >>>> files created in the input dir should NOT be named the same as it is > > >>>> shared between all tests in the same process (which is all tests when > > >>>> running binman with -P 1 or with -T). > > >>>> > > >>>> Once signed, it's checked with fit_check_sign with the associated > > >>>> certificate. > > >>>> > > >>>> Finally, a new softhsm2_util bintool is added so that we can initialize > > >>>> the token and import keypairs. On Debian, the package also brings > > >>>> libsofthsm2 which is required for OpenSSL to interact with SoftHSMv2. It > > >>>> is not the only package required though, as it also needs p11-kit and > > >>>> libengine-pkcs11-openssl (the latter bringing the former). We can detect > > >>>> if it's properly installed by running openssl engine dynamic -c pkcs11. > > >>>> If that fails, we simply skip the test. > > >>>> The package is installed in the CI container by default. > > >>>> > > >>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de> > > >>>> --- > > >>>> tools/binman/btool/softhsm2_util.py | 21 ++ > > >>>> tools/binman/ftest.py | 223 +++++++++++++++++++++ > > >>>> tools/binman/test/340_dummy-rsa4096.crt | 31 +++ > > >>>> tools/binman/test/340_fit_signature_engine.dts | 99 +++++++++ > > >>>> .../test/340_fit_signature_engine_encrypt.dts | 100 +++++++++ > > >>>> .../test/340_fit_signature_engine_pkcs11.dts | 99 +++++++++ > > >>>> .../340_fit_signature_engine_pkcs11_object.dts | 100 +++++++++ > > >>>> tools/binman/test/340_openssl.conf | 10 + > > >>>> tools/binman/test/340_softhsm2.conf | 16 ++ > > >>>> tools/binman/test/Makefile | 6 +- > > >>>> tools/binman/test/dummy-rsa-engine.c | 149 ++++++++++++++ > > >>>> 11 files changed, 853 insertions(+), 1 deletion(-) > > >>> > > >>> Not sure of the changes from last time, but I assume the test coverage > > >>> is finished. > > >>> > > >> > > >> They are listed in the cover letter in the Changes section. > > >> > > >> $ b4 diff -v 2 3 -- > > >> https://lore.kernel.org/u-boot/20251121-binman-engine-v3-0-b80180aaa783@cherry.de/T//#t > > >> > > >> will show you the git-range-diff between both versions for a given commit. > > > > > > I normally review just in email (often on a Chromebook) so I don't > > > have that. It is also an extra step and I don't know where your log > > > argument comes from. It would be better to put the change log in the > > > > What do you mean by "your log argument"? > > Basically this is an email review flow. It is true that sometimes we > apply patches to look into them in detail, but as I said I am often on > a machine where I cannot. Please keep in mind that the biggest email review flow project in history is currently the linux kernel, where this tool originates from. Suggestions are welcome by the upstream maintainer, please go make them. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 0/4] fit: allow signing with an OpenSSL engine 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz ` (3 preceding siblings ...) 2025-11-21 17:15 ` [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine Quentin Schulz @ 2025-12-07 14:04 ` Tom Rini 4 siblings, 0 replies; 18+ messages in thread From: Tom Rini @ 2025-12-07 14:04 UTC (permalink / raw) To: u-boot, Quentin Schulz Cc: Aristo Chen, Rasmus Villemoes, Marek Vasut, Simon Glass, Paul HENRYS, Heinrich Schuchardt, Shiji Yang, Anton Moryakov, Alper Nebi Yasak, Alice Guo, Bryan Brattlof, Wolfgang Wallner, Peter Robinson, Eddie Kovsky, Kever Yang, Yannic Moog, Quentin Schulz On Fri, 21 Nov 2025 18:14:56 +0100, Quentin Schulz wrote: > I have a couple of products whose U-Boot FIT is signed via a proprietary > OpenSSL engine which only expects the name of a "slot" to select the key > to sign data with. > > Currently mkimage fit support expects either a key-dir (-k) or a > key-file (-G) as a toggle for signing, however this doesn't apply to our > usecase because we use an OpenSSL engine (so no key-file to provide) > which doesn't mimic a directory layout like key-dir implies. Moreover, > binman really expects private keys (.key extension) to be available in > this key-dir directory, which we of course cannot provide. > > [...] Applied to u-boot/next, thanks! [1/4] fit: support signing with only an engine_id commit: 5207e1ff20ff26e0f3969b13701bb38610183c6a [2/4] tools: binman: mkimage: add support for passing the engine commit: 9f9de386c1e54e6b009e5510ff335ab339a89a62 [3/4] tools: binman: fit: add support for OpenSSL engines commit: fc75d216f0162d4a85b60fefa9938b8690480e27 [4/4] tools: binman: fit: add tests for signing with an OpenSSL engine commit: 564c6682fa9689e408e6b795cc8c8c40e5e60e81 -- Tom ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-12-10 14:29 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-21 17:14 [PATCH v3 0/4] fit: allow signing with an OpenSSL engine Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 1/4] fit: support signing with only an engine_id Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 2/4] tools: binman: mkimage: add support for passing the engine Quentin Schulz 2025-11-21 17:14 ` [PATCH v3 3/4] tools: binman: fit: add support for OpenSSL engines Quentin Schulz 2025-11-25 22:15 ` Simon Glass 2025-11-26 11:28 ` Quentin Schulz 2025-11-21 17:15 ` [PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine Quentin Schulz 2025-11-25 22:15 ` Simon Glass 2025-11-26 11:44 ` Quentin Schulz 2025-12-02 20:06 ` Simon Glass 2025-12-02 20:14 ` Tom Rini 2025-12-04 11:52 ` Quentin Schulz 2025-12-04 14:25 ` Tom Rini 2025-12-10 12:32 ` Simon Glass 2025-12-04 11:50 ` Quentin Schulz 2025-12-10 12:32 ` Simon Glass 2025-12-10 14:29 ` Tom Rini 2025-12-07 14:04 ` [PATCH v3 0/4] fit: allow " Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox