Add |EC_GROUP_get0_order| to replace |EC_GROUP_get_order|.
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.
Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec/ec.c b/crypto/ec/ec.c
index eb8e1c9..827cc57 100644
--- a/crypto/ec/ec.c
+++ b/crypto/ec/ec.c
@@ -613,12 +613,16 @@
return group->generator;
}
+const BIGNUM *EC_GROUP_get0_order(const EC_GROUP *group) {
+ assert(!BN_is_zero(&group->order));
+ return &group->order;
+}
+
int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order, BN_CTX *ctx) {
- if (!BN_copy(order, &group->order)) {
+ if (BN_copy(order, EC_GROUP_get0_order(group)) == NULL) {
return 0;
}
-
- return !BN_is_zero(order);
+ return 1;
}
int EC_GROUP_get_cofactor(const EC_GROUP *group, BIGNUM *cofactor,
diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c
index afc5a9f..c2dd9d8 100644
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -289,7 +289,6 @@
int EC_KEY_check_key(const EC_KEY *eckey) {
int ok = 0;
BN_CTX *ctx = NULL;
- const BIGNUM *order = NULL;
EC_POINT *point = NULL;
if (!eckey || !eckey->group || !eckey->pub_key) {
@@ -318,11 +317,7 @@
/* testing whether pub_key * order is the point at infinity */
/* TODO(fork): can this be skipped if the cofactor is one or if we're about
* to check the private key, below? */
- order = &eckey->group->order;
- if (BN_is_zero(order)) {
- OPENSSL_PUT_ERROR(EC, EC_R_INVALID_GROUP_ORDER);
- goto err;
- }
+ const BIGNUM *order = EC_GROUP_get0_order(eckey->group);
if (!EC_POINT_mul(eckey->group, point, NULL, eckey->pub_key, order, ctx)) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
goto err;
@@ -408,8 +403,7 @@
int EC_KEY_generate_key(EC_KEY *eckey) {
int ok = 0;
- BN_CTX *ctx = NULL;
- BIGNUM *priv_key = NULL, *order = NULL;
+ BIGNUM *priv_key = NULL;
EC_POINT *pub_key = NULL;
if (!eckey || !eckey->group) {
@@ -417,14 +411,6 @@
return 0;
}
- order = BN_new();
- ctx = BN_CTX_new();
-
- if (order == NULL ||
- ctx == NULL) {
- goto err;
- }
-
if (eckey->priv_key == NULL) {
priv_key = BN_new();
if (priv_key == NULL) {
@@ -434,10 +420,7 @@
priv_key = eckey->priv_key;
}
- if (!EC_GROUP_get_order(eckey->group, order, ctx)) {
- goto err;
- }
-
+ const BIGNUM *order = EC_GROUP_get0_order(eckey->group);
do {
if (!BN_rand_range(priv_key, order)) {
goto err;
@@ -453,7 +436,7 @@
pub_key = eckey->pub_key;
}
- if (!EC_POINT_mul(eckey->group, pub_key, priv_key, NULL, NULL, ctx)) {
+ if (!EC_POINT_mul(eckey->group, pub_key, priv_key, NULL, NULL, NULL)) {
goto err;
}
@@ -463,14 +446,12 @@
ok = 1;
err:
- BN_free(order);
if (eckey->pub_key == NULL) {
EC_POINT_free(pub_key);
}
if (eckey->priv_key == NULL) {
BN_free(priv_key);
}
- BN_CTX_free(ctx);
return ok;
}
diff --git a/crypto/ecdsa/ecdsa.c b/crypto/ecdsa/ecdsa.c
index a718cf8..16760ed 100644
--- a/crypto/ecdsa/ecdsa.c
+++ b/crypto/ecdsa/ecdsa.c
@@ -143,7 +143,7 @@
const ECDSA_SIG *sig, EC_KEY *eckey) {
int ret = 0;
BN_CTX *ctx;
- BIGNUM *order, *u1, *u2, *m, *X;
+ BIGNUM *u1, *u2, *m, *X;
EC_POINT *point = NULL;
const EC_GROUP *group;
const EC_POINT *pub_key;
@@ -167,21 +167,16 @@
return 0;
}
BN_CTX_start(ctx);
- order = BN_CTX_get(ctx);
u1 = BN_CTX_get(ctx);
u2 = BN_CTX_get(ctx);
m = BN_CTX_get(ctx);
X = BN_CTX_get(ctx);
- if (order == NULL || u1 == NULL || u2 == NULL || m == NULL || X == NULL) {
+ if (u1 == NULL || u2 == NULL || m == NULL || X == NULL) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_BN_LIB);
goto err;
}
- if (!EC_GROUP_get_order(group, order, ctx)) {
- OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
- goto err;
- }
-
+ const BIGNUM *order = EC_GROUP_get0_order(group);
if (BN_is_zero(sig->r) || BN_is_negative(sig->r) ||
BN_ucmp(sig->r, order) >= 0 || BN_is_zero(sig->s) ||
BN_is_negative(sig->s) || BN_ucmp(sig->s, order) >= 0) {
@@ -229,7 +224,7 @@
ret = (BN_ucmp(u1, sig->r) == 0);
err:
- BN_CTX_end(ctx);
+ BN_CTX_end(ctx);
BN_CTX_free(ctx);
EC_POINT_free(point);
return ret;
@@ -239,7 +234,7 @@
BIGNUM **rp, const uint8_t *digest,
size_t digest_len) {
BN_CTX *ctx = NULL;
- BIGNUM *k = NULL, *r = NULL, *order = NULL, *X = NULL;
+ BIGNUM *k = NULL, *r = NULL, *X = NULL;
EC_POINT *tmp_point = NULL;
const EC_GROUP *group;
int ret = 0;
@@ -260,9 +255,8 @@
k = BN_new(); /* this value is later returned in *kinvp */
r = BN_new(); /* this value is later returned in *rp */
- order = BN_new();
X = BN_new();
- if (!k || !r || !order || !X) {
+ if (k == NULL || r == NULL || X == NULL) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
goto err;
}
@@ -271,10 +265,8 @@
OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
goto err;
}
- if (!EC_GROUP_get_order(group, order, ctx)) {
- OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
- goto err;
- }
+
+ const BIGNUM *order = EC_GROUP_get0_order(group);
do {
/* If possible, we'll include the private key and message digest in the k
@@ -360,7 +352,6 @@
if (ctx_in == NULL) {
BN_CTX_free(ctx);
}
- BN_free(order);
EC_POINT_free(tmp_point);
BN_clear_free(X);
return ret;
@@ -374,7 +365,7 @@
const BIGNUM *in_kinv, const BIGNUM *in_r,
EC_KEY *eckey) {
int ok = 0;
- BIGNUM *kinv = NULL, *s, *m = NULL, *tmp = NULL, *order = NULL;
+ BIGNUM *kinv = NULL, *s, *m = NULL, *tmp = NULL;
const BIGNUM *ckinv;
BN_CTX *ctx = NULL;
const EC_GROUP *group;
@@ -401,16 +392,15 @@
}
s = ret->s;
- if ((ctx = BN_CTX_new()) == NULL || (order = BN_new()) == NULL ||
- (tmp = BN_new()) == NULL || (m = BN_new()) == NULL) {
+ if ((ctx = BN_CTX_new()) == NULL ||
+ (tmp = BN_new()) == NULL ||
+ (m = BN_new()) == NULL) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
goto err;
}
- if (!EC_GROUP_get_order(group, order, ctx)) {
- OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
- goto err;
- }
+ const BIGNUM *order = EC_GROUP_get0_order(group);
+
if (!digest_to_bn(m, digest, digest_len, order)) {
goto err;
}
@@ -464,7 +454,6 @@
BN_CTX_free(ctx);
BN_clear_free(m);
BN_clear_free(tmp);
- BN_free(order);
BN_clear_free(kinv);
return ret;
}
diff --git a/crypto/ecdsa/ecdsa_asn1.c b/crypto/ecdsa/ecdsa_asn1.c
index f2d7c36..3fee191 100644
--- a/crypto/ecdsa/ecdsa_asn1.c
+++ b/crypto/ecdsa/ecdsa_asn1.c
@@ -78,17 +78,7 @@
return 0;
}
- BIGNUM *order = BN_new();
- if (order == NULL) {
- return 0;
- }
- if (!EC_GROUP_get_order(group, order, NULL)) {
- BN_clear_free(order);
- return 0;
- }
-
- group_order_size = BN_num_bytes(order);
- BN_clear_free(order);
+ group_order_size = BN_num_bytes(EC_GROUP_get0_order(group));
}
return ECDSA_SIG_max_len(group_order_size);
diff --git a/crypto/ecdsa/ecdsa_test.cc b/crypto/ecdsa/ecdsa_test.cc
index 5e021bd..a0c8fbd 100644
--- a/crypto/ecdsa/ecdsa_test.cc
+++ b/crypto/ecdsa/ecdsa_test.cc
@@ -176,12 +176,8 @@
fprintf(out, " failed\n");
return false;
}
- ScopedBIGNUM order(BN_new());
- if (!order || !EC_GROUP_get_order(group.get(), order.get(), NULL)) {
- fprintf(out, " failed\n");
- return false;
- }
- if (BN_num_bits(order.get()) < 160) {
+ const BIGNUM *order = EC_GROUP_get0_order(group.get());
+ if (BN_num_bits(order) < 160) {
// Too small to test.
fprintf(out, " skipped\n");
continue;
@@ -261,7 +257,7 @@
signature.data(), signature.size()));
if (!ecdsa_sig ||
!TestTamperedSig(out, kEncodedApi, digest, 20, ecdsa_sig.get(),
- eckey.get(), order.get())) {
+ eckey.get(), order)) {
fprintf(out, " failed\n");
return false;
}
@@ -300,7 +296,7 @@
fflush(out);
// Verify a tampered signature.
if (!TestTamperedSig(out, kRawApi, digest, 20, ecdsa_sig.get(), eckey.get(),
- order.get())) {
+ order)) {
fprintf(out, " failed\n");
return false;
}
diff --git a/crypto/evp/p_ec_asn1.c b/crypto/evp/p_ec_asn1.c
index c583e0f..f40b976 100644
--- a/crypto/evp/p_ec_asn1.c
+++ b/crypto/evp/p_ec_asn1.c
@@ -337,23 +337,12 @@
}
static int ec_bits(const EVP_PKEY *pkey) {
- BIGNUM *order = BN_new();
- const EC_GROUP *group;
- int ret;
-
- if (!order) {
+ const EC_GROUP *group = EC_KEY_get0_group(pkey->pkey.ec);
+ if (group == NULL) {
ERR_clear_error();
return 0;
}
- group = EC_KEY_get0_group(pkey->pkey.ec);
- if (!EC_GROUP_get_order(group, order, NULL)) {
- ERR_clear_error();
- return 0;
- }
-
- ret = BN_num_bits(order);
- BN_free(order);
- return ret;
+ return BN_num_bits(EC_GROUP_get0_order(group));
}
static int ec_missing_parameters(const EVP_PKEY *pkey) {
@@ -387,7 +376,6 @@
const char *ecstr;
size_t buf_len = 0, i;
int ret = 0, reason = ERR_R_BIO_LIB;
- BIGNUM *order = NULL;
BN_CTX *ctx = NULL;
const EC_GROUP *group;
const EC_POINT *public_key;
@@ -458,9 +446,8 @@
if (!BIO_indent(bp, off, 128)) {
goto err;
}
- order = BN_new();
- if (order == NULL || !EC_GROUP_get_order(group, order, NULL) ||
- BIO_printf(bp, "%s: (%d bit)\n", ecstr, BN_num_bits(order)) <= 0) {
+ const BIGNUM *order = EC_GROUP_get0_order(group);
+ if (BIO_printf(bp, "%s: (%d bit)\n", ecstr, BN_num_bits(order)) <= 0) {
goto err;
}
@@ -482,7 +469,6 @@
OPENSSL_PUT_ERROR(EVP, reason);
}
OPENSSL_free(pub_key_bytes);
- BN_free(order);
BN_CTX_free(ctx);
OPENSSL_free(buffer);
return ret;
diff --git a/include/openssl/ec.h b/include/openssl/ec.h
index b0a84c6..667be3b 100644
--- a/include/openssl/ec.h
+++ b/include/openssl/ec.h
@@ -120,10 +120,9 @@
* in |group| that specifies the generator for the group. */
OPENSSL_EXPORT const EC_POINT *EC_GROUP_get0_generator(const EC_GROUP *group);
-/* EC_GROUP_get_order sets |*order| to the order of |group|, if it's not
- * NULL. It returns one on success and zero otherwise. |ctx| is ignored. */
-OPENSSL_EXPORT int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order,
- BN_CTX *ctx);
+/* EC_GROUP_get0_order returns a pointer to the internal |BIGNUM| object in
+ * |group| that specifies the order of the group. */
+OPENSSL_EXPORT const BIGNUM *EC_GROUP_get0_order(const EC_GROUP *group);
/* EC_GROUP_get_cofactor sets |*cofactor| to the cofactor of |group| using
* |ctx|, if it's not NULL. It returns one on success and zero otherwise. */
@@ -283,6 +282,12 @@
const BIGNUM *a,
const BIGNUM *b, BN_CTX *ctx);
+/* EC_GROUP_get_order sets |*order| to the order of |group|, if it's not
+ * NULL. It returns one on success and zero otherwise. |ctx| is ignored. Use
+ * |EC_GROUP_get0_order| instead. */
+OPENSSL_EXPORT int EC_GROUP_get_order(const EC_GROUP *group, BIGNUM *order,
+ BN_CTX *ctx);
+
/* EC_GROUP_set_generator sets the generator for |group| to |generator|, which
* must have the given order and cofactor. This should only be used with
* |EC_GROUP| objects returned by |EC_GROUP_new_curve_GFp|. */
