Refcount EC_GROUP.
I really need to resurrect the CL to make them entirely static
(https://crbug.com/boringssl/20), but, in the meantime, to make
replacing the EC_METHOD pointer in EC_POINT with EC_GROUP not
*completely* insane, make them refcounted.
OpenSSL did not do this because their EC_GROUPs are mutable
(EC_GROUP_set_asn1_flag and EC_GROUP_set_point_conversion_form). Ours
are immutable but for the two-function dance around custom curves (more
of OpenSSL's habit of making their objects too complex), which is good
enough to refcount.
Change-Id: I3650993737a97da0ddcf0e5fb7a15876e724cadc
Reviewed-on: https://boringssl-review.googlesource.com/22244
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c
index 4e8f2d2..7caa5d8 100644
--- a/crypto/fipsmodule/ec/ec.c
+++ b/crypto/fipsmodule/ec/ec.c
@@ -354,6 +354,7 @@
}
OPENSSL_memset(ret, 0, sizeof(EC_GROUP));
+ ret->references = 1;
ret->meth = meth;
BN_init(&ret->order);
@@ -500,11 +501,12 @@
}
void EC_GROUP_free(EC_GROUP *group) {
- if (!group) {
+ if (!group ||
+ !CRYPTO_refcount_dec_and_test_zero(&group->references)) {
return;
}
- if (group->meth->group_finish != 0) {
+ if (group->meth->group_finish != NULL) {
group->meth->group_finish(group);
}
@@ -523,36 +525,11 @@
return NULL;
}
- if (a->meth->group_copy == NULL) {
- OPENSSL_PUT_ERROR(EC, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
- return NULL;
- }
-
- EC_GROUP *ret = ec_group_new(a->meth);
- if (ret == NULL) {
- return NULL;
- }
-
- ret->order_mont = a->order_mont;
- ret->curve_name = a->curve_name;
-
- if (a->generator != NULL) {
- ret->generator = EC_POINT_dup(a->generator, ret);
- if (ret->generator == NULL) {
- goto err;
- }
- }
-
- if (!BN_copy(&ret->order, &a->order) ||
- !ret->meth->group_copy(ret, a)) {
- goto err;
- }
-
- return ret;
-
-err:
- EC_GROUP_free(ret);
- return NULL;
+ // Groups are logically immutable (but for |EC_GROUP_set_generator| which must
+ // be called early on), so we simply take a reference.
+ EC_GROUP *group = (EC_GROUP *)a;
+ CRYPTO_refcount_inc(&group->references);
+ return group;
}
int EC_GROUP_cmp(const EC_GROUP *a, const EC_GROUP *b, BN_CTX *ignored) {
diff --git a/crypto/fipsmodule/ec/ec_montgomery.c b/crypto/fipsmodule/ec/ec_montgomery.c
index c5f240b..6670b84 100644
--- a/crypto/fipsmodule/ec/ec_montgomery.c
+++ b/crypto/fipsmodule/ec/ec_montgomery.c
@@ -90,32 +90,6 @@
ec_GFp_simple_group_finish(group);
}
-int ec_GFp_mont_group_copy(EC_GROUP *dest, const EC_GROUP *src) {
- BN_MONT_CTX_free(dest->mont);
- dest->mont = NULL;
-
- if (!ec_GFp_simple_group_copy(dest, src)) {
- return 0;
- }
-
- if (src->mont != NULL) {
- dest->mont = BN_MONT_CTX_new();
- if (dest->mont == NULL) {
- return 0;
- }
- if (!BN_MONT_CTX_copy(dest->mont, src->mont)) {
- goto err;
- }
- }
-
- return 1;
-
-err:
- BN_MONT_CTX_free(dest->mont);
- dest->mont = NULL;
- return 0;
-}
-
int ec_GFp_mont_group_set_curve(EC_GROUP *group, const BIGNUM *p,
const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) {
BN_CTX *new_ctx = NULL;
@@ -293,7 +267,6 @@
DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_mont_method) {
out->group_init = ec_GFp_mont_group_init;
out->group_finish = ec_GFp_mont_group_finish;
- out->group_copy = ec_GFp_mont_group_copy;
out->group_set_curve = ec_GFp_mont_group_set_curve;
out->point_get_affine_coordinates = ec_GFp_mont_point_get_affine_coordinates;
out->mul = ec_wNAF_mul /* XXX: Not constant time. */;
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index 22cd42a..1042310 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -82,7 +82,6 @@
struct ec_method_st {
int (*group_init)(EC_GROUP *);
void (*group_finish)(EC_GROUP *);
- int (*group_copy)(EC_GROUP *, const EC_GROUP *);
int (*group_set_curve)(EC_GROUP *, const BIGNUM *p, const BIGNUM *a,
const BIGNUM *b, BN_CTX *);
int (*point_get_affine_coordinates)(const EC_GROUP *, const EC_POINT *,
@@ -130,6 +129,8 @@
int a_is_minus3; // enable optimized point arithmetics for special case
+ CRYPTO_refcount_t references;
+
BN_MONT_CTX *mont; // Montgomery structure.
BIGNUM one; // The value one.
@@ -145,7 +146,6 @@
} /* EC_POINT */;
EC_GROUP *ec_group_new(const EC_METHOD *meth);
-int ec_group_copy(EC_GROUP *dest, const EC_GROUP *src);
// ec_group_get_order_mont returns a Montgomery context for operations modulo
// |group|'s order. It may return NULL in the case that |group| is not a
@@ -158,7 +158,6 @@
// method functions in simple.c
int ec_GFp_simple_group_init(EC_GROUP *);
void ec_GFp_simple_group_finish(EC_GROUP *);
-int ec_GFp_simple_group_copy(EC_GROUP *, const EC_GROUP *);
int ec_GFp_simple_group_set_curve(EC_GROUP *, const BIGNUM *p, const BIGNUM *a,
const BIGNUM *b, BN_CTX *);
int ec_GFp_simple_group_get_curve(const EC_GROUP *, BIGNUM *p, BIGNUM *a,
@@ -204,7 +203,6 @@
int ec_GFp_mont_group_set_curve(EC_GROUP *, const BIGNUM *p, const BIGNUM *a,
const BIGNUM *b, BN_CTX *);
void ec_GFp_mont_group_finish(EC_GROUP *);
-int ec_GFp_mont_group_copy(EC_GROUP *, const EC_GROUP *);
int ec_GFp_mont_field_mul(const EC_GROUP *, BIGNUM *r, const BIGNUM *a,
const BIGNUM *b, BN_CTX *);
int ec_GFp_mont_field_sqr(const EC_GROUP *, BIGNUM *r, const BIGNUM *a,
diff --git a/crypto/fipsmodule/ec/p224-64.c b/crypto/fipsmodule/ec/p224-64.c
index ec5a93d..26a94b9 100644
--- a/crypto/fipsmodule/ec/p224-64.c
+++ b/crypto/fipsmodule/ec/p224-64.c
@@ -1151,7 +1151,6 @@
DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_nistp224_method) {
out->group_init = ec_GFp_simple_group_init;
out->group_finish = ec_GFp_simple_group_finish;
- out->group_copy = ec_GFp_simple_group_copy;
out->group_set_curve = ec_GFp_simple_group_set_curve;
out->point_get_affine_coordinates =
ec_GFp_nistp224_point_get_affine_coordinates;
diff --git a/crypto/fipsmodule/ec/p256-64.c b/crypto/fipsmodule/ec/p256-64.c
index f7d1ff1..04ae56b 100644
--- a/crypto/fipsmodule/ec/p256-64.c
+++ b/crypto/fipsmodule/ec/p256-64.c
@@ -1694,7 +1694,6 @@
DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_nistp256_method) {
out->group_init = ec_GFp_simple_group_init;
out->group_finish = ec_GFp_simple_group_finish;
- out->group_copy = ec_GFp_simple_group_copy;
out->group_set_curve = ec_GFp_simple_group_set_curve;
out->point_get_affine_coordinates =
ec_GFp_nistp256_point_get_affine_coordinates;
diff --git a/crypto/fipsmodule/ec/p256-x86_64.c b/crypto/fipsmodule/ec/p256-x86_64.c
index 8b51677..0004add 100644
--- a/crypto/fipsmodule/ec/p256-x86_64.c
+++ b/crypto/fipsmodule/ec/p256-x86_64.c
@@ -547,7 +547,6 @@
DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_nistz256_method) {
out->group_init = ec_GFp_mont_group_init;
out->group_finish = ec_GFp_mont_group_finish;
- out->group_copy = ec_GFp_mont_group_copy;
out->group_set_curve = ec_GFp_mont_group_set_curve;
out->point_get_affine_coordinates = ecp_nistz256_get_affine;
out->mul = ecp_nistz256_points_mul;
diff --git a/crypto/fipsmodule/ec/simple.c b/crypto/fipsmodule/ec/simple.c
index e46550f..36b90c8 100644
--- a/crypto/fipsmodule/ec/simple.c
+++ b/crypto/fipsmodule/ec/simple.c
@@ -104,18 +104,6 @@
BN_free(&group->one);
}
-int ec_GFp_simple_group_copy(EC_GROUP *dest, const EC_GROUP *src) {
- if (!BN_copy(&dest->field, &src->field) ||
- !BN_copy(&dest->a, &src->a) ||
- !BN_copy(&dest->b, &src->b) ||
- !BN_copy(&dest->one, &src->one)) {
- return 0;
- }
-
- dest->a_is_minus3 = src->a_is_minus3;
- return 1;
-}
-
int ec_GFp_simple_group_set_curve(EC_GROUP *group, const BIGNUM *p,
const BIGNUM *a, const BIGNUM *b,
BN_CTX *ctx) {
