[PATCH v3 4/4] tools: binman: fit: add tests for signing with an OpenSSL engine

[Quentin Schulz]

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 at 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 at 0leil.net> wrote:
>>>>
>>>> From: Quentin Schulz <quentin.schulz at 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 at 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