1
0
mirror of https://github.com/wolfSSL/wolfssl.git synced 2026-10-24 04:09:07 +03:00

Falcon: address PR review feedback

- cmake/functions.cmake: remove duplicated BUILD_FALCON conditional block;
  BUILD_FALCON is now set in one place.
- wolfssl/wolfcrypt/include.am: list the internal wc_falcon_*.h headers under
  an "if BUILD_FALCON" block so they ship in "make dist" tarballs and install
  when Falcon is enabled (the public falcon.h was already listed). Automake
  distributes both branches of the conditional, so a Falcon-disabled dist
  still contains them.
- wc_falcon_fft_neon.c: fix strict-aliasing UB in falcon_FFT/falcon_iFFT.
  fpr is a word64 bit pattern; casting fpr* to double* and using
  vld1q_f64/vst1q_f64 accesses the store through the wrong type and can
  miscompile at -O2/-O3. Load/store through uint64_t (the real object type)
  and reinterpret the vector register to/from f64 via new FALCON_VLD/FALCON_VST
  macros. Verified under qemu (NEON_FFT_PASS) with fmla v.2d still emitted.
- wc_falcon_sampler.c / wc_falcon_sampler.h: falcon_prng_get_u8/get_u64 have
  no error return, so a SHAKE256 squeeze failure previously went unnoticed and
  stale buffer bytes could feed the signer. Add a sticky "err" field latched on
  the first refill failure.
- wc_falcon_sign.c: falcon_sign_core now rejects the signature when the PRNG
  latched an error, and falcon_do_sign_tree bounds its restart loop
  (FALCON_SIGN_MAX_RESTARTS) so a wedged sampler terminates instead of
  spinning forever. The bound is far beyond any legitimate restart count.
This commit is contained in:
Daniele Lacamera
2026-07-01 11:50:08 +02:00
parent f2d79b554e
commit 89f024f660
6 changed files with 88 additions and 32 deletions

View File

@@ -262,9 +262,6 @@ function(generate_build_flags)
if(WOLFSSL_FALCON OR WOLFSSL_USER_SETTINGS)
set(BUILD_FALCON "yes" PARENT_SCOPE)
endif()
if(WOLFSSL_FALCON OR WOLFSSL_USER_SETTINGS)
set(BUILD_FALCON "yes" PARENT_SCOPE)
endif()
if(WOLFSSL_LMS OR WOLFSSL_USER_SETTINGS)
set(BUILD_WC_LMS "yes" PARENT_SCOPE)
endif()

View File

