Skip to content

Commit

Permalink
Fixes related to separation of DH and DHX types
Browse files Browse the repository at this point in the history 
Fix dh_rfc5114 option in genpkey.

Fixes #14145
Fixes #13956
Fixes #13952
Fixes #13871
Fixes #14054
Fixes #14444

Updated documentation for app to indicate what options are available for
DH and DHX keys.

DH and DHX now have different keymanager gen_set_params() methods.

Added CHANGES entry to indicate the breaking change.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #14883)
  • Loading branch information
@slontis @t8m
slontis authored and t8m committed Apr 26, 2021
1 parent 6c9bc25 commit f1ffaae
Show file tree
Hide file tree
Showing 18 changed files with 638 additions and 222 deletions.
7 changes: 7 additions & 0 deletions CHANGES.md
View file
Expand Up @@ -23,6 +23,13 @@ OpenSSL 3.0

### Changes between 1.1.1 and 3.0 [xx XXX xxxx]

* For the key types DH and DHX the allowed settable parameters are now different.
Previously (in 1.1.1) these conflicting parameters were allowed, but will now
result in errors. See EVP_PKEY-DH(7) for further details. This affects the
behaviour of openssl-genpkey(1) for DH parameter generation.

*Shane Lontis*

* The default manual page suffix ($MANSUFFIX) has been changed to "ossl"

*Matt Caswell*
Expand Down
43 changes: 11 additions & 32 deletions crypto/dh/dh_pmeth.c
View file
Expand Up @@ -35,7 +35,6 @@ typedef struct {
int pad;
/* message digest used for parameter generation */
const EVP_MD *md;
int rfc5114_param;
int param_nid;
/* Keygen callback info */
int gentmp[2];
Expand Down Expand Up @@ -98,7 +97,6 @@ static int pkey_dh_copy(EVP_PKEY_CTX *dst, const EVP_PKEY_CTX *src)
dctx->paramgen_type = sctx->paramgen_type;
dctx->pad = sctx->pad;
dctx->md = sctx->md;
dctx->rfc5114_param = sctx->rfc5114_param;
dctx->param_nid = sctx->param_nid;

dctx->kdf_type = sctx->kdf_type;
Expand Down Expand Up @@ -156,11 +154,11 @@ static int pkey_dh_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *p2)
case EVP_PKEY_CTRL_DH_RFC5114:
if (p1 < 1 || p1 > 3 || dctx->param_nid != NID_undef)
return -2;
dctx->rfc5114_param = p1;
dctx->param_nid = p1;
return 1;

