Constrain file store decoders to the caller's provider boundary#430
Constrain file store decoders to the caller's provider boundary#430yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a provider-boundary enforcement gap in the wolfProvider file store decoder selection: the store’s explicitly-fetched “terminal” decoders now respect the caller’s OSSL_STORE_PARAM_PROPERTIES query (e.g., provider=wolfprov) instead of only applying it to OSSL_DECODER_CTX_add_extra.
Changes:
- Added a file-local helper to combine each decoder’s structure property query with the caller’s property query.
- Updated explicit
OSSL_DECODER_fetch()calls in the file store to use the combined query, ensuring provider boundary constraints are consistently enforced.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #430
Scan targets checked: wolfprovider-bugs, wolfprovider-src
No new issues found in the changed files. ✅
aidangarske
left a comment
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: COMMENT
Findings: 1 total — 1 posted, 0 skipped
Posted findings
- [Medium] New property-bound decoder fetch path lacks regression coverage —
src/wp_file_store.c:425-431
Review generated by Skoll.
| 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, |
There was a problem hiding this comment.
🟡 [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.
Constrain file store decoders to the caller's provider boundary
Summary
The wolfProvider file store fetched its terminal decoders using only the
structure property and ignored the caller's property query, so a store load
routed to wolfProvider could add and use a decoder from another provider. This
PR propagates the caller's property query into those explicit fetches so a
provider=wolfprovboundary is honored consistently. Fixes finding f_5534.Problem
wp_file_set_decoder()fetched each decoder like this:The caller-supplied
ctx->propQuery(where an application expresses aprovider=wolfprovcompliance boundary viaOSSL_STORE_PARAM_PROPERTIES) wasapplied only to
OSSL_DECODER_CTX_add_extra, not to these explicit fetches.As a result the store's own terminal decoders were fetched without any provider
constraint and could resolve to another provider's implementation.
Fix
Add a file-local helper that combines each decoder's structure query with the
caller's property query, and use it for every explicit fetch:
NULL) → behavior unchanged ("structure=...").provider=wolfprov→ combined query ("structure=...,provider=wolfprov"),so the boundary applies to the explicit fetches as well as to
OSSL_DECODER_CTX_add_extra.Behavior / compatibility
provider=wolfprov, the boundary is now enforced on allof the store's decoder fetches. A load that previously resolved silently
through another provider will now stay within wolfProvider — or fail if
wolfProvider cannot satisfy it, which is the intended boundary semantics.
Testing
(
-Werror -Wextra -Wshadow -Wshorten-64-to-32 ...).OPENSSL_CONF=provider.conf), including thestore-load paths:
test_rsa_load_key,test_rsa_load_cert,test_ec_load_key,test_ec_load_cert.