Always use Fermat's Little Theorem in ecdsa_sign_setup.

The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.

Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.

Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>

crypto/ec/ec.c

@@ -397,6 +397,12 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
     return 0;
   }
 
+  /* Require a cofactor of one for custom curves, which implies prime order. */
+  if (!BN_is_one(cofactor)) {
+    OPENSSL_PUT_ERROR(EC, EC_R_INVALID_COFACTOR);
+    return 0;
+  }
+
   group->generator = EC_POINT_new(group);
   return group->generator != NULL &&
          EC_POINT_copy(group->generator, generator) &&

crypto/ecdsa/ecdsa.c

@@ -225,7 +225,7 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
                             BIGNUM **rp, const uint8_t *digest,
                             size_t digest_len) {
   BN_CTX *ctx = NULL;
-  BIGNUM *k = NULL, *r = NULL, *X = NULL;
+  BIGNUM *k = NULL, *r = NULL, *tmp = NULL;
   EC_POINT *tmp_point = NULL;
   const EC_GROUP *group;
   int ret = 0;
@@ -246,8 +246,8 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
 
   k = BN_new(); /* this value is later returned in *kinvp */
   r = BN_new(); /* this value is later returned in *rp    */
-  X = BN_new();
-  if (k == NULL || r == NULL || X == NULL) {
+  tmp = BN_new();
+  if (k == NULL || r == NULL || tmp == NULL) {
     OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
     goto err;
   }
@@ -296,33 +296,25 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
       OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
       goto err;
     }
-    if (!EC_POINT_get_affine_coordinates_GFp(group, tmp_point, X, NULL, ctx)) {
+    if (!EC_POINT_get_affine_coordinates_GFp(group, tmp_point, tmp, NULL,
+                                             ctx)) {
       OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
       goto err;
     }
 
-    if (!BN_nnmod(r, X, order, ctx)) {
+    if (!BN_nnmod(r, tmp, order, ctx)) {
       OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
       goto err;
     }
   } while (BN_is_zero(r));
 
-  /* compute the inverse of k */
-  if (ec_group_get_mont_data(group) != NULL) {
-    /* We want inverse in constant time, therefore we use that the order must
-     * be prime and thus we can use Fermat's Little Theorem. */
-    if (!BN_set_word(X, 2) ||
-        !BN_sub(X, order, X)) {
-      OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
-      goto err;
-    }
-    BN_set_flags(X, BN_FLG_CONSTTIME);
-    if (!BN_mod_exp_mont_consttime(k, k, X, order, ctx,
-                                   ec_group_get_mont_data(group))) {
-      OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
-      goto err;
-    }
-  } else if (!BN_mod_inverse(k, k, order, ctx)) {
+  /* Compute the inverse of k. The order is a prime, so use Fermat's Little
+   * Theorem. */
+  if (!BN_set_word(tmp, 2) ||
+      !BN_sub(tmp, order, tmp) ||
+      /* Note |ec_group_get_mont_data| may return NULL but |BN_mod_exp_mont|
+       * allows it to be. */
+      !BN_mod_exp_mont(k, k, tmp, order, ctx, ec_group_get_mont_data(group))) {
     OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
     goto err;
   }
@@ -344,7 +336,7 @@ err:
     BN_CTX_free(ctx);
   }
   EC_POINT_free(tmp_point);
-  BN_clear_free(X);
+  BN_clear_free(tmp);
   return ret;
 }
 

crypto/err/ec.errordata

@@ -9,6 +9,7 @@ EC,104,GROUP2PKPARAMETERS_FAILURE
 EC,130,GROUP_MISMATCH
 EC,105,I2D_ECPKPARAMETERS_FAILURE
 EC,106,INCOMPATIBLE_OBJECTS
+EC,131,INVALID_COFACTOR
 EC,107,INVALID_COMPRESSED_POINT
 EC,108,INVALID_COMPRESSION_BIT
 EC,109,INVALID_ENCODING

include/openssl/ec.h

@@ -392,5 +392,6 @@ OPENSSL_EXPORT size_t EC_get_builtin_curves(EC_builtin_curve *out_curves,
 #define EC_R_DECODE_ERROR 128
 #define EC_R_ENCODE_ERROR 129
 #define EC_R_GROUP_MISMATCH 130
+#define EC_R_INVALID_COFACTOR 131
 
 #endif  /* OPENSSL_HEADER_EC_H */