[PATCH] imx: hab: add documentation about the required keys/certs

[Claudius Heine]

Hi Rasmus,

On 2024-05-16 11:50 am, Rasmus Villemoes wrote:
> On 16/05/2024 10.25, Claudius Heine wrote:
>> Hi Tim and Marek,
>>
>> On 2024-05-16 12:46 am, Tim Harvey wrote:
>>> On Tue, May 14, 2024 at 11:50 AM Tim Harvey <tharvey at gateworks.com>
>>> wrote:
>>>> On Sun, May 12, 2024 at 10:08 PM Marek Vasut <marex at denx.de> wrote:
>>>>> On 5/8/24 9:23 AM, Claudius Heine wrote:
>>>>>> On 2024-05-07 3:28 pm, Marek Vasut wrote:
>>>>>>> It would be good to mention the DT properties which govern the crypto
>>>>>>> material paths -- nxp,srk-table, nxp,csf-crt, nxp,img-crt --
>>>>>>> somewhere
>>>>>>> around this sentence.
>>>>>>
>>>>>> This is something that should be documented with the changes where
>>>>>> that
>>>>>> code was added, IMO. I only documented here what I found out and have
>>>>>> used myself, I haven't used those.
>>>>>>
>>>>>> I would be interested in reading how to best overwrite those paths and
>>>>>> the image structured from board u-boot.dtsi files myself.
>>>>>>
>>>>>> If you want to can pickup my patch and integrate it into your
>>>>>> series and
>>>>>> extend it.
>>>>>
>>>>> I'll keep it in mind for V3.
>>>
>>> Hi Marek,
>>>
>>> The documentation patch here by Claudius does resolve my issues
>>> discussed in the other thread and I can confirm symlinks work fine so
>>> I think something like the following should be added:
>>>
>>> CST_DIR=/usr/src/cst-3.3.2/
>>> ln -s $CST_DIR/crts .
>>> ln -s $CST_DIR/keys .
>>
>> `keys` and `crts` are very short and generic names, and putting them
>> into the build directory might cause issues at some point. But I would
>> not be against putting them into a sub directory (`imx-hab/{keys,crts}`?).
> 
> It is probably useful to be aware of the quality of the cst code. For
> reference, I quote get_key_file()
> 
> int32_t get_key_file(const char* cert_file, char* key_file)
> {
>      /* Algorithm to locate key file from given cert file  */
>      /* for now just assume the key to present in the      */
>      /* same folder as cert file. The crt in the name will */
>      /* will be replaced with key */
>      char * folder;
>      int32_t i = strlen(cert_file);  /**< Index into key filename,
> initialized
>                                           to filename length */
> 
>      strcpy(key_file, cert_file);
>      key_file[i] = 0;
> 
>      key_file[i-5] = 'y';
>      key_file[i-6] = 'e';
>      key_file[i-7] = 'k';
> 
>      /* Search for folder name "certs" in the file and replace it with
> "keys" */
>      /* Keys are found in "keys" folder and certs are in "certs" folder
>      */
> 
>      folder = strstr(key_file, "crts");
>      if(folder)
>      {
>          folder[0] = 'k';
>          folder[1] = 'e';
>          folder[2] = 'y';
>          folder[3] = 's';
>      }
>      return CAL_SUCCESS;
> }
> 
> Ignoring the inconsistencies in the comments, obviously there are a lot
> of implicit assumptions on file names and paths. First, the assumption
> that the filename of they key corresponding to the certificate can be
> obtained by replacing [-7:-5] by "key". Second, and much more egregious,
> is the use of strstr() on key_file searching for "crts", and just
> blindly replacing the first such with "keys", and ignoring it if not
> found. So if that string appears anywhere in the path (say, my homedir
> is /home/dcrts/ and I have the key material somewhere below that) this
> will replace the wrong occurrence (and look in /home/dkeys/ ....).
> 
> And of course it was unthinkable that this could have been written using
> the much shorter memcpy(..., "keys", 4) so that one could actually `git
> grep 'keys'` and figure out what was going on.

Exactly. I had the pleasure to read cst code a bit as well to figure out 
some issue. This is also a reason I suggested to just set the base path 
to the CST/HAB files instead of setting the individual paths to the 
keys/certs in the hope that this is a more robust way for cst to find 
its implicitly required files.

regards,
Claudius

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch at denx.de