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; }