Increased PKCS#11 decryption performance with p11-kit

Hi,
I had poor decryption performance with my board which is using opTee +
pkcs11 TA backed by RPMB storage when using encrypted .swu images.

After digging a bit deeper I found out that wolfSSL is not keeping the
pkcs11 decryption context as it is using "C_Decrypt" function
internally for each decryption block instead of calling "C_DecryptUpdate" and
after all the blocks are processed "C_DecryptFinal".

I created another PKCS#11 decryption implementation using pure p11-kit
APIs where the context is kept during decryption.
This resulted in reducing flash time from ~50minutes to ~3minutes.

I also added test using softhsm2 for PKCS#11 decryption

If I missed something just let me know

From 14061da90bf9c1e6a5907d3e26b1a2ac99c2f6b9 Mon Sep 17 00:00:00 2001
From: Matej Zachar <zachar.matej@gmail.com>
Date: Thu, 17 Oct 2024 22:27:45 +0200
Subject: [PATCH] Increased PKCS#11 decryption performance with p11-kit

On platforms using "opTee + pkcs11 TA + RPMB" the decryption
speed is increased by order of magnitude - as the C_DecryptInit
context is not lost between decryption updates (C_DecryptUpdate).
On my board (iMX8MP) it went from 50min with wolfSSL to 3min with p11-kit

Signed-off-by: Matej Zachar <zachar.matej@gmail.com>
---
 Kconfig                                       |  28 +-
 Makefile.flags                                |   2 +-
 corelib/Makefile                              |  12 +-
 corelib/swupdate_decrypt_pkcs11_p11kit.c      | 290 ++++++++++++++++++
 ...11.c => swupdate_decrypt_pkcs11_wolfssl.c} |   0
 include/sslapi.h                              |  23 +-
 test/Makefile                                 |  31 +-
 test/data/token/softhsm.conf                  |   2 +
 test/test_crypt_pkcs11.c                      |  99 ++++++
 9 files changed, 475 insertions(+), 12 deletions(-)
 create mode 100644 corelib/swupdate_decrypt_pkcs11_p11kit.c
 rename corelib/{swupdate_decrypt_pkcs11.c =>
swupdate_decrypt_pkcs11_wolfssl.c} (100%)
 create mode 100644 test/data/token/softhsm.conf
 create mode 100644 test/test_crypt_pkcs11.c

+
+int main(void)
+{
+   const struct CMUnitTest crypt_pkcs11_tests[] = {
+         cmocka_unit_test(test_crypt_pkcs11_256)
+   };
+   return cmocka_run_group_tests_name("crypt_pkcs11",
crypt_pkcs11_tests, NULL, NULL);
+}
\ No newline at end of file

Hi Matej,

On 17.10.24 23:08, Matej Zachar wrote:
> Hi,
> I had poor decryption performance with my board which is using opTee +
> pkcs11 TA backed by RPMB storage when using encrypted .swu images.
>
> After digging a bit deeper I found out that wolfSSL is not keeping the
> pkcs11 decryption context as it is using "C_Decrypt" function
> internally for each decryption block instead of calling "C_DecryptUpdate" and
> after all the blocks are processed "C_DecryptFinal".
>
> I created another PKCS#11 decryption implementation using pure p11-kit
> APIs where the context is kept during decryption.
> This resulted in reducing flash time from ~50minutes to ~3minutes.
>
> I also added test using softhsm2 for PKCS#11 decryption
>
> If I missed something just let me know
>

Thanks for this - however, as already reported several times on this
list, current implementation for crypto engines in SWUpdate is quite
hard-coded, and it is difficult to add / remove engines and / or
algorithms. See
https://github.com/sbabic/swupdate/blob/master/doc/source/improvement_proposals.rst#security--crypto-engines

For that, changes regarding crypto are put in stand-by, until a more
efficient subsystem will be created and it will be allowed to add /
remove modules, exactly as in other subsystems in SWUpdate (bootloader,
handlers, etc.). It should be then possible to pick up a module (pkcs#11
with libp11 for example) at run time.

Until then, this will be hold (and then a rebase will be required).

Best regards,
Stefano Babic

