Rust mlkem Fenrir fixes 2026-08-27 - #11302
Conversation
|
Can one of the admins verify this patch? |
|
retest this please (Calling Pipeline was cancelled) |
|
retest this please (build removed) |
1 similar comment
|
retest this please (build removed) |
philljj
left a comment
There was a problem hiding this comment.
All looks good except for one define guard question in wolfcrypt test.c
7e25ece to
9547114
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11302
Scan targets checked: wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfcrypt-rs-src, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 6
5 finding(s) posted as inline comments (see file-level comments below)
Required changes (1)
test_sizes_match_runtime exercises all three parameter sets without level gates
File: wrapper/rust/wolfssl-wolfcrypt/tests/test_mlkem_kem.rs:37
Function: test_sizes_match_runtime
Category: Missing/incorrect platform conditionals
MlKem::new() returns Err when wc_MlKemKey_Init reports NOT_COMPILED_IN for a disabled level (wc_mlkem.c:390-408), so the .expect("new TYPE_512") calls panic when WOLFSSL_NO_ML_KEM_512/768/1024 is set. The PR added mlkem_512/768/1024 cfgs but did not apply them here.
Recommendation: Split this test per parameter set and gate each part on the matching mlkem_512/mlkem_768/mlkem_1024 cfg.
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| @@ -52,6 +52,7 @@ fn test_sizes_match_runtime() { | |||
|
|
|||
| /// Generate, encapsulate, and decapsulate with ML-KEM-512 via the kem traits. | |||
| #[test] | |||
There was a problem hiding this comment.
test_ek_try_new_zeroed_key missing #[cfg(mlkem_768)] gate · Copy-paste errors
MlKem768EncapsulationKey is now generated only under #[cfg(mlkem_768)] in mlkem_kem.rs, but this test references it without a gate while every sibling 768 test received one. Builds with ML-KEM-768 disabled fail to compile the test crate (E0433).
Fix: Add #[cfg(mlkem_768)] to test_ek_try_new_zeroed_key, matching the other ML-KEM-768 tests in the file.
| */ | ||
|
|
||
| #![cfg(all(mlkem, random, feature = "kem", feature = "rand_core"))] | ||
| #![cfg(all(mlkem_encapsulate, mlkem_decapsulate, mlkem_make_key, random, feature = "kem", feature = "rand_core"))] |
There was a problem hiding this comment.
test_mlkem_kem.rs module gate drops the mlkem cfg, breaking WC_NO_CONSTRUCTORS builds · Missing/incorrect platform conditionals
The gate was changed from mlkem to the three operation cfgs. Under WC_NO_CONSTRUCTORS the operation cfgs are still set while mlkem is off, so this file compiles but wolfssl_wolfcrypt::mlkem::MlKem (empty module) and wolfssl_wolfcrypt::mlkem_kem (not declared in lib.rs:72) fail to resolve. Adjacent to known finding #11270, which covers scan_cfg in build.rs, not this test gate.
Fix: Add mlkem back into the all(...) list in the module-level cfg.
| /// arrive at the same shared secret. | ||
| #[test] | ||
| #[cfg(random)] | ||
| #[cfg(all(mlkem_make_key, mlkem_encapsulate, mlkem_decapsulate, random))] |
There was a problem hiding this comment.
test_mlkem.rs level-specific tests are not gated on the new per-level cfgs · Missing/incorrect platform conditionals
test_encap_decap_type512, test_encap_decap_type768 (:114), test_encap_decap_type1024 (:148) and the TYPE_768 tests at :173/:202/:237/:271 received operation cfgs but no mlkem_512/mlkem_768/mlkem_1024 gate, so they panic on NOT_COMPILED_IN when that parameter set is disabled. test_encap_decap_all_types (:306) iterates all three levels unconditionally.
Fix: Add the matching mlkem_512/mlkem_768/mlkem_1024 cfg to each level-specific test and gate the per-type loop iterations.
|
|
||
| ```rust | ||
| #[cfg(all(mlkem, random))] | ||
| #[cfg(all(mlkem_make_key, mlkem_encapsulate, mlkem_decapsulate, random))] |
There was a problem hiding this comment.
mlkem.rs doc examples use TYPE_768 without an mlkem_768 cfg · Missing/incorrect platform conditionals
The doctests updated by this PR (lines 41, 116, 148, 192, 224, 457, 523, 594, 650, 689, 726, 765) all call MlKem::…(MlKem::TYPE_768, …).expect(…) but gate only on operation cfgs, so they panic under WOLFSSL_NO_ML_KEM_768. The equivalent example in mlkem_kem.rs:46 does include mlkem_768.
Fix: Add mlkem_768 to the cfg list of every mlkem.rs doc example that constructs a TYPE_768 key.
| check_cfg(&binding, "wc_MlKemKey_MakeKey", "mlkem_make_key"); | ||
| check_cfg(&binding, "wc_MlKemKey_Encapsulate", "mlkem_encapsulate"); | ||
| check_cfg(&binding, "wc_MlKemKey_Decapsulate", "mlkem_decapsulate"); | ||
| check_cfg(&binding, "WC_ML_KEM_512_K", "mlkem_512"); |
There was a problem hiding this comment.
New mlkem_512/768/1024 cfgs are set in --enable-mlkem=original builds where the ML-KEM type values are rejected · Incorrect feature flag gating
--enable-mlkem=original sets WOLFSSL_MLKEM_KYBER plus WOLFSSL_NO_ML_KEM, which still defines WOLFSSL_WC_ML_KEM_512/768/1024 and therefore WC_ML_KEM_*_K (wc_mlkem.h:78-90, 106). All three cfgs get set, but wc_MlKemKey_Init has no WC_ML_KEM_* cases under WOLFSSL_NO_ML_KEM (wc_mlkem.c:390-434) and returns BAD_FUNC_ARG, so the gated impl_mlkem_kem! adapters still panic in MlKem::new(...).expect(...). Adjacent to known finding #12238, which the per-level cfgs were added to fix; this build configuration is not covered.
Related known finding #11270 (similar but distinct): Both are in scan_cfg ML-KEM gating and expose wrapper APIs from insufficient probes, but #11270 gates the whole module on wc_MlKemKey_Init under WC_NO_CONSTRUCTORS, where New/Delete are absent; this candidate gates per-level adapters on constants that remain defined under WOLFSSL_NO_ML_KEM, where Init rejects those types. The faulting operations, configurations, root causes, and required probes/patches differ.
Fix: Probe a symbol that is absent under WOLFSSL_NO_ML_KEM (e.g. WC_ML_KEM_512_PUBLIC_KEY_SIZE gated additionally on ML-KEM being enabled) so Kyber-original builds do not set the per-level cfgs.
Description
Testing
How did you test?
Checklist