On 7/18/26 12:56, Fabio Estevam wrote:
From: Wojciech Dubowik <[email protected]>

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.

Reviewed-by: Simon Glass <[email protected]>
Acked-by: Quentin Schulz <[email protected]>
Suggested-by: Tom Rini <[email protected]>
Cc: Franz Schnyder <[email protected]>
Signed-off-by: Wojciech Dubowik <[email protected]>
Signed-off-by: Fabio Estevam <[email protected]>
---
Changes since v7:
- Also handle gnutls_global_deinit() and gnutls_pkcs11_deinit() - Ilias.

  tools/Makefile       |   5 ++
  tools/mkeficapsule.c | 111 ++++++++++++++++++++++++++++++++-----------
  2 files changed, 89 insertions(+), 27 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index 1a5f425ecda..e85f5a354b8 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -271,6 +271,11 @@ mkeficapsule-objs := generated/lib/uuid.o \
        $(LIBFDT_OBJS) \
        mkeficapsule.o
  hostprogs-always-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
+GNUTLS_SUPPORTS_P11KIT = $(shell pkg-config --libs gnutls 
--print-requires-private \

Shouldn't we throw a build warning indicating that the created mkeficapsule will be incomplete due to missing PKCS11 support?

+                        2> /dev/null | grep p11-kit-1)
+ifeq ($(GNUTLS_SUPPORTS_P11KIT),p11-kit-1)
+HOSTCFLAGS_mkeficapsule.o += -DMKEFICAPSULE_PKCS11
+endif
include tools/fwumdata_src/fwumdata.mk diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index ec640c57e8a..df64e1b7497 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -207,6 +207,81 @@ static int write_capsule_file(FILE *f, void *data, size_t 
size, const char *msg)
        return 0;
  }
+#ifdef MKEFICAPSULE_PKCS11
+static int pkcs11_init(void)
+{
+       const char *lib;
+       int ret;
+
+       lib = getenv("PKCS11_MODULE_PATH");

Nowhere in our documentation we describe this environment variable. How should a user know what value is expected?

I guess doc/mkeficapsule.1 and doc/develop/uefi/uefi.rst need an update.

+       if (!lib) {
+               fprintf(stdout,
+                       "PKCS11_MODULE_PATH not set in the environment\n");
+               return -1;
+       }
+
+       gnutls_pkcs11_init(GNUTLS_PKCS11_FLAG_MANUAL, NULL);
+       gnutls_global_init();
+
+       ret = gnutls_pkcs11_add_provider(lib, "trusted");
+       if (ret < 0) {
+               fprintf(stdout, "Failed to add pkcs11 provider\n");
+               return -1;
+       }
+
+       return 0;
+}
+
+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 -1;
+
+       gnutls_x509_crt_import_pkcs11(*x509, obj_list[0]);
+
+       return 0;
+}
+
+static int import_pkcs11_key(gnutls_privkey_t *pkey, struct auth_context *ctx)
+{
+       return gnutls_privkey_import_pkcs11_url(*pkey, ctx->key_file);
+}
+
+static void pkcs11_deinit(void)
+{
+       gnutls_global_deinit();
+       gnutls_pkcs11_deinit();
+}
+#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;
+}
+
+static void pkcs11_deinit(void)
+{
+}
+#endif
+
  /**
   * create_auth_data - compose authentication data in capsule
   * @auth_context:     Pointer to authentication context
@@ -229,9 +304,6 @@ static int create_auth_data(struct auth_context *ctx)
        gnutls_pkcs7_t pkcs7;
        gnutls_datum_t data;
        gnutls_datum_t signature;
-       gnutls_pkcs11_obj_t *obj_list;
-       unsigned int obj_list_size = 0;
-       const char *lib;
        int ret;
        bool pkcs11_cert = false;
        bool pkcs11_key = false;
@@ -243,19 +315,8 @@ static int create_auth_data(struct auth_context *ctx)
                pkcs11_key = true;
if (pkcs11_cert || pkcs11_key) {
-               lib = getenv("PKCS11_MODULE_PATH");
-               if (!lib) {
-                       fprintf(stdout,
-                               "PKCS11_MODULE_PATH not set in the 
environment\n");
-                       return -1;
-               }
-
-               gnutls_pkcs11_init(GNUTLS_PKCS11_FLAG_MANUAL, NULL);
-               gnutls_global_init();
-
-               ret = gnutls_pkcs11_add_provider(lib, "trusted");
+               ret = pkcs11_init();
                if (ret < 0) {
-                       fprintf(stdout, "Failed to add pkcs11 provider\n");
                        return -1;
                }
        }
@@ -301,14 +362,12 @@ static int create_auth_data(struct auth_context *ctx)
/* load x509 certificate */
        if (pkcs11_cert) {
-               ret = gnutls_pkcs11_obj_list_import_url4(&obj_list, 
&obj_list_size,
-                                                        ctx->cert_file, 0);
-               if (ret < 0 || obj_list_size == 0) {
-                       fprintf(stdout, "Failed to import crt_file URI 
objects\n");
+               ret =  import_pkcs11_crt(&x509, ctx);
+               if (ret < 0) {
+                       fprintf(stderr, "error in import_pkcs11_crt(): %s\n",
+                               gnutls_strerror(ret));

So here you are printing in sequence:

"Pkcs11 support is disabled\n"
"error in import_pkcs11_crt(): %s}\n"

This message would better be thrown in the functional version of import_pkcs11_crt(). For a user of mkeficapsule the function name import_pkcs11_crt() is irrelevant. Instead, please, describe that the import of a PKCS11 certificate failed.

                        return -1;
                }
-
-               gnutls_x509_crt_import_pkcs11(x509, obj_list[0]);
        } else {
                ret = gnutls_x509_crt_import(x509, &cert, GNUTLS_X509_FMT_PEM);
                if (ret < 0) {
@@ -320,9 +379,9 @@ static int create_auth_data(struct auth_context *ctx)
/* load a private key */
        if (pkcs11_key) {
-               ret = gnutls_privkey_import_pkcs11_url(pkey, ctx->key_file);
+               ret = import_pkcs11_key(&pkey, ctx);
                if (ret < 0) {
-                       fprintf(stderr, "error in %d: %s\n", __LINE__,
+                       fprintf(stderr, "error in import_pkcs11_key(): %s\n",
                                gnutls_strerror(ret));

Same here.

Best regards

Heinrich

                        return -1;
                }
@@ -403,10 +462,8 @@ static int create_auth_data(struct auth_context *ctx)
         *   gnutls_free(signature.data);
         */
- if (pkcs11_cert || pkcs11_key) {
-               gnutls_global_deinit();
-               gnutls_pkcs11_deinit();
-       }
+       if (pkcs11_cert || pkcs11_key)
+               pkcs11_deinit();
return 0;
  }

Reply via email to