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>

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->order, order);
-         BN_copy(&group->cofactor, cofactor);
 }
 
 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) ||
+  if (!BN_bin2bn(params + 5 * param_len, param_len, &group->order)) {
-      !BN_set_word(&group->cofactor, (BN_ULONG)data->cofactor)) {
     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) ||
+  if (!BN_copy(&dest->order, &src->order)) {
-      !BN_copy(&dest->cofactor, &src->cofactor)) {
     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)) {
+  /* All |EC_GROUP|s have cofactor 1. */
-    return 0;
+  return BN_set_word(cofactor, 1);
-  }
-
-  return !BN_is_zero(&group->cofactor);
 }
 
 int EC_GROUP_get_curve_GFp(const EC_GROUP *group, BIGNUM *out_p, BIGNUM *out_a,

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
    */

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,

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. */

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 */,

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 */,
   };

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,