Skip to content

dh: reject generation controls wolfSSL cannot apply #486

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
yosuke-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

dh: reject generation controls wolfSSL cannot apply #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 59 additions & 13 deletions src/wp_dh_kmgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,7 @@ typedef struct wp_DhGenCtx {
char name[WP_MAX_DH_GROUP_NAME_SZ];
/** Number of bits in prime. */
int bits;
/** Length of private key to generate - value ignored. */
int privLen;
/** DH generator parameter to use in generation - value ignored. */
/** Generator requested by the caller. 0 when not requested. */
int generator;
} wp_DhGenCtx;

Expand Down Expand Up @@ -1585,7 +1583,6 @@ static wp_DhGenCtx* wp_dh_gen_init(WOLFPROV_CTX* provCtx,
ctx->provCtx = provCtx;
ctx->selection = selection;
ctx->bits = 2048;
ctx->generator = 2;
}
if (ok) {
if (!wp_dh_gen_set_params(ctx, params)) {
Expand Down Expand Up @@ -1652,6 +1649,8 @@ static int wp_dh_gen_set_params(wp_DhGenCtx* ctx, const OSSL_PARAM params[])
{
int ok = 1;
int bits;
int privLen;
int generator;
const OSSL_PARAM* p;

WOLFPROV_ENTER(WP_LOG_COMP_DH, "wp_dh_gen_set_params");
Expand All @@ -1673,14 +1672,32 @@ static int wp_dh_gen_set_params(wp_DhGenCtx* ctx, const OSSL_PARAM params[])
}
if (ok) {
p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_DH_PRIV_LEN);
if ((p != NULL) && (!OSSL_PARAM_get_int(p, &ctx->privLen))) {
return 0;
if (p != NULL) {
if (!OSSL_PARAM_get_int(p, &privLen)) {
ok = 0;
}
/* wolfSSL takes the private key length from q, or from the size
* of p when there is no q. A requested length cannot be applied. */
if (ok && (privLen != 0)) {
ERR_raise(ERR_LIB_PROV, PROV_R_NOT_SUPPORTED);
ok = 0;
}
}
}
if (ok) {
p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_DH_GENERATOR);
if ((p != NULL) && (!OSSL_PARAM_get_int(p, &ctx->generator))) {
ok = 0;
if (p != NULL) {
if (!OSSL_PARAM_get_int(p, &generator)) {
ok = 0;
}
/* A generator below 2 is not usable. */
if (ok && (generator < 2)) {
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_DATA);
ok = 0;
}
if (ok) {
ctx->generator = generator;
}
}
}
if (ok && (!wp_params_get_utf8_string(params, OSSL_PKEY_PARAM_GROUP_NAME,
Expand All @@ -1707,11 +1724,20 @@ static int wp_dh_gen_parameters(wp_DhGenCtx *ctx, wp_Dh* dh)

WOLFPROV_ENTER(WP_LOG_COMP_DH, "wp_dh_gen_parameters");

rc = wc_DhGenerateParams(&ctx->rng, ctx->bits, &dh->key);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_DhGenerateParams", rc);
/* wc_DhGenerateParams picks the generator itself. OpenSSL applications
* send 2 when the caller named none, so only another value is refused. */
if ((ctx->generator != 0) && (ctx->generator != 2)) {
ERR_raise(ERR_LIB_PROV, PROV_R_NOT_SUPPORTED);
ok = 0;
}
if (ok) {
rc = wc_DhGenerateParams(&ctx->rng, ctx->bits, &dh->key);
if (rc != 0) {
WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_DhGenerateParams",
rc);
ok = 0;
}
}
if (ok) {
dh->bits = mp_count_bits(&dh->key.p);
}
Expand All @@ -1723,7 +1749,8 @@ static int wp_dh_gen_parameters(wp_DhGenCtx *ctx, wp_Dh* dh)
/**
* Copy the group parameters into the DH key object.
*
* Use the template key if available. Otherwise use the group name.
* Use the template key if available. Otherwise use the group name. Fails
* when the caller asked for a generator the group does not have.
*
* @param [in] ctx DH generation context object.
* @param [in, out] dh DH key object.
Expand All @@ -1733,6 +1760,7 @@ static int wp_dh_gen_parameters(wp_DhGenCtx *ctx, wp_Dh* dh)
static int wp_dh_gen_copy_parameters(wp_DhGenCtx *ctx, wp_Dh* dh)
{
int ok = 1;
mp_int gen;

WOLFPROV_ENTER(WP_LOG_COMP_DH, "wp_dh_gen_copy_parameters");

Expand Down Expand Up @@ -1769,6 +1797,23 @@ static int wp_dh_gen_copy_parameters(wp_DhGenCtx *ctx, wp_Dh* dh)
ok = 0;
}

/* A requested generator is met only when the group already has it. */
if (ok && (ctx->generator != 0)) {
if (mp_init(&gen) != MP_OKAY) {
ok = 0;
}
else {
if (mp_set_int(&gen, (unsigned long)ctx->generator) != MP_OKAY) {
ok = 0;
}
if (ok && (mp_cmp(&dh->key.g, &gen) != MP_EQ)) {
ERR_raise(ERR_LIB_PROV, PROV_R_NOT_SUPPORTED);
ok = 0;
}
mp_clear(&gen);
}
}

WOLFPROV_LEAVE(WP_LOG_COMP_DH, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
return ok;
}
Expand Down Expand Up @@ -1990,7 +2035,8 @@ static const OSSL_PARAM* wp_dh_gen_settable_params(wp_DhGenCtx* ctx,
WOLFPROV_CTX* provCtx)
{
/**
* Supported settable parameters for DH generation context.
* Settable parameters for DH generation context. A value that wolfSSL
* cannot apply is rejected when set.
*/
static OSSL_PARAM wp_dh_gen_supported_settable[] = {
OSSL_PARAM_utf8_string(OSSL_PKEY_PARAM_GROUP_NAME, NULL, 0),
Expand Down
232 changes: 232 additions & 0 deletions test/test_dh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1735,4 +1735,236 @@ int test_dh_pgen_min_bits(void *data)
return err;
}

/* The generation interface must not report success for controls wolfSSL
* cannot apply: it picks the generator during parameter generation and
* derives the private key length from the group. */
#define TEST_DH_GEN_PRIV_LEN 256

/* Parameter generation refuses a generator wolfSSL cannot produce. It takes
* 2 because that is what applications send when the caller named none. */
static int test_dh_pgen_generator(int gen, int expectOk)
{
int err = 0;
int rc;
EVP_PKEY_CTX *ctx = NULL;
EVP_PKEY *keyParams = NULL;

ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
if (err == 0) {
err = EVP_PKEY_paramgen_init(ctx) != 1;
}
if (err == 0) {
err = EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx, TEST_DH_MIN_BITS)
!= 1;
}
if (err == 0) {
err = EVP_PKEY_CTX_set_dh_paramgen_generator(ctx, gen) != 1;
if (err != 0) {
PRINT_MSG("set_dh_paramgen_generator refused a usable generator");
}
}
if (err == 0) {
rc = EVP_PKEY_paramgen(ctx, &keyParams);
if (expectOk && (rc != 1)) {
Comment thread
yosuke-wolfssl marked this conversation as resolved.
PRINT_MSG("paramgen refused the generator applications default to");
err = 1;
}
if ((!expectOk) && (rc == 1)) {
PRINT_MSG("paramgen accepted a generator it cannot produce");
err = 1;
}
}

EVP_PKEY_free(keyParams);
EVP_PKEY_CTX_free(ctx);
return err;
}

/* Build a DH parameters key for the group the generator tests use. */
static int test_dh_params_from_group(EVP_PKEY **keyParams)
{
int err;
EVP_PKEY_CTX *ctx = NULL;
OSSL_PARAM params[2];

params[0] = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_GROUP_NAME,
(char*)"ffdhe2048", 0);
params[1] = OSSL_PARAM_construct_end();

ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
if (err == 0) {
err = EVP_PKEY_fromdata_init(ctx) != 1;
}
if (err == 0) {
err = EVP_PKEY_fromdata(ctx, keyParams, EVP_PKEY_KEY_PARAMETERS,
params) != 1;
}

EVP_PKEY_CTX_free(ctx);
return err;
}

/* Key generation takes a generator only when the domain parameters already
* have it, whether they come from a group name or a parameters key. */
static int test_dh_keygen_generator(int gen, int expectOk, int useTemplate)
{
int err = 0;
int rc;
int idx = 0;
EVP_PKEY_CTX *ctx = NULL;
EVP_PKEY *keyParams = NULL;
EVP_PKEY *key = NULL;
BIGNUM *g = NULL;
OSSL_PARAM params[3];

if (useTemplate) {
err = test_dh_params_from_group(&keyParams);
}
else {
params[idx++] = OSSL_PARAM_construct_utf8_string(
OSSL_PKEY_PARAM_GROUP_NAME, (char*)"ffdhe2048", 0);
}
params[idx++] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_DH_GENERATOR,
&gen);
params[idx] = OSSL_PARAM_construct_end();

if (err == 0) {
if (useTemplate) {
ctx = EVP_PKEY_CTX_new_from_pkey(wpLibCtx, keyParams, NULL);
}
else {
ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
}
err = ctx == NULL;
}
if (err == 0) {
err = EVP_PKEY_keygen_init(ctx) != 1;
}
if (err == 0) {
err = EVP_PKEY_CTX_set_params(ctx, params) != 1;
}
if (err == 0) {
rc = EVP_PKEY_generate(ctx, &key);
if (expectOk && (rc != 1)) {
PRINT_MSG("keygen refused the generator the parameters have");
err = 1;
}
if ((!expectOk) && (rc == 1)) {
PRINT_MSG("keygen accepted a generator the parameters lack");
err = 1;
}
}
if ((err == 0) && expectOk) {
err = EVP_PKEY_get_bn_param(key, OSSL_PKEY_PARAM_FFC_G, &g) != 1;
if ((err == 0) && (!BN_is_word(g, (BN_ULONG)gen))) {
PRINT_MSG("keygen produced a generator other than the requested");
err = 1;
}
}

BN_free(g);
EVP_PKEY_free(key);
EVP_PKEY_free(keyParams);
EVP_PKEY_CTX_free(ctx);
return err;
}

int test_dh_pgen_controls(void *data)
{
int err = 0;
int privLen = TEST_DH_GEN_PRIV_LEN;
EVP_PKEY_CTX *ctx = NULL;
OSSL_PARAM params[2];

(void)data;

PRINT_MSG("Testing DH generator and private length generation controls");

PRINT_MSG("Parameter generation refuses a generator it cannot produce");
err = test_dh_pgen_generator(5, 0);
if (err == 0) {
PRINT_MSG("Parameter generation takes the generator 2 applications "
"send by default");
err = test_dh_pgen_generator(2, 1);
}

PRINT_MSG("A generator below 2 is refused when set");
if (err == 0) {
ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
}
if (err == 0) {
err = EVP_PKEY_paramgen_init(ctx) != 1;
}
if (err == 0) {
if (EVP_PKEY_CTX_set_dh_paramgen_generator(ctx, 1) == 1) {
PRINT_MSG("set_dh_paramgen_generator accepted a generator of 1");
err = 1;
}
}
EVP_PKEY_CTX_free(ctx);
ctx = NULL;

if (err == 0) {
PRINT_MSG("Key generation takes the generator the named group has");
err = test_dh_keygen_generator(2, 1, 0);
}
if (err == 0) {
PRINT_MSG("Key generation refuses a generator the named group lacks");
err = test_dh_keygen_generator(5, 0, 0);
}
if (err == 0) {
PRINT_MSG("Key generation takes the generator a parameters key has");
err = test_dh_keygen_generator(2, 1, 1);
}
if (err == 0) {
PRINT_MSG("Key generation refuses a generator a parameters key lacks");
err = test_dh_keygen_generator(5, 0, 1);
}

PRINT_MSG("A private key length is refused when set");
if (err == 0) {
ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
}
if (err == 0) {
err = EVP_PKEY_keygen_init(ctx) != 1;
}
if (err == 0) {
params[0] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_DH_PRIV_LEN,
&privLen);
params[1] = OSSL_PARAM_construct_end();
if (EVP_PKEY_CTX_set_params(ctx, params) == 1) {
PRINT_MSG("set_params accepted a private key length");
err = 1;
}
}
EVP_PKEY_CTX_free(ctx);
ctx = NULL;

PRINT_MSG("A private key length of 0 is taken");
if (err == 0) {
ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "DH", NULL);
err = ctx == NULL;
}
if (err == 0) {
err = EVP_PKEY_keygen_init(ctx) != 1;
}
if (err == 0) {
privLen = 0;
params[0] = OSSL_PARAM_construct_int(OSSL_PKEY_PARAM_DH_PRIV_LEN,
&privLen);
params[1] = OSSL_PARAM_construct_end();
if (EVP_PKEY_CTX_set_params(ctx, params) != 1) {
PRINT_MSG("set_params refused a private key length of 0");
err = 1;
}
}
EVP_PKEY_CTX_free(ctx);

return err;
}

#endif /* WP_HAVE_DH */
1 change: 1 addition & 0 deletions test/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_dh_param_check_explicit, NULL),
TEST_DECL(test_dh_import_group_no_nul, NULL),
TEST_DECL(test_dh_pgen_min_bits, NULL),
TEST_DECL(test_dh_pgen_controls, NULL),
#ifndef WOLFPROV_QUICKTEST
TEST_DECL(test_dh_get_params, NULL),
#endif
Expand Down
1 change: 1 addition & 0 deletions test/unit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ int test_dh_fromdata_oversize(void *data);
int test_dh_param_check_explicit(void *data);
int test_dh_import_group_no_nul(void *data);
int test_dh_pgen_min_bits(void *data);
int test_dh_pgen_controls(void *data);
#endif /* WP_HAVE_DH */

#ifdef WP_HAVE_ECC
Expand Down
Loading