WIP: Signing TI x509 certificates using binman

Neha Malcom Francis n-francis at ti.com
Mon Apr 3 10:30:40 CEST 2023 

Hi Simon

On 03/04/23 00:16, Simon Glass wrote:
> Hi Neha,
> 
> On Mon, 3 Apr 2023 at 02:19, Neha Malcom Francis <n-francis at ti.com> wrote:
>>
>> Hi Simon
>>
>> On 02/04/23 18:00, Neha Malcom Francis wrote:
>>> Hi Simon
>>>
>>> On 01/04/23 12:02, Simon Glass wrote:
>>>> Hi Neha,
>>>>
>>>> On Sat, 1 Apr 2023 at 00:14, Neha Malcom Francis <n-francis at ti.com>
>>>> wrote:
>>>>>
>>>>> Hi Simon
>>>>>
>>>>> On 31/03/23 02:01, Simon Glass wrote:
>>>>>> Hi Neha,
>>>>>>
>>>>>> On Fri, 24 Mar 2023 at 22:28, Neha Malcom Francis <n-francis at ti.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Simon,
>>>>>>>
>>>>>>> Before I roll out the entire series that works for packaging K3
>>>>>>> bootloader images, wanted to get some reviews and comments
>>>>>>> regarding the
>>>>>>> implementation of the signing etype [1] . I believe I've taken into
>>>>>>> consideration what we've discussed earlier [2].
>>>>>>>
>>>>>>> Let me know your thoughts. The tree is also WIP, and was mainly
>>>>>>> designed
>>>>>>> for testing the signing etype on two of the devices. I will add the
>>>>>>> remaining and refine the series based on the next comments.
>>>>>>
>>>>>> Yes this looks reasonable to me. For the openssl method, can you
>>>>>> create a new 'real' method and put the cert stuff in there instead of
>>>>>> using a 'custom' one? It seems to have a lot in common with what is
>>>>>> there. We should really have an internal cert-generator method in that
>>>>>> class, with other functions in that class doing appropriate things. I
>>>>>> am looking for code reuse here, as well as clear indication of what
>>>>>> the cert is for or does.
>>>>>
>>>>> So you're suggesting to include the config creation within the openssl
>>>>> btool, am I right? For example methods to generate a config, add section
>>>>> to a config, add extension to a section of a config... etc? I can take a
>>>>> look at implementing this if this is what you have suggested.
>>>>
>>>> I am more thinking that your use case could be a new method in that
>>>> file, with arguments that suit your case, but with common code to
>>>> create a cert of the correct type. That means pulling out the existing
>>>> cert creation into a new (internal) function in that btool that does
>>>> both types of cert. For the CN args I think a dict would be suitable
>>>> to pass in the settings.
>>>>
>>>> I'm trying to avoid people passing in a cert every time, since that is
>>>> going to create a lot of code duplication and it will be hard to take
>>>> advantage of common algos.
>>>>
>>>
>>> Okay I think I get it, let me respin with this in mind.
>>
>> I am still a little doubtful about putting in *this* cert generation in
>> the openssl btool. These sections and extensions are TI specific and I
>> don't think there would be code duplication in future. The least code
>> duplication along with functionality would be acheived by allowing
>> creating configs, sections, extensions etc. in openssl btool or x509
>> etype. and then creating the final cert binary in openssl. What do you
>> think?
> 
> It is tricky because we only have one example in the etype at present
> so it is always hard to know which way to go. I think it will be
> clearer down the track when we have a bit more code here. But if we go
> with having the bintool just be a shell, then we definitely won't get
> any reuse for the next customer.
> 
> The bit I would like to see in the bintool is the creation of the text
> file containing the cert. The etype should ideally provide properties
> (which can be set up the ti_secure etype in the __init__() method)
> that cover the required openssl params. The simplest thing would be
> just 'this is TI signing' but I would like to pick this apart a bit so
> that the properties are more generic, if possible.
> 
> So the btool takes arguments that tell it about the text file to
> create. It creates the text file and runs openssl.
> 
> The x509 etype calls the openssl bintool's z509 method to actually
> create the cert, presumably passing quite a few args and maybe a dict.
> 
> So:
> 
> - expand openssl.x509_cert() with more args
> - expand x509 etype with more properties (using the defaults)
> - update your ti-secure etype to set those properties in certain ways
> 
> Since the bintool code that is there is just an example cert, you can
> switch it to do whatever you want for this case, i.e. change the
> defaults. That might simplify things for you.
> 
> Anyway, for an initial series I think that will provide a good basis
> for this work. If we need to refactor later, we have our test coverage
> to help us.
> 
> Regards,
> Simon
> 
> 

