Repository navigation
Conversation
|
There was a problem hiding this comment.
🟡 Changes recommended
With WC_RNG_SEED_CT_BARRIER enabled, the accepted cutoff 0xffffffffU causes the constant-time comparison to incorrectly reject non-repeating seeds with ENTROPY_RT_E.
1 open issue
What changed in this PR
This PR hardens wolfCrypt’s constant-time code against compiler-introduced branches in KEM decapsulation and RNG seed health tests.
Changes:
- Adds the
ctHide32()optimizer value barrier. - Applies barriers to ML-KEM and FrodoKEM ciphertext-validity masks.
- Reworks RNG health-test loops with target-dependent barriers and a shorter majority-vote dependency chain.
| File | Description |
|---|---|
wolfssl/wolfcrypt/misc.h |
Declares ctHide32(). |
wolfcrypt/src/wc_mlkem_poly.c |
Protects the ciphertext-comparison mask with barriers. |
wolfcrypt/src/wc_frodokem.c |
Protects implicit-rejection mask generation with barriers. |
wolfcrypt/src/random.c |
Adds conditional barriers and rewrites seed health-test arithmetic. |
wolfcrypt/src/misc.c |
Implements the value barrier with assembly and volatile fallbacks. |
Review effort: Balanced
60595ce to
8dc107f
Compare
|
@claude review |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It changes constant-time code in the RNG seed health tests and in ML-KEM/FrodoKEM implicit rejection, so a human crypto reviewer should still look at it.
What was reviewed:
ctHide32in misc.c/misc.h: the inline-asm guard, the volatile fallback, and that the NO_INLINE prototype matches.- The
wc_RNG_TestSeedrewrite. The RCT mask select still gives the same result now thatprevByteis always updated.votes + (same<<1) - 1matches the old vote update, including whentakeis set. The borrow formula inRngSeedCtGTEis correct across the full word32 range. - The
isEq/mlkem_cmp_cbarrier wrapping. The inputs stay below 2^31, so the>> 31formulas are still exact. - Not done: no check of the generated assembly on Thumb-1, RISC-V, MSP430 or AVR. Whether the compiler still emits branches can only be confirmed by compiling for those targets.
Extended reasoning...
The change touches wolfCrypt constant-time code: misc.c/misc.h, the seed health tests in random.c, and the ML-KEM and FrodoKEM decapsulation comparisons. This is security-sensitive crypto code where the side-channel properties matter. The diff is small (about 127 lines) and reading it found no logic errors. Whether it works depends on the code the compiler generates for specific embedded targets, which reading the source cannot confirm. Because it is crypto hardening code, a human review is warranted rather than an automated approval.
There was a problem hiding this comment.
🟡 Changes recommended
The RNG health-test changes truncate accepted repetition-count cutoffs and mishandle large APT vote counts, causing false seed rejections or missed APT failures.
2 open issues
1 resolved since last review
Review effort: Balanced
misc.c: add ctHide32(), a value barrier that hides a value from the optimizer so mask formulas cannot be turned back into branches. ML-KEM (mlkem_cmp_c) and FrodoKEM decapsulation: use ctHide32() on the ciphertext-comparison result so implicit rejection does not branch on ciphertext validity. wc_RNG_TestSeed(): rewrite the RCT/APT loops branch-free. Barriers are enabled via WC_RNG_SEED_CT_BARRIER, defined by default on affected targets; other targets use plain comparisons. Also shorten the majority-vote dependency chain.
8dc107f to
84a4b03
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Constant-time guarantees require cryptography-maintainer validation of generated assembly across supported targets and compiler configurations, including volatile fallback and barrier-disabled paths, but those artifacts are unavailable.
No open issues 🎉
2 resolved since last review
Review effort: Balanced

Description
misc.c: add ctHide32(), a value barrier that hides a value from the optimizer so mask formulas cannot be turned back into branches.
ML-KEM (mlkem_cmp_c) and FrodoKEM decapsulation: use ctHide32() on the ciphertext-comparison result so implicit rejection does not branch on ciphertext validity.
wc_RNG_TestSeed(): rewrite the RCT/APT loops branch-free. Barriers are enabled via WC_RNG_SEED_CT_BARRIER, defined by default on affected targets; other targets use plain comparisons. Also shorten the majority-vote dependency chain.
Fixes zd#22785, zd#22784, zd#22772
Testing
Checking of assembly output for different platforms.