Remove ECDSA_sign_setup and friends.

These allow precomputation of k, but bypass our nonce hardening and also
make it harder to excise BIGNUM. As a bonus, ECDSATest.SignTestVectors
is now actually covering the k^-1 and r computations.

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

4 files changed, 17 insertions, 103 deletions

diff --git a/crypto/ecdsa_extra/ecdsa_asn1.c b/crypto/ecdsa_extra/ecdsa_asn1.c
index 8d0bc41..fbf4cca 100644
--- a/crypto/ecdsa_extra/ecdsa_asn1.c
+++ b/crypto/ecdsa_extra/ecdsa_asn1.c
@@ -73,13 +73,6 @@ int ECDSA_sign(int type, const uint8_t *digest, size_t digest_len, uint8_t *sig,
                                    (EC_KEY*) eckey /* cast away const */);
   }
 
-  return ECDSA_sign_ex(type, digest, digest_len, sig, sig_len, NULL, NULL,
-                       eckey);
-}
-
-int ECDSA_sign_ex(int type, const uint8_t *digest, size_t digest_len,
-                  uint8_t *sig, unsigned int *sig_len, const BIGNUM *kinv,
-                  const BIGNUM *r, const EC_KEY *eckey) {
   int ret = 0;
   ECDSA_SIG *s = NULL;
 
@@ -89,7 +82,7 @@ int ECDSA_sign_ex(int type, const uint8_t *digest, size_t digest_len,
     goto err;
   }
 
-  s = ECDSA_do_sign_ex(digest, digest_len, kinv, r, eckey);
+  s = ECDSA_do_sign(digest, digest_len, eckey);
   if (s == NULL) {
     *sig_len = 0;
     goto err;
diff --git a/crypto/fipsmodule/ecdsa/ecdsa.c b/crypto/fipsmodule/ecdsa/ecdsa.c
index 4bed580..7af35f7 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa.c
+++ b/crypto/fipsmodule/ecdsa/ecdsa.c
@@ -137,11 +137,6 @@ int ECDSA_SIG_set0(ECDSA_SIG *sig, BIGNUM *r, BIGNUM *s) {
   return 1;
 }
 
-ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
-                         const EC_KEY *key) {
-  return ECDSA_do_sign_ex(digest, digest_len, NULL, NULL, key);
-}
-
 int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
                     const ECDSA_SIG *sig, const EC_KEY *eckey) {
   int ret = 0;
@@ -233,10 +228,9 @@ err:
   return ret;
 }
 
-static int ecdsa_sign_setup(const EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
+static int ecdsa_sign_setup(const EC_KEY *eckey, BN_CTX *ctx, BIGNUM **kinvp,
                             BIGNUM **rp, const uint8_t *digest,
                             size_t digest_len) {
-  BN_CTX *ctx = NULL;
   BIGNUM *k = NULL, *kinv = NULL, *r = NULL, *tmp = NULL;
   EC_POINT *tmp_point = NULL;
   const EC_GROUP *group;
@@ -247,15 +241,6 @@ static int ecdsa_sign_setup(const EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
     return 0;
   }
 
-  if (ctx_in == NULL) {
-    if ((ctx = BN_CTX_new()) == NULL) {
-      OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
-      return 0;
-    }
-  } else {
-    ctx = ctx_in;
-  }
-
   k = BN_new();
   kinv = BN_new();  // this value is later returned in *kinvp
   r = BN_new();  // this value is later returned in *rp
@@ -280,20 +265,13 @@ static int ecdsa_sign_setup(const EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
   }
 
   do {
-    // If possible, we'll include the private key and message digest in the k
-    // generation. The |digest| argument is only empty if |ECDSA_sign_setup| is
-    // being used.
+    // Include the private key and message digest in the k generation.
     if (eckey->fixed_k != NULL) {
       if (!BN_copy(k, eckey->fixed_k)) {
         goto err;
       }
-    } else if (digest_len > 0) {
-      if (!BN_generate_dsa_nonce(k, order, EC_KEY_get0_private_key(eckey),
-                                 digest, digest_len, ctx)) {
-        OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED);
-        goto err;
-      }
-    } else if (!BN_rand_range_ex(k, 1, order)) {
+    } else if (!BN_generate_dsa_nonce(k, order, EC_KEY_get0_private_key(eckey),
+                                      digest, digest_len, ctx)) {
       OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED);
       goto err;
     }
@@ -350,25 +328,15 @@ err:
     BN_clear_free(kinv);
     BN_clear_free(r);
   }
-  if (ctx_in == NULL) {
-    BN_CTX_free(ctx);
-  }
   EC_POINT_free(tmp_point);
   BN_clear_free(tmp);
   return ret;
 }
 
