Re: EXTERNAL - [PATCH v4 6/6] test: binman: Add test for pkcs11 signed capsule

[Wojciech Dubowik <Wojciech.Dubowik@mt.com>]

On Tue, Jan 20, 2026 at 04:53:04PM +0100, Quentin Schulz wrote:
Hello Quentin,
> Hi Wojciech,
> 
> On 1/20/26 9:12 AM, Wojciech Dubowik wrote:
> > Test pkcs11 URI support for UEFI capsule generation. For
> > simplicity only private key is defined in binman section
> > as softhsm tool doesn't support certificate import (yet).
> > 
> > Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> > Reviewed-by: Simon Glass <simon.glass@canonical.com>
> > ---
> >   tools/binman/ftest.py                         | 53 +++++++++++++++++++
> >   .../binman/test/351_capsule_signed_pkcs11.dts | 22 ++++++++
> >   2 files changed, 75 insertions(+)
> >   create mode 100644 tools/binman/test/351_capsule_signed_pkcs11.dts
> > 
> > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > index 21ec48d86fd1..a005a167e414 100644
> > --- a/tools/binman/ftest.py
> > +++ b/tools/binman/ftest.py
> > @@ -7,6 +7,7 @@
> >   #    python -m unittest func_test.TestFunctional.testHelp
> >   import collections
> > +import configparser
> >   import glob
> >   import gzip
> >   import hashlib
> > @@ -7532,6 +7533,58 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
> >           self._CheckCapsule(data, signed_capsule=True)
> > +    def testPkcs11SignedCapsuleGen(self):
> > +        """Test generation of EFI capsule (with PKCS11)"""
> > +        data = tools.read_file(self.TestFile("key.key"))
> > +        private_key = self._MakeInputFile("key.key", data)
> > +        data = tools.read_file(self.TestFile("key.pem"))
> > +        cert_file = self._MakeInputFile("key.crt", data)
> > +
> > +        softhsm2_util = bintool.Bintool.create('softhsm2_util')
> > +        self._CheckBintool(softhsm2_util)
> > +
> > +        prefix = "testPkcs11SignedCapsuleGen."
> > +        # Configure SoftHSMv2
> > +        data = tools.read_file(self.TestFile('340_softhsm2.conf'))
> > +        softhsm2_conf = self._MakeInputFile(f'{prefix}softhsm2.conf', data)
> > +        softhsm2_tokens_dir = self._MakeInputDir(f'{prefix}softhsm2.tokens')
> > +        tools.write_file(softhsm2_conf, data +
> > +                         f'\ndirectories.tokendir = \
> > +                         {softhsm2_tokens_dir}\n'.encode("utf-8"))
> > +
> 
> data is already in softhsm2_conf due to calling
> self._MakeInputFile(f'{prefix}softhsm2.conf', data)
> you can simply do the same I did in testFitSignPKCS11Simple and append to
> the file. Or you can append to data before you call _MakeInputFile().
> 
> > +        p11_kit_config = configparser.ConfigParser()
> > +        out = tools.run('p11-kit', 'print-config')
> 
> Please create a bintool and call CheckBintool so we can skip the test if
> p11-kit isn't installed. It'll be a bit awkward because there's no
> --version, --help, to print the help text but not have a non-zero exit code.
> 
> > +        p11_kit_config.read_string(out)
> > +        softhsm2_lib = p11_kit_config['softhsm2']['module']
> > +
> 
> We probably want to have some try..except here, or a more forgiving
> .get('softhsm2', {}).get('module')
> and fail the test if we don't get a path?
> 
> > +        os.environ['SOFTHSM2_CONF'] = softhsm2_conf
> 
> This is wrong, you'll be messing up with the environment of all tests being
> run in the same thread. You must use the "with
> unittest.mock.patch.dict('os.environ'," implementation I used in
> testFitSignPKCS11Simple.

Well, I have done so in my V2 but has been commented as wrong by the
first reviewer. I will restore it back.

> 
> > +        tools.run('softhsm2-util', '--init-token', '--free', '--label',
> > +                  'U-Boot token', '--pin', '1111', '--so-pin',
> > +                  '222222')
> > +        tools.run('softhsm2-util', '--import', private_key, '--token',
> > +                  'U-Boot token', '--label', 'test_key', '--id', '999999',
> > +                  '--pin', '1111')
> > +
> > +        os.environ['PKCS11_MODULE_PATH'] = softhsm2_lib
> 
> Same issue as with SOFTHSM2_CONF, you must mock the environment and not
> write to it.
> 
> > +        data = self._DoReadFile('351_capsule_signed_pkcs11.dts')
> > +
> > +        self._CheckCapsule(data, signed_capsule=True)
> > +
> > +        # Verify signed capsule
> > +        hdr = self._GetCapsuleHeaders(data)
> > +        monotonic_count = hdr['EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT']
> > +
> > +        with open(self._indir + '/capsule_input.bin', 'ab') as f:
> 
> Please use self._MakeInputFile() and prefix it with the name of the method
> so there's no possible name clash.
> 
> > +            f.write(struct.pack('<Q', int(monotonic_count, 16)))
> > +
> > +        try:
> > +            tools.run('openssl', 'smime', '-verify', '-inform', 'DER',
> 
> This means you depend on openssl being present. Should we maybe do something
> like
> 
> openssl = bintool.Bintool.create('openssl')
> self._CheckBintool(openssl)
> [...]
> openssl.run_cmd(['smime', ...])
> 
> to skip the test if it isn't installed?
> 
> > +                      '-in', tools.get_output_dir() + '/capsule.efi-capsule.p7',
> 
> tools.get_output_filename('capsule.efi-capsule.p7')
> 
> > +                      '-content', self._indir + '/capsule_input.bin',
> 
> Reuse the path created by self._MakeInputFile().
> 
> > +                      '-CAfile', cert_file, '-no_check_time')
> > +        except ValueError:
> > +            self.assertIn('UEFI Capsule verification failed')
> > +
> 
> I don't think this is valid.
> https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertIn
> states you need 2 (or three) argumentis, only one's provided here.
> 
> I'm assuming we don't need the try..except since the raised Exception will
> just stop (and fail) the execution of the test but continue with the other
> ones? Can you check (locally) by purposefully using the wrong key for
> example?

It has worked (failed) with try/except. I had to add no time check as the certificate
used for uefi capsules tests has expired in 2024. Maybe somebody can refresh
it one day. I will try other methods to see if we still get meaningful error
messages.

I will try to handle issues you mentioned also in other threads this/next week.

Cheers,
Wojtek
> 
> Cheers,
> Quentin