Skip to content

Fenrir fixes in wp_kbkdf.c #465

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

Merged
aidangarske merged 4 commits into from
Sep 1, 2026
Merged

Fenrir fixes in wp_kbkdf.c #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c1aedc
F-8681: check result of param lookup before comparison
padelsbach Aug 10, 2026
4f36f47
F-2789: add MODE, SEED, and INFO to KBKDF settable params
padelsbach Aug 10, 2026
a3cef47
F-2243: zeroize hmac/cmac context when freeing
padelsbach Aug 11, 2026
124a417
PR feedback: use type field rather than boolean, add test cases
padelsbach Aug 31, 2026
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
58 changes: 40 additions & 18 deletions src/wp_kbkdf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@
/** Base set of parameters settable against context for KBKDF. */
#define WP_KBKDF_BASE_SETTABLES \
OSSL_PARAM_utf8_string(OSSL_KDF_PARAM_PROPERTIES, NULL, 0), \
OSSL_PARAM_utf8_string(OSSL_KDF_PARAM_MODE, NULL, 0), \
OSSL_PARAM_utf8_string(OSSL_KDF_PARAM_DIGEST, NULL, 0), \
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_KEY, NULL, 0), \
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_SALT, NULL, 0), \
OSSL_PARAM_utf8_string(OSSL_KDF_PARAM_MAC, NULL, 0), \
OSSL_PARAM_utf8_string(OSSL_KDF_PARAM_CIPHER, NULL, 0), \
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_LABEL, NULL, 0)
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_LABEL, NULL, 0), \
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_SEED, NULL, 0), \
OSSL_PARAM_octet_string(OSSL_KDF_PARAM_INFO, NULL, 0)

/**
* The KBKDF context structure.
Expand All @@ -68,6 +71,8 @@ typedef struct wp_KbkdfCtx {
/** Mode and parameters */
int mode;
int mac;
/** MAC type whose object is initialised, 0 when none is. */
int macInitedType;
/** Cipher name */
char cipher[16];
/** Digest name */
Expand Down Expand Up @@ -108,6 +113,9 @@ static wp_KbkdfCtx* wp_kdf_kbkdf_new(WOLFPROV_CTX* provCtx)
return ctx;
}

/* Prototype for releasing the MAC object in use. */
static void wp_kbkdf_mac_free(wp_KbkdfCtx* ctx);

