diff options
16 files changed, 215 insertions, 95 deletions
diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 47a9312266e..9dd310326b0 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 @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.IO; +using System.Net; using System.Net.Security; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -223,6 +224,19 @@ internal static partial class Interop return context; } + internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out byte[]? outputBuffer) + { + int ret = Interop.Ssl.SslRenegotiate(sslContext); + + outputBuffer = Array.Empty<byte>(); + if (ret != 1) + { + GetSslError(sslContext, ret, out Exception? exception); + return new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, exception); + } + return new SecurityStatusPal(SecurityStatusPalErrorCode.OK); + } + internal static bool DoSslHandshake(SafeSslHandle context, ReadOnlySpan<byte> input, out byte[]? sendBuf, out int sendCount) { sendBuf = null; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index d080cf2f0d7..29154b77da6 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 Interop [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRead", SetLastError = true)] internal static extern int SslRead(SafeSslHandle ssl, ref byte buf, int num); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslRenegotiate")] + internal static extern int SslRenegotiate(SafeSslHandle ssl); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_IsSslRenegotiatePending")] [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool IsSslRenegotiatePending(SafeSslHandle ssl); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index daf13002b11..0414eb2b271 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 1b866bc4474..968e12dad15 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/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 996921777f6..99db84bfc3c 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -286,6 +286,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_BioWrite) DllImportEntry(CryptoNative_EnsureLibSslInitialized) DllImportEntry(CryptoNative_GetOpenSslCipherSuiteName) + DllImportEntry(CryptoNative_SslRenegotiate) DllImportEntry(CryptoNative_IsSslRenegotiatePending) DllImportEntry(CryptoNative_IsSslStateOK) DllImportEntry(CryptoNative_SetCiphers) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 050cdc25140..510d319a11d 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -458,7 +458,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_new) \ LIGHTUP_FUNCTION(SSL_CTX_set_alpn_protos) \ LIGHTUP_FUNCTION(SSL_CTX_set_alpn_select_cb) \ - REQUIRED_FUNCTION(SSL_CTX_set_cert_verify_callback) \ REQUIRED_FUNCTION(SSL_CTX_set_cipher_list) \ LIGHTUP_FUNCTION(SSL_CTX_set_ciphersuites) \ REQUIRED_FUNCTION(SSL_CTX_set_client_cert_cb) \ @@ -484,12 +483,16 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); LEGACY_FUNCTION(SSL_library_init) \ LEGACY_FUNCTION(SSL_load_error_strings) \ REQUIRED_FUNCTION(SSL_new) \ + REQUIRED_FUNCTION(SSL_peek) \ REQUIRED_FUNCTION(SSL_read) \ + REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ REQUIRED_FUNCTION(SSL_set_connect_state) \ + FALLBACK_FUNCTION(SSL_set_options) \ + REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ LEGACY_FUNCTION(SSLeay) \ @@ -895,7 +898,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CTX_new SSL_CTX_new_ptr #define SSL_CTX_set_alpn_protos SSL_CTX_set_alpn_protos_ptr #define SSL_CTX_set_alpn_select_cb SSL_CTX_set_alpn_select_cb_ptr -#define SSL_CTX_set_cert_verify_callback SSL_CTX_set_cert_verify_callback_ptr #define SSL_CTX_set_cipher_list SSL_CTX_set_cipher_list_ptr #define SSL_CTX_set_ciphersuites SSL_CTX_set_ciphersuites_ptr #define SSL_CTX_set_client_cert_cb SSL_CTX_set_client_cert_cb_ptr @@ -922,12 +924,18 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_library_init SSL_library_init_ptr #define SSL_load_error_strings SSL_load_error_strings_ptr #define SSL_new SSL_new_ptr +#define SSL_peek SSL_peek_ptr +#define SSL_state_string_long SSL_state_string_long_ptr #define SSL_read SSL_read_ptr +#define ERR_print_errors_fp ERR_print_errors_fp_ptr +#define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr #define SSL_session_reused SSL_session_reused_ptr #define SSL_set_accept_state SSL_set_accept_state_ptr #define SSL_set_bio SSL_set_bio_ptr #define SSL_set_connect_state SSL_set_connect_state_ptr +#define SSL_set_options SSL_set_options_ptr +#define SSL_set_verify SSL_set_verify_ptr #define SSL_shutdown SSL_shutdown_ptr #define SSL_state SSL_state_ptr #define SSLeay SSLeay_ptr diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/osslcompat_111.h index e9a1b4939ba..6791e306019 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; @@ -56,6 +57,7 @@ int SSL_CTX_config(SSL_CTX* ctx, const char* name); unsigned long SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options); void SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level); int32_t SSL_is_init_finished(SSL* ssl); +unsigned long SSL_set_options(SSL* ctx, unsigned long options); int SSL_session_reused(SSL* ssl); const SSL_METHOD* TLS_method(void); const ASN1_TIME* X509_CRL_get0_nextUpdate(const X509_CRL* crl); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index 524179cdcb2..acdc977b5f3 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 @@ -368,8 +368,36 @@ int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num) return SSL_read(ssl, buf, num); } +static int verify_callback(int preverify_ok, X509_STORE_CTX* store) +{ + (void)preverify_ok; + (void)store; + // We don't care. Real verification happens in managed code. + return 1; +} + +int32_t CryptoNative_SslRenegotiate(SSL* ssl) +{ + // The openssl context is destroyed so we can't use ticket or session resumption. + SSL_set_options(ssl, SSL_OP_NO_TICKET | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION); + + int pending = SSL_renegotiate_pending(ssl); + if (!pending) + { + SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); + int ret = SSL_renegotiate(ssl); + if(ret != 1) + return ret; + + return SSL_do_handshake(ssl); + } + + return 0; +} + int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { + SSL_peek(ssl, NULL, 0); return SSL_renegotiate_pending(ssl) != 0; } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h index d7b995c5a19..79c6cbe22f9 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 @@ -214,6 +214,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. Returns 1 when negotiation is requested; 0 once a handshake has finished. diff --git a/src/libraries/System.Net.Security/src/Resources/Strings.resx b/src/libraries/System.Net.Security/src/Resources/Strings.resx index 513a54ccc2a..e33b4122a7d 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 @@ <data name="net_ssl_renegotiate_data" xml:space="preserve"> <value>Received data during renegotiation.</value> </data> + <data name="net_ssl_renegotiate_buffer" xml:space="preserve"> + <value>Client stream needs to be drained before renegotiation.</value> + </data> <data name="net_android_ssl_api_level_unsupported" xml:space="preserve"> <value>Setting an SNI hostname is not supported on this API level.</value> </data> 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 2ecc2a3932a..f0f0b954d4a 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 @@ namespace System.Net.Security private SslAuthenticationOptions? _sslAuthenticationOptions; private int _nestedAuth; + private bool _isRenego; private enum Framing { @@ -296,7 +297,8 @@ namespace System.Net.Security } // This will initiate renegotiation or PHA for Tls1.3 - private async Task RenegotiateAsync(CancellationToken cancellationToken) + private async Task RenegotiateAsync<TIOAdapter>(TIOAdapter adapter) + where TIOAdapter : IReadWriteAdapter { if (Interlocked.Exchange(ref _nestedAuth, 1) == 1) { @@ -314,13 +316,19 @@ namespace System.Net.Security 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; - IReadWriteAdapter adapter = new AsyncReadWriteAdapter(InnerStream, cancellationToken); + _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.FlushAsync().ConfigureAwait(false); @@ -330,20 +338,39 @@ namespace System.Net.Security { 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); } - // Issue empty read to get renegotiation going. - await ReadAsyncInternal(adapter, Memory<byte>.Empty, renegotiation: true).ConfigureAwait(false); + _handshakeBuffer = new ArrayBuffer(InitialHandshakeBufferSize); + ProtocolToken message = null!; + do { + 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); + } + } 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. } } @@ -452,25 +479,7 @@ namespace System.Net.Security _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 { @@ -478,6 +487,7 @@ namespace System.Net.Security if (reAuthenticationData == null) { _nestedAuth = 0; + _isRenego = false; } } @@ -534,51 +544,60 @@ namespace System.Net.Security } // 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) - { - 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: + if (!_isRenego && _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()) + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Log.ReceivedFrame(this, _lastFrame); + } + } + break; + case TlsContentType.AppData: + // TLS1.3 it is not possible to distinguish between late Handshake and Application Data + if (_isRenego && SslProtocol != SslProtocols.Tls13) { - NetEventSource.Log.ReceivedFrame(this, _lastFrame); + throw new InvalidOperationException(SR.net_ssl_renegotiate_data); } - } + break; + } return ProcessBlob(frameSize); @@ -674,6 +693,29 @@ namespace System.Net.Security 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>(TIOAdapter writeAdapter, ReadOnlyMemory<byte> buffer) where TIOAdapter : struct, IReadWriteAdapter { @@ -916,15 +958,12 @@ namespace System.Net.Security return status; } - private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, Memory<byte> buffer, bool renegotiation = false) + private async ValueTask<int> ReadAsyncInternal<TIOAdapter>(TIOAdapter adapter, Memory<byte> 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(); @@ -937,11 +976,6 @@ namespace System.Net.Security { if (_decryptedBytesCount != 0) { - if (renegotiation) - { - throw new InvalidOperationException(SR.net_ssl_renegotiate_data); - } - processedLength = CopyDecryptedData(buffer); if (processedLength == buffer.Length || !HaveFullTlsFrame(out payloadBytes)) { @@ -1006,11 +1040,6 @@ namespace System.Net.Security 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; } @@ -1064,7 +1093,7 @@ namespace System.Net.Security } 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/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 75c2938d444..276652dbcb9 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 @@ -697,7 +697,7 @@ namespace System.Net.Security 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.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 02045613170..ed0bcaaccd1 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 @@ -36,11 +36,6 @@ namespace System.Net.Security return HandshakeInternal(credential!, ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions); } - public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? context, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer) - { - throw new PlatformNotSupportedException(); - } - public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { @@ -120,6 +115,19 @@ namespace System.Net.Security return bindingHandle; } + public static SecurityStatusPal Renegotiate(ref SafeFreeCredentials? credentialsHandle, ref SafeDeleteSslContext? securityContext, SslAuthenticationOptions sslAuthenticationOptions, out byte[]? outputBuffer) + { + var sslContext = ((SafeDeleteSslContext)securityContext!).SslContext; + SecurityStatusPal status = Interop.OpenSsl.SslRenegotiate(sslContext, out outputBuffer); + + outputBuffer = Array.Empty<byte>(); + 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) { streamSizes = StreamSizes.Default; @@ -150,7 +158,13 @@ namespace System.Net.Security context = new SafeDeleteSslContext((credential as SafeFreeSslCredentials)!, sslAuthenticationOptions); } - bool done = Interop.OpenSsl.DoSslHandshake(context.SslContext, inputBuffer, out output, out outputSize); + 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<byte>.Empty, out output, out outputSize); + } // 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/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 6ff3afb23f3..b3a5f5e21f4 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 @@ namespace System.Net.Security.Tests public class SslStreamNetworkStreamTest : IClassFixture<CertificateSetup> { + private static bool SupportsRenegotiation => TestConfiguration.SupportsRenegotiation; + readonly ITestOutputHelper _output; readonly CertificateSetup certificates; @@ -172,10 +174,10 @@ namespace System.Net.Security.Tests } } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ConditionalTheory(nameof(SupportsRenegotiation))] [InlineData(true)] [InlineData(false)] - [PlatformSpecific(TestPlatforms.Windows)] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_Succeeds(bool sendClientCertificate) { bool negotiateClientCertificateCalled = false; @@ -214,6 +216,7 @@ namespace System.Net.Security.Tests return true; }; + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions, cts.Token), server.AuthenticateAsServerAsync(serverOptions, cts.Token)); @@ -234,19 +237,21 @@ namespace System.Net.Security.Tests { Assert.Null(server.RemoteCertificate); } + // Finish the client's read await server.WriteAsync(TestHelper.s_ping, cts.Token); 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); } } - [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] + [ConditionalTheory(nameof(SupportsRenegotiation))] [InlineData(true)] [InlineData(false)] - [PlatformSpecific(TestPlatforms.Windows)] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsyncNoRenego_Succeeds(bool sendClientCertificate) { bool negotiateClientCertificateCalled = false; @@ -316,8 +321,7 @@ namespace System.Net.Security.Tests } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] - [PlatformSpecific(TestPlatforms.Windows)] - [ActiveIssue("https://github.com/dotnet/runtime/pull/54692")] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_ClientWriteData() { using CancellationTokenSource cts = new CancellationTokenSource(); @@ -344,7 +348,6 @@ namespace System.Net.Security.Tests Assert.Null(server.RemoteCertificate); - var t = server.NegotiateClientCertificateAsync(cts.Token); // Send application data instead of Client hello. @@ -354,8 +357,8 @@ namespace System.Net.Security.Tests } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindows7))] - [PlatformSpecific(TestPlatforms.Windows)] + [ConditionalFact(nameof(SupportsRenegotiation))] + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] public async Task SslStream_NegotiateClientCertificateAsync_ServerDontDrainClientData() { using CancellationTokenSource cts = new CancellationTokenSource(); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs index a7dbdc9451e..4bd1b0ee30f 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 @@ namespace System.Net.Security.Tests 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, 1)); } } public static Task WhenAllOrAnyFailedWithTimeout(params Task[] tasks) => tasks.WhenAllOrAnyFailed(PassingTestTimeoutMilliseconds); 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 4e146153626..ee160631b40 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 @@ namespace System.Net.Security return Task.Run(() => {}); } - private Task RenegotiateAsync(CancellationToken cancellationToken) => throw new PlatformNotSupportedException(); + private Task RenegotiateAsync(AsyncReadWriteAdapter adapter) => throw new PlatformNotSupportedException(); private void ReturnReadBufferIfEmpty() { |