@@ -79,13 +79,22 @@ static WC_INLINE double falcon_neon_d(fpr x)
(out_im) = vfmaq_f64(vmulq_f64((yr), (si)), (yi), (sr)); \
} while (0)
/* Aliasing-safe vector load/store. fpr is a word64 IEEE-754 bit pattern, so
* the backing store's real object type is the 64-bit integer, not double.
* Load/store through uint64_t (the actual type) and reinterpret the vector
* register to/from f64 -- casting fpr* to double* and using vld1q_f64/vst1q_f64
* would be a strict-aliasing violation that can miscompile at -O2/-O3. */
#define FALCON_VLD(pf) \
vreinterpretq_f64_u64(vld1q_u64((const uint64_t*)(const void*)(pf)))
#define FALCON_VST(pf, v) \
vst1q_u64((uint64_t*)(void*)(pf), vreinterpretq_u64_f64(v))
/* ------------------------------------------------------------------------- */
/* Forward FFT */
/* ------------------------------------------------------------------------- */
void falcon_FFT(fpr* f, unsigned logn)
{
double* fd = (double*)f;
unsigned u;
size_t t, n, hn, m;
@@ -102,16 +111,16 @@ void falcon_FFT(fpr* f, unsigned logn)
float64x2_t vsr = vdupq_n_f64(falcon_neon_d(s_re));
float64x2_t vsi = vdupq_n_f64(falcon_neon_d(s_im));
for (j = j1; j < j2; j += 2) {
float64x2_t xr = vld1q_f64(fd + j);
float64x2_t xi = vld1q_f64(fd + j + hn);
float64x2_t yr = vld1q_f64(fd + j + ht);
float64x2_t yi = vld1q_f64(fd + j + ht + hn);
float64x2_t xr = FALCON_VLD(f + j);
float64x2_t xi = FALCON_VLD(f + j + hn);
float64x2_t yr = FALCON_VLD(f + j + ht);
float64x2_t yi = FALCON_VLD(f + j + ht + hn);
float64x2_t tr, ti;
FALCON_VCMUL(tr, ti, yr, yi, vsr, vsi);
vst1q_f64(fd + j, vaddq_f64(xr, tr));
vst1q_f64(fd + j + hn, vaddq_f64(xi, ti));
vst1q_f64(fd + j + ht, vsubq_f64(xr, tr));
vst1q_f64(fd + j + ht + hn, vsubq_f64(xi, ti));
FALCON_VST(f + j, vaddq_f64(xr, tr));
FALCON_VST(f + j + hn, vaddq_f64(xi, ti));
FALCON_VST(f + j + ht, vsubq_f64(xr, tr));
FALCON_VST(f + j + ht + hn, vsubq_f64(xi, ti));
}
}
else {
@@ -135,7 +144,6 @@ void falcon_FFT(fpr* f, unsigned logn)
void falcon_iFFT(fpr* f, unsigned logn)
{
double* fd = (double*)f;
int u;
size_t n = (size_t)1 << logn, hn = n >> 1;
@@ -151,18 +159,18 @@ void falcon_iFFT(fpr* f, unsigned logn)
float64x2_t vsr = vdupq_n_f64(falcon_neon_d(s_re));
float64x2_t vsi = vdupq_n_f64(falcon_neon_d(s_im));
for (j = j1; j < j2; j += 2) {
float64x2_t ar = vld1q_f64(fd + j);
float64x2_t ai = vld1q_f64(fd + j + hn);
float64x2_t br = vld1q_f64(fd + j + t);
float64x2_t bi = vld1q_f64(fd + j + t + hn);
float64x2_t ar = FALCON_VLD(f + j);
float64x2_t ai = FALCON_VLD(f + j + hn);
float64x2_t br = FALCON_VLD(f + j + t);
float64x2_t bi = FALCON_VLD(f + j + t + hn);
float64x2_t dr = vsubq_f64(ar, br);
float64x2_t di = vsubq_f64(ai, bi);
float64x2_t pr, pi;
vst1q_f64(fd + j, vaddq_f64(ar, br));
vst1q_f64(fd + j + hn, vaddq_f64(ai, bi));
FALCON_VST(f + j, vaddq_f64(ar, br));
FALCON_VST(f + j + hn, vaddq_f64(ai, bi));
FALCON_VCMUL(pr, pi, dr, di, vsr, vsi);
vst1q_f64(fd + j + t, pr);
vst1q_f64(fd + j + t + hn, pi);
FALCON_VST(f + j + t, pr);
FALCON_VST(f + j + t + hn, pi);
}
}
else {
@@ -184,7 +192,7 @@ void falcon_iFFT(fpr* f, unsigned logn)
float64x2_t vni = vdupq_n_f64(falcon_neon_d(ni));
size_t j;
for (j = 0; j < n; j += 2) {
vst1q_f64(fd + j, vmulq_f64(vld1q_f64(fd + j), vni));
FALCON_VST(f + j, vmulq_f64(FALCON_VLD(f + j), vni));
}
}
else {

View File

@@ -121,6 +121,12 @@ static int falcon_prng_refill(falcon_prng* p)
int ret = wc_Shake256_SqueezeBlocks(&p->shake, p->buf, FALCON_PRNG_BLOCKS);
p->ptr = 0;
p->len = (ret == 0) ? (word32)FALCON_PRNG_BUFLEN : 0;
/* Latch the first failure. get_u8/get_u64 have no error return, so a squeeze
* failure is made sticky here and checked by the signer (falcon_sign_core),
* which rejects any signature produced from an invalid PRNG state instead of
* consuming stale buffer bytes. */
if (ret != 0 && p->err == 0)
p->err = ret;
return ret;
}
@@ -140,6 +146,7 @@ int falcon_prng_init(falcon_prng* p, WC_RNG* rng)
p->ptr = 0;
p->len = 0;
p->err = 0;
ForceZero(seed, (word32)sizeof(seed));
if (ret == 0)
@@ -152,6 +159,9 @@ byte falcon_prng_get_u8(falcon_prng* p)
{
byte v;
/* On a refill failure len becomes 0 and p->err is latched; the buffer read
* below stays in bounds (ptr reset to 0) but yields a discarded value --
* falcon_sign_core checks p->err and rejects the resulting signature. */
if (p->ptr + 1U > p->len)
(void)falcon_prng_refill(p);
v = p->buf[p->ptr];

View File

@@ -44,6 +44,15 @@
#define FALCON_Q 12289
/* Safety bound on the sign-tree restart loop. The per-iteration restart
* probability is well below 1 (expected iteration count ~1), so the chance of
* legitimately reaching even a few dozen restarts is astronomically small; this
* bound is never approached in normal operation. It only guarantees termination
* if the sampler can no longer produce accepting candidates (e.g. a failed PRNG
* squeeze), in which case falcon_sign_core additionally rejects the result via
* the PRNG's sticky error. */
#define FALCON_SIGN_MAX_RESTARTS 4096UL
/* IEEE-754 binary64 bit patterns (the fpr seam carries doubles as word64).
* These mirror named constants from the reference fpr.h that are not part of
* the public wc_falcon_fpr.h API. fpr_invsqrt2 / fpr_invsqrt8 ARE exported by
@@ -656,27 +665,44 @@ int falcon_do_sign_tree(falcon_samplerZ samp, void* samp_ctx, sword16* s2,
}
/* Loop until the candidate (s1, s2) is short enough. With degrees 512 and
* 1024 a restart is very rare. */
for (;;) {
if (do_sign_tree_once(samp, samp_ctx, s2, expanded, hm, logn, tmp)) {
return 0;
}
* 1024 a restart is very rare (expected iteration count is ~1), so the
* bound below is astronomically beyond any legitimate run; it exists only
* so a wedged sampler -- e.g. a PRNG whose SHAKE256 squeeze started failing,
* yielding candidates that never pass the norm bound -- terminates instead
* of spinning forever. */
{
unsigned long iter;
for (iter = 0; iter < FALCON_SIGN_MAX_RESTARTS; iter++) {
if (do_sign_tree_once(samp, samp_ctx, s2, expanded, hm, logn, tmp)) {
return 0;
}
#ifdef WOLFSSL_FALCON_SIGN_STATS
/* Optional instrumentation for test harnesses: counts the rare
* ffSampling restarts. Not compiled into production builds. */
extern unsigned long falcon_sign_restart_count;
falcon_sign_restart_count++;
/* Optional instrumentation for test harnesses: counts the rare
* ffSampling restarts. Not compiled into production builds. */
extern unsigned long falcon_sign_restart_count;
falcon_sign_restart_count++;
#endif
}
}
/* Exhausted the restart bound: treat as an operational failure. */
return WC_FAILURE;
}
int falcon_sign_core(falcon_sampler_ctx* spc, const fpr* expanded,
const word16* c, sword16* s2, fpr* tmp, unsigned logn)
{
int ret;
if (spc == NULL) {
return BAD_FUNC_ARG;
}
return falcon_do_sign_tree(falcon_sampler_z, spc, s2, expanded, c, logn, tmp);
ret = falcon_do_sign_tree(falcon_sampler_z, spc, s2, expanded, c, logn, tmp);
/* Reject the signature if the sampler's PRNG failed at any point: the
* squeezed bytes it consumed would be invalid, so the result is unsafe. */
if (ret == 0 && spc->p.err != 0) {
ret = spc->p.err;
}
return ret;
}
#endif /* HAVE_FALCON && !WOLFSSL_FALCON_VERIFY_ONLY */

View File

@@ -253,3 +253,17 @@ if BUILD_RISCV_ASM
nobase_include_HEADERS+= wolfssl/wolfcrypt/port/riscv/riscv-64-asm.h
endif
if BUILD_FALCON
# Internal Falcon headers (the public API is wolfssl/wolfcrypt/falcon.h, listed
# unconditionally above). Automake distributes both branches of a conditional,
# so these are still included in "make dist" when Falcon is disabled.
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_bigint.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_codec.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_fft.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_fpr.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_keygen.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_poly.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_sampler.h
nobase_include_HEADERS+= wolfssl/wolfcrypt/wc_falcon_sign.h
endif

View File

@@ -70,6 +70,7 @@ typedef struct falcon_prng {
byte buf[FALCON_PRNG_BUFLEN];/* squeezed stream buffer */
word32 ptr; /* index of next byte to consume */
word32 len; /* number of valid bytes in buf */
int err; /* sticky: first refill error, or 0 */
} falcon_prng;
/* Sampler context: the PRNG plus the parameter-set-dependent sigma_min. */