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.

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

Jump to bottom

Fix dh_rfc5114 option in genpkey. #14883

Closed
wants to merge 8 commits into from
Closed

Fix dh_rfc5114 option in genpkey. #14883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
89a0948
Fix dh_rfc5114 option in genpkey.
slontis Apr 15, 2021
d1816fe
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 20, 2021
3e36fce
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 21, 2021
fdb07bc
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 21, 2021
9aad974
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 21, 2021
c9a0bd3
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 22, 2021
09a2e18
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 22, 2021
19bed35
fixup! Fix dh_rfc5114 option in genpkey.
slontis Apr 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGES.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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) {
slontis marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
Expand Up @@ -884,13 +884,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
Open in desktop
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)))
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE - this line was a hack to fix a decoder test issue - the root cause was fixed so this is no longer needed. This allows RFC5114 to be able to be loaded correctly as a named group. (The original problem was that the DH asn1 callback was not setting the nid - but it was for DHX asn1)

&& (q == NULL || BN_cmp(q, dh_named_groups[i].q) == 0))
return &dh_named_groups[i];
}
return NULL;
Expand Down