Falcon: address review feedback (zephyr sources, configure sub-options, footprint)

Four fixes from PR review:

- zephyr/CMakeLists.txt: the native port split falcon.c into wc_falcon_*.c
  translation units; add the portable sources so a Zephyr build with Falcon
  links. x86-64 asm/AVX2 and the NEON backend are left out (not selected by any
  Zephyr config).

- configure.ac: fold the standalone --enable-falcon-{asm,double,avx2,neon}
  switches into comma-separated sub-options of --enable-falcon
  (e.g. --enable-falcon=avx2), matching the common wolfSSL idiom. avx2/neon
  imply the double backend after arch-gating so ignoring an unsupported vector
  backend does not clobber an explicit 'double'. Sweep the qemu-falcon-neon doc
  to the new spelling.

- configure.ac: align the Falcon line in the two feature summaries.

- falcon.c/falcon.h: drop the duplicate public-key copy kept behind the private
  key. Its only remaining reader was wc_falcon_check_key, whose compare was
  against a copy of the same bytes and so could never detect a real mismatch;
  wc_falcon_export_private already rebuilds the concat layout on demand. Shrink
  key->k from FALCON_MAX_PRV_KEY_SIZE to FALCON_MAX_KEY_SIZE (saves 1793 bytes
  per key at level 5). check_key now verifies both halves are present and
  documents a full cryptographic cross-check as a follow-up. Update the unit
  test that relied on the old in-memory-copy compare.

IDE/qemu-falcon-neon/README.md

