Skip to content
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.

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

add support for EVP_PKEY_CTX callback functions #1905

Merged
merged 4 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 59 additions & 0 deletions crypto/evp_extra/evp_extra_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2936,3 +2936,62 @@ TEST_P(PerMLKEMTest, InputValidation) {
ctx->pkey->pkey.kem_key->secret_key[GetParam().secret_key_len - 64] ^= 1;
ASSERT_FALSE(EVP_PKEY_decapsulate(ctx.get(), ss_expected.data(), &ss_len, ct.data(), ct_len));
}

struct dummy_cb_app_data {
bool state;
};

// Dummy callback function used for testing.
static int dummy_gen_cb(EVP_PKEY_CTX *ctx) {
// Get the application-specific data.
auto *app_data =
static_cast<dummy_cb_app_data *>(EVP_PKEY_CTX_get_app_data(ctx));
EXPECT_TRUE(app_data);

app_data->state = true;

return 1; // Return success (1).
}

TEST(EVPExtraTest, Callbacks) {
bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, nullptr));
ASSERT_TRUE(ctx);

// Check the initial values of |ctx->keygen_info|.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the keygen operation be initialized (EVP_PKEY_keygen_init) prior to this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial intention for the check here was to see if the new EVP_PKEY_CTX fields had been initialized correctly upon creating a new EVP_PKEY_CTX.
So the fields should have a readable value, regardless of EVP_PKEY_keygen_init was called.

int keygen_info = EVP_PKEY_CTX_get_keygen_info(ctx.get(), -1);
ASSERT_EQ(keygen_info, EVP_PKEY_CTX_KEYGEN_INFO_COUNT);
for (int i = 0; i < keygen_info; i++) {
EXPECT_EQ(EVP_PKEY_CTX_get_keygen_info(ctx.get(), i), 0);
}

// Generating an RSA key would have triggered the callback.
EVP_PKEY *pkey = EVP_PKEY_new();
ASSERT_EQ(EVP_PKEY_keygen_init(ctx.get()), 1);
ASSERT_TRUE(EVP_PKEY_keygen(ctx.get(), &pkey));
ASSERT_TRUE(pkey);

// Verify that |ctx->keygen_info| has not been updated since a callback hasn't
// been set.
for (int i = 0; i < keygen_info; i++) {
EXPECT_EQ(EVP_PKEY_CTX_get_keygen_info(ctx.get(), i), 0);
}

// Now we set the application data and callback.
dummy_cb_app_data app_data{false};
EVP_PKEY_CTX_set_app_data(ctx.get(), &app_data);
EVP_PKEY_CTX_set_cb(ctx.get(), dummy_gen_cb);
EXPECT_FALSE(app_data.state);

// Call key generation again to trigger the callback.
ASSERT_TRUE(EVP_PKEY_keygen(ctx.get(), &pkey));
ASSERT_TRUE(pkey);
bssl::UniquePtr<EVP_PKEY> ptr(pkey);

// The callback function should set the state to true. The contents of
// |ctx->keygen_info| will only be populated once the callback has been set.
EXPECT_TRUE(app_data.state);
for (int i = 0; i < keygen_info; i++) {
EXPECT_GT(EVP_PKEY_CTX_get_keygen_info(ctx.get(), i), 0);
}
}

42 changes: 35 additions & 7 deletions crypto/fipsmodule/evp/evp_ctx.c
Original file line number Diff line number Diff line change
Expand Up @@ -669,21 +669,49 @@ int EVP_PKEY_CTX_ctrl_str(EVP_PKEY_CTX *ctx, const char *name,
return ctx->pmeth->ctrl_str(ctx, name, value);
}

// Deprecated keygen NO-OP functions

static int trans_cb(int a, int b, BN_GENCB *gcb) {
EVP_PKEY_CTX *ctx = BN_GENCB_get_arg(gcb);
ctx->keygen_info[0] = a;
ctx->keygen_info[1] = b;
return ctx->pkey_gencb(ctx);
}


void evp_pkey_set_cb_translate(BN_GENCB *cb, EVP_PKEY_CTX *ctx) {
BN_GENCB_set(cb, trans_cb, ctx);
}

void EVP_PKEY_CTX_set_cb(EVP_PKEY_CTX *ctx, EVP_PKEY_gen_cb *cb) {
// No-op
if (ctx == NULL) {
return;
}
ctx->pkey_gencb = cb;
}

void EVP_PKEY_CTX_set_app_data(EVP_PKEY_CTX *ctx, void *data) {
// No-op
if (ctx == NULL) {
return;
}
ctx->app_data = data;
}

void *EVP_PKEY_CTX_get_app_data(EVP_PKEY_CTX *ctx) {
// No-op
return NULL;
if (ctx == NULL) {
return NULL;
}
return ctx->app_data;
}

