diff options
| -rw-r--r-- | CHANGES | 11 | ||||
| -rw-r--r-- | crypto/ec/ec_asn1.c | 71 | ||||
| -rw-r--r-- | crypto/ec/ec_curve.c | 112 | ||||
| -rw-r--r-- | crypto/ec/ec_lcl.h | 2 | ||||
| -rw-r--r-- | test/ectest.c | 269 |
5 files changed, 460 insertions, 5 deletions
@@ -9,6 +9,17 @@ Changes between 1.1.1c and 1.1.1d [xx XXX xxxx] + *) For built-in EC curves, ensure an EC_GROUP built from the curve name is + used even when parsing explicit parameters, when loading a serialized key + or calling `EC_GROUP_new_from_ecpkparameters()`/ + `EC_GROUP_new_from_ecparameters()`. + This prevents bypass of security hardening and performance gains, + especially for curves with specialized EC_METHODs. + By default, if a key encoded with explicit parameters is loaded and later + serialized, the output is still encoded with explicit parameters, even if + internally a "named" EC_GROUP is used for computation. + [Nicola Tuveri] + *) Compute ECC cofactors if not provided during EC_GROUP construction. Before this change, EC_GROUP_set_generator would accept order and/or cofactor as NULL. After this change, only the cofactor parameter can be NULL. It also diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c index 13c56a621d..8d8dc95e45 100644 --- a/crypto/ec/ec_asn1.c +++ b/crypto/ec/ec_asn1.c @@ -568,10 +568,12 @@ ECPKPARAMETERS *EC_GROUP_get_ecpkparameters(const EC_GROUP *group, EC_GROUP *EC_GROUP_new_from_ecparameters(const ECPARAMETERS *params) { int ok = 0, tmp; - EC_GROUP *ret = NULL; + EC_GROUP *ret = NULL, *dup = NULL; BIGNUM *p = NULL, *a = NULL, *b = NULL; EC_POINT *point = NULL; long field_bits; + int curve_name = NID_undef; + BN_CTX *ctx = NULL; if (!params->fieldID || !params->fieldID->fieldType || !params->fieldID->p.ptr) { @@ -789,18 +791,79 @@ EC_GROUP *EC_GROUP_new_from_ecparameters(const ECPARAMETERS *params) goto err; } + /* + * Check if the explicit parameters group just created matches one of the + * built-in curves. + * + * We create a copy of the group just built, so that we can remove optional + * fields for the lookup: we do this to avoid the possibility that one of + * the optional parameters is used to force the library into using a less + * performant and less secure EC_METHOD instead of the specialized one. + * In any case, `seed` is not really used in any computation, while a + * cofactor different from the one in the built-in table is just + * mathematically wrong anyway and should not be used. + */ + if ((ctx = BN_CTX_new()) == NULL) { + ECerr(EC_F_EC_GROUP_NEW_FROM_ECPARAMETERS, ERR_R_BN_LIB); + goto err; + } + if ((dup = EC_GROUP_dup(ret)) == NULL + || EC_GROUP_set_seed(dup, NULL, 0) != 1 + || !EC_GROUP_set_generator(dup, point, a, NULL)) { + ECerr(EC_F_EC_GROUP_NEW_FROM_ECPARAMETERS, ERR_R_EC_LIB); + goto err; + } + if ((curve_name = ec_curve_nid_from_params(dup, ctx)) != NID_undef) { + /* + * The input explicit parameters successfully matched one of the + * built-in curves: often for built-in curves we have specialized + * methods with better performance and hardening. + * + * In this case we replace the `EC_GROUP` created through explicit + * parameters with one created from a named group. + */ + EC_GROUP *named_group = NULL; + +#ifndef OPENSSL_NO_EC_NISTP_64_GCC_128 + /* + * NID_wap_wsg_idm_ecid_wtls12 and NID_secp224r1 are both aliases for + * the same curve, we prefer the SECP nid when matching explicit + * parameters as that is associated with a specialized EC_METHOD. + */ + if (curve_name == NID_wap_wsg_idm_ecid_wtls12) + curve_name = NID_secp224r1; +#endif /* !def(OPENSSL_NO_EC_NISTP_64_GCC_128) */ + + if ((named_group = EC_GROUP_new_by_curve_name(curve_name)) == NULL) { + ECerr(EC_F_EC_GROUP_NEW_FROM_ECPARAMETERS, ERR_R_EC_LIB); + goto err; + } + EC_GROUP_free(ret); + ret = named_group; + + /* + * Set the flag so that EC_GROUPs created from explicit parameters are + * serialized using explicit parameters by default. + */ + EC_GROUP_set_asn1_flag(ret, OPENSSL_EC_EXPLICIT_CURVE); + } + ok = 1; err: if (!ok) { - EC_GROUP_clear_free(ret); + EC_GROUP_free(ret); ret = NULL; } + EC_GROUP_free(dup); BN_free(p); BN_free(a); BN_free(b); EC_POINT_free(point); + + BN_CTX_free(ctx); + return ret; } @@ -861,7 +924,7 @@ EC_GROUP *d2i_ECPKParameters(EC_GROUP **a, const unsigned char **in, long len) } if (a) { - EC_GROUP_clear_free(*a); + EC_GROUP_free(*a); *a = group; } @@ -909,7 +972,7 @@ EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const unsigned char **in, long len) ret = *a; if (priv_key->parameters) { - EC_GROUP_clear_free(ret->group); + EC_GROUP_free(ret->group); ret->group = EC_GROUP_new_from_ecpkparameters(priv_key->parameters); } diff --git a/crypto/ec/ec_curve.c b/crypto/ec/ec_curve.c index bb1ce196d0..07acf2652c 100644 --- a/crypto/ec/ec_curve.c +++ b/crypto/ec/ec_curve.c @@ -3197,3 +3197,115 @@ int EC_curve_nist2nid(const char *name) } return NID_undef; } + +#define NUM_BN_FIELDS 6 +/* + * Validates EC domain parameter data for known named curves. + * This can be used when a curve is loaded explicitly (without a curve + * name) or to validate that domain parameters have not been modified. + * + * Returns: The nid associated with the found named curve, or NID_undef + * if not found. If there was an error it returns -1. + */ +int ec_curve_nid_from_params(const EC_GROUP *group, BN_CTX *ctx) +{ + int ret = -1, nid, len, field_type, param_len; + size_t i, seed_len; + const unsigned char *seed, *params_seed, *params; + unsigned char *param_bytes = NULL; + const EC_CURVE_DATA *data; + const EC_POINT *generator = NULL; + const EC_METHOD *meth; + const BIGNUM *cofactor = NULL; + /* An array of BIGNUMs for (p, a, b, x, y, order) */ + BIGNUM *bn[NUM_BN_FIELDS] = {NULL, NULL, NULL, NULL, NULL, NULL}; + + meth = EC_GROUP_method_of(group); + if (meth == NULL) + return -1; + /* Use the optional named curve nid as a search field */ + nid = EC_GROUP_get_curve_name(group); + field_type = EC_METHOD_get_field_type(meth); + seed_len = EC_GROUP_get_seed_len(group); + seed = EC_GROUP_get0_seed(group); + cofactor = EC_GROUP_get0_cofactor(group); + + BN_CTX_start(ctx); + + /* + * The built-in curves contains data fields (p, a, b, x, y, order) that are + * all zero-padded to be the same size. The size of the padding is + * determined by either the number of bytes in the field modulus (p) or the + * EC group order, whichever is larger. + */ + param_len = BN_num_bytes(group->order); + len = BN_num_bytes(group->field); + if (len > param_len) + param_len = len; + + /* Allocate space to store the padded data for (p, a, b, x, y, order) */ + param_bytes = OPENSSL_malloc(param_len * NUM_BN_FIELDS); + if (param_bytes == NULL) + goto end; + + /* Create the bignums */ + for (i = 0; i < NUM_BN_FIELDS; ++i) { + if ((bn[i] = BN_CTX_get(ctx)) == NULL) + goto end; + } + /* + * Fill in the bn array with the same values as the internal curves + * i.e. the values are p, a, b, x, y, order. + */ + /* Get p, a & b */ + if (!(EC_GROUP_get_curve(group, bn[0], bn[1], bn[2], ctx) + && ((generator = EC_GROUP_get0_generator(group)) != NULL) + /* Get x & y */ + && EC_POINT_get_affine_coordinates(group, generator, bn[3], bn[4], ctx) + /* Get order */ + && EC_GROUP_get_order(group, bn[5], ctx))) + goto end; + + /* + * Convert the bignum array to bytes that are joined together to form + * a single buffer that contains data for all fields. + * (p, a, b, x, y, order) are all zero padded to be the same size. + */ + for (i = 0; i < NUM_BN_FIELDS; ++i) { + if (BN_bn2binpad(bn[i], ¶m_bytes[i*param_len], param_len) <= 0) + goto end; + } + + for (i = 0; i < curve_list_length; i++) { + const ec_list_element curve = curve_list[i]; + + data = curve.data; + /* Get the raw order byte data */ + params_seed = (const unsigned char *)(data + 1); /* skip header */ + params = params_seed + data->seed_len; + + /* Look for unique fields in the fixed curve data */ + if (data->field_type == field_type + && param_len == data->param_len + && (nid <= 0 || nid == curve.nid) + /* check the optional cofactor (ignore if its zero) */ + && (BN_is_zero(cofactor) + || BN_is_word(cofactor, (const BN_ULONG)curve.data->cofactor)) + /* Check the optional seed (ignore if its not set) */ + && (data->seed_len == 0 || seed_len == 0 + || ((size_t)data->seed_len == seed_len + && memcmp(params_seed, seed, seed_len) == 0)) + /* Check that the groups params match the built-in curve params */ + && memcmp(param_bytes, params, param_len * NUM_BN_FIELDS) + == 0) { + ret = curve.nid; + goto end; + } + } + /* Gets here if the group was not found */ + ret = NID_undef; +end: + OPENSSL_free(param_bytes); + BN_CTX_end(ctx); + return ret; +} diff --git a/crypto/ec/ec_lcl.h b/crypto/ec/ec_lcl.h index e4189d7328..fbdb04ea3a 100644 --- a/crypto/ec/ec_lcl.h +++ b/crypto/ec/ec_lcl.h @@ -595,6 +595,8 @@ int ec_key_simple_generate_key(EC_KEY *eckey); int ec_key_simple_generate_public_key(EC_KEY *eckey); int ec_key_simple_check_key(const EC_KEY *eckey); +int ec_curve_nid_from_params(const EC_GROUP *group, BN_CTX *ctx); + /* EC_METHOD definitions */ struct ec_key_method_st { diff --git a/test/ectest.c b/test/ectest.c index ddc4ea11d6..ebd831ca40 100644 --- a/test/ectest.c +++ b/test/ectest.c @@ -1519,6 +1519,271 @@ static const unsigned char p521_explicit[] = { 0xbb, 0x6f, 0xb7, 0x1e, 0x91, 0x38, 0x64, 0x09, 0x02, 0x01, 0x01, }; +/* + * Sometime we cannot compare nids for equality, as the built-in curve table + * includes aliases with different names for the same curve. + * + * This function returns TRUE (1) if the checked nids are identical, or if they + * alias to the same curve. FALSE (0) otherwise. + */ +static ossl_inline +int are_ec_nids_compatible(int n1d, int n2d) +{ + int ret = 0; + switch (n1d) { +# ifndef OPENSSL_NO_EC2M + case NID_sect113r1: + case NID_wap_wsg_idm_ecid_wtls4: + ret = (n2d == NID_sect113r1 || n2d == NID_wap_wsg_idm_ecid_wtls4); + break; + case NID_sect163k1: + case NID_wap_wsg_idm_ecid_wtls3: + ret = (n2d == NID_sect163k1 || n2d == NID_wap_wsg_idm_ecid_wtls3); + break; + case NID_sect233k1: + case NID_wap_wsg_idm_ecid_wtls10: + ret = (n2d == NID_sect233k1 || n2d == NID_wap_wsg_idm_ecid_wtls10); + break; + case NID_sect233r1: + case NID_wap_wsg_idm_ecid_wtls11: + ret = (n2d == NID_sect233r1 || n2d == NID_wap_wsg_idm_ecid_wtls11); + break; + case NID_X9_62_c2pnb163v1: + case NID_wap_wsg_idm_ecid_wtls5: + ret = (n2d == NID_X9_62_c2pnb163v1 + || n2d == NID_wap_wsg_idm_ecid_wtls5); + break; +# endif /* OPENSSL_NO_EC2M */ + case NID_secp112r1: + case NID_wap_wsg_idm_ecid_wtls6: + ret = (n2d == NID_secp112r1 || n2d == NID_wap_wsg_idm_ecid_wtls6); + break; + case NID_secp160r2: + case NID_wap_wsg_idm_ecid_wtls7: + ret = (n2d == NID_secp160r2 || n2d == NID_wap_wsg_idm_ecid_wtls7); + break; +# ifdef OPENSSL_NO_EC_NISTP_64_GCC_128 + case NID_secp224r1: + case NID_wap_wsg_idm_ecid_wtls12: + ret = (n2d == NID_secp224r1 || n2d == NID_wap_wsg_idm_ecid_wtls12); + break; +# else + /* + * For SEC P-224 we want to ensure that the SECP nid is returned, as + * that is associated with a specialized method. + */ + case NID_wap_wsg_idm_ecid_wtls12: + ret = (n2d == NID_secp224r1); + break; +# endif /* def(OPENSSL_NO_EC_NISTP_64_GCC_128) */ + + default: + ret = (n1d == n2d); + } + return ret; +} + +/* + * This checks that EC_GROUP_bew_from_ecparameters() returns a "named" + * EC_GROUP for built-in curves. + * + * Note that it is possible to retrieve an alternative alias that does not match + * the original nid. + * + * Ensure that the OPENSSL_EC_EXPLICIT_CURVE ASN1 flag is set. + */ +static int check_named_curve_from_ecparameters(int id) +{ + int ret = 0, nid, tnid; + EC_GROUP *group = NULL, *tgroup = NULL, *tmpg = NULL; + const EC_POINT *group_gen = NULL; + EC_POINT *other_gen = NULL; + BIGNUM *group_cofactor = NULL, *other_cofactor = NULL; + BIGNUM *other_gen_x = NULL, *other_gen_y = NULL; + const BIGNUM *group_order = NULL; + BIGNUM *other_order = NULL; + BN_CTX *bn_ctx = NULL; + static const unsigned char invalid_seed[] = "THIS IS NOT A VALID SEED"; + static size_t invalid_seed_len = sizeof(invalid_seed); + ECPARAMETERS *params = NULL, *other_params = NULL; + EC_GROUP *g_ary[8] = {NULL}; + EC_GROUP **g_next = &g_ary[0]; + ECPARAMETERS *p_ary[8] = {NULL}; + ECPARAMETERS **p_next = &p_ary[0]; + + /* Do some setup */ + nid = curves[id].nid; + TEST_note("Curve %s", OBJ_nid2sn(nid)); + if (!TEST_ptr(bn_ctx = BN_CTX_new())) + return ret; + BN_CTX_start(bn_ctx); + + if (/* Allocations */ + !TEST_ptr(group_cofactor = BN_CTX_get(bn_ctx)) + || !TEST_ptr(other_gen_x = BN_CTX_get(bn_ctx)) + || !TEST_ptr(other_gen_y = BN_CTX_get(bn_ctx)) + || !TEST_ptr(other_order = BN_CTX_get(bn_ctx)) + || !TEST_ptr(other_cofactor = BN_CTX_get(bn_ctx)) + /* Generate reference group and params */ + || !TEST_ptr(group = EC_GROUP_new_by_curve_name(nid)) + || !TEST_ptr(params = EC_GROUP_get_ecparameters(group, NULL)) + || !TEST_ptr(group_gen = EC_GROUP_get0_generator(group)) + || !TEST_ptr(group_order = EC_GROUP_get0_order(group)) + || !TEST_true(EC_GROUP_get_cofactor(group, group_cofactor, NULL)) + /* compute `other_*` values */ + || !TEST_ptr(tmpg = EC_GROUP_dup(group)) + || !TEST_ptr(other_gen = EC_POINT_dup(group_gen, group)) + || !TEST_true(EC_POINT_add(group, other_gen, group_gen, group_gen, NULL)) + || !TEST_true(EC_POINT_get_affine_coordinates(group, other_gen, + other_gen_x, other_gen_y, bn_ctx)) + || !TEST_true(BN_copy(other_order, group_order)) + || !TEST_true(BN_add_word(other_order, 1)) + || !TEST_true(BN_copy(other_cofactor, group_cofactor)) + || !TEST_true(BN_add_word(other_cofactor, 1))) + goto err; + + EC_POINT_free(other_gen); + other_gen = NULL; + + if (!TEST_ptr(other_gen = EC_POINT_new(tmpg)) + || !TEST_true(EC_POINT_set_affine_coordinates(tmpg, other_gen, + other_gen_x, other_gen_y, + bn_ctx))) + goto err; + + /* + * ########################### + * # Actual tests start here # + * ########################### + */ + + /* + * Creating a group from built-in explicit parameters returns a + * "named" EC_GROUP + */ + if (!TEST_ptr(tgroup = *g_next++ = EC_GROUP_new_from_ecparameters(params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef)) + goto err; + /* + * We cannot always guarantee the names match, as the built-in table + * contains aliases for the same curve with different names. + */ + if (!TEST_true(are_ec_nids_compatible(nid, tnid))) { + TEST_info("nid = %s, tnid = %s", OBJ_nid2sn(nid), OBJ_nid2sn(tnid)); + goto err; + } + /* Ensure that the OPENSSL_EC_EXPLICIT_CURVE ASN1 flag is set. */ + if (!TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), OPENSSL_EC_EXPLICIT_CURVE)) + goto err; + + /* + * An invalid seed in the parameters should be ignored: expect a "named" + * group. + */ + if (!TEST_int_eq(EC_GROUP_set_seed(tmpg, invalid_seed, invalid_seed_len), + invalid_seed_len) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + || !TEST_true(are_ec_nids_compatible(nid, tnid)) + || !TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), + OPENSSL_EC_EXPLICIT_CURVE)) { + TEST_info("nid = %s, tnid = %s", OBJ_nid2sn(nid), OBJ_nid2sn(tnid)); + goto err; + } + + /* + * A null seed in the parameters should be ignored, as it is optional: + * expect a "named" group. + */ + if (!TEST_int_eq(EC_GROUP_set_seed(tmpg, NULL, 0), 1) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + || !TEST_true(are_ec_nids_compatible(nid, tnid)) + || !TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), + OPENSSL_EC_EXPLICIT_CURVE)) { + TEST_info("nid = %s, tnid = %s", OBJ_nid2sn(nid), OBJ_nid2sn(tnid)); + goto err; + } + + /* + * Check that changing any of the generator parameters does not yield a + * match with the built-in curves + */ + if (/* Other gen, same group order & cofactor */ + !TEST_true(EC_GROUP_set_generator(tmpg, other_gen, group_order, + group_cofactor)) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_eq((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + /* Same gen & cofactor, different order */ + || !TEST_true(EC_GROUP_set_generator(tmpg, group_gen, other_order, + group_cofactor)) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_eq((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + /* The order is not an optional field, so this should fail */ + || !TEST_false(EC_GROUP_set_generator(tmpg, group_gen, NULL, + group_cofactor)) + /* Check that a wrong cofactor is ignored, and we still match */ + || !TEST_true(EC_GROUP_set_generator(tmpg, group_gen, group_order, + other_cofactor)) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + || !TEST_true(are_ec_nids_compatible(nid, tnid)) + || !TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), + OPENSSL_EC_EXPLICIT_CURVE) + /* Check that if the cofactor is not set then it still matches */ + || !TEST_true(EC_GROUP_set_generator(tmpg, group_gen, group_order, + NULL)) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + || !TEST_true(are_ec_nids_compatible(nid, tnid)) + || !TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), + OPENSSL_EC_EXPLICIT_CURVE) + /* check that restoring the generator passes */ + || !TEST_true(EC_GROUP_set_generator(tmpg, group_gen, group_order, + group_cofactor)) + || !TEST_ptr(other_params = *p_next++ = + EC_GROUP_get_ecparameters(tmpg, NULL)) + || !TEST_ptr(tgroup = *g_next++ = + EC_GROUP_new_from_ecparameters(other_params)) + || !TEST_int_ne((tnid = EC_GROUP_get_curve_name(tgroup)), NID_undef) + || !TEST_true(are_ec_nids_compatible(nid, tnid)) + || !TEST_int_eq(EC_GROUP_get_asn1_flag(tgroup), + OPENSSL_EC_EXPLICIT_CURVE)) + goto err; + + ret = 1; +err: + for (g_next = &g_ary[0]; g_next < g_ary + OSSL_NELEM(g_ary); g_next++) + EC_GROUP_free(*g_next); + for (p_next = &p_ary[0]; p_next < p_ary + OSSL_NELEM(g_ary); p_next++) + ECPARAMETERS_free(*p_next); + ECPARAMETERS_free(params); + EC_POINT_free(other_gen); + EC_GROUP_free(tmpg); + EC_GROUP_free(group); + BN_CTX_end(bn_ctx); + BN_CTX_free(bn_ctx); + return ret; +} + static int parameter_test(void) { EC_GROUP *group = NULL, *group2 = NULL; @@ -1667,7 +1932,9 @@ int setup_tests(void) # endif ADD_ALL_TESTS(internal_curve_test, crv_len); ADD_ALL_TESTS(internal_curve_test_method, crv_len); -#endif + + ADD_ALL_TESTS(check_named_curve_from_ecparameters, crv_len); +#endif /* OPENSSL_NO_EC */ return 1; } |