Skip to content

feat: route configurable devId through key init calls#390

Open
MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
MarkAtwood:feat/devid-routing
Open

feat: route configurable devId through key init calls#390
MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
MarkAtwood:feat/devid-routing

Conversation

@MarkAtwood
Copy link
Copy Markdown

Summary

wolfProvider currently hardcodes INVALID_DEVID in all key initialization
calls, silently bypassing wolfCrypt's device callback layer. This makes it
impossible to route OpenSSL operations through a wolfHSM server (or any
other wolfCrypt device callback) via the provider interface.

This PR closes that gap for RSA, ECC, and DH:

  • Adds int devId to WOLFPROV_CTX, initialized to INVALID_DEVID (no behavior change for existing users)
  • Exposes wolfprovider_devid as a settable OSSL_PARAM so callers can configure the device ID at runtime
  • Routes devId through RSA (wc_InitRsaKey_ex), ECC (wc_ecc_init_ex), and DH (wc_InitDhKey_ex) key init calls
  • Routes devId through the ECX gen-context RNG init (wc_InitRng_ex)

Known gap

ECX key init functions (wc_curve25519_init, wc_ed25519_init, wc_ed448_init) use WP_ECX_INIT function pointers typed as int(*)(void*), which carry no devId parameter. Routing devId through ECX key init requires changing the table shape and all four call sites — that is a larger, separate change. The RNG used during ECX key generation does use devId after this PR.

wc_curve448_init_ex does not exist in wolfCrypt as of wolfSSL 5.x, so curve448 cannot be fixed without a wolfCrypt change.

Behavior for existing users

devId defaults to INVALID_DEVID, so all existing behavior is unchanged unless the caller explicitly sets wolfprovider_devid via OSSL_PROVIDER_set_params.

Test plan

  • Existing wolfProvider test suite passes (no regression)
  • RSA sign/verify routes through wolfHSM device callback when wolfprovider_devid is set to WH_DEV_ID
  • ECC sign/verify routes through wolfHSM device callback when wolfprovider_devid is set
  • DH key agreement routes through wolfHSM device callback when wolfprovider_devid is set
  • Setting wolfprovider_devid to INVALID_DEVID restores direct wolfCrypt behavior

Adds int devId to WOLFPROV_CTX (initialized to INVALID_DEVID) and
exposes it as a settable OSSL_PARAM ("wolfprovider_devid") so callers
can route provider operations through a wolfHSM device callback.

Routes devId through:
- RSA: wc_InitRsaKey -> wc_InitRsaKey_ex
- ECC: wc_ecc_init_ex (was hardcoding INVALID_DEVID)
- DH:  wc_InitDhKey_ex (was hardcoding INVALID_DEVID)
- ECX gen-context RNG: wc_InitRng -> wc_InitRng_ex

Known gap: ECX key init functions (wc_curve25519_init, wc_ed25519_init,
wc_ed448_init) use WP_ECX_INIT function pointers with no devId
parameter; fixing them requires a table-shape change tracked separately.
Copilot AI review requested due to automatic review settings April 10, 2026 01:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Routes a configurable wolfCrypt devId through provider-managed key/RNG initialization to enable device callback routing (e.g., wolfHSM) instead of always using INVALID_DEVID.

Changes:

  • Adds devId to WOLFPROV_CTX and sets a default of INVALID_DEVID.
  • Exposes wolfprovider_devid via provider settable_params/set_params.
  • Passes devId into RSA/ECC/DH key init and ECX keygen RNG init.

Reviewed changes

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

Show a summary per file
File Description
src/wp_wolfprov.c Adds provider parameter plumbing and default devId in provider context.
src/wp_rsa_kmgmt.c Uses provCtx->devId for RSA key init and RSA keygen RNG init.
src/wp_ecx_kmgmt.c Uses provCtx->devId for ECX keygen RNG init (not ECX key init).
src/wp_ecc_kmgmt.c Uses provCtx->devId for ECC key init.
src/wp_dh_kmgmt.c Uses provCtx->devId for DH key init.
include/wolfprovider/internal.h Adds devId to the provider context struct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

