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

Open
padelsbach wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fenrir fixes in wp_kbkdf.c #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 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
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
55 changes: 38 additions & 17 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 object holds a key schedule and needs releasing. */
int macInited;
/** 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->macInited = 1;
}

WOLFPROV_LEAVE(WP_LOG_COMP_KDF, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
Expand Down Expand Up @@ -526,6 +541,11 @@ static int wp_kbkdf_mac_update(wp_KbkdfCtx* ctx, const unsigned char *data,
static void wp_kbkdf_mac_free(wp_KbkdfCtx* ctx)
{
int ret = 0;

if (!ctx->macInited) {
return;
}

switch(ctx->mac) {
#ifdef WP_HAVE_HMAC
case WP_MAC_TYPE_HMAC:
Expand All @@ -541,6 +561,7 @@ static void wp_kbkdf_mac_free(wp_KbkdfCtx* ctx)
#endif
}

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

Expand Down
217 changes: 217 additions & 0 deletions test/test_kbkdf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,210 @@ static int test_kbkdf_counter(void)
return err;
}

/* Mode must be a UTF8 string; any other type is rejected without being
* used. */
static int test_kbkdf_bad_mode_type(void)
{
int err = 0;
EVP_KDF* kdf = NULL;
EVP_KDF_CTX* kctx = NULL;
OSSL_PARAM params[2];
int modeInt = 1;
unsigned char modeOctet[] = { 'C', 'O', 'U', 'N', 'T', 'E', 'R' };

PRINT_MSG("\nTesting KBKDF with wrong type for mode parameter");

kdf = EVP_KDF_fetch(wpLibCtx, "KBKDF", NULL);
if (kdf == NULL) {
PRINT_MSG("Failed to fetch KBKDF");
err = 1;
goto done;
}

kctx = EVP_KDF_CTX_new(kdf);
if (kctx == NULL) {
PRINT_MSG("Failed to create KBKDF context");
err = 1;
goto done;
}

params[0] = OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &modeInt);
params[1] = OSSL_PARAM_construct_end();
if (EVP_KDF_CTX_set_params(kctx, params) > 0) {
PRINT_MSG("Integer mode parameter was accepted");
err = 1;
goto done;
}

params[0] = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_MODE,
modeOctet, sizeof(modeOctet));
params[1] = OSSL_PARAM_construct_end();
if (EVP_KDF_CTX_set_params(kctx, params) > 0) {
PRINT_MSG("Octet string mode parameter was accepted");
err = 1;
goto done;
}

done:
EVP_KDF_free(kdf);
EVP_KDF_CTX_free(kctx);
return err;
}

/* Every parameter the derive calls above rely on must be discoverable
* through the settable parameter list. */
static int test_kbkdf_settable_params(void)
{
int err = 0;
size_t i;
EVP_KDF* kdf = NULL;
EVP_KDF_CTX* kctx = NULL;
const OSSL_PARAM* settable = NULL;
static const struct {
const char* name;
unsigned int dataType;
} expected[] = {
{ OSSL_KDF_PARAM_MODE, OSSL_PARAM_UTF8_STRING },
{ OSSL_KDF_PARAM_DIGEST, OSSL_PARAM_UTF8_STRING },
{ OSSL_KDF_PARAM_MAC, OSSL_PARAM_UTF8_STRING },
{ OSSL_KDF_PARAM_CIPHER, OSSL_PARAM_UTF8_STRING },
{ OSSL_KDF_PARAM_KEY, OSSL_PARAM_OCTET_STRING },
{ OSSL_KDF_PARAM_SALT, OSSL_PARAM_OCTET_STRING },
{ OSSL_KDF_PARAM_LABEL, OSSL_PARAM_OCTET_STRING },
{ OSSL_KDF_PARAM_SEED, OSSL_PARAM_OCTET_STRING },
{ OSSL_KDF_PARAM_INFO, OSSL_PARAM_OCTET_STRING }
};

PRINT_MSG("\nTesting KBKDF settable parameter list");

kdf = EVP_KDF_fetch(wpLibCtx, "KBKDF", NULL);
if (kdf == NULL) {
PRINT_MSG("Failed to fetch KBKDF");
err = 1;
goto done;
}

kctx = EVP_KDF_CTX_new(kdf);
if (kctx == NULL) {
PRINT_MSG("Failed to create KBKDF context");
err = 1;
goto done;
}

settable = EVP_KDF_CTX_settable_params(kctx);
if (settable == NULL) {
PRINT_MSG("No settable parameters reported");
err = 1;
goto done;
}

for (i = 0; i < sizeof(expected) / sizeof(*expected); i++) {
const OSSL_PARAM* p = OSSL_PARAM_locate_const(settable,
expected[i].name);
if (p == NULL) {
PRINT_MSG("Settable parameter missing: %s", expected[i].name);
err = 1;
}
else if (p->data_type != expected[i].dataType) {
PRINT_MSG("Settable parameter has wrong type: %s",
expected[i].name);
err = 1;
}
}

done:
EVP_KDF_free(kdf);
EVP_KDF_CTX_free(kctx);
return err;
}

