[PATCH v5] tools: mkeficapsule: Rework pkcs11 support

[Quentin Schulz]

Hi Simon,

There's a v6 already, please post your reviews there again if applicable.

On 6/3/26 6:43 PM, Simon Glass wrote:
> Hi Wojciech,
> 
> On 2026-05-28T08:03:54, Wojciech Dubowik <Wojciech.Dubowik at mt.com> wrote:
>> tools: mkeficapsule: Rework pkcs11 support
>>
>> Some distros like OpenEmbedded are using gnutls library
>> without pkcs11 support and linking of mkeficapsule will fail.
>> It would make maintenance of default configs a hurdle.
>> Add detection of pkcs11 support in gnutls so it's enabled
>> when available and doesn't need to be set explicitly.
>>
>> Suggested-by: Tom Rini <trini at konsulko.com>
>> Cc: Franz Schnyder <fra.schnyder at gmail.com>
>> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik at mt.com>
>> Acked-by: Quentin Schulz <quentin.schulz at cherry.de>
>>
>> tools/Makefile       |  5 +++
>>   tools/mkeficapsule.c | 95 +++++++++++++++++++++++++++++++++++++++-------------
>>   2 files changed, 77 insertions(+), 23 deletions(-)
> 
>> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
>> @@ -207,6 +207,71 @@ static int write_capsule_file(FILE *f, void *data, size_t size, const char *msg)
>> +static int import_pkcs11_crt(gnutls_x509_crt_t *x509, struct auth_context *ctx)
>> +{
>> +     gnutls_pkcs11_obj_t *obj_list;
>> +     unsigned int obj_list_size = 0;
>> +     int ret;
>> +
>> +     ret = gnutls_pkcs11_obj_list_import_url4(&obj_list, &obj_list_size,
>> +                                              ctx->cert_file, 0);
>> +     if (ret < 0 || obj_list_size == 0)
>> +             return ret;
> 
> !obj_list_size
> 
> Behaviour change: when obj_list_size == 0 but ret >= 0, the caller's
> 'if (ret < 0)' check passes and execution continues with an empty
> obj_list. Please return -1 explicitly in that case.
> 

Yes, good catch, we can have ret == 0 here when obj_list_size==0 and 
thus return 0.

>> +static int import_pkcs11_crt(gnutls_x509_crt_t *x509, struct auth_context *ctx)
>> +{
> 
> ...
>> +     ret = gnutls_x509_crt_import_pkcs11(*x509, obj_list[0]);
> ...
>> +static int import_pkcs11_key(gnutls_privkey_t *pkey, struct auth_context *ctx)
>> +{
>> +     return gnutls_privkey_import_pkcs11_url(*pkey, ctx->key_file);
>> +}
> 
> gnutls_x509_crt_t and gnutls_privkey_t are already pointer typedefs
> and neither function reassigns the handle, so the extra indirection
> buys nothing. Please pass the handles by value and drop the '&' at the
> call sites.
> 
>> +#else
>> +static int pkcs11_init(void)
>> +{
>> +     fprintf(stderr, "Pkcs11 support is disabled\n");
>> +     return -1;
>> +}
>> +
>> +static int import_pkcs11_crt(gnutls_x509_crt_t *x509, struct auth_context *ctx)
>> +{
>> +     fprintf(stderr, "Pkcs11 support is disabled\n");
>> +     return -1;
>> +}
>> +
>> +static int import_pkcs11_key(gnutls_privkey_t *pkey, struct auth_context *ctx)
>> +{
>> +     fprintf(stderr, "Pkcs11 support is disabled\n");
>> +     return -1;
>> +}
>> +#endif
> 
> "PKCS#11 support..."
> 
> import_pkcs11_crt() and import_pkcs11_key() are only reached after
> pkcs11_init() has already failed with the same message, so these two
> stubs look like dead code.
> 

How do you compile the code if you don't have those stubs?

Cheers,
Quentin