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

kmgmt: add encoder import-object wrappers#475
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_11552

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

The encoder OSSL_FUNC_ENCODER_IMPORT_OBJECT slot must be
void *(void *encoder_ctx, int selection, const OSSL_PARAM[]): it receives the
context from OSSL_FUNC_ENCODER_NEWCTX and returns a newly allocated key. All 72
encoder dispatch tables registered the key-management import instead, which is
int (wp_<Alg> *key, int, const OSSL_PARAM[]). The DFUNC cast hides both
mismatches:

  • Arg 1 — OpenSSL passes a wp_<Alg>EncDecCtx * (7 scalar fields) where a much
    larger key struct is expected, so the import writes key material past its end.
  • Return — the int 1/0 is read as the object pointer, so encoder_encode()
    dereferences it and encoder_destruct_pkey() frees it.

import_object is called only when the key's keymgmt provider differs from the
encoder's, i.e. OSSL_ENCODER_CTX_new_for_pkey() on a foreign key in a
multi-provider OSSL_LIB_CTX. Registering the slot is what makes wolfProvider's
encoders eligible for that path — collect_encoder() skips a foreign encoder whose
import_object is NULL. The sharpest case is a key from OpenSSL's FIPS provider,
which ships no encoders of its own. Not reachable in single-provider or
replace-default deployments.

Fix (src/wp_*_kmgmt.c)

A correctly typed wp_<alg>_import_object() per algorithm, mirroring OpenSSL's own
ossl_prov_import_key() and the existing wp_<alg>_export_object() wrappers:

static wp_Ecc* wp_ecc_import_object(wp_EccEncDecCtx* ctx, int selection,
    const OSSL_PARAM params[])
{
    wp_Ecc* ecc = wp_ecc_new(ctx->provCtx);

    if ((ecc != NULL) && (!wp_ecc_import(ecc, selection, params))) {
        wp_ecc_free(ecc);
        ecc = NULL;
    }
    return ecc;
}
File Tables Key allocated with
wp_ecx_kmgmt.c 24 wp_ecx_new_by_type(ctx->provCtx, ctx->keyType, NULL)
wp_mldsa_kmgmt.c 18 ctx->newKey(ctx->provCtx)
wp_rsa_kmgmt.c 12 wp_rsa_base_new(ctx->provCtx, ctx->type)
wp_ecc_kmgmt.c 10 wp_ecc_new(ctx->provCtx)
wp_dh_kmgmt.c 8 wp_dh_new(ctx->provCtx)

OSSL_FUNC_ENCODER_FREE_OBJECT (wp_<alg>_free) is already shaped for a real key
object and is unchanged. wp_ecx_new_by_type() factors the key-type chain out of
wp_ecx_decode() so both callers share it.

Closes f_11552.

Tests

test_encoder_import_object() in test/test_pkey.c drives the dispatch entry
directly, so it does not depend on which encoder OpenSSL would pick: it pulls the
encoder and keymgmt tables from the provider's operation query, runs NEWCTX and
IMPORT_OBJECT over parameters from EVP_PKEY_todata() on a default-provider key,
checks the returned object against the keymgmt export, then calls FREE_OBJECT and
FREECTX. Per-algorithm drivers cover RSA and RSA-PSS, EC, all four ECX key types,
DH and ML-DSA-44 — every context-dependent allocation branch above.

Verification

  • Clean build under the project's -Werror flag set; no new warnings.
  • Unit suite 211 passed, 0 failed.
  • Negative control: with the fix reverted, EC and X25519 abort with SIGSEGV and
    RSA and DH fail on a NULL object.

@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
- wp_rsa_import_object(), wp_ecc_import_object(),
  wp_ecx_import_object(), wp_dh_import_object() and
  wp_mldsa_import_object() allocate a key from the encoder context's
  provider context, call the algorithm's key-management import, and
  return the key, freeing it and returning NULL on failure. The
  encoder dispatch tables register these as
  OSSL_FUNC_ENCODER_IMPORT_OBJECT.
- wp_ecx_new_by_type() creates the ECX key for a context's key type;
  wp_ecx_decode() calls it in place of its own key-type chain.
- test_encoder_import_object() in test_pkey.c reaches an encoder's
  dispatch table through the provider's operation query, runs NEWCTX
  and IMPORT_OBJECT over parameters from EVP_PKEY_todata(), checks
  the returned object against the key-management export, then calls
  FREE_OBJECT and FREECTX.
- Per-algorithm tests drive it for RSA and RSA-PSS, EC, X25519,
  ED25519, X448, ED448, DH and ML-DSA-44, generating each key with
  the default provider.

Issue: F-11552

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

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