commit d62fe6f3e83a97d7fd81726ca8c2c4b84a2b3eb5
author David Benjamin <davidben@google.com> Tue Mar 06 20:28:10 2018 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Mar 07 21:17:02 2018 +0000
tree 23390709fe82bafc7809fb7877bbb07811926d9c
parent 40cdb3b5dad0a1028e40fbc73fa62a3e6d31fed7

Fold EC_KEY_copy into EC_KEY_dup.

EC_KEY_copy left unset fields alone, which meant it was possible to
create an EC_KEY with mismatched private key and group. Nothing was
using EC_KEY_copy anyway, and in keeping of us generally preferring
fresh objects over object reuse, remove it. EC_KEY_dup itself can also
be made simpler by using the very setters available.

Additionally, skip copying the method table. As of
https://boringssl-review.googlesource.com/16344, we no longer copy the
ex_data, so we probably shouldn't copy the method pointers either,
aligning with RSAPrivateKey_dup.

Update-Note: If I missed anything and someone uses EC_KEY_copy, it
   should be easy to port them to EC_KEY_dup.

Change-Id: Ibbdcea73345d91fa143fbe70a15bb527972693e8
Reviewed-on: https://boringssl-review.googlesource.com/26404
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/fipsmodule/ec/ec_key.c

diff --git a/crypto/fipsmodule/ec/ec_key.c b/crypto/fipsmodule/ec/ec_key.c
index 3fcc04f..b56139d 100644
--- a/crypto/fipsmodule/ec/ec_key.c
+++ b/crypto/fipsmodule/ec/ec_key.c
@@ -159,65 +159,29 @@
   OPENSSL_free(r);
 }
 
-EC_KEY *EC_KEY_copy(EC_KEY *dest, const EC_KEY *src) {
-  if (dest == NULL || src == NULL) {
+EC_KEY *EC_KEY_dup(const EC_KEY *src) {
+  if (src == NULL) {
     OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
     return NULL;
   }
-  // Copy the parameters.
-  if (src->group) {
-    // TODO(fork): duplicating the group seems wasteful.
-    EC_GROUP_free(dest->group);
-    dest->group = EC_GROUP_dup(src->group);
-    if (dest->group == NULL) {
-      return NULL;
-    }
-  }
 
-  // Copy the public key.
-  if (src->pub_key && src->group) {
-    EC_POINT_free(dest->pub_key);
-    dest->pub_key = EC_POINT_dup(src->pub_key, src->group);
-    if (dest->pub_key == NULL) {
-      return NULL;
-    }
-  }
-
-  // copy the private key
-  if (src->priv_key) {
-    if (dest->priv_key == NULL) {
-      dest->priv_key = BN_new();
-      if (dest->priv_key == NULL) {
-        return NULL;
-      }
-    }
-    if (!BN_copy(dest->priv_key, src->priv_key)) {
-      return NULL;
-    }
-  }
-  // copy method/extra data
-  if (src->ecdsa_meth) {
-      METHOD_unref(dest->ecdsa_meth);
-      dest->ecdsa_meth = src->ecdsa_meth;
-      METHOD_ref(dest->ecdsa_meth);
-  }
-
-  // copy the rest
-  dest->enc_flag = src->enc_flag;
-  dest->conv_form = src->conv_form;
-
-  return dest;
-}
-
-EC_KEY *EC_KEY_dup(const EC_KEY *ec_key) {
   EC_KEY *ret = EC_KEY_new();
   if (ret == NULL) {
     return NULL;
   }
-  if (EC_KEY_copy(ret, ec_key) == NULL) {
+
+  if ((src->group != NULL &&
+       !EC_KEY_set_group(ret, src->group)) ||
+      (src->pub_key != NULL &&
+       !EC_KEY_set_public_key(ret, src->pub_key)) ||
+      (src->priv_key != NULL &&
+       !EC_KEY_set_private_key(ret, src->priv_key))) {
     EC_KEY_free(ret);
     return NULL;
   }
+
+  ret->enc_flag = src->enc_flag;
+  ret->conv_form = src->conv_form;
   return ret;
 }
 

include/openssl/ec_key.h

diff --git a/include/openssl/ec_key.h b/include/openssl/ec_key.h
index 5fe0318..cc075e5 100644
--- a/include/openssl/ec_key.h
+++ b/include/openssl/ec_key.h
@@ -99,9 +99,6 @@
 // EC_KEY_free frees all the data owned by |key| and |key| itself.
 OPENSSL_EXPORT void EC_KEY_free(EC_KEY *key);
 
-// EC_KEY_copy sets |dst| equal to |src| and returns |dst| or NULL on error.
-OPENSSL_EXPORT EC_KEY *EC_KEY_copy(EC_KEY *dst, const EC_KEY *src);
-
 // EC_KEY_dup returns a fresh copy of |src| or NULL on error.
 OPENSSL_EXPORT EC_KEY *EC_KEY_dup(const EC_KEY *src);