Update ECDSA comments and logic for FIPS 186-5

The main consequence is that FIPS 186-5 now leaves the retry when
rejection sampling fails as an exercise to the reader, and the minimum
group order is 224 bits.

As part of this, move the group order check closer to where it is in
FIPS 186-5 (the keygen and nonce generation algorithms). I think they
got moved around as the code evolved.

Update-Note: Matching FIPS 186-5's new requirements, callers using the
legacy-only custom curves machinery will no longer be able to construct
curves less than 224 bits. This has no impact on the supported named
curves.

Change-Id: I0539b54434f755d67102799a296b61ad737ed165
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/87809
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/bn/random.cc.inc b/crypto/fipsmodule/bn/random.cc.inc
index fdfeb29..591cb8f 100644
--- a/crypto/fipsmodule/bn/random.cc.inc
+++ b/crypto/fipsmodule/bn/random.cc.inc
@@ -155,9 +155,9 @@
 int bssl::bn_rand_range_words(BN_ULONG *out, BN_ULONG min_inclusive,
                               const BN_ULONG *max_exclusive, size_t len,
                               const uint8_t additional_data[32]) {
-  // This function implements the equivalent of steps 4 through 7 of FIPS 186-4
-  // appendices B.4.2 and B.5.2. When called in those contexts, |max_exclusive|
-  // is n and |min_inclusive| is one.
+  // This function implements the equivalent of steps 1 through 4 of FIPS 186-5
+  // appendices A.2.2 and A.3.2, repeating the process on failure. When called
+  // in those contexts, |max_exclusive| is n and |min_inclusive| is one.
 
   // Compute the bit length of |max_exclusive| (step 1), in terms of a number of
   // |words| worth of entropy to fill and a mask of bits to clear in the top
@@ -178,21 +178,19 @@
       return 0;
     }
 
-    // Steps 4 and 5. Use |words| and |mask| together to obtain a string of N
-    // bits, where N is the bit length of |max_exclusive|.
+    // Use |words| and |mask| together to obtain a string of N bits, where N is
+    // the bit length of |max_exclusive|.
     FIPS_service_indicator_lock_state();
     BCM_rand_bytes_with_additional_data(
         (uint8_t *)out, words * sizeof(BN_ULONG), additional_data);
     FIPS_service_indicator_unlock_state();
     out[words - 1] &= mask;
 
-    // If out >= max_exclusive or out < min_inclusive, retry. This implements
-    // the equivalent of steps 6 and 7 without leaking the value of |out|. The
-    // result of this comparison may be treated as public. It only reveals how
-    // many attempts were needed before we found a value in range. This is
-    // independent of the final secret output, and has a distribution that
-    // depends only on |min_inclusive| and |max_exclusive|, both of which are
-    // public.
+    // If out >= max_exclusive or out < min_inclusive, retry. The result of this
+    // comparison may be treated as public. It only reveals how many attempts
+    // were needed before we found a value in range. This is independent of the
+    // final secret output, and has a distribution that depends only on
+    // |min_inclusive| and |max_exclusive|, both of which are public.
   } while (!constant_time_declassify_int(
       bn_in_range_words(out, min_inclusive, max_exclusive, words)));
   return 1;
diff --git a/crypto/fipsmodule/ec/ec_key.cc.inc b/crypto/fipsmodule/ec/ec_key.cc.inc
index 1dd2893..d5133a7 100644
--- a/crypto/fipsmodule/ec/ec_key.cc.inc
+++ b/crypto/fipsmodule/ec/ec_key.cc.inc
@@ -430,8 +430,11 @@
     return 0;
   }
 
-  // Check that the group order is FIPS compliant (FIPS 186-4 B.4.2).
-  if (EC_GROUP_order_bits(key->group) < 160) {
+  // Generate an ECDSA key pair via rejection sampling. This function implements
+  // FIPS 186-5, A.2.2, repeating the process on failure.
+
+  // Check the group order is large enough. See step 1 of FIPS 186-5, A.2.2.
+  if (EC_GROUP_order_bits(key->group) < 224) {
     OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
     return 0;
   }
@@ -440,7 +443,6 @@
   EC_WRAPPED_SCALAR *priv_key = ec_wrapped_scalar_new(key->group);
   EC_POINT *pub_key = EC_POINT_new(key->group);
   if (priv_key == nullptr || pub_key == nullptr ||
-      // Generate the private key by testing candidates (FIPS 186-4 B.4.2).
       !ec_random_nonzero_scalar(key->group, &priv_key->scalar,
                                 kDefaultAdditionalData) ||
       !ec_point_mul_scalar_base(key->group, &pub_key->raw, &priv_key->scalar)) {
diff --git a/crypto/fipsmodule/ecdsa/ecdsa.cc.inc b/crypto/fipsmodule/ecdsa/ecdsa.cc.inc
index 0ea0d1d..7ff46f9 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa.cc.inc
+++ b/crypto/fipsmodule/ecdsa/ecdsa.cc.inc
@@ -123,14 +123,7 @@
                            const uint8_t *digest, size_t digest_len) {
   *out_retry = 0;
 
-  // Check that the size of the group order is FIPS compliant (FIPS 186-4
-  // B.5.2).
   const BIGNUM *order = EC_GROUP_get0_order(group);
-  if (BN_num_bits(order) < 160) {
-    OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
-    return 0;
-  }
-
   size_t sig_len = 2 * BN_num_bytes(order);
   if (sig_len > max_sig_len) {
     OPENSSL_PUT_ERROR(EC, EC_R_BUFFER_TOO_SMALL);
@@ -231,6 +224,15 @@
   const BIGNUM *order = EC_GROUP_get0_order(group);
   const EC_SCALAR *priv_key = &eckey->priv_key->scalar;
 
+  // Generate the ECDSA per-message secret number by rejection sampling. This
+  // function implements FIPS 186-5, A.3.2, repeating the process on failure.
+
+  // Check the group order is large enough. See step 1 of FIPS 186-5, A.3.2.
+  if (BN_num_bits(order) < 224) {
+    OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
+    return 0;
+  }
+
   // Pass a SHA512 hash of the private key and digest as additional data
   // into the RBG. This is a hardening measure against entropy failure.
   static_assert(SHA512_DIGEST_LENGTH >= 32,