Skip to content

kmgmt: add encoder import-object wrappers - #475

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

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

Conversation

@yosuke-wolfssl

@yosuke-wolfssl yosuke-wolfssl commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Problem

All 72 encoder dispatch tables registered the key-management import as
OSSL_FUNC_ENCODER_IMPORT_OBJECT. OpenSSL passes the encoder context, not a key,
so the import writes past the context, and its int 1/0 return is used as the
key pointer and later freed. OpenSSL calls this slot only for a key whose
provider has no encoders, such as OpenSSL's FIPS provider.

Fix (src/wp_*_kmgmt.c)

A correctly typed wp_<alg>_import_object() for RSA, EC, ECX, DH and ML-DSA
allocates a key from the encoder context, imports into it, and returns it, or
NULL on failure. Closes f_11552.

Tests

test_encoder_import_object() calls IMPORT_OBJECT with OpenSSL's real inputs
(EVP_PKEY_todata() output, EVP_PKEY_PUBLIC_KEY or EVP_PKEY_KEYPAIR) and
checks every key value comes back. test_encoder_import_object_rejected()
expects NULL for keys wolfProvider cannot hold: a 16384-bit RSA modulus, EC
sect233k1, DH modp_1536, an invalid ED25519 point.

Verification

  • 237 passed, 0 failed (OpenSSL 3.0.17); 194 passed, 0 failed (FIPS v5.2.4,
    OpenSSL 3.5.4).
  • ASan + UBSan clean on Linux.
  • With the fix reverted, the DH and ECX tests segfault and the RSA and EC tests fail.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 26, 2026
Copilot AI lite review requested due to automatic review settings August 26, 2026 07:25

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

Fixes wolfProvider encoder dispatch tables to implement OSSL_FUNC_ENCODER_IMPORT_OBJECT with the correct “import-object” semantics (receive encoder ctx, allocate and return a new provider key object), preventing memory corruption and invalid frees when encoding keys owned by a different provider in multi-provider OSSL_LIB_CTX scenarios.

Changes:

  • Add per-algorithm *_import_object() wrappers that allocate a new key object and import parameters into it, and register them in encoder dispatch tables.
  • Refactor ECX key allocation into wp_ecx_new_by_type() so decode and import-object share the same key-type selection logic.
  • Add unit tests that directly drive NEWCTX + IMPORT_OBJECT + FREE_OBJECT + FREECTX and validate imported key material via keymgmt export.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/unit.h Declares new import-object contract test helpers and per-alg test entrypoints.
test/unit.c Registers new import-object tests in the unit test table.
test/test_pkey.c Implements generic provider dispatch-table probing + import-object contract test harness.
test/test_rsa.c Adds RSA/RSA-PSS coverage for encoder import-object behavior.
test/test_ecc.c Adds EC encoder import-object test (P-256 public+params).
test/test_ecx.c Adds ECX import-object coverage across X25519/ED25519/X448/ED448 variants.
test/test_dh.c Adds DH import-object test using named group parameters.
test/test_mldsa.c Adds ML-DSA-44 import-object test coverage.
src/wp_rsa_kmgmt.c Adds wp_rsa_import_object() and wires it into all RSA encoder dispatch tables.
src/wp_ecc_kmgmt.c Adds wp_ecc_import_object() and wires it into ECC encoder dispatch tables.
src/wp_ecx_kmgmt.c Adds wp_ecx_new_by_type() + wp_ecx_import_object() and updates ECX encoder dispatch tables.
src/wp_dh_kmgmt.c Adds wp_dh_import_object() and wires it into DH encoder dispatch tables.
src/wp_mldsa_kmgmt.c Adds wp_mldsa_import_object() and wires it into ML-DSA encoder dispatch tables (macro-generated).

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

Comment thread test/test_pkey.c
Comment thread test/test_pkey.c Outdated
Comment thread test/test_dh.c Outdated

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

Scan targets checked: wolfprovider-bugs
Failed targets: wolfprovider-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

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

Scan targets checked: wolfprovider-bugs, wolfprovider-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Comment thread test/unit.c Outdated
@yosuke-wolfssl
yosuke-wolfssl force-pushed the fix/f_11552 branch 2 times, most recently from 81db920 to a1a7295 Compare August 26, 2026 23:44
Comment thread test/unit.c Outdated

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

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.

@wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot dismissed their stale review August 27, 2026 00:07

Fenrir's latest completed scan found no issues; clearing the prior automated change request.

@ColtonWilley ColtonWilley added the ci:all PR OSP toggle: run all label Sep 29, 2026
@ColtonWilley ColtonWilley added ci:all PR OSP toggle: run all and removed ci:all PR OSP toggle: run all labels Oct 1, 2026
- wp_rsa_import_object(), wp_ecc_import_object(),
  wp_ecx_import_object(), wp_dh_import_object() and
  wp_mldsa_import_object() build a key from the encoder context's
  provider context via the key-management import, returning NULL on
  failure. The encoder dispatch tables register them as
  OSSL_FUNC_ENCODER_IMPORT_OBJECT.
- wp_ecx_new_by_type() creates the ECX key for a key type;
  wp_ecx_decode() uses it.
- test_encoder_import_object() in test_pkey.c runs an encoder's
  IMPORT_OBJECT over EVP_PKEY_todata() output and checks the
  re-exported key returns every key-material value;
  test_encoder_import_object_rejected() expects NULL.
- RSA, RSA-PSS, EC, ECX, DH and ML-DSA tests drive both with
  default-provider keys and EVP_PKEY_PUBLIC_KEY or EVP_PKEY_KEYPAIR.

Issue: F-11552
@yosuke-wolfssl yosuke-wolfssl added ci:all PR OSP toggle: run all and removed ci:all PR OSP toggle: run all labels Oct 2, 2026
@yosuke-wolfssl yosuke-wolfssl self-assigned this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:all PR OSP toggle: run all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants