sp x86_64: separate lane selection from vector-register ownership - #11422
kaleb-himes wants to merge 50 commits into
Conversation
c046941 to
aea72e2
Compare
|
aea72e2 to
7f0414d
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11422
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Findings: 4
4 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
7f0414d to
c086ede
Compare
c086ede to
59dd1c3
Compare
|
retest this please |
Dismiss to re-request
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11422
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Findings: 1
Required changes (1)
Small-memory signing configuration no longer compiles
File: wolfcrypt/src/wc_mldsa.c:10447
Function: mldsa_sign_with_seed_mu
Category: API contract violations
mldsa_sign_with_seed_mu() calls the new four-argument mldsa_vec_check_low() with two arguments under WOLFSSL_MLDSA_SIGN_SMALL_MEM and WOLFSSL_MLDSA_SIGN_CHECK_W0. Known #6771 instead concerns signing conformance.
Related known finding #6771 (similar but distinct): Both concern optional early-rejection checks in mldsa_sign_with_seed_mu, but this is a mismatched helper-call signature causing a compile failure, while #6771 is a FIPS-conformance deviation caused by the checks themselves; their fixes differ.
Suggested fix: Pass the vector length and &valid, assigning the helper's return value to ret as at the other updated call sites.
Basis: ISO/IEC 9899:2018 §6.5.2.2 requires function-call arguments to agree with the visible prototype.
Referenced code: wolfcrypt/src/wc_mldsa.c:10447-10448 (2 lines)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
bee7a73 to
b1f293d
Compare
b1f293d to
75ae16b
Compare
Outdated review has been addressed and Fenrir is now disabled for reviews.
75ae16b to
830bc6d
Compare
830bc6d to
7bbfd5c
Compare
| /* A refused save falls through so the hash below is still freed. */ | ||
| ret = SAVE_VECTOR_REGISTERS2(); | ||
| if (ret == 0) { | ||
| ret = slhdsakey_fors_pk_from_sig_x4(key, sig_fors, indices, | ||
| pk_seed, adrs); | ||
| RESTORE_VECTOR_REGISTERS(); |


Description
Pairs with: https://github.com/wolfSSL/scripts/pull/677
CPUID alone picks the lane; the chosen lane saves the vector registers. This is a CPU step, not a fallback.
A refused save returns the error. The other lane never runs.
Every SP mulmod entry point saves for itself, both lanes (the non-AVX2 table lookups are SSE2). P-1024 base lane excepted, it has none.
FIPS v7 reads the CPU features once at power on. cpuid_select_flags / cpuid_set_flag / cpuid_clear_flag do nothing in a FIPS v7 build; --enable-fips=dev and dev-no-post keep the upstream behavior. A microcode update that changes CPU features needs a reboot (Linux Documentation/arch/x86/microcode.rst, late loading), which re-detects them.
ML-KEM: a decoded public or private key no longer keeps the matrix cached (WOLFSSL_MLKEM_CACHE_A) from the key it replaces.
ML-KEM: once a public or private key decode starts replacing the key, a failure leaves no key, so it cannot be used with a NULL or half-replaced buffer (previously a failed allocation under WOLFSSL_MLKEM_DYNAMIC_KEYS left the old flags set and the next encapsulate or decapsulate dereferenced NULL). A wrong-length input fails before anything changes and leaves the key as it was. Software decapsulation refuses a key without its public half before it decrypts anything (FIPS 203 Algorithm 18 re-encrypts with the public key).
The SP x86_64 white-box test now fails if a refused save still lets a call run, so a regression is caught in tree.
Behavior change: on AVX2-capable x86_64, SP RSA/ECC entry points can return the vector-register save error instead of silently running the non-AVX2 lane. Userspace never refuses the save.
Testing
40 test cells, all passing.
Builds and gates
suite on each.
proving the pre-change file still differs.
settings enable none of it, and with those settings it compiles to zero
symbols. Windows gets no code from this change.
Refused saves must fail, never switch lanes
in a non-FIPS build and again against the real FIPS v7 module. Every call
fails; results after the injection stops match those from before it, so
the fixed point cache is not left half built.
failing, a quarter alternating: 750 clean results and 450 refusals, every
clean result matching the reference. Non-FIPS and FIPS.
CPU features read once at power on
calling them leaves the flags, the lane and every self test state as they
were. Control: the same build with the v7 guard removed, where all three
setters write and the flags move.
leave the flags and the ECDSA self test state unchanged. Control on each:
the module with the v7 guard removed moves the flags.
ECDH, AES-XTS and RNG load, flags unchanged, no failures, and NMI repeated
8 times with no hang.
v7 module each one is a single return instruction. Control: the module with
the v7 guard removed, where all three do work.
ML-KEM key reuse
private key, must agree on the shared secret both ways. It fails without
the fix on a cache-a build (CI's pq-all.json builds cache-a).
decapsulate to be refused. Without the fix it returns 0. Coverage shows
the refused call no longer reaches the decryption step at all.
Real kernels
and 7.1.9: the FIPS module with the in-kernel crypto test, booted twice on
each, once with AVX2 visible and once with it hidden so the non-AVX2 lanes
run. Every boot passes the power-on self test, passes the in-kernel test,
re-verifies at unload and unloads clean, with no kernel warnings.
one reported the error, no crash and no kernel warning.
AMD EPYC and Intel Xeon, each running the FIPS test suite, the power-on
only check, the forced failure check, and the FIPS kernel module loading
with the power-on self test and the in-kernel test, then unloading clean.
shared by Intel, aarch64, 32-bit Arm and PowerPC, so the FIPS build with
the Arm assembly lanes runs the test suite and the power-on only check
there too.
Checklist