[PATCH RFC v3 05/11] ti: etype: x509: Add etype for x509 certificate for K3 devices
Alper Nebi Yasak
alpernebiyasak at gmail.com
Fri Jul 1 21:07:46 CEST 2022
On 15/06/2022 09:47, Neha Malcom Francis wrote:
> K3 devices requires x509 certificate to be added as header of bootloader
> binaries that allows ROM to validate the integrity of the image. Etype
> that generates a TI x509 certificate is added.
>
> Currently this etype is scaled for J721E. For J721E, tiboot3.bin
> requires the x509 certificate binary to be prepended to the R5 SPL.
>
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
> test/py/requirements.txt | 1 +
> tools/binman/entries.rst | 15 ++
> tools/binman/etype/ti_x509_cert.py | 241 ++++++++++++++++++++++++++++
> tools/binman/ftest.py | 6 +
> tools/binman/test/232_x509_cert.dts | 18 +++
> 5 files changed, 281 insertions(+)
> create mode 100644 tools/binman/etype/ti_x509_cert.py
> create mode 100644 tools/binman/test/232_x509_cert.dts
>
> diff --git a/test/py/requirements.txt b/test/py/requirements.txt
> index a91ba64563..add264bdaf 100644
> --- a/test/py/requirements.txt
> +++ b/test/py/requirements.txt
> @@ -11,6 +11,7 @@ packaging==19.2
> pbr==5.4.3
> pluggy==0.13.0
> py==1.10.0
> +pycryptodome==3.14.1
You don't need this if you use Cryptodome instead of Crypto. It's the
same thing as pycryptodomex below.
> pycryptodomex==3.9.8
> pyelftools==0.27
> pygit2==0.28.2
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index b6915ef12e..e7757b3e06 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -1890,6 +1890,21 @@ and kernel are genuine.
>
>
>
> +Entry: ti-x509-cert: Texas Instruments x509 certificate for K3 devices
> +------------------------------------------------
> +
> +Properties / Entry arguments:
> + - content: Phandle of binary to sign
> + - output: Name of the final output file
> + - key_file: File with key inside it. If not provided, script generates RSA degenerate key
> + - core: Target core ID on which image would be running
> + - load: Target load address of the binary in hex
> +
> + Output files:
> + - certificate.bin: Signed certificate binary
> +
> +
> +
> Entry: x86-reset16: x86 16-bit reset code for U-Boot
> ----------------------------------------------------
>
> diff --git a/tools/binman/etype/ti_x509_cert.py b/tools/binman/etype/ti_x509_cert.py
> new file mode 100644
> index 0000000000..b79946c0b9
> --- /dev/null
> +++ b/tools/binman/etype/ti_x509_cert.py
> @@ -0,0 +1,241 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2018 Google, Inc
> +# Written by Simon Glass <sjg at chromium.org>
> +#
> +
> +# Support for a TI x509 certificate for signing K3 devices
> +
> +from subprocess import Popen, PIPE
Anything you want to run should normally be a bintool that provides a
pythonic wrapper for it, see tools/binman/btool for examples. But if
that's too much, openssl calls must at least use the patman.command
functions instead.
> +from sys import stderr, stdout
Don't mess with stderr, stdout. See tools/patman/command for things you
can use to run commands and capture their output.
> +import os
> +import tempfile
> +
> +from Crypto.PublicKey import RSA
Prefer Cryptodome instead.
> +
> +from binman.etype.collection import Entry_collection
> +from dtoc import fdt_util
> +from patman import tools
> +
> +
> +class Entry_ti_x509_cert(Entry_collection):
> + """An entry which contains an x509 certificate binary signed with 1024 bit RSA key
> +
> + Properties / Entry arguments:
> + - content: Phandle of binary to generate signature for
> + - key_file: File with key inside it. If not provided, script generates RSA degenrate key
> + - core: Target core ID on which image would be running
> + - load: Target load address of the binary in hex
> +
> + Output files:
> + - certificate.bin: Signed certificate binary"""
> +
> + def __init__(self, section, etype, node):
> + super().__init__(section, etype, node)
> + self.key_file = fdt_util.GetString(self._node, 'key-file', "")
> + self.core = fdt_util.GetInt(self._node, 'core', 0)
> + self.load_addr = fdt_util.GetInt(self._node, 'load', 0x41c00000)
These should actually be in ReadNode(). You should set things to
None-like values here to show which attributes will exist, and also do
it for attributes set later in other methods.
> + self.cert = fdt_util.GetString(self._node, 'cert', 'certificate.bin')
This looks unused.
> +
> + # temporary directory for intermediate files
> + self.outdir = tempfile.mkdtemp(prefix='binman.x509.')
For now, omit this and put the outputs in the build directory. See below
for an example. (We should eventually reorganize intermediate files, but
it should be done by generic helpers that all entries can use).
> +
> + def ReadNode(self):
> + super().ReadNode()
> + if self.key_file == "":
> + self.key_int_file = os.path.join(self.outdir, 'eckey.pem')
> + self.GenerateDegenKey()
> + else:
> + self.key_int_file = self.key_file
ReadNode() should only copy the device-tree values to the entry object,
it shouldn't generate the key here.
> +
> + def ObtainContents(self):
> + self.image = self.GetContents(False)
> + if self.image is None:
> + return False
> + self.image_file = os.path.join(self.outdir, 'x509.image')
> + with open(self.image_file, 'wb') as f:
> + f.write(self.image)
Prepare intermediate files like:
uniq = self.GetUniqueName()
image_fname = tools.get_output_filename(f'{uniq}.x509-image')
tools.write_file(image_fname, data)
Also, better do this in _TICreateCertificateLegacy() because you have to
do it in ProcessContents() as well.
> + self.cert_data = self._TICreateCertificateLegacy()
> + self.SetContents(self.cert_data)
> + return True
> +
> + def ProcessContents(self):
> + # The blob may have changed due to WriteSymbols()
You need to read the data again and recreate the certificate here,
because the signed data (like u-boot-spl) might have changed, which
would make the cert invalid.
> + return super().ProcessContentsUpdate(self.cert_data)
> +
> + def _TICreateCertificateLegacy(self):
> + """Create certificate for legacy boot flow"""
> +
> + sha_val = self.GetShaVal(self.image_file)
> + bin_size = self.GetFileSize(self.image_file)
> + addr = "%08x" % self.load_addr
Can be f"{self.load_addr:08x}".
> + if self.core == 16:
> + self.cert_type = 1
> + else:
> + self.cert_type = 2
> + self.debug_type = 0
> + self.bootcore_opts = 0
Looks like this doesn't handle the '# Non BOOTCORE, loaded by SYSFW'
case in k3_gen_x509_cert.sh.
> +
> + self.GenerateTemplate()
> + self.GenerateCertificate(bin_size, sha_val, addr)
> +
> + return tools.read_file(self.cert_file)
> +
> + def GetShaVal(self, binary_file):
> + process = Popen(['openssl', 'dgst', '-sha512', '-hex',
> + binary_file], stdout=PIPE, stderr=PIPE)
> + stdout, stderr = process.communicate()
> + sha_val = stdout.split()[1]
> + return sha_val
This can be something like `hashlib.sha512(data).hexdigest()`, or
something with Cryptodome.Hash.SHA512.
> +
> + def GetFileSize(self, binary_file):
> + return os.path.getsize(binary_file)
No need for an extra function for this, and maybe even for the hash.
> +
> + def ParseKey(self, inp_key, section):
> + parsed_key = ""
> + section_true = False
> + with open(inp_key, 'r') as file:
> + for line in file:
> + if section in line:
> + section_true = True
> + elif section_true:
> + if " " not in line:
> + break
> + else:
> + parsed_key += line.replace(":", "").replace(" ", "")
> + return parsed_key.replace("\n", "")
Better to use Cryptodome for parsing key information. I see there is a
Cryptodome.PublicKey.RSA.import_key() if you need to read a provided
key, but the values are already there for the degenerate key generated
below.
> +
> + def GenerateDegenKey(self):
> + """Generate a 4096 bit RSA key"""
> + # generates 1024 bit PEM encoded RSA key in PKCS#1 format
> + private_key = RSA.generate(1024)
> + self.key_pem_file = os.path.join(self.outdir, 'key.pem')
> + with open(self.key_pem_file, 'wb') as f:
> + f.write(private_key.exportKey('PEM'))
> +
> + self.key_text_file = os.path.join(self.outdir, 'key.txt')
> + process = Popen(['openssl', 'rsa', '-in', self.key_pem_file,
> + '-text', '-out', self.key_text_file], stdout=PIPE, stderr=PIPE)
> + stdout, stderr = process.communicate()
> +
> + DEGEN_MODULUS = self.ParseKey(self.key_text_file, "modulus")
> + DEGEN_P = self.ParseKey(self.key_text_file, "prime1")
> + DEGEN_Q = self.ParseKey(self.key_text_file, "prime2")
> + DEGEN_COEFF = self.ParseKey(self.key_text_file, "coefficient")
These should be derived from private_key.{n,p,q,u}. I guess
f"{private_key.n:x}" and so on should work?
I tried a bit, and looks like pycryptodome and openssl disagree on the
coefficient definition, but I can get what openssl gives by switching
the p and q like `RSA.construct((n, e, d, q, p)).u`
> +
> + self.GenerateDegenTemplate()
> +
> + self.degen_key = os.path.join(self.outdir, 'x509.degenerateKey.txt')
> + with open(self.degen_temp_file, 'r') as file_input:
> + with open(self.degen_key, 'w') as file_output:
> + for line in file_input:
> + s = line.replace("DEGEN_MODULUS", DEGEN_MODULUS).replace(
> + "DEGEN_P", DEGEN_P).replace("DEGEN_Q", DEGEN_Q).replace("DEGEN_COEFF", DEGEN_COEFF)
> + file_output.write(s)
It would be easier to fill the template in Python with str.format(),
something like:
DEGENERATE_KEY_TEMPLATE = """\
asn1=SEQUENCE:rsa_key
[rsa_key]
version=INTEGER:0
modulus=INTEGER:0x{modulus:x}
pubExp=INTEGER:1
privExp=INTEGER:1
p=INTEGER:0x{p:x}
q=INTEGER:0x{q:x}
e1=INTEGER:1
e2=INTEGER:1
coeff=INTEGER:0x{coeff:x}
"""
coeff = RSA.construct((
private_key.n, private_key.e, private_key.d,
private_key.q, private_key.p,
)).u
degen_conf = DEGENERATE_KEY_TEMPLATE.format(
modulus=private_key.n,
p=private_key.p,
q=private_key.q,
coeff=coeff,
)
> +
> + self.degen_key_der = os.path.join(
> + self.outdir, 'x509.degenerateKey.der')
> + process = Popen(['openssl', 'asn1parse', '-genconf', self.degen_key,
> + '-out', self.degen_key_der], stdout=PIPE, stderr=PIPE)
> + stdout, stderr = process.communicate()
> +
> + process = Popen(['openssl', 'rsa', '-in', self.degen_key_der,
> + '-inform', 'DER', '-outform', 'PEM', '-out', self.key_int_file])
> + stdout, stderr = process.communicate()
> +
> + def GenerateCertificate(self, bin_size, sha_val, addr):
> + self.temp_x509 = os.path.join(self.outdir, 'x509.temp.cert')
> + self.cert_file = os.path.join(self.outdir, 'x509.certificate.bin')
> +
> + temp_dict = {}
> + temp_dict['TEST_IMAGE_LENGTH'] = str(bin_size)
> + temp_dict['TEST_IMAGE_SHA_VAL'] = sha_val.decode("utf-8")
> + temp_dict['TEST_CERT_TYPE'] = str(self.cert_type)
> + temp_dict['TEST_BOOT_CORE_OPTS'] = str(self.bootcore_opts)
> + temp_dict['TEST_BOOT_CORE'] = str(self.core)
> + temp_dict['TEST_BOOT_ADDR'] = str(addr)
> + temp_dict['TEST_DEBUG_TYPE'] = str(self.debug_type)
A literal dict looks better here:
temp_dict = {
'TEST_IMAGE_LENGTH': str(bin_size),
...
}
> +
> + with open(self.temp_x509, "w") as output_file:
> + with open(self.temp_file, "r") as input_file:
> + for line in input_file:
> + l = line
> + for key in temp_dict:
> + if key in line:
> + l = l.replace(key, temp_dict[key])
> + output_file.write(l)
But again, this would be easier with str.format():
X509_TEMPLATE = """\
[...]
[ boot_seq ]
certType = INTEGER:{certType}
bootCore = INTEGER:{bootCore}
bootCoreOpts = INTEGER:{bootCoreOpts}
destAddr = FORMAT:HEX,OCT:{destAddr:08x}
imageSize = INTEGER:{imageSize}
[...]
"""
x509_config = X509_TEMPLATE.format(
...
'certType': self.cert_type,
'bootCoreOpts': self.bootcore_opts,
'bootCore': self.core,
'destAddr': addr,
...
)
> +
> + process = Popen(['openssl', 'req', '-new', '-x509', '-key', self.key_int_file, '-nodes', '-outform',
> + 'DER', '-out', self.cert_file, '-config', self.temp_x509, '-sha512'], stdout=PIPE, stderr=PIPE)
> + stdout, stderr = process.communicate()
Read the cert file here and return its bytes, instead of leaving it to
_TICreateCertificateLegacy().
In general I think creating fewer files and limiting their scopes to as
small as possible is better.
> +
> + def GenerateDegenTemplate(self):
> + self.degen_temp_file = os.path.join(self.outdir, 'x509.degen-template')
> + with open(self.degen_temp_file, 'w+', encoding='utf-8') as f:
> + degen_temp = """
> +asn1=SEQUENCE:rsa_key
> +
> +[rsa_key]
> +version=INTEGER:0
> +modulus=INTEGER:0xDEGEN_MODULUS
> +pubExp=INTEGER:1
> +privExp=INTEGER:1
> +p=INTEGER:0xDEGEN_P
> +q=INTEGER:0xDEGEN_Q
> +e1=INTEGER:1
> +e2=INTEGER:1
> +coeff=INTEGER:0xDEGEN_COEFF"""
> + f.write(degen_temp)
> +
> + def GenerateTemplate(self):
> + self.temp_file = os.path.join(self.outdir, 'x509.template')
> + with open(self.temp_file, 'w+', encoding='utf-8') as f:
> + x509template = """
> +[ req ]
> +distinguished_name = req_distinguished_name
> +x509_extensions = v3_ca
> +prompt = no
> +dirstring_type = nobmp
> +
> +[ req_distinguished_name ]
> +C = US
> +ST = TX
> +L = Dallas
> +O = Texas Instruments Incorporated
> +OU = Processors
> +CN = TI support
> +emailAddress = support at ti.com
> +
> +[ v3_ca ]
> +basicConstraints = CA:true
> +1.3.6.1.4.1.294.1.1 = ASN1:SEQUENCE:boot_seq
> +1.3.6.1.4.1.294.1.2 = ASN1:SEQUENCE:image_integrity
> +1.3.6.1.4.1.294.1.3 = ASN1:SEQUENCE:swrv
> +# 1.3.6.1.4.1.294.1.4 = ASN1:SEQUENCE:encryption
> +1.3.6.1.4.1.294.1.8 = ASN1:SEQUENCE:debug
> +
> +[ boot_seq ]
> +certType = INTEGER:TEST_CERT_TYPE
> +bootCore = INTEGER:TEST_BOOT_CORE
> +bootCoreOpts = INTEGER:TEST_BOOT_CORE_OPTS
> +destAddr = FORMAT:HEX,OCT:TEST_BOOT_ADDR
> +imageSize = INTEGER:TEST_IMAGE_LENGTH
> +
> +[ image_integrity ]
> +shaType = OID:2.16.840.1.101.3.4.2.3
> +shaValue = FORMAT:HEX,OCT:TEST_IMAGE_SHA_VAL
> +
> +[ swrv ]
> +swrv = INTEGER:0
> +
> +# [ encryption ]
> +# initalVector = FORMAT:HEX,OCT:TEST_IMAGE_ENC_IV
> +# randomString = FORMAT:HEX,OCT:TEST_IMAGE_ENC_RS
> +# iterationCnt = INTEGER:TEST_IMAGE_KEY_DERIVE_INDEX
> +# salt = FORMAT:HEX,OCT:TEST_IMAGE_KEY_DERIVE_SALT
> +
> +[ debug ]
> +debugUID = FORMAT:HEX,OCT:0000000000000000000000000000000000000000000000000000000000000000
> +debugType = INTEGER:TEST_DEBUG_TYPE
> +coreDbgEn = INTEGER:0
> +coreDbgSecEn = INTEGER:0"""
> + f.write(x509template)
I think these templates would be better as top-level constants.
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 3709b68297..70bda9738c 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -96,6 +96,7 @@ ENV_DATA = b'var1=1\nvar2="2"'
> PRE_LOAD_MAGIC = b'UBSH'
> PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big')
> PRE_LOAD_HDR_SIZE = 0x00001000.to_bytes(4, 'big')
> +X509_DATA = b'filetobesigned'
>
> # Subdirectory of the input dir to use to put test FDTs
> TEST_FDT_SUBDIR = 'fdts'
> @@ -200,6 +201,7 @@ class TestFunctional(unittest.TestCase):
> TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
> TestFunctional._MakeInputFile('sysfw.bin', TI_SYSFW_DATA)
> TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
> + TestFunctional._MakeInputFile('tosign.bin', X509_DATA)
>
> # Add a few .dtb files for testing
> TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
> @@ -5716,6 +5718,10 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
> dts='234_replace_section_simple.dts')
> self.assertEqual(new_data, data)
>
> + def testX509Cert(self):
> + """Test an image with the default x509 certificate header"""
> + data = self._DoReadFile('232_x509_cert.dts')
> + self.assertGreater(len(data), len(X509_DATA))
If you used a bintool, you could do something like _HandleVblockCommand
to capture the openssl calls, create a mock file as the output
certificate, and check that here.
>
> if __name__ == "__main__":
> unittest.main()
> diff --git a/tools/binman/test/232_x509_cert.dts b/tools/binman/test/232_x509_cert.dts
> new file mode 100644
> index 0000000000..3e68309de5
> --- /dev/null
> +++ b/tools/binman/test/232_x509_cert.dts
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + binman {
> + ti-x509-cert {
> + content = <&image>;
> + };
> +
> + image: blob-ext {
> + filename = "tosign.bin";
> + };
> + };
> +};
More information about the U-Boot
mailing list