Delayed client certificate (#54692)

* initial prototype * Restore TLS 1.2 renegotiation * First windows functionality merge * reenable client certificate * Add more renegotiate tests * Remove client certificates * Cleanup * add test log * Apply PR comments * Add Data frame test * Add drain buffer test * Fix tls 1.3 incomming app data frame * Restore verify callback * Remove debug log * Remove keylog callback and unused method * Fix test build * Attempt to fix openssl version api difference * Sort shim * fix build * CI log * Restore mac tests * Add logs * fix test runs on old openssl * fix tests * fix w7 condition * feedback from review Co-authored-by: wfurt <[email protected]>
dotnet · Jul 13, 2021 · 2c275e8 · 2c275e8
1 parent a490a34
commit 2c275e8
Show file tree

Hide file tree

Showing 16 changed files with 215 additions and 95 deletions.
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
@@ -6,6 +6,7 @@
 using System.Diagnostics;
 using System.Globalization;
 using System.IO;
+using System.Net;
 using System.Net.Security;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
@@ -223,6 +224,19 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50
             return context;
         }
 
+        internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out byte[]? outputBuffer)
+        {
+            int ret = Interop.Ssl.SslRenegotiate(sslContext);
+
+            outputBuffer = Array.Empty<byte>();
+            if (ret != 1)
+            {
+                GetSslError(sslContext, ret, out Exception? exception);
+                return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, exception);
+            }
+            return new SecurityStatusPal(SecurityStatusPalErrorCode.OK);
+        }
+
         internal static bool DoSslHandshake(SafeSslHandle context, ReadOnlySpan<byte> input, out byte[]? sendBuf, out int sendCount)
         {
             sendBuf = null;

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
@@ -74,6 +74,9 @@ internal static partial class Ssl
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRead", SetLastError = true)]
         internal static extern int SslRead(SafeSslHandle ssl, ref byte buf, int num);
 
+        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRenegotiate")]
+        internal static extern int SslRenegotiate(SafeSslHandle ssl);
+
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_IsSslRenegotiatePending")]
         [return: MarshalAs(UnmanagedType.Bool)]
         internal static extern bool IsSslRenegotiatePending(SafeSslHandle ssl);

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
@@ -785,6 +785,13 @@ unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options)
     return (unsigned long)SSL_CTX_ctrl(ctx, SSL_CTRL_OPTIONS, (long)options, NULL);
 }
 
+unsigned long local_SSL_set_options(SSL* ssl, unsigned long options)
+{
+   // SSL_ctrl is signed long in and signed long out; but SSL_set_options,
+   // which was a macro call to SSL_ctrl in 1.0, is unsigned/unsigned.
+   return (unsigned long)SSL_ctrl(ssl, SSL_CTRL_OPTIONS, (long)options, NULL);
+}
+
 int local_SSL_session_reused(SSL* ssl)
 {
     return (int)SSL_ctrl(ssl, SSL_CTRL_GET_SESSION_REUSED, 0, NULL);

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h
@@ -32,6 +32,7 @@ int32_t local_RSA_pkey_ctx_ctrl(EVP_PKEY_CTX* ctx, int32_t optype, int32_t cmd,
 int32_t local_SSL_is_init_finished(const SSL* ssl);
 int32_t local_SSL_CTX_config(SSL_CTX* ctx, const char* name);
 unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options);
+unsigned long local_SSL_set_options(SSL* ssl, unsigned long options);
 void local_SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level);
 int local_SSL_session_reused(SSL* ssl);
 int32_t local_X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername);

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c
@@ -286,6 +286,7 @@ static const Entry s_cryptoNative[] =
     DllImportEntry(CryptoNative_BioWrite)
     DllImportEntry(CryptoNative_EnsureLibSslInitialized)
     DllImportEntry(CryptoNative_GetOpenSslCipherSuiteName)
+    DllImportEntry(CryptoNative_SslRenegotiate)
     DllImportEntry(CryptoNative_IsSslRenegotiatePending)
     DllImportEntry(CryptoNative_IsSslStateOK)
     DllImportEntry(CryptoNative_SetCiphers)

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h
@@ -458,7 +458,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void);
     REQUIRED_FUNCTION(SSL_CTX_new) \
     LIGHTUP_FUNCTION(SSL_CTX_set_alpn_protos) \
     LIGHTUP_FUNCTION(SSL_CTX_set_alpn_select_cb) \
-    REQUIRED_FUNCTION(SSL_CTX_set_cert_verify_callback) \
     REQUIRED_FUNCTION(SSL_CTX_set_cipher_list) \
     LIGHTUP_FUNCTION(SSL_CTX_set_ciphersuites) \
     REQUIRED_FUNCTION(SSL_CTX_set_client_cert_cb) \
@@ -484,12 +483,16 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void);
     LEGACY_FUNCTION(SSL_library_init) \
     LEGACY_FUNCTION(SSL_load_error_strings) \
     REQUIRED_FUNCTION(SSL_new) \
+    REQUIRED_FUNCTION(SSL_peek) \
     REQUIRED_FUNCTION(SSL_read) \
