[PATCH v2 3/4] tools: binman: fit: add support for OpenSSL engines

[Wolfgang Wallner]

Hi Quentin,

From: Quentin Schulz <quentin.schulz at cherry.de>
>Hi Wolfgang,
>
>[...]
>
>>
>> This description is quite convoluted. I would propose to describe it as a list,
>
>I'm very good at writing convoluted paragraphs, so thanks for suggesting
>something more readable :)
>
>> something like the following:
>>
>> When `fit,engine` is set to `pkcs11`, the following cases are distinguished
>> regarding the value of `fit,engine-keydir`:
>>
>>   - If `fit,engine-keydir` is not present, value of `key-name-hint` is prefixed
>>     with `pkcs11:object=`, and then passed as-is to the OpenSSL engine API.
>>
>>     PKCS#11 id: `pkcs11:object=<key-name-hint>`
>
>I have no experience with PKCS#11, but shouldn't that rather be a
>"PKCS#11 URI" instead if " PKCS#11 id"?

Yes, of course, my formulation was sloppy here. Please use the term URI.

> c.f.
>https://urldefense.com/v3/__https://www.rfc-editor.org/rfc/rfc7512__;!!NLW3fF9v!Kt-msudWj52ujfJPS_zBSV_Jf8pvI1_WgEK0f17edIPw_o_DMpGzg7pNOQtcBIFfoTYDZoLczC-z4B0OcGBBqqPWDZ5RHT8LGN3kRdkY$  Especially since there is
>actually a possible id field, see
>https://urldefense.com/v3/__https://www.rfc-editor.org/rfc/rfc7512*section-2.3__;Iw!!NLW3fF9v!Kt-msudWj52ujfJPS_zBSV_Jf8pvI1_WgEK0f17edIPw_o_DMpGzg7pNOQtcBIFfoTYDZoLczC-z4B0OcGBBqqPWDZ5RHT8LGPYmW1op$  pk11-id.
>
>>
>>   - If `fit,engine-keydir` is present, and its value contains either `object=` or
>>     `id=`, then the value of `fit,engine-keydir` is passed as-is to theOpenSSL
>>     engine API. The value of `fit,engine-keydir` has to start with `pkcs11:`.
>>
>>     PKCS#11 id: `<fit,engine-keydir>`
>>
>>   - If `fit,engine-keydir` is present, but its value does not contain either
>>     `object=` or `id=`, then the value of `fit,engine-keydir` is concatenated
>>     with `;object=` and the value of `key-name-hint`, and passed as-is to the
>>     OpenSSL engine API. The value of `fit,engine-keydir` has to start with
>>     `pkcs11:`.
>>
>>     PKCS#11 id: `<fit,engine-keydir>; object=<key-name-hint>`
>>
>
>*Technically*, pkcs11: prefix isn't required as it'll be automatically
>added if it's missing (and you'll be warned about it), c.f.
>https://urldefense.com/v3/__https://elixir.bootlin.com/u-boot/v2025.10/source/lib/rsa/rsa-sign.c*L120__;Iw!!NLW3fF9v!Kt-msudWj52ujfJPS_zBSV_Jf8pvI1_WgEK0f17edIPw_o_DMpGzg7pNOQtcBIFfoTYDZoLczC-z4B0OcGBBqqPWDZ5RHT8LGL2ft2Fl$

Oh, I missed that. Somehow I thought it would warn and then abort.

>But yes, I'll do this rewording in a v3. This is not up for merging
>until 2026.01 is released as it's not a bugfix, so we've time for some
>other feedback/review before I send a v3.
>
>[...]
>
>>
>> Tested-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
>>
>> I did test the 3 cases listed in the description of fit,engine:
>>
>> Test 1: No fit,engine-keydir:
>>
>>      fit,sign;
>>      fit,engine = "pkcs11";
>>      key-name-hint = "<object-id-of-my-key>";
>>
>> Test 2: fit,engine-keydir present, and contains the string 'object=':
>>
>>      fit,sign;
>>      fit,engine = "pkcs11";
>>      fit,engine-keydir = "pkcs11:object=<object-id-of-my-key>";
>>      key-name-hint = "MyTestKey";
>>
>> Test 3: fit,engine-keydir present, but does not contain'object=' of 'id=':
>>
>>      fit,sign;
>>      fit,engine = "pkcs11";
>>      fit,engine-keydir = "pkcs11:serial=<serial-of-my-key>";
>>      key-name-hint = "<object-id-of-my-key>";
>>
>> I think the suggestions above would improve the clarity, but even if you
>> choose to keep the text as is would be fine for me.
>> Both code + documentation:
>> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
>>
>
>Great, thanks!
>
>I was wondering if you had experience with using id= in the pkcs11 URI?

Thanks for pointing that out, I have now additionally tested with 'id=%01%02%03':
    fit,sign;
    fit,engine = "pkcs11";
    fit,engine-keydir = "pkcs11:id=<id-of-my-key>";
    key-name-hint = "MyTestKey";
    
There is one other aspect of the current solution that I'm not sure about:
 * key-name-hint is an attribute per signature
 * fit,engine-keydir is an attribute once per FIT description
 
I don't know wheter this is a use case for anyone, but I think if one would
like to have multiple signature nodes in a FIT description (e.g. multiple
configurations) and would like to have them signed with different keys, then
it would only be possible to do that via the key-name-hint, and thus only
via "object=xxx", but not via "id=xxx".

>It'd be nice if I could add a test with SoftHSMv2 for that as well but I
>wasn't able to do that fiddling around.
>
>See 0707f73a8ba2 ("lib/rsa: allow matching pkcs11 path by object id")
>for the commit that added support for it.

regards, Wolfgang