Skip to content

wp_mlx_kmgmt: derive the classical public point when duplicating - #472

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_11549
Open

wp_mlx_kmgmt: derive the classical public point when duplicating#472
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_11549

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

wp_mlx_dup() duplicates the public half first, then imports the classical
private key with wc_ecc_import_private_key_ex(..., pub=NULL, ...). That path
sets key->type = ECC_PRIVATEKEY_ONLY, so every later wc_ecc_export_x963()
returns ECC_PRIVATEONLY_E while dst->hasPub stays 1.

EVP_PKEY_dup() copies with OSSL_KEYMGMT_SELECT_ALL — exactly this
combination. Duplicating a SecP256r1MLKEM768 or SecP384r1MLKEM1024 key pair
yields a key whose public half cannot be read: OSSL_PKEY_PARAM_PUB_KEY and
ENCODED_PUBLIC_KEY fail, wp_mlx_export() fails before its callback (taking
the private half down with it), and EVP_PKEY_eq() fails on a key pair
selection. X25519MLKEM768 is unaffected — wc_curve25519_import_private_ex()
never touches pubSet.

Failures are loud error returns; no memory-safety or key-material exposure.
libssl does not dup key-share keys, so TLS 1.3 hybrid key exchange is not on the
failing path.

Fix (src/wp_mlx_kmgmt.c)

Derive the public point after the private import, matching what
wp_mlx_load_keys() already does for the same hazard:

rc = wc_ecc_import_private_key_ex(cPrivBuf, len, NULL, 0,
    &dst->classical.ecc, src->data->curveId);
if (rc == 0) {
    rc = wc_ecc_make_pub(&dst->classical.ecc, NULL);
}

Closes f_11549.

Tests (test/test_mlkem.c)

test_mlx_dup — the hybrid groups had no dup coverage at all; test_mlkem_dup
exercises only the pure ML-KEM levels. For each of the three hybrid names it
dups the key, compares public key bytes against the original, encapsulates
against the duplicate, and decapsulates that ciphertext with both keys.

Verification

  • Negative control: fails on SecP256r1MLKEM768 without the fix
    (Dup public key retrieval failed); X25519MLKEM768 passes.
  • With the fix: all three groups pass; full unit suite 212/212, 0 failures.
  • Build clean under -Werror (OpenSSL 3.6.0, wolfSSL master, --enable-mlkem).

- wp_mlx_dup() calls wc_ecc_make_pub() on the duplicate's ECC key
  once wc_ecc_import_private_key_ex() succeeds.
- test_mlx_dup covers EVP_PKEY_dup() over the hybrid groups: it
  compares the OSSL_PKEY_PARAM_PUB_KEY bytes of the duplicate and
  the original, encapsulates against the duplicate, and decapsulates
  that ciphertext with both keys.
- test/unit.h declares test_mlx_dup and test/unit.c registers it
  after test_mlx_encap_decap.

Issue: F-11549
Copilot AI lite review requested due to automatic review settings August 26, 2026 04:12
@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 26, 2026

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.

Pull request overview

This PR fixes hybrid ML-KEM + classical key duplication so that duplicated keypairs retain a usable classical public component (matching OpenSSL’s EVP_PKEY_dup() behavior) and adds unit coverage to prevent regressions.

Changes:

  • Derive the ECC public point after importing an ECC private key during wp_mlx_dup() to avoid ending up with an ECC_PRIVATEKEY_ONLY state that breaks public export.
  • Add a new hybrid-group duplication roundtrip test that validates public-key retrieval and encapsulate/decapsulate behavior across original and duplicated keys.
  • Register the new test in the unit test declarations and test case list.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/wp_mlx_kmgmt.c Ensures duplicated ECC hybrids derive a classical public point after private import so public export works.
test/test_mlkem.c Adds test_mlx_dup covering EVP_PKEY_dup() for hybrid groups, including pub-key byte equality and KEM roundtrips.
test/unit.h Declares the new test_mlx_dup unit test.
test/unit.c Registers test_mlx_dup in the unit test table.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@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 #472

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

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.

4 participants