+    REQUIRED_FUNCTION(SSL_renegotiate) \
     REQUIRED_FUNCTION(SSL_renegotiate_pending) \
     FALLBACK_FUNCTION(SSL_session_reused) \
     REQUIRED_FUNCTION(SSL_set_accept_state) \
     REQUIRED_FUNCTION(SSL_set_bio) \
     REQUIRED_FUNCTION(SSL_set_connect_state) \
+    FALLBACK_FUNCTION(SSL_set_options) \
+    REQUIRED_FUNCTION(SSL_set_verify) \
     REQUIRED_FUNCTION(SSL_shutdown) \
     LEGACY_FUNCTION(SSL_state) \
     LEGACY_FUNCTION(SSLeay) \
@@ -895,7 +898,6 @@ FOR_ALL_OPENSSL_FUNCTIONS
 #define SSL_CTX_new SSL_CTX_new_ptr
 #define SSL_CTX_set_alpn_protos SSL_CTX_set_alpn_protos_ptr
 #define SSL_CTX_set_alpn_select_cb SSL_CTX_set_alpn_select_cb_ptr
-#define SSL_CTX_set_cert_verify_callback SSL_CTX_set_cert_verify_callback_ptr
 #define SSL_CTX_set_cipher_list SSL_CTX_set_cipher_list_ptr
 #define SSL_CTX_set_ciphersuites SSL_CTX_set_ciphersuites_ptr
 #define SSL_CTX_set_client_cert_cb SSL_CTX_set_client_cert_cb_ptr
@@ -922,12 +924,18 @@ FOR_ALL_OPENSSL_FUNCTIONS
 #define SSL_library_init SSL_library_init_ptr
 #define SSL_load_error_strings SSL_load_error_strings_ptr
 #define SSL_new SSL_new_ptr
+#define SSL_peek SSL_peek_ptr
+#define SSL_state_string_long SSL_state_string_long_ptr
 #define SSL_read SSL_read_ptr
+#define ERR_print_errors_fp ERR_print_errors_fp_ptr
+#define SSL_renegotiate SSL_renegotiate_ptr
 #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr
 #define SSL_session_reused SSL_session_reused_ptr
 #define SSL_set_accept_state SSL_set_accept_state_ptr
 #define SSL_set_bio SSL_set_bio_ptr
 #define SSL_set_connect_state SSL_set_connect_state_ptr
+#define SSL_set_options SSL_set_options_ptr
+#define SSL_set_verify SSL_set_verify_ptr
 #define SSL_shutdown SSL_shutdown_ptr
 #define SSL_state SSL_state_ptr
 #define SSLeay SSLeay_ptr

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h
@@ -7,6 +7,7 @@
 #include "pal_types.h"
 
 #undef SSL_CTX_set_options
+#undef SSL_set_options
 #undef SSL_session_reused
 
 typedef struct ossl_init_settings_st OPENSSL_INIT_SETTINGS;
@@ -56,6 +57,7 @@ int SSL_CTX_config(SSL_CTX* ctx, const char* name);
 unsigned long SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options);
 void SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level);
 int32_t SSL_is_init_finished(SSL* ssl);
+unsigned long SSL_set_options(SSL* ctx, unsigned long options);
 int SSL_session_reused(SSL* ssl);
 const SSL_METHOD* TLS_method(void);
 const ASN1_TIME* X509_CRL_get0_nextUpdate(const X509_CRL* crl);

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c
@@ -368,8 +368,36 @@ int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num)
     return SSL_read(ssl, buf, num);
 }
 
+static int verify_callback(int preverify_ok, X509_STORE_CTX* store)
+{
+    (void)preverify_ok;
+    (void)store;
+    // We don't care. Real verification happens in managed code.
+    return 1;
+}
+
+int32_t CryptoNative_SslRenegotiate(SSL* ssl)
+{
+    // The openssl context is destroyed so we can't use ticket or session resumption.
+    SSL_set_options(ssl, SSL_OP_NO_TICKET | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION);
+
+    int pending = SSL_renegotiate_pending(ssl);
+    if (!pending)
+    {
+        SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback);
+        int ret = SSL_renegotiate(ssl);
+        if(ret != 1)
+            return ret;
+
+        return SSL_do_handshake(ssl);
+    }
+
+    return 0;
+}
+
 int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl)
 {
+    SSL_peek(ssl, NULL, 0);
     return SSL_renegotiate_pending(ssl) != 0;
 }
 

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h
@@ -213,6 +213,13 @@ when an error is encountered.
 */
 PALEXPORT int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num);
 
+/*
+Shims the SSL_renegotiate method.
+
+Returns 1 when renegotiation started; 0 on error.
+*/
+PALEXPORT int32_t CryptoNative_SslRenegotiate(SSL* ssl);
+
 /*
 Shims the SSL_renegotiate_pending method.
 

diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx
@@ -455,6 +455,9 @@
   <data name="net_ssl_renegotiate_data" xml:space="preserve">
     <value>Received data during renegotiation.</value>
   </data>
+  <data name="net_ssl_renegotiate_buffer" xml:space="preserve">
+    <value>Client stream needs to be drained before renegotiation.</value>
+  </data>  
   <data name="net_android_ssl_api_level_unsupported" xml:space="preserve">
     <value>Setting an SNI hostname is not supported on this API level.</value>
   </data>