From 2677a3bcd72474a04d1b189e3f9088a698d9afbb Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 May 2021 15:50:57 -0700 Subject: [PATCH 01/26] initial prototype --- .../Interop.Ssl.cs | 3 + .../entrypoints.c | 1 + .../opensslshim.h | 14 +++ .../pal_ssl.c | 75 +++++++++++++++- .../pal_ssl.h | 7 ++ .../ref/System.Net.Security.cs | 1 + .../src/System.Net.Security.csproj | 1 + .../Security/CipherSuitesPolicyPal.Linux.cs | 8 +- .../src/System/Net/Security/SecureChannel.cs | 9 +- .../Net/Security/SslStream.Implementation.cs | 85 +++++++++++++++++++ .../src/System/Net/Security/SslStream.cs | 17 ++++ .../Net/Security/SslStreamPal.Android.cs | 5 ++ .../System/Net/Security/SslStreamPal.OSX.cs | 5 ++ .../System/Net/Security/SslStreamPal.Unix.cs | 23 ++++- .../Net/Security/SslStreamPal.Windows.cs | 5 ++ 15 files changed, 251 insertions(+), 8 deletions(-) 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 057e4dc8279a42..5ce167d4bc2220 100644 --- 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 unsafe int SslRead(SafeSslHandle ssl, 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/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index bd379df88242e1..09f0ff5f0c8017 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -281,6 +281,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 index 5490d1a83666ce..38b482c2787c62 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -507,12 +507,19 @@ void SSL_get0_alpn_selected(const SSL* ssl, const unsigned char** protocol, unsi LEGACY_FUNCTION(SSL_library_init) \ LEGACY_FUNCTION(SSL_load_error_strings) \ REQUIRED_FUNCTION(SSL_new) \ + REQUIRED_FUNCTION(SSL_peek) \ + REQUIRED_FUNCTION(SSL_state_string_long) \ REQUIRED_FUNCTION(SSL_read) \ + REQUIRED_FUNCTION(SSL_get_state) \ + REQUIRED_FUNCTION(ERR_print_errors_fp) \ + REQUIRED_FUNCTION(SSL_in_init) \ + 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) \ + REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ LEGACY_FUNCTION(SSLeay) \ @@ -925,12 +932,19 @@ 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 SSL_get_state SSL_get_state_ptr +#define ERR_print_errors_fp ERR_print_errors_fp_ptr +#define SSL_in_init SSL_in_init_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_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/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index c0dbf05a4154b9..dde47a583e452f 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -19,6 +19,7 @@ c_static_assert(PAL_SSL_ERROR_WANT_WRITE == SSL_ERROR_WANT_WRITE); c_static_assert(PAL_SSL_ERROR_SYSCALL == SSL_ERROR_SYSCALL); c_static_assert(PAL_SSL_ERROR_ZERO_RETURN == SSL_ERROR_ZERO_RETURN); +/* #define DOTNET_DEFAULT_CIPHERSTRING \ "ECDHE-ECDSA-AES256-GCM-SHA384:" \ "ECDHE-ECDSA-AES128-GCM-SHA256:" \ @@ -28,6 +29,9 @@ c_static_assert(PAL_SSL_ERROR_ZERO_RETURN == SSL_ERROR_ZERO_RETURN); "ECDHE-ECDSA-AES128-SHA256:" \ "ECDHE-RSA-AES256-SHA384:" \ "ECDHE-RSA-AES128-SHA256:" \ + "DHE-RSA-AES256-GCM-SHA384:ALL" \ + +*/ int32_t CryptoNative_EnsureOpenSslInitialized(void); @@ -39,7 +43,7 @@ static void EnsureLibSsl10Initialized() } #endif -static int32_t g_config_specified_ciphersuites = 0; +static int32_t g_config_specified_ciphersuites = 1; static void DetectCiphersuiteConfiguration() { @@ -165,12 +169,14 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) // If openssl.cnf doesn't have an opinion for CipherString, then use this value instead if (!g_config_specified_ciphersuites) { - if (!SSL_CTX_set_cipher_list(ctx, DOTNET_DEFAULT_CIPHERSTRING)) + //if (!SSL_CTX_set_cipher_list(ctx, DOTNET_DEFAULT_CIPHERSTRING)) + if (!SSL_CTX_set_cipher_list(ctx, "ALL")) { SSL_CTX_free(ctx); return NULL; } } + SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF); } return ctx; @@ -368,6 +374,71 @@ 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. + printf("%s:%d: called!!!\n", __func__, __LINE__); + return 1; +} + +int32_t CryptoNative_SslRenegotiate(SSL* ssl) +{ + int mode = SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE; + + int pending = SSL_renegotiate_pending(ssl); + printf("%s:%d: +++++++++++++++++++++++++++++++++++++++++++++++++\n", __func__, __LINE__); + printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, pending, (void*)ssl); + // static int pending = 0; + int ret=0; +printf("%s:%d: version=%lu 0x%lx pending %d state %d in_init %d\n", __func__, __LINE__, OpenSSL_version_num(), OpenSSL_version_num(), pending , SSL_get_state(ssl), SSL_in_init(ssl)); + if (!pending) + { + + SSL_set_verify(ssl, mode, verify_callback); + printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); + ret = SSL_renegotiate(ssl); + //SSL_set_accept_state(ssl); + int ret1 = SSL_do_handshake(ssl); + printf("%s:%d: set verify in init %s state ret = %d err- %d\n", __func__, __LINE__, SSL_state_string_long(ssl), ret1, SSL_get_error(ssl, ret1)); +// SSL_set_accept_state(ssl); + // SSL_set_state(ssl, SSL_ST_ACCEPT); + //SSL_set_accept_state +// ssl->state = SSL_ST_ACCEPT; +// ret = SSL_do_handshake(ssl); + printf("%s:%d: set verify in init %s state erorr %d \n", __func__, __LINE__, SSL_state_string_long(ssl), SSL_get_error(ssl, ret)); + printf("%s:%d: started renego %d %d\n", __func__, __LINE__, ret, ret1); + pending = 1; + + return 1; + } + + +// SSL_set_verify(ssl, mode, verify_callback); + +// int32_t ret = SSL_renegotiate(ssl); + if (ret == 1 || pending) + { +// printf("%s:%d: SSL_renegotiate ok. moving on\n", __func__, __LINE__); +// SSL_set_state(ssl, SSL_ST_ACCEPT); +// ret = SSL_do_handshake(ssl); + // attempt to trigger +// SSL_set_state(ssl, SSL_ST_ACCEPT); +// ret = SSL_do_handshake(ssl); +// printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); + ret = SSL_peek(ssl, &mode, 0); +// SSL_write(ssl, &mode, 1); +//ERR_print_errors_fp(stdout); +//ERR_clear_error(); + +// printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); +// printf("%s:%d: set state, handshake amd peek aall done %d err=%d\n", __func__, __LINE__, ret, SSL_get_error(ssl,ret)); + } + + return ret; +} + int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { 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 index d7b995c5a19e9b..79c6cbe22f955b 100644 --- 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/ref/System.Net.Security.cs b/src/libraries/System.Net.Security/ref/System.Net.Security.cs index ab478a68272a8f..031fa08a1d7cf2 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -231,6 +231,7 @@ public override void EndWrite(System.IAsyncResult asyncResult) { } ~SslStream() { } public override void Flush() { } public override System.Threading.Tasks.Task FlushAsync(System.Threading.CancellationToken cancellationToken) { throw null; } + public virtual System.Threading.Tasks.Task GetRemoteCertificate() { throw null; } public override int Read(byte[] buffer, int offset, int count) { throw null; } public override System.Threading.Tasks.Task ReadAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.ValueTask ReadAsync(System.Memory buffer, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index c60878d25835e1..26d28746de4a38 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -399,6 +399,7 @@ + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs index 0cbeb85dbd331b..abcabd70241b29 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs @@ -28,10 +28,10 @@ internal class CipherSuitesPolicyPal internal CipherSuitesPolicyPal(IEnumerable allowedCipherSuites) { - if (!Interop.Ssl.Tls13Supported) - { - throw new PlatformNotSupportedException(SR.net_ssl_ciphersuites_policy_not_supported); - } +// if (!Interop.Ssl.Tls13Supported) +// { +// throw new PlatformNotSupportedException(SR.net_ssl_ciphersuites_policy_not_supported); +// } using (SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method)) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 8c54dbbd719217..5b0dbe74d3be54 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -21,7 +21,7 @@ internal class SecureChannel internal const int ReadHeaderSize = 5; private SafeFreeCredentials? _credentialsHandle; - private SafeDeleteSslContext? _securityContext; + internal SafeDeleteSslContext? _securityContext; private SslConnectionInfo? _connectionInfo; private X509Certificate? _selectedClientCertificate; @@ -935,9 +935,11 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot X509Chain? chain = null; X509Certificate2Collection? remoteCertificateStore = null; + try { X509Certificate2? certificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, out remoteCertificateStore); +Console.WriteLine("VerifyRemoteCertificate called, remote is {0}", certificate); if (_remoteCertificate != null && certificate != null && certificate.RawData.AsSpan().SequenceEqual(_remoteCertificate.RawData)) @@ -1105,6 +1107,11 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot return GenerateAlertToken(); } + public int Renegotiate() + { + return SslStreamPal.Renegotiate(_securityContext!); + } + private ProtocolToken GenerateAlertToken() { byte[]? nextmsg = null; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 0f7b5073c5a046..eed3ae546569ab 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -303,6 +303,85 @@ private async Task ReplyOnReAuthenticationAsync(TIOAdapter adapter, } } + private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false, bool renego = false) + where TIOAdapter : IReadWriteAdapter + { + int foo = _context!.Renegotiate(); + + if (foo < 0 ){ + + await ForceAuthenticationAsync(adapter, false, null, isApm).ConfigureAwait(false); + + Console.WriteLine("Renegotiate: short doen!"); + } + +// ProtocolToken message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Renegotiate: Next message?????? ---------------------------------------------"); + ProtocolToken message = _context!.NextMessage(reAuthenticationData); + Console.WriteLine("Writig {0} {1}", message.Size, message.Status); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + + Console.WriteLine("REad done with {0}", "len"); +// await ForceAuthenticationAsync(adapter, false, null).ConfigureAwait(false); +// Console.WriteLine("ForceAuthenticationAsync????? done??"); + + + //Interop.OpenSsl.SslRenegotiate(_context._securityContext); + + + // Console.WriteLine("Renegotiate ?????"); + // _context!.Renegotiate(); + +do { + + // message = _context!.NextMessage(reAuthenticationData); + Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); + message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + else + { + Console.WriteLine("Renegotiate WTF???? --------------------------------"); + _context!.Renegotiate(); + Console.WriteLine("Renegotiate WTF!!! --------------------------------"); + message = _context!.NextMessage(reAuthenticationData); + Console.WriteLine("Writng3 {0} ", message.Size); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + + + } + if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) + { + Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); + break; + } +} while (true); + var remoteCert = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); + +// ProtocolToken? alertToken = null; +// if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) +// { +//Console.WriteLine(" CompleteHandshakefailed!"); +// } + Console.WriteLine("Renegotiate is NONE {0}", remoteCert); +// //await ReceiveBlobAsync(adapter, true); +// byte[] buffer = new byte[32000]; +// int len = await ReadAsyncInternal(adapter, buffer, true).ConfigureAwait(false); +// Console.WriteLine("REad done with {0}", len); + } + // reAuthenticationData is only used on Windows in case of renegotiation. private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false) where TIOAdapter : IReadWriteAdapter @@ -327,6 +406,7 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool message = _context!.NextMessage(reAuthenticationData); if (message.Size > 0) { + Console.WriteLine("Senfing {0}", message.Size); await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); if (NetEventSource.Log.IsEnabled()) @@ -353,12 +433,15 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool while (!handshakeCompleted) { + Console.WriteLine("Going to receive!"); message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Senfing2 {0} error={1}", message.Size, message.Status.ErrorCode); byte[]? payload = null; int size = 0; if (message.Size > 0) { + Console.WriteLine("Senfing2 {0}", message.Size); payload = message.Payload; size = message.Size; } @@ -488,6 +571,8 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a await FillHandshakeBufferAsync(adapter, frameSize).ConfigureAwait(false); } + Console.WriteLine("REceive blob done. frame size {0}, got {1}", frameSize, _handshakeBuffer.ActiveLength); + // At this point, we have at least one TLS frame. if (_lastFrame.Header.Type == TlsContentType.Alert) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 0acf683d85830a..8bffc855c59abd 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -531,6 +531,10 @@ public virtual X509Certificate? LocalCertificate get { ThrowIfExceptionalOrNotAuthenticated(); + if (_nestedAuth == 0) + { + var crt = GetRemoteCertificate().GetAwaiter().GetResult(); + } return _context!.IsServer ? _context.LocalServerCertificate : _context.LocalClientCertificate; } } @@ -820,6 +824,19 @@ public override ValueTask ReadAsync(Memory buffer, CancellationToken return ReadAsyncInternal(new AsyncReadWriteAdapter(InnerStream, cancellationToken), buffer); } + public virtual async Task GetRemoteCertificate() + { + ThrowIfExceptionalOrNotAuthenticated(); + + Console.WriteLine("Calling GetRemoteCertificate ???"); + _sslAuthenticationOptions!.RemoteCertRequired = true; + //await ProcessAuthentication(true)!.ConfigureAwait(false); + // await ForceAuthenticationAsync(new AsyncReadWriteAdapter(InnerStream, CancellationToken.None), false, null, false, renego: true).ConfigureAwait(false); + await Renegotiate(new AsyncReadWriteAdapter(InnerStream, CancellationToken.None), false, null, false, renego: true).ConfigureAwait(false); + Console.WriteLine("Calling GetRemoteCertificate Finished!!!"); + return RemoteCertificate; + } + private void ThrowIfExceptional() { ExceptionDispatchInfo? e = _exception; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 6245f00ae4570a..77ef25606e86fd 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -175,5 +175,10 @@ public static SecurityStatusPal ApplyShutdownToken( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); } + + public static int Renegotiate(SafeDeleteContext securityContext) + { + throw new PlatformNotSupportedException(); + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 5f1c1bb346676e..6fab3b2b5dd7ec 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -218,6 +218,11 @@ public static void QueryContextConnectionInfo( connectionInfo = new SslConnectionInfo(((SafeDeleteSslContext)securityContext).SslContext); } + public static int Renegotiate(SafeDeleteContext securityContext) + { + throw new PlatformNotSupportedException(); + } + private static SecurityStatusPal HandshakeInternal( SafeFreeCredentials credential, ref SafeDeleteSslContext? context, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index aeae1edc9fe946..0743ecf9144f72 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -81,6 +81,17 @@ public static SecurityStatusPal DecryptMessage(SafeDeleteContext securityContext return bindingHandle; } + public static int Renegotiate(SafeDeleteContext securityContext) + { +// SafeDeleteSslContext sslContext = ((SafeDeleteSslContext)securityContext); + + Console.WriteLine("RENEGO ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext).SslContext)); + + int ret = Interop.Ssl.SslRenegotiate(((SafeDeleteSslContext)securityContext).SslContext); + Console.WriteLine("RENEGO {1} ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext).SslContext), ret); + return 1; + } + public static void QueryContextStreamSizes(SafeDeleteContext? securityContext, out StreamSizes streamSizes) { streamSizes = StreamSizes.Default; @@ -104,6 +115,8 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia byte[]? output = null; int outputSize = 0; + Console.WriteLine("HandshakeInternal: giving {0} bytes", inputBuffer.Length); + try { if ((null == context) || context.IsInvalid) @@ -111,7 +124,15 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia context = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); } +Console.WriteLine("RENEGO ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)); bool done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, inputBuffer, out output, out outputSize); + if (outputSize == 0 && Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)) + { + Console.WriteLine("WTF fif got nothe! {0} trying again", done); + done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, ReadOnlySpan.Empty, out output, out outputSize); + Console.WriteLine("WTF {0} {1}", done, outputSize); + } +Console.WriteLine("RENEGO ? {0} done ? {1}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext), done); // When the handshake is done, and the context is server, check if the alpnHandle target was set to null during ALPN. // If it was, then that indicates ALPN failed, send failure. @@ -128,7 +149,7 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia outputSize == output!.Length ? output : new Span(output, 0, outputSize).ToArray(); - return new SecurityStatusPal(done ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.ContinueNeeded); + return new SecurityStatusPal(done && !Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext) ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.ContinueNeeded); } catch (Exception exc) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index b93e89957bc761..4a7f7df1bf39f4 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -422,6 +422,11 @@ public static void QueryContextConnectionInfo(SafeDeleteContext securityContext, connectionInfo = new SslConnectionInfo(interopConnectionInfo, cipherSuite); } + public static int Renegotiate(SafeDeleteContext securityContext) + { + throw new PlatformNotSupportedException(); + } + private static int GetProtocolFlagsFromSslProtocols(SslProtocols protocols, bool isServer) { int protocolFlags = (int)protocols; From 38007a63f080f57722798404b691f5bcc033810f Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Fri, 18 Jun 2021 08:24:12 +0000 Subject: [PATCH 02/26] Restore TLS 1.2 renegotiation --- .../Interop.OpenSsl.cs | 2 + .../Interop.Ssl.cs | 3 + .../entrypoints.c | 1 + .../opensslshim.h | 4 + .../pal_ssl.c | 98 ++++++++------- .../pal_ssl.h | 10 ++ .../src/System/Net/Security/SecureChannel.cs | 6 + .../Net/Security/SslStream.Implementation.cs | 118 +++++++++++------- .../src/System/Net/Security/SslStream.cs | 13 -- .../Net/Security/SslStreamPal.Android.cs | 7 +- .../System/Net/Security/SslStreamPal.OSX.cs | 7 +- .../System/Net/Security/SslStreamPal.Unix.cs | 11 +- .../Net/Security/SslStreamPal.Windows.cs | 7 +- .../SslStreamNegotiatedCipherSuiteTest.cs | 13 +- .../SslStreamNetworkStreamTest.cs | 48 ++++--- .../tests/FunctionalTests/TestHelper.cs | 17 ++- 16 files changed, 236 insertions(+), 129 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 index 71ee76ad2a6d0a..dde5d9691481d9 100644 --- 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 @@ -241,7 +241,9 @@ internal static bool DoSslHandshake(SafeSslHandle context, ReadOnlySpan in } } + Console.WriteLine("Going to do handshake " + Ssl.IsSslRenegotiatePending(context)); int retVal = Ssl.SslDoHandshake(context); + Console.WriteLine("After handshake " + Ssl.IsSslRenegotiatePending(context)); if (retVal != 1) { Exception? innerError; 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 5ce167d4bc2220..c37adb45670f7a 100644 --- 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 unsafe int SslRead(SafeSslHandle ssl, byte* buf, int num); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslPeek")] + internal static extern int SslPeek(SafeSslHandle ssl); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRenegotiate")] internal static extern int SslRenegotiate(SafeSslHandle ssl); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index cd492502dce441..6f96744895323c 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -285,6 +285,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BioWrite) DllImportEntry(CryptoNative_EnsureLibSslInitialized) DllImportEntry(CryptoNative_GetOpenSslCipherSuiteName) + DllImportEntry(CryptoNative_SslPeek) DllImportEntry(CryptoNative_SslRenegotiate) DllImportEntry(CryptoNative_IsSslRenegotiatePending) DllImportEntry(CryptoNative_IsSslStateOK) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index ea8fc8e641f114..9551a38a187527 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -464,6 +464,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ + REQUIRED_FUNCTION(SSL_CTX_set_keylog_callback) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ @@ -491,6 +492,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_in_init) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ + REQUIRED_FUNCTION(SSL_set_options) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ @@ -901,6 +903,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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_keylog_callback SSL_CTX_set_keylog_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 @@ -935,6 +938,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_in_init SSL_in_init_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr +#define SSL_set_options SSL_set_options_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 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 index d84869ec4af38e..b327b14f7b576a 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -143,6 +143,15 @@ void CryptoNative_EnsureLibSslInitialized() EnsureLibSsl10Initialized(); #endif + printf("%s:%d: ++++++++++++++++++++++Init keylog\n", __func__, __LINE__); + fp = fopen("/tmp/keylog.txt", "a+"); + if(setvbuf(fp, NULL, _IONBF, 0)) + { + fclose(fp); + return; + } + printf("%s:%d: ++++++++++++++++++++++Initialised\n", __func__, __LINE__); + DetectCiphersuiteConfiguration(); } @@ -153,6 +162,21 @@ const SSL_METHOD* CryptoNative_SslV2_3Method() return method; } +void SSL_CTX_keylog_cb_func_cb(const SSL *ssl, const char *line); +void SSL_CTX_keylog_cb_func_cb(const SSL *ssl, const char *line) +{ + + if (!line || !fp) { + return; + } + + printf("%s:%d: SSL_CTX_keylog_cb_func_cb\n", __func__, __LINE__); + (void)ssl; + + fprintf(fp, "%s\n" ,line); + fflush(fp); +} + SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) { SSL_CTX* ctx = SSL_CTX_new(method); @@ -176,7 +200,7 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) return NULL; } } - SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_OFF); + SSL_CTX_set_keylog_callback(ctx, SSL_CTX_keylog_cb_func_cb); } return ctx; @@ -296,7 +320,8 @@ void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) SSL* CryptoNative_SslCreate(SSL_CTX* ctx) { - return SSL_new(ctx); + SSL* ssl = SSL_new(ctx); + return ssl; } int32_t CryptoNative_SslGetError(SSL* ssl, int32_t ret) @@ -332,11 +357,13 @@ void CryptoNative_SslCtxDestroy(SSL_CTX* ctx) void CryptoNative_SslSetConnectState(SSL* ssl) { + printf("%s:%d: connset state\n", __func__, __LINE__); SSL_set_connect_state(ssl); } void CryptoNative_SslSetAcceptState(SSL* ssl) { + printf("%s:%d: accept state\n", __func__, __LINE__); SSL_set_accept_state(ssl); } @@ -374,66 +401,44 @@ 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) +// 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. +// printf("%s:%d: called!!!\n", __func__, __LINE__); +// return 1; +// } + +int32_t CryptoNative_SslPeek(SSL* ssl) { - (void)preverify_ok; - (void)store; - // We don't care. Real verification happens in managed code. - printf("%s:%d: called!!!\n", __func__, __LINE__); - return 1; + return SSL_peek(ssl, NULL, 0); } + int32_t CryptoNative_SslRenegotiate(SSL* ssl) { - int mode = SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE; + // pokusit se odmazat + // int mode = SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE; + + SSL_set_options(ssl, SSL_OP_NO_TICKET | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION); int pending = SSL_renegotiate_pending(ssl); printf("%s:%d: +++++++++++++++++++++++++++++++++++++++++++++++++\n", __func__, __LINE__); printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, pending, (void*)ssl); - // static int pending = 0; + int ret=0; printf("%s:%d: version=%lu 0x%lx pending %d state %d in_init %d\n", __func__, __LINE__, OpenSSL_version_num(), OpenSSL_version_num(), pending , SSL_get_state(ssl), SSL_in_init(ssl)); if (!pending) { - SSL_set_verify(ssl, mode, verify_callback); + // SSL_set_verify(ssl, mode, verify_callback); printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); ret = SSL_renegotiate(ssl); - //SSL_set_accept_state(ssl); - int ret1 = SSL_do_handshake(ssl); - printf("%s:%d: set verify in init %s state ret = %d err- %d\n", __func__, __LINE__, SSL_state_string_long(ssl), ret1, SSL_get_error(ssl, ret1)); -// SSL_set_accept_state(ssl); - // SSL_set_state(ssl, SSL_ST_ACCEPT); - //SSL_set_accept_state -// ssl->state = SSL_ST_ACCEPT; -// ret = SSL_do_handshake(ssl); - printf("%s:%d: set verify in init %s state erorr %d \n", __func__, __LINE__, SSL_state_string_long(ssl), SSL_get_error(ssl, ret)); - printf("%s:%d: started renego %d %d\n", __func__, __LINE__, ret, ret1); - pending = 1; - - return 1; - } - - -// SSL_set_verify(ssl, mode, verify_callback); - -// int32_t ret = SSL_renegotiate(ssl); - if (ret == 1 || pending) - { -// printf("%s:%d: SSL_renegotiate ok. moving on\n", __func__, __LINE__); -// SSL_set_state(ssl, SSL_ST_ACCEPT); -// ret = SSL_do_handshake(ssl); - // attempt to trigger -// SSL_set_state(ssl, SSL_ST_ACCEPT); -// ret = SSL_do_handshake(ssl); -// printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); - ret = SSL_peek(ssl, &mode, 0); -// SSL_write(ssl, &mode, 1); -//ERR_print_errors_fp(stdout); -//ERR_clear_error(); + if(ret!=1) + return ret; -// printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); -// printf("%s:%d: set state, handshake amd peek aall done %d err=%d\n", __func__, __LINE__, ret, SSL_get_error(ssl,ret)); + return SSL_do_handshake(ssl); } return ret; @@ -441,6 +446,9 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl) int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { + printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, SSL_renegotiate_pending(ssl), (void*)ssl); + SSL_peek(ssl, NULL, 0); + printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, SSL_renegotiate_pending(ssl), (void*)ssl); 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 index 79c6cbe22f955b..5604ee473d0b12 100644 --- 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,14 @@ when an error is encountered. */ PALEXPORT int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num); +/* +Shims the SSL_peak method. + +Returns 1 when renegotiation started; 0 on error. +*/ +PALEXPORT int32_t CryptoNative_SslPeek(SSL* ssl); + + /* Shims the SSL_renegotiate method. @@ -392,3 +400,5 @@ PALEXPORT const char* CryptoNative_GetOpenSslCipherSuiteName(SSL* ssl, int32_t c Checks if given protocol version is supported. */ PALEXPORT int32_t CryptoNative_OpenSslGetProtocolSupport(SslProtocols protocol); + +static FILE * fp = NULL; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index a2b0852b4697fe..935a1616e4327a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -1121,6 +1121,12 @@ public SecurityStatusPal Renegotiate() return SslStreamPal.Renegotiate(_securityContext!); } + + public SecurityStatusPal Peek() + { + return SslStreamPal.Peek(ref _securityContext!); + } + private ProtocolToken GenerateAlertToken() { byte[]? nextmsg = null; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index b7b4679e42617b..f843e5d25d507a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -305,13 +305,13 @@ private async Task ReplyOnReAuthenticationAsync(TIOAdapter adapter, } } -/* - private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false, bool renego = false) + + private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false, bool renego = false) where TIOAdapter : IReadWriteAdapter { - int foo = _context!.Renegotiate(); + SecurityStatusPal foo = _context!.Renegotiate(); - if (foo < 0 ){ + if (foo.ErrorCode != SecurityStatusPalErrorCode.OK ){ await ForceAuthenticationAsync(adapter, false, null, isApm).ConfigureAwait(false); @@ -320,6 +320,8 @@ private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst // ProtocolToken message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); Console.WriteLine("Renegotiate: Next message?????? ---------------------------------------------"); + + // Ask for "Hello Request" message ProtocolToken message = _context!.NextMessage(reAuthenticationData); Console.WriteLine("Writig {0} {1}", message.Size, message.Status); if (message.Size > 0) @@ -338,57 +340,44 @@ private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst // Console.WriteLine("Renegotiate ?????"); // _context!.Renegotiate(); + _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); + do { -do { - - // message = _context!.NextMessage(reAuthenticationData); - Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); - message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - else - { - Console.WriteLine("Renegotiate WTF???? --------------------------------"); - _context!.Renegotiate(); - Console.WriteLine("Renegotiate WTF!!! --------------------------------"); - message = _context!.NextMessage(reAuthenticationData); - Console.WriteLine("Writng3 {0} ", message.Size); + Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); + message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); if (message.Size > 0) { await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } + if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) + { + Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); + } + } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); + var crt = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); - } - if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) - { - Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); - break; - } -} while (true); - var remoteCert = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); + ProtocolToken? alertToken = null; + if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + { +Console.WriteLine(" CompleteHandshakefailed!"); + } + + Console.WriteLine("Renegotiate is NONE {0}", crt); -// ProtocolToken? alertToken = null; -// if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) -// { -//Console.WriteLine(" CompleteHandshakefailed!"); -// } - Console.WriteLine("Renegotiate is NONE {0}", remoteCert); -// //await ReceiveBlobAsync(adapter, true); +// await ReceiveBlobAsync(adapter, true); // byte[] buffer = new byte[32000]; // int len = await ReadAsyncInternal(adapter, buffer, true).ConfigureAwait(false); // Console.WriteLine("REad done with {0}", len); -*/ +} // This will initiate renegotiation or PHA for Tls1.3 private async Task RenegotiateAsync(CancellationToken cancellationToken) { + if (Interlocked.Exchange(ref _nestedAuth, 1) == 1) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidnestedcall, "NegotiateClientCertificateAsync", "renegotiate")); @@ -406,14 +395,18 @@ private async Task RenegotiateAsync(CancellationToken cancellationToken) } _sslAuthenticationOptions!.RemoteCertRequired = true; - IReadWriteAdapter adapter = new AsyncReadWriteAdapter(InnerStream, cancellationToken); + byte[]? reAuthenticationData = null; try { - SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg); - if (nextmsg?.Length > 0) + await Renegotiate(new AsyncReadWriteAdapter(InnerStream, cancellationToken), false, reAuthenticationData, false, renego: true).ConfigureAwait(false); + /* + SecurityStatusPal status = _context!.Renegotiate(); + + ProtocolToken message = _context!.NextMessage(reAuthenticationData); + if (message.Size > 0) { - await adapter.WriteAsync(nextmsg, 0, nextmsg.Length).ConfigureAwait(false); + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } @@ -428,8 +421,43 @@ private async Task RenegotiateAsync(CancellationToken cancellationToken) throw SslStreamPal.GetException(status); } + // If Windows: // Issue empty read to get renegotiation going. - await ReadAsyncInternal(adapter, Memory.Empty, renegotiation: true).ConfigureAwait(false); + //await ReadAsyncInternal(adapter, Memory.Empty, renegotiation: true).ConfigureAwait(false); + + do { + + // message = _context!.NextMessage(reAuthenticationData); + Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); + message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + else + { + Console.WriteLine("Renegotiate WTF???? --------------------------------"); + _context!.Renegotiate(); + Console.WriteLine("Renegotiate WTF!!! --------------------------------"); + message = _context!.NextMessage(reAuthenticationData); + Console.WriteLine("Writng3 {0} ", message.Size); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + + + } + if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) + { + Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); + break; + } + } while (true); + */ } finally { @@ -498,7 +526,7 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool int size = 0; if (message.Size > 0) { - Console.WriteLine("Senfing2 {0}", message.Size); + Console.WriteLine("Senfing2 {0}", message.Size); payload = message.Payload; size = message.Size; } @@ -640,6 +668,8 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a } else if (_lastFrame.Header.Type == TlsContentType.Handshake) { + // During renegotiation the client hello is encrypted. Possibly race condition when the encrypted client hello flag is 0x01 but the content of the message is encrypted. + // Assume the Client Hello is not encrypted. It doesn't apply for renegotiation. if (_handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && (_sslAuthenticationOptions!.ServerCertSelectionDelegate != null || _sslAuthenticationOptions!.ServerOptionDelegate != null)) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index b8b96912e3b30a..55c79ebd3077e9 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -831,19 +831,6 @@ public override ValueTask ReadAsync(Memory buffer, CancellationToken return ReadAsyncInternal(new AsyncReadWriteAdapter(InnerStream, cancellationToken), buffer); } - // public virtual async Task GetRemoteCertificate() - // { - // ThrowIfExceptionalOrNotAuthenticated(); - - // Console.WriteLine("Calling GetRemoteCertificate ???"); - // _sslAuthenticationOptions!.RemoteCertRequired = true; - // //await ProcessAuthentication(true)!.ConfigureAwait(false); - // // await ForceAuthenticationAsync(new AsyncReadWriteAdapter(InnerStream, CancellationToken.None), false, null, false, renego: true).ConfigureAwait(false); - // await Renegotiate(new AsyncReadWriteAdapter(InnerStream, CancellationToken.None), false, null, false, renego: true).ConfigureAwait(false); - // Console.WriteLine("Calling GetRemoteCertificate Finished!!!"); - // return RemoteCertificate; - // } - private void ThrowIfExceptional() { ExceptionDispatchInfo? e = _exception; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 3d9693223970be..8ac203af6087c7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -242,7 +242,12 @@ public static SecurityStatusPal ApplyShutdownToken( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); } - public static int Renegotiate(SafeDeleteContext securityContext) + public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) + { + throw new PlatformNotSupportedException(); + } + + public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) { throw new PlatformNotSupportedException(); } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index ad8c77848aeff3..c454aaca47d69d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -221,7 +221,12 @@ public static void QueryContextConnectionInfo( connectionInfo = new SslConnectionInfo(securityContext.SslContext); } - public static int Renegotiate(SafeDeleteContext securityContext) + public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) + { + throw new PlatformNotSupportedException(); + } + + public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) { throw new PlatformNotSupportedException(); } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 467b84ad1a3bbb..68482683677114 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -81,6 +81,13 @@ public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityCont return bindingHandle; } + public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) + { + int ret = Interop.Ssl.SslPeek(((SafeDeleteSslContext)securityContext!).SslContext); + return new SecurityStatusPal(ret == 0 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); + } + + public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? securityContext, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer) { // SafeDeleteSslContext sslContext = ((SafeDeleteSslContext)securityContext); @@ -103,7 +110,7 @@ public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) int ret = Interop.Ssl.SslRenegotiate(((SafeDeleteSslContext)securityContext).SslContext); Console.WriteLine("RENEGO {1} ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext).SslContext), ret); - return new SecurityStatusPal(ret == 0 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); + return new SecurityStatusPal(ret == 1 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); } public static void QueryContextStreamSizes(SafeDeleteContext? securityContext, out StreamSizes streamSizes) @@ -165,7 +172,7 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia outputSize == output!.Length ? output : new Span(output, 0, outputSize).ToArray(); - return new SecurityStatusPal(done && !Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext) ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.ContinueNeeded); + return new SecurityStatusPal(done ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.ContinueNeeded); } catch (Exception exc) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index ed42e050801588..e0c61f76da56c7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -430,7 +430,12 @@ public static void QueryContextConnectionInfo(SafeDeleteContext securityContext, connectionInfo = new SslConnectionInfo(interopConnectionInfo, cipherSuite); } - public static int Renegotiate(SafeDeleteContext securityContext) + public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) + { + throw new PlatformNotSupportedException(); + } + + public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) { throw new PlatformNotSupportedException(); } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs index 5b1e1c7bd146ca..2280677a587c7f 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs @@ -575,10 +575,15 @@ private static IReadOnlyList GetSupportedNonTls13CipherSuites() if (!CipherSuitesPolicySupported) return null; - var ret = new List(); - AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); - - return ret; + // var ret = new Lazy>(()=>{ + // var ret = new List(); + // AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); + // return ret; + // }); + + + + return new List(); } private static bool RequiredByTls13Spec(TlsCipherSuite cs) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 68a2ea2a997878..f884d194a3aa8f 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -174,7 +174,7 @@ public async Task SslStream_NetworkStream_Renegotiation_Succeeds(bool useSync) [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] [InlineData(true)] - [InlineData(false)] + // [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendClientCertificate) { @@ -183,11 +183,12 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl cts.CancelAfter(TestConfiguration.PassingTestTimeout); (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); - using (client) + //using (client) using (server) - using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) - using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) { + using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); + using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = Guid.NewGuid().ToString("N"), @@ -204,7 +205,8 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl { if (negotiateClientCertificateCalled && sendClientCertificate) { - Assert.Equal(clientCertificate.GetCertHash(), certificate?.GetCertHash()); + if (TestHelper.AllowClient) + Assert.Equal(clientCertificate.GetCertHash(), certificate?.GetCertHash()); } else { @@ -214,15 +216,23 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl return true; }; - await TestConfiguration.WhenAllOrAnyFailedWithTimeout( - client.AuthenticateAsClientAsync(clientOptions, cts.Token), - server.AuthenticateAsServerAsync(serverOptions, cts.Token)); + if (TestHelper.AllowClient) + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions, cts.Token), + server.AuthenticateAsServerAsync(serverOptions, cts.Token)); + else + await server.AuthenticateAsServerAsync(serverOptions, cts.Token); Assert.Null(server.RemoteCertificate); - // Client needs to be reading for renegotiation to happen. - byte[] buffer = new byte[TestHelper.s_ping.Length]; - ValueTask t = client.ReadAsync(buffer, cts.Token); + + ValueTask t = new (); + if (TestHelper.AllowClient) + { + // Client needs to be reading for renegotiation to happen. + byte[] buffer = new byte[TestHelper.s_ping.Length]; + t = client.ReadAsync(buffer, cts.Token); + } negotiateClientCertificateCalled = true; await server.NegotiateClientCertificateAsync(cts.Token); @@ -234,12 +244,22 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( { Assert.Null(server.RemoteCertificate); } + + + // Finish the client's read await server.WriteAsync(TestHelper.s_ping, cts.Token); - await t; + if (TestHelper.AllowClient) + await t; + + // verify that the session is usable with or without client's certificate - await TestHelper.PingPong(client, server, cts.Token); - await TestHelper.PingPong(server, client, cts.Token); + if (TestHelper.AllowClient) + { + await TestHelper.PingPong(client, server, cts.Token); + await TestHelper.PingPong(server, client, cts.Token); + } + } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 998e93813ec1e8..181e9b032cbe0e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -19,6 +19,9 @@ namespace System.Net.Security.Tests { public static class TestHelper { + + public static bool AllowClient = true; + private static readonly X509KeyUsageExtension s_eeKeyUsage = new X509KeyUsageExtension( X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.KeyEncipherment | X509KeyUsageFlags.DataEncipherment, @@ -49,7 +52,9 @@ public static class TestHelper public static (SslStream ClientStream, SslStream ServerStream) GetConnectedSslStreams() { (Stream clientStream, Stream serverStream) = GetConnectedStreams(); - return (new SslStream(clientStream), new SslStream(serverStream)); + return ( + AllowClient ? new SslStream(clientStream) : null, + new SslStream(serverStream)); } public static (Stream ClientStream, Stream ServerStream) GetConnectedStreams() @@ -67,17 +72,21 @@ internal static (NetworkStream ClientStream, NetworkStream ServerStream) GetConn { using (Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { - listener.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + listener.Bind(new IPEndPoint(IPAddress.Loopback, 7001)); listener.Listen(1); var clientSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - clientSocket.Connect(listener.LocalEndPoint); + if(AllowClient) + clientSocket.Connect(listener.LocalEndPoint); Socket serverSocket = listener.Accept(); serverSocket.NoDelay = true; clientSocket.NoDelay = true; - return (new NetworkStream(clientSocket, ownsSocket: true), new NetworkStream(serverSocket, ownsSocket: true)); + return ( + AllowClient ? new NetworkStream(clientSocket, ownsSocket: true) : + null + , new NetworkStream(serverSocket, ownsSocket: true)); } } From 28dc58cd2fe19c45637d9f1c25f061d4e3991926 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Tue, 22 Jun 2021 15:21:52 +0200 Subject: [PATCH 03/26] First windows functionality merge --- .../Net/Security/SslStream.Implementation.cs | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index f843e5d25d507a..b083967a74f852 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -399,14 +399,18 @@ private async Task RenegotiateAsync(CancellationToken cancellationToken) byte[]? reAuthenticationData = null; try { - await Renegotiate(new AsyncReadWriteAdapter(InnerStream, cancellationToken), false, reAuthenticationData, false, renego: true).ConfigureAwait(false); - /* - SecurityStatusPal status = _context!.Renegotiate(); + var adapter = new AsyncReadWriteAdapter(InnerStream, cancellationToken); - ProtocolToken message = _context!.NextMessage(reAuthenticationData); - if (message.Size > 0) + SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg); + ProtocolToken message; + if (nextmsg!.Length == 0){ + message = _context!.NextMessage(reAuthenticationData); + nextmsg = message.Payload; + } + + if (nextmsg!.Length > 0) { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } @@ -425,39 +429,33 @@ private async Task RenegotiateAsync(CancellationToken cancellationToken) // Issue empty read to get renegotiation going. //await ReadAsyncInternal(adapter, Memory.Empty, renegotiation: true).ConfigureAwait(false); + + _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); do { - // message = _context!.NextMessage(reAuthenticationData); - Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); - message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - else - { - Console.WriteLine("Renegotiate WTF???? --------------------------------"); - _context!.Renegotiate(); - Console.WriteLine("Renegotiate WTF!!! --------------------------------"); - message = _context!.NextMessage(reAuthenticationData); - Console.WriteLine("Writng3 {0} ", message.Size); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } + Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); + message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) + { + Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); + } + } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); + var crt = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); - } - if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) - { - Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); - break; - } - } while (true); - */ + ProtocolToken? alertToken = null; + if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + { + Console.WriteLine(" CompleteHandshakefailed!"); + } + + Console.WriteLine("Renegotiate is NONE {0}", crt); } finally { From 42468df87f7dd4a93566b008b5d4641b276511c3 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 23 Jun 2021 08:15:17 +0000 Subject: [PATCH 04/26] reenable client certificate --- .../pal_ssl.c | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) 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 index b327b14f7b576a..0c7d95d67adcc6 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -401,14 +401,14 @@ 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. -// printf("%s:%d: called!!!\n", __func__, __LINE__); -// return 1; -// } +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. + printf("%s:%d: called!!!\n", __func__, __LINE__); + return 1; +} int32_t CryptoNative_SslPeek(SSL* ssl) { @@ -418,9 +418,6 @@ int32_t CryptoNative_SslPeek(SSL* ssl) int32_t CryptoNative_SslRenegotiate(SSL* ssl) { - // pokusit se odmazat - // int mode = SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE; - SSL_set_options(ssl, SSL_OP_NO_TICKET | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION); int pending = SSL_renegotiate_pending(ssl); @@ -431,8 +428,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl) printf("%s:%d: version=%lu 0x%lx pending %d state %d in_init %d\n", __func__, __LINE__, OpenSSL_version_num(), OpenSSL_version_num(), pending , SSL_get_state(ssl), SSL_in_init(ssl)); if (!pending) { - - // SSL_set_verify(ssl, mode, verify_callback); + SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); ret = SSL_renegotiate(ssl); if(ret!=1) From 93d101be25431dfffb0d99a96f36a974504bbc0d Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 23 Jun 2021 14:34:38 +0200 Subject: [PATCH 05/26] Add more renegotiate tests --- .../SslStreamNetworkStreamTest.cs | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 00fafd6652bf78..be587b8c2f494b 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -315,6 +315,106 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [PlatformSpecific(TestPlatforms.Windows)] + public async Task SslStream_NegotiateClientCertificateAsync_ClientWriteData() + { + using CancellationTokenSource cts = new CancellationTokenSource(); + cts.CancelAfter(TestConfiguration.PassingTestTimeout); + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); + using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); + + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() + { + TargetHost = Guid.NewGuid().ToString("N"), + EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, + }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => + { + //Assert.True(false, "Clent shouldn't send certificate in this test"); + return clientCertificate; + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = serverCertificate }; + serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + { + //Assert.True(false, "Server shouldn't receive certificate in this test"); + return true; + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions, cts.Token), + server.AuthenticateAsServerAsync(serverOptions, cts.Token)); + + Assert.Null(server.RemoteCertificate); + + + var t = server.NegotiateClientCertificateAsync(cts.Token); + + // Send application data instead of Client hello. + await client.WriteAsync(new byte[500], cts.Token); + // Fail as it is not allowed to receive non hnadshake frames during handshake. + await Assert.ThrowsAsync(()=> t); + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [PlatformSpecific(TestPlatforms.Windows)] + public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClientData() + { + using CancellationTokenSource cts = new CancellationTokenSource(); + cts.CancelAfter(TestConfiguration.PassingTestTimeout); + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); + using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); + + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() + { + TargetHost = Guid.NewGuid().ToString("N"), + EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, + }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => + { + //Assert.True(false, "Clent shouldn't send certificate in this test"); + return clientCertificate; + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = serverCertificate }; + serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => + { + //Assert.True(false, "Server shouldn't receive certificate in this test"); + return true; + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions, cts.Token), + server.AuthenticateAsServerAsync(serverOptions, cts.Token)); + + Assert.Null(server.RemoteCertificate); + + // Send application data instead of Client hello. + await client.WriteAsync(new byte[500], cts.Token); + // Server don't drain the client data + await server.ReadAsync(new byte[1]); + // Fail as it is not allowed to receive non hnadshake frames during handshake. + await Assert.ThrowsAsync(()=> + server.NegotiateClientCertificateAsync(cts.Token) + ); + } + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsTls13))] [InlineData(true)] [InlineData(false)] From 4d3173731a1c19a4947a7886bd671cd93ef15ac6 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 24 Jun 2021 17:44:37 +0200 Subject: [PATCH 06/26] Remove client certificates --- .../SslStreamNetworkStreamTest.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index be587b8c2f494b..27e2ecf5b3f5e1 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -327,7 +327,6 @@ public async Task SslStream_NegotiateClientCertificateAsync_ClientWriteData() using (server) { using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); - using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { @@ -335,18 +334,8 @@ public async Task SslStream_NegotiateClientCertificateAsync_ClientWriteData() EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; - clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => - { - //Assert.True(false, "Clent shouldn't send certificate in this test"); - return clientCertificate; - }; SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = serverCertificate }; - serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => - { - //Assert.True(false, "Server shouldn't receive certificate in this test"); - return true; - }; await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions, cts.Token), @@ -376,7 +365,6 @@ public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClien using (server) { using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); - using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { @@ -384,18 +372,7 @@ public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClien EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; - clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => - { - //Assert.True(false, "Clent shouldn't send certificate in this test"); - return clientCertificate; - }; - SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = serverCertificate }; - serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => - { - //Assert.True(false, "Server shouldn't receive certificate in this test"); - return true; - }; await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions, cts.Token), From 400a65630ce59e0d50dafad970956503e480a7f6 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 24 Jun 2021 13:05:19 +0000 Subject: [PATCH 07/26] Cleanup --- .../Interop.OpenSsl.cs | 16 ++- .../Interop.Ssl.cs | 3 - .../entrypoints.c | 1 - .../opensslshim.h | 2 - .../pal_ssl.c | 66 +-------- .../pal_ssl.h | 10 -- .../src/System.Net.Security.csproj | 1 - .../Security/CipherSuitesPolicyPal.Linux.cs | 5 - .../src/System/Net/Security/SecureChannel.cs | 15 +- .../Net/Security/SslStream.Implementation.cs | 131 +++--------------- .../src/System/Net/Security/SslStream.cs | 24 ++-- .../Net/Security/SslStreamPal.Android.cs | 10 -- .../System/Net/Security/SslStreamPal.OSX.cs | 10 -- .../System/Net/Security/SslStreamPal.Unix.cs | 41 +----- .../Net/Security/SslStreamPal.Windows.cs | 10 -- .../SslStreamNegotiatedCipherSuiteTest.cs | 13 +- .../SslStreamNetworkStreamTest.cs | 45 ++---- .../tests/FunctionalTests/TestHelper.cs | 17 +-- 18 files changed, 77 insertions(+), 343 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 index dde5d9691481d9..487cb62920b410 100644 --- 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 @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Globalization; using System.IO; +using System.Net; using System.Net.Http; using System.Net.Security; using System.Runtime.CompilerServices; @@ -226,6 +227,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(); + 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 input, out byte[]? sendBuf, out int sendCount) { sendBuf = null; @@ -241,9 +255,7 @@ internal static bool DoSslHandshake(SafeSslHandle context, ReadOnlySpan in } } - Console.WriteLine("Going to do handshake " + Ssl.IsSslRenegotiatePending(context)); int retVal = Ssl.SslDoHandshake(context); - Console.WriteLine("After handshake " + Ssl.IsSslRenegotiatePending(context)); if (retVal != 1) { Exception? innerError; 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 c37adb45670f7a..5ce167d4bc2220 100644 --- 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,9 +74,6 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRead", SetLastError = true)] internal static extern unsafe int SslRead(SafeSslHandle ssl, byte* buf, int num); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslPeek")] - internal static extern int SslPeek(SafeSslHandle ssl); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRenegotiate")] internal static extern int SslRenegotiate(SafeSslHandle ssl); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 6f96744895323c..cd492502dce441 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -285,7 +285,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BioWrite) DllImportEntry(CryptoNative_EnsureLibSslInitialized) DllImportEntry(CryptoNative_GetOpenSslCipherSuiteName) - DllImportEntry(CryptoNative_SslPeek) DllImportEntry(CryptoNative_SslRenegotiate) DllImportEntry(CryptoNative_IsSslRenegotiatePending) DllImportEntry(CryptoNative_IsSslStateOK) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 9551a38a187527..49516979d008a6 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -457,7 +457,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) \ @@ -902,7 +901,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_keylog_callback SSL_CTX_set_keylog_callback_ptr #define SSL_CTX_set_cipher_list SSL_CTX_set_cipher_list_ptr #define SSL_CTX_set_ciphersuites SSL_CTX_set_ciphersuites_ptr 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 index 0c7d95d67adcc6..00e8fff29f99f3 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -19,20 +19,6 @@ c_static_assert(PAL_SSL_ERROR_WANT_WRITE == SSL_ERROR_WANT_WRITE); c_static_assert(PAL_SSL_ERROR_SYSCALL == SSL_ERROR_SYSCALL); c_static_assert(PAL_SSL_ERROR_ZERO_RETURN == SSL_ERROR_ZERO_RETURN); -/* -#define DOTNET_DEFAULT_CIPHERSTRING \ - "ECDHE-ECDSA-AES256-GCM-SHA384:" \ - "ECDHE-ECDSA-AES128-GCM-SHA256:" \ - "ECDHE-RSA-AES256-GCM-SHA384:" \ - "ECDHE-RSA-AES128-GCM-SHA256:" \ - "ECDHE-ECDSA-AES256-SHA384:" \ - "ECDHE-ECDSA-AES128-SHA256:" \ - "ECDHE-RSA-AES256-SHA384:" \ - "ECDHE-RSA-AES128-SHA256:" \ - "DHE-RSA-AES256-GCM-SHA384:ALL" \ - -*/ - int32_t CryptoNative_EnsureOpenSslInitialized(void); #ifdef NEED_OPENSSL_1_0 @@ -143,15 +129,6 @@ void CryptoNative_EnsureLibSslInitialized() EnsureLibSsl10Initialized(); #endif - printf("%s:%d: ++++++++++++++++++++++Init keylog\n", __func__, __LINE__); - fp = fopen("/tmp/keylog.txt", "a+"); - if(setvbuf(fp, NULL, _IONBF, 0)) - { - fclose(fp); - return; - } - printf("%s:%d: ++++++++++++++++++++++Initialised\n", __func__, __LINE__); - DetectCiphersuiteConfiguration(); } @@ -162,21 +139,6 @@ const SSL_METHOD* CryptoNative_SslV2_3Method() return method; } -void SSL_CTX_keylog_cb_func_cb(const SSL *ssl, const char *line); -void SSL_CTX_keylog_cb_func_cb(const SSL *ssl, const char *line) -{ - - if (!line || !fp) { - return; - } - - printf("%s:%d: SSL_CTX_keylog_cb_func_cb\n", __func__, __LINE__); - (void)ssl; - - fprintf(fp, "%s\n" ,line); - fflush(fp); -} - SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) { SSL_CTX* ctx = SSL_CTX_new(method); @@ -193,14 +155,12 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) // If openssl.cnf doesn't have an opinion for CipherString, then use this value instead if (!g_config_specified_ciphersuites) { - //if (!SSL_CTX_set_cipher_list(ctx, DOTNET_DEFAULT_CIPHERSTRING)) if (!SSL_CTX_set_cipher_list(ctx, "ALL")) { SSL_CTX_free(ctx); return NULL; } } - SSL_CTX_set_keylog_callback(ctx, SSL_CTX_keylog_cb_func_cb); } return ctx; @@ -320,8 +280,7 @@ void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) SSL* CryptoNative_SslCreate(SSL_CTX* ctx) { - SSL* ssl = SSL_new(ctx); - return ssl; + return SSL_new(ctx); } int32_t CryptoNative_SslGetError(SSL* ssl, int32_t ret) @@ -357,13 +316,11 @@ void CryptoNative_SslCtxDestroy(SSL_CTX* ctx) void CryptoNative_SslSetConnectState(SSL* ssl) { - printf("%s:%d: connset state\n", __func__, __LINE__); SSL_set_connect_state(ssl); } void CryptoNative_SslSetAcceptState(SSL* ssl) { - printf("%s:%d: accept state\n", __func__, __LINE__); SSL_set_accept_state(ssl); } @@ -406,45 +363,30 @@ 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. - printf("%s:%d: called!!!\n", __func__, __LINE__); return 1; } -int32_t CryptoNative_SslPeek(SSL* ssl) -{ - return SSL_peek(ssl, NULL, 0); -} - - int32_t CryptoNative_SslRenegotiate(SSL* ssl) { SSL_set_options(ssl, SSL_OP_NO_TICKET | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION); int pending = SSL_renegotiate_pending(ssl); - printf("%s:%d: +++++++++++++++++++++++++++++++++++++++++++++++++\n", __func__, __LINE__); - printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, pending, (void*)ssl); - - int ret=0; - printf("%s:%d: version=%lu 0x%lx pending %d state %d in_init %d\n", __func__, __LINE__, OpenSSL_version_num(), OpenSSL_version_num(), pending , SSL_get_state(ssl), SSL_in_init(ssl)); if (!pending) { SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); - printf("%s:%d: set verify in init %s state \n", __func__, __LINE__, SSL_state_string_long(ssl)); - ret = SSL_renegotiate(ssl); - if(ret!=1) + int ret = SSL_renegotiate(ssl); + if(ret != 1) return ret; return SSL_do_handshake(ssl); } - return ret; + return 0; } int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { - printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, SSL_renegotiate_pending(ssl), (void*)ssl); SSL_peek(ssl, NULL, 0); - printf("%s:%d: pending=%d ssl=%p\n", __func__, __LINE__, SSL_renegotiate_pending(ssl), (void*)ssl); 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 index 5604ee473d0b12..79c6cbe22f955b 100644 --- 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,14 +213,6 @@ when an error is encountered. */ PALEXPORT int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num); -/* -Shims the SSL_peak method. - -Returns 1 when renegotiation started; 0 on error. -*/ -PALEXPORT int32_t CryptoNative_SslPeek(SSL* ssl); - - /* Shims the SSL_renegotiate method. @@ -400,5 +392,3 @@ PALEXPORT const char* CryptoNative_GetOpenSslCipherSuiteName(SSL* ssl, int32_t c Checks if given protocol version is supported. */ PALEXPORT int32_t CryptoNative_OpenSslGetProtocolSupport(SslProtocols protocol); - -static FILE * fp = NULL; diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index 12f769ff39f3a6..100115a0920a2c 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -423,7 +423,6 @@ - diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs index 1b4f578f86b643..feb27260e0656b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs @@ -28,11 +28,6 @@ internal sealed class CipherSuitesPolicyPal internal CipherSuitesPolicyPal(IEnumerable allowedCipherSuites) { -// if (!Interop.Ssl.Tls13Supported) -// { -// throw new PlatformNotSupportedException(SR.net_ssl_ciphersuites_policy_not_supported); -// } - using (SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method)) { if (innerContext.IsInvalid) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 935a1616e4327a..5d234b5886215f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -21,7 +21,7 @@ internal sealed class SecureChannel internal const int ReadHeaderSize = 5; private SafeFreeCredentials? _credentialsHandle; - internal SafeDeleteSslContext? _securityContext; + private SafeDeleteSslContext? _securityContext; private SslConnectionInfo? _connectionInfo; private X509Certificate? _selectedClientCertificate; @@ -944,11 +944,9 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot X509Chain? chain = null; X509Certificate2Collection? remoteCertificateStore = null; - try { X509Certificate2? certificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, out remoteCertificateStore); -Console.WriteLine("VerifyRemoteCertificate called, remote is {0}", certificate); if (_remoteCertificate != null && certificate != null && certificate.RawData.AsSpan().SequenceEqual(_remoteCertificate.RawData)) @@ -1116,17 +1114,6 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot return GenerateAlertToken(); } - public SecurityStatusPal Renegotiate() - { - return SslStreamPal.Renegotiate(_securityContext!); - } - - - public SecurityStatusPal Peek() - { - return SslStreamPal.Peek(ref _securityContext!); - } - private ProtocolToken GenerateAlertToken() { byte[]? nextmsg = null; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index b083967a74f852..f9610e2a28c7fb 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -305,79 +305,10 @@ private async Task ReplyOnReAuthenticationAsync(TIOAdapter adapter, } } - - private async Task Renegotiate(TIOAdapter adapter, bool receiveFirst, byte[]? reAuthenticationData, bool isApm = false, bool renego = false) - where TIOAdapter : IReadWriteAdapter - { - SecurityStatusPal foo = _context!.Renegotiate(); - - if (foo.ErrorCode != SecurityStatusPalErrorCode.OK ){ - - await ForceAuthenticationAsync(adapter, false, null, isApm).ConfigureAwait(false); - - Console.WriteLine("Renegotiate: short doen!"); - } - -// ProtocolToken message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Renegotiate: Next message?????? ---------------------------------------------"); - - // Ask for "Hello Request" message - ProtocolToken message = _context!.NextMessage(reAuthenticationData); - Console.WriteLine("Writig {0} {1}", message.Size, message.Status); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - - Console.WriteLine("REad done with {0}", "len"); -// await ForceAuthenticationAsync(adapter, false, null).ConfigureAwait(false); -// Console.WriteLine("ForceAuthenticationAsync????? done??"); - - - //Interop.OpenSsl.SslRenegotiate(_context._securityContext); - - - // Console.WriteLine("Renegotiate ?????"); - // _context!.Renegotiate(); - _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); - do { - - Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); - message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) - { - Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); - } - } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); - - var crt = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); - - ProtocolToken? alertToken = null; - if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) - { -Console.WriteLine(" CompleteHandshakefailed!"); - } - - Console.WriteLine("Renegotiate is NONE {0}", crt); - -// await ReceiveBlobAsync(adapter, true); -// byte[] buffer = new byte[32000]; -// int len = await ReadAsyncInternal(adapter, buffer, true).ConfigureAwait(false); -// Console.WriteLine("REad done with {0}", len); - -} - // This will initiate renegotiation or PHA for Tls1.3 - private async Task RenegotiateAsync(CancellationToken cancellationToken) + private async Task RenegotiateAsync(TIOAdapter adapter) + where TIOAdapter : IReadWriteAdapter { - if (Interlocked.Exchange(ref _nestedAuth, 1) == 1) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidnestedcall, "NegotiateClientCertificateAsync", "renegotiate")); @@ -396,66 +327,44 @@ private async Task RenegotiateAsync(CancellationToken cancellationToken) _sslAuthenticationOptions!.RemoteCertRequired = true; - byte[]? reAuthenticationData = null; try { - var adapter = new AsyncReadWriteAdapter(InnerStream, cancellationToken); - SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg); - ProtocolToken message; - if (nextmsg!.Length == 0){ - message = _context!.NextMessage(reAuthenticationData); - nextmsg = message.Payload; - } - - if (nextmsg!.Length > 0) - { - await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { if (status.ErrorCode == SecurityStatusPalErrorCode.NoRenegotiation) { - // peer does not want to renegotiate. That should keep session usable. + // Peer does not want to renegotiate. That should keep session usable. return; } throw SslStreamPal.GetException(status); } - // If Windows: - // Issue empty read to get renegotiation going. - //await ReadAsyncInternal(adapter, Memory.Empty, renegotiation: true).ConfigureAwait(false); - + if (nextmsg!.Length > 0) + { + await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); + ProtocolToken message = null!; do { - - Console.WriteLine("Renegotiate: Waiting for ReceiveBlobAsync?????? ---------------------------------------------"); - message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Writng2 {0} and {1}", message.Size, message.Status); - if (message.Size > 0) - { - await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - if (message.Status.ErrorCode != SecurityStatusPalErrorCode.ContinueNeeded) - { - Console.WriteLine("Renegotiate: finished with {0} ---------------------------------------------", message.Status.ErrorCode); - } + message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); + if (message.Size > 0) + { + await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + // else: Should it throw? } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); - var crt = CertificateValidationPal.GetRemoteCertificate(_context!._securityContext!); - ProtocolToken? alertToken = null; if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) { - Console.WriteLine(" CompleteHandshakefailed!"); + //Should it throw? Console.WriteLine(" CompleteHandshakefailed!"); } - - Console.WriteLine("Renegotiate is NONE {0}", crt); } finally { @@ -489,7 +398,6 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool message = _context!.NextMessage(reAuthenticationData); if (message.Size > 0) { - Console.WriteLine("Senfing {0}", message.Size); await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); if (NetEventSource.Log.IsEnabled()) @@ -516,15 +424,12 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool while (!handshakeCompleted) { - Console.WriteLine("Going to receive!"); message = await ReceiveBlobAsync(adapter).ConfigureAwait(false); - Console.WriteLine("Senfing2 {0} error={1}", message.Size, message.Status.ErrorCode); byte[]? payload = null; int size = 0; if (message.Size > 0) { - Console.WriteLine("Senfing2 {0}", message.Size); payload = message.Payload; size = message.Size; } @@ -654,8 +559,6 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a await FillHandshakeBufferAsync(adapter, frameSize).ConfigureAwait(false); } - Console.WriteLine("REceive blob done. frame size {0}, got {1}", frameSize, _handshakeBuffer.ActiveLength); - // At this point, we have at least one TLS frame. if (_lastFrame.Header.Type == TlsContentType.Alert) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 55c79ebd3077e9..276652dbcb94f0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -523,17 +523,17 @@ private SslProtocols GetSslProtocolInternal() public virtual bool CheckCertRevocationStatus => _context != null && _context.CheckCertRevocationStatus != X509RevocationMode.NoCheck; - // - // This will return selected local cert for both client/server streams - // - public virtual X509Certificate? LocalCertificate - { - get - { - ThrowIfExceptionalOrNotAuthenticated(); - return _context!.IsServer ? _context.LocalServerCertificate : _context.LocalClientCertificate; - } - } + // + // This will return selected local cert for both client/server streams + // + public virtual X509Certificate? LocalCertificate + { + get + { + ThrowIfExceptionalOrNotAuthenticated(); + return _context!.IsServer ? _context.LocalServerCertificate : _context.LocalClientCertificate; + } + } public virtual X509Certificate? RemoteCertificate { @@ -697,7 +697,7 @@ public virtual Task NegotiateClientCertificateAsync(CancellationToken cancellati throw new InvalidOperationException(SR.net_ssl_certificate_exist); } - return RenegotiateAsync(cancellationToken); + return RenegotiateAsync(new AsyncReadWriteAdapter(InnerStream, cancellationToken)); } protected override void Dispose(bool disposing) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index 8ac203af6087c7..24f1e8e0e01b60 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -241,15 +241,5 @@ public static SecurityStatusPal ApplyShutdownToken( return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError); } - - public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) - { - throw new PlatformNotSupportedException(); - } - - public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) - { - throw new PlatformNotSupportedException(); - } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index c454aaca47d69d..53c4d12c99ab81 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -221,16 +221,6 @@ public static void QueryContextConnectionInfo( connectionInfo = new SslConnectionInfo(securityContext.SslContext); } - public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) - { - throw new PlatformNotSupportedException(); - } - - public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) - { - throw new PlatformNotSupportedException(); - } - private static SecurityStatusPal HandshakeInternal( SafeFreeCredentials credential, ref SafeDeleteSslContext? context, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 68482683677114..612ee0c6ae588a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -81,36 +81,17 @@ public static SecurityStatusPal DecryptMessage(SafeDeleteSslContext securityCont return bindingHandle; } - public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) - { - int ret = Interop.Ssl.SslPeek(((SafeDeleteSslContext)securityContext!).SslContext); - return new SecurityStatusPal(ret == 0 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); - } - - public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? securityContext, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer) { -// SafeDeleteSslContext sslContext = ((SafeDeleteSslContext)securityContext); - - Console.WriteLine("RENEGO ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext!).SslContext)); - - int ret = Interop.Ssl.SslRenegotiate(((SafeDeleteSslContext)securityContext).SslContext); - Console.WriteLine("RENEGO {1} ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext!).SslContext), ret); + var sslContext = ((SafeDeleteSslContext)securityContext!).SslContext; + SecurityStatusPal status = Interop.OpenSsl.SslRenegotiate(sslContext, out outputBuffer); outputBuffer = Array.Empty(); - - return new SecurityStatusPal(ret == 0 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); - } - - public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) - { -// SafeDeleteSslContext sslContext = ((SafeDeleteSslContext)securityContext); - - Console.WriteLine("RENEGO ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext).SslContext)); - - int ret = Interop.Ssl.SslRenegotiate(((SafeDeleteSslContext)securityContext).SslContext); - Console.WriteLine("RENEGO {1} ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)securityContext).SslContext), ret); - return new SecurityStatusPal(ret == 1 ? SecurityStatusPalErrorCode.OK : SecurityStatusPalErrorCode.InternalError); + if (status.ErrorCode != SecurityStatusPalErrorCode.OK) + { + return status; + } + return HandshakeInternal(credentialsHandle!, ref securityContext, null, ref outputBuffer, sslAuthenticationOptions); } public static void QueryContextStreamSizes(SafeDeleteContext? securityContext, out StreamSizes streamSizes) @@ -136,8 +117,6 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia byte[]? output = null; int outputSize = 0; - Console.WriteLine("HandshakeInternal: giving {0} bytes", inputBuffer.Length); - try { if ((null == context) || context.IsInvalid) @@ -145,17 +124,11 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia context = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); } - -Console.WriteLine("RENEGO ? {0}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)); bool done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, inputBuffer, out output, out outputSize); if (outputSize == 0 && Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)) { - Console.WriteLine("WTF fif got nothe! {0} trying again", done); done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, ReadOnlySpan.Empty, out output, out outputSize); - Console.WriteLine("WTF {0} {1}", done, outputSize); } -Console.WriteLine("RENEGO ? {0} done ? {1}", Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext), done); - // When the handshake is done, and the context is server, check if the alpnHandle target was set to null during ALPN. // If it was, then that indicates ALPN failed, send failure. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index e0c61f76da56c7..59a781b470a9e5 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -430,16 +430,6 @@ public static void QueryContextConnectionInfo(SafeDeleteContext securityContext, connectionInfo = new SslConnectionInfo(interopConnectionInfo, cipherSuite); } - public static SecurityStatusPal Peek(ref SafeDeleteSslContext? securityContext) - { - throw new PlatformNotSupportedException(); - } - - public static SecurityStatusPal Renegotiate(SafeDeleteContext securityContext) - { - throw new PlatformNotSupportedException(); - } - private static int GetProtocolFlagsFromSslProtocols(SslProtocols protocols, bool isServer) { int protocolFlags = (int)protocols; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs index 2280677a587c7f..5b1e1c7bd146ca 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs @@ -575,15 +575,10 @@ private static IReadOnlyList GetSupportedNonTls13CipherSuites() if (!CipherSuitesPolicySupported) return null; - // var ret = new Lazy>(()=>{ - // var ret = new List(); - // AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); - // return ret; - // }); - - - - return new List(); + var ret = new List(); + AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); + + return ret; } private static bool RequiredByTls13Spec(TlsCipherSuite cs) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index f884d194a3aa8f..421c14ae1d5191 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -183,12 +183,11 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl cts.CancelAfter(TestConfiguration.PassingTestTimeout); (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); - //using (client) + using (client) using (server) + using (X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate()) + using (X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate()) { - using X509Certificate2 serverCertificate = Configuration.Certificates.GetServerCertificate(); - using X509Certificate2 clientCertificate = Configuration.Certificates.GetClientCertificate(); - SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = Guid.NewGuid().ToString("N"), @@ -205,8 +204,7 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl { if (negotiateClientCertificateCalled && sendClientCertificate) { - if (TestHelper.AllowClient) - Assert.Equal(clientCertificate.GetCertHash(), certificate?.GetCertHash()); + Assert.Equal(clientCertificate.GetCertHash(), certificate?.GetCertHash()); } else { @@ -217,22 +215,15 @@ public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendCl }; - if (TestHelper.AllowClient) - await TestConfiguration.WhenAllOrAnyFailedWithTimeout( - client.AuthenticateAsClientAsync(clientOptions, cts.Token), - server.AuthenticateAsServerAsync(serverOptions, cts.Token)); - else - await server.AuthenticateAsServerAsync(serverOptions, cts.Token); - Assert.Null(server.RemoteCertificate); + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions, cts.Token), + server.AuthenticateAsServerAsync(serverOptions, cts.Token)); + Assert.Null(server.RemoteCertificate); - ValueTask t = new (); - if (TestHelper.AllowClient) - { - // Client needs to be reading for renegotiation to happen. - byte[] buffer = new byte[TestHelper.s_ping.Length]; - t = client.ReadAsync(buffer, cts.Token); - } + // Client needs to be reading for renegotiation to happen. + byte[] buffer = new byte[TestHelper.s_ping.Length]; + ValueTask t = client.ReadAsync(buffer, cts.Token); negotiateClientCertificateCalled = true; await server.NegotiateClientCertificateAsync(cts.Token); @@ -245,21 +236,13 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( Assert.Null(server.RemoteCertificate); } - - // Finish the client's read await server.WriteAsync(TestHelper.s_ping, cts.Token); - if (TestHelper.AllowClient) - await t; - + await t; // verify that the session is usable with or without client's certificate - if (TestHelper.AllowClient) - { - await TestHelper.PingPong(client, server, cts.Token); - await TestHelper.PingPong(server, client, cts.Token); - } - + await TestHelper.PingPong(client, server, cts.Token); + await TestHelper.PingPong(server, client, cts.Token); } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 181e9b032cbe0e..998e93813ec1e8 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -19,9 +19,6 @@ namespace System.Net.Security.Tests { public static class TestHelper { - - public static bool AllowClient = true; - private static readonly X509KeyUsageExtension s_eeKeyUsage = new X509KeyUsageExtension( X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.KeyEncipherment | X509KeyUsageFlags.DataEncipherment, @@ -52,9 +49,7 @@ public static class TestHelper public static (SslStream ClientStream, SslStream ServerStream) GetConnectedSslStreams() { (Stream clientStream, Stream serverStream) = GetConnectedStreams(); - return ( - AllowClient ? new SslStream(clientStream) : null, - new SslStream(serverStream)); + return (new SslStream(clientStream), new SslStream(serverStream)); } public static (Stream ClientStream, Stream ServerStream) GetConnectedStreams() @@ -72,21 +67,17 @@ internal static (NetworkStream ClientStream, NetworkStream ServerStream) GetConn { using (Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { - listener.Bind(new IPEndPoint(IPAddress.Loopback, 7001)); + listener.Bind(new IPEndPoint(IPAddress.Loopback, 0)); listener.Listen(1); var clientSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - if(AllowClient) - clientSocket.Connect(listener.LocalEndPoint); + clientSocket.Connect(listener.LocalEndPoint); Socket serverSocket = listener.Accept(); serverSocket.NoDelay = true; clientSocket.NoDelay = true; - return ( - AllowClient ? new NetworkStream(clientSocket, ownsSocket: true) : - null - , new NetworkStream(serverSocket, ownsSocket: true)); + return (new NetworkStream(clientSocket, ownsSocket: true), new NetworkStream(serverSocket, ownsSocket: true)); } } From 9cc6f069046622540c1aea947f66f92df4bf8f3a Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Fri, 25 Jun 2021 16:24:13 +0200 Subject: [PATCH 08/26] add test log --- .../tests/FunctionalTests/SslStreamNetworkStreamTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 27e2ecf5b3f5e1..b60d85a48317e6 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -343,6 +343,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( Assert.Null(server.RemoteCertificate); + Console.WriteLine("AA " + server.SslProtocol); var t = server.NegotiateClientCertificateAsync(cts.Token); From e648618c5bd0d4dac1813590bc77d460417ca52b Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Fri, 25 Jun 2021 14:42:35 +0000 Subject: [PATCH 09/26] Apply PR comments --- .../pal_ssl.c | 18 +++++- .../Security/CipherSuitesPolicyPal.Linux.cs | 5 ++ .../Net/Security/SslStream.Implementation.cs | 63 ++++++++++--------- .../SslStreamNegotiatedCipherSuiteTest.cs | 2 +- .../SslStreamNetworkStreamTest.cs | 2 +- 5 files changed, 54 insertions(+), 36 deletions(-) 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 index 00e8fff29f99f3..9e2b1a7904f667 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -19,6 +19,16 @@ c_static_assert(PAL_SSL_ERROR_WANT_WRITE == SSL_ERROR_WANT_WRITE); c_static_assert(PAL_SSL_ERROR_SYSCALL == SSL_ERROR_SYSCALL); c_static_assert(PAL_SSL_ERROR_ZERO_RETURN == SSL_ERROR_ZERO_RETURN); +#define DOTNET_DEFAULT_CIPHERSTRING \ + "ECDHE-ECDSA-AES256-GCM-SHA384:" \ + "ECDHE-ECDSA-AES128-GCM-SHA256:" \ + "ECDHE-RSA-AES256-GCM-SHA384:" \ + "ECDHE-RSA-AES128-GCM-SHA256:" \ + "ECDHE-ECDSA-AES256-SHA384:" \ + "ECDHE-ECDSA-AES128-SHA256:" \ + "ECDHE-RSA-AES256-SHA384:" \ + "ECDHE-RSA-AES128-SHA256:" \ + int32_t CryptoNative_EnsureOpenSslInitialized(void); #ifdef NEED_OPENSSL_1_0 @@ -29,7 +39,7 @@ static void EnsureLibSsl10Initialized() } #endif -static int32_t g_config_specified_ciphersuites = 1; +static int32_t g_config_specified_ciphersuites = 0; static void DetectCiphersuiteConfiguration() { @@ -155,7 +165,7 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) // If openssl.cnf doesn't have an opinion for CipherString, then use this value instead if (!g_config_specified_ciphersuites) { - if (!SSL_CTX_set_cipher_list(ctx, "ALL")) + if (!SSL_CTX_set_cipher_list(ctx, DOTNET_DEFAULT_CIPHERSTRING)) { SSL_CTX_free(ctx); return NULL; @@ -368,12 +378,14 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX* store) 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 | SSL_VERIFY_CLIENT_ONCE, verify_callback); + + // SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); int ret = SSL_renegotiate(ssl); if(ret != 1) return ret; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs index feb27260e0656b..0ddef7980c836f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/CipherSuitesPolicyPal.Linux.cs @@ -28,6 +28,11 @@ internal sealed class CipherSuitesPolicyPal internal CipherSuitesPolicyPal(IEnumerable allowedCipherSuites) { + if (!Interop.Ssl.Tls13Supported) + { + throw new PlatformNotSupportedException(SR.net_ssl_ciphersuites_policy_not_supported); + } + using (SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method)) { if (innerContext.IsInvalid) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 6ea9a8d587d069..ab61d8bdec7cf4 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -321,6 +321,12 @@ private async Task RenegotiateAsync(TIOAdapter adapter) { SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg); + if (nextmsg!.Length > 0) + { + await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); + await adapter.FlushAsync().ConfigureAwait(false); + } + if (status.ErrorCode != SecurityStatusPalErrorCode.OK) { if (status.ErrorCode == SecurityStatusPalErrorCode.NoRenegotiation) @@ -332,12 +338,6 @@ private async Task RenegotiateAsync(TIOAdapter adapter) throw SslStreamPal.GetException(status); } - if (nextmsg!.Length > 0) - { - await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); - await adapter.FlushAsync().ConfigureAwait(false); - } - _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); ProtocolToken message = null!; do { @@ -347,14 +347,10 @@ private async Task RenegotiateAsync(TIOAdapter adapter) await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } - // else: Should it throw? + Debug.Assert(!(message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded && message.Size == 0)); } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); - ProtocolToken? alertToken = null; - if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) - { - //Should it throw? Console.WriteLine(" CompleteHandshakefailed!"); - } + CompleteHandshake(_sslAuthenticationOptions!); } finally { @@ -468,25 +464,7 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool _internalBufferCount = _handshakeBuffer.ActiveLength; } - ProtocolToken? alertToken = null; - if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) - { - if (_sslAuthenticationOptions!.CertValidationDelegate != null) - { - // there may be some chain errors but the decision was made by custom callback. Details should be tracing if enabled. - SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.net_ssl_io_cert_custom_validation, null))); - } - else if (sslPolicyErrors == SslPolicyErrors.RemoteCertificateChainErrors && chainStatus != X509ChainStatusFlags.NoError) - { - // We failed only because of chain and we have some insight. - SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); - } - else - { - // Simple add sslPolicyErrors as crude info. - SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); - } - } + CompleteHandshake(_sslAuthenticationOptions!); } finally { @@ -692,6 +670,29 @@ private bool CompleteHandshake(ref ProtocolToken? alertToken, out SslPolicyError return true; } + private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions) + { + ProtocolToken? alertToken = null; + if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + { + if (sslAuthenticationOptions!.CertValidationDelegate != null) + { + // there may be some chain errors but the decision was made by custom callback. Details should be tracing if enabled. + SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.net_ssl_io_cert_custom_validation, null))); + } + else if (sslPolicyErrors == SslPolicyErrors.RemoteCertificateChainErrors && chainStatus != X509ChainStatusFlags.NoError) + { + // We failed only because of chain and we have some insight. + SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); + } + else + { + // Simple add sslPolicyErrors as crude info. + SendAuthResetSignal(alertToken, ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); + } + } + } + private async ValueTask WriteAsyncChunked(TIOAdapter writeAdapter, ReadOnlyMemory buffer) where TIOAdapter : struct, IReadWriteAdapter { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs index 5b1e1c7bd146ca..b73c699079f044 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs @@ -576,7 +576,7 @@ private static IReadOnlyList GetSupportedNonTls13CipherSuites() return null; var ret = new List(); - AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); + //AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); return ret; } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index eb85d8dc30814d..03ff791c57afc9 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -174,7 +174,7 @@ public async Task SslStream_NetworkStream_Renegotiation_Succeeds(bool useSync) [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] [InlineData(true)] - // [InlineData(false)] + [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendClientCertificate) { From e05ffdbd0c0755096d7334ab2d6c136bd4bca2ce Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Fri, 25 Jun 2021 15:56:42 +0000 Subject: [PATCH 10/26] Add Data frame test --- .../Net/Security/SslStream.Implementation.cs | 96 +++++++++---------- .../SslStreamNetworkStreamTest.cs | 6 +- 2 files changed, 46 insertions(+), 56 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index ab61d8bdec7cf4..9e2e7a4196213d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -528,53 +528,56 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a } // At this point, we have at least one TLS frame. - if (_lastFrame.Header.Type == TlsContentType.Alert) + switch (_lastFrame.Header.Type) { - if (TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame)) - { - if (NetEventSource.Log.IsEnabled() && _lastFrame.AlertDescription != TlsAlertDescription.CloseNotify) NetEventSource.Error(this, $"Received TLS alert {_lastFrame.AlertDescription}"); - } - } - else if (_lastFrame.Header.Type == TlsContentType.Handshake) - { - // During renegotiation the client hello is encrypted. Possibly race condition when the encrypted client hello flag is 0x01 but the content of the message is encrypted. - // Assume the Client Hello is not encrypted. It doesn't apply for renegotiation. - if (_handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && - (_sslAuthenticationOptions!.ServerCertSelectionDelegate != null || - _sslAuthenticationOptions!.ServerOptionDelegate != null)) - { - TlsFrameHelper.ProcessingOptions options = NetEventSource.Log.IsEnabled() ? - TlsFrameHelper.ProcessingOptions.All : - TlsFrameHelper.ProcessingOptions.ServerName; - - // Process SNI from Client Hello message - if (!TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame, options)) + case TlsContentType.Alert: + if (TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame)) { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"Failed to parse TLS hello."); + if (NetEventSource.Log.IsEnabled() && _lastFrame.AlertDescription != TlsAlertDescription.CloseNotify) NetEventSource.Error(this, $"Received TLS alert {_lastFrame.AlertDescription}"); } - - if (_lastFrame.HandshakeType == TlsHandshakeType.ClientHello) + break; + case TlsContentType.Handshake: + // During renegotiation the client hello is encrypted. Possibly race condition when the encrypted client hello flag is 0x01 but the content of the message is encrypted. + // Assume the Client Hello is not encrypted. It doesn't apply for renegotiation. + if (_handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && + (_sslAuthenticationOptions!.ServerCertSelectionDelegate != null || + _sslAuthenticationOptions!.ServerOptionDelegate != null)) { - // SNI if it exist. Even if we could not parse the hello, we can fall-back to default certificate. - if (_lastFrame.TargetName != null) + TlsFrameHelper.ProcessingOptions options = NetEventSource.Log.IsEnabled() ? + TlsFrameHelper.ProcessingOptions.All : + TlsFrameHelper.ProcessingOptions.ServerName; + + // Process SNI from Client Hello message + if (!TlsFrameHelper.TryGetFrameInfo(_handshakeBuffer.ActiveReadOnlySpan, ref _lastFrame, options)) { - _sslAuthenticationOptions!.TargetHost = _lastFrame.TargetName; + if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"Failed to parse TLS hello."); } - if (_sslAuthenticationOptions.ServerOptionDelegate != null) + if (_lastFrame.HandshakeType == TlsHandshakeType.ClientHello) { - SslServerAuthenticationOptions userOptions = - await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_sslAuthenticationOptions.TargetHost, _lastFrame.SupportedVersions), - _sslAuthenticationOptions.UserState, adapter.CancellationToken).ConfigureAwait(false); - _sslAuthenticationOptions.UpdateOptions(userOptions); + // SNI if it exist. Even if we could not parse the hello, we can fall-back to default certificate. + if (_lastFrame.TargetName != null) + { + _sslAuthenticationOptions!.TargetHost = _lastFrame.TargetName; + } + + if (_sslAuthenticationOptions.ServerOptionDelegate != null) + { + SslServerAuthenticationOptions userOptions = + await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_sslAuthenticationOptions.TargetHost, _lastFrame.SupportedVersions), + _sslAuthenticationOptions.UserState, adapter.CancellationToken).ConfigureAwait(false); + _sslAuthenticationOptions.UpdateOptions(userOptions); + } } - } - if (NetEventSource.Log.IsEnabled()) - { - NetEventSource.Log.ReceivedFrame(this, _lastFrame); + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Log.ReceivedFrame(this, _lastFrame); + } } - } + break; + case TlsContentType.AppData: + throw new InvalidOperationException(SR.net_ssl_renegotiate_data); } return ProcessBlob(frameSize); @@ -935,15 +938,12 @@ private SecurityStatusPal DecryptData(int frameSize) return status; } - private async ValueTask ReadAsyncInternal(TIOAdapter adapter, Memory buffer, bool renegotiation = false) + private async ValueTask ReadAsyncInternal(TIOAdapter adapter, Memory buffer) where TIOAdapter : IReadWriteAdapter { - if (!renegotiation) + if (Interlocked.Exchange(ref _nestedRead, 1) == 1) { - if (Interlocked.Exchange(ref _nestedRead, 1) == 1) - { - throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, nameof(SslStream.ReadAsync), "read")); - } + throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, nameof(SslStream.ReadAsync), "read")); } ThrowIfExceptionalOrNotAuthenticated(); @@ -956,11 +956,6 @@ private async ValueTask ReadAsyncInternal(TIOAdapter adapter, M { if (_decryptedBytesCount != 0) { - if (renegotiation) - { - throw new InvalidOperationException(SR.net_ssl_renegotiate_data); - } - processedLength = CopyDecryptedData(buffer); if (processedLength == buffer.Length || !HaveFullTlsFrame(out payloadBytes)) { @@ -1025,11 +1020,6 @@ private async ValueTask ReadAsyncInternal(TIOAdapter adapter, M throw new IOException(SR.net_ssl_io_renego); } await ReplyOnReAuthenticationAsync(adapter, extraBuffer).ConfigureAwait(false); - if (renegotiation) - { - // if we received data frame instead, we would not be here but we would decrypt data and hit check above. - return 0; - } // Loop on read. continue; } @@ -1083,7 +1073,7 @@ private async ValueTask ReadAsyncInternal(TIOAdapter adapter, M } catch (Exception e) { - if (e is IOException || (e is OperationCanceledException && adapter.CancellationToken.IsCancellationRequested) || renegotiation) + if (e is IOException || (e is OperationCanceledException && adapter.CancellationToken.IsCancellationRequested)) { throw; } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index df3c6b4b33d212..e747268cc8dd2f 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -249,7 +249,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] [InlineData(true)] [InlineData(false)] - [PlatformSpecific(TestPlatforms.Windows)] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsyncNoRenego_Succeeds(bool sendClientCertificate) { bool negotiateClientCertificateCalled = false; @@ -319,7 +319,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] - [PlatformSpecific(TestPlatforms.Windows)] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_ClientWriteData() { using CancellationTokenSource cts = new CancellationTokenSource(); @@ -358,7 +358,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] - [PlatformSpecific(TestPlatforms.Windows)] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClientData() { using CancellationTokenSource cts = new CancellationTokenSource(); From 7a9aadb0f2cdfc759a9f7ccc51c2dead2482d9aa Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Mon, 28 Jun 2021 15:36:05 +0000 Subject: [PATCH 11/26] Add drain buffer test --- src/libraries/System.Net.Security/src/Resources/Strings.resx | 3 +++ .../src/System/Net/Security/SslStream.Implementation.cs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx index 8181afecaaa620..a1447e81dfeac3 100644 --- a/src/libraries/System.Net.Security/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Security/src/Resources/Strings.resx @@ -455,6 +455,9 @@ Received data during renegotiation. + + Client stream needs to be drain before renegotiation. + Setting an SNI hostname is not supported on this API level. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 9e2e7a4196213d..192c603b596554 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -315,6 +315,11 @@ private async Task RenegotiateAsync(TIOAdapter adapter) throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, nameof(WriteAsync), "write")); } + if (_decryptedBytesCount is not 0) + { + throw new InvalidOperationException(SR.net_ssl_renegotiate_buffer); + } + _sslAuthenticationOptions!.RemoteCertRequired = true; try From 71191e51817706a92b4a8d44b838145d03c53c03 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Tue, 29 Jun 2021 11:48:41 +0200 Subject: [PATCH 12/26] Fix tls 1.3 incomming app data frame --- .../Net/Security/SslStream.Implementation.cs | 16 ++++++++++++---- .../SslStreamNetworkStreamTest.cs | 10 ++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 192c603b596554..02ca5363309b7c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -19,6 +19,7 @@ public partial class SslStream private SslAuthenticationOptions? _sslAuthenticationOptions; private int _nestedAuth; + private bool _isRenego; private enum Framing { @@ -321,14 +322,15 @@ private async Task RenegotiateAsync(TIOAdapter adapter) } _sslAuthenticationOptions!.RemoteCertRequired = true; + _isRenego = true; try { SecurityStatusPal status = _context!.Renegotiate(out byte[]? nextmsg); - if (nextmsg!.Length > 0) + if (nextmsg is {} && nextmsg.Length > 0) { - await adapter.WriteAsync(nextmsg, 0, nextmsg!.Length).ConfigureAwait(false); + await adapter.WriteAsync(nextmsg, 0, nextmsg.Length).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } @@ -352,7 +354,6 @@ private async Task RenegotiateAsync(TIOAdapter adapter) await adapter.WriteAsync(message.Payload!, 0, message.Size).ConfigureAwait(false); await adapter.FlushAsync().ConfigureAwait(false); } - Debug.Assert(!(message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded && message.Size == 0)); } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); CompleteHandshake(_sslAuthenticationOptions!); @@ -477,6 +478,7 @@ private async Task ForceAuthenticationAsync(TIOAdapter adapter, bool if (reAuthenticationData == null) { _nestedAuth = 0; + _isRenego = false; } } @@ -582,7 +584,13 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a } break; case TlsContentType.AppData: - throw new InvalidOperationException(SR.net_ssl_renegotiate_data); + // TLS1.3 it is not possible to distinguish between late Handshake and Application Data + if (_isRenego && SslProtocol != SslProtocols.Tls13) + { + throw new InvalidOperationException(SR.net_ssl_renegotiate_data); + } + break; + } return ProcessBlob(frameSize); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index e747268cc8dd2f..e0deb7b12c382a 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -397,10 +397,12 @@ await Assert.ThrowsAsync(()=> [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsTls13))] - [InlineData(true)] - [InlineData(false)] + [InlineData(true, SslProtocols.Tls12)] + [InlineData(false, SslProtocols.Tls12)] + [InlineData(true, SslProtocols.Tls13)] + [InlineData(false, SslProtocols.Tls13)] [PlatformSpecific(TestPlatforms.Windows)] - public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool sendClientCertificate) + public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool sendClientCertificate, SslProtocols protocol) { bool negotiateClientCertificateCalled = false; using CancellationTokenSource cts = new CancellationTokenSource(); @@ -415,7 +417,7 @@ public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool s SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = Guid.NewGuid().ToString("N"), - EnabledSslProtocols = SslProtocols.Tls13, + EnabledSslProtocols = protocol, }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => From 5d09969f206e18f91a57ac1fd97f48c21038c5b9 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Tue, 29 Jun 2021 14:15:10 +0000 Subject: [PATCH 13/26] Restore verify callback --- .../Native/Unix/System.Security.Cryptography.Native/pal_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 9e2b1a7904f667..d546426c927433 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -385,7 +385,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl) if (!pending) { - // SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); + SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); int ret = SSL_renegotiate(ssl); if(ret != 1) return ret; From 50dd05ee56b4d1971763b44aae95836879e04120 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 30 Jun 2021 09:22:19 +0000 Subject: [PATCH 14/26] Remove debug log --- .../FunctionalTests/SslStreamNetworkStreamTest.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index e0deb7b12c382a..870afcb5ee7e0e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -346,8 +346,6 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( Assert.Null(server.RemoteCertificate); - Console.WriteLine("AA " + server.SslProtocol); - var t = server.NegotiateClientCertificateAsync(cts.Token); // Send application data instead of Client hello. @@ -397,12 +395,10 @@ await Assert.ThrowsAsync(()=> [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.SupportsTls13))] - [InlineData(true, SslProtocols.Tls12)] - [InlineData(false, SslProtocols.Tls12)] - [InlineData(true, SslProtocols.Tls13)] - [InlineData(false, SslProtocols.Tls13)] + [InlineData(true)] + [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows)] - public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool sendClientCertificate, SslProtocols protocol) + public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool sendClientCertificate) { bool negotiateClientCertificateCalled = false; using CancellationTokenSource cts = new CancellationTokenSource(); @@ -417,7 +413,7 @@ public async Task SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(bool s SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = Guid.NewGuid().ToString("N"), - EnabledSslProtocols = protocol, + EnabledSslProtocols = SslProtocols.Tls13, }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; clientOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => From 5be456699077939dc70b80bae93ddd9149e4c0f8 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 30 Jun 2021 12:29:51 +0000 Subject: [PATCH 15/26] Remove keylog callback and unused method --- .../Unix/System.Security.Cryptography.Native/opensslshim.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index bd56cb7353a669..ac1f663b17577e 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -464,7 +464,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ - REQUIRED_FUNCTION(SSL_CTX_set_keylog_callback) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ @@ -489,7 +488,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_get_state) \ REQUIRED_FUNCTION(ERR_print_errors_fp) \ - REQUIRED_FUNCTION(SSL_in_init) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ REQUIRED_FUNCTION(SSL_set_options) \ @@ -903,7 +901,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_keylog_callback SSL_CTX_set_keylog_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 @@ -935,7 +932,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_read SSL_read_ptr #define SSL_get_state SSL_get_state_ptr #define ERR_print_errors_fp ERR_print_errors_fp_ptr -#define SSL_in_init SSL_in_init_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr #define SSL_set_options SSL_set_options_ptr From 65c143f0e6ffc574e81fff2ac75d4313f2a4bf17 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 30 Jun 2021 12:52:29 +0000 Subject: [PATCH 16/26] Fix test build --- .../tests/UnitTests/Fakes/FakeSslStream.Implementation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs index 4e146153626b31..ee160631b400ff 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs @@ -59,7 +59,7 @@ private Task ProcessAuthentication(bool isAsync = false, bool isApm = false, Can return Task.Run(() => {}); } - private Task RenegotiateAsync(CancellationToken cancellationToken) => throw new PlatformNotSupportedException(); + private Task RenegotiateAsync(AsyncReadWriteAdapter adapter) => throw new PlatformNotSupportedException(); private void ReturnReadBufferIfEmpty() { From 6798888fd492e67b7cbba0eb9a558586824d1774 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 30 Jun 2021 15:43:26 +0000 Subject: [PATCH 17/26] Attempt to fix openssl version api difference --- .../Unix/System.Security.Cryptography.Native/apibridge.c | 7 +++++++ .../Unix/System.Security.Cryptography.Native/apibridge.h | 1 + .../Unix/System.Security.Cryptography.Native/opensslshim.h | 7 ++----- .../System.Security.Cryptography.Native/osslcompat_111.h | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index daf13002b11851..7a34995dd1dd7c 100644 --- 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 index 1b866bc4474d28..968e12dad15724 100644 --- 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/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index ac1f663b17577e..db6da531e334ee 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -54,7 +54,6 @@ #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_1_0_RTM // Remove problematic #defines -#undef SSL_get_state #undef SSL_is_init_finished #undef X509_get_X509_PUBKEY #undef X509_get_version @@ -463,6 +462,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_client_cert_cb) \ REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ + REQUIRED_FUNCTION(SSL_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ @@ -486,11 +486,9 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_peek) \ REQUIRED_FUNCTION(SSL_state_string_long) \ REQUIRED_FUNCTION(SSL_read) \ - REQUIRED_FUNCTION(SSL_get_state) \ REQUIRED_FUNCTION(ERR_print_errors_fp) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ - REQUIRED_FUNCTION(SSL_set_options) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ @@ -904,6 +902,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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 +#define SSL_set_options SSL_set_options_ptr #define SSL_CTX_set_options SSL_CTX_set_options_ptr #define SSL_CTX_set_quiet_shutdown SSL_CTX_set_quiet_shutdown_ptr #define SSL_CTX_set_security_level SSL_CTX_set_security_level_ptr @@ -930,11 +929,9 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_peek SSL_peek_ptr #define SSL_state_string_long SSL_state_string_long_ptr #define SSL_read SSL_read_ptr -#define SSL_get_state SSL_get_state_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_set_options SSL_set_options_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 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 index e9a1b4939bab3d..67047e2fc0fa05 100644 --- 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; From 09fd4c61c8fa5f5aa4c8401917201d2a99cee04f Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Wed, 30 Jun 2021 15:49:57 +0000 Subject: [PATCH 18/26] Sort shim --- .../Unix/System.Security.Cryptography.Native/opensslshim.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index db6da531e334ee..febed1eb3f4311 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -462,7 +462,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_client_cert_cb) \ REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ - REQUIRED_FUNCTION(SSL_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ @@ -493,6 +492,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ REQUIRED_FUNCTION(SSL_set_connect_state) \ + REQUIRED_FUNCTION(SSL_set_options) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ @@ -902,7 +902,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #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 -#define SSL_set_options SSL_set_options_ptr #define SSL_CTX_set_options SSL_CTX_set_options_ptr #define SSL_CTX_set_quiet_shutdown SSL_CTX_set_quiet_shutdown_ptr #define SSL_CTX_set_security_level SSL_CTX_set_security_level_ptr @@ -936,6 +935,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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 From 9b28f705640bfefc7fbcb1b4374afbfd94efd8ec Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 30 Jun 2021 22:54:52 +0000 Subject: [PATCH 19/26] fix build --- .../Unix/System.Security.Cryptography.Native/opensslshim.h | 2 +- .../Unix/System.Security.Cryptography.Native/osslcompat_111.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index febed1eb3f4311..7651b7b1414349 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -492,7 +492,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ REQUIRED_FUNCTION(SSL_set_connect_state) \ - REQUIRED_FUNCTION(SSL_set_options) \ + FALLBACK_FUNCTION(SSL_set_options) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ 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 index 67047e2fc0fa05..6791e306019b77 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h @@ -57,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); From 7bfe54f917a4a90ea3ccbcd33ad0df62a25b2540 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 1 Jul 2021 07:12:01 +0000 Subject: [PATCH 20/26] CI log --- .../Native/Unix/System.Security.Cryptography.Native/apibridge.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index 7a34995dd1dd7c..13c8238c5a5109 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c @@ -787,6 +787,8 @@ unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options) unsigned long local_SSL_set_options(SSL* ssl, unsigned long options) { + + printf("%s:%d local fn called", __FILE__, __LINE__); // 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); From d86628bb8ae4d3e2b5379c16858262fd9526b907 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 1 Jul 2021 11:10:17 +0200 Subject: [PATCH 21/26] Restore mac tests --- .../tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs index b73c699079f044..5b1e1c7bd146ca 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNegotiatedCipherSuiteTest.cs @@ -576,7 +576,7 @@ private static IReadOnlyList GetSupportedNonTls13CipherSuites() return null; var ret = new List(); - //AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); + AllowOneOnOneSide(GetNonTls13CipherSuites(), (cs) => false, (cs) => ret.Add(cs)); return ret; } From 100bb82b303bbeadc9a5e1eeec17e0f11f15af57 Mon Sep 17 00:00:00 2001 From: Jan Jahoda Date: Thu, 1 Jul 2021 13:30:34 +0000 Subject: [PATCH 22/26] Add logs --- .../Unix/System.Security.Cryptography.Native/apibridge.c | 3 +-- .../Unix/System.Security.Cryptography.Native/opensslshim.h | 2 +- .../Native/Unix/System.Security.Cryptography.Native/pal_ssl.c | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index 13c8238c5a5109..6df94f733d9cfe 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c @@ -787,8 +787,7 @@ unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options) unsigned long local_SSL_set_options(SSL* ssl, unsigned long options) { - - printf("%s:%d local fn called", __FILE__, __LINE__); + printf("%s:%d local fn called %s \n", __FILE__, __LINE__, OPENSSL_VERSION_TEXT); // 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); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 7651b7b1414349..febed1eb3f4311 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -492,7 +492,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); 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_options) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ 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 index d546426c927433..8154290e7bd291 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -378,6 +378,7 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX* store) int32_t CryptoNative_SslRenegotiate(SSL* ssl) { + printf("%s:%d %s \n", __FILE__, __LINE__, OPENSSL_VERSION_TEXT); // 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); From f7bc9f3cd54da349aa503a6494f695ed41d77e01 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 7 Jul 2021 18:44:17 +0000 Subject: [PATCH 23/26] fix test runs on old openssl --- .../tests/FunctionalTests/SslStreamNetworkStreamTest.cs | 8 +++++--- .../tests/FunctionalTests/TestConfiguration.cs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 870afcb5ee7e0e..b3a5f5e21f46ce 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -42,6 +42,8 @@ public void Dispose() public class SslStreamNetworkStreamTest : IClassFixture { + private static bool SupportsRenegotiation => TestConfiguration.SupportsRenegotiation; + readonly ITestOutputHelper _output; readonly CertificateSetup certificates; @@ -172,7 +174,7 @@ public async Task SslStream_NetworkStream_Renegotiation_Succeeds(bool useSync) } } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ConditionalTheory(nameof(SupportsRenegotiation))] [InlineData(true)] [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] @@ -246,7 +248,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ConditionalTheory(nameof(SupportsRenegotiation))] [InlineData(true)] [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] @@ -355,7 +357,7 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ConditionalFact(nameof(SupportsRenegotiation))] [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClientData() { diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs index a7dbdc9451ea4b..525f682f19d34f 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs @@ -27,8 +27,8 @@ internal static class TestConfiguration public const string NtlmUserFilePath = "/var/tmp/ntlm_user_file"; public static bool SupportsNullEncryption { get { return s_supportsNullEncryption.Value; } } - public static bool SupportsHandshakeAlerts { get { return OperatingSystem.IsLinux() || OperatingSystem.IsWindows(); } } + public static bool SupportsRenegotiation { get { return (OperatingSystem.IsWindows() && PlatformDetection.IsWindows7) || ((OperatingSystem.IsLinux() || OperatingSystem.IsFreeBSD()) && PlatformDetection.OpenSslVersion >= new Version(1, 1, 0)); } } public static Task WhenAllOrAnyFailedWithTimeout(params Task[] tasks) => tasks.WhenAllOrAnyFailed(PassingTestTimeoutMilliseconds); From e7a4e795da832f6621e213593239c581358e2642 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 8 Jul 2021 22:58:32 +0000 Subject: [PATCH 24/26] fix tests --- .../Native/Unix/System.Security.Cryptography.Native/pal_ssl.c | 2 +- .../tests/FunctionalTests/TestConfiguration.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index 8154290e7bd291..d36a3ef4ca29e2 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -386,7 +386,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl) if (!pending) { - SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_callback); + SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); int ret = SSL_renegotiate(ssl); if(ret != 1) return ret; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs index 525f682f19d34f..357fadbcbcda9d 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs @@ -28,7 +28,7 @@ internal static class TestConfiguration public static bool SupportsNullEncryption { get { return s_supportsNullEncryption.Value; } } public static bool SupportsHandshakeAlerts { get { return OperatingSystem.IsLinux() || OperatingSystem.IsWindows(); } } - public static bool SupportsRenegotiation { get { return (OperatingSystem.IsWindows() && PlatformDetection.IsWindows7) || ((OperatingSystem.IsLinux() || OperatingSystem.IsFreeBSD()) && PlatformDetection.OpenSslVersion >= new Version(1, 1, 0)); } } + public static bool SupportsRenegotiation { get { return (OperatingSystem.IsWindows() && PlatformDetection.IsWindows7) || ((OperatingSystem.IsLinux() || OperatingSystem.IsFreeBSD()) && PlatformDetection.OpenSslVersion >= new Version(1, 1, 1)); } } public static Task WhenAllOrAnyFailedWithTimeout(params Task[] tasks) => tasks.WhenAllOrAnyFailed(PassingTestTimeoutMilliseconds); From b9b797a9ff8f66bb6dfe820a0da27ccb0e91923e Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 9 Jul 2021 05:50:13 +0000 Subject: [PATCH 25/26] fix w7 condition --- .../tests/FunctionalTests/TestConfiguration.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs index 357fadbcbcda9d..4bd1b0ee30f9a2 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs @@ -28,7 +28,7 @@ internal static class TestConfiguration public static bool SupportsNullEncryption { get { return s_supportsNullEncryption.Value; } } public static bool SupportsHandshakeAlerts { get { return OperatingSystem.IsLinux() || OperatingSystem.IsWindows(); } } - public static bool SupportsRenegotiation { get { return (OperatingSystem.IsWindows() && PlatformDetection.IsWindows7) || ((OperatingSystem.IsLinux() || OperatingSystem.IsFreeBSD()) && PlatformDetection.OpenSslVersion >= new Version(1, 1, 1)); } } + public static bool SupportsRenegotiation { get { return (OperatingSystem.IsWindows() && !PlatformDetection.IsWindows7) || ((OperatingSystem.IsLinux() || OperatingSystem.IsFreeBSD()) && PlatformDetection.OpenSslVersion >= new Version(1, 1, 1)); } } public static Task WhenAllOrAnyFailedWithTimeout(params Task[] tasks) => tasks.WhenAllOrAnyFailed(PassingTestTimeoutMilliseconds); From 86b98096f203bae0113ee311dc5dea585125be14 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 9 Jul 2021 19:43:36 +0000 Subject: [PATCH 26/26] feedback from review --- .../System.Security.Cryptography.Native/apibridge.c | 7 +++---- .../opensslshim.c | 1 - .../opensslshim.h | 3 +-- .../System.Security.Cryptography.Native/pal_ssl.c | 2 -- .../System.Net.Security/src/Resources/Strings.resx | 2 +- .../System/Net/Security/SslStream.Implementation.cs | 13 ++++++++++--- .../src/System/Net/Security/SslStreamPal.Unix.cs | 2 ++ 7 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index 6df94f733d9cfe..0414eb2b27108d 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c @@ -787,10 +787,9 @@ unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options) unsigned long local_SSL_set_options(SSL* ssl, unsigned long options) { - printf("%s:%d local fn called %s \n", __FILE__, __LINE__, OPENSSL_VERSION_TEXT); - // 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); + // 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) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.c index e4512d1e5a8c19..1b040d7aa7c188 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.c @@ -72,7 +72,6 @@ int OpenLibrary() strcat(soName, versionOverride); #endif - printf("%s:%d %s \n", __FILE__, __LINE__, soName); DlOpen(soName); } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 7651b7b1414349..0144578a2dcce3 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -54,6 +54,7 @@ #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_1_0_RTM // Remove problematic #defines +#undef SSL_get_state #undef SSL_is_init_finished #undef X509_get_X509_PUBKEY #undef X509_get_version @@ -483,9 +484,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); LEGACY_FUNCTION(SSL_load_error_strings) \ REQUIRED_FUNCTION(SSL_new) \ REQUIRED_FUNCTION(SSL_peek) \ - REQUIRED_FUNCTION(SSL_state_string_long) \ REQUIRED_FUNCTION(SSL_read) \ - REQUIRED_FUNCTION(ERR_print_errors_fp) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ FALLBACK_FUNCTION(SSL_session_reused) \ 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 index d36a3ef4ca29e2..acdc977b5f32de 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -378,14 +378,12 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX* store) int32_t CryptoNative_SslRenegotiate(SSL* ssl) { - printf("%s:%d %s \n", __FILE__, __LINE__, OPENSSL_VERSION_TEXT); // 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) diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx index a1447e81dfeac3..dc54446dc1c4e4 100644 --- a/src/libraries/System.Net.Security/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Security/src/Resources/Strings.resx @@ -456,7 +456,7 @@ Received data during renegotiation. - Client stream needs to be drain before renegotiation. + Client stream needs to be drained before renegotiation. Setting an SNI hostname is not supported on this API level. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 02ca5363309b7c..b554cc410287b8 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -356,12 +356,21 @@ private async Task RenegotiateAsync(TIOAdapter adapter) } } while (message.Status.ErrorCode == SecurityStatusPalErrorCode.ContinueNeeded); + if (_handshakeBuffer.ActiveLength > 0) + { + // If we read more than we needed for handshake, move it to input buffer for further processing. + ResetReadBuffer(); + _handshakeBuffer.ActiveSpan.CopyTo(_internalBuffer); + _internalBufferCount = _handshakeBuffer.ActiveLength; + } + CompleteHandshake(_sslAuthenticationOptions!); } finally { _nestedRead = 0; _nestedWrite = 0; + _isRenego = false; // We will not release _nestedAuth at this point to prevent another renegotiation attempt. } } @@ -544,9 +553,7 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a } break; case TlsContentType.Handshake: - // During renegotiation the client hello is encrypted. Possibly race condition when the encrypted client hello flag is 0x01 but the content of the message is encrypted. - // Assume the Client Hello is not encrypted. It doesn't apply for renegotiation. - if (_handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && + if (!_isRenego && _handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && (_sslAuthenticationOptions!.ServerCertSelectionDelegate != null || _sslAuthenticationOptions!.ServerOptionDelegate != null)) { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index dc431b573a6531..ed0bcaaccd1b9b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -159,6 +159,8 @@ private static SecurityStatusPal HandshakeInternal(SafeFreeCredentials credentia } bool done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, inputBuffer, out output, out outputSize); + // sometimes during renegotiation processing messgae does not yield new output. + // That seems to be flaw in OpenSSL state machine and we have workaround to peek it and try it again. if (outputSize == 0 && Interop.Ssl.IsSslRenegotiatePending(((SafeDeleteSslContext)context).SslContext)) { done = Interop.OpenSsl.DoSslHandshake(((SafeDeleteSslContext)context).SslContext, ReadOnlySpan.Empty, out output, out outputSize);