Fold EC_GROUP_new_curve_GFp and EC_GROUP_set_generator into a EC_GROUP_new_arbitrary.
This is only for Conscrypt which always calls the pair in succession. (Indeed
it wouldn't make any sense to not call it.) Remove those two APIs and replace
with a single merged API. This way incomplete EC_GROUPs never escape outside
our API boundary and EC_GROUPs may *finally* be made immutable.
Also add a test for this to make sure I didn't mess it up.
Add a temporary BORINGSSL_201512 define to ease the transition for Conscrypt.
Conscrypt requires https://android-review.googlesource.com/#/c/187801/ before
picking up this change.
Change-Id: I3706c2ceac31ed2313175ba5ee724bd5c74ef6e1
Reviewed-on: https://boringssl-review.googlesource.com/6550
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec/ec.c b/crypto/ec/ec.c
index 827cc57..2f8d2b1 100644
--- a/crypto/ec/ec.c
+++ b/crypto/ec/ec.c
@@ -350,8 +350,8 @@
return ret;
}
-EC_GROUP *EC_GROUP_new_curve_GFp(const BIGNUM *p, const BIGNUM *a,
- const BIGNUM *b, BN_CTX *ctx) {
+static EC_GROUP *ec_group_new_curve_GFp(const BIGNUM *p, const BIGNUM *a,
+ const BIGNUM *b, BN_CTX *ctx) {
const EC_METHOD *meth = EC_GFp_mont_method();
EC_GROUP *ret;
@@ -371,42 +371,38 @@
return ret;
}
-int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
- const BIGNUM *order, const BIGNUM *cofactor) {
- if (group->curve_name != NID_undef) {
- /* |EC_GROUP_set_generator| should only be used with |EC_GROUP|s returned
- * by |EC_GROUP_new_curve_GFp|. */
- return 0;
+EC_GROUP *EC_GROUP_new_arbitrary(const BIGNUM *p, const BIGNUM *a,
+ const BIGNUM *b, const BIGNUM *gx,
+ const BIGNUM *gy, const BIGNUM *order,
+ const BIGNUM *cofactor) {
+ EC_GROUP *ret = NULL;
+ BN_CTX *ctx;
+
+ ctx = BN_CTX_new();
+ if (ctx == NULL) {
+ goto err;
}
- if (group->generator == NULL) {
- group->generator = EC_POINT_new(group);
- if (group->generator == NULL) {
- return 0;
- }
+ ret = ec_group_new_curve_GFp(p, a, b, ctx);
+ if (ret == NULL) {
+ goto err;
}
- if (!EC_POINT_copy(group->generator, generator)) {
- return 0;
+ ret->generator = EC_POINT_new(ret);
+ if (ret->generator == NULL ||
+ !EC_POINT_set_affine_coordinates_GFp(ret, ret->generator, gx, gy, ctx) ||
+ !BN_copy(&ret->order, order) ||
+ !BN_copy(&ret->cofactor, cofactor)) {
+ goto err;
}
- if (order != NULL) {
- if (!BN_copy(&group->order, order)) {
- return 0;
- }
- } else {
- BN_zero(&group->order);
- }
+ BN_CTX_free(ctx);
+ return ret;
- if (cofactor != NULL) {
- if (!BN_copy(&group->cofactor, cofactor)) {
- return 0;
- }
- } else {
- BN_zero(&group->cofactor);
- }
-
- return 1;
+err:
+ EC_GROUP_free(ret);
+ BN_CTX_free(ctx);
+ return NULL;
}
static EC_GROUP *ec_group_new_from_data(unsigned built_in_index) {
@@ -442,7 +438,7 @@
goto err;
}
} else {
- if ((group = EC_GROUP_new_curve_GFp(p, a, b, ctx)) == NULL) {
+ if ((group = ec_group_new_curve_GFp(p, a, b, ctx)) == NULL) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
goto err;
}
diff --git a/crypto/ec/ec_test.cc b/crypto/ec/ec_test.cc
index 2088e72..a20911b 100644
--- a/crypto/ec/ec_test.cc
+++ b/crypto/ec/ec_test.cc
@@ -240,6 +240,92 @@
return true;
}
+static bool TestArbitraryCurve() {
+ // Make a P-256 key and extract the affine coordinates.
+ ScopedEC_KEY key(EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
+ if (!key || !EC_KEY_generate_key(key.get())) {
+ return false;
+ }
+
+ // Make an arbitrary curve which is identical to P-256.
+ static const uint8_t kP[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+ };
+ static const uint8_t kA[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfc,
+ };
+ static const uint8_t kB[] = {
+ 0x5a, 0xc6, 0x35, 0xd8, 0xaa, 0x3a, 0x93, 0xe7, 0xb3, 0xeb, 0xbd,
+ 0x55, 0x76, 0x98, 0x86, 0xbc, 0x65, 0x1d, 0x06, 0xb0, 0xcc, 0x53,
+ 0xb0, 0xf6, 0x3b, 0xce, 0x3c, 0x3e, 0x27, 0xd2, 0x60, 0x4b,
+ };
+ static const uint8_t kX[] = {
+ 0x6b, 0x17, 0xd1, 0xf2, 0xe1, 0x2c, 0x42, 0x47, 0xf8, 0xbc, 0xe6,
+ 0xe5, 0x63, 0xa4, 0x40, 0xf2, 0x77, 0x03, 0x7d, 0x81, 0x2d, 0xeb,
+ 0x33, 0xa0, 0xf4, 0xa1, 0x39, 0x45, 0xd8, 0x98, 0xc2, 0x96,
+ };
+ static const uint8_t kY[] = {
+ 0x4f, 0xe3, 0x42, 0xe2, 0xfe, 0x1a, 0x7f, 0x9b, 0x8e, 0xe7, 0xeb,
+ 0x4a, 0x7c, 0x0f, 0x9e, 0x16, 0x2b, 0xce, 0x33, 0x57, 0x6b, 0x31,
+ 0x5e, 0xce, 0xcb, 0xb6, 0x40, 0x68, 0x37, 0xbf, 0x51, 0xf5,
+ };
+ static const uint8_t kOrder[] = {
+ 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xbc, 0xe6, 0xfa, 0xad, 0xa7, 0x17,
+ 0x9e, 0x84, 0xf3, 0xb9, 0xca, 0xc2, 0xfc, 0x63, 0x25, 0x51,
+ };
+ ScopedBIGNUM p(BN_bin2bn(kP, sizeof(kP), nullptr));
+ ScopedBIGNUM a(BN_bin2bn(kA, sizeof(kA), nullptr));
+ ScopedBIGNUM b(BN_bin2bn(kB, sizeof(kB), nullptr));
+ ScopedBIGNUM x(BN_bin2bn(kX, sizeof(kX), nullptr));
+ ScopedBIGNUM y(BN_bin2bn(kY, sizeof(kY), nullptr));
+ ScopedBIGNUM order(BN_bin2bn(kOrder, sizeof(kOrder), nullptr));
+ ScopedBIGNUM cofactor(BN_new());
+ if (!p || !a || !b || !x || !y || !order || !cofactor ||
+ !BN_set_word(cofactor.get(), 1)) {
+ return false;
+ }
+ ScopedEC_GROUP group(EC_GROUP_new_arbitrary(p.get(), a.get(), b.get(),
+ x.get(), y.get(), order.get(),
+ cofactor.get()));
+ if (!group) {
+ return false;
+ }
+
+ // |group| should not have a curve name.
+ if (EC_GROUP_get_curve_name(group.get()) != NID_undef) {
+ return false;
+ }
+
+ // Copy |key| to |key2| using |group|.
+ ScopedEC_KEY key2(EC_KEY_new());
+ ScopedEC_POINT point(EC_POINT_new(group.get()));
+ if (!key2 || !point ||
+ !EC_KEY_set_group(key2.get(), group.get()) ||
+ !EC_KEY_set_private_key(key2.get(), EC_KEY_get0_private_key(key.get())) ||
+ !EC_POINT_get_affine_coordinates_GFp(EC_KEY_get0_group(key.get()),
+ EC_KEY_get0_public_key(key.get()),
+ x.get(), y.get(), nullptr) ||
+ !EC_POINT_set_affine_coordinates_GFp(group.get(), point.get(), x.get(),
+ y.get(), nullptr) ||
+ !EC_KEY_set_public_key(key2.get(), point.get())) {
+ fprintf(stderr, "Could not copy key.\n");
+ return false;
+ }
+
+ // The key must be valid according to the new group too.
+ if (!EC_KEY_check_key(key2.get())) {
+ fprintf(stderr, "Copied key is not valid.\n");
+ return false;
+ }
+
+ return true;
+}
+
int main(void) {
CRYPTO_library_init();
ERR_load_crypto_strings();
@@ -249,7 +335,8 @@
!TestSetAffine(NID_secp224r1) ||
!TestSetAffine(NID_X9_62_prime256v1) ||
!TestSetAffine(NID_secp384r1) ||
- !TestSetAffine(NID_secp521r1)) {
+ !TestSetAffine(NID_secp521r1) ||
+ !TestArbitraryCurve()) {
fprintf(stderr, "failed\n");
return 1;
}
diff --git a/include/openssl/base.h b/include/openssl/base.h
index 65ab3f7..79f02cd 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -108,7 +108,7 @@
#endif
#define OPENSSL_IS_BORINGSSL
-#define BORINGSSL_201510
+#define BORINGSSL_201512
#define OPENSSL_VERSION_NUMBER 0x10002000
#define SSLEAY_VERSION_NUMBER OPENSSL_VERSION_NUMBER
diff --git a/include/openssl/ec.h b/include/openssl/ec.h
index 667be3b..26f40b7 100644
--- a/include/openssl/ec.h
+++ b/include/openssl/ec.h
@@ -271,16 +271,16 @@
/* Deprecated functions. */
-/* EC_GROUP_new_curve_GFp creates a new, arbitrary elliptic curve group based
- * on the equation y² = x³ + a·x + b. It returns the new group or NULL on
- * error.
+/* EC_GROUP_new_arbitrary creates a new, arbitrary elliptic curve group based on
+ * the equation y² = x³ + a·x + b. The generator is set to (gx, gy) which must
+ * have the given order and cofactor. It returns the new group or NULL on error.
*
* |EC_GROUP|s returned by this function will always compare as unequal via
* |EC_GROUP_cmp| (even to themselves). |EC_GROUP_get_curve_name| will always
* return |NID_undef|. */
-OPENSSL_EXPORT EC_GROUP *EC_GROUP_new_curve_GFp(const BIGNUM *p,
- const BIGNUM *a,
- const BIGNUM *b, BN_CTX *ctx);
+OPENSSL_EXPORT EC_GROUP *EC_GROUP_new_arbitrary(
+ const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, const BIGNUM *gx,
+ const BIGNUM *gy, const BIGNUM *order, const BIGNUM *cofactor);
/* 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
@@ -288,14 +288,6 @@
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|. */
-OPENSSL_EXPORT int EC_GROUP_set_generator(EC_GROUP *group,
- const EC_POINT *generator,
- const BIGNUM *order,
- const BIGNUM *cofactor);
-
/* EC_GROUP_set_asn1_flag does nothing. */
OPENSSL_EXPORT void EC_GROUP_set_asn1_flag(EC_GROUP *group, int flag);
