Hi Lorenz,
On 2026-09-17T06:55:42, Lorenz Kofler <[email protected]> wrote:
> binman: pre-load: Test signing with an OpenSSL engine
>
> Add a test that signs a pre-load header through the pkcs11 engine, with a
> key imported into SoftHSM, like the existing FIT signature tests. It is
> skipped when softhsm2-util is not installed.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Lorenz Kofler <[email protected]>
>
> tools/binman/ftest.py | 79 ++++++++++++++++++++++
> .../test/security/pre_load_engine_pkcs11.dts | 24 +++++++
> .../test/security/pre_load_engine_pkcs11_pss.dts | 24 +++++++
> 3 files changed, 127 insertions(+)
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> @@ -5937,6 +5937,85 @@ fdt fdtmap Extract the
> devicetree blob from the fdtmap
> + def _SetupPreLoadEngine(self, prefix):
> + """Set up a SoftHSM token holding the key used by the engine tests
...
> + 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?')
> +
> + data = tools.read_file(self.TestFile('fit/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')
You could use tools.write_file() here if you like
> +
> + with unittest.mock.patch.dict('os.environ',
> + {'SOFTHSM2_CONF': softhsm2_conf}):
> + softhsm2_util.run_cmd('--init-token', '--free', '--label',
> + 'U-Boot token', '--pin', '1111',
> + '--so-pin', '222222')
> + softhsm2_util.run_cmd('--import', self.TestFile('dev.key'),
> + '--token', 'U-Boot token', '--label',
> + 'test_key', '--id', '999999', '--pin',
> '1111')
Nearly identical to the SoftHSM setup open-coded in
testFitSignPKCS11Simple() and testFitSignPKCS11Object() (and the CI
capsule tests). Please can you factor this out to a shared
_SetupSoftHsm() helper on the base class and have both the FIT and
pre-load tests call it? Otherwise the third copy will drift from the
first two.
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> @@ -5937,6 +5937,85 @@ fdt fdtmap Extract the
> devicetree blob from the fdtmap
> + def testPreLoadEnginePkcs11(self):
> + """Test a pre-load header signed through the pkcs11 OpenSSL engine"""
> + softhsm2_conf = self._SetupPreLoadEngine('testPreLoadEnginePkcs11.')
> + self._CheckPreLoadEngine('security/pre_load_engine_pkcs11.dts',
> + softhsm2_conf, 'pkcs-1.5')
> +
> + def testPreLoadEnginePkcs11Pss(self):
> + """Test a pre-load header signed through the engine with pss
> padding"""
> + softhsm2_conf =
> self._SetupPreLoadEngine('testPreLoadEnginePkcs11Pss.')
> + self._CheckPreLoadEngine('security/pre_load_engine_pkcs11_pss.dts',
> + softhsm2_conf, 'pss')
Both DTS files use the bare key-name = "test_key" form, so only the
URI-expansion branch of _sign_engine() is exercised. The docstring in
2/3 documents key-name = "pkcs11:object=preload;type=private" as
supported - please add a case using a full PKCS#11 URI so the
startswith('pkcs11:') branch is covered.
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> @@ -5937,6 +5937,85 @@ fdt fdtmap Extract the
> devicetree blob from the fdtmap
> + def testPreLoadEngineMissingOpenssl(self):
> + """Test that a missing openssl is reported when using an engine"""
> + with terminal.capture() as (_, stderr):
> + self._DoTestFile('security/pre_load_engine_pkcs11.dts',
> + force_missing_bintools='openssl')
> + err = stderr.getvalue()
> + self.assertRegex(err, "Image 'image'.*missing bintools.*: openssl")
Nothing covers the new 'if self.engine and len(sig) !=
RSAS[sign_name]' check from patch 2 - with openssl forced missing the
stub signature is exactly RSAS[sign_name] bytes so the check never
fires. Please add a case where the engine returns a signature of the
wrong size (e.g. by mocking Bintoolopenssl.sign() to return a shorter
buffer) and assert that "don't have the expected size" is raised.
> diff --git a/tools/binman/test/security/pre_load_engine_pkcs11.dts
> b/tools/binman/test/security/pre_load_engine_pkcs11.dts
> @@ -0,0 +1,24 @@
> + pre-load {
> + content = <&image>;
> + algo-name = "sha256,rsa2048";
> + padding-name = "pkcs-1.5";
> + key-name = "test_key";
> + engine = "pkcs11";
> + header-size = <4096>;
> + version = <0x11223344>;
> + };
The two DTS files differ only in padding-name. Since
_CheckPreLoadEngine() already parametrises padding, please keep a
single DTS and inject the padding via an entry arg or a small template
rather than carrying two near-identical files.
Regards,
Simon
