Add CryptoNative_X509UpRef

Use X509UpRef instead of X509Duplicate

X509Duplicate copies the certificate data into new memory, but none of the
callers needed copy semantics, just ensuring that the native components
which also are using the same memory don't unexpectedly free the object.

Protection from early/double-free is what the native reference count field is
for, so use UpRef instead of Duplicate.

Since we are currently targeting OpenSSL 1.0.x X509Upref adds to
references directly, X509_up_ref does not exist in the 1.0.x ABI.

Microbenchmark:
Cloning a certificate 1000 times by cert.Handle, measuring maximum resident set:
X509Duplicate (before): 48213 +/- 1501KB
X509UpRef (after): 39208 +/- 1021KB
Memory reduction: 9005 +/- 1816KB

Commit migrated from https://github.com/dotnet/corefx/commit/8f5eac55eb498c3c94f5a89655f4d7ad79a8711a

diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
index 16b3ed2..2108276 100644 (file)
--- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
+++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
@@ -427,7 +427,7 @@ internal static partial class Interop
                         continue;
                     }
 
-                    using (SafeX509Handle certHandle = Crypto.X509Duplicate(certificate.Handle))
+                    using (SafeX509Handle certHandle = Crypto.X509UpRef(certificate.Handle))
                     {
                         using (SafeX509NameHandle nameHandle = Crypto.DuplicateX509Name(Crypto.X509GetIssuerName(certHandle)))
                         {

diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs
index ccd231f..e361168 100644 (file)
--- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs
+++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs
@@ -141,7 +141,7 @@ internal static partial class Interop
 
             for (int i = chain.ChainElements.Count - 2; i > 0; i--)
             {
-                SafeX509Handle dupCertHandle = Crypto.X509Duplicate(chain.ChainElements[i].Certificate.Handle);
+                SafeX509Handle dupCertHandle = Crypto.X509UpRef(chain.ChainElements[i].Certificate.Handle);
                 Crypto.CheckValidOpenSslHandle(dupCertHandle);
                 if (!SslAddExtraChainCert(sslContext, dupCertHandle))
                 {

diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs
index c8fb6de..ba09d5a 100644 (file)
--- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs
+++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs
@@ -32,12 +32,32 @@ internal static partial class Interop
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509Destroy")]
         internal static extern void X509Destroy(IntPtr a);
 
+        /// <summary>
+        /// Clone the input certificate into a new object.
+        /// </summary>
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509Duplicate")]
         internal static extern SafeX509Handle X509Duplicate(IntPtr handle);
 
+        /// <summary>
+        /// Clone the input certificate into a new object.
+        /// </summary>
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509Duplicate")]
         internal static extern SafeX509Handle X509Duplicate(SafeX509Handle handle);
 
+        /// <summary>
+        /// Increment the native reference count of the certificate to protect against
+        /// a free from another pointer-holder.
+        /// </summary>
+        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509UpRef")]
+        internal static extern SafeX509Handle X509UpRef(IntPtr handle);
+
+        /// <summary>
+        /// Increment the native reference count of the certificate to protect against
+        /// a free from another pointer-holder.
+        /// </summary>
+        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509UpRef")]
+        internal static extern SafeX509Handle X509UpRef(SafeX509Handle handle);
+
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_PemReadX509FromBio")]
         internal static extern SafeX509Handle PemReadX509FromBio(SafeBioHandle bio);
 

diff --git a/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs b/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
index c4314f4..bf4e092 100644 (file)
--- a/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
+++ b/src/libraries/Common/src/System/Net/Security/CertificateValidation.Unix.cs
@@ -28,7 +28,7 @@ namespace System.Net.Security
             }
 
             int hostNameMatch;
-            using (SafeX509Handle certHandle = Interop.Crypto.X509Duplicate(remoteCertificate.Handle))
+            using (SafeX509Handle certHandle = Interop.Crypto.X509UpRef(remoteCertificate.Handle))
             {
                 IPAddress hostnameAsIp;
                 if (IPAddress.TryParse(hostName, out hostnameAsIp))

diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs
index 3039de4..4d9995d 100644 (file)
--- a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs
+++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs
@@ -79,7 +79,7 @@ namespace System.Net.Security
                     throw new NotSupportedException(SR.net_ssl_io_no_server_cert);
                 }
 
-                _certHandle = Interop.Crypto.X509Duplicate(cert.Handle);
+                _certHandle = Interop.Crypto.X509UpRef(cert.Handle);
                 Interop.Crypto.CheckValidOpenSslHandle(_certHandle);
             }
 

diff --git a/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.cpp b/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.cpp
index 58cca0f..72ffb1c 100644 (file)
--- a/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.cpp
+++ b/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.cpp
@@ -291,3 +291,13 @@ extern "C" int32_t CryptoNative_EncodeX509SubjectPublicKeyInfo(X509* x509, uint8
     // X509_get_X509_PUBKEY returns an interior pointer, so should not be freed
     return i2d_X509_PUBKEY(X509_get_X509_PUBKEY(x509), &buf);
 }
+
+extern "C" X509* CryptoNative_X509UpRef(X509* x509)
+{
+    if (x509 != nullptr)
+    {
+        CRYPTO_add(&x509->references, 1, CRYPTO_LOCK_X509);
+    }
+
+    return x509;
+}

diff --git a/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.h b/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.h
index 5272884..e2be7a2 100644 (file)
--- a/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.h
+++ b/src/libraries/Native/System.Security.Cryptography.Native/pal_x509.h
@@ -293,3 +293,13 @@ Shims the i2d_X509_PUBKEY method, providing X509_get_X509_PUBKEY(x) as the input
 Returns the number of bytes written to buf.
 */
 extern "C" int32_t CryptoNative_EncodeX509SubjectPublicKeyInfo(X509* x, uint8_t* buf);
+
+/*
+Increases the reference count of the X509*, thereby increasing the number of calls
+required to the free function.
+
+Unlike X509Duplicate, this modifies an existing object, so no new memory is allocated.
+
+Returns the input value.
+*/
+extern "C" X509* CryptoNative_X509UpRef(X509* x509);

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.ClientCertificateProvider.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.ClientCertificateProvider.cs
index 9621963..afeb5fb 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.ClientCertificateProvider.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/Unix/CurlHandler.ClientCertificateProvider.cs
@@ -97,7 +97,7 @@ namespace System.Net.Http
                         return NoCertificateSet;
                     }
 
-                    SafeX509Handle certSafeHandle = Interop.Crypto.X509Duplicate(certificate.Handle);
+                    SafeX509Handle certSafeHandle = Interop.Crypto.X509UpRef(certificate.Handle);
                     Interop.Crypto.CheckValidOpenSslHandle(certSafeHandle);
                     if (chain != null)
                     {

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs
index 7574660..a26d5dc 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs
@@ -17,7 +17,7 @@ namespace Internal.Cryptography.Pal
             if (handle == IntPtr.Zero)
                 throw new ArgumentException(SR.Arg_InvalidHandle, nameof(handle));
 
-            return new OpenSslX509CertificateReader(Interop.Crypto.X509Duplicate(handle));
+            return new OpenSslX509CertificateReader(Interop.Crypto.X509UpRef(handle));
         }
 
         public static ICertificatePal FromBlob(byte[] rawData, string password, X509KeyStorageFlags keyStorageFlags)

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/ExportProvider.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/ExportProvider.cs
index f7fde74..5477e3c 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/ExportProvider.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/ExportProvider.cs
@@ -142,7 +142,7 @@ namespace Internal.Cryptography.Pal
 
         private static void PushHandle(IntPtr certPtr, SafeX509StackHandle publicCerts)
         {
-            using (SafeX509Handle certHandle = Interop.Crypto.X509Duplicate(certPtr))
+            using (SafeX509Handle certHandle = Interop.Crypto.X509UpRef(certPtr))
             {
                 if (!Interop.Crypto.PushX509StackField(publicCerts, certHandle))
                 {

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslPkcs12Reader.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslPkcs12Reader.cs
index dce4b9b..4656b56 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslPkcs12Reader.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslPkcs12Reader.cs
@@ -106,8 +106,8 @@ namespace Internal.Cryptography.Pal
 
                     if (certPtr != IntPtr.Zero)
                     {
-                        // The STACK_OF(X509) still needs to be cleaned up, so duplicate the handle out of it.
-                        certs.Add(new OpenSslX509CertificateReader(Interop.Crypto.X509Duplicate(certPtr)));
+                        // The STACK_OF(X509) still needs to be cleaned up, so upref the handle out of it.
+                        certs.Add(new OpenSslX509CertificateReader(Interop.Crypto.X509UpRef(certPtr)));
                     }
                 }
             }

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
index 86c296f..590f038 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
@@ -338,7 +338,7 @@ namespace Internal.Cryptography.Pal
 
         internal OpenSslX509CertificateReader DuplicateHandles()
         {
-            SafeX509Handle certHandle = Interop.Crypto.X509Duplicate(_cert);
+            SafeX509Handle certHandle = Interop.Crypto.X509UpRef(_cert);
             OpenSslX509CertificateReader duplicate = new OpenSslX509CertificateReader(certHandle);
 
             if (_privateKey != null)