Address code review follow-ups for Falcon refactor

claude · claude · commit ae7098b3735c · 2026-04-22T08:14:01.000Z
Fix pre-existing wc_Falcon_KeyToDer pubKeyLen typo: it was passing FALCON_LEVELx_KEY_SIZE (secret-key size) as the pubKeyLen argument to SetAsymKeyDer, producing DER with padding/junk bytes instead of the real public key. Now passes FALCON_LEVELx_PUB_KEY_SIZE. Restore the "Note for some CPUs smaller than 32 bit..." header comment to the oid_sum.h generator so it survives regeneration. Was silently dropped by the previous regen. Make Falcon private-key decode accept both wire formats: * wc_Falcon_PrivateKeyDecode no longer routes the full DER back through parse_private_key's legacy OCTET(OCTET(priv||pub)) parser. After DecodeAsymKey extracts privKey and pubKey separately, either use them directly (RFC 5958, as oqs-provider emits) or split the concatenated priv||pub if the legacy double-OCTET wrapping is present. * ProcessBufferTryDecodeFalcon now auto-detects the level via the OID (by trying each level through wc_Falcon_PrivateKeyDecode), and falls back to wc_falcon_import_private_only only when the DER length actually matches a Falcon raw-blob size. The previous length-based level guess erroneously matched Falcon-1024 against an ML-DSA-65 seed-priv PKCS8, masking the correct Dilithium dispatch. Minor: sweep "see mlkem.h" comments in Espressif user_settings.h templates to "see wc_mlkem.h" to match the renamed header, and point the INSTALL SPHINCS+ note at wolfSSL#10261 where the native SLH-DSA replacement is landing. Verified end-to-end against oqs-provider 0.10.0 on OpenSSL 3.0.13: * Four-way X.509 cert matrix (oqs<->wolfSSL, level 1 + 5) passes. * wolfSSL_CTX_use_PrivateKey_file loads both oqs-provider RFC 5958 PEM keys and wolfSSL legacy-format DER bench keys. * make check passes with --enable-falcon --with-liboqs.
diff --git a/IDE/Espressif/ESP-IDF/examples/template/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/template/components/wolfssl/include/user_settings.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_benchmark/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/wolfssl_benchmark/components/wolfssl/include/user_settings.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_client/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/wolfssl_client/components/wolfssl/include/user_settings.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_server/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/wolfssl_server/components/wolfssl/include/user_settings.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h b/IDE/Espressif/ESP-IDF/examples/wolfssl_test/components/wolfssl/include/user_settings.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/INSTALL b/INSTALL
@@ -260,6 +260,9 @@
     (CMake: -DWOLFSSL_OQS=yes -DWOLFSSL_FALCON=yes). Passing --with-liboqs
     without --enable-falcon (or vice versa) is now an error.
 
+    SPHINCS+ is in the middle of being replaced with native SLH-DSA; see
+    PR #10261. Until that lands, SPHINCS+ continues to build via liboqs.
+
     The following NIST Competition Round 3 finalist algorithms were supported,
     but have been removed after 5.3.3
     - SABER (KEM)
diff --git a/examples/configs/user_settings_espressif.h b/examples/configs/user_settings_espressif.h
@@ -234,7 +234,7 @@
         #define WOLFSSL_NO_ML_KEM_768
         #define NO_SESSION_CACHE
     #else
-        /* Only needed for older wolfssl versions, see mlkem.h */
+        /* Only needed for older wolfssl versions, see wc_mlkem.h */
         #define WOLFSSL_KYBER1024
         /* optional alternative sizes:    */
         /* #define WOLFSSL_KYBER768       */
diff --git a/scripts/asn1_oid_sum.pl b/scripts/asn1_oid_sum.pl
@@ -201,6 +201,10 @@ sub print_header {
 #ifndef WOLF_CRYPT_OID_SUM_H
 #define WOLF_CRYPT_OID_SUM_H
 
+/* Note for some CPUs smaller than 32 bit, the upper 16 bits of new OID
+ * values may be ignored. If collisions are encountered, consider WC_16BIT_CPU
+ * and/or WOLFSSL_OLD_OID_SUM to force smaller, old OID values. */
+
 "
 }
 
