Skip to content

feat: route configurable devId through key init calls #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MarkAtwood wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: route configurable devId through key init calls #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/wolfprovider/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ typedef struct WOLFPROV_CTX {
wolfSSL_Mutex rng_mutex;
#endif
BIO_METHOD *coreBioMethod;
int devId;
} WOLFPROV_CTX;

#if defined(WP_HAVE_SEED_SRC) && defined(WP_HAVE_RANDOM)
Expand Down
2 changes: 1 addition & 1 deletion src/wp_dh_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ static wp_Dh* wp_dh_new(WOLFPROV_CTX *provCtx)
int ok = 1;
int rc;

rc = wc_InitDhKey_ex(&dh->key, NULL, INVALID_DEVID);
rc = wc_InitDhKey_ex(&dh->key, NULL, provCtx->devId);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitDhKey_ex", rc);
ok = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/wp_ecc_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ static wp_Ecc* wp_ecc_new(WOLFPROV_CTX *provCtx)
int ok = 1;
int rc;

rc = wc_ecc_init_ex(&ecc->key, NULL, INVALID_DEVID);
rc = wc_ecc_init_ex(&ecc->key, NULL, provCtx->devId);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_ecc_init_ex", rc);
ok = 0;
Expand Down
7 changes: 4 additions & 3 deletions src/wp_ecx_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,13 +1203,14 @@ static wp_EcxGenCtx* wp_ecx_gen_init(WOLFPROV_CTX* provCtx,
int rc;
int ok = 1;

rc = wc_InitRng(&ctx->rng);
/* provCtx assigned before RNG init: ctx->provCtx->devId must be valid */
ctx->provCtx = provCtx;
rc = wc_InitRng_ex(&ctx->rng, NULL, ctx->provCtx->devId);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRng", rc);
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRng_ex", rc);
ok = 0;
}
if (ok) {
ctx->provCtx = provCtx;
ctx->name = name;
if (!wp_ecx_gen_set_params(ctx, params)) {
wc_FreeRng(&ctx->rng);
Expand Down
6 changes: 3 additions & 3 deletions src/wp_rsa_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,9 @@ static wp_Rsa* wp_rsa_base_new(WOLFPROV_CTX* provCtx, int type)
int ok = 1;
int rc;

rc = wc_InitRsaKey(&rsa->key, NULL);
rc = wc_InitRsaKey_ex(&rsa->key, NULL, provCtx->devId);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRsaKey", rc);
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRsaKey_ex", rc);
ok = 0;
}

Expand Down Expand Up @@ -1533,7 +1533,7 @@ static wp_RsaGenCtx* wp_rsa_base_gen_init(WOLFPROV_CTX* provCtx,
int ok = 1;
int rc;

rc = wc_InitRng_ex(&ctx->rng, NULL, INVALID_DEVID);
rc = wc_InitRng_ex(&ctx->rng, NULL, provCtx->devId);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_InitRng_ex", rc);
ok = 0;
Expand Down
49 changes: 49 additions & 0 deletions src/wp_wolfprov.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ static const OSSL_PARAM wolfssl_param_types[] = {
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.
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.
OSSL_PARAM_END
};

Expand Down Expand Up @@ -218,6 +219,9 @@ static WOLFPROV_CTX* wolfssl_prov_ctx_new(void)
WP_CHECK_FIPS_ALGO_PTR(WP_CAST_ALGO_DRBG);

ctx = (WOLFPROV_CTX*)OPENSSL_zalloc(sizeof(*ctx));
if (ctx != NULL) {
ctx->devId = INVALID_DEVID;
}
if ((ctx != NULL) && (wc_InitRng(&ctx->rng) != 0)) {
OPENSSL_free(ctx);
ctx = NULL;
Expand Down Expand Up @@ -372,6 +376,49 @@ static int wolfprov_get_params(void* provCtx, OSSL_PARAM params[])
return ok;
}

/*
* Get the table of parameters that can be set on wolfProv.
*
* @param [in] provCtx Unused.
* @return Table of settable parameters.
*/
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.
OSSL_PARAM_END
};
(void)provCtx;
return settable;
}

/*
* Set parameters on the provider context.
*
* @param [in] provCtx Provider context.
* @param [in] params Parameters to set.
* @return 1 on success.
* @return 0 on failure.
*/
static int wolfprov_set_params(void* provCtx, const OSSL_PARAM params[])
{
int ok = 1;
const OSSL_PARAM* p;
WOLFPROV_CTX* ctx = (WOLFPROV_CTX*)provCtx;
Comment on lines +403 to +407
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.

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.
if (p != NULL) {
if (!OSSL_PARAM_get_int(p, &ctx->devId)) {
ok = 0;
}
}
Comment on lines +411 to +416
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.

WOLFPROV_LEAVE(WP_LOG_COMP_PROVIDER, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}

#ifdef HAVE_FIPS
/* Properties of wolfSSL provider: name and FIPS wolfSSL. */
#define WOLFPROV_PROPERTIES "provider=wolfprov,fips=yes"
Expand Down Expand Up @@ -1214,6 +1261,8 @@ static const OSSL_DISPATCH wolfprov_dispatch_table[] = {
{ OSSL_FUNC_PROVIDER_TEARDOWN, (DFUNC)wolfprov_teardown },
{ OSSL_FUNC_PROVIDER_GETTABLE_PARAMS, (DFUNC)wolfprov_gettable_params },
{ OSSL_FUNC_PROVIDER_GET_PARAMS, (DFUNC)wolfprov_get_params },
{ OSSL_FUNC_PROVIDER_SETTABLE_PARAMS, (DFUNC)wolfprov_settable_params },
{ OSSL_FUNC_PROVIDER_SET_PARAMS, (DFUNC)wolfprov_set_params },
{ OSSL_FUNC_PROVIDER_QUERY_OPERATION, (DFUNC)wolfprov_query },
{ OSSL_FUNC_PROVIDER_GET_CAPABILITIES,
(DFUNC)wolfssl_prov_get_capabilities },
Expand Down
Loading