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 <agl@google.com>
diff --git a/crypto/ec/ec.c b/crypto/ec/ec.c
index 20a71a3..75e906b 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 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 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 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 @@
ret->meth = meth;
BN_init(&ret->order);
- BN_init(&ret->cofactor);
if (!meth->group_init(ret)) {
OPENSSL_free(ret);
@@ -406,8 +401,7 @@
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 @@
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 @@
EC_POINT_free(group->generator);
BN_free(&group->order);
- BN_free(&group->cofactor);
OPENSSL_free(group);
}
@@ -563,8 +555,7 @@
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_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 fee71fe..dcacdfa 100644
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -317,14 +317,6 @@
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 35df365..215867d 100644
--- a/crypto/ec/ec_montgomery.c
+++ b/crypto/ec/ec_montgomery.c
@@ -195,26 +195,6 @@
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 @@
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 f2cbb96..10770d9 100644
--- a/crypto/ec/internal.h
+++ b/crypto/ec/internal.h
@@ -96,15 +96,6 @@
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 @@
const EC_METHOD *meth;
EC_POINT *generator;
- BIGNUM order, cofactor;
+ BIGNUM order;
int curve_name; /* optional NID for named curve */
@@ -260,9 +251,6 @@
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 7bf889c..1b09cb9 100644
--- a/crypto/ec/p224-64.c
+++ b/crypto/ec/p224-64.c
@@ -1186,7 +1186,6 @@
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 c4259b6..31bf0ad 100644
--- a/crypto/ec/p256-64.c
+++ b/crypto/ec/p256-64.c
@@ -1742,7 +1742,6 @@
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 e1afec4..89a48c4 100644
--- a/crypto/ec/p256-x86_64.c
+++ b/crypto/ec/p256-x86_64.c
@@ -559,7 +559,6 @@
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,
