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>

crypto/ec_extra/ec_asn1.c

@@ -159,8 +159,8 @@ EC_KEY *EC_KEY_parse_private_key(CBS *cbs, const EC_GROUP *group) {
         (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,
+    if (!ec_point_mul_scalar(group, &ret->pub_key->raw, &ret->priv_key->scalar,
-                             NULL)) {
+                             NULL, NULL)) {
       goto err;
     }
     // Remember the original private-key-only encoding.

crypto/ecdh_extra/ecdh_extra.c

@@ -87,6 +87,11 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
     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 @@ int ECDH_compute_key(void *out, size_t outlen, const EC_POINT *pub_key,
   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;
   }

crypto/fipsmodule/ec/ec.c

@@ -865,6 +865,12 @@ int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar,
     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 @@ int EC_POINT_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *g_scalar,
     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 @@ err:
   return ret;
 }
 
-int ec_point_mul_scalar_public(const EC_GROUP *group, EC_POINT *r,
+int ec_point_mul_scalar_public(const EC_GROUP *group, EC_RAW_POINT *r,
-                               const EC_SCALAR *g_scalar, const EC_POINT *p,
+                               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 ||
+  group->meth->mul_public(group, r, g_scalar, p, p_scalar);
-      (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);
   return 1;
 }
 
-int ec_point_mul_scalar(const EC_GROUP *group, EC_POINT *r,
+int ec_point_mul_scalar(const EC_GROUP *group, EC_RAW_POINT *r,
-                        const EC_SCALAR *g_scalar, const EC_POINT *p,
+                        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 ||
+  group->meth->mul(group, r, g_scalar, p, p_scalar);
-      (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);
   return 1;
 }
 

crypto/fipsmodule/ec/ec_key.c

@@ -322,8 +322,8 @@ int EC_KEY_check_key(const EC_KEY *eckey) {
   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,
+        !ec_point_mul_scalar(eckey->group, &point->raw,
-                             NULL, NULL)) {
+                             &eckey->priv_key->scalar, NULL, NULL)) {
       OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
       goto err;
     }
@@ -413,7 +413,7 @@ int EC_KEY_generate_key(EC_KEY *key) {
       // 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);

crypto/fipsmodule/ec/ec_test.cc

@@ -726,7 +726,8 @@ TEST_P(ECCurveTest, DoubleSpecialCase) {
 
   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 @@ TEST(ECTest, ScalarBaseMultVectors) {
       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());
     }

crypto/fipsmodule/ec/internal.h

@@ -239,6 +239,10 @@ struct ec_point_st {
   // 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 @@ int ec_scalar_inv_montgomery_vartime(const EC_GROUP *group, EC_SCALAR *r,
 // |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,
+int ec_point_mul_scalar(const EC_GROUP *group, EC_RAW_POINT *r,
-                        const EC_SCALAR *g_scalar, const EC_POINT *p,
+                        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(
+OPENSSL_EXPORT int ec_point_mul_scalar_public(const EC_GROUP *group,
-    const EC_GROUP *group, EC_POINT *r, const EC_SCALAR *g_scalar,
+                                              EC_RAW_POINT *r,
-    const EC_POINT *p, const EC_SCALAR *p_scalar);
+                                              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

crypto/fipsmodule/ecdh/ecdh.c

@@ -86,6 +86,11 @@ int ECDH_compute_key_fips(uint8_t *out, size_t out_len, const EC_POINT *pub_key,
     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 @@ int ECDH_compute_key_fips(uint8_t *out, size_t out_len, const EC_POINT *pub_key,
   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;
   }

crypto/fipsmodule/ecdsa/ecdsa.c

@@ -173,27 +173,18 @@ int ECDSA_do_verify(const uint8_t *digest, size_t digest_len,
   ec_scalar_mul_montgomery(group, &u1, &m, &s_inv_mont);
   ec_scalar_mul_montgomery(group, &u2, &r, &s_inv_mont);
 
-  int ret = 0;
+  EC_RAW_POINT point;
-  EC_POINT *point = EC_POINT_new(group);
+  if (!ec_point_mul_scalar_public(group, &point, &u1, &pub_key->raw, &u2)) {
-  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)) {
     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;
+  return 1;
-
-err:
-  EC_POINT_free(point);
-  return ret;
 }
 
 static int ecdsa_sign_setup(const EC_KEY *eckey, EC_SCALAR *out_kinv_mont,
@@ -210,12 +201,7 @@ static int ecdsa_sign_setup(const EC_KEY *eckey, EC_SCALAR *out_kinv_mont,
 
   int ret = 0;
   EC_SCALAR k;
-  EC_POINT *tmp_point = EC_POINT_new(group);
+  EC_RAW_POINT tmp_point;
-  if (tmp_point == NULL) {
-    OPENSSL_PUT_ERROR(ECDSA, ERR_R_EC_LIB);
-    goto err;
-  }
-
   do {
     // Include the private key and message digest in the k generation.
     if (eckey->fixed_k != NULL) {
@@ -246,8 +232,8 @@ static int ecdsa_sign_setup(const EC_KEY *eckey, EC_SCALAR *out_kinv_mont,
     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) ||
+    if (!ec_point_mul_scalar(group, &tmp_point, &k, NULL, NULL) ||
-        !ec_get_x_coordinate_as_scalar(group, out_r, &tmp_point->raw)) {
+        !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 @@ static int ecdsa_sign_setup(const EC_KEY *eckey, EC_SCALAR *out_kinv_mont,
 
 err:
   OPENSSL_cleanse(&k, sizeof(k));
-  EC_POINT_free(tmp_point);
   return ret;
 }