[PATCH v2 02/18] binman: ti-secure: Add support for TI signing

[Neha Malcom Francis]

Hi Simon

On 06/04/23 00:07, Simon Glass wrote:
> kHi Neha,
> 
> On Wed, 5 Apr 2023 at 00:13, Neha Malcom Francis <n-francis at ti.com> wrote:
>>
>> The ti-secure entry contains certificate for binaries that will be
>> loaded or booted by system firmware whereas the ti-secure-rom entry
>> contains certificate for binaries that will be booted by ROM. Support
>> for both these types of certificates is necessary for booting of K3
>> devices.
>>
>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>> ---
>>   board/ti/keys/custMpk.pem                     |  51 ++++
>>   board/ti/keys/ti-degenerate-key.pem           |  10 +
>>   tools/binman/btool/openssl.py                 | 244 ++++++++++++++++++
>>   tools/binman/entries.rst                      |  25 ++
>>   tools/binman/etype/ti_secure.py               |  83 ++++++
>>   tools/binman/etype/ti_secure_rom.py           | 233 +++++++++++++++++
>>   tools/binman/etype/x509_cert.py               |  87 ++++++-
>>   tools/binman/ftest.py                         |  46 ++++
>>   tools/binman/test/279_ti_secure.dts           |  17 ++
>>   tools/binman/test/280_ti_secure_rom.dts       |  17 ++
>>   .../test/281_ti_secure_rom_combined.dts       |  25 ++
>>   11 files changed, 830 insertions(+), 8 deletions(-)
>>   create mode 100644 board/ti/keys/custMpk.pem
>>   create mode 100644 board/ti/keys/ti-degenerate-key.pem
>>   create mode 100644 tools/binman/etype/ti_secure.py
>>   create mode 100644 tools/binman/etype/ti_secure_rom.py
>>   create mode 100644 tools/binman/test/279_ti_secure.dts
>>   create mode 100644 tools/binman/test/280_ti_secure_rom.dts
>>   create mode 100644 tools/binman/test/281_ti_secure_rom_combined.dts
> 
>> diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
>> index 3a4dbdd6d7..aad3b61ae2 100644
>> --- a/tools/binman/btool/openssl.py
>> +++ b/tools/binman/btool/openssl.py
>> @@ -15,6 +15,13 @@ import hashlib
>>   from binman import bintool
>>   from u_boot_pylib import tools
>>
>> +
>> +VALID_SHAS = [256, 384, 512, 224]
>> +SHA_OIDS = {256:'2.16.840.1.101.3.4.2.1',
>> +            384:'2.16.840.1.101.3.4.2.2',
>> +            512:'2.16.840.1.101.3.4.2.3',
>> +            224:'2.16.840.1.101.3.4.2.4'}
>> +
>>   class Bintoolopenssl(bintool.Bintool):
>>       """openssl tool
>>
>> @@ -74,6 +81,243 @@ imageSize              = INTEGER:{len(indata)}
>>                   '-sha512']
>>           return self.run_cmd(*args)
>>
>> +    def x509_cert_sysfw(self, cert_fname, input_fname, key_fname, sw_rev,
>> +                  config_fname, req_dist_name_dict):
>> +        """Create a certificate to be booted by system firmware
>> +
>> +        Args:
>> +            cert_fname (str): Filename of certificate to create
>> +            input_fname (str): Filename containing data to sign
>> +            key_fname (str): Filename of .pem file
>> +            sw_rev (int): Software revision
>> +            config_fname (str): Filename to write fconfig into
>> +            req_dist_name_dict (dict): Dictionary containing key-value pairs of
>> +            req_distinguished_name section extensions, must contain extensions for
>> +            C, ST, L, O, OU, CN and emailAddress
>> +
>> +        Returns:
>> +            str: Tool output
>> +        """
>> +        indata = tools.read_file(input_fname)
>> +        hashval = hashlib.sha512(indata).hexdigest()
>> +        with open(config_fname, 'w', encoding='utf-8') as outf:
>> +            print(f'''[ req ]
>> +distinguished_name     = req_distinguished_name
>> +x509_extensions        = v3_ca
>> +prompt                 = no
>> +dirstring_type         = nobmp
>> +
>> +[ req_distinguished_name ]
>> +C                      = {req_dist_name_dict['C']}
>> +ST                     = {req_dist_name_dict['ST']}
>> +L                      = {req_dist_name_dict['L']}
>> +O                      = {req_dist_name_dict['O']}
>> +OU                     = {req_dist_name_dict['OU']}
>> +CN                     = {req_dist_name_dict['CN']}
>> +emailAddress           = {req_dist_name_dict['emailAddress']}
>> +
>> +[ v3_ca ]
>> +basicConstraints       = CA:true
>> +1.3.6.1.4.1.294.1.3    = ASN1:SEQUENCE:swrv
>> +1.3.6.1.4.1.294.1.34   = ASN1:SEQUENCE:sysfw_image_integrity
>> +1.3.6.1.4.1.294.1.35   = ASN1:SEQUENCE:sysfw_image_load
>> +
>> +[ swrv ]
>> +swrv = INTEGER:{sw_rev}
>> +
>> +[ sysfw_image_integrity ]
>> +shaType                = OID:2.16.840.1.101.3.4.2.3
>> +shaValue               = FORMAT:HEX,OCT:{hashval}
>> +imageSize              = INTEGER:{len(indata)}
> 
> There's a lot of duplication here, but at least it is in one file.
> 
> Would it make sense, for example, to have a function like
> 
> add_dn(buf, dict)
> 
> which adds the req_distinguished_name to a stringio buffer? Then that
> could be calls from multiple places.
> 

Right that is possible, plus it gives an outline if this etype can be 
simplified in the future similarly.

> Also, please check test coverage (binman test -T). That should be 100%
> so you will need to add tests for failing cases as well.
> 

I'll take note of these corrections and send v3 soon, thanks!

> Regards,
> Simon

-- 
Thanking You
Neha Malcom Francis