diff --git a/src/ssl_load.c b/src/ssl_load.c
@@ -825,16 +825,74 @@ static int ProcessBufferTryDecodeFalcon(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
     /* Initialize Falcon key. */
     ret = wc_falcon_init(key);
     if (ret == 0) {
-        /* Set up key to parse the format specified. */
-        if ((*keyFormat == FALCON_LEVEL1k) || ((*keyFormat == 0) &&
-                ((der->length == FALCON_LEVEL1_KEY_SIZE) ||
-                 (der->length == FALCON_LEVEL1_PRV_KEY_SIZE)))) {
-            ret = wc_falcon_set_level(key, 1);
+        byte level = 0;
+        word32 idx;
+
+        if (*keyFormat == FALCON_LEVEL1k) {
+            level = 1;
+        }
+        else if (*keyFormat == FALCON_LEVEL5k) {
+            level = 5;
         }
-        else if ((*keyFormat == FALCON_LEVEL5k) || ((*keyFormat == 0) &&
-                 ((der->length == FALCON_LEVEL5_KEY_SIZE) ||
-                  (der->length == FALCON_LEVEL5_PRV_KEY_SIZE)))) {
-            ret = wc_falcon_set_level(key, 5);
+
+        if (level != 0) {
+            /* Caller told us the level via the OID sum - decode directly. */
+            ret = wc_falcon_set_level(key, level);
+            if (ret == 0) {
+                idx = 0;
+                ret = wc_Falcon_PrivateKeyDecode(der->buffer, &idx, key,
+                                                  der->length);
+                if (ret != 0) {
+                    /* Accept raw priv||pub blob for the declared level. */
+                    ret = wc_falcon_import_private_only(der->buffer,
+                                                        der->length, key);
+                }
+            }
+        }
+        else if (*keyFormat == 0) {
+            /* Key format unknown. Try both levels as a PKCS8-wrapped key
+             * (OID drives the choice inside wc_Falcon_PrivateKeyDecode); if
+             * that fails, try raw blob matching on length. */
+            idx = 0;
+            if (wc_falcon_set_level(key, 1) == 0 &&
+                wc_Falcon_PrivateKeyDecode(der->buffer, &idx, key,
+                                           der->length) == 0) {
+                level = 1;
+            }
+            else {
+                idx = 0;
+                if (wc_falcon_set_level(key, 5) == 0 &&
+                    wc_Falcon_PrivateKeyDecode(der->buffer, &idx, key,
+                                               der->length) == 0) {
+                    level = 5;
+                }
+            }
+            if (level == 0 &&
+                (der->length == FALCON_LEVEL1_KEY_SIZE ||
+                 der->length == FALCON_LEVEL1_PRV_KEY_SIZE)) {
+                if (wc_falcon_set_level(key, 1) == 0 &&
+                    wc_falcon_import_private_only(der->buffer, der->length,
+                                                    key) == 0) {
+                    level = 1;
+                }
+            }
+            if (level == 0 &&
+                (der->length == FALCON_LEVEL5_KEY_SIZE ||
+                 der->length == FALCON_LEVEL5_PRV_KEY_SIZE)) {
+                if (wc_falcon_set_level(key, 5) == 0 &&
+                    wc_falcon_import_private_only(der->buffer, der->length,
+                                                    key) == 0) {
+                    level = 5;
+                }
+            }
+            if (level == 0) {
+                /* Not a Falcon key; let caller try another algorithm. */
+                WOLFSSL_MSG("Not a Falcon key");
+                wc_falcon_free(key);
+                XFREE(key, heap, DYNAMIC_TYPE_FALCON);
+                return 0;
+            }
+            ret = 0;
         }
         else {
             wc_falcon_free(key);
@@ -843,38 +901,27 @@ static int ProcessBufferTryDecodeFalcon(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
     }
 
     if (ret == 0) {
-        /* Decode as a Falcon private key. */
-        ret = wc_falcon_import_private_only(der->buffer, der->length, key);
-        if (ret == 0) {
-            /* Get the minimum Falcon key size from SSL or SSL context object.
-             */
-            int minKeySz = ssl ? ssl->options.minFalconKeySz :
-                                 ctx->minFalconKeySz;
-
-            /* Format is known. */
-            if (*keyFormat == FALCON_LEVEL1k) {
-                *keyType = falcon_level1_sa_algo;
-                *keySize = FALCON_LEVEL1_KEY_SIZE;
-            }
-            else {
-                *keyType = falcon_level5_sa_algo;
-                *keySize = FALCON_LEVEL5_KEY_SIZE;
-            }
+        /* Get the minimum Falcon key size from SSL or SSL context object. */
+        int minKeySz = ssl ? ssl->options.minFalconKeySz :
+                             ctx->minFalconKeySz;
 
-            /* Check that the size of the Falcon key is enough. */
-            if (*keySize < minKeySz) {
-                WOLFSSL_MSG("Falcon private key too small");
-                ret = FALCON_KEY_SIZE_E;
-            }
+        if (key->level == 1) {
+            *keyFormat = FALCON_LEVEL1k;
+            *keyType = falcon_level1_sa_algo;
+            *keySize = FALCON_LEVEL1_KEY_SIZE;
         }
-        /* Not a Falcon key but check whether we know what it is. */
-        else if (*keyFormat == 0) {
-            WOLFSSL_MSG("Not a Falcon key");
-            /* Format unknown so keep trying. */
-            ret = 0;
+        else {
+            *keyFormat = FALCON_LEVEL5k;
+            *keyType = falcon_level5_sa_algo;
+            *keySize = FALCON_LEVEL5_KEY_SIZE;
+        }
+
+        /* Check that the size of the Falcon key is enough. */
+        if (*keySize < minKeySz) {
+            WOLFSSL_MSG("Falcon private key too small");
+            ret = FALCON_KEY_SIZE_E;
         }
 
-        /* Free dynamically allocated data in key. */
         wc_falcon_free(key);
     }
     else if ((ret == WC_NO_ERR_TRACE(ALGO_ID_E)) && (*keyFormat == 0)) {
diff --git a/wolfcrypt/src/falcon.c b/wolfcrypt/src/falcon.c
@@ -844,12 +844,29 @@ int wc_Falcon_PrivateKeyDecode(const byte* input, word32* inOutIdx,
     ret = DecodeAsymKey(input, inOutIdx, inSz, privKey, &privKeyLen,
                         pubKey, &pubKeyLen, keytype);
     if (ret == 0) {
-        if (pubKeyLen == 0) {
-            ret = wc_falcon_import_private_key(input, inSz, NULL, 0, key);
+        word32 skSz = (key->level == 1) ? FALCON_LEVEL1_KEY_SIZE
+                                        : FALCON_LEVEL5_KEY_SIZE;
+        word32 pkSz = (key->level == 1) ? FALCON_LEVEL1_PUB_KEY_SIZE
+                                        : FALCON_LEVEL5_PUB_KEY_SIZE;
+
+        /* Legacy wolfSSL layout: privateKey OCTET STRING carries priv||pub
+         * concatenated and there is no separate publicKey field. Split the
+         * buffer so the size validation below handles both layouts. */
+        if (pubKeyLen == 0 && privKeyLen == skSz + pkSz) {
+            XMEMCPY(pubKey, privKey + skSz, pkSz);
+            pubKeyLen = pkSz;
+            privKeyLen = skSz;
+        }
+
+        if (privKeyLen != skSz || pubKeyLen != pkSz) {
+            ret = BAD_FUNC_ARG;
         }
         else {
-            ret = wc_falcon_import_private_key(input, inSz, pubKey,
-                                               pubKeyLen, key);
+            ret = wc_falcon_import_public(pubKey, pubKeyLen, key);
+            if (ret == 0) {
+                XMEMCPY(key->k, privKey, privKeyLen);
+                key->prvKeySet = 1;
+            }
         }
     }
 
@@ -955,12 +972,12 @@ int wc_Falcon_KeyToDer(falcon_key* key, byte* output, word32 inLen)
 
     if (key->level == 1) {
         return SetAsymKeyDer(key->k, FALCON_LEVEL1_KEY_SIZE, key->p,
-                             FALCON_LEVEL1_KEY_SIZE, output, inLen,
+                             FALCON_LEVEL1_PUB_KEY_SIZE, output, inLen,
                              FALCON_LEVEL1k);
     }
     else if (key->level == 5) {
         return SetAsymKeyDer(key->k, FALCON_LEVEL5_KEY_SIZE, key->p,
-                             FALCON_LEVEL5_KEY_SIZE, output, inLen,
+                             FALCON_LEVEL5_PUB_KEY_SIZE, output, inLen,
                              FALCON_LEVEL5k);
     }
 
diff --git a/wolfssl/wolfcrypt/oid_sum.h b/wolfssl/wolfcrypt/oid_sum.h
@@ -26,6 +26,10 @@
 #ifndef WOLF_CRYPT_OID_SUM_H
 #define WOLF_CRYPT_OID_SUM_H
 
+/* Note for some CPUs smaller than 32 bit, the upper 16 bits of new OID
+ * values may be ignored. If collisions are encountered, consider WC_16BIT_CPU
+ * and/or WOLFSSL_OLD_OID_SUM to force smaller, old OID values. */
+
 enum Hash_Sum {
 #ifdef WOLFSSL_OLD_OID_SUM
     /* 0x2a,0x86,0x48,0x86,0xf7,0x0d,0x02,0x02  */

Original file line number	Diff line number	Diff line change
`@@ -201,6 +201,10 @@ sub print_header {`
`201`	`201`	`#ifndef WOLF_CRYPT_OID_SUM_H`
`202`	`202`	`#define WOLF_CRYPT_OID_SUM_H`
`203`	`203`
	`204`	`+/* Note for some CPUs smaller than 32 bit, the upper 16 bits of new OID`
	`205`	`+ * values may be ignored. If collisions are encountered, consider WC_16BIT_CPU`
	`206`	`+ * and/or WOLFSSL_OLD_OID_SUM to force smaller, old OID values. */`
	`207`	`+`
`204`	`208`	`"`
`205`	`209`	`}`
`206`	`210`