Hi Sergio, On 2026-05-13T17:56:34, Sergio Prado <[email protected]> wrote: > binman: x509_cert: add PKCS#11/HSM signing support > > Allow X509 certificates used for TI K3 secure boot to be signed via an > HSM using the PKCS#11 standard, so that the private key never leaves > the hardware token. > > A new make variable BINMAN_X509_KEY_URI is introduced. When set, it is > forwarded to binman as the x509-key-uri entry argument, overriding the > keyfile property at signing time: > > make BINMAN_X509_KEY_URI="pkcs11:token=mytk;object=mykey;type=private" \ > OPENSSL_CONF=/path/to/openssl.cnf > > The OpenSSL pkcs11 provider or engine and the PKCS#11 module must be > configured externally via openssl.cnf. Two URI forms are supported on > OpenSSL 3.x: > > - Provider path: pkcs11:token=...;object=...;type=private > - Engine path: org.openssl.engine:pkcs11:pkcs11:token=...;... > > [...] > > Makefile | 1 + > tools/binman/binman.rst | 50 +++++++++++++++ > tools/binman/etype/ti_secure.py | 5 ++ > tools/binman/etype/ti_secure_rom.py | 5 ++ > tools/binman/etype/x509_cert.py | 52 ++++++++++++++-- > tools/binman/ftest.py | 118 > ++++++++++++++++++++++++++++++++++++ > 6 files changed, 226 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass <[email protected]> Thoughts below > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > @@ -6905,6 +6906,123 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > + # Find the softhsm2 library path via p11-kit > + p11_kit_config = configparser.ConfigParser() > + p11_kit_config.read_string(tools.run('p11-kit', 'print-config')) > + softhsm2_lib = p11_kit_config.get('softhsm2', 'module', > fallback=None) > + if softhsm2_lib is None: > + self.skipTest('softhsm2 module not found in p11-kit config') > + > + # Find the OpenSSL pkcs11 provider module via the modules dir > + # reported by 'openssl version -m'. This handles multiarch paths > + # like /usr/lib/x86_64-linux-gnu/ossl-modules on Debian/Ubuntu, > + # which OPENSSLDIR does not point at. > + m = re.search(r'MODULESDIR: '([^']+)"', > + tools.run('openssl', 'version', '-m')) You construct the bintool instances above (openssl, softhsm2_util, pkcs11_tool, p11_kit) but then call tools.run() directly. Commit 08bcf962c5f routed the existing eight call sites through <bintool>.run_cmd(), at Quentin's suggestion. Can you do the same here? - p11_kit.run_cmd('print-config'), openssl.run_cmd('version', '-m'), and the softhsm2_util.run_cmd()/pkcs11_tool.run_cmd() calls below. > diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py > @@ -168,3 +198,15 @@ class Entry_x509_cert(Entry_collection): > + @staticmethod > + def _build_pkcs11_key(uri, pin): > + """Append pin-value to a PKCS#11 URI when a PIN is provided. > + > + Uses '&' as separator if the URI already contains '?', else '?'. > + Returns the URI unchanged when pin is None or empty. > + """ > + if not pin: > + return uri > + sep = '&' if '?' in uri else '?' > + return f'{uri}{sep}pin-value={pin}' The PIN is concatenated raw, but RFC 7512 says the query component is percent-encoded.Wouldn't it be better to use urllib.parse.quote() on the PIN before appending? You could add a simple test for that. > diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py > @@ -18,7 +18,25 @@ class Entry_x509_cert(Entry_collection): > """An entry which contains an X509 certificate > > Properties / Entry arguments: > - - content: List of phandles to entries to sign > + - content: List of phandles to entries to sign. > + - keyfile: Filename of the PEM key file used to sign the binary. > + - x509-key-uri: URI of a key to use for signing instead of the PEM ... > + - cert-ca: Common Name (CN) embedded in the certificate. Used when > + generating a generic x509 certificate. > + - cert-revision-int: Integer certificate revision number. Used when > + generating a generic x509 certificate. Defaults to 0. > + - sw-rev: Software revision number embedded in the certificate by > + the sysfw/rom variants used by the TI K3 secure boot subclasses. > + Defaults to 1. Ideally this would go into a separate preparatory commit. Regards, Simon
