Fix hybrid LMS/XMSS keystore import and PQ secondary builds

.github/workflows/test-sunnyday-simulator.yml

@@ -867,3 +867,52 @@ jobs:
       - name: Run sunny day hybrid boot test
         run: |
           ./wolfboot.elf get_version
+
+      # 64 Bit simulator, Hybrid auth LMS + ECDSA, both orderings, and
+      # XMSS + ECDSA. These need WOLFBOOT_UNIVERSAL_KEYSTORE, which is what
+      # makes the keystore slot wider than a stateful hash-based public key.
+      #
+      - name: make clean
+        run: |
+          make keysclean
+
+      - name: Select config (64 bit simulator) Hybrid LMS + ECC
+        run: |
+          cp config/examples/sim-lms.config .config
+
+      - name: Build wolfboot.elf
+        run: |
+          make clean && make SIGN_SECONDARY=ECC256 WOLFBOOT_UNIVERSAL_KEYSTORE=1
+
+      - name: Run sunny day hybrid LMS boot test
+        run: |
+          ./wolfboot.elf get_version
+
+      - name: make clean
+        run: |
+          make keysclean
+
+      - name: Build wolfboot.elf (ECC primary, LMS secondary)
+        run: |
+          make clean && make SIGN=ECC256 SIGN_SECONDARY=LMS \
+            WOLFBOOT_UNIVERSAL_KEYSTORE=1
+
+      - name: Run sunny day hybrid ECC + LMS boot test
+        run: |
+          ./wolfboot.elf get_version
+
+      - name: make clean
+        run: |
+          make keysclean
+
+      - name: Select config (64 bit simulator) Hybrid XMSS + ECC
+        run: |
+          cp config/examples/sim-xmss.config .config
+
+      - name: Build wolfboot.elf
+        run: |
+          make clean && make SIGN_SECONDARY=ECC256 WOLFBOOT_UNIVERSAL_KEYSTORE=1
+
+      - name: Run sunny day hybrid XMSS boot test
+        run: |
+          ./wolfboot.elf get_version

include/keystore.h

