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