Hi Wojciech,

On 2026-05-28T08:03:54, Wojciech Dubowik <[email protected]> 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 <[email protected]>
> Cc: Franz Schnyder <[email protected]>
> Signed-off-by: Wojciech Dubowik <[email protected]>
> Acked-by: Quentin Schulz <[email protected]>
>
> 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.

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

> @@ -243,19 +305,8 @@ static int create_auth_data(struct auth_context *ctx)
>       if (pkcs11_cert || pkcs11_key) {
> -             lib = getenv('PKCS11_MODULE_PATH');
...
> +             ret = pkcs11_init();
>               if (ret < 0) {
> -                     fprintf(stdout, "Failed to add pkcs11 provider\n");
>                       return -1;
>               }
>       }

Single-statement block, drop the braces. Or fold to 'if (pkcs11_init()
< 0) return -1;' since ret is not used afterwards.

> @@ -301,14 +352,12 @@ static int create_auth_data(struct auth_context *ctx)
>       /* load x509 certificate */
>       if (pkcs11_cert) {
> +             ret =  import_pkcs11_crt(&x509, ctx);

Double space after '='.

Regards,
Simon
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#238104): 
https://lists.openembedded.org/g/openembedded-core/message/238104
Mute This Topic: https://lists.openembedded.org/mt/119526819/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to