From 926b21117df939241f1cd63f2f9e3ab87819f0ed Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 21 May 2018 15:24:56 +0100 Subject: [PATCH] Improve compatibility of point and curve checks We check that the curve name associated with the point is the same as that for the curve. Fixes #6302 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/6323) (cherry picked from commit b14e60155009f4f1d168e220fa01cd2b75557b72) --- crypto/ec/ec2_smpl.c | 1 + crypto/ec/ec_curve.c | 4 ++-- crypto/ec/ec_lcl.h | 16 ++++++++++++++++ crypto/ec/ec_lib.c | 39 ++++++++++++++++++++++----------------- crypto/ec/ec_mult.c | 4 ++-- crypto/ec/ec_oct.c | 8 ++++---- crypto/ec/ecp_nistz256.c | 4 ++-- crypto/ec/ecp_smpl.c | 1 + 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/crypto/ec/ec2_smpl.c b/crypto/ec/ec2_smpl.c index cfeef5fc74..1bd96eeed0 100644 --- a/crypto/ec/ec2_smpl.c +++ b/crypto/ec/ec2_smpl.c @@ -330,6 +330,7 @@ int ec_GF2m_simple_point_copy(EC_POINT *dest, const EC_POINT *src) if (!BN_copy(dest->Z, src->Z)) return 0; dest->Z_is_one = src->Z_is_one; + dest->curve_name = src->curve_name; return 1; } diff --git a/crypto/ec/ec_curve.c b/crypto/ec/ec_curve.c index f8a3846fd5..9634f3fcf0 100644 --- a/crypto/ec/ec_curve.c +++ b/crypto/ec/ec_curve.c @@ -3036,6 +3036,8 @@ static EC_GROUP *ec_group_new_from_data(const ec_list_element curve) } #endif + EC_GROUP_set_curve_name(group, curve.nid); + if ((P = EC_POINT_new(group)) == NULL) { ECerr(EC_F_EC_GROUP_NEW_FROM_DATA, ERR_R_EC_LIB); goto err; @@ -3101,8 +3103,6 @@ EC_GROUP *EC_GROUP_new_by_curve_name(int nid) return NULL; } - EC_GROUP_set_curve_name(ret, nid); - return ret; } diff --git a/crypto/ec/ec_lcl.h b/crypto/ec/ec_lcl.h index ded35a72a0..9d1974b2b1 100644 --- a/crypto/ec/ec_lcl.h +++ b/crypto/ec/ec_lcl.h @@ -269,6 +269,8 @@ struct ec_key_st { struct ec_point_st { const EC_METHOD *meth; + /* NID for the curve if known */ + int curve_name; /* * All members except 'meth' are handled by the method functions, even if * they appear generic @@ -281,6 +283,20 @@ struct ec_point_st { * special case */ }; + +static ossl_inline int ec_point_is_compat(const EC_POINT *point, + const EC_GROUP *group) +{ + if (group->meth != point->meth + || (group->curve_name != 0 + && point->curve_name != 0 + && group->curve_name != point->curve_name)) + return 0; + + return 1; +} + + NISTP224_PRE_COMP *EC_nistp224_pre_comp_dup(NISTP224_PRE_COMP *); NISTP256_PRE_COMP *EC_nistp256_pre_comp_dup(NISTP256_PRE_COMP *); NISTP521_PRE_COMP *EC_nistp521_pre_comp_dup(NISTP521_PRE_COMP *); diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c index 7cb4bfee28..95505891c6 100644 --- a/crypto/ec/ec_lib.c +++ b/crypto/ec/ec_lib.c @@ -140,6 +140,8 @@ int EC_GROUP_copy(EC_GROUP *dest, const EC_GROUP *src) if (dest == src) return 1; + dest->curve_name = src->curve_name; + /* Copy precomputed */ dest->pre_comp_type = src->pre_comp_type; switch (src->pre_comp_type) { @@ -202,7 +204,6 @@ int EC_GROUP_copy(EC_GROUP *dest, const EC_GROUP *src) return 0; } - dest->curve_name = src->curve_name; dest->asn1_flag = src->asn1_flag; dest->asn1_form = src->asn1_form; @@ -563,6 +564,7 @@ EC_POINT *EC_POINT_new(const EC_GROUP *group) } ret->meth = group->meth; + ret->curve_name = group->curve_name; if (!ret->meth->point_init(ret)) { OPENSSL_free(ret); @@ -600,7 +602,10 @@ int EC_POINT_copy(EC_POINT *dest, const EC_POINT *src) ECerr(EC_F_EC_POINT_COPY, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (dest->meth != src->meth) { + if (dest->meth != src->meth + || (dest->curve_name != src->curve_name + && dest->curve_name != 0 + && src->curve_name != 0)) { ECerr(EC_F_EC_POINT_COPY, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -657,7 +662,7 @@ int EC_POINT_set_Jprojective_coordinates_GFp(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_SET_JPROJECTIVE_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -676,7 +681,7 @@ int EC_POINT_get_Jprojective_coordinates_GFp(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_GET_JPROJECTIVE_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -694,7 +699,7 @@ int EC_POINT_set_affine_coordinates_GFp(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -720,7 +725,7 @@ int EC_POINT_set_affine_coordinates_GF2m(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_SET_AFFINE_COORDINATES_GF2M, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -746,7 +751,7 @@ int EC_POINT_get_affine_coordinates_GFp(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_GET_AFFINE_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -764,7 +769,7 @@ int EC_POINT_get_affine_coordinates_GF2m(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_GET_AFFINE_COORDINATES_GF2M, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -780,8 +785,8 @@ int EC_POINT_add(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a, ECerr(EC_F_EC_POINT_ADD, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if ((group->meth != r->meth) || (r->meth != a->meth) - || (a->meth != b->meth)) { + if (!ec_point_is_compat(r, group) || !ec_point_is_compat(a, group) + || !ec_point_is_compat(b, group)) { ECerr(EC_F_EC_POINT_ADD, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -795,7 +800,7 @@ int EC_POINT_dbl(const EC_GROUP *group, EC_POINT *r, const EC_POINT *a, ECerr(EC_F_EC_POINT_DBL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if ((group->meth != r->meth) || (r->meth != a->meth)) { + if (!ec_point_is_compat(r, group) || !ec_point_is_compat(a, group)) { ECerr(EC_F_EC_POINT_DBL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -808,7 +813,7 @@ int EC_POINT_invert(const EC_GROUP *group, EC_POINT *a, BN_CTX *ctx) ECerr(EC_F_EC_POINT_INVERT, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != a->meth) { + if (!ec_point_is_compat(a, group)) { ECerr(EC_F_EC_POINT_INVERT, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -822,7 +827,7 @@ int EC_POINT_is_at_infinity(const EC_GROUP *group, const EC_POINT *point) ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_IS_AT_INFINITY, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -843,7 +848,7 @@ int EC_POINT_is_on_curve(const EC_GROUP *group, const EC_POINT *point, ECerr(EC_F_EC_POINT_IS_ON_CURVE, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_IS_ON_CURVE, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -857,7 +862,7 @@ int EC_POINT_cmp(const EC_GROUP *group, const EC_POINT *a, const EC_POINT *b, ECerr(EC_F_EC_POINT_CMP, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return -1; } - if ((group->meth != a->meth) || (a->meth != b->meth)) { + if (!ec_point_is_compat(a, group) || !ec_point_is_compat(b, group)) { ECerr(EC_F_EC_POINT_CMP, EC_R_INCOMPATIBLE_OBJECTS); return -1; } @@ -870,7 +875,7 @@ int EC_POINT_make_affine(const EC_GROUP *group, EC_POINT *point, BN_CTX *ctx) ECerr(EC_F_EC_POINT_MAKE_AFFINE, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_MAKE_AFFINE, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -887,7 +892,7 @@ int EC_POINTs_make_affine(const EC_GROUP *group, size_t num, return 0; } for (i = 0; i < num; i++) { - if (group->meth != points[i]->meth) { + if (!ec_point_is_compat(points[i], group)) { ECerr(EC_F_EC_POINTS_MAKE_AFFINE, EC_R_INCOMPATIBLE_OBJECTS); return 0; } diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c index f69271e0ee..cac9591fac 100644 --- a/crypto/ec/ec_mult.c +++ b/crypto/ec/ec_mult.c @@ -371,7 +371,7 @@ int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar, * precomputation is not available */ int ret = 0; - if (group->meth != r->meth) { + if (!ec_point_is_compat(r, group)) { ECerr(EC_F_EC_WNAF_MUL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -407,7 +407,7 @@ int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar, } for (i = 0; i < num; i++) { - if (group->meth != points[i]->meth) { + if (!ec_point_is_compat(points[i], group)) { ECerr(EC_F_EC_WNAF_MUL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } diff --git a/crypto/ec/ec_oct.c b/crypto/ec/ec_oct.c index effc42a344..fd1fb0b8be 100644 --- a/crypto/ec/ec_oct.c +++ b/crypto/ec/ec_oct.c @@ -30,7 +30,7 @@ int EC_POINT_set_compressed_coordinates_GFp(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_SET_COMPRESSED_COORDINATES_GFP, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -66,7 +66,7 @@ int EC_POINT_set_compressed_coordinates_GF2m(const EC_GROUP *group, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_SET_COMPRESSED_COORDINATES_GF2M, EC_R_INCOMPATIBLE_OBJECTS); return 0; @@ -93,7 +93,7 @@ size_t EC_POINT_point2oct(const EC_GROUP *group, const EC_POINT *point, ECerr(EC_F_EC_POINT_POINT2OCT, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_POINT2OCT, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -123,7 +123,7 @@ int EC_POINT_oct2point(const EC_GROUP *group, EC_POINT *point, ECerr(EC_F_EC_POINT_OCT2POINT, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); return 0; } - if (group->meth != point->meth) { + if (!ec_point_is_compat(point, group)) { ECerr(EC_F_EC_POINT_OCT2POINT, EC_R_INCOMPATIBLE_OBJECTS); return 0; } diff --git a/crypto/ec/ecp_nistz256.c b/crypto/ec/ecp_nistz256.c index 246189833e..153f39012a 100644 --- a/crypto/ec/ecp_nistz256.c +++ b/crypto/ec/ecp_nistz256.c @@ -1168,7 +1168,7 @@ __owur static int ecp_nistz256_points_mul(const EC_GROUP *group, return 0; } - if (group->meth != r->meth) { + if (!ec_point_is_compat(r, group)) { ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } @@ -1177,7 +1177,7 @@ __owur static int ecp_nistz256_points_mul(const EC_GROUP *group, return EC_POINT_set_to_infinity(group, r); for (j = 0; j < num; j++) { - if (group->meth != points[j]->meth) { + if (!ec_point_is_compat(points[j], group)) { ECerr(EC_F_ECP_NISTZ256_POINTS_MUL, EC_R_INCOMPATIBLE_OBJECTS); return 0; } diff --git a/crypto/ec/ecp_smpl.c b/crypto/ec/ecp_smpl.c index abd3795046..d6a61d146d 100644 --- a/crypto/ec/ecp_smpl.c +++ b/crypto/ec/ecp_smpl.c @@ -352,6 +352,7 @@ int ec_GFp_simple_point_copy(EC_POINT *dest, const EC_POINT *src) if (!BN_copy(dest->Z, src->Z)) return 0; dest->Z_is_one = src->Z_is_one; + dest->curve_name = src->curve_name; return 1; } -- 2.34.1