Skip to content

Test ML-KEM mixed-NULL KEM requests - #458

Merged
padelsbach merged 1 commit into
wolfSSL:masterfrom
aidangarske:fix-mlkem-mixed-null
Aug 4, 2026
Merged

Test ML-KEM mixed-NULL KEM requests#458
padelsbach merged 1 commit into
wolfSSL:masterfrom
aidangarske:fix-mlkem-mixed-null

Conversation

@aidangarske

Copy link
Copy Markdown
Member

The provider already implements OpenSSL-compatible mixed-NULL handling in e44edc6.

Add EVP_PKEY encapsulate/decapsulate regression coverage to ensure mixed-NULL requests are rejected while valid size queries remain supported.

Copilot AI review requested due to automatic review settings August 3, 2026 21:52

Copilot AI 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.

🟡 Not ready to approve

The new test should better align with existing ML-KEM test patterns (coverage across levels and clearer diagnostics/comment intent) to reliably meet the stated regression goals.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds a new unit/regression test to validate OpenSSL-compatible handling of ML-KEM “mixed-NULL” KEM API usage, ensuring invalid mixed-NULL encapsulation output requests are rejected (and integrating that test into the unit test runner).

Changes:

  • Registers a new ML-KEM unit test (test_mlkem_mixed_null) in the unit test declarations and test-case list.
  • Adds test_mlkem_mixed_null() to exercise mixed-NULL EVP_PKEY_encapsulate() calls (and a NULL-ciphertext EVP_PKEY_decapsulate() call).
File summaries
File Description
test/unit.h Declares the new ML-KEM mixed-NULL test function.
test/unit.c Adds the new ML-KEM mixed-NULL test to the unit test execution list.
test/test_mlkem.c Implements the new regression test for mixed-NULL encapsulate/decapsulate calls.
Review details

Suppressed comments (2)

test/test_mlkem.c:364

  • Same as above: if mixed-NULL encapsulate (secret=NULL) unexpectedly succeeds, the test should emit a clear error message to simplify debugging.
    if (err == 0) {
        ctLen = sizeof(ct);
        secretLen = sizeof(secret);
        err = EVP_PKEY_encapsulate(ectx, ct, &ctLen, NULL,
            &secretLen) == 1;
    }

test/test_mlkem.c:372

  • If decapsulate unexpectedly succeeds with a NULL ciphertext pointer, adding a PRINT_ERR_MSG would make the failure clearer (consistent with other negative-path assertions in this file).
    if (err == 0) {
        err = EVP_PKEY_decapsulate(dctx, secret, &secretLen, NULL,
            ctLen) == 1;
    }
  • Files reviewed: 3/3 changed files
  • Comments generated: 3
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread test/test_mlkem.c Outdated
Comment thread test/test_mlkem.c Outdated
Comment thread test/test_mlkem.c Outdated
@aidangarske
aidangarske force-pushed the fix-mlkem-mixed-null branch from e1f6410 to 659a511 Compare August 3, 2026 21:58
@aidangarske
aidangarske requested a review from padelsbach August 3, 2026 22:06
@aidangarske
aidangarske force-pushed the fix-mlkem-mixed-null branch 2 times, most recently from cd2e083 to 5dd643d Compare August 3, 2026 22:18
Comment thread test/test_mlkem.c Outdated
Comment thread test/test_mlkem.c Outdated
@aidangarske
aidangarske force-pushed the fix-mlkem-mixed-null branch from 5dd643d to 8a7d413 Compare August 4, 2026 19:06
@aidangarske
aidangarske requested a review from padelsbach August 4, 2026 19:37

@padelsbach padelsbach 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.

looks good, will merge when CI wraps up

@padelsbach
padelsbach merged commit 625b651 into wolfSSL:master Aug 4, 2026
72 checks passed
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.

3 participants