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