WIP: Signing TI x509 certificates using binman

[Neha Malcom Francis]

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.
>>
>> 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