Skip to content

ML-KEM: harden constant-time cmov and add cross-implementation test coverage#40

Merged
notABot101010 merged 1 commit into
mainfrom
copilot/perform-code-review-ml-kem
Jun 3, 2026
Merged

ML-KEM: harden constant-time cmov and add cross-implementation test coverage#40
notABot101010 merged 1 commit into
mainfrom
copilot/perform-code-review-ml-kem

Conversation

Copilot AI commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Code review of ML-KEM (FIPS 203) implementation for spec conformance, side-channel resistance, and test coverage.

Side-channel fix

The cmov function converted a bool to a mask without a compiler barrier, allowing the optimizer to potentially convert the branchless XOR pattern into a conditional branch—leaking timing in the FO transform's implicit rejection path.

fn cmov(out: &mut [u8; 32], value: &[u8; 32], cond: bool) {
    let mask = ct_mask_u8(cond); // now goes through asm barrier
    for i in 0..32 {
        out[i] ^= mask & (out[i] ^ value[i]);
    }
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
fn ct_barrier_u8(mut value: u8) -> u8 {
    unsafe { core::arch::asm!("/* {0} */", inout(reg_byte) value, options(pure, nomem, nostack, preserves_flags)); }
    value
}

Same technique used by the constant_time_eq crate in this repo. Platform coverage: x86/x86_64, ARM/AArch64, RISC-V, with black_box fallback.

Conformance review (no issues found)

  • G(d || k) keygen domain separation correct
  • v - s^T·u decryption order correct (Algorithm 6 step 3)
  • FO implicit rejection correct (Algorithm 17)
  • All compression uses division-free Barrett multiplication constants
  • Rejection sampling only branches on public data (matrix A)

New tests (34 → 53)

  • Cross-implementation: 10 ML-KEM-768 + 10 ML-KEM-1024 vectors generated by pqcrypto/liboqs, decapsulated by our implementation
  • NIST FIPS 203: Full intermediate value verification (keygen + encaps + decaps) for both parameter sets
  • Internal primitives: cmov, Barrett reduce, Montgomery reduce, NTT structure, compress/decompress roundtrips (4/5/10/11-bit), poly_tobytes roundtrip, gen_matrix transpose, CBD2 range, rej_uniform rejection, poly_tomsg boundaries

…mprehensive tests

- Add ct_barrier_u8 using inline asm (x86/arm/riscv) to prevent optimizer
  from converting cmov into a branch (side-channel protection)
- Add NIST FIPS 203 intermediate value tests (full keygen/encaps/decaps)
- Add cross-implementation tests using pqcrypto (liboqs) generated vectors
- Add unit tests for: cmov, barrett_reduce, montgomery_reduce, NTT,
  compression/decompression roundtrips, CBD2, rej_uniform, poly_tomsg boundaries
- Add gen_matrix transpose relationship verification
- Total: 53 ML-KEM tests passing (up from 34)
Copilot AI changed the title ML-KEM: code review improvements - strengthen constant-time ops and add comprehensive tests ML-KEM: harden constant-time cmov and add cross-implementation test coverage Jun 3, 2026
Copilot AI requested a review from notABot101010 June 3, 2026 16:09
@notABot101010 notABot101010 marked this pull request as ready for review June 3, 2026 16:14
@notABot101010 notABot101010 merged commit a944890 into main Jun 3, 2026
1 check passed
@notABot101010 notABot101010 deleted the copilot/perform-code-review-ml-kem branch June 5, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants