From 4580c303fa88f77a98461fee5fe26b5db725967c Mon Sep 17 00:00:00 2001 From: Todd Short Date: Thu, 1 Feb 2024 23:09:38 -0500 Subject: [PATCH 1/2] Fix EVP_PKEY_CTX_add1_hkdf_info() behavior Fix #23448 `EVP_PKEY_CTX_add1_hkdf_info()` behaves like a `set1` function. Fix the setting of the parameter in the params code. Update the TLS_PRF code to also use the params code. Add tests. Reviewed-by: Shane Lontis Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/23456) (cherry picked from commit 6b566687b58fde08b28e3331377f050768fad89b) --- crypto/evp/pmeth_lib.c | 65 ++++++++++++++++++- providers/implementations/exchange/kdf_exch.c | 42 ++++++++++++ providers/implementations/kdfs/hkdf.c | 8 +++ test/pkey_meth_kdf_test.c | 53 +++++++++++---- 4 files changed, 156 insertions(+), 12 deletions(-) diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index ba1971c..d0eeaf7 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -1028,6 +1028,69 @@ static int evp_pkey_ctx_set1_octet_string(EVP_PKEY_CTX *ctx, int fallback, return EVP_PKEY_CTX_set_params(ctx, octet_string_params); } +static int evp_pkey_ctx_add1_octet_string(EVP_PKEY_CTX *ctx, int fallback, + const char *param, int op, int ctrl, + const unsigned char *data, + int datalen) +{ + OSSL_PARAM os_params[2]; + unsigned char *info = NULL; + size_t info_len = 0; + size_t info_alloc = 0; + int ret = 0; + + if (ctx == NULL || (ctx->operation & op) == 0) { + ERR_raise(ERR_LIB_EVP, EVP_R_COMMAND_NOT_SUPPORTED); + /* Uses the same return values as EVP_PKEY_CTX_ctrl */ + return -2; + } + + /* Code below to be removed when legacy support is dropped. */ + if (fallback) + return EVP_PKEY_CTX_ctrl(ctx, -1, op, ctrl, datalen, (void *)(data)); + /* end of legacy support */ + + if (datalen < 0) { + ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_LENGTH); + return 0; + } + + /* Get the original value length */ + os_params[0] = OSSL_PARAM_construct_octet_string(param, NULL, 0); + os_params[1] = OSSL_PARAM_construct_end(); + + if (!EVP_PKEY_CTX_get_params(ctx, os_params)) + return 0; + + /* Older provider that doesn't support getting this parameter */ + if (os_params[0].return_size == OSSL_PARAM_UNMODIFIED) + return evp_pkey_ctx_set1_octet_string(ctx, fallback, param, op, ctrl, data, datalen); + + info_alloc = os_params[0].return_size + datalen; + if (info_alloc == 0) + return 0; + info = OPENSSL_zalloc(info_alloc); + if (info == NULL) + return 0; + info_len = os_params[0].return_size; + + os_params[0] = OSSL_PARAM_construct_octet_string(param, info, info_alloc); + + /* if we have data, then go get it */ + if (info_len > 0) { + if (!EVP_PKEY_CTX_get_params(ctx, os_params)) + goto error; + } + + /* Copy the input data */ + memcpy(&info[info_len], data, datalen); + ret = EVP_PKEY_CTX_set_params(ctx, os_params); + + error: + OPENSSL_clear_free(info, info_alloc); + return ret; +} + int EVP_PKEY_CTX_set1_tls1_prf_secret(EVP_PKEY_CTX *ctx, const unsigned char *sec, int seclen) { @@ -1078,7 +1141,7 @@ int EVP_PKEY_CTX_set1_hkdf_key(EVP_PKEY_CTX *ctx, int EVP_PKEY_CTX_add1_hkdf_info(EVP_PKEY_CTX *ctx, const unsigned char *info, int infolen) { - return evp_pkey_ctx_set1_octet_string(ctx, ctx->op.kex.algctx == NULL, + return evp_pkey_ctx_add1_octet_string(ctx, ctx->op.kex.algctx == NULL, OSSL_KDF_PARAM_INFO, EVP_PKEY_OP_DERIVE, EVP_PKEY_CTRL_HKDF_INFO, diff --git a/providers/implementations/exchange/kdf_exch.c b/providers/implementations/exchange/kdf_exch.c index 527a866..4bc8102 100644 --- a/providers/implementations/exchange/kdf_exch.c +++ b/providers/implementations/exchange/kdf_exch.c @@ -28,9 +28,13 @@ static OSSL_FUNC_keyexch_derive_fn kdf_derive; static OSSL_FUNC_keyexch_freectx_fn kdf_freectx; static OSSL_FUNC_keyexch_dupctx_fn kdf_dupctx; static OSSL_FUNC_keyexch_set_ctx_params_fn kdf_set_ctx_params; +static OSSL_FUNC_keyexch_get_ctx_params_fn kdf_get_ctx_params; static OSSL_FUNC_keyexch_settable_ctx_params_fn kdf_tls1_prf_settable_ctx_params; static OSSL_FUNC_keyexch_settable_ctx_params_fn kdf_hkdf_settable_ctx_params; static OSSL_FUNC_keyexch_settable_ctx_params_fn kdf_scrypt_settable_ctx_params; +static OSSL_FUNC_keyexch_gettable_ctx_params_fn kdf_tls1_prf_gettable_ctx_params; +static OSSL_FUNC_keyexch_gettable_ctx_params_fn kdf_hkdf_gettable_ctx_params; +static OSSL_FUNC_keyexch_gettable_ctx_params_fn kdf_scrypt_gettable_ctx_params; typedef struct { void *provctx; @@ -169,6 +173,13 @@ static int kdf_set_ctx_params(void *vpkdfctx, const OSSL_PARAM params[]) return EVP_KDF_CTX_set_params(pkdfctx->kdfctx, params); } +static int kdf_get_ctx_params(void *vpkdfctx, OSSL_PARAM params[]) +{ + PROV_KDF_CTX *pkdfctx = (PROV_KDF_CTX *)vpkdfctx; + + return EVP_KDF_CTX_get_params(pkdfctx->kdfctx, params); +} + static const OSSL_PARAM *kdf_settable_ctx_params(ossl_unused void *vpkdfctx, void *provctx, const char *kdfname) @@ -197,6 +208,34 @@ KDF_SETTABLE_CTX_PARAMS(tls1_prf, "TLS1-PRF") KDF_SETTABLE_CTX_PARAMS(hkdf, "HKDF") KDF_SETTABLE_CTX_PARAMS(scrypt, "SCRYPT") +static const OSSL_PARAM *kdf_gettable_ctx_params(ossl_unused void *vpkdfctx, + void *provctx, + const char *kdfname) +{ + EVP_KDF *kdf = EVP_KDF_fetch(PROV_LIBCTX_OF(provctx), kdfname, + NULL); + const OSSL_PARAM *params; + + if (kdf == NULL) + return NULL; + + params = EVP_KDF_gettable_ctx_params(kdf); + EVP_KDF_free(kdf); + + return params; +} + +#define KDF_GETTABLE_CTX_PARAMS(funcname, kdfname) \ + static const OSSL_PARAM *kdf_##funcname##_gettable_ctx_params(void *vpkdfctx, \ + void *provctx) \ + { \ + return kdf_gettable_ctx_params(vpkdfctx, provctx, kdfname); \ + } + +KDF_GETTABLE_CTX_PARAMS(tls1_prf, "TLS1-PRF") +KDF_GETTABLE_CTX_PARAMS(hkdf, "HKDF") +KDF_GETTABLE_CTX_PARAMS(scrypt, "SCRYPT") + #define KDF_KEYEXCH_FUNCTIONS(funcname) \ const OSSL_DISPATCH ossl_kdf_##funcname##_keyexch_functions[] = { \ { OSSL_FUNC_KEYEXCH_NEWCTX, (void (*)(void))kdf_##funcname##_newctx }, \ @@ -205,8 +244,11 @@ KDF_SETTABLE_CTX_PARAMS(scrypt, "SCRYPT") { OSSL_FUNC_KEYEXCH_FREECTX, (void (*)(void))kdf_freectx }, \ { OSSL_FUNC_KEYEXCH_DUPCTX, (void (*)(void))kdf_dupctx }, \ { OSSL_FUNC_KEYEXCH_SET_CTX_PARAMS, (void (*)(void))kdf_set_ctx_params }, \ + { OSSL_FUNC_KEYEXCH_GET_CTX_PARAMS, (void (*)(void))kdf_get_ctx_params }, \ { OSSL_FUNC_KEYEXCH_SETTABLE_CTX_PARAMS, \ (void (*)(void))kdf_##funcname##_settable_ctx_params }, \ + { OSSL_FUNC_KEYEXCH_GETTABLE_CTX_PARAMS, \ + (void (*)(void))kdf_##funcname##_gettable_ctx_params }, \ { 0, NULL } \ }; diff --git a/providers/implementations/kdfs/hkdf.c b/providers/implementations/kdfs/hkdf.c index daa619b..dd65a2a 100644 --- a/providers/implementations/kdfs/hkdf.c +++ b/providers/implementations/kdfs/hkdf.c @@ -371,6 +371,13 @@ static int kdf_hkdf_get_ctx_params(void *vctx, OSSL_PARAM params[]) return 0; return OSSL_PARAM_set_size_t(p, sz); } + if ((p = OSSL_PARAM_locate(params, OSSL_KDF_PARAM_INFO)) != NULL) { + if (ctx->info == NULL || ctx->info_len == 0) { + p->return_size = 0; + return 1; + } + return OSSL_PARAM_set_octet_string(p, ctx->info, ctx->info_len); + } return -2; } @@ -379,6 +386,7 @@ static const OSSL_PARAM *kdf_hkdf_gettable_ctx_params(ossl_unused void *ctx, { static const OSSL_PARAM known_gettable_ctx_params[] = { OSSL_PARAM_size_t(OSSL_KDF_PARAM_SIZE, NULL), + OSSL_PARAM_octet_string(OSSL_KDF_PARAM_INFO, NULL, 0), OSSL_PARAM_END }; return known_gettable_ctx_params; diff --git a/test/pkey_meth_kdf_test.c b/test/pkey_meth_kdf_test.c index f816d24..c09e2f3 100644 --- a/test/pkey_meth_kdf_test.c +++ b/test/pkey_meth_kdf_test.c @@ -16,7 +16,7 @@ #include #include "testutil.h" -static int test_kdf_tls1_prf(void) +static int test_kdf_tls1_prf(int index) { int ret = 0; EVP_PKEY_CTX *pctx; @@ -40,10 +40,23 @@ static int test_kdf_tls1_prf(void) TEST_error("EVP_PKEY_CTX_set1_tls1_prf_secret"); goto err; } - if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, - (unsigned char *)"seed", 4) <= 0) { - TEST_error("EVP_PKEY_CTX_add1_tls1_prf_seed"); - goto err; + if (index == 0) { + if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, + (unsigned char *)"seed", 4) <= 0) { + TEST_error("EVP_PKEY_CTX_add1_tls1_prf_seed"); + goto err; + } + } else { + if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, + (unsigned char *)"se", 2) <= 0) { + TEST_error("EVP_PKEY_CTX_add1_tls1_prf_seed"); + goto err; + } + if (EVP_PKEY_CTX_add1_tls1_prf_seed(pctx, + (unsigned char *)"ed", 2) <= 0) { + TEST_error("EVP_PKEY_CTX_add1_tls1_prf_seed"); + goto err; + } } if (EVP_PKEY_derive(pctx, out, &outlen) <= 0) { TEST_error("EVP_PKEY_derive"); @@ -65,7 +78,7 @@ err: return ret; } -static int test_kdf_hkdf(void) +static int test_kdf_hkdf(int index) { int ret = 0; EVP_PKEY_CTX *pctx; @@ -94,10 +107,23 @@ static int test_kdf_hkdf(void) TEST_error("EVP_PKEY_CTX_set1_hkdf_key"); goto err; } - if (EVP_PKEY_CTX_add1_hkdf_info(pctx, (const unsigned char *)"label", 5) + if (index == 0) { + if (EVP_PKEY_CTX_add1_hkdf_info(pctx, (const unsigned char *)"label", 5) <= 0) { - TEST_error("EVP_PKEY_CTX_set1_hkdf_info"); - goto err; + TEST_error("EVP_PKEY_CTX_add1_hkdf_info"); + goto err; + } + } else { + if (EVP_PKEY_CTX_add1_hkdf_info(pctx, (const unsigned char *)"lab", 3) + <= 0) { + TEST_error("EVP_PKEY_CTX_add1_hkdf_info"); + goto err; + } + if (EVP_PKEY_CTX_add1_hkdf_info(pctx, (const unsigned char *)"el", 2) + <= 0) { + TEST_error("EVP_PKEY_CTX_add1_hkdf_info"); + goto err; + } } if (EVP_PKEY_derive(pctx, out, &outlen) <= 0) { TEST_error("EVP_PKEY_derive"); @@ -195,8 +221,13 @@ err: int setup_tests(void) { - ADD_TEST(test_kdf_tls1_prf); - ADD_TEST(test_kdf_hkdf); + int tests = 1; + + if (fips_provider_version_ge(NULL, 3, 3, 1)) + tests = 2; + + ADD_ALL_TESTS(test_kdf_tls1_prf, tests); + ADD_ALL_TESTS(test_kdf_hkdf, tests); #ifndef OPENSSL_NO_SCRYPT ADD_TEST(test_kdf_scrypt); #endif -- 2.45.1