Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVP_PKEY_get0 implementation #1749

Merged
merged 3 commits into from
Aug 16, 2024
Merged

EVP_PKEY_get0 implementation #1749

merged 3 commits into from
Aug 16, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Aug 7, 2024

Issues:

Addresses CryptoAlg-1716

Description of changes:

  • Provide a working implementation of the (deprecated) function EVP_PKEY_get0.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth requested a review from a team as a code owner August 7, 2024 16:28
@justsmth justsmth requested a review from samuel40791765 August 7, 2024 16:28
@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.30%. Comparing base (94f021b) to head (8ff46c9).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1749      +/-   ##
==========================================
- Coverage   78.46%   78.30%   -0.16%     
==========================================
  Files         580      580              
  Lines       96779    97002     +223     
  Branches    13865    13903      +38     
==========================================
+ Hits        75936    75956      +20     
- Misses      20226    20425     +199     
- Partials      617      621       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

include/openssl/evp.h Show resolved Hide resolved
crypto/fipsmodule/evp/evp.c Show resolved Hide resolved
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index d738e5907..830605086 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -2462,9 +2462,13 @@ TEST(ASN1Test, ASN1Dup) {
             0);

   bssl::UniquePtr<EVP_PKEY> evp_pkey(EVP_PKEY_new());
+  OPENSSL_BEGIN_ALLOW_DEPRECATED
+  ASSERT_FALSE(EVP_PKEY_get0(evp_pkey.get()));
   X509_PUBKEY *tmp_key = nullptr;
   ASSERT_TRUE(evp_pkey);
   ASSERT_TRUE(EVP_PKEY_set1_EC_KEY(evp_pkey.get(), key.get()));
+  ASSERT_EQ(key.get(), EVP_PKEY_get0(evp_pkey.get()));
+  OPENSSL_END_ALLOW_DEPRECATED
   ASSERT_TRUE(X509_PUBKEY_set(&tmp_key, evp_pkey.get()));
   bssl::UniquePtr<X509_PUBKEY> x509_pubkey(tmp_key);
   bssl::UniquePtr<X509_PUBKEY> x509_pubkey_copy((X509_PUBKEY *)ASN1_dup(
diff --git a/crypto/ecdh_extra/ecdh_test.cc b/crypto/ecdh_extra/ecdh_test.cc
index 45c40db3d..eaa29956b 100644
--- a/crypto/ecdh_extra/ecdh_test.cc
+++ b/crypto/ecdh_extra/ecdh_test.cc
@@ -264,7 +264,9 @@ static void RunWycheproofTest(FileTest *t) {
   }
   EC_KEY *peer_ec = EVP_PKEY_get0_EC_KEY(peer_evp.get());
   ASSERT_TRUE(peer_ec);
-
+  OPENSSL_BEGIN_ALLOW_DEPRECATED
+  ASSERT_EQ(peer_ec, EVP_PKEY_get0(peer_evp.get()));
+  OPENSSL_END_ALLOW_DEPRECATED
   bssl::UniquePtr<EC_KEY> key(EC_KEY_new());
   ASSERT_TRUE(key);
   ASSERT_TRUE(EC_KEY_set_group(key.get(), group));
diff --git a/crypto/evp_extra/evp_test.cc b/crypto/evp_extra/evp_test.cc
index fb87e4981..9202e9490 100644
--- a/crypto/evp_extra/evp_test.cc
+++ b/crypto/evp_extra/evp_test.cc
@@ -662,6 +662,9 @@ static void RunWycheproofVerifyTest(const char *path) {
     if (EVP_PKEY_id(key.get()) == EVP_PKEY_DSA) {
       // DSA is deprecated and is not usable via EVP.
       DSA *dsa = EVP_PKEY_get0_DSA(key.get());
+      OPENSSL_BEGIN_ALLOW_DEPRECATED
+      ASSERT_EQ(dsa, EVP_PKEY_get0(key.get()));
+      OPENSSL_END_ALLOW_DEPRECATED
       uint8_t digest[EVP_MAX_MD_SIZE];
       unsigned digest_len;
       ASSERT_TRUE(
@@ -1037,7 +1040,11 @@ static EVP_PKEY * instantiate_and_set_private_key(const uint8_t *private_key,
     BN_free(private_key_bn);
     pkey = EVP_PKEY_new();
     EXPECT_TRUE(pkey);
+    OPENSSL_BEGIN_ALLOW_DEPRECATED
+    EXPECT_FALSE(EVP_PKEY_get0(pkey));
     EXPECT_TRUE(EVP_PKEY_assign(pkey, key_type, (EC_KEY *) ec_key));
+    EXPECT_EQ(ec_key, EVP_PKEY_get0(pkey));
+    OPENSSL_END_ALLOW_DEPRECATED
   }

   return pkey;
diff --git a/crypto/fipsmodule/evp/evp.c b/crypto/fipsmodule/evp/evp.c
index 7257621ce..06e84738b 100644
--- a/crypto/fipsmodule/evp/evp.c
+++ b/crypto/fipsmodule/evp/evp.c
@@ -581,12 +581,11 @@ int EVP_PKEY_CTX_get_signature_md(EVP_PKEY_CTX *ctx, const EVP_MD **out_md) {

 void *EVP_PKEY_get0(const EVP_PKEY *pkey) {
   SET_DIT_AUTO_DISABLE;
-  // Node references, but never calls this function, so for now we return NULL.
-  // If other projects require complete support, call |EVP_PKEY_get0_RSA|, etc.,
-  // rather than reading |pkey->pkey.ptr| directly. This avoids problems if our
-  // internal representation does not match the type the caller expects from
-  // OpenSSL.
-  return NULL;
+  GUARD_PTR(pkey);
+  if (pkey->type == EVP_PKEY_NONE) {
+    return NULL;
+  }
+  return pkey->pkey.ptr;
 }

 void OpenSSL_add_all_algorithms(void) {}
diff --git a/include/openssl/evp.h b/include/openssl/evp.h
index eea831786..53719f3d0 100644
--- a/include/openssl/evp.h
+++ b/include/openssl/evp.h
@@ -1142,17 +1142,17 @@ OPENSSL_EXPORT EVP_PKEY *EVP_PKEY_new_mac_key(int type, ENGINE *engine,
                                               size_t mac_key_len);

-// General No-op Functions [Deprecated].
+// Deprecated functions

-// EVP_PKEY_get0 returns NULL. This function is provided for compatibility with
-// OpenSSL but does not return anything. Use the typed |EVP_PKEY_get0_*|
-// functions instead.
+// EVP_PKEY_get0 returns the consumed key. The type of value returned differs
+// depending on the type of the |EVP_PKEY|.
 //
-// Note: In OpenSSL, the returned type will be different depending on the type
-//       of |EVP_PKEY| consumed. This leads to misuage very easily and has been
-//       deprecated as a no-op to avoid so.
+// This function is provided only for compatibility with OpenSSL.
+// Prefer the use the typed |EVP_PKEY_get0_*| functions instead.
 OPENSSL_EXPORT OPENSSL_DEPRECATED void *EVP_PKEY_get0(const EVP_PKEY *pkey);

+// General No-op Functions [Deprecated].
+
 // OpenSSL_add_all_algorithms does nothing. This has been deprecated since
 // OpenSSL 1.1.0.
 //
@justsmth justsmth force-pushed the evp-pkey-get0 branch 2 times, most recently from f88c7c8 to fb03bff Compare August 15, 2024 19:01
@justsmth justsmth merged commit 9b4b5a1 into aws:main Aug 16, 2024
106 checks passed
@justsmth justsmth deleted the evp-pkey-get0 branch August 16, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants