Hi Yan, On 2026-10-12T10:47:59, Yan WANG <[email protected]> wrote: > binman: test: Add SoftHSM functional test for pre-load HSM signing
This subject is almost the same as the one for patch 5, so the two are hard to tell apart. Binman expects full test coverage for each feature. Please can you fold this patch and patch 5 into patch 3, so the HSM support arrives with its tests? If you keep them separate, please add a short paragraph on what the test sets up: a SoftHSM token, a key generated on the token, and a signature check with preload_check_sign. > > Add testPreLoadHsmSign, a functional test that exercises end-to-end > pre-load header signing using SoftHSM as a PKCS#11 token. > > Signed-off-by: Yan WANG <[email protected]> > > tools/binman/ftest.py | 64 ++++++++++++++++++++++ > tools/binman/test/security/pre_load_hsm_sign.dts | 68 > ++++++++++++++++++++++++ > 2 files changed, 132 insertions(+) > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + # Generate key pair directly in SoftHSM > + pkcs11_tool.run_cmd('--module', > '/usr/lib/softhsm/libsofthsm2.so', > + '--keypairgen', '--key-type', 'rsa:2048', > + '--label', 'uboot-key', '--login', '--pin', > '1234') The library path is hard-coded here and in pre_load_hsm_sign.dts. That path is Debian/Ubuntu-specific. Fedora uses /usr/lib64/pkcs11/libsofthsm2.so, and Arch uses /usr/lib/softhsm/libsofthsm2.so via a different package. On those hosts softhsm2-util and pkcs11-tool are present, so the test fails rather than skipping, because sign() raises 'PKCS#11 module not found'. testPkcs11SignedCapsuleGen already finds the library with 'p11-kit print-config'. Please can you do the same here and pass the path into the image rather than putting it in the dts? For example, hsm-library could come from an entry arg via GetEntryArgsOrProps(), as pre-load-key-path does. That would also help real users, since the library location is a host detail that doesn't belong in the board's devicetree. > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + # Place holder of public key file > + public_key = self._MakeInputFile("public.key", b"") > + pubkey_der = self._MakeInputFile("pubkey.der", b"") There is no need to create empty files just to get a path: os.path.join(self._indir, ...) or tools.get_output_filename() is enough. > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + tools.run('openssl', 'rsa', '-pubin', '-inform', 'DER', > + '-in', pubkey_der, '-out', public_key) Please use the openssl bintool and call _CheckBintool() on it, as testPkcs11SignedCapsuleGen does, so the test skips cleanly if openssl is missing. > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + # Create AES key for FIT encryption (required by > pre_load_hsm_sign.dts) > + # Put it in a subdirectory to avoid polluting the main input > directory > + data = tools.read_file(self.TestFile("fit/aes256.bin")) > + self._MakeInputFile("hsm_keys/aes256.bin", data) Just to check, why use an encrypted FIT here? It looks copied from testPreLoadEncryptedFit, but FIT encryption has nothing to do with HSM signing. With a plain blob or u-boot entry as the content, you could drop the AES key, the subdirectory and extra_indirs, and the test would be easier to follow. > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + image_fname = tools.get_output_filename('image.bin') > + is_signed = self._CheckPreload(image_fname, public_key) This only covers pkcs-1.5. The HSM path has its own padding mapping, and with PSS, pkcs11-tool chooses the salt length rather than binman. Please can you also cover PSS, with padding-name = "pss" in the dts and padding='pss' passed to _CheckPreload()? A second dts would do, or a parameterised helper. > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -5931,6 +5931,70 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + self.assertEqual(is_signed, True) > + > + This leaves two blank lines between methods. Please drop one. Regards, Simon
