Use EC_RAW_POINT in ECDSA.
Now the only allocations in ECDSA are the ECDSA_SIG input and output.
Change-Id: If1fcde6dc2ee2c53f5adc16a7f692e22e9c238de
Reviewed-on: https://boringssl-review.googlesource.com/c/33069
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/ec_extra/ec_asn1.c b/crypto/ec_extra/ec_asn1.c
index 9d9a200..6e21275 100644
--- a/crypto/ec_extra/ec_asn1.c
+++ b/crypto/ec_extra/ec_asn1.c
@@ -159,8 +159,8 @@
(point_conversion_form_t)(CBS_data(&public_key)[0] & ~0x01);
} else {
// Compute the public key instead.
- if (!ec_point_mul_scalar(group, ret->pub_key, &ret->priv_key->scalar, NULL,
- NULL)) {
+ if (!ec_point_mul_scalar(group, &ret->pub_key->raw, &ret->priv_key->scalar,
+ NULL, NULL)) {
goto err;
}
// Remember the original private-key-only encoding.
diff --git a/crypto/ecdh_extra/ecdh_extra.c b/crypto/ecdh_extra/ecdh_extra.c
index 80dcfb0..eb321f7 100644
--- a/crypto/ecdh_extra/ecdh_extra.c
+++ b/crypto/ecdh_extra/ecdh_extra.c
@@ -87,6 +87,11 @@
return -1;
}
const EC_SCALAR *const priv = &priv_key->priv_key->scalar;
+ const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
+ if (EC_GROUP_cmp(group, pub_key->group, NULL) != 0) {
+ OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
+ return -1;
+ }
BN_CTX *ctx = BN_CTX_new();
if (ctx == NULL) {
@@ -98,14 +103,13 @@
size_t buflen = 0;
uint8_t *buf = NULL;
- const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
EC_POINT *tmp = EC_POINT_new(group);
if (tmp == NULL) {
OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
goto err;
}
- if (!ec_point_mul_scalar(group, tmp, NULL, pub_key, priv)) {
+ if (!ec_point_mul_scalar(group, &tmp->raw, NULL, &pub_key->raw, priv)) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
goto err;
}
diff --git a/crypto/fipsmodule/ec/ec.c b/crypto/fipsmodule/ec/ec.c
index 4420c08..34383e8 100644
--- a/crypto/fipsmodule/ec/ec.c
+++ b/crypto/fipsmodule/ec/ec.c
@@ -865,6 +865,12 @@
return 0;
}
+ if (EC_GROUP_cmp(group, r->group, NULL) != 0 ||
+ (p != NULL && EC_GROUP_cmp(group, p->group, NULL) != 0)) {
+ OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
+ return 0;
+ }
+
int ret = 0;
EC_SCALAR g_scalar_storage, p_scalar_storage;
EC_SCALAR *g_scalar_arg = NULL, *p_scalar_arg = NULL;
@@ -891,7 +897,8 @@
p_scalar_arg = &p_scalar_storage;
}
- ret = ec_point_mul_scalar(group, r, g_scalar_arg, p, p_scalar_arg);
+ ret = ec_point_mul_scalar(group, &r->raw, g_scalar_arg,
+ p == NULL ? NULL : &p->raw, p_scalar_arg);
err:
BN_CTX_free(new_ctx);
@@ -900,42 +907,29 @@
return ret;
}
-int ec_point_mul_scalar_public(const EC_GROUP *group, EC_POINT *r,
- const EC_SCALAR *g_scalar, const EC_POINT *p,
+int ec_point_mul_scalar_public(const EC_GROUP *group, EC_RAW_POINT *r,
+ const EC_SCALAR *g_scalar, const EC_RAW_POINT *p,
const EC_SCALAR *p_scalar) {
if ((g_scalar == NULL && p_scalar == NULL) ||
- (p == NULL) != (p_scalar == NULL)) {
+ (p == NULL) != (p_scalar == NULL)) {
OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
- if (EC_GROUP_cmp(group, r->group, NULL) != 0 ||
- (p != NULL && EC_GROUP_cmp(group, p->group, NULL) != 0)) {
- OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
- return 0;
- }
-
- group->meth->mul_public(group, &r->raw, g_scalar, &p->raw, p_scalar);
+ group->meth->mul_public(group, r, g_scalar, p, p_scalar);
return 1;
}
-int ec_point_mul_scalar(const EC_GROUP *group, EC_POINT *r,
- const EC_SCALAR *g_scalar, const EC_POINT *p,
+int ec_point_mul_scalar(const EC_GROUP *group, EC_RAW_POINT *r,
+ const EC_SCALAR *g_scalar, const EC_RAW_POINT *p,
const EC_SCALAR *p_scalar) {
if ((g_scalar == NULL && p_scalar == NULL) ||
- (p == NULL) != (p_scalar == NULL)) {
+ (p == NULL) != (p_scalar == NULL)) {
OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
- if (EC_GROUP_cmp(group, r->group, NULL) != 0 ||
- (p != NULL && EC_GROUP_cmp(group, p->group, NULL) != 0)) {
- OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
- return 0;
- }
-
- group->meth->mul(group, &r->raw, g_scalar, (p == NULL) ? NULL : &p->raw,
- p_scalar);
+ group->meth->mul(group, r, g_scalar, p, p_scalar);
return 1;
}
diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c
index defd77c..632dc9b 100644
--- a/crypto/fipsmodule/ec/ec_key.c
+++ b/crypto/fipsmodule/ec/ec_key.c
@@ -322,8 +322,8 @@
if (eckey->priv_key != NULL) {
point = EC_POINT_new(eckey->group);
if (point == NULL ||
- !ec_point_mul_scalar(eckey->group, point, &eckey->priv_key->scalar,
- NULL, NULL)) {
+ !ec_point_mul_scalar(eckey->group, &point->raw,
+ &eckey->priv_key->scalar, NULL, NULL)) {
OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
goto err;
}
@@ -413,7 +413,7 @@
// Generate the private key by testing candidates (FIPS 186-4 B.4.2).
!ec_random_nonzero_scalar(key->group, &priv_key->scalar,
kDefaultAdditionalData) ||
- !ec_point_mul_scalar(key->group, pub_key, &priv_key->scalar, NULL,
+ !ec_point_mul_scalar(key->group, &pub_key->raw, &priv_key->scalar, NULL,
NULL)) {
EC_POINT_free(pub_key);
ec_wrapped_scalar_free(priv_key);
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc
index d45a52f..87146ad 100644
--- a/crypto/fipsmodule/ec/ec_test.cc
+++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -726,7 +726,8 @@
EC_SCALAR one;
ASSERT_TRUE(ec_bignum_to_scalar(group(), &one, BN_value_one()));
- ASSERT_TRUE(ec_point_mul_scalar_public(group(), p.get(), &one, g, &one));
+ ASSERT_TRUE(
+ ec_point_mul_scalar_public(group(), &p->raw, &one, &g->raw, &one));
EXPECT_EQ(0, EC_POINT_cmp(group(), p.get(), two_g.get(), nullptr));
}
@@ -871,7 +872,7 @@
EC_SCALAR a_scalar, b_scalar;
ASSERT_TRUE(ec_bignum_to_scalar(group.get(), &a_scalar, a.get()));
ASSERT_TRUE(ec_bignum_to_scalar(group.get(), &b_scalar, b.get()));
- ASSERT_TRUE(ec_point_mul_scalar_public(group.get(), p.get(), &a_scalar, g,
+ ASSERT_TRUE(ec_point_mul_scalar_public(group.get(), &p->raw, &a_scalar, &g->raw,
&b_scalar));
check_point(p.get());
}
diff --git a/crypto/fipsmodule/ec/internal.h b/crypto/fipsmodule/ec/internal.h
index be548a3..7c7937b 100644
--- a/crypto/fipsmodule/ec/internal.h
+++ b/crypto/fipsmodule/ec/internal.h
@@ -239,6 +239,10 @@
// group is an owning reference to |group|, unless this is
// |group->generator|.
EC_GROUP *group;
+ // raw is the group-specific point data. Functions that take |EC_POINT|
+ // typically check consistency with |EC_GROUP| while functions that take
+ // |EC_RAW_POINT| do not. Thus accesses to this field should be externally
+ // checked for consistency.
EC_RAW_POINT raw;
} /* EC_POINT */;
@@ -325,16 +329,18 @@
// |p_scalar|. Unlike other functions which take |EC_SCALAR|, |g_scalar| and
// |p_scalar| need not be fully reduced. They need only contain as many bits as
// the order.
-int ec_point_mul_scalar(const EC_GROUP *group, EC_POINT *r,
- const EC_SCALAR *g_scalar, const EC_POINT *p,
+int ec_point_mul_scalar(const EC_GROUP *group, EC_RAW_POINT *r,
+ const EC_SCALAR *g_scalar, const EC_RAW_POINT *p,
const EC_SCALAR *p_scalar);
// ec_point_mul_scalar_public performs the same computation as
// ec_point_mul_scalar. It further assumes that the inputs are public so
// there is no concern about leaking their values through timing.
-OPENSSL_EXPORT int ec_point_mul_scalar_public(
- const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar,
- const EC_POINT *p, const EC_SCALAR *p_scalar);
+OPENSSL_EXPORT int ec_point_mul_scalar_public(const EC_GROUP *group,
+ EC_RAW_POINT *r,
+ const EC_SCALAR *g_scalar,
+ const EC_RAW_POINT *p,
+ const EC_SCALAR *p_scalar);
// ec_cmp_x_coordinate compares the x (affine) coordinate of |p|, mod the group
// order, with |r|. It returns one if the values match and zero if |p| is the
diff --git a/crypto/fipsmodule/ecdh/ecdh.c b/crypto/fipsmodule/ecdh/ecdh.c
index 726fa6d..19d12c9 100644
--- a/crypto/fipsmodule/ecdh/ecdh.c
+++ b/crypto/fipsmodule/ecdh/ecdh.c
@@ -86,6 +86,11 @@
return 0;
}
const EC_SCALAR *const priv = &priv_key->priv_key->scalar;
+ const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
+ if (EC_GROUP_cmp(group, pub_key->group, NULL) != 0) {
+ OPENSSL_PUT_ERROR(EC, EC_R_INCOMPATIBLE_OBJECTS);
+ return 0;
+ }
BN_CTX *ctx = BN_CTX_new();
if (ctx == NULL) {
@@ -97,14 +102,14 @@
size_t buflen = 0;
uint8_t *buf = NULL;
- const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
EC_POINT *shared_point = EC_POINT_new(group);
if (shared_point == NULL) {
OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
goto err;
}
- if (!ec_point_mul_scalar(group, shared_point, NULL, pub_key, priv)) {
+ if (!ec_point_mul_scalar(group, &shared_point->raw, NULL, &pub_key->raw,
+ priv)) {
OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
goto err;
}
diff --git a/crypto/fipsmodule/ecdsa/ecdsa.c b/crypto/fipsmodule/ecdsa/ecdsa.c
index 6d5d388..0e89b43 100644
--- a/crypto/fipsmodule/ecdsa/ecdsa.c
+++ b/crypto/fipsmodule/ecdsa/ecdsa.c
@@ -173,27 +173,18 @@
ec_scalar_mul_montgomery(group, &u1, &m, &s_inv_mont);
ec_scalar_mul_montgomery(group, &u2, &r, &s_inv_mont);
- int ret = 0;
- EC_POINT *point = EC_POINT_new(group);
- if (point == NULL) {
- OPENSSL_PUT_ERROR(ECDSA, ERR_R_MALLOC_FAILURE);
- goto err;
- }
- if (!ec_point_mul_scalar_public(group, point, &u1, pub_key, &u2)) {
+ EC_RAW_POINT point;
+ if (!ec_point_mul_scalar_public(group, &point, &u1, &pub_key->raw, &u2)) {
OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
- goto err;
+ return 0;
}
- if (!ec_cmp_x_coordinate(group, &point->raw, &r)) {
+ if (!ec_cmp_x_coordinate(group, &point, &r)) {
OPENSSL_PUT_ERROR(ECDSA, ECDSA_R_BAD_SIGNATURE);
- goto err;
+ return 0;
}
- ret = 1;
-
-err:
- EC_POINT_free(point);
- return ret;
+ return 1;
}
static int ecdsa_sign_setup(const EC_KEY *eckey, EC_SCALAR *out_kinv_mont,
@@ -210,12 +201,7 @@
int ret = 0;
EC_SCALAR k;
- EC_POINT *tmp_point = EC_POINT_new(group);
- if (tmp_point == NULL) {
- OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
- goto err;
- }
-
+ EC_RAW_POINT tmp_point;
do {
// Include the private key and message digest in the k generation.
if (eckey->fixed_k != NULL) {
@@ -246,8 +232,8 @@
ec_scalar_from_montgomery(group, out_kinv_mont, out_kinv_mont);
// Compute r, the x-coordinate of generator * k.
- if (!ec_point_mul_scalar(group, tmp_point, &k, NULL, NULL) ||
- !ec_get_x_coordinate_as_scalar(group, out_r, &tmp_point->raw)) {
+ if (!ec_point_mul_scalar(group, &tmp_point, &k, NULL, NULL) ||
+ !ec_get_x_coordinate_as_scalar(group, out_r, &tmp_point)) {
goto err;
}
} while (ec_scalar_is_zero(group, out_r));
@@ -256,7 +242,6 @@
err:
OPENSSL_cleanse(&k, sizeof(k));
- EC_POINT_free(tmp_point);
return ret;
}