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>
diff --git a/crypto/ec/ec.c b/crypto/ec/ec.c
index 1d1ebb6..20a71a3 100644
--- a/crypto/ec/ec.c
+++ b/crypto/ec/ec.c
@@ -397,6 +397,12 @@
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) &&
diff --git a/crypto/ecdsa/ecdsa.c b/crypto/ecdsa/ecdsa.c
index 70cb118..6938325 100644
--- a/crypto/ecdsa/ecdsa.c
+++ b/crypto/ecdsa/ecdsa.c
@@ -225,7 +225,7 @@
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 @@
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 @@
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 @@
BN_CTX_free(ctx);
}
EC_POINT_free(tmp_point);
- BN_clear_free(X);
+ BN_clear_free(tmp);
return ret;
}
diff --git a/crypto/err/ec.errordata b/crypto/err/ec.errordata
index d074afc..aada76e 100644
--- a/crypto/err/ec.errordata
+++ b/crypto/err/ec.errordata
@@ -9,6 +9,7 @@
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
diff --git a/include/openssl/ec.h b/include/openssl/ec.h
index 05218f3..32aded6 100644
--- a/include/openssl/ec.h
+++ b/include/openssl/ec.h
@@ -392,5 +392,6 @@
#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 */
