From 4186b711f46cb353fa18dd06ea424bb452f8d310 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 16 Jun 2016 20:40:18 -0400 Subject: [PATCH] Don't bother storing the cofactor. It's always one. We don't support other kinds of curves with this framework. (Curve25519 uses a much simpler API.) This also allows us to remove the check_pub_key_order logic. Change-Id: Ic15e1ecd68662b838c76b1e0aa15c3a93200d744 Reviewed-on: https://boringssl-review.googlesource.com/8350 Reviewed-by: Adam Langley --- crypto/ec/ec.c | 22 +++++----------------- crypto/ec/ec_key.c | 8 -------- crypto/ec/ec_montgomery.c | 21 --------------------- crypto/ec/internal.h | 14 +------------- crypto/ec/p224-64.c | 1 - crypto/ec/p256-64.c | 1 - crypto/ec/p256-x86_64.c | 1 - 7 files changed, 6 insertions(+), 62 deletions(-) diff --git a/crypto/ec/ec.c b/crypto/ec/ec.c index 20a71a31..75e906bd 100644 --- a/crypto/ec/ec.c +++ b/crypto/ec/ec.c @@ -82,7 +82,6 @@ static const struct curve_data P224 = { "NIST P-224", 28, - 1, {/* p */ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -112,7 +111,6 @@ static const struct curve_data P224 = { static const struct curve_data P256 = { "NIST P-256", 32, - 1, {/* p */ 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, @@ -141,7 +139,6 @@ static const struct curve_data P256 = { static const struct curve_data P384 = { "NIST P-384", 48, - 1, {/* p */ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, @@ -176,7 +173,6 @@ static const struct curve_data P384 = { static const struct curve_data P521 = { "NIST P-521", 66, - 1, {/* p */ 0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, @@ -358,7 +354,6 @@ EC_GROUP *ec_group_new(const EC_METHOD *meth) { ret->meth = meth; BN_init(&ret->order); - BN_init(&ret->cofactor); if (!meth->group_init(ret)) { OPENSSL_free(ret); @@ -406,8 +401,7 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator, group->generator = EC_POINT_new(group); return group->generator != NULL && EC_POINT_copy(group->generator, generator) && - BN_copy(&group->order, order) && - BN_copy(&group->cofactor, cofactor); + BN_copy(&group->order, order); } static EC_GROUP *ec_group_new_from_data(unsigned built_in_index) { @@ -464,8 +458,7 @@ static EC_GROUP *ec_group_new_from_data(unsigned built_in_index) { OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB); goto err; } - if (!BN_bin2bn(params + 5 * param_len, param_len, &group->order) || - !BN_set_word(&group->cofactor, (BN_ULONG)data->cofactor)) { + if (!BN_bin2bn(params + 5 * param_len, param_len, &group->order)) { OPENSSL_PUT_ERROR(EC, ERR_R_BN_LIB); goto err; } @@ -528,7 +521,6 @@ void EC_GROUP_free(EC_GROUP *group) { EC_POINT_free(group->generator); BN_free(&group->order); - BN_free(&group->cofactor); OPENSSL_free(group); } @@ -563,8 +555,7 @@ int ec_group_copy(EC_GROUP *dest, const EC_GROUP *src) { dest->generator = NULL; } - if (!BN_copy(&dest->order, &src->order) || - !BN_copy(&dest->cofactor, &src->cofactor)) { + if (!BN_copy(&dest->order, &src->order)) { return 0; } @@ -628,11 +619,8 @@ int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order, BN_CTX *ctx) { int EC_GROUP_get_cofactor(const EC_GROUP *group, BIGNUM *cofactor, BN_CTX *ctx) { - if (!BN_copy(cofactor, &group->cofactor)) { - return 0; - } - - return !BN_is_zero(&group->cofactor); + /* All |EC_GROUP|s have cofactor 1. */ + return BN_set_word(cofactor, 1); } int EC_GROUP_get_curve_GFp(const EC_GROUP *group, BIGNUM *out_p, BIGNUM *out_a, diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c index fee71fed..dcacdfa4 100644 --- a/crypto/ec/ec_key.c +++ b/crypto/ec/ec_key.c @@ -317,14 +317,6 @@ int EC_KEY_check_key(const EC_KEY *eckey) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_IS_NOT_ON_CURVE); goto err; } - /* TODO(fork): can this be skipped if the cofactor is one or if we're about - * to check the private key, below? */ - if (eckey->group->meth->check_pub_key_order != NULL && - !eckey->group->meth->check_pub_key_order(eckey->group, eckey->pub_key, - ctx)) { - OPENSSL_PUT_ERROR(EC, EC_R_WRONG_ORDER); - goto err; - } /* in case the priv_key is present : * check if generator * priv_key == pub_key */ diff --git a/crypto/ec/ec_montgomery.c b/crypto/ec/ec_montgomery.c index 35df3651..215867de 100644 --- a/crypto/ec/ec_montgomery.c +++ b/crypto/ec/ec_montgomery.c @@ -195,26 +195,6 @@ int ec_GFp_mont_field_decode(const EC_GROUP *group, BIGNUM *r, const BIGNUM *a, return BN_from_montgomery(r, a, group->mont, ctx); } -static int ec_GFp_mont_check_pub_key_order(const EC_GROUP *group, - const EC_POINT* pub_key, - BN_CTX *ctx) { - EC_POINT *point = EC_POINT_new(group); - int ret = 0; - - if (point == NULL || - !ec_wNAF_mul(group, point, NULL, pub_key, EC_GROUP_get0_order(group), - ctx) || - !EC_POINT_is_at_infinity(group, point)) { - goto err; - } - - ret = 1; - -err: - EC_POINT_free(point); - return ret; -} - static int ec_GFp_mont_point_get_affine_coordinates(const EC_GROUP *group, const EC_POINT *point, BIGNUM *x, BIGNUM *y, @@ -312,7 +292,6 @@ const EC_METHOD *EC_GFp_mont_method(void) { ec_GFp_mont_group_set_curve, ec_GFp_mont_point_get_affine_coordinates, ec_wNAF_mul /* XXX: Not constant time. */, - ec_GFp_mont_check_pub_key_order, ec_GFp_mont_field_mul, ec_GFp_mont_field_sqr, ec_GFp_mont_field_encode, diff --git a/crypto/ec/internal.h b/crypto/ec/internal.h index f2cbb961..10770d94 100644 --- a/crypto/ec/internal.h +++ b/crypto/ec/internal.h @@ -96,15 +96,6 @@ struct ec_method_st { int (*mul)(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar, const EC_POINT *p, const BIGNUM *p_scalar, BN_CTX *ctx); - /* |check_pub_key_order| checks that the public key is in the proper subgroup - * by checking that |pub_key*group->order| is the point at infinity. This may - * be NULL for |EC_METHOD|s specialized for prime-order curves (i.e. with - * cofactor one), as this check is not necessary for such curves (See section - * A.3 of the NSA's "Suite B Implementer's Guide to FIPS 186-3 - * (ECDSA)"). */ - int (*check_pub_key_order)(const EC_GROUP *group, const EC_POINT *pub_key, - BN_CTX *ctx); - /* 'field_mul' and 'field_sqr' can be used by 'add' and 'dbl' so that the * same implementations of point operations can be used with different * optimized implementations of expensive field operations: */ @@ -124,7 +115,7 @@ struct ec_group_st { const EC_METHOD *meth; EC_POINT *generator; - BIGNUM order, cofactor; + BIGNUM order; int curve_name; /* optional NID for named curve */ @@ -260,9 +251,6 @@ struct curve_data { const char *comment; /* param_len is the number of bytes needed to store a field element. */ uint8_t param_len; - /* cofactor is the cofactor of the group (i.e. the number of elements in the - * group divided by the size of the main subgroup. */ - uint8_t cofactor; /* promoted to BN_ULONG */ /* data points to an array of 6*|param_len| bytes which hold the field * elements of the following (in big-endian order): prime, a, b, generator x, * generator y, order. */ diff --git a/crypto/ec/p224-64.c b/crypto/ec/p224-64.c index 7bf889c9..1b09cb90 100644 --- a/crypto/ec/p224-64.c +++ b/crypto/ec/p224-64.c @@ -1186,7 +1186,6 @@ const EC_METHOD *EC_GFp_nistp224_method(void) { ec_GFp_simple_group_set_curve, ec_GFp_nistp224_point_get_affine_coordinates, ec_GFp_nistp224_points_mul, - 0 /* check_pub_key_order */, ec_GFp_simple_field_mul, ec_GFp_simple_field_sqr, 0 /* field_encode */, diff --git a/crypto/ec/p256-64.c b/crypto/ec/p256-64.c index c4259b62..31bf0adb 100644 --- a/crypto/ec/p256-64.c +++ b/crypto/ec/p256-64.c @@ -1742,7 +1742,6 @@ const EC_METHOD *EC_GFp_nistp256_method(void) { ec_GFp_simple_group_set_curve, ec_GFp_nistp256_point_get_affine_coordinates, ec_GFp_nistp256_points_mul, - 0 /* check_pub_key_order */, ec_GFp_simple_field_mul, ec_GFp_simple_field_sqr, 0 /* field_encode */, 0 /* field_decode */, }; diff --git a/crypto/ec/p256-x86_64.c b/crypto/ec/p256-x86_64.c index e1afec48..89a48c46 100644 --- a/crypto/ec/p256-x86_64.c +++ b/crypto/ec/p256-x86_64.c @@ -559,7 +559,6 @@ const EC_METHOD *EC_GFp_nistz256_method(void) { ec_GFp_mont_group_set_curve, ecp_nistz256_get_affine, ecp_nistz256_points_mul, - 0 /* check_pub_key_order */, ec_GFp_mont_field_mul, ec_GFp_mont_field_sqr, ec_GFp_mont_field_encode,