On 01/06/2022 09:02, Neha Malcom Francis wrote:
> Hi Roger,
>
> On 31/05/22 14:50, Roger Quadros wrote:
>>
>>
>> On 06/05/2022 07:37, Neha Malcom Francis wrote:
>>> K3 devices x509 certificate added to certain binaries that allows ROM to
>>
>> what binaries?
>>
>>> validate the integrity of the image. Etype that generates an x509
>>> certificate depending on boot flow added.
>>
>> Could you please explain in more detail as to what exactly is happening here.
>>
>> What do you mean by "depending on boot flow"?
>>
>
> I will reformat the commit messages accordingly.
>>>
>>> Signed-off-by: Neha Malcom Francis <n-fran...@ti.com>
>>> ---
>>> tools/binman/entries.rst | 15 ++
>>> tools/binman/etype/x509_cert.py | 248 ++++++++++++++++++++++++++++
>>> tools/binman/ftest.py | 7 +
>>> tools/binman/test/232_x509_cert.dts | 18 ++
>>> tools/k3_gen_x509_cert.sh | 10 +-
>>> 5 files changed, 293 insertions(+), 5 deletions(-)
>>> create mode 100644 tools/binman/etype/x509_cert.py
>>> create mode 100644 tools/binman/test/232_x509_cert.dts
>>>
>>> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
>>> index 0c6d82fce8..dfa281e49f 100644
>>> --- a/tools/binman/entries.rst
>>> +++ b/tools/binman/entries.rst
>>> @@ -1890,6 +1890,21 @@ and kernel are genuine.
>>> +Entry: x509cert: x509 certificate for K3 devices
>>> +------------------------------------------------
>>> +
>>
>> x509 is a generic standard. Can this be made usable by other vendors as well
>> or
>> is it very specific to TI?
>> If this is TI specific then I'd suggest a "ti-" prefix to the entry name.
>>
>>> +Properties / Entry arguments:
>>> + - content: Phandle of binary to sign
>>> + - output: Name of the final output file
>>
>> why do you need output property?
>>
>
> That is not required, I had later changed it to always using certificate.bin.
> Will make the necessary changes.
>
>>> + - 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/x509_cert.py
>>> b/tools/binman/etype/x509_cert.py
>>> new file mode 100644
>>> index 0000000000..0009973155
>>> --- /dev/null
>>> +++ b/tools/binman/etype/x509_cert.py
>>> @@ -0,0 +1,248 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +# Copyright (c) 2018 Google, Inc
>>> +# Written by Simon Glass <s...@chromium.org>
>>> +#
>>> +
>>> +# Support for a x509 certificate for signing K3 devices
>>> +
>>> +import os
>>> +from collections import OrderedDict
>>> +from subprocess import Popen, PIPE
>>> +from sys import stderr, stdout
>>> +
>>> +import asn1
>>> +from Crypto.PublicKey import RSA
>>> +from cryptography.hazmat.backends import default_backend
>>> +from cryptography.hazmat.primitives import serialization
>>> +
>>> +from binman.etype.collection import Entry_collection
>>> +from dtoc import fdt_util
>>> +from patman import tools
>>> +
>>> +temp_x509 = "x509-temp.cert"
>>> +cert = "certificate.bin"
>>> +rand_key = "eckey.pem"
>>> +bootcore_opts = 0
>>> +bootcore = 0
>>> +debug_type = 0
>>> +
>>> +
>>> +class Entry_x509_cert(Entry_collection):
>>> + """ An entry which contains a x509 certificate
>>> +
>>> + Properties / Entry arguments:
>>> + - content: Phandle of binary to sign
>>> + - 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"""
>>> +
>>> + 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)
>>> +
>>> + def ReadNode(self):
>>> + super().ReadNode()
>>> + if self.key_file == "":
>>> + self.degen_key = True
>>> + else:
>>> + self.degen_key = False
>>> +
>>> + def _CreateCertificate(self):
>>> + """Create certificate for legacy boot flow"""
>>> + if self.degen_key == True:
>>> + gen_degen_key()
>>> + self.key_file = rand_key
>>> +
>>> + sha_val = get_sha_val("intermediate-sysfw.bin")
>>> + bin_size = get_file_size("intermediate-sysfw.bin")
>>> + addr = "%08x" % self.load_addr
>>> + if self.core == 0:
>>> + cert_type = 2
>>> + elif self.core == 16:
>>> + cert_type = 1
>>> + else:
>>> + cert_type = 2
>>> + debug_type = 0
>>> +
>>> + gen_template()
>>> + gen_cert(bin_size, sha_val, cert_type, bootcore_opts,
>>> + self.core, addr, debug_type, self.key_file)
>>> +
>>> + return tools.read_file("certificate.bin")
>>> +
>>> + def ObtainContents(self):
>>> + self.image = self.GetContents(False)
>>> + if self.image is None:
>>> + return False
>>> + f = open("intermediate-sysfw.bin", "wb")
>>> + f.write(self.image)
>>> + f.close()
>>> + self.SetContents(self._CreateCertificate())
>>> + return True
>>> +
>>> + def ProcessContents(self):
>>> + data = self._CreateCertificate()
>>> + return self.ProcessContentsUpdate(data)
>>
>> Why do you need _CreateCertificate() and ProcessContents()?
>> Just have one ObtainContents() and try to get rid of all the intermediate
>> files.
>>
>
> I used etype/vblock.py as a reference. I will clean up this etype further.
>
There were some more comments below, in case you missed them.
>>> +
>>> +
>>> +def get_sha_val(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
>>> +
>>> +
>>> +def get_file_size(binary_file):
>>> + return os.path.getsize(binary_file)
>>> +
>>> +
>>> +def gen_degen_template():
>>> + with open("degen-template.txt", '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 gen_template():
>>> + """Generate x509 Template"""
>>> + with open("x509-template.txt", "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 = supp...@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)
>>> +
>>> +
>>> +def parse_key(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", "")
>>> +
>>> +
>>> +def gen_degen_key():
>>> + """Generate a 4096 bit RSA key"""
>>> + try:
>>> + # generates 1024 bit PEM encoded RSA key in PKCS#1 format
>>> + private_key = RSA.generate(1024)
>>> + f = open('key.pem', 'wb')
>>> + f.write(private_key.exportKey('PEM'))
>>> + f.close()
>>> + except:
>>> + raise(Exception)
>>> +
>>> + try:
>>> + process = Popen(['openssl', 'rsa', '-in', 'key.pem',
>>> + '-text', '-out', 'key.txt'], stdout=PIPE,
>>> stderr=PIPE)
>>> + stdout, stderr = process.communicate()
>>> + except:
>>> + raise(stderr)
>>> +
>>> + DEGEN_MODULUS = parse_key("key.txt", "modulus")
>>> + DEGEN_P = parse_key("key.txt", "prime1")
>>> + DEGEN_Q = parse_key("key.txt", "prime2")
>>> + DEGEN_COEFF = parse_key("key.txt", "coefficient")
>>> +
>>> + gen_degen_template()
>>> +
>>> + with open("degen-template.txt", 'r') as file_input:
>>> + with open("degenerateKey.txt", '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)
>>> +
>>> + try:
>>> + process = Popen(['openssl', 'asn1parse', '-genconf',
>>> 'degenerateKey.txt',
>>> + '-out', 'degenerateKey.der'], stdout=PIPE,
>>> stderr=PIPE)
>>> + stdout, stderr = process.communicate()
>>> + except:
>>> + raise(stderr)
>>> +
>>> + try:
>>> + process = Popen(['openssl', 'rsa', '-in', 'degenerateKey.der',
>>> + '-inform', 'DER', '-outform', 'PEM', '-out',
>>> rand_key])
>>> + stdout, stderr = process.communicate()
>>> + except:
>>> + raise(stderr)
>>> +
>>> +
>>> +def gen_cert(bin_size, sha_val, cert_type, bootcore_opts, bootcore, addr,
>>> debug_type, key):
>>> + with open(temp_x509, "w") as output_file:
>>> + with open("x509-template.txt", "r") as input_file:
>>> + for line in input_file:
>>> + output_file.write(line.replace("TEST_IMAGE_LENGTH",
>>> str(bin_size)).replace("TEST_IMAGE_SHA_VAL",
>>> sha_val.decode("utf-8")).replace("TEST_CERT_TYPE", str(cert_type)).replace(
>>> + "TEST_BOOT_CORE_OPTS",
>>> str(bootcore_opts)).replace("TEST_BOOT_CORE",
>>> str(bootcore)).replace("TEST_BOOT_ADDR",
>>> str(addr)).replace("TEST_DEBUG_TYPE", str(debug_type)))
>>> + process = Popen(['openssl', 'req', '-new', '-x509', '-key', key,
>>> '-nodes', '-outform',
>>> + 'DER', '-out', cert, '-config', temp_x509, '-sha512'],
>>> stdout=PIPE, stderr=PIPE)
>>> + stdout, stderr = process.communicate()
>>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>>> index 5ff294a386..d8ee592250 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,
>>> @@ -5537,5 +5539,10 @@ fdt fdtmap Extract the
>>> devicetree blob from the fdtmap
>>> data = self._DoReadFile('232_ti_sysfw.dts')
>>> self.assertEqual(TI_SYSFW_DATA, data[:len(TI_SYSFW_DATA)])
>>> + def testX509Cert(self):
>>> + """Test an image with the default x509 certificate header"""
>>> + data = self._DoReadFile('232_x509_cert.dts')
>>> + self.assertEqual(X509_DATA, data[938:938 + len(X509_DATA)])
>>
>> what is 938?
>>
>> Isn't it easier to just assert that _DoReadFile('232_x509_cert.dts') is
>> greater than len(X509_DATA)?
>>
>>> +
>>> 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..f768568ca7
>>> --- /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 {
>>> + x509-cert {
>>> + content = <&image>;
>>> + };
>>> +
>>> + image: blob-ext {
>>> + filename = "tosign.bin";
>>> + };
>>> + };
>>> +};
>>> diff --git a/tools/k3_gen_x509_cert.sh b/tools/k3_gen_x509_cert.sh
>>> index 298cec1313..b6ef5a2de3 100755
>>> --- a/tools/k3_gen_x509_cert.sh
>>> +++ b/tools/k3_gen_x509_cert.sh
>>> @@ -109,7 +109,7 @@ gen_degen_key() {
>>> openssl asn1parse -genconf degenerateKey.txt -out degenerateKey.der
>>> >>/dev/null 2>&1
>>> openssl rsa -in degenerateKey.der -inform DER -outform PEM -out
>>> $RAND_KEY >>/dev/null 2>&1
>>> KEY=$RAND_KEY
>>> - rm key.pem key.txt degen-template.txt degenerateKey.txt
>>> degenerateKey.der
>>> + #rm key.pem key.txt degen-template.txt degenerateKey.txt
>>> degenerateKey.der
>>> }
>>> declare -A options_help
>>> @@ -246,7 +246,7 @@ gen_cert
>>> cat $CERT $BIN > $OUTPUT
>>> # Remove all intermediate files
>>> -rm $TEMP_X509 $CERT x509-template.txt
>>> -if [ "$KEY" == "$RAND_KEY" ]; then
>>> - rm $RAND_KEY
>>> -fi
>>> +#rm $TEMP_X509 $CERT x509-template.txt
>>> +#if [ "$KEY" == "$RAND_KEY" ]; then
>>> +# rm $RAND_KEY
>>> +#fi
>>
>> Why these changes?
>> Maybe you should include them within
>> "ifndef CONFIG_BINMAN ... endif" to avoid breaking platforms not using
>> BINMAN.
>>
>> cheers,
>> -roger
>
cheers,
-roger
