Route KDF key-exchange and MAC-signature through wolfProvider directly#429
Route KDF key-exchange and MAC-signature through wolfProvider directly#429yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR ensures the HKDF/TLS1-PRF key-exchange and HMAC/CMAC “MAC-via-signature” paths are always executed by wolfProvider’s own implementations, rather than accidentally being satisfied by another provider via EVP fetches from the provider’s child libctx.
Changes:
- Reworked KDF key-exchange (HKDF/TLS1-PRF) to call wolfProvider KDF implementations via their dispatch tables, avoiding EVP_KDF_fetch entirely.
- Reworked MAC-signature bridging (HMAC/CMAC) to call wolfProvider MAC implementations via their dispatch tables, avoiding EVP_MAC_fetch entirely.
- Guarded TLS1-PRF keyexch algorithm registration behind
WP_HAVE_TLS1_PRF.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/wp_kdf_exch.c | Switches HKDF/TLS1-PRF keyexch to invoke wolfProvider KDF dispatch directly (no EVP fetch), with TLS1-PRF code compiled under WP_HAVE_TLS1_PRF. |
| src/wp_mac_sig.c | Switches HMAC/CMAC signature path to invoke wolfProvider MAC dispatch directly (no EVP fetch) and removes property-query retention. |
| src/wp_wolfprov.c | Wraps TLS1-PRF keyexch algorithm registration with #ifdef WP_HAVE_TLS1_PRF to match symbol availability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5b8d8df to
fc05260
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #429
Scan targets checked: wolfprovider-bugs, wolfprovider-src
No new issues found in the changed files. ✅
fc05260 to
eabc2eb
Compare
eabc2eb to
ecc2a86
Compare
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
- [Low] Direct MAC dispatch breaks standard signature size queries —
src/wp_mac_sig.c:369-373
Review generated by Skoll.
| @@ -312,7 +369,7 @@ static int wp_mac_digest_sign_final(wp_MacSigCtx *ctx, unsigned char *sig, | |||
| if (ok && (sigSize == MAX_SIZE_T) && (ctx->type == WP_MAC_TYPE_CMAC)) { | |||
There was a problem hiding this comment.
🔵 [Low] Direct MAC dispatch breaks standard signature size queries
💡 SUGGEST
The PR replaces EVP_MAC_final() with a direct call to the wolfProvider MAC final dispatch. That exposes the internal HMAC/CMAC finals directly, but those finals reject outSize < macCtx->size before handling out == NULL. Data Flow: application calls the standard two-pass EVP_DigestSignFinal(mdctx, NULL, &len) size query for an HMAC/CMAC signature selected from wolfProvider -> OpenSSL invokes wp_mac_digest_sign_final with sig == NULL -> the new direct ctx->final(...) call reaches wp_hmac_final or wp_cmac_final -> size check fails instead of setting *sigLen -> MAC generation fails for callers using the normal OpenSSL size-query contract. The PR introduces this because the old EVP path delegated to a fetched MAC implementation, commonly OpenSSL default, whose EVP_MAC_final path handles size queries. Tests exercise buffered EVP_DigestSignFinal calls for HMAC/CMAC, but not the sig == NULL size-query path.
Recommendation: Preserve EVP signature semantics before calling the MAC final. Load OSSL_FUNC_MAC_GET_CTX_PARAMS in wp_mac_ctx_load, then answer NULL-output queries without finalizing state:
if (ok && (sig == NULL)) {
OSSL_PARAM p[2];
size_t macSize = 0;
p[0] = OSSL_PARAM_construct_size_t(OSSL_MAC_PARAM_SIZE, &macSize);
p[1] = OSSL_PARAM_construct_end();
if ((ctx->getParams == NULL) || !ctx->getParams(ctx->macCtx, p)) {
ok = 0;
}
else {
*sigLen = macSize;
return 1;
}
}
Add HMAC and CMAC regression tests that call EVP_DigestSignFinal(ctx, NULL, &len) before allocating the output buffer.
Summary
The HKDF/TLS1-PRF key-exchange and HMAC/CMAC signature bridges advertised by
wolfProvider silently delegated the actual derivation/MAC to another provider.
This pins both to wolfProvider's own implementations, and fixes a related
correctness bug in the KDF key-exchange context duplication.
Fixes 5533 (KDF key-exchange) and 6498 (MAC-via-signature).
Root cause
wp_kdf_ctx_newandwp_mac_ctx_newre-fetched their backingEVP_KDF/EVP_MACfromprovCtx->libCtx:provCtx->libCtxis not the application's libctx — it is the child libctxcreated in
OSSL_provider_initviaOSSL_LIB_CTX_new_child(wp_wolfprov.c).A provider is not a member of the child context it spawns, so wolfProvider's own
algorithms are absent there; only the sibling/default providers bridged from the
parent are present. The fetch therefore resolved to whichever provider answered
the name — commonly the OpenSSL default provider.
Verified empirically (instrumented derive in the real test):
So a caller that selected
provider=wolfprovfor the key-exchange/signaturecould still have the derivation/MAC computed elsewhere, with no error. Appending
provider=wolfprov(the findings' suggested fix) does not work — the queryreturns NULL in the child libctx and breaks the path.
Change
Drive wolfProvider's own KDF/MAC implementation dispatch tables directly
instead of fetching through a libctx. The function pointers are resolved once
from the exported dispatch (
wp_kdf_hkdf_functions,wp_kdf_tls1_prf_functions,wp_hmac_functions,wp_cmac_functions) into the context and called with theprovider context — the same enriched-ctx + loader technique
wp_drbg.calreadyuses for the parent rand dispatch. This binds the backing implementation at
compile time and removes the libctx dependency entirely, so HKDF, TLS1-PRF, HMAC
and CMAC can no longer fall back to another provider.
This is consistent with the rest of the provider: of ~130 context constructors,
these two were the only EVP-passthrough shims; everything else already calls
wolfCrypt directly.
Additional correctness fixes
DUPCTXwas shallow.OSSL_FUNC_KEYEXCH_DUPCTXisregistered, so
EVP_PKEY_CTX_dupcallswp_kdf_ctx_dup, but it created afresh, unconfigured KDF context — the duplicate silently lost its
salt/key/info and derived incorrectly (or failed). The wolfProvider KDF
dispatch had no
OSSL_FUNC_KDF_DUPCTXto copy state, and there is no getterto read the config back. Added deep-copy
dupctximplementations towp_hkdf.candwp_tls1_prf.c, wired through the same loader as the otherops. Also makes
EVP_KDF_CTX_dupwork for the standalone HKDF/TLS1-PRF KDFs.set_ctx_paramsfailure.wp_mac_digest_sign_initnowpropagates a
setParamsfailure into the error path instead of continuing.Files
src/wp_kdf_exch.c— HKDF/TLS1-PRF keyexch drive wolfProvider's KDF dispatchdirectly (
wp_kdf_ctx_load); deep-copywp_kdf_ctx_dupvia the resolveddupctx.src/wp_mac_sig.c— HMAC/CMAC signature drive wolfProvider's MAC dispatchdirectly (
wp_mac_ctx_load); propagatesetParamsreturn.src/wp_hkdf.c— addwp_kdf_hkdf_dup(OSSL_FUNC_KDF_DUPCTX).src/wp_tls1_prf.c— addwp_kdf_tls1_prf_dup(OSSL_FUNC_KDF_DUPCTX).src/wp_wolfprov.c— guard the TLS1-PRF keyexch registration underWP_HAVE_TLS1_PRF(the backing KDF symbol only exists there).Incidental cleanup left behind by removing the fetch path: dead
propQueryandlibCtxfields (MAC), TLS1-PRF param helpers moved under theWP_HAVE_TLS1_PRFguard, and unused
<openssl/evp.h>/<openssl/kdf.h>/<openssl/ec.h>includes.
Verification
(macOS, no leak detection); the new
dupctxallocations report noleaks/overflows.
EVP_PKEY_CTX_dupsit, and confirms both derive identical output (MATCH). Negative control
(deep-copy disabled) reproduces the bug (
dup derive FAILED), confirming thecheck is meaningful.
nmon isolated objects compiled withWP_HAVE_TLS1_PRFoff: no reference towp_kdf_tls1_prf_functions, and no-Wunused-functionunder-Werror.