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

[Tom Rini <trini@konsulko.com>]

[-- 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 --]