Hi Quentin, On Wed, 26 Nov 2025 at 04:44, Quentin Schulz <[email protected]> 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 <[email protected]> wrote: > >> > >> From: Quentin Schulz <[email protected]> > >> > >> 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 <[email protected]> > >> --- > >> 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/[email protected]/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/[email protected]/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
