Signing an x509 certificate entry with a key held in an HSM needs the token PIN. It can be put in openssl.cnf via pkcs11-module-token-pin, or in the PKCS#11 URI itself as a pin-value attribute, but both mean writing the PIN into a file which is part of the build. That is not user-friendly in CI, where the PIN typically arrives as a secret in the environment.
Read the PIN from the PKCS11_PIN environment variable and append it to the URI as a percent-encoded pin-value attribute, as described by RFC 7512. The rewritten URI, not the original, is what reaches 'openssl -key'. PKCS11_PIN is a fallback. A URI which already has a pin-value or a pin-source attribute is passed through untouched, since the PIN named by the URI is the one OpenSSL uses; appending a second pin-value would silently override it, as the last occurrence wins. Note that PKCS11_PIN keeps the PIN out of the build files but not off the openssl command line, where it is visible via 'ps' and may be recorded in build logs. Configuring the PIN in openssl.cnf remains the option which avoids that, and this is documented alongside the variable. Signed-off-by: Sergio Prado <[email protected]> --- tools/binman/binman.rst | 11 ++++ tools/binman/etype/x509_cert.py | 31 ++++++++++ tools/binman/ftest.py | 105 ++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 86e15741fa35..106c34efb76c 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1595,6 +1595,17 @@ If both are given, the ``pin-value`` in the URI wins. Note that a PIN placed in ``ps`` and may be recorded in build logs; keeping the PIN in ``openssl.cnf`` avoids that. +As a third option, binman reads the PIN from the ``PKCS11_PIN`` environment +variable and appends it to the URI as a ``pin-value`` attribute, so that the +PIN does not have to be written into either file:: + + PKCS11_PIN=1234 binman build ... + +``PKCS11_PIN`` is a fallback only: a URI which already has a ``pin-value`` or +``pin-source`` attribute is passed through untouched. It keeps the PIN out of +the build files, but not off the ``openssl`` command line - for that, use +``openssl.cnf``. + .. _`BinmanLogging`: Logging diff --git a/tools/binman/etype/x509_cert.py b/tools/binman/etype/x509_cert.py index 6d7883af58ef..66c9987976c2 100644 --- a/tools/binman/etype/x509_cert.py +++ b/tools/binman/etype/x509_cert.py @@ -7,6 +7,7 @@ from collections import OrderedDict import os +import urllib.parse from binman.entry import EntryArg from binman.etype.collection import Entry_collection @@ -70,6 +71,9 @@ class Entry_x509_cert(Entry_collection): self._cert_rev = fdt_util.GetInt(self._node, 'cert-revision-int', 0) self.key_fname = self.GetEntryArgsOrProps([ EntryArg('keyfile', str)], required=True)[0] + if self.key_fname.startswith(('pkcs11:', 'org.openssl.engine:pkcs11:')): + self.key_fname = self._add_pkcs11_pin(self.key_fname, + os.environ.get('PKCS11_PIN')) self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev', 1) def GetCertificate(self, required, type='generic'): @@ -178,3 +182,30 @@ class Entry_x509_cert(Entry_collection): def AddBintools(self, btools): super().AddBintools(btools) self.openssl = self.AddBintool(btools, 'openssl') + + @staticmethod + def _add_pkcs11_pin(uri, pin): + """Add a PIN to a PKCS#11 URI, so that signing runs unattended + + Appends a 'pin-value' attribute holding the PIN, percent-encoded as + required by RFC 7512. A URI which already says where its PIN comes + from is left alone, since that takes precedence over the PIN passed + here. + + Args: + uri (str): PKCS#11 URI naming the signing key + pin (str): PIN to add, or None if there is none + + Returns: + str: Value to pass to 'openssl -key' + """ + if not pin: + return uri + + # Only the query component can hold pin-value / pin-source + query = uri.partition('?')[2] + if 'pin-value=' in query or 'pin-source=' in query: + return uri + + sep = '&' if query else '?' + return f'{uri}{sep}pin-value={urllib.parse.quote(pin, safe="")}' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 10373a2fa899..e91768c52fe9 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -35,6 +35,7 @@ from dtoc import fdt from dtoc import fdt_util from binman.etype import fdtmap from binman.etype import image_header +from binman.etype.x509_cert import Entry_x509_cert from binman.image import Image from u_boot_pylib import command from u_boot_pylib import terminal @@ -7001,6 +7002,110 @@ fdt fdtmap Extract the devicetree blob from the fdtmap entry_args=entry_args)[0] self.assertEqual(U_BOOT_DATA, data[-4:]) + def testX509CertPkcs11Pin(self): + """Test signing with a key in an HSM, with the PIN from PKCS11_PIN""" + self._CheckPkcs11Provider() + + token = 'x509-test-pin' + key_label = 'testkey' + pin = '1234' + env = self._SetupPkcs11Token('testX509CertPkcs11Pin.', token, + key_label, pin) + env['PKCS11_PIN'] = pin + + # This time the URI carries no PIN, so binman appends the one from + # PKCS11_PIN before handing the URI to openssl + entry_args = { + 'keyfile': f'pkcs11:token={token};object={key_label};type=private', + } + with unittest.mock.patch.dict('os.environ', env): + data = self._DoReadFileDtb('security/x509_cert.dts', + entry_args=entry_args)[0] + self.assertEqual(U_BOOT_DATA, data[-4:]) + + def testX509CertPkcs11PinEngine(self): + """Test PKCS11_PIN with an engine-prefixed PKCS#11 URI""" + uri = ('org.openssl.engine:pkcs11:pkcs11:token=mytoken;' + 'object=mykey;type=private') + entry_args = { + 'keyfile': uri, + } + + # openssl is forced missing so that this needs no pkcs11 engine to be + # installed; what matters is the keyfile it would have been given + with unittest.mock.patch.dict('os.environ', {'PKCS11_PIN': '1234'}): + with terminal.capture(): + self._DoTestFile('security/x509_cert.dts', + force_missing_bintools='openssl', + entry_args=entry_args) + entry = control.images['image'].GetEntries()['x509-cert'] + self.assertEqual(f'{uri}?pin-value=1234', entry.key_fname) + + def testX509CertPkcs11PinNotUri(self): + """Test PKCS11_PIN is ignored when keyfile is not a PKCS#11 URI""" + # A path which merely contains 'pkcs11:' is not a URI, so it must + # reach openssl untouched + keyfile = '/keys/pkcs11:key' + entry_args = { + 'keyfile': keyfile, + } + with unittest.mock.patch.dict('os.environ', {'PKCS11_PIN': '1234'}): + with terminal.capture(): + self._DoTestFile('security/x509_cert.dts', + force_missing_bintools='openssl', + entry_args=entry_args) + entry = control.images['image'].GetEntries()['x509-cert'] + self.assertEqual(keyfile, entry.key_fname) + + def testX509CertPkcs11PinSubclass(self): + """Test PKCS11_PIN reaches the TI K3 x509 certificate subclasses""" + uri = 'pkcs11:token=mytoken;object=mykey;type=private' + entry_args = { + 'keyfile': uri, + } + + # These read 'keyfile' through Entry_x509_cert.ReadNode(), so the PIN + # must survive into the entry they build + for dts, name in [('vendor/ti_secure.dts', 'ti-secure'), + ('vendor/ti_secure_rom.dts', 'ti-secure-rom')]: + with unittest.mock.patch.dict('os.environ', + {'PKCS11_PIN': '1234'}): + with terminal.capture(): + self._DoTestFile(dts, force_missing_bintools='openssl', + entry_args=entry_args) + entry = control.images['image'].GetEntries()[name] + self.assertEqual(f'{uri}?pin-value=1234', entry.key_fname) + + def testX509CertAddPkcs11Pin(self): + """Test adding a PIN to a PKCS#11 URI""" + add = Entry_x509_cert._add_pkcs11_pin + uri = 'pkcs11:token=t;object=o;type=private' + + # No PIN to add, so the keyfile is unchanged + self.assertEqual(uri, add(uri, None)) + self.assertEqual(uri, add(uri, '')) + + # PKCS#11 URI, in both the provider and the engine form + self.assertEqual(f'{uri}?pin-value=1234', add(uri, '1234')) + engine_uri = f'org.openssl.engine:pkcs11:{uri}' + self.assertEqual(f'{engine_uri}?pin-value=1234', + add(engine_uri, '1234')) + + # URI which already has a query component, so '&' separates the PIN + self.assertEqual(f'{uri}?module-name=softhsm2&pin-value=1234', + add(f'{uri}?module-name=softhsm2', '1234')) + + # URI which already says where its PIN comes from, so it takes + # precedence and the keyfile is unchanged + self.assertEqual(f'{uri}?pin-value=5678', + add(f'{uri}?pin-value=5678', '1234')) + self.assertEqual(f'{uri}?pin-source=file:/etc/pin', + add(f'{uri}?pin-source=file:/etc/pin', '1234')) + + # PIN percent-encoded as required by RFC 7512 + self.assertEqual(f'{uri}?pin-value=a%26b%3Fc%3Dd', add(uri, 'a&b?c=d')) + self.assertEqual(f'{uri}?pin-value=a%20b%2Bc', add(uri, 'a b+c')) + def testPackRockchipTpl(self): """Test that an image with a Rockchip TPL binary can be created""" data = self._DoReadFile('vendor/rockchip_tpl.dts') -- 2.34.1