>  From 14061da90bf9c1e6a5907d3e26b1a2ac99c2f6b9 Mon Sep 17 00:00:00 2001
> From: Matej Zachar <zachar.matej@gmail.com>
> Date: Thu, 17 Oct 2024 22:27:45 +0200
> Subject: [PATCH] Increased PKCS#11 decryption performance with p11-kit
>
> On platforms using "opTee + pkcs11 TA + RPMB" the decryption
> speed is increased by order of magnitude - as the C_DecryptInit
> context is not lost between decryption updates (C_DecryptUpdate).
> On my board (iMX8MP) it went from 50min with wolfSSL to 3min with p11-kit
>
> Signed-off-by: Matej Zachar <zachar.matej@gmail.com>
> ---
>   Kconfig                                       |  28 +-
>   Makefile.flags                                |   2 +-
>   corelib/Makefile                              |  12 +-
>   corelib/swupdate_decrypt_pkcs11_p11kit.c      | 290 ++++++++++++++++++
>   ...11.c => swupdate_decrypt_pkcs11_wolfssl.c} |   0
>   include/sslapi.h                              |  23 +-
>   test/Makefile                                 |  31 +-
>   test/data/token/softhsm.conf                  |   2 +
>   test/test_crypt_pkcs11.c                      |  99 ++++++
>   9 files changed, 475 insertions(+), 12 deletions(-)
>   create mode 100644 corelib/swupdate_decrypt_pkcs11_p11kit.c
>   rename corelib/{swupdate_decrypt_pkcs11.c =>
> swupdate_decrypt_pkcs11_wolfssl.c} (100%)
>   create mode 100644 test/data/token/softhsm.conf
>   create mode 100644 test/test_crypt_pkcs11.c
>
> diff --git a/Kconfig b/Kconfig
> index 7139617c..1ee70cb9 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -516,11 +516,33 @@ config ENCRYPTED_IMAGES_HARDEN_LOGGING
>   config PKCS11
>      bool "Enable PKCS#11 cryptographic operations"
>      default n
> -   depends on HAVE_WOLFSSL && HAVE_P11KIT && ENCRYPTED_IMAGES
> +   depends on HAVE_P11KIT && ENCRYPTED_IMAGES
>      help
>        Enable using PKCS#11 for AES decryption instead of having the plain
> -     key available in a file. This is implemented with wolfSSL independent
> -     from the SSL implementation and replaces the plain key method.
> +     key available in a file.
> +
> +choice
> +   prompt "PKCS#11 implementation"
> +   depends on PKCS11
> +   default PKCS11_IMPL_WOLFSSL
> +   help
> +     Select PKCS#11 implementation provider
> +
> +   config PKCS11_IMPL_WOLFSSL
> +      bool "wolfSSL"
> +      depends on HAVE_WOLFSSL && HAVE_P11KIT && ENCRYPTED_IMAGES
> +      help
> +        Enable using PKCS#11 for AES decryption instead of having the plain
> +        key available in a file. This is implemented with wolfSSL independent
> +        from the SSL implementation and replaces the plain key method.
> +
> +   config PKCS11_IMPL_P11KIT
> +      bool "P11-KIT"
> +      depends on HAVE_P11KIT && ENCRYPTED_IMAGES
> +      help
> +        Enable using PKCS#11 for AES decryption instead of having the plain
> +        key available in a file. This is implemented with p11-kit api
> +endchoice
>
>   comment "Compressors (zlib always on)"
>
> diff --git a/Makefile.flags b/Makefile.flags
> index 8598f38d..ae0e13ed 100644
> --- a/Makefile.flags
> +++ b/Makefile.flags
> @@ -149,7 +149,7 @@ endif
>   ifeq ($(CONFIG_SSL_IMPL_WOLFSSL),y)
>   KBUILD_CPPFLAGS += -DOPENSSL_ALL
>   LDLIBS += wolfssl
> -else ifeq ($(CONFIG_PKCS11),y)
> +else ifeq ($(CONFIG_PKCS11_IMPL_WOLFSSL),y)
>   LDLIBS += wolfssl
>   endif
>
> diff --git a/corelib/Makefile b/corelib/Makefile
> index 5917e379..ea510fe5 100644
> --- a/corelib/Makefile
> +++ b/corelib/Makefile
> @@ -11,8 +11,10 @@ lib-$(CONFIG_MTD)       += mtd-interface.o
>   lib-$(CONFIG_LUA)     += lua_interface.o lua_compat.o
>   ifeq ($(CONFIG_SSL_IMPL_OPENSSL)$(CONFIG_SSL_IMPL_WOLFSSL),y)
>   lib-$(CONFIG_HASH_VERIFY)  += verify_signature.o
> -ifeq ($(CONFIG_PKCS11),y)
> -lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11.o
> +ifeq ($(CONFIG_PKCS11_IMPL_WOLFSSL),y)
> +lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11_wolfssl.o
> +else ifeq ($(CONFIG_PKCS11_IMPL_P11KIT),y)
> +lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11_p11kit.o
>   else
>   lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_openssl.o
>   endif
> @@ -27,8 +29,10 @@ lib-$(CONFIG_SIGALG_CMS) += swupdate_pkcs7_verify.o
>   endif
>   ifeq ($(CONFIG_SSL_IMPL_MBEDTLS),y)
>   lib-$(CONFIG_HASH_VERIFY)  += verify_signature_mbedtls.o
> -ifeq ($(CONFIG_PKCS11),y)
> -lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11.o
> +ifeq ($(CONFIG_PKCS11_IMPL_WOLFSSL),y)
> +lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11_wolfssl.o
> +else ifeq ($(CONFIG_PKCS11_IMPL_P11KIT),y)
> +lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_pkcs11_p11kit.o
>   else
>   lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_decrypt_mbedtls.o
>   endif
> diff --git a/corelib/swupdate_decrypt_pkcs11_p11kit.c
> b/corelib/swupdate_decrypt_pkcs11_p11kit.c
> new file mode 100644
> index 00000000..49cd929a
> --- /dev/null
> +++ b/corelib/swupdate_decrypt_pkcs11_p11kit.c
> @@ -0,0 +1,290 @@
> +// SPDX-FileCopyrightText: 2024 Matej Zachar
> +//
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Inspired by the wolfssl implementation done by Bastian Germann
> + */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include "swupdate.h"
> +#include "sslapi.h"
> +#include "util.h"
> +
> +static CK_SLOT_ID find_slot(CK_FUNCTION_LIST_PTR module, P11KitUri *uri)
> +{
> +   CK_RV rv;
> +
> +   CK_SLOT_ID slot_id = p11_kit_uri_get_slot_id(uri);
> +   if (slot_id != (CK_SLOT_ID)-1)
> +      return slot_id;
> +
> +   size_t slot_count;
> +   rv = module->C_GetSlotList(1, NULL_PTR, &slot_count);
> +   if (rv != CKR_OK)
> +      return (CK_SLOT_ID)-1;
> +
> +   CK_SLOT_ID slot_ids[slot_count];
> +   rv = module->C_GetSlotList(1, &slot_ids[0], &slot_count);
> +   if (rv != CKR_OK)
> +      return (CK_SLOT_ID)-1;
> +
> +   CK_TOKEN_INFO token_info;
> +   for (int i = 0; i < slot_count; ++i) {
> +      slot_id = slot_ids[i];
> +
> +      rv = module->C_GetTokenInfo(slot_id, &token_info);
> +      if (rv != CKR_OK)
> +         return (CK_SLOT_ID)-1;
> +
> +      if (p11_kit_uri_match_token_info(uri, &token_info))
> +         return slot_id;
> +   }
> +
> +   return (CK_SLOT_ID)-1;
> +}
> +
> +static CK_RV find_key(CK_FUNCTION_LIST_PTR module, CK_SESSION_HANDLE session,
> +   CK_ATTRIBUTE_PTR key_id, CK_OBJECT_HANDLE *key_handle)
> +{
> +   CK_RV rv;
> +
> +   CK_ATTRIBUTE find_template[] = {
> +         { CKA_ID, key_id->pValue, key_id->ulValueLen }
> +   };
> +
> +   rv = module->C_FindObjectsInit(session, find_template, 1);
> +   if (rv != CKR_OK) {
> +      return rv;
> +   }
> +
> +   CK_ULONG object_count;
> +   rv = module->C_FindObjects(session, key_handle, 1, &object_count);
> +   if (rv != CKR_OK) {
> +      return rv;
> +   }
> +
> +   rv = module->C_FindObjectsFinal(session);
> +   if (rv != CKR_OK) {
> +      return rv;
> +   }
> +
> +   if (object_count == 0) {
> +      return CKR_DATA_INVALID;
> +   }
> +
> +   return CKR_OK;
> +}
> +
> +struct swupdate_digest *swupdate_DECRYPT_init(unsigned char *uri,
> +               char __attribute__ ((__unused__)) keylen, unsigned char *iv)
> +{
> +   struct swupdate_digest *dgst;
> +   CK_SLOT_ID slot_id;
> +   CK_ATTRIBUTE_PTR key_id;
> +   const char *pin;
> +   const char *module_path;
> +   const char *msg;
> +   int err = 0;
> +   CK_RV rv;
> +
> +   if (uri == NULL || iv == NULL) {
> +      ERROR("PKCS#11 URI or AES IV missing for decryption!");
> +      return NULL;
> +   }
> +
> +   dgst = calloc(1, sizeof(*dgst));
> +   if (!dgst) {
> +      return NULL;
> +   }
> +
> +   dgst->uri = p11_kit_uri_new();
> +   err = p11_kit_uri_parse((const char*)uri,
> P11_KIT_URI_FOR_OBJECT_ON_TOKEN_AND_MODULE, dgst->uri);
> +   if (err) {
> +      msg = p11_kit_uri_message(err);
> +      ERROR("PKCS#11 URI: %s", msg);
> +      goto free_digest;
> +   }
> +
> +   key_id  = p11_kit_uri_get_attribute(dgst->uri, CKA_ID);
> +   pin     = p11_kit_uri_get_pin_value(dgst->uri);
> +   module_path = p11_kit_uri_get_module_path(dgst->uri);
> +   if (key_id == NULL || pin == NULL || module_path == NULL) {
> +      ERROR("PKCS#11 URI must contain id, pin-value and module-path.");
> +      goto free_digest;
> +   }
> +
> +   dgst->module = p11_kit_module_load(module_path, 0);
> +   if (dgst->module == NULL) {
> +      msg = p11_kit_message();
> +      ERROR("Failed to load PKCS#11 module [%s]: %s\n", module_path, msg);
> +      goto free_digest;
> +   }
> +
> +   rv = dgst->module->C_Initialize(NULL_PTR);
> +   if (rv != CKR_OK)
> +      goto err_msg;
> +
> +   slot_id = find_slot(dgst->module, dgst->uri);
> +   if (slot_id == -1) {
> +      ERROR("PKCS#11 URI must contain slot-id or token identification
> such as token, model, serial, manufacturer.");
> +      goto free_digest;
> +   }
> +
> +   rv = dgst->module->C_OpenSession(slot_id, CKF_SERIAL_SESSION |
> CKF_RW_SESSION, NULL_PTR, NULL_PTR, &dgst->session);
> +   if (rv != CKR_OK)
> +      goto err_msg;
> +
> +   rv = dgst->module->C_Login(dgst->session, CKU_USER, (unsigned char
> *)pin, strnlen(pin, 32));
> +   if (rv != CKR_OK)
> +      goto err_msg;
> +
> +   CK_OBJECT_HANDLE key;
> +   rv = find_key(dgst->module, dgst->session, key_id, &key);
> +   if (rv != CKR_OK)
> +      goto err_msg;
> +
> +   // Setup a valid PKCS#7 block plus one state octet
> +   for (int i = 0; i <= AES_BLK_SIZE; ++i) {
> +      dgst->last[i] = AES_BLK_SIZE;
> +   }
> +
> +   // Setup IV vector & mechanism
> +   memcpy(dgst->iv, iv, AES_BLOCK_SIZE);
> +   dgst->mechanism.mechanism = CKM_AES_CBC;
> +   dgst->mechanism.pParameter = dgst->iv;
> +   dgst->mechanism.ulParameterLen = AES_BLOCK_SIZE;
> +
> +   rv = dgst->module->C_DecryptInit(dgst->session, &dgst->mechanism, key);
> +   if (rv != CKR_OK)
> +      goto err_msg;
> +
> +   INFO("PKCS#11 key set up successfully.");
> +   return dgst;
> +
> +err_msg:
> +   msg = p11_kit_strerror(rv);
> +   ERROR("PKCS#11 initialization failed: %s", msg);
> +
> +free_digest:
> +   if (dgst->uri)
> +      p11_kit_uri_free(dgst->uri);
> +
> +   if (dgst->session)
> +      dgst->module->C_CloseSession(dgst->session);
> +
> +   if (dgst->module) {
> +      dgst->module->C_Finalize(NULL_PTR);
> +      p11_kit_module_release(dgst->module);
> +   }
> +
> +   free(dgst);
> +
> +   return NULL;
> +}
> +
> +int swupdate_DECRYPT_update(struct swupdate_digest *dgst, unsigned char *buf,
> +            int *outlen, const unsigned char *cryptbuf, int inlen)
> +{
> +   // precondition: len(buf) >= inlen + AES_BLK_SIZE
> +   unsigned long buf_len = inlen + AES_BLOCK_SIZE;
> +   CK_RV rv;
> +
> +   if (inlen < AES_BLK_SIZE)
> +      return -EFAULT;
> +
> +   if (dgst->last[AES_BLOCK_SIZE]) {
> +      dgst->last[AES_BLOCK_SIZE] = 0;
> +      // first run - there is no block to append
> +      *outlen = 0;
> +   } else {
> +      // append previously decrypted last AES block
> +      memcpy(buf, dgst->last, AES_BLOCK_SIZE);
> +      buf += AES_BLOCK_SIZE;
> +      *outlen = AES_BLOCK_SIZE;
> +   }
> +
> +   rv = dgst->module->C_DecryptUpdate(dgst->session, (unsigned
> char*)cryptbuf, inlen, buf, &buf_len);
> +   if (rv != CKR_OK) {
> +      ERROR("PKCS#11 AES decryption failed: %s", p11_kit_strerror(rv));
> +      return -EFAULT;
> +   }
> +
> +   // strip and remember last AES block from decoded buffer
> +   // it will get appended either in the next call to DECRYPT_update
> or DECRYPT_final
> +   buf_len -= AES_BLOCK_SIZE;
> +   memcpy(dgst->last, &buf[buf_len], AES_BLOCK_SIZE);
> +
> +   // update iv for the next block
> +   memcpy(dgst->iv, cryptbuf + inlen - AES_BLOCK_SIZE, AES_BLOCK_SIZE);
> +
> +   *outlen += (int)buf_len;
> +   return 0;
> +}
> +
> +int swupdate_DECRYPT_final(struct swupdate_digest *dgst, unsigned
> char *buf, int *outlen)
> +{
> +   CK_RV rv;
> +   unsigned long extra_len = 0;
> +
> +   if (dgst->last[AES_BLOCK_SIZE]) {
> +#ifndef CONFIG_ENCRYPTED_IMAGES_HARDEN_LOGGING
> +      ERROR("AES: At least one call to swupdate_DECRYPT_update was expected");
> +#endif
> +      return -EINVAL;
> +   }
> +
> +   // append previously decrypted last AES block if any
> +   memcpy(buf, dgst->last, AES_BLOCK_SIZE);
> +
> +   rv = dgst->module->C_DecryptFinal(dgst->session,
> &buf[AES_BLOCK_SIZE], &extra_len);
> +   if (rv != CKR_OK)
> +      return -EFAULT;
> +
> +   // obtain last AES block after C_DecryptFinal
> +   CK_BYTE_PTR last = &buf[extra_len];
> +
> +   // Handle manual PKCS#7 padding removal
> +   CK_BYTE padding_value = last[AES_BLOCK_SIZE - 1];
> +
> +   if (padding_value <= 0 || padding_value > AES_BLOCK_SIZE) {
> +#ifndef CONFIG_ENCRYPTED_IMAGES_HARDEN_LOGGING
> +      ERROR("AES: Invalid PKCS#7 padding value [%u]", padding_value);
> +#endif
> +      return -EFAULT;
> +   }
> +
> +   // Verify that padding is correct
> +   for (CK_BYTE i = 0; i < padding_value; ++i) {
> +      if (last[AES_BLOCK_SIZE - 1 - i] != padding_value) {
> +#ifndef CONFIG_ENCRYPTED_IMAGES_HARDEN_LOGGING
> +         ERROR("AES: Invalid PKCS#7 padding value [%u] at offset %u",
> padding_value, i);
> +#endif
> +         return -EINVAL;
> +      }
> +   }
> +
> +   *outlen = (int)extra_len + AES_BLOCK_SIZE - padding_value;
> +   return 0;
> +}
> +
> +void swupdate_DECRYPT_cleanup(struct swupdate_digest *dgst)
> +{
> +   if (dgst) {
> +      if (dgst->uri)
> +         p11_kit_uri_free(dgst->uri);
> +
> +      if (dgst->session)
> +         dgst->module->C_CloseSession(dgst->session);
> +
> +      if (dgst->module) {
> +         dgst->module->C_Finalize(NULL_PTR);
> +         p11_kit_module_release(dgst->module);
> +      }
> +
> +      free(dgst);
> +      dgst = NULL;
> +   }
> +}
> diff --git a/corelib/swupdate_decrypt_pkcs11.c
> b/corelib/swupdate_decrypt_pkcs11_wolfssl.c
> similarity index 100%
> rename from corelib/swupdate_decrypt_pkcs11.c
> rename to corelib/swupdate_decrypt_pkcs11_wolfssl.c
> diff --git a/include/sslapi.h b/include/sslapi.h
> index 64640184..184ee0db 100644
> --- a/include/sslapi.h
> +++ b/include/sslapi.h
> @@ -19,6 +19,7 @@
>   #if defined(CONFIG_HASH_VERIFY) || defined(CONFIG_ENCRYPTED_IMAGES)
>
>   #ifdef CONFIG_PKCS11
> +#ifdef CONFIG_PKCS11_IMPL_WOLFSSL
>   #include <wolfssl/options.h>
>   #include <wolfssl/ssl.h>
>   #include <wolfssl/wolfcrypt/aes.h>
> @@ -26,6 +27,10 @@
>   // Exclude p11-kit's pkcs11.h to prevent conflicting with wolfssl's
>   #define PKCS11_H 1
>   #include <p11-kit/uri.h>
> +#elif defined(CONFIG_PKCS11_IMPL_P11KIT)
> +#include <p11-kit/uri.h>
> +#include <p11-kit/p11-kit.h>
> +#endif
>   #endif
>
>   #ifdef CONFIG_SSL_IMPL_OPENSSL
> @@ -99,12 +104,19 @@ struct swupdate_digest {
>      EVP_PKEY_CTX *ckey;    /* this is used for RSA key */
>      X509_STORE *certs; /* this is used if CMS is set */
>      EVP_MD_CTX *ctx;
> -#ifdef CONFIG_PKCS11
> +#ifdef CONFIG_PKCS11_IMPL_WOLFSSL
>      unsigned char last_decr[AES_BLOCK_SIZE + 1];
>      P11KitUri *p11uri;
>      Aes ctxdec;
>      Pkcs11Dev pkdev;
>      Pkcs11Token pktoken;
> +#elif defined(CONFIG_PKCS11_IMPL_P11KIT)
> +   P11KitUri *uri;
> +   CK_FUNCTION_LIST_PTR module;
> +   CK_SESSION_HANDLE session;
> +   CK_MECHANISM mechanism;
> +   CK_BYTE iv[AES_BLOCK_SIZE];
> +   CK_BYTE last[AES_BLOCK_SIZE + 1];
>   #elif OPENSSL_VERSION_NUMBER < 0x10100000L
>      EVP_CIPHER_CTX ctxdec;
>   #else
> @@ -155,12 +167,19 @@ struct swupdate_digest {
>   #ifdef CONFIG_SIGNED_IMAGES
>      mbedtls_pk_context mbedtls_pk_context;
>   #endif /* CONFIG_SIGNED_IMAGES */
> -#ifdef CONFIG_PKCS11
> +#ifdef CONFIG_PKCS11_IMPL_WOLFSSL
>      unsigned char last_decr[AES_BLOCK_SIZE + 1];
>      P11KitUri *p11uri;
>      Aes ctxdec;
>      Pkcs11Dev pkdev;
>      Pkcs11Token pktoken;
> +#elif defined(CONFIG_PKCS11_IMPL_P11KIT)
> +   P11KitUri *uri;
> +   CK_FUNCTION_LIST_PTR module;
> +   CK_SESSION_HANDLE session;
> +   CK_MECHANISM mechanism;
> +   CK_BYTE iv[AES_BLOCK_SIZE];
> +   CK_BYTE last[AES_BLOCK_SIZE + 1];
>   #elif defined(CONFIG_ENCRYPTED_IMAGES)
>      mbedtls_cipher_context_t mbedtls_cipher_context;
>   #endif /* CONFIG_PKCS11 */
> diff --git a/test/Makefile b/test/Makefile
> index ad26b40d..af7e3e39 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -17,7 +17,9 @@
>   ## along with this program; if not, write to the Free Software
>   ## Foundation, Inc.
>
> -ifneq ($(CONFIG_PKCS11),y)
> +ifeq ($(CONFIG_PKCS11),y)
> +tests-$(CONFIG_ENCRYPTED_IMAGES) += test_crypt_pkcs11
> +else
>   tests-$(CONFIG_ENCRYPTED_IMAGES) += test_crypt
>   endif
>   tests-$(CONFIG_HASH_VERIFY) += test_hash
> @@ -56,7 +58,7 @@ quiet_cmd_linktestexe = LD      $(basename $@)
>                     "$(SWLIBS)" \
>                     "$(LDLIBS) cmocka"
>
> -EXECUTE_TEST = echo "RUN $(subst $(obj)/,,$(var))";
> LD_LIBRARY_PATH=$(objtree) CMOCKA_MESSAGE_OUTPUT=TAP $(var)
> +EXECUTE_TEST = echo "RUN $(subst $(obj)/,,$(var))";
> LD_LIBRARY_PATH=$(objtree) CMOCKA_MESSAGE_OUTPUT=TAP
> SOFTHSM2_CONF=$(DATADIR)/token/softhsm.conf $(var)
>
>   PHONY += default
>   default:
> @@ -97,4 +99,29 @@ $(DATADIR)/signing-secret.pem:
>      $(if $(Q),@echo "  GEN     $@")
>      $(Q)openssl genrsa -out $@ 2>/dev/null
>
> +ifeq ($(CONFIG_PKCS11),y)
> +$(obj)/test_crypt_pkcs11.o: $(DATADIR)/softshm
> +
> +TOKEN_AES_KEY :=
> dd020ce5ebd5c468556288d6a75169c88a5b335d9f569e30751c50401467d230
> +TOKEN_AES_IV := c1f390d21dd06118cbd333144a3318ca
> +
> +.INTERMEDIATE: $(DATADIR)/softshm
> +$(DATADIR)/softshm: export SOFTHSM2_CONF=$(DATADIR)/token/softhsm.conf
> +$(DATADIR)/softshm: $(DATADIR)/token/softhsm.conf
> +   $(if $(Q),@echo "  INIT     $@")
> +   $(Q)rm -rf $(DATADIR)/token/*/
> +
> +   $(if $(Q),@echo "  GEN      $(DATADIR)/token/original.data")
> +   $(Q)openssl rand 131075 > $(DATADIR)/token/original.data
> +
> +   $(if $(Q),@echo "  ENCRYPT  $(DATADIR)/token/original.data")
> +   $(Q)openssl enc -aes-256-cbc -in $(DATADIR)/token/original.data
> -out $(DATADIR)/token/encrypted.data -K $(TOKEN_AES_KEY) -iv
> $(TOKEN_AES_IV)
> +   $(Q)echo -n "$(TOKEN_AES_IV)" | xxd -p -r >
> $(DATADIR)/token/encrypted.data.iv
> +
> +   $(if $(Q),@echo "  IMPORT   $(DATADIR)/token/aes.key")
> +   $(Q)echo -n "$(TOKEN_AES_KEY)" | xxd -p -r > $(DATADIR)/token/aes.key
> +   $(Q)softhsm2-util --init-token --slot 0 --label "TestToken"
> --so-pin 123456 --pin 1234
> +   $(Q)softhsm2-util --import $(DATADIR)/token/aes.key --aes --token
> "TestToken" --label "AES key" --id A1B2 --pin 1234
> +endif
> +
>   .PHONY: $(PHONY)
> diff --git a/test/data/token/softhsm.conf b/test/data/token/softhsm.conf
> new file mode 100644
> index 00000000..bd43fe45
> --- /dev/null
> +++ b/test/data/token/softhsm.conf
> @@ -0,0 +1,2 @@
> +directories.tokendir = test/data/token
> +objectstore.backend = file
> \ No newline at end of file
> diff --git a/test/test_crypt_pkcs11.c b/test/test_crypt_pkcs11.c
> new file mode 100644
> index 00000000..6ed4b84d
> --- /dev/null
> +++ b/test/test_crypt_pkcs11.c
> @@ -0,0 +1,99 @@
> +// SPDX-FileCopyrightText: 2024 Matej Zachar
> +//
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <stddef.h>
> +#include <setjmp.h>
> +#include <cmocka.h>
> +#include <util.h>
> +#include <sslapi.h>
> +
> +#define BUFFER_SIZE (AES_BLOCK_SIZE * 1024)
> +#define TOKENDIR "test/data/token"
> +
> +static int read_file(const char *path, unsigned char *buffer, size_t *size)
> +{
> +   FILE *fp = fopen(path, "r");
> +   if (!fp) {
> +      fprintf(stderr, "Failed to open file '%s'\n", path);
> +      return -1;
> +   }
> +
> +   size_t len = fread(buffer, sizeof(char), *size, fp);
> +   if (ferror(fp) != 0) {
> +      fprintf(stderr, "Error reading file '%s'\n", path);
> +        fclose(fp);
> +      return -1;
> +   }
> +
> +   *size = len;
> +    fclose(fp);
> +
> +    return 0;
> +}
> +
> +static void test_crypt_pkcs11_256(void **state)
> +{
> +   (void) state;
> +   int err;
> +
> +   const char * uri =
> "pkcs11:token=TestToken;id=%A1%B2?pin-value=1234&module-path=/usr/lib/x86_64-linux-gnu/softhsm/libsofthsm2.so";
> +
> +   size_t original_data_len = 128 * 1024;/* 128KiB */
> +   unsigned char original_data[original_data_len];
> +   err = read_file(TOKENDIR "/original.data", &original_data[0],
> &original_data_len);
> +   assert_true(err == 0);
> +
> +    size_t encrypted_data_len = 128 * 1024 + AES_BLOCK_SIZE;/* 128KiB
> + AES_BLOCK_SIZE(16B) */
> +    unsigned char encrypted_data[encrypted_data_len];
> +    err = read_file(TOKENDIR "/encrypted.data", &encrypted_data[0],
> &encrypted_data_len);
> +   assert_true(err == 0);
> +
> +    unsigned char decrypted_data[encrypted_data_len];
> +
> +   size_t iv_size = 16;
> +   unsigned char iv[iv_size];
> +   err = read_file(TOKENDIR "/encrypted.data.iv", &iv[0], &iv_size);
> +   assert_true(err == 0);
> +
> +   unsigned char buffer[BUFFER_SIZE + AES_BLOCK_SIZE];
> +
> +   struct swupdate_digest *dgst = swupdate_DECRYPT_init((unsigned
> char *)uri, 0, &iv[0]);
> +   assert_non_null(dgst);
> +
> +   int len;
> +   size_t e_offset = 0;
> +   size_t d_offset = 0;
> +    while (e_offset < encrypted_data_len) {
> +      size_t chunk_size = (encrypted_data_len - e_offset >
> BUFFER_SIZE) ? BUFFER_SIZE : encrypted_data_len - e_offset;
> +
> +       err = swupdate_DECRYPT_update(dgst, buffer, &len,
> encrypted_data + e_offset, chunk_size);
> +       assert_true(err == 0);
> +       assert_true(len >= AES_BLOCK_SIZE && len <= chunk_size);
> +       e_offset += chunk_size;
> +
> +       memcpy(&decrypted_data[d_offset], buffer, len);
> +        d_offset += len;
> +    }
> +
> +   err = swupdate_DECRYPT_final(dgst, buffer, &len);
> +   assert_true(err == 0);
> +   assert_true(len == 3); /* as the size is 128*1024+3 */
> +
> +   memcpy(&decrypted_data[d_offset], buffer, len);
> +   d_offset += len;
> +
> +   assert_true(strncmp((const char *)decrypted_data, (const char
> *)original_data, original_data_len) == 0);
> +}
> +
> +
> +int main(void)
> +{
> +   const struct CMUnitTest crypt_pkcs11_tests[] = {
> +         cmocka_unit_test(test_crypt_pkcs11_256)
> +   };
> +   return cmocka_run_group_tests_name("crypt_pkcs11",
> crypt_pkcs11_tests, NULL, NULL);
> +}
> \ No newline at end of file

Hi Stefano,

On Tue, Oct 29, 2024 at 12:38 PM Stefano Babic
<stefano.babic@swupdate.org> wrote:
>
> Until then, this will be hold (and then a rebase will be required).
>
I will be happy to rebase the work once the new subsystem is ready.

Best regards,
Matej Zachar

Hi,

This approach will likely work only on the i.MX8 platform, as its PKCS11 TA 
has a larger reserved size, allowing it to handle a large buffer in 
swupdate_DECRYPT_update/swupdate_ENCRYPT_update. However, this could 
potentially crash on platforms with significantly smaller stack sizes, such 
as the STM32MP1.

It seems this behavior should depend on the hardware, and WolfSSL uses a 
much smaller buffer, which likely explains why it takes considerably more 
time.

Best regards,
On Saturday, November 2, 2024 at 11:39:34 AM UTC+1 Matej Zachar wrote:

> Hi Stefano,
>
> On Tue, Oct 29, 2024 at 12:38 PM Stefano Babic
> <stefan...@swupdate.org> wrote:
> >
> > Until then, this will be hold (and then a rebase will be required).
> >
> I will be happy to rebase the work once the new subsystem is ready.
>
> Best regards,
> Matej Zachar
>