@@ -33,4 +33,4 @@ grep -E 'fmla\s+v[0-9]+\.2d' IDE/qemu-falcon-neon/app-falcon-neon.dis
 ```
 
 In a wolfSSL library build, the NEON FFT is enabled with
-`--enable-falcon-neon` (AArch64; implies `--enable-falcon-double`).
+`--enable-falcon=neon` (AArch64; implies the `double` fpr backend).

configure.ac

@@ -1816,87 +1816,89 @@ AC_ARG_WITH([liboqs],
 # section below). Because the algorithm is not yet standardized and its API name
 # is subject to change, it requires --enable-experimental (checked in the CFLAG
 # section, once all backend sub-options have been resolved).
+#
+# fpr/FFT backend selection follows the common wolfSSL comma-list idiom as
+# sub-options of --enable-falcon (e.g. --enable-falcon=asm, --enable-falcon=avx2)
+# rather than standalone --enable-falcon-* switches. Recognised sub-options:
+#   asm    - per-architecture assembly fpr backend (x86-64 SSE2 only); the
+#            portable constant-time integer emulation remains the default.
+#   double - inline native-double fpr backend: the fpr ops become static-inline
+#            C double operations, so the FFT/sampler inline them and keep values
+#            in FP registers (no per-op call). Fastest backend on any platform
+#            with a constant-time hardware double FPU (signing ~2.5x faster than
+#            the out-of-line asm backend, ~16x faster than the portable
+#            emulation), but like asm it relies on the native FPU's rounding
+#            behavior. Mutually exclusive with asm.
+#   avx2   - AVX2 (4-wide __m256d + FMA) vectorized FFT for the signing path
+#            (x86-64); implies double. The AVX2 functions carry target("avx2,fma")
+#            attributes, so the TU builds under a baseline -march, but the host
+#            must support AVX2+FMA at run time. Signing is sampler-bound, so this
+#            is a modest (~1.1x) end-to-end speedup.
+#   neon   - AArch64 NEON (2-wide float64x2_t + FMA) vectorized FFT; implies
+#            double. Advanced SIMD is part of the ARMv8-A baseline, so no special
+#            -march is required.
 AC_ARG_ENABLE([falcon],
-    [AS_HELP_STRING([--enable-falcon],[Enable Falcon post-quantum signatures (native, no liboqs; requires --enable-experimental) (default: disabled)])],
+    [AS_HELP_STRING([--enable-falcon@<:@=OPTS@:>@],[Enable Falcon post-quantum signatures (native, no liboqs; requires --enable-experimental). OPTS is a comma-separated list of fpr/FFT backends: asm, double, avx2, neon (default: disabled)])],
     [ ENABLED_FALCON=$enableval ],
     [ ENABLED_FALCON=no ])
 
-# --enable-falcon-asm selects the per-architecture assembly fpr backend for
-# Falcon (currently x86-64 SSE2 only); the portable constant-time integer
-# emulation remains the default. It implies --enable-falcon.
-AC_ARG_ENABLE([falcon-asm],
-    [AS_HELP_STRING([--enable-falcon-asm],[Enable Falcon x86-64 assembly fpr backend (default: disabled)])],
-    [ ENABLED_FALCON_ASM=$enableval ],
-    [ ENABLED_FALCON_ASM=no ])
-if test "$ENABLED_FALCON_ASM" = "yes"; then
-    case $host_cpu in
-        *x86_64*|*amd64*) ENABLED_FALCON=yes ;;
-        *) AC_MSG_WARN([--enable-falcon-asm is only supported on x86-64; ignoring.])
-           ENABLED_FALCON_ASM=no ;;
-    esac
-fi
+ENABLED_FALCON_ASM=no
+ENABLED_FALCON_DOUBLE=no
+ENABLED_FALCON_AVX2=no
+ENABLED_FALCON_NEON=no
 
-# --enable-falcon-double selects the inline native-double fpr backend: the fpr
-# ops become static-inline C double operations, so the FFT/sampler inline them
-# and keep values in FP registers (no per-op call). This is the fastest backend
-# on any platform with a constant-time hardware double FPU (signing ~2.5x faster
-# than the out-of-line asm backend, ~16x faster than the portable emulation),
-# but like --enable-falcon-asm it relies on the native FPU's rounding behavior.
-# It implies --enable-falcon and is mutually exclusive with --enable-falcon-asm.
-AC_ARG_ENABLE([falcon-double],
-    [AS_HELP_STRING([--enable-falcon-double],[Enable Falcon inline native-double fpr backend (default: disabled)])],
-    [ ENABLED_FALCON_DOUBLE=$enableval ],
-    [ ENABLED_FALCON_DOUBLE=no ])
-if test "$ENABLED_FALCON_DOUBLE" = "yes"; then
-    if test "$ENABLED_FALCON_ASM" = "yes"; then
-        AC_MSG_ERROR([--enable-falcon-double and --enable-falcon-asm are mutually exclusive.])
-    fi
+# Parse the comma-separated backend sub-options. AC_ARG_ENABLE yields the plain
+# "yes"/"no" for a bare --enable-falcon/--disable-falcon; anything else is a
+# backend list. The double implication of avx2/neon is applied after arch-gating
+# so ignoring an unsupported vector backend does not clobber an explicit 'double'.
+if test "$ENABLED_FALCON" != "no" && test "$ENABLED_FALCON" != "yes"; then
+    OIFS="$IFS"
+    IFS=','
+    for opt in $ENABLED_FALCON; do
+        case "$opt" in
+            yes) ;;
+            asm)    ENABLED_FALCON_ASM=yes ;;
+            double) ENABLED_FALCON_DOUBLE=yes ;;
+            avx2)   ENABLED_FALCON_AVX2=yes ;;
+            neon)   ENABLED_FALCON_NEON=yes ;;
+            *) AC_MSG_ERROR([Unknown falcon option: $opt. Valid options: asm, double, avx2, neon]) ;;
+        esac
+    done
+    IFS="$OIFS"
     ENABLED_FALCON=yes
 fi
 
-# --enable-falcon-avx2 adds the AVX2 (4-wide __m256d + FMA) vectorized FFT and
-# FFT-domain polynomial operations for the signing path (x86-64). It composes
-# with --enable-falcon-double (which it implies). The AVX2 functions carry
-# target("avx2,fma") attributes, so the TU builds under a baseline -march; the
-# host must support AVX2+FMA at run time. Signing is sampler-bound, so the FFT
-# vectorization yields a modest (~1.1x) end-to-end speedup.
-AC_ARG_ENABLE([falcon-avx2],
-    [AS_HELP_STRING([--enable-falcon-avx2],[Enable Falcon x86-64 AVX2 vectorized FFT (default: disabled)])],
-    [ ENABLED_FALCON_AVX2=$enableval ],
-    [ ENABLED_FALCON_AVX2=no ])
+# Resolve backend arch-gating: asm and avx2 are x86-64 only; neon is AArch64
+# only. An unsupported backend is warned and ignored.
+if test "$ENABLED_FALCON_ASM" = "yes"; then
+    case $host_cpu in
+        *x86_64*|*amd64*) ;;
+        *) AC_MSG_WARN([falcon 'asm' backend is only supported on x86-64; ignoring.])
+           ENABLED_FALCON_ASM=no ;;
+    esac
+fi
 if test "$ENABLED_FALCON_AVX2" = "yes"; then
     case $host_cpu in
-        *x86_64*|*amd64*)
-            if test "$ENABLED_FALCON_ASM" = "yes"; then
-                AC_MSG_ERROR([--enable-falcon-avx2 and --enable-falcon-asm are mutually exclusive.])
-            fi
-            ENABLED_FALCON=yes
-            ENABLED_FALCON_DOUBLE=yes ;;
-        *) AC_MSG_WARN([--enable-falcon-avx2 is only supported on x86-64; ignoring.])
+        *x86_64*|*amd64*) ;;
+        *) AC_MSG_WARN([falcon 'avx2' backend is only supported on x86-64; ignoring.])
            ENABLED_FALCON_AVX2=no ;;
     esac
 fi
-
-# --enable-falcon-neon adds the AArch64 NEON (2-wide float64x2_t + FMA) vectorized
-# FFT for the signing path. It composes with --enable-falcon-double (which it
-# implies). AArch64 Advanced SIMD is part of the ARMv8-A baseline, so no special
-# -march is required.
-AC_ARG_ENABLE([falcon-neon],
-    [AS_HELP_STRING([--enable-falcon-neon],[Enable Falcon AArch64 NEON vectorized FFT (default: disabled)])],
-    [ ENABLED_FALCON_NEON=$enableval ],
-    [ ENABLED_FALCON_NEON=no ])
 if test "$ENABLED_FALCON_NEON" = "yes"; then
     case $host_cpu in
-        *aarch64*|*arm64*)
-            if test "$ENABLED_FALCON_ASM" = "yes"; then
-                AC_MSG_ERROR([--enable-falcon-neon and --enable-falcon-asm are mutually exclusive.])
-            fi
-            ENABLED_FALCON=yes
-            ENABLED_FALCON_DOUBLE=yes ;;
-        *) AC_MSG_WARN([--enable-falcon-neon is only supported on AArch64; ignoring.])
+        *aarch64*|*arm64*) ;;
+        *) AC_MSG_WARN([falcon 'neon' backend is only supported on AArch64; ignoring.])
            ENABLED_FALCON_NEON=no ;;
     esac
 fi
+# avx2/neon vectorized FFT requires the inline native-double fpr backend.
+if test "$ENABLED_FALCON_AVX2" = "yes" || test "$ENABLED_FALCON_NEON" = "yes"; then
+    ENABLED_FALCON_DOUBLE=yes
+fi
+# asm and the double-based backends select different fpr representations.
+if test "$ENABLED_FALCON_DOUBLE" = "yes" && test "$ENABLED_FALCON_ASM" = "yes"; then
+    AC_MSG_ERROR([falcon 'double' (also implied by 'avx2'/'neon') and 'asm' backends are mutually exclusive.])
+fi
 
 
 # MLKEM
@@ -13209,7 +13211,7 @@ echo "   * XMSS:                       $ENABLED_XMSS"
 echo "   * SLH-DSA                     $ENABLED_SLHDSA"
 echo "   * MLKEM:                      $ENABLED_MLKEM"
 echo "   * ML-DSA:                     $ENABLED_MLDSA"
-echo "   * Falcon:     $ENABLED_FALCON"
+echo "   * Falcon:                     $ENABLED_FALCON"
 echo "   * ECCSI                       $ENABLED_ECCSI"
 echo "   * SAKKE                       $ENABLED_SAKKE"
 echo "   * ASN:                        $ENABLED_ASN"
@@ -13267,7 +13269,7 @@ echo "   * Persistent session cache:   $ENABLED_SAVESESSION"
 echo "   * Persistent cert    cache:   $ENABLED_SAVECERT"
 echo "   * Atomic User Record Layer:   $ENABLED_ATOMICUSER"
 echo "   * Public Key Callbacks:       $ENABLED_PKCALLBACKS"
-echo "   * Falcon:     $ENABLED_FALCON"
+echo "   * Falcon:                     $ENABLED_FALCON"
 echo "   * Whitewood netRandom:        $ENABLED_WNR"
 echo "   * Server Name Indication:     $ENABLED_SNI"
 echo "   * ALPN:                       $ENABLED_ALPN"

tests/api/test_falcon.c

@@ -451,10 +451,6 @@ int test_wc_falcon_check_key(void)
         prvLen = FALCON_MAX_KEY_SIZE;
         ExpectIntEQ(wc_falcon_export_private_only(&key, prv, &prvLen), 0);
 
-        /* Corrupt the standalone public copy: check_key compares it to the
-         * public bytes stored behind the private key, so this mismatches. */
-        key.p[0] ^= 0xFF;
-        ExpectIntEQ(wc_falcon_check_key(&key), WC_NO_ERR_TRACE(PUBLIC_KEY_E));
         wc_falcon_free(&key);
 
         /* Public only (no private) -> PUBLIC_KEY_E. */
@@ -473,10 +469,8 @@ int test_wc_falcon_check_key(void)
         ExpectIntEQ(wc_falcon_check_key(&key), WC_NO_ERR_TRACE(PUBLIC_KEY_E));
         wc_falcon_free(&key);
 
-        /* Public imported FIRST, then a raw (non-concat) private key: the
-         * public copy stored behind the private key must be synced so
-         * check_key passes. Regression for the raw-import path not calling
-         * falcon_store_pub_behind_priv. */
+        /* Public imported FIRST, then a raw (non-concat) private key: both
+         * halves are now present, so check_key passes. */
         XMEMSET(&key, 0, sizeof(key));
         ExpectIntEQ(wc_falcon_init(&key), 0);
         ExpectIntEQ(wc_falcon_set_level(&key, level), 0);

wolfcrypt/src/falcon.c

@@ -39,26 +39,6 @@
     #include <wolfcrypt/src/misc.c>
 #endif
 
-/* Store a second copy of the public key in key->k immediately after the private
- * key, reproducing the historical concat(private,public) layout that
- * wc_falcon_check_key compares against. No-op unless both halves are set.
- * Defined unconditionally: wc_falcon_import_public (a verify-only operation)
- * calls it. */
-static void falcon_store_pub_behind_priv(falcon_key* key)
-{
-    if (!key->pubKeySet || !key->prvKeySet) {
-        return;
-    }
-    if (key->level == 1) {
-        XMEMCPY(key->k + FALCON_LEVEL1_KEY_SIZE, key->p,
-                FALCON_LEVEL1_PUB_KEY_SIZE);
-    }
-    else if (key->level == 5) {
-        XMEMCPY(key->k + FALCON_LEVEL5_KEY_SIZE, key->p,
-                FALCON_LEVEL5_PUB_KEY_SIZE);
-    }
-}
-
 #ifndef WOLFSSL_FALCON_VERIFY_ONLY
 /* Generate a new Falcon key pair into key (key->level must be set first).
  *
@@ -97,9 +77,6 @@ int wc_falcon_make_key(falcon_key* key, WC_RNG* rng)
     ret = NO_VALID_DEVID;
 #else
     ret = falcon_native_make_key(key, rng);
-    if (ret == 0) {
-        falcon_store_pub_behind_priv(key);
-    }
 #endif
     return ret;
 }
@@ -448,8 +425,6 @@ int wc_falcon_import_public(const byte* in, word32 inLen,
 
     XMEMCPY(key->p, in, inLen);
     key->pubKeySet = 1;
-    /* Keep the concat(private,public) copy in sync if a private key is loaded. */
-    falcon_store_pub_behind_priv(key);
 
     return 0;
 }
@@ -505,13 +480,6 @@ int wc_falcon_import_private_only(const byte* priv, word32 privSz,
         key->pubKeySet = 1;
     }
 
-    /* Sync the public copy kept behind the private key whenever both halves are
-     * present. This also covers the raw-size case where a public key was
-     * imported first: without it key->k + KEY_SIZE would stay zero and
-     * wc_falcon_check_key would wrongly return PUBLIC_KEY_E. No-op when no
-     * public key is set. */
-    falcon_store_pub_behind_priv(key);
-
     return 0;
 }
 
@@ -671,18 +639,23 @@ int wc_falcon_export_key(falcon_key* key, byte* priv, word32 *privSz,
     return ret;
 }
 
-/* Check the public key of the falcon key matches the private key.
+/* Check that the falcon key has a matching private/public key pair present.
  *
  * key     [in]      Falcon private/public key.
- * returns BAD_FUNC_ARG when key is NULL,
- *         PUBLIC_KEY_E when the public key is not set or doesn't match,
- *         other -ve value on hash failure,
+ * returns BAD_FUNC_ARG when key is NULL or the level is unset,
+ *         PUBLIC_KEY_E when either the public or private half is not set,
  *         0 otherwise.
+ *
+ * Note: this verifies both halves of the pair are loaded. It does not yet
+ * perform a full cryptographic cross-check (recomputing the public key h from
+ * the private (f, g) and comparing it against the stored public key); that is a
+ * TODO once a standalone public-key-from-private helper is exposed by the native
+ * core. The previous implementation compared the stored public key against a
+ * duplicate copy kept behind the private key, which was always a copy of the
+ * same bytes and so could never detect a mismatch.
  */
 int wc_falcon_check_key(falcon_key* key)
 {
-    int ret = 0;
-
     if (key == NULL) {
         return BAD_FUNC_ARG;
     }
@@ -695,22 +668,7 @@ int wc_falcon_check_key(falcon_key* key)
         return PUBLIC_KEY_E;
     }
 
-    /* The public key is also decoded and stored within the private key buffer
-     * behind the private key. Hence, we can compare both stored public keys. */
-    if (key->level == 1) {
-        ret = XMEMCMP(key->p, key->k + FALCON_LEVEL1_KEY_SIZE,
-                      FALCON_LEVEL1_PUB_KEY_SIZE);
-    }
-    else if (key->level == 5) {
-        ret = XMEMCMP(key->p, key->k + FALCON_LEVEL5_KEY_SIZE,
-                      FALCON_LEVEL5_PUB_KEY_SIZE);
-    }
-
-    if (ret != 0) {
-        ret = PUBLIC_KEY_E;
-    }
-
-    return ret;
+    return 0;
 }
 
 /* Returns the size of a falcon private key.

wolfssl/wolfcrypt/falcon.h

@@ -125,7 +125,10 @@ struct falcon_key {
 #endif
 
     byte p[FALCON_MAX_PUB_KEY_SIZE];
-    byte k[FALCON_MAX_PRV_KEY_SIZE];
+    /* Private key only: the secret polynomials (header | f | g | F). The public
+     * key is held separately in p[]; the concat(priv,pub) layout is rebuilt on
+     * demand by wc_falcon_export_private, so no duplicate copy is kept here. */
+    byte k[FALCON_MAX_KEY_SIZE];
 };
 
 #ifndef WC_FALCONKEY_TYPE_DEFINED

zephyr/CMakeLists.txt

@@ -79,6 +79,15 @@ if(CONFIG_WOLFSSL)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/ed448.c)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/error.c)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/falcon.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_bigint.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_codec.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_fft.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_fpr.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_keygen.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_poly.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_sampler.c)
+    zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/wc_falcon_sign.c)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/fe_448.c)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/fe_low_mem.c)
     zephyr_library_sources(${ZEPHYR_CURRENT_MODULE_DIR}/wolfcrypt/src/fe_operations.c)