Skip to content

Commit

Permalink
Add support for EVP_PKEY_CTX callback functions (#1905)
Browse files Browse the repository at this point in the history
We tried to no-op these functions, but it turns out Ruby depends on them
pretty extensively as the interruption mechanism for threads. One of 
Ruby's tests depends on `EVP_PKEY_CTX_get_app_data` to return an
actual value from the callback function, but we return NULL as a no-op. Ruby
seems to depend on the `EVP_PKEY` callback function and relevant
application data to correctly handle interruptions. Based on the relevant
commit messages, the expectation is that the operation is interrupted, but
AWS-LC continues resuming the operation and returns a generated RSA key.
It looks like we may have to consider implementing functionality for
these callback functions. This issue also applies to a test failure in
`test/openssl/test_pkey_dh.rb` and `test/openssl/test_pkey_dsa.rb`. We
probably aren't going to support DSA, but this will need to be applied
to DH somewhere down the line.

* Commits:
* ruby/openssl@88b90fb
* ruby/ruby@d3507e3

### Testing:
new test that verifies this works with `EVP_PKEY_RSA`

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
samuel40791765 authored Oct 7, 2024
1 parent a0f8512 commit c64ec41
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 25 deletions.
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 @@ -2913,3 +2913,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|.
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;
}
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

0 comments on commit c64ec41

Please sign in to comment.