-int ECDSA_sign_setup(const EC_KEY *eckey, BN_CTX *ctx, BIGNUM **kinv,
-                     BIGNUM **rp) {
-  return ecdsa_sign_setup(eckey, ctx, kinv, rp, NULL, 0);
-}
-
-ECDSA_SIG *ECDSA_do_sign_ex(const uint8_t *digest, size_t digest_len,
-                            const BIGNUM *in_kinv, const BIGNUM *in_r,
-                            const EC_KEY *eckey) {
+ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len,
+                         const EC_KEY *eckey) {
   int ok = 0;
   BIGNUM *kinv = NULL, *s, *m = NULL, *tmp = NULL;
-  const BIGNUM *ckinv;
   BN_CTX *ctx = NULL;
   const EC_GROUP *group;
   ECDSA_SIG *ret;
@@ -407,18 +375,9 @@ ECDSA_SIG *ECDSA_do_sign_ex(const uint8_t *digest, size_t digest_len,
     goto err;
   }
   for (;;) {
-    if (in_kinv == NULL || in_r == NULL) {
-      if (!ecdsa_sign_setup(eckey, ctx, &kinv, &ret->r, digest, digest_len)) {
-        OPENSSL_PUT_ERROR(ECDSA, ERR_R_ECDSA_LIB);
-        goto err;
-      }
-      ckinv = kinv;
-    } else {
-      ckinv = in_kinv;
-      if (BN_copy(ret->r, in_r) == NULL) {
-        OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
-        goto err;
-      }
+    if (!ecdsa_sign_setup(eckey, ctx, &kinv, &ret->r, digest, digest_len)) {
+      OPENSSL_PUT_ERROR(ECDSA, ERR_R_ECDSA_LIB);
+      goto err;
     }
 
     if (!BN_mod_mul(tmp, priv_key, ret->r, order, ctx)) {
@@ -429,18 +388,11 @@ ECDSA_SIG *ECDSA_do_sign_ex(const uint8_t *digest, size_t digest_len,
       OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
       goto err;
     }
-    if (!BN_mod_mul(s, s, ckinv, order, ctx)) {
+    if (!BN_mod_mul(s, s, kinv, order, ctx)) {
       OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
       goto err;
     }
-    if (BN_is_zero(s)) {
-      // if kinv and r have been supplied by the caller
-      // don't to generate new kinv and r values
-      if (in_kinv != NULL && in_r != NULL) {
-        OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_NEED_NEW_SETUP_VALUES);
-        goto err;
-      }
-    } else {
+    if (!BN_is_zero(s)) {
       // s != 0 => we have a valid signature
       break;
     }
diff --git a/crypto/fipsmodule/ecdsa/ecdsa_test.cc b/crypto/fipsmodule/ecdsa/ecdsa_test.cc
index f904bda..258c128 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa_test.cc
+++ b/crypto/fipsmodule/ecdsa/ecdsa_test.cc
@@ -64,6 +64,7 @@
 #include <openssl/nid.h>
 #include <openssl/rand.h>
 
+#include "../ec/internal.h"
 #include "../../test/file_test.h"
 
 
@@ -396,14 +397,10 @@ TEST(ECDSATest, SignTestVectors) {
       ASSERT_TRUE(EC_KEY_set_public_key(key.get(), pub_key.get()));
       ASSERT_TRUE(EC_KEY_check_key(key.get()));
 
-      // |ECDSA_do_sign_ex| expects |k| to already be inverted.
-      bssl::UniquePtr<BN_CTX> ctx(BN_CTX_new());
-      ASSERT_TRUE(ctx);
-      ASSERT_TRUE(BN_mod_inverse(k.get(), k.get(),
-                                 EC_GROUP_get0_order(group.get()), ctx.get()));
-
-      bssl::UniquePtr<ECDSA_SIG> sig(ECDSA_do_sign_ex(
-          digest.data(), digest.size(), k.get(), r.get(), key.get()));
+      // Set the fixed k for testing purposes.
+      key->fixed_k = k.release();
+      bssl::UniquePtr<ECDSA_SIG> sig(
+          ECDSA_do_sign(digest.data(), digest.size(), key.get()));
       ASSERT_TRUE(sig);
 
       EXPECT_EQ(0, BN_cmp(r.get(), sig->r));
diff --git a/include/openssl/ecdsa.h b/include/openssl/ecdsa.h
index d2a828d..42da1c6 100644
--- a/include/openssl/ecdsa.h
+++ b/include/openssl/ecdsa.h
@@ -128,34 +128,6 @@ OPENSSL_EXPORT int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
                                    const ECDSA_SIG *sig, const EC_KEY *key);
 
 
-// Signing with precomputation.
-//
-// Parts of the ECDSA signature can be independent of the message to be signed
-// thus it's possible to precompute them and reduce the signing latency.
-//
-// TODO(fork): remove support for this as it cannot support safe-randomness.
-
-// ECDSA_sign_setup precomputes parts of an ECDSA signing operation. It sets
-// |*kinv| and |*rp| to the precomputed values and uses the |ctx| argument, if
-// not NULL. It returns one on success and zero otherwise.
-OPENSSL_EXPORT int ECDSA_sign_setup(const EC_KEY *eckey, BN_CTX *ctx,
-                                    BIGNUM **kinv, BIGNUM **rp);
-
-// ECDSA_do_sign_ex is the same as |ECDSA_do_sign| but takes precomputed values
-// as generated by |ECDSA_sign_setup|.
-OPENSSL_EXPORT ECDSA_SIG *ECDSA_do_sign_ex(const uint8_t *digest,
-                                           size_t digest_len,
-                                           const BIGNUM *kinv, const BIGNUM *rp,
-                                           const EC_KEY *eckey);
-
-// ECDSA_sign_ex is the same as |ECDSA_sign| but takes precomputed values as
-// generated by |ECDSA_sign_setup|.
-OPENSSL_EXPORT int ECDSA_sign_ex(int type, const uint8_t *digest,
-                                 size_t digest_len, uint8_t *sig,
-                                 unsigned int *sig_len, const BIGNUM *kinv,
-                                 const BIGNUM *rp, const EC_KEY *eckey);
-
-
 // ASN.1 functions.
 
 // ECDSA_SIG_parse parses a DER-encoded ECDSA-Sig-Value structure from |cbs| and