Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 66 additions & 6 deletions src/wp_file_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
#include <wolfprovider/settings.h>
#include <wolfprovider/alg_funcs.h>

#include <wolfssl/wolfcrypt/types.h>

/* TODO: support directory access. */

/**
Expand Down Expand Up @@ -353,6 +355,55 @@ static const wp_DecoderInfo wp_decoders[] = {
/** Number of decoders supported. */
#define WP_DECODERS_SIZE (sizeof(wp_decoders) / sizeof(*wp_decoders))

/**
* Combine a decoder's structure query with the caller's property query.
*
* @param [in] base Structure property query. May be NULL.
* @param [in] caller Caller's property query. May be NULL.
* @param [out] query Combined property query string, or NULL when neither
* input constrains the fetch.
* @return 1 on success.
* @return 0 on allocation failure.
*/
static int wp_file_decoder_prop_query(const char* base, const char* caller,
char** query)
{
int ok = 1;
size_t len;

/* Treat an empty query string the same as an absent (NULL) one. */
if ((base != NULL) && (*base == '\0')) {
base = NULL;
}
if ((caller != NULL) && (*caller == '\0')) {
caller = NULL;
}

*query = NULL;
if (base == NULL) {
if (caller != NULL) {
*query = OPENSSL_strdup(caller);
ok = (*query != NULL);
}
}
else if (caller == NULL) {
*query = OPENSSL_strdup(base);
ok = (*query != NULL);
}
else {
len = XSTRLEN(base) + XSTRLEN(caller) + 2;
*query = OPENSSL_malloc(len);
if (*query == NULL) {
ok = 0;
}
else {
XSNPRINTF(*query, len, "%s,%s", base, caller);
}
}

return ok;
}

/**
* Set the decoders into the decoder context.
*
Expand All @@ -366,19 +417,28 @@ static int wp_file_set_decoder(wp_FileCtx* ctx, OSSL_DECODER_CTX* decCtx)
int ok = 1;
size_t i;
OSSL_DECODER* decoder;
char* query = NULL;

WOLFPROV_ENTER(WP_LOG_COMP_PROVIDER, "wp_file_set_decoder");

for (i = 0; ok && (i < WP_DECODERS_SIZE); i++) {
decoder = OSSL_DECODER_fetch(ctx->provCtx->libCtx, wp_decoders[i].name,
wp_decoders[i].propQuery);
if (decoder == NULL) {
if (!wp_file_decoder_prop_query(wp_decoders[i].propQuery,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟡 [Medium] New property-bound decoder fetch path lacks regression coverage
💡 SUGGEST test

The PR's core behavior is that explicit OSSL_DECODER_fetch() calls now include ctx->propQuery, but the existing store-load tests do not pass OSSL_STORE_PARAM_PROPERTIES; repository search found that parameter only in src/wp_file_store.c. That leaves the fixed provider-boundary path untested, so a future change could drop the caller query again while the current RSA/EC load tests still pass.

Recommendation: Add a store-load regression test that passes OSSL_STORE_PARAM_PROPERTIES with provider=wolfprov and verifies the key/cert load succeeds within wolfProvider. A negative case using a property query that excludes wolfProvider (e.g. provider=default) would also exercise the failure boundary. Add these focused tests for OSSL_STORE_PARAM_PROPERTIES on the file store path before or soon after merge.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, and added the regression tests

ctx->propQuery, &query)) {
ok = 0;
}
if (ok && !OSSL_DECODER_CTX_add_decoder(decCtx, decoder)) {
ok = 0;
if (ok) {
decoder = OSSL_DECODER_fetch(ctx->provCtx->libCtx,
wp_decoders[i].name, query);
if (decoder == NULL) {
ok = 0;
}
if (ok && !OSSL_DECODER_CTX_add_decoder(decCtx, decoder)) {
ok = 0;
}
OSSL_DECODER_free(decoder);
}
OSSL_DECODER_free(decoder);
OPENSSL_free(query);
query = NULL;
}

WOLFPROV_LEAVE(WP_LOG_COMP_PROVIDER, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
Expand Down
73 changes: 73 additions & 0 deletions test/test_rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1580,6 +1580,79 @@ int test_rsa_load_cert(void* data)
return err;
}

/**
* Load the RSA private key from the file store with a caller property query.
*
* @param [in] props Property query passed as OSSL_STORE_PARAM_PROPERTIES.
* @return 1 when a key was loaded, 0 otherwise.
*/
static int test_rsa_store_load_props(const char* props)
{
int loaded = 0;
OSSL_PARAM params[2];
OSSL_STORE_CTX* ctx = NULL;
OSSL_STORE_INFO* info = NULL;
EVP_PKEY* pkey = NULL;

params[0] = OSSL_PARAM_construct_utf8_string(OSSL_STORE_PARAM_PROPERTIES,
(char *)props, 0);
params[1] = OSSL_PARAM_construct_end();

ctx = OSSL_STORE_open_ex(CERTS_DIR "/server-key.pem", wpLibCtx, NULL, NULL,
NULL, params, NULL, NULL);
if (ctx != NULL) {
info = OSSL_STORE_load(ctx);
}
if (info != NULL) {
pkey = OSSL_STORE_INFO_get1_PKEY(info);
loaded = pkey != NULL;
}

EVP_PKEY_free(pkey);
OSSL_STORE_INFO_free(info);
OSSL_STORE_close(ctx);
return loaded;
}

int test_rsa_load_key_prop_query(void* data)
{
int err = 0;

(void)data;

PRINT_MSG("Load RSA private key within the wolfProvider boundary");
if (!test_rsa_store_load_props("provider=wolfprov")) {
PRINT_ERR_MSG("Store load with provider=wolfprov failed");
err = 1;
}

if (err == 0) {
PRINT_MSG("Load RSA private key with an empty property query");
if (!test_rsa_store_load_props("")) {
PRINT_ERR_MSG("Store load with an empty property query failed");
err = 1;
}
}

if (err == 0) {
PRINT_MSG("Load RSA private key with an optional provider clause");
if (!test_rsa_store_load_props("?provider=wp_no_such_provider")) {
PRINT_ERR_MSG("Store load with an optional clause failed");
err = 1;
}
}

if (err == 0) {
PRINT_MSG("Load RSA private key with an unavailable provider");
if (test_rsa_store_load_props("provider=wp_no_such_provider")) {
PRINT_ERR_MSG("Store load ignored the caller's property query");
err = 1;
}
}

return err;
}

int test_rsa_fromdata(void* data)
{
(void)data;
Expand Down
1 change: 1 addition & 0 deletions test/unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_rsa_pss_restrictions, NULL),
TEST_DECL(test_rsa_load_key, NULL),
TEST_DECL(test_rsa_load_cert, NULL),
TEST_DECL(test_rsa_load_key_prop_query, NULL),
TEST_DECL(test_rsa_fromdata, NULL),
TEST_DECL(test_rsa_fromdata_oversize, NULL),
TEST_DECL(test_rsa_decode_pkcs8, NULL),
Expand Down
1 change: 1 addition & 0 deletions test/unit.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ int test_rsa_pss_restrictions(void *data);

int test_rsa_load_key(void* data);
int test_rsa_load_cert(void* data);
int test_rsa_load_key_prop_query(void* data);
int test_rsa_fromdata(void* data);
int test_rsa_fromdata_oversize(void* data);
int test_rsa_decode_pkcs8(void* data);
Expand Down
Loading