int EVP_PKEY_CTX_get_keygen_info(EVP_PKEY_CTX *ctx, int idx) {
// No-op
return 0;
GUARD_PTR(ctx);
if (idx == -1) {
return EVP_PKEY_CTX_KEYGEN_INFO_COUNT;
}
samuel40791765 marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines 706 to +710
Copy link
Contributor

@justsmth justsmth Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Should this return 0 if ctx->operation != EVP_PKEY_OP_KEYGEN && ctx->operation != EVP_PKEY_OP_PARAMGEN?
  • EVP_PKEY_CTX_KEYGEN_INFO_COUNT is the maximum possible, but might the actual number of indices available differ by the type and the operation? Should there be a comment here relating to that possibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Sure, I think that makes sense. I initially tried returning 0 if the key type was not RSA or DH, but that didn't work because there was the possibility where EVP_PKEY_CTX did not have a set type yet. This should be a good way to enforce something similar.
  2. My interpretation of the array and how it's used in OpenSSL is that it's only used to map to the arguments available in BN_GENCB. Each operation that uses this only has 2 available arguments to configure, so I thought it was safe to lock this as 2.
    I did document some parts of this in EVP_PKEY_CTX_get_keygen_info, but forget to mention how I came to the conclusion for this variable. Will add!

if (idx < 0 || idx >= EVP_PKEY_CTX_KEYGEN_INFO_COUNT ||
(ctx->operation != EVP_PKEY_OP_KEYGEN &&
ctx->operation != EVP_PKEY_OP_PARAMGEN)) {
return 0;
}
return ctx->keygen_info[idx];
}
21 changes: 21 additions & 0 deletions crypto/fipsmodule/evp/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,15 @@ int EVP_RSA_PKEY_CTX_ctrl(EVP_PKEY_CTX *ctx, int optype, int cmd, int p1, void *
#define EVP_PKEY_CTRL_HKDF_INFO (EVP_PKEY_ALG_CTRL + 18)
#define EVP_PKEY_CTRL_DH_PAD (EVP_PKEY_ALG_CTRL + 19)

// EVP_PKEY_CTX_KEYGEN_INFO_COUNT is the maximum array length for
// |EVP_PKEY_CTX->keygen_info|. The array length corresponds to the number of
// arguments |BN_GENCB|'s callback function handles.
//
// |ctx->keygen_info| map to the following values in |BN_GENCB|:
// 1. |ctx->keygen_info[0]| -> |event|
// 2. |ctx->keygen_info[1]| -> |n|
#define EVP_PKEY_CTX_KEYGEN_INFO_COUNT 2

struct evp_pkey_ctx_st {
// Method associated with this operation
const EVP_PKEY_METHOD *pmeth;
Expand All @@ -255,6 +264,14 @@ struct evp_pkey_ctx_st {
int operation;
// Algorithm specific data
void *data;
// Application specific data used by the callback.
void *app_data;
// Callback and specific keygen data that is mapped to |BN_GENCB| for relevant
// implementations. This is only used for DSA, DH, and RSA in OpenSSL. AWS-LC
// only supports RSA as of now.
// See |EVP_PKEY_CTX_get_keygen_info| for more details.
EVP_PKEY_gen_cb *pkey_gencb;
int keygen_info[EVP_PKEY_CTX_KEYGEN_INFO_COUNT];
}; // EVP_PKEY_CTX

struct evp_pkey_method_st {
Expand Down Expand Up @@ -346,6 +363,10 @@ typedef struct {
char has_private;
} ED25519_KEY;

// evp_pkey_set_cb_translate translates |ctx|'s |pkey_gencb| and sets it as the
// callback function for |cb|.
void evp_pkey_set_cb_translate(BN_GENCB *cb, EVP_PKEY_CTX *ctx);

#define ED25519_PUBLIC_KEY_OFFSET 32

#define FIPS_EVP_PKEY_METHODS 7
Expand Down
18 changes: 15 additions & 3 deletions crypto/fipsmodule/evp/p_rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ static int pkey_rsa_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) {
int ret = 0;
RSA *rsa = NULL;
RSA_PKEY_CTX *rctx = ctx->data;
BN_GENCB *pkey_ctx_cb = NULL;

// In FIPS mode, the public exponent is set within |RSA_generate_key_fips|
if (!is_fips_build() && !rctx->pub_exp) {
Expand All @@ -667,23 +668,34 @@ static int pkey_rsa_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) {
goto end;
}

if (ctx->pkey_gencb) {
pkey_ctx_cb = BN_GENCB_new();
if (pkey_ctx_cb == NULL) {
goto end;
}
evp_pkey_set_cb_translate(pkey_ctx_cb, ctx);
}

// In FIPS build, |RSA_generate_key_fips| updates the service indicator so lock it here
FIPS_service_indicator_lock_state();
if ((!is_fips_build() && !RSA_generate_key_ex(rsa, rctx->nbits, rctx->pub_exp, NULL)) ||
( is_fips_build() && !RSA_generate_key_fips(rsa, rctx->nbits, NULL)) ||
if ((!is_fips_build() &&
!RSA_generate_key_ex(rsa, rctx->nbits, rctx->pub_exp, pkey_ctx_cb)) ||
(is_fips_build() &&
!RSA_generate_key_fips(rsa, rctx->nbits, pkey_ctx_cb)) ||
!rsa_set_pss_param(rsa, ctx)) {
FIPS_service_indicator_unlock_state();
goto end;
}

FIPS_service_indicator_unlock_state();

if (pkey_ctx_is_pss(ctx)) {
ret = EVP_PKEY_assign(pkey, EVP_PKEY_RSA_PSS, rsa);
} else {
ret = EVP_PKEY_assign_RSA(pkey, rsa);
}

end:
BN_GENCB_free(pkey_ctx_cb);
if (!ret && rsa) {
RSA_free(rsa);
}
Expand Down
54 changes: 39 additions & 15 deletions include/openssl/evp.h
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,45 @@ OPENSSL_EXPORT int EVP_PKEY_asn1_get0_info(int *ppkey_id, int *pkey_base_id,
const EVP_PKEY_ASN1_METHOD *ameth);


// EVP_PKEY_CTX keygen/paramgen functions.

typedef int EVP_PKEY_gen_cb(EVP_PKEY_CTX *ctx);

// EVP_PKEY_CTX_set_cb sets |cb| as the key or parameter generation callback
// function for |ctx|. The callback function is then translated and used as the
// underlying |BN_GENCB| for |ctx|. Once |cb| is set for |ctx|, any information
// regarding key or parameter generation can be retrieved via
// |EVP_PKEY_CTX_get_keygen_info|.
// This behavior only applies to |EVP_PKEY|s that have calls to |BN_GENCB|
// available, which is only |EVP_PKEY_RSA|.
//
// TODO: Add support for |EVP_PKEY_DH| once we have param_gen support.
OPENSSL_EXPORT void EVP_PKEY_CTX_set_cb(EVP_PKEY_CTX *ctx, EVP_PKEY_gen_cb *cb);

// EVP_PKEY_CTX_get_keygen_info returns the values associated with the
// |EVP_PKEY_gen_cb|/|BN_GENCB| assigned to |ctx|. This should only be used if
// |EVP_PKEY_CTX_set_cb| has been called. If |idx| is -1, the total number of
// available parameters is returned. Any non-negative value less than the total
// number of available parameters, returns the indexed value in the parameter
// array. We return 0 for any invalid |idx| or key type.
//
// The |idx|s in |ctx->keygen_info| correspond to the following values for
// |BN_GENCB|:
// 1. |ctx->keygen_info[0]| -> |event|
// 2. |ctx->keygen_info[1]| -> |n|
// See documentation for |BN_GENCB| for more details regarding the definition
// of each parameter.
//
// TODO: Add support for |EVP_PKEY_DH| once we have param_gen support.
OPENSSL_EXPORT int EVP_PKEY_CTX_get_keygen_info(EVP_PKEY_CTX *ctx, int idx);

// EVP_PKEY_CTX_set_app_data sets |app_data| for |ctx|.
OPENSSL_EXPORT void EVP_PKEY_CTX_set_app_data(EVP_PKEY_CTX *ctx, void *data);

// EVP_PKEY_CTX_get_app_data returns |ctx|'s |app_data|.
OPENSSL_EXPORT void *EVP_PKEY_CTX_get_app_data(EVP_PKEY_CTX *ctx);


// Deprecated functions.

// EVP_PKEY_RSA2 was historically an alternate form for RSA public keys (OID
Expand Down Expand Up @@ -1271,21 +1310,6 @@ OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_set_dsa_paramgen_q_bits(
OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_ctrl_str(EVP_PKEY_CTX *ctx, const char *type,
const char *value);

// EVP_PKEY_CTX keygen no-ops [Deprecated].

typedef int EVP_PKEY_gen_cb(EVP_PKEY_CTX *ctx);

// EVP_PKEY_CTX_set_cb is a no-op.
OPENSSL_EXPORT OPENSSL_DEPRECATED void EVP_PKEY_CTX_set_cb(EVP_PKEY_CTX *ctx, EVP_PKEY_gen_cb *cb);

// EVP_PKEY_CTX_set_app_data is a no-op.
OPENSSL_EXPORT OPENSSL_DEPRECATED void EVP_PKEY_CTX_set_app_data(EVP_PKEY_CTX *ctx, void *data);

// EVP_PKEY_CTX_get_app_data is a no-op. Return value is |NULL|.
OPENSSL_EXPORT OPENSSL_DEPRECATED void *EVP_PKEY_CTX_get_app_data(EVP_PKEY_CTX *ctx);

// EVP_PKEY_CTX_get_keygen_info is a no-op. Return value is 0.
OPENSSL_EXPORT OPENSSL_DEPRECATED int EVP_PKEY_CTX_get_keygen_info(EVP_PKEY_CTX *ctx, int idx);

// Preprocessor compatibility section (hidden).
//
Expand Down
Loading