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>
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 @@
(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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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