OSSL_PARAM_DEFN(OSSL_PROV_PARAM_VERSION, OSSL_PARAM_UTF8_PTR, NULL, 0),
OSSL_PARAM_DEFN(OSSL_PROV_PARAM_BUILDINFO, OSSL_PARAM_UTF8_PTR, NULL, 0),
OSSL_PARAM_DEFN(OSSL_PROV_PARAM_STATUS, OSSL_PARAM_INTEGER, NULL, 0),
OSSL_PARAM_int("wolfprovider_devid", NULL),
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wolfprovider_devid is being advertised as a gettable provider parameter via wolfssl_param_types, but this PR only adds support for setting it (wolfprov_set_params). This is an API contract mismatch: either implement returning the current value in wolfprov_get_params, or remove it from the gettable table and keep it only in wolfprov_settable_params.

Suggested change
OSSL_PARAM_int("wolfprovider_devid", NULL),

Copilot uses AI. Check for mistakes.
Comment on lines +403 to +407
static int wolfprov_set_params(void* provCtx, const OSSL_PARAM params[])
{
int ok = 1;
const OSSL_PARAM* p;
WOLFPROV_CTX* ctx = (WOLFPROV_CTX*)provCtx;
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx->devId becomes runtime-mutable via wolfprov_set_params, and is read concurrently in multiple algorithm init paths (RSA/ECC/DH/ECX RNG). In C, an unsynchronized read/write to the same non-atomic object across threads is a data race (undefined behavior). Consider making devId an atomic type (or guarding reads/writes with an existing/new mutex), or documenting/enforcing that wolfprovider_devid must only be set before any concurrent operations begin.

Copilot uses AI. Check for mistakes.
Comment on lines +411 to +416
p = OSSL_PARAM_locate_const(params, "wolfprovider_devid");
if (p != NULL) {
if (!OSSL_PARAM_get_int(p, &ctx->devId)) {
ok = 0;
}
}
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx->devId becomes runtime-mutable via wolfprov_set_params, and is read concurrently in multiple algorithm init paths (RSA/ECC/DH/ECX RNG). In C, an unsynchronized read/write to the same non-atomic object across threads is a data race (undefined behavior). Consider making devId an atomic type (or guarding reads/writes with an existing/new mutex), or documenting/enforcing that wolfprovider_devid must only be set before any concurrent operations begin.

Copilot uses AI. Check for mistakes.
OSSL_PARAM_DEFN(OSSL_PROV_PARAM_VERSION, OSSL_PARAM_UTF8_PTR, NULL, 0),
OSSL_PARAM_DEFN(OSSL_PROV_PARAM_BUILDINFO, OSSL_PARAM_UTF8_PTR, NULL, 0),
OSSL_PARAM_DEFN(OSSL_PROV_PARAM_STATUS, OSSL_PARAM_INTEGER, NULL, 0),
OSSL_PARAM_int("wolfprovider_devid", NULL),
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter name string literal is duplicated in multiple places. To reduce the risk of typos and make future refactors easier, define a single constant/macro (e.g., #define WP_PARAM_DEVID "wolfprovider_devid") and use it consistently across the gettable/settable/set handlers.

Copilot uses AI. Check for mistakes.
static const OSSL_PARAM* wolfprov_settable_params(void* provCtx)
{
static const OSSL_PARAM settable[] = {
OSSL_PARAM_int("wolfprovider_devid", NULL),
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter name string literal is duplicated in multiple places. To reduce the risk of typos and make future refactors easier, define a single constant/macro (e.g., #define WP_PARAM_DEVID "wolfprovider_devid") and use it consistently across the gettable/settable/set handlers.

Copilot uses AI. Check for mistakes.

WOLFPROV_ENTER(WP_LOG_COMP_PROVIDER, "wolfprov_set_params");

p = OSSL_PARAM_locate_const(params, "wolfprovider_devid");
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter name string literal is duplicated in multiple places. To reduce the risk of typos and make future refactors easier, define a single constant/macro (e.g., #define WP_PARAM_DEVID "wolfprovider_devid") and use it consistently across the gettable/settable/set handlers.

Copilot uses AI. Check for mistakes.
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.

2 participants