/**
* Clear KBKDF context object.
*
Expand All @@ -116,10 +124,11 @@ static wp_KbkdfCtx* wp_kdf_kbkdf_new(WOLFPROV_CTX* provCtx)
static void wp_kdf_kbkdf_clear(wp_KbkdfCtx* ctx)
{
if (ctx != NULL) {
wp_kbkdf_mac_free(ctx);
OPENSSL_clear_free(ctx->key, ctx->keySz);
OPENSSL_free(ctx->label);
OPENSSL_free(ctx->context);
OPENSSL_free(ctx->iv);
OPENSSL_clear_free(ctx->label, ctx->labelLen);
OPENSSL_clear_free(ctx->context, ctx->contextLen);
OPENSSL_clear_free(ctx->iv, ctx->ivLen);
}
}

Expand All @@ -132,7 +141,7 @@ static void wp_kdf_kbkdf_free(wp_KbkdfCtx* ctx)
{
if (ctx != NULL) {
wp_kdf_kbkdf_clear(ctx);
OPENSSL_free(ctx);
OPENSSL_clear_free(ctx, sizeof(*ctx));
}
}

Expand Down Expand Up @@ -177,15 +186,18 @@ static int wp_kdf_kbkdf_set_ctx_params(wp_KbkdfCtx* ctx,
if (!OSSL_PARAM_get_utf8_string_ptr(p, &mode)) {
ok = 0;
}
if (XSTRCMP(mode, "COUNTER") == 0) {
ctx->mode = WP_KDF_MODE_COUNTER;
}
else if (XSTRCMP(mode, "FEEDBACK") == 0) {
ctx->mode = WP_KDF_MODE_FEEDBACK;
}
else {
WOLFPROV_MSG(WP_LOG_COMP_KDF, "Invalid KDF mode: %s", mode);
ok = 0;
if (ok) {
if (XSTRCMP(mode, "COUNTER") == 0) {
ctx->mode = WP_KDF_MODE_COUNTER;
}
else if (XSTRCMP(mode, "FEEDBACK") == 0) {
ctx->mode = WP_KDF_MODE_FEEDBACK;
}
else {
WOLFPROV_MSG(WP_LOG_COMP_KDF, "Invalid KDF mode: %s",
mode);
ok = 0;
}
}
}
}
Expand Down Expand Up @@ -245,7 +257,7 @@ static int wp_kdf_kbkdf_set_ctx_params(wp_KbkdfCtx* ctx,
if (ok) {
p = OSSL_PARAM_locate((OSSL_PARAM*)params, OSSL_KDF_PARAM_SALT);
if ((p != NULL) && (p->data != NULL)) {
OPENSSL_free(ctx->label);
OPENSSL_clear_free(ctx->label, ctx->labelLen);
ctx->label = NULL;
if (!OSSL_PARAM_get_octet_string(p, (void**)&ctx->label, 0,
&ctx->labelLen)) {
Expand All @@ -257,7 +269,7 @@ static int wp_kdf_kbkdf_set_ctx_params(wp_KbkdfCtx* ctx,
if (ok) {
p = OSSL_PARAM_locate((OSSL_PARAM*)params, OSSL_KDF_PARAM_LABEL);
if ((p != NULL) && (p->data != NULL)) {
OPENSSL_free(ctx->label);
OPENSSL_clear_free(ctx->label, ctx->labelLen);
ctx->label = NULL;
if (!OSSL_PARAM_get_octet_string(p, (void**)&ctx->label, 0,
&ctx->labelLen)) {
Expand All @@ -269,7 +281,7 @@ static int wp_kdf_kbkdf_set_ctx_params(wp_KbkdfCtx* ctx,
if (ok) {
p = OSSL_PARAM_locate((OSSL_PARAM*)params, OSSL_KDF_PARAM_INFO);
if ((p != NULL) && (p->data != NULL)) {
OPENSSL_free(ctx->context);
OPENSSL_clear_free(ctx->context, ctx->contextLen);
ctx->context = NULL;
if (!OSSL_PARAM_get_octet_string(p, (void**)&ctx->context, 0,
&ctx->contextLen)) {
Expand Down Expand Up @@ -463,6 +475,9 @@ static int wp_kbkdf_init_mac(wp_KbkdfCtx* ctx, unsigned char* key,
if (rc != 0) {
ok = 0;
}
else {
ctx->macInitedType = ctx->mac;
}

WOLFPROV_LEAVE(WP_LOG_COMP_KDF, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
Expand Down Expand Up @@ -523,10 +538,13 @@ static int wp_kbkdf_mac_update(wp_KbkdfCtx* ctx, const unsigned char *data,
return ok;
}

/* Dispatch on the type that was initialised. Parameters can change ctx->mac
* after a MAC object is live, so that field must not select the object. */
static void wp_kbkdf_mac_free(wp_KbkdfCtx* ctx)
Comment thread
padelsbach marked this conversation as resolved.
{
int ret = 0;
switch(ctx->mac) {

switch(ctx->macInitedType) {
#ifdef WP_HAVE_HMAC
case WP_MAC_TYPE_HMAC:
wc_HmacFree(&ctx->hmacCtx);
Expand All @@ -541,6 +559,7 @@ static void wp_kbkdf_mac_free(wp_KbkdfCtx* ctx)
#endif
}

ctx->macInitedType = 0;
(void)ret;
}

Expand Down Expand Up @@ -711,6 +730,9 @@ static int wp_kdf_kbkdf_derive(wp_KbkdfCtx* ctx, unsigned char* key,
wp_kbkdf_mac_free(ctx);
}

/* A failed block breaks out with the MAC still live. */
wp_kbkdf_mac_free(ctx);

/* k_i holds derived key block(s) (and feedback state). */
OPENSSL_cleanse(k_i, sizeof(k_i));

Expand Down
Loading
Loading