Skip to content

Commit

Permalink
Avoid allocating EVP_PKEY on size checks
Browse files Browse the repository at this point in the history
`EVP_PKEY_keygen_deterministic` allows callers to query the required
size for the `seed` parameter. This change avoids the unnecessary
allocation of memory for the `out_pkey` parameter to ensure that this
integer getter has no side effects.
  • Loading branch information
geedo0 committed Oct 11, 2024
1 parent 9ff8458 commit e0f4597
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 13 deletions.
23 changes: 15 additions & 8 deletions crypto/evp_extra/evp_extra_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2748,23 +2748,25 @@ TEST_P(PerKEMTest, KeygenSeedTest) {
EVP_PKEY *raw = nullptr;
ASSERT_TRUE(EVP_PKEY_keygen_init(ctx.get()));

// ---- 2. Test ctx->pkey == NULL ----
ASSERT_FALSE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw, nullptr,
nullptr));
// ---- 2. Test passing in a context without the KEM parameters set. ----
size_t keygen_seed_len = GetParam().keygen_seed_len;
std::vector<uint8_t> keygen_seed(keygen_seed_len);
ASSERT_FALSE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw,
keygen_seed.data(),
&keygen_seed_len));
EXPECT_EQ(EVP_R_NO_PARAMETERS_SET, ERR_GET_REASON(ERR_peek_last_error()));

// Setup the context with specific KEM parameters.
ASSERT_TRUE(EVP_PKEY_CTX_kem_set_params(ctx.get(), GetParam().nid));

// ---- 3. Test getting the lengths only ----
size_t keygen_seed_len;
ASSERT_TRUE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw, nullptr,
keygen_seed_len = 0;
ASSERT_TRUE(EVP_PKEY_keygen_deterministic(ctx.get(), nullptr, nullptr,
&keygen_seed_len));
EXPECT_EQ(keygen_seed_len, GetParam().keygen_seed_len);


// ---- 4. Test failure mode on a seed len NULL ----
std::vector<uint8_t> keygen_seed(keygen_seed_len);
// ---- 4. Test failure mode on a seed len NULL ----
keygen_seed.resize(keygen_seed_len);
EXPECT_FALSE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw, keygen_seed.data(),
nullptr));
EXPECT_EQ(ERR_R_PASSED_NULL_PARAMETER, ERR_GET_REASON(ERR_peek_last_error()));
Expand All @@ -2784,6 +2786,11 @@ TEST_P(PerKEMTest, KeygenSeedTest) {
EXPECT_FALSE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw, big_keygen_seed.data(),
&keygen_seed_len));
EXPECT_EQ(EVP_R_INVALID_PARAMETERS, ERR_GET_REASON(ERR_peek_last_error()));

// ---- 7. Test failure mode on a non-NULL out_pkey and NULL seed ----
EXPECT_FALSE(EVP_PKEY_keygen_deterministic(ctx.get(), &raw, nullptr,
&keygen_seed_len));
EXPECT_EQ(EVP_R_INVALID_PARAMETERS, ERR_GET_REASON(ERR_peek_last_error()));
}

TEST_P(PerKEMTest, EncapsSeedTest) {
Expand Down
12 changes: 11 additions & 1 deletion crypto/fipsmodule/evp/evp_ctx.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,17 @@ int EVP_PKEY_keygen_deterministic(EVP_PKEY_CTX *ctx,
goto end;
}

if (!out_pkey) {
if ((out_pkey == NULL) != (seed == NULL)) {
OPENSSL_PUT_ERROR(EVP, EVP_R_INVALID_PARAMETERS);
goto end;
}

// Caller is performing a size check.
if (out_pkey == NULL && seed == NULL) {
if (!ctx->pmeth->keygen_deterministic(ctx, NULL, NULL, seed_len)) {
goto end;
}
ret = 1;
goto end;
}

Expand Down
6 changes: 3 additions & 3 deletions include/openssl/experimental/kem_deterministic_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ extern "C" {
// performs deterministic keygen based on the value specified in |seed| of
// length |seed_len| bytes.
//
// If |seed| is set to NULL it is assumed that the caller is doing a size check:
// the function will write the size of the required seed in |seed_len| and return
// successfully.
// If |out_pkey| and |seed| are set to NULL it is assumed that the caller is
// doing a size check and the function will write the size of the required seed
// in |seed_len| and return successfully.
//
// EVP_PKEY_keygen_deterministic performs a deterministic key generation
// operation using the values from |ctx|, and the given |seed|. If |*out_pkey|
Expand Down
2 changes: 1 addition & 1 deletion util/fipstools/acvp/modulewrapper/modulewrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2918,7 +2918,7 @@ static bool ML_KEM_KEYGEN(const Span<const uint8_t> args[],
bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, nullptr));
if (!EVP_PKEY_CTX_kem_set_params(ctx.get(), nid) ||
!EVP_PKEY_keygen_init(ctx.get()) ||
!EVP_PKEY_keygen_deterministic(ctx.get(), &raw, NULL, &seed_len) ||
!EVP_PKEY_keygen_deterministic(ctx.get(), NULL, NULL, &seed_len) ||
seed_len != seed.size() ||
!EVP_PKEY_keygen_deterministic(ctx.get(), &raw, seed.data(), &seed_len)) {
return false;
Expand Down

0 comments on commit e0f4597

Please sign in to comment.