@@ -32,7 +32,9 @@ extern "C" {
 #ifndef KEYSTORE_PUBKEY_SIZE
     /* allow building version for external API use */
     #define KEYSTORE_ANY
-#ifdef WOLFBOOT_SIGN_ML_DSA
+/* A universal keystore holds every configured algorithm in one slot width,
+ * so ML-DSA sets it whether it is the primary or the secondary. */
+#if defined(WOLFBOOT_SIGN_ML_DSA) || defined(WOLFBOOT_SIGN_SECONDARY_ML_DSA)
     #define KEYSTORE_PUBKEY_SIZE KEYSTORE_PUBKEY_SIZE_ML_DSA
 #else
     #define KEYSTORE_PUBKEY_SIZE KEYSTORE_PUBKEY_SIZE_RSA4096

options.mk

@@ -911,18 +911,50 @@ ifneq ($(SIGN_SECONDARY),)
   endif
   ifeq ($(SIGN_SECONDARY),LMS)
     WOLFCRYPT_OBJS+=$(LMS_OBJS)
-    CFLAGS+=-D"WOLFBOOT_SIGN_LMS" $(LMS_EXTRA)
+    CFLAGS+=$(LMS_EXTRA)
   endif
   ifeq ($(SIGN_SECONDARY),XMSS)
     WOLFCRYPT_OBJS+= $(XMSS_OBJS)
-    CFLAGS+=-D"WOLFBOOT_SIGN_XMSS" $(XMSS_EXTRA)
+    CFLAGS+=$(XMSS_EXTRA)
   endif
   ifeq ($(SIGN_SECONDARY),ML_DSA)
     WOLFCRYPT_OBJS+= $(ML_DSA_OBJS)
     ifneq ($(HASH),SHA3)
       WOLFCRYPT_OBJS+=$(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/sha3.o
     endif
-    CFLAGS+=-D"WOLFBOOT_SIGN_ML_DSA" $(ML_DSA_EXTRA)
+    CFLAGS+=$(ML_DSA_EXTRA)
+  endif
+
+  # The two signatures are verified in sequence, so the ceiling is the deeper
+  # of the two paths. STACK_USAGE above describes the primary only, which
+  # under-reports whenever a hash-based primary is paired with ECC or RSA.
+  # These are the worst case for the family rather than for the exact size.
+  ifneq ($(filter $(SIGN_SECONDARY),ECC256 ED25519 ED448),)
+    STACK_USAGE_SECONDARY=7632
+  endif
+  ifneq ($(filter $(SIGN_SECONDARY),ECC384 ECC521),)
+    STACK_USAGE_SECONDARY=11256
+  endif
+  ifneq ($(filter $(SIGN_SECONDARY),RSA2048 RSA3072 RSA4096 RSAPSS2048 RSAPSS3072 RSAPSS4096),)
+    ifneq ($(SPMATH),1)
+      STACK_USAGE_SECONDARY=69232
+    else
+      STACK_USAGE_SECONDARY=18064
+    endif
+  endif
+  ifeq ($(SIGN_SECONDARY),LMS)
+    STACK_USAGE_SECONDARY=1320
+  endif
+  ifeq ($(SIGN_SECONDARY),XMSS)
+    STACK_USAGE_SECONDARY=9352
+  endif
+  ifeq ($(SIGN_SECONDARY),ML_DSA)
+    STACK_USAGE_SECONDARY=25000
+  endif
+  ifneq ($(STACK_USAGE_SECONDARY),)
+    ifeq ($(shell test $(STACK_USAGE_SECONDARY) -gt $(STACK_USAGE); echo $$?),0)
+      STACK_USAGE=$(STACK_USAGE_SECONDARY)
+    endif
   endif
 endif
 

src/image.c

@@ -697,7 +697,7 @@ static void wolfBoot_verify_signature_rsa_common(uint8_t key_slot,
 
 #endif /* WOLFBOOT_SIGN_RSA_ANY */
 
-#ifdef WOLFBOOT_SIGN_LMS
+#if defined(WOLFBOOT_SIGN_LMS) || defined(WOLFBOOT_SIGN_SECONDARY_LMS)
 #ifdef HAVE_LIBLMS
     #include <wolfssl/wolfcrypt/ext_lms.h>
 #else
@@ -740,8 +740,10 @@ static void wolfBoot_verify_signature_lms(uint8_t key_slot,
     wolfBoot_printf("info: using LMS parameters: L%d-H%d-W%d\n", LMS_LEVELS,
                     LMS_HEIGHT, LMS_WINTERNITZ);
 
-    /* Set the public key. */
-    ret = wc_LmsKey_ImportPubRaw(&lms, pubkey, KEYSTORE_PUBKEY_SIZE);
+    /* Set the public key. A universal keystore sizes every slot for the widest
+     * algorithm in the build, so the slot can be wider than this key; the key
+     * tools always store exactly KEYSTORE_PUBKEY_SIZE_LMS bytes in it. */
+    ret = wc_LmsKey_ImportPubRaw(&lms, pubkey, KEYSTORE_PUBKEY_SIZE_LMS);
     if (ret != 0) {
         /* Something is wrong with the pub key or LMS parameters. */
         wolfBoot_printf("error: wc_LmsKey_ImportPubRaw" \
@@ -764,9 +766,9 @@ static void wolfBoot_verify_signature_lms(uint8_t key_slot,
     wc_LmsKey_Free(&lms);
 }
 
-#endif /* WOLFBOOT_SIGN_LMS */
+#endif /* WOLFBOOT_SIGN_LMS || WOLFBOOT_SIGN_SECONDARY_LMS */
 
-#ifdef WOLFBOOT_SIGN_XMSS
+#if defined(WOLFBOOT_SIGN_XMSS) || defined(WOLFBOOT_SIGN_SECONDARY_XMSS)
 #ifdef HAVE_LIBXMSS
     #include <wolfssl/wolfcrypt/ext_xmss.h>
 #else
@@ -806,7 +808,9 @@ static void wolfBoot_verify_signature_xmss(uint8_t key_slot,
     wolfBoot_printf("info: using XMSS parameters: %s\n", WOLFBOOT_XMSS_PARAMS);
 
     /* Set the public key. */
-    ret = wc_XmssKey_ImportPubRaw(&xmss, pubkey, KEYSTORE_PUBKEY_SIZE);
+    /* As for LMS: the slot may be wider than the key, and the key tools store
+     * exactly KEYSTORE_PUBKEY_SIZE_XMSS bytes in it. */
+    ret = wc_XmssKey_ImportPubRaw(&xmss, pubkey, KEYSTORE_PUBKEY_SIZE_XMSS);
     if (ret != 0) {
         /* Something is wrong with the pub key or XMSS parameters. */
         wolfBoot_printf("error: wc_XmssKey_ImportPubRaw" \
@@ -828,9 +832,9 @@ static void wolfBoot_verify_signature_xmss(uint8_t key_slot,
     wc_XmssKey_Free(&xmss);
 }
 
-#endif /* WOLFBOOT_SIGN_XMSS */
+#endif /* WOLFBOOT_SIGN_XMSS || WOLFBOOT_SIGN_SECONDARY_XMSS */
 
-#ifdef WOLFBOOT_SIGN_ML_DSA
+#if defined(WOLFBOOT_SIGN_ML_DSA) || defined(WOLFBOOT_SIGN_SECONDARY_ML_DSA)
 #include <wolfssl/wolfcrypt/wc_mldsa.h>
 
 static void wolfBoot_verify_signature_ml_dsa(uint8_t key_slot,
@@ -1012,7 +1016,7 @@ static void wolfBoot_verify_signature_ml_dsa(uint8_t key_slot,
     }
 }
 
-#endif /* WOLFBOOT_SIGN_ML_DSA */
+#endif /* WOLFBOOT_SIGN_ML_DSA || WOLFBOOT_SIGN_SECONDARY_ML_DSA */
 
 #endif /* WOLFBOOT_TPM && WOLFBOOT_TPM_VERIFY */
 

tools/keytools/keygen.c

@@ -1490,6 +1490,21 @@ static void key_import(uint32_t ktype, const char *fname, uint32_t id_mask)
                 ret = wc_ecc_export_public_raw(eccKey, buf, &qxSz,
                     buf + keySz / 2, &qySz);
             }
+            else {
+                /* Not DER. A keystore slot is zero-padded out to the widest
+                 * algorithm in the build, so a raw key read back out of one
+                 * has nothing but zeros past its own length and the
+                 * truncation to keySz below is what it needs. Any other
+                 * trailing content means the file is not a padded raw key. */
+                int i;
+                ret = 0;
+                for (i = (int)keySz; i < readLen; i++) {
+                    if (buf[i] != 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            }
 
             if (initKey == 0)
                 wc_ecc_free(eccKey);

tools/keytools/sign.c

@@ -324,6 +324,10 @@ struct signing_key {
     LmsKey lms;
     XmssKey xmss;
     wc_MlDsaKey ml_dsa;
+    /* An LMS key is loaded once and then reused. wc_LmsKey_Reload() only
+     * accepts a key that has not been loaded yet, and signing leaves the key
+     * usable, so a second signature in the same run must not reload. */
+    int lms_loaded;
 };
 
 /* Hybrid signing keeps the primary and the secondary private key decoded at
@@ -1246,15 +1250,24 @@ static int sign_digest(int sign, int hash_algo,
             key_file = CMD.secondary_key_file;
         }
         /* Set the callbacks, so LMS can update the private key while signing */
-        ret = wc_LmsKey_SetWriteCb(&k->lms, lms_write_key);
-        if (ret == 0) {
-            ret = wc_LmsKey_SetReadCb(&k->lms, lms_read_key);
-        }
-        if (ret == 0) {
-            ret = wc_LmsKey_SetContext(&k->lms, (void*)key_file);
-        }
-        if (ret == 0) {
-            ret = wc_LmsKey_Reload(&k->lms);
+        /* Load the callbacks and the private state once. wolfCrypt rejects
+         * both on a key that is already working, and a successful sign leaves
+         * the key ready to sign again, so a second signature in the same run
+         * - a delta image signs its own header - reuses what is loaded. */
+        if (!k->lms_loaded) {
+            ret = wc_LmsKey_SetWriteCb(&k->lms, lms_write_key);
+            if (ret == 0) {
+                ret = wc_LmsKey_SetReadCb(&k->lms, lms_read_key);
+            }
+            if (ret == 0) {
+                ret = wc_LmsKey_SetContext(&k->lms, (void*)key_file);
+            }
+            if (ret == 0) {
+                ret = wc_LmsKey_Reload(&k->lms);
+            }
+            if (ret == 0) {
+                k->lms_loaded = 1;
+            }
         }
         if (ret == 0) {
             ret = wc_LmsKey_Sign(&k->lms, signature, signature_sz, digest,
@@ -1617,10 +1630,10 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
     XMEMSET(key, 0, sizeof(key));
     XMEMSET(iv, 0, sizeof(iv));
 
-    /* Check certificate chain file size before allocating header, and adjust
-     * header size if needed */
-    if ((CMD.cert_chain_file != NULL) || (CMD.custom_tlvs > 0) ||
-            (CMD.dts_file != NULL)) {
+    /* Size the header from the fields it will actually carry. Done for every
+     * image, not just when a certificate chain, custom TLV or DTS is present:
+     * a large signature alone can outgrow the configured header. */
+    {
         uint32_t hdr_cert_chain_sz = 0;
         uint32_t required_space;
 
@@ -2848,7 +2861,7 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in
      * Resolve that expansion here, using the same logic, so patch_inv_off
      * reflects the header size actually written; otherwise HDR_IMG_DELTA_INVERSE
      * would encode a stale, too-small offset and break inverse-patch rollback. */
-    if ((CMD.cert_chain_file != NULL) || (CMD.custom_tlvs > 0)) {
+    {
         uint32_t cert_chain_sz = 0;
         uint32_t required_space;
         if (CMD.cert_chain_file != NULL) {
@@ -3276,8 +3289,8 @@ static void set_signature_sizes(int secondary)
 
         DEBUG_PRINT("info: LMS signature size: %d\n", sig_sz);
 
-        if (CMD.header_sz < 2 * sig_sz)
-            CMD.header_sz = 2 * sig_sz;
+        if (CMD.header_sz < sig_sz)
+            CMD.header_sz = sig_sz;
         *sz = sig_sz;
     }
     else if (*sign == SIGN_XMSS) {
@@ -3314,8 +3327,8 @@ static void set_signature_sizes(int secondary)
 
         DEBUG_PRINT("info: XMSS signature size: %d\n", sig_sz);
 
-        if (CMD.header_sz < 2 * sig_sz)
-            CMD.header_sz = 2 * sig_sz;
+        if (CMD.header_sz < sig_sz)
+            CMD.header_sz = sig_sz;
         *sz = sig_sz;
     }
     else if (*sign == SIGN_ML_DSA) {
@@ -3351,8 +3364,8 @@ static void set_signature_sizes(int secondary)
 
         DEBUG_PRINT("info: ML-DSA signature size: %d\n", sig_sz);
 
-        if (CMD.header_sz < 2 * sig_sz)
-            CMD.header_sz = 2 * sig_sz;
+        if (CMD.header_sz < sig_sz)
+            CMD.header_sz = sig_sz;
         *sz = sig_sz;
     }