Thanks for explaining! I will work on it this way.

>>
>>>>>
>>>>> If most of the cert is the same, you could
>>>>>> pass a dict for the CN stuff, perhaps?
>>>>>>
>>>>>> What is the taml for? It is hard to tell, from the example provided.
>>>>> Did you mean YAML? If so in the patch I linked, I don't think I had a
>>>>> example for that. Could you point out what exactly you're asking about?
>>>>
>>>> Well I am wondering why it is in the code...is there a yaml file that
>>>> you need to ingest and process? What is it for?
>>>>
>>>> ...actually I see the yaml files in your tree mentioned below. Does
>>>> this come out of some other tool?
>>>>
>>>
>>> Right, so currently we have certain board specific configurations in
>>> YAML. This is input to our board configuration binary entry type [6],
>>> which converts YAML into binary blobs prepended with certain headers
>>> needed.
>>>
>>>>>
>>>>>>
>>>>>> Do you have a .dts which shows the full image for a board? I think the
>>>>>> cert stuff looks right, but it's a bit hard to tell.
>>>>>
>>>>> Yes, in the same github link I have the whole tree [3] that contains
>>>>> DTBs for couple of the devices [4] and [5], please have a look and let
>>>>> me know (I might force push a few changes in the next couple of days, so
>>>>> better to look from the tree)
>>>>
>>>> OK I see. Yes it looks pretty good to me.
>>>>
>>>>>
>>>>>>
>>>>>> When sending the patches please do do follow the function-commenting
>>>>>> style and make sure that it is clear what each arg means. E.g. I saw a
>>>>>> hash integer which I assume is used to pass 256 or 384 or 512 for sha
>>>>>> hashing. It should indicate the possible values / meaning in the arg.
>>>>>> In fact, in that case, it might be better to pass a string like
>>>>>> 'sha256'.
>>>>>
>>>>> Yes, you're right. Will address that when sending the patch series
>>>>> completely.
>>>>
>>>> OK
>>>>
>>>>>
>>>>>>
>>>>>> Anyway, apart from my questions it seems good.
>>>>>>
>>>>>> Regards,
>>>>>> Simon
>>>>>>
>>>>>>>
>>>>>>> [1]
>>>>>>> https://github.com/nehamalcom/u-boot/commit/ea7413ed5864340bd6f01e704e8bdcc073a7896b#diff-efb03d61a324724c4f86bf42b45c4e4e614cab18e1b3184f63721d62280a11b5
>>>>>>>
>>>>>>> [2]
>>>>>>> https://patchwork.ozlabs.org/project/uboot/patch/20230224120340.587786-1-n-francis@ti.com/
>>>>>>>
>>>>>>> --
>>>>>>> Thanking You
>>>>>>> Neha Malcom Francis
>>>>>
>>>>> [3] https://github.com/nehamalcom/u-boot/commits/ti-secure-functionality
>>>>> [4]
>>>>> https://github.com/nehamalcom/u-boot/commit/dda1f9caf436df59c576466f35262df90aa1c0af
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>>
>>> [6]
>>> https://github.com/nehamalcom/u-boot/commit/3ca258562c374e553a7005a5b6d6441358801e78
>>>
>>
>> --
>> Thanking You
>> Neha Malcom Francis

-- 
Thanking You
Neha Malcom Francis

More information about the U-Boot mailing list