/* Reset releases the MAC object, so a reused context must still derive
* the same key. */
static int test_kbkdf_reset_reuse(const char* mac, const char* alg)
{
int err = 0;
EVP_KDF* kdf = NULL;
EVP_KDF_CTX* kctx = NULL;
OSSL_PARAM params[7], *p;
unsigned char first[16];
unsigned char second[16];
unsigned char key[] = {
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10
};
unsigned char label[] = {
0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18
};
unsigned char context[] = {
0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
};
char counter[] = "COUNTER";

PRINT_MSG("Test KBKDF reset and reuse with %s", mac);

kdf = EVP_KDF_fetch(wpLibCtx, "KBKDF", NULL);
if (kdf == NULL) {
PRINT_MSG("Failed to fetch KBKDF");
err = 1;
goto done;
}

kctx = EVP_KDF_CTX_new(kdf);
if (kctx == NULL) {
PRINT_MSG("Failed to create KBKDF context");
err = 1;
goto done;
}

p = params;
*p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_MODE, counter, 0);
*p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_MAC, (char*)mac, 0);
if (XSTRCMP(mac, "CMAC") == 0) {
*p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_CIPHER,
(char*)alg, 0);
}
else {
*p++ = OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST,
(char*)alg, 0);
}
*p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, key,
sizeof(key));
*p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, label,
sizeof(label));
*p++ = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, context,
sizeof(context));
*p = OSSL_PARAM_construct_end();

if (EVP_KDF_derive(kctx, first, sizeof(first), params) <= 0) {
PRINT_MSG("First derive failed");
err = 1;
goto done;
}

EVP_KDF_CTX_reset(kctx);

if (EVP_KDF_derive(kctx, second, sizeof(second), params) <= 0) {
PRINT_MSG("Derive after reset failed");
err = 1;
goto done;
}

if (XMEMCMP(first, second, sizeof(first)) != 0) {
PRINT_MSG("Derived key changed after reset");
err = 1;
goto done;
}

/* Reset with nothing to release must also be safe. */
EVP_KDF_CTX_reset(kctx);
EVP_KDF_CTX_reset(kctx);

done:
EVP_KDF_free(kdf);
EVP_KDF_CTX_free(kctx);
return err;
}

int test_kbkdf(void *data)
{
int err = 0;
Expand All @@ -386,6 +590,19 @@ int test_kbkdf(void *data)
if (err == 0) {
err = test_kbkdf_counter();
}
if (err == 0) {
err = test_kbkdf_bad_mode_type();
}
if (err == 0) {
err = test_kbkdf_settable_params();
}
if (err == 0) {
PRINT_MSG("\nTesting KBKDF context reset and reuse:");
err = test_kbkdf_reset_reuse("CMAC", "AES-128-CBC");
}
if (err == 0) {
err = test_kbkdf_reset_reuse("HMAC", "SHA256");
}

return err;
}
Expand Down
Loading