case EVP_PKEY_CTRL_DH_NID:
if (p1 <= 0 || dctx->rfc5114_param != 0)
if (p1 <= 0 || dctx->param_nid != NID_undef)
return -2;
dctx->param_nid = p1;
return 1;
Expand Down Expand Up @@ -233,11 +231,12 @@ static int pkey_dh_ctrl_str(EVP_PKEY_CTX *ctx,
}
if (strcmp(type, "dh_rfc5114") == 0) {
DH_PKEY_CTX *dctx = ctx->data;
int len;
len = atoi(value);
if (len < 0 || len > 3)
int id;

id = atoi(value);
if (id < 0 || id > 3)
return -2;
dctx->rfc5114_param = len;
dctx->param_nid = id;
return 1;
}
if (strcmp(type, "dh_param") == 0) {
Expand Down Expand Up @@ -331,36 +330,16 @@ static int pkey_dh_paramgen(EVP_PKEY_CTX *ctx,
/*
* Look for a safe prime group for key establishment. Which uses
* either RFC_3526 (modp_XXXX) or RFC_7919 (ffdheXXXX).
* RFC_5114 is also handled here for param_nid = (1..3)
*/
if (dctx->param_nid != NID_undef) {
int type = dctx->param_nid <= 3 ? EVP_PKEY_DHX : EVP_PKEY_DH;

if ((dh = DH_new_by_nid(dctx->param_nid)) == NULL)
return 0;
EVP_PKEY_assign(pkey, EVP_PKEY_DH, dh);
return 1;
}

#ifndef FIPS_MODULE
if (dctx->rfc5114_param) {
switch (dctx->rfc5114_param) {
case 1:
dh = DH_get_1024_160();
break;

case 2:
dh = DH_get_2048_224();
break;

case 3:
dh = DH_get_2048_256();
break;

default:
return -2;
}
EVP_PKEY_assign(pkey, EVP_PKEY_DHX, dh);
EVP_PKEY_assign(pkey, type, dh);
return 1;
}
#endif /* FIPS_MODULE */

if (ctx->pkey_gencb != NULL) {
pcb = BN_GENCB_new();
Expand Down
104 changes: 61 additions & 43 deletions crypto/evp/ctrl_params_translate.c
View file
Expand Up @@ -977,7 +977,7 @@ static int fix_oid(enum state state,
return ret;
}

/* EVP_PKEY_CTRL_DH_NID, ...??? */
/* EVP_PKEY_CTRL_DH_NID */
static int fix_dh_nid(enum state state,
const struct translation_st *translation,
struct translation_ctx_st *ctx)
Expand All @@ -987,7 +987,7 @@ static int fix_dh_nid(enum state state,
if ((ret = default_check(state, translation, ctx)) <= 0)
return ret;

/* This is currently only settable */
/* This is only settable */
if (ctx->action_type != SET)
return 0;

Expand All @@ -997,16 +997,30 @@ static int fix_dh_nid(enum state state,
ctx->p1 = 0;
}

if ((ret = default_fixup_args(state, translation, ctx)) <= 0)
return default_fixup_args(state, translation, ctx);
}

/* EVP_PKEY_CTRL_DH_RFC5114 */
static int fix_dh_nid5114(enum state state,
const struct translation_st *translation,
struct translation_ctx_st *ctx)
{
int ret;

if ((ret = default_check(state, translation, ctx)) <= 0)
return ret;

if (state == PRE_PARAMS_TO_CTRL) {
ctx->p1 =
ossl_ffc_named_group_get_uid(ossl_ffc_name_to_dh_named_group(ctx->p2));
ctx->p2 = NULL;
/* This is only settable */
if (ctx->action_type != SET)
return 0;

if (state == PRE_CTRL_STR_TO_PARAMS) {
ctx->p2 = (char *)ossl_ffc_named_group_get_name
(ossl_ffc_uid_to_dh_named_group(atoi(ctx->p2)));
ctx->p1 = 0;
}

return ret;
return default_fixup_args(state, translation, ctx);
}

/* EVP_PKEY_CTRL_DH_PARAMGEN_TYPE */
Expand All @@ -1019,24 +1033,16 @@ static int fix_dh_paramgen_type(enum state state,
if ((ret = default_check(state, translation, ctx)) <= 0)
return ret;

/* This is currently only settable */
/* This is only settable */
if (ctx->action_type != SET)
return 0;

if (state == PRE_CTRL_TO_PARAMS) {
ctx->p2 = (char *)ossl_dh_gen_type_id2name(ctx->p1);
ctx->p1 = 0;
}

if ((ret = default_fixup_args(state, translation, ctx)) <= 0)
return ret;

if (state == PRE_PARAMS_TO_CTRL) {
ctx->p1 = ossl_dh_gen_type_name2id(ctx->p2);
ctx->p2 = NULL;
if (state == PRE_CTRL_STR_TO_PARAMS) {
ctx->p2 = (char *)ossl_dh_gen_type_id2name(atoi(ctx->p2));
ctx->p1 = strlen(ctx->p2);
}

return ret;
return default_fixup_args(state, translation, ctx);
}

/* EVP_PKEY_CTRL_EC_PARAM_ENC */
Expand Down Expand Up @@ -1927,35 +1933,47 @@ static const struct translation_st evp_pkey_ctx_translations[] = {
EVP_PKEY_CTRL_GET_DH_KDF_OID, NULL, NULL,
OSSL_KDF_PARAM_CEK_ALG, OSSL_PARAM_UTF8_STRING, fix_oid },

{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_DERIVE,
EVP_PKEY_CTRL_DH_PAD, "dh_pad", NULL,
OSSL_EXCHANGE_PARAM_PAD, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
/* DHX Keygen Parameters that are shared with DH */
{ SET, EVP_PKEY_DHX, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_TYPE, "dh_paramgen_type", NULL,
OSSL_PKEY_PARAM_FFC_TYPE, OSSL_PARAM_UTF8_STRING, fix_dh_paramgen_type },
{ SET, EVP_PKEY_DHX, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_PRIME_LEN, "dh_paramgen_prime_len", NULL,
OSSL_PKEY_PARAM_FFC_PBITS, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
{ SET, EVP_PKEY_DHX, 0, EVP_PKEY_OP_PARAMGEN | EVP_PKEY_OP_KEYGEN,
EVP_PKEY_CTRL_DH_NID, "dh_param", NULL,
OSSL_PKEY_PARAM_GROUP_NAME, OSSL_PARAM_UTF8_STRING, NULL },
{ SET, EVP_PKEY_DHX, 0, EVP_PKEY_OP_PARAMGEN | EVP_PKEY_OP_KEYGEN,
EVP_PKEY_CTRL_DH_RFC5114, "dh_rfc5114", NULL,
OSSL_PKEY_PARAM_GROUP_NAME, OSSL_PARAM_UTF8_STRING, fix_dh_nid5114 },

/* DH Keygen Parameters that are shared with DHX */
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_TYPE, "dh_paramgen_type", NULL,
OSSL_PKEY_PARAM_FFC_TYPE, OSSL_PARAM_UTF8_STRING, fix_dh_paramgen_type },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_PRIME_LEN, "dh_paramgen_prime_len", NULL,
OSSL_PKEY_PARAM_FFC_PBITS, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN | EVP_PKEY_OP_KEYGEN,
EVP_PKEY_CTRL_DH_NID, "dh_param", NULL,
OSSL_PKEY_PARAM_GROUP_NAME, OSSL_PARAM_UTF8_STRING, fix_dh_nid },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_PRIME_LEN, NULL, NULL,
OSSL_PKEY_PARAM_FFC_PBITS, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_SUBPRIME_LEN, "dh_paramgen_subprime_len", NULL,
OSSL_PKEY_PARAM_FFC_QBITS, OSSL_PARAM_UNSIGNED_INTEGER, NULL },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN | EVP_PKEY_OP_KEYGEN,
EVP_PKEY_CTRL_DH_RFC5114, "dh_rfc5114", NULL,
OSSL_PKEY_PARAM_GROUP_NAME, OSSL_PARAM_UTF8_STRING, fix_dh_nid5114 },

/* DH specific Keygen Parameters */
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_GENERATOR, "dh_paramgen_generator", NULL,
OSSL_PKEY_PARAM_DH_GENERATOR, OSSL_PARAM_INTEGER, NULL },
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_TYPE, "dh_paramgen_type", NULL,
OSSL_PKEY_PARAM_FFC_TYPE, OSSL_PARAM_UTF8_STRING, fix_dh_paramgen_type },
/*
* This is know to be incorrect, will be fixed and enabled when the
* underlying code is corrected.
* Until then, we simply don't support it here.
*/
#if 0
{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_RFC5114, "dh_rfc5114", NULL,
OSSL_PKEY_PARAM_GROUP_NAME, OSSL_PARAM_INTEGER, NULL },
#endif

/* DHX specific Keygen Parameters */
{ SET, EVP_PKEY_DHX, 0, EVP_PKEY_OP_PARAMGEN,
EVP_PKEY_CTRL_DH_PARAMGEN_SUBPRIME_LEN, "dh_paramgen_subprime_len", NULL,
OSSL_PKEY_PARAM_FFC_QBITS, OSSL_PARAM_UNSIGNED_INTEGER, NULL },

{ SET, EVP_PKEY_DH, 0, EVP_PKEY_OP_DERIVE,
EVP_PKEY_CTRL_DH_PAD, "dh_pad", NULL,
OSSL_EXCHANGE_PARAM_PAD, OSSL_PARAM_UNSIGNED_INTEGER, NULL },

/*-
* DSA
Expand Down
29 changes: 22 additions & 7 deletions crypto/evp/dh_support.c
View file
Expand Up @@ -15,14 +15,25 @@
typedef struct dh_name2id_st{
const char *name;
int id;
int type;
} DH_GENTYPE_NAME2ID;

static const DH_GENTYPE_NAME2ID dhtype2id[]=
/* Indicates that the paramgen_type can be used for either DH or DHX */
#define TYPE_ANY -1
#ifndef OPENSSL_NO_DH
# define TYPE_DH DH_FLAG_TYPE_DH
# define TYPE_DHX DH_FLAG_TYPE_DHX
#else
# define TYPE_DH 0
# define TYPE_DHX 0
#endif

static const DH_GENTYPE_NAME2ID dhtype2id[] =
{
{ "fips186_4", DH_PARAMGEN_TYPE_FIPS_186_4 },
{ "fips186_2", DH_PARAMGEN_TYPE_FIPS_186_2 },
{ "group", DH_PARAMGEN_TYPE_GROUP },
{ "generator", DH_PARAMGEN_TYPE_GENERATOR }
{ "group", DH_PARAMGEN_TYPE_GROUP, TYPE_ANY },
{ "generator", DH_PARAMGEN_TYPE_GENERATOR, TYPE_DH },
{ "fips186_4", DH_PARAMGEN_TYPE_FIPS_186_4, TYPE_DHX },
{ "fips186_2", DH_PARAMGEN_TYPE_FIPS_186_2, TYPE_DHX },
};

const char *ossl_dh_gen_type_id2name(int id)
Expand All @@ -36,13 +47,17 @@ const char *ossl_dh_gen_type_id2name(int id)
return NULL;
}

int ossl_dh_gen_type_name2id(const char *name)
#ifndef OPENSSL_NO_DH
int ossl_dh_gen_type_name2id(const char *name, int type)
{
size_t i;

for (i = 0; i < OSSL_NELEM(dhtype2id); ++i) {
if (strcmp(dhtype2id[i].name, name) == 0)
if ((dhtype2id[i].type == TYPE_ANY
|| type == dhtype2id[i].type)
&& strcmp(dhtype2id[i].name, name) == 0)
return dhtype2id[i].id;
}
return -1;
}
#endif
33 changes: 29 additions & 4 deletions crypto/evp/p_lib.c
View file
Expand Up @@ -890,13 +890,38 @@ IMPLEMENT_ECX_VARIANT(ED448)

# if !defined(OPENSSL_NO_DH) && !defined(OPENSSL_NO_DEPRECATED_3_0)

int EVP_PKEY_set1_DH(EVP_PKEY *pkey, DH *key)
int EVP_PKEY_set1_DH(EVP_PKEY *pkey, DH *dhkey)
{
int type = DH_get0_q(key) == NULL ? EVP_PKEY_DH : EVP_PKEY_DHX;
int ret = EVP_PKEY_assign(pkey, type, key);
int ret, type;

/*
* ossl_dh_is_named_safe_prime_group() returns 1 for named safe prime groups
* related to ffdhe and modp (which cache q = (p - 1) / 2),
* and returns 0 for all other dh parameter generation types including
* RFC5114 named groups.
*
* The EVP_PKEY_DH type is used for dh parameter generation types:
* - named safe prime groups related to ffdhe and modp
* - safe prime generator
*
* The type EVP_PKEY_DHX is used for dh parameter generation types
* - fips186-4 and fips186-2
* - rfc5114 named groups.
*
* The EVP_PKEY_DH type is used to save PKCS#3 data than can be stored
* without a q value.
* The EVP_PKEY_DHX type is used to save X9.42 data that requires the
* q value to be stored.
*/
if (ossl_dh_is_named_safe_prime_group(dhkey))
type = EVP_PKEY_DH;
else
type = DH_get0_q(dhkey) == NULL ? EVP_PKEY_DH : EVP_PKEY_DHX;

ret = EVP_PKEY_assign(pkey, type, dhkey);

if (ret)
DH_up_ref(key);
DH_up_ref(dhkey);
return ret;
}

Expand Down
4 changes: 1 addition & 3 deletions crypto/ffc/ffc_dh.c
View file
Expand Up @@ -113,9 +113,7 @@ const DH_NAMED_GROUP *ossl_ffc_numbers_to_dh_named_group(const BIGNUM *p,
if (BN_cmp(p, dh_named_groups[i].p) == 0
&& BN_cmp(g, dh_named_groups[i].g) == 0
/* Verify q is correct if it exists */
&& ((q != NULL && BN_cmp(q, dh_named_groups[i].q) == 0)
/* Do not match RFC 5114 groups without q */
|| (q == NULL && dh_named_groups[i].uid > 3)))
&& (q == NULL || BN_cmp(q, dh_named_groups[i].q) == 0))
return &dh_named_groups[i];
}
return NULL;
Expand Down

0 comments on commit f1ffaae

Please sign in to comment.