[PATCH RFC v2 05/11] ti: etype: x509: Add etype for x509 certificate for K3 devices

Neha Malcom Francis n-francis at ti.com
Thu Jun 2 06:09:52 CEST 2022


Hi Roger,

On 01/06/22 16:18, Roger Quadros wrote:
> Neha,
> 
> On 01/06/2022 12:48, Neha Malcom Francis wrote:
>> Hi Roger,
>>
>> On 01/06/22 14:54, Roger Quadros wrote:
>>>
>>>
>>> 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-francis at 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 <sjg at 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.
>> Thanks!
>>
>>>
>>>>>> +
>>>>>> +
>>>>>> +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           = 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)
>>>>>> +
>>>>>> +
>>>>>> +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?
>>
>> 938 was the size of the expected x509 certificate to which the data is appended.
> 
> And that is not exepected to change anytime in future?
> If not, then you can use a macro define for that.
> If yes, then this solution is not good.
> 

Right, I'll make changes to the testing.

>>
>>>>>
>>>>> Isn't it easier to just assert that _DoReadFile('232_x509_cert.dts') is greater than len(X509_DATA)?
>> Would that test ensure the proper creation of certificate.bin?
>>
> No but it would just tell you that something was appended or not and
> will not break if certificate size changes.
> 
> Your above check was not checking certificate contents either.
> 

I'm not sure about where to draw the line to test the contents of the 
certificate, I'll ask around about that. If you have any suggestions 
please let me know. I will definitely add that test to check whether 
anything was appended or not for now.

>>>>>
>>>>>> +
>>>>>>     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.
>>
>> These file changes (k3_gen_x509_cert.sh) were not intended to be present in the patch series, sorry for that.
>>
> 
> 
> cheers,
> -roger

-- 
Thanking You
Neha Malcom Francis


More information about the U-Boot mailing list