Skip to content

sp x86_64: separate lane selection from vector-register ownership - #11422

Open
kaleb-himes wants to merge 50 commits into
wolfSSL:masterfrom
kaleb-himes:PQ-FS-2026-Part3-SecurityReview-nofallback-M
Open

kaleb-himes wants to merge 50 commits into
wolfSSL:masterfrom
kaleb-himes:PQ-FS-2026-Part3-SecurityReview-nofallback-M

Conversation

@kaleb-himes

@kaleb-himes kaleb-himes commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

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

  • FIPS-ready, FIPS v7, and FIPS v7 with the SP assembly lanes on, full test
    suite on each.
  • A build with AVX2 compiled out entirely.
  • The fail-closed dev build under strict warnings.
  • sp_x86_64.c reproduces byte for byte from the generator, with a control
    proving the pre-change file still differs.
  • Windows: the whole file sits inside one feature guard, the Windows FIPS
    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

  • Forced save failures into ECC P-256, P-384, P-521 and RSA 2048 and 3072,
    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.
  • 8 threads at once on one shared cache entry, half clean, a quarter always
    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

  • Userspace, FIPS v7: the three setters compile to a bare return, and
    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.
  • Kernel 5.15.170, 6.16.12 and 7.1.9: the same calls from inside the module
    leave the flags and the ECDSA self test state unchanged. Control on each:
    the module with the v7 guard removed moves the flags.
  • Kernel 6.16.12, every context: process, softirq, hardirq and NMI, under
    ECDH, AES-XTS and RNG load, flags unchanged, no failures, and NMI repeated
    8 times with no hang.
  • Kernel 4.18.9: the module exports the setters to other modules, and in the
    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

  • New API test: a made key reused for a decoded public key, then a decoded
    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).
  • The same test decodes a bad public key into a full key and expects
    decapsulate to be refused. Without the fix it returns 0. Coverage shows
    the refused call no longer reaches the decryption step at all.
  • New API test under WOLFSSL_MLKEM_DYNAMIC_KEYS (CI's mlkem-dynamic-keys job): a decode into a full key with one buffer allocation failing must leave the key refusing encapsulate and decapsulate. The old code segfaults.

Real kernels

  • Local KVM on an Intel Core Ultra 9 285K, kernels 4.18.9, 5.15.170, 6.16.12
    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.
  • 6.16.12 with a save fuzzer: 8 loads, every one met a refused save, every
    one reported the error, no crash and no kernel warning.
  • AWS spot on real silicon against the stock Amazon Linux kernel 6.12.103:
    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.
  • AWS spot Graviton, aarch64: the cpuid change is in the part of cpuid.c
    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

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@kaleb-himes kaleb-himes self-assigned this Sep 9, 2026
@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from c046941 to aea72e2 Compare September 10, 2026 00:45
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text +128 B (+0.0%, 790,620 B / 1,048,576 B, total: 75% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +384 B (+0.1%, 308,656 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +448 B (+0.1%, 309,616 B / 1,048,576 B, total: 30% used)

linuxkm-standard

@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from aea72e2 to 7f0414d Compare September 11, 2026 18:47

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread wolfcrypt/src/cpuid.c Outdated
Comment thread wolfcrypt/src/wc_mldsa.c
Comment thread wolfcrypt/src/cpuid.c Outdated
Comment thread tests/unit-mcdc/test_sp_x86_64_whitebox.c
@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from 7f0414d to c086ede Compare September 11, 2026 21:31
@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from c086ede to 59dd1c3 Compare September 11, 2026 22:10
@kaleb-himes

Copy link
Copy Markdown
Contributor Author

retest this please

@kaleb-himes
kaleb-himes requested review from wolfSSL-Fenrir-bot and removed request for wolfSSL-Fenrir-bot September 13, 2026 16:50

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from bee7a73 to b1f293d Compare September 15, 2026 15:42
@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from b1f293d to 75ae16b Compare September 24, 2026 23:41
@kaleb-himes kaleb-himes added the For FIPS v7 Module Related to FIPS v7.0.0 module prep for submission label Sep 29, 2026
@kaleb-himes
kaleb-himes dismissed wolfSSL-Fenrir-bot’s stale review September 29, 2026 21:26

Outdated review has been addressed and Fenrir is now disabled for reviews.

Copilot AI balanced review requested due to automatic review settings October 1, 2026 23:15
@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from 75ae16b to 830bc6d Compare October 1, 2026 23:15

This comment was marked as resolved.

@kaleb-himes
kaleb-himes force-pushed the PQ-FS-2026-Part3-SecurityReview-nofallback-M branch from 830bc6d to 7bbfd5c Compare October 2, 2026 22:36
@kaleb-himes
kaleb-himes requested a balanced review from Copilot October 3, 2026 10:15

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Post-FORS vector-save failures can return partial signatures containing private-key-derived values without clearing them.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread wolfcrypt/src/wc_slhdsa.c
Comment on lines +8533 to +8538
/* 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();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For FIPS v7 Module Related to FIPS v7.0.0 module prep for submission

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants