From b6fb7621a45e6af5345bd3ec5749b1514956ce89 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 7 May 2025 13:36:14 +0200 Subject: [PATCH 01/16] Dirty WIP --- .../Interop.OpenSsl.cs | 21 +++- .../Interop.Ssl.cs | 9 ++ .../Interop.SslCtx.cs | 14 +++ .../Net/CertificateValidationPal.Unix.cs | 12 ++- .../src/System/Net/Security/SslStream.IO.cs | 55 +++++------ .../System/Net/Security/SslStream.Protocol.cs | 42 +++++--- .../src/System/Net/Security/SslStream.cs | 1 + .../System/Net/Security/SslStreamPal.Unix.cs | 13 +++ .../src/System/Net/SecurityStatusPal.cs | 1 + .../entrypoints.c | 3 + .../opensslshim.h | 31 +++++- .../pal_ssl.c | 96 ++++++++++++++++++- .../pal_ssl.h | 20 +++- 13 files changed, 273 insertions(+), 45 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 d89ce63d496fb3..13c33e2eefbd40 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 @@ -213,6 +213,8 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication throw CreateSslException(SR.net_allocate_ssl_context_failed); } + Ssl.SslCtxSetCertVerifyCallback(sslCtx, &Ssl.CertVerifyCallback); + Ssl.SslCtxSetProtocolOptions(sslCtx, protocols); if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) @@ -247,7 +249,7 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication // If you find yourself wanting to remove this line to enable bidirectional // close-notify, you'll probably need to rewrite SafeSslHandle.Disconnect(). // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html - Ssl.SslCtxSetQuietShutdown(sslCtx); + // Ssl.SslCtxSetQuietShutdown(sslCtx); if (enableResume) { @@ -434,6 +436,12 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth if (sslAuthenticationOptions.IsClient) { + // Client side always verifies the server's certificate. + Ssl.SslSetVerifyPeer(sslHandle); + + // HACK: set a bogus code to indicate that we did not perform the validation yet + // Ssl.SslSetVerifyResult(sslHandle, -1); + if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !IPAddress.IsValid(sslAuthenticationOptions.TargetHost)) { // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. @@ -466,6 +474,8 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth if (sslAuthenticationOptions.RemoteCertRequired) { Ssl.SslSetVerifyPeer(sslHandle); + // HACK: set a bogus code to indicate that we did not perform the validation yet + Ssl.SslSetVerifyResult(sslHandle, -1); } if (sslAuthenticationOptions.CertificateContext != null) @@ -542,6 +552,8 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, } } +#pragma warning disable CS0618 + System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_do_handshake"); int retVal = Ssl.SslDoHandshake(context, out Ssl.SslErrorCode errorCode); if (retVal != 1) { @@ -550,6 +562,13 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, return SecurityStatusPalErrorCode.CredentialsNeeded; } + if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_ASYNC) + // if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_RETRY_VERIFY) + { + System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_ERROR_WANT_ASYNC"); + return SecurityStatusPalErrorCode.PeerCertVerifyRequired; + } + if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ)) { Exception? innerError = GetSslError(retVal, errorCode); 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 65c0d37e3d5a30..fa56c48e390d4f 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 @@ -204,6 +204,12 @@ internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl) [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetData")] internal static partial void SslSessionSetData(IntPtr session, IntPtr val); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyResult")] + internal static partial void SslSetVerifyResult(SafeSslHandle ssl, long verifyResult); + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetVerifyResult")] + internal static partial long SslGetVerifyResult(SafeSslHandle ssl); + internal static class Capabilities { // needs separate type (separate static cctor) to be sure OpenSSL is initialized. @@ -340,6 +346,9 @@ internal enum SslErrorCode SSL_ERROR_SYSCALL = 5, SSL_ERROR_ZERO_RETURN = 6, + SSL_ERROR_WANT_ASYNC = 9, + SSL_ERROR_WANT_RETRY_VERIFY = 12, + // NOTE: this SslErrorCode value doesn't exist in OpenSSL, but // we use it to distinguish when a renegotiation is pending. // Choosing an arbitrarily large value that shouldn't conflict diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs index 7c3e8d2e14bfc6..9511b361da4ec1 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs @@ -62,6 +62,20 @@ internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, ReadOnl return true; } + + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCertVerifyCallback")] + internal static unsafe partial void SslCtxSetCertVerifyCallback(SafeSslContextHandle ctx, delegate* unmanaged callback); + + [UnmanagedCallersOnly] + internal static int CertVerifyCallback(IntPtr ssl, IntPtr store) + { + System.Console.WriteLine($"CertVerifyCallback called with store: {store:x8}, ssl: {ssl:x8}"); + IntPtr data = Ssl.SslGetData(ssl); + System.Console.WriteLine($"SSL data: {data:x8}"); + GCHandle gch = GCHandle.FromIntPtr(data); + System.Console.WriteLine($"SSL data handle: {gch.Target}"); + return 0; + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs index 6fda4835d4ae2e..360123ca0e0f08 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs @@ -35,8 +35,18 @@ internal static SslPolicyErrors VerifyCertificateProperties( return null; } + IntPtr remoteCertificate = IntPtr.Zero; + { + using var chainStack = Interop.OpenSsl.GetPeerCertificateChain((SafeSslHandle)securityContext); + int count = Interop.Crypto.GetX509StackFieldCount(chainStack); + if (count > 0) + { + remoteCertificate = Interop.Crypto.GetX509StackField(chainStack, 0); + } + } + X509Certificate2? result = null; - IntPtr remoteCertificate = Interop.OpenSsl.GetPeerCertificate((SafeSslHandle)securityContext); + try { if (remoteCertificate == IntPtr.Zero) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index eac06538b92119..6dc57a5a8769ca 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -297,7 +297,8 @@ private async Task ForceAuthenticationAsync(bool receiveFirst, byte[ { if (!receiveFirst) { - token = NextMessage(reAuthenticationData, out int consumed); + int consumed; + (token, consumed) = await NextMessage(reAuthenticationData).ConfigureAwait(false); Debug.Assert(consumed == (reAuthenticationData?.Length ?? 0)); if (token.Size > 0) @@ -489,14 +490,14 @@ private ProtocolToken ProcessTlsFrame(int frameSize) { int chunkSize = frameSize; - ReadOnlySpan availableData = _buffer.EncryptedReadOnlySpan; + ReadOnlyMemory availableData = _buffer.EncryptedReadOnlyMemory; // Often more TLS messages fit into same packet. Get as many complete frames as we can. while (_buffer.EncryptedLength - chunkSize > TlsFrameHelper.HeaderSize) { TlsFrameHeader nextHeader = default; - if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize), ref nextHeader)) + if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize).Span, ref nextHeader)) { break; } @@ -514,7 +515,7 @@ private ProtocolToken ProcessTlsFrame(int frameSize) chunkSize += frameSize; } - ProtocolToken token = NextMessage(availableData.Slice(0, chunkSize), out int consumed); + (ProtocolToken token, int consumed) = NextMessage(availableData.Slice(0, chunkSize)).GetAwaiter().GetResult(); _buffer.DiscardEncrypted(consumed); return token; } @@ -523,20 +524,15 @@ private ProtocolToken ProcessTlsFrame(int frameSize) // This is to reset auth state on remote side. // If this write succeeds we will allow auth retrying. // - private void SendAuthResetSignal(ReadOnlySpan alert, ExceptionDispatchInfo exception) + private void SendAuthResetAndThrow(ReadOnlySpan alert, ExceptionDispatchInfo exception) { SetException(exception.SourceException); - if (alert.Length == 0) + if (alert.Length >= 0) { - // - // We don't have an alert to send so cannot retry and fail prematurely. - // - exception.Throw(); + InnerStream.Write(alert); } - InnerStream.Write(alert); - exception.Throw(); } @@ -591,21 +587,26 @@ private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions ProtocolToken alertToken = default; 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(new ReadOnlySpan(alertToken.Payload), 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(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); - } - else - { - // Simple add sslPolicyErrors as crude info. - SendAuthResetSignal(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); - } + ProcessFailedCertificateValidation(sslAuthenticationOptions, ref alertToken, sslPolicyErrors, chainStatus); + } + } + + private void ProcessFailedCertificateValidation(SslAuthenticationOptions sslAuthenticationOptions, ref ProtocolToken alertToken, SslPolicyErrors sslPolicyErrors, 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. + SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), 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. + SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); + } + else + { + // Simple add sslPolicyErrors as crude info. + SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 1e0a8ed0e2a756..8db6358d22bb09 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -11,6 +11,7 @@ using System.Security.Authentication.ExtendedProtection; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; namespace System.Net.Security { @@ -813,9 +814,10 @@ static DateTime GetExpiryTimestamp(SslStreamCertificateContext certificateContex } // - internal ProtocolToken NextMessage(ReadOnlySpan incomingBuffer, out int consumed) + internal async Task<(ProtocolToken, int)> NextMessage(ReadOnlyMemory incomingBuffer) { - ProtocolToken token = GenerateToken(incomingBuffer, out consumed); + (ProtocolToken token, int consumed) = await GenerateToken(incomingBuffer).ConfigureAwait(false); + if (NetEventSource.Log.IsEnabled()) { if (token.Failed) @@ -824,7 +826,7 @@ internal ProtocolToken NextMessage(ReadOnlySpan incomingBuffer, out int co } } - return token; + return (token, consumed); } /*++ @@ -840,7 +842,7 @@ generates a set of bytes that will be sent next to Return: token - ProtocolToken with status and optionally buffer. --*/ - private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int consumed) + private async Task<(ProtocolToken, int)> GenerateToken(ReadOnlyMemory inputBuffer) { bool cachedCreds = false; bool sendTrustList = false; @@ -849,6 +851,9 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons ProtocolToken token = default; token.RentBuffer = true; + int consumed = 0; + int tmpConsumed; + // We need to try get credentials at the beginning. // _credentialsHandle may be always null on some platforms but // _securityContext will be allocated on first call. @@ -861,6 +866,7 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons { do { + retry: thumbPrint = null; if (refreshCredentialNeeded) { @@ -876,8 +882,8 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons token = SslStreamPal.AcceptSecurityContext( ref _credentialsHandle!, ref _securityContext, - inputBuffer, - out consumed, + inputBuffer.Span, + out tmpConsumed, _sslAuthenticationOptions); if (token.Status.ErrorCode == SecurityStatusPalErrorCode.HandshakeStarted) { @@ -905,8 +911,8 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons ref _credentialsHandle!, ref _securityContext, hostName, - inputBuffer, - out consumed, + inputBuffer.Span, + out tmpConsumed, _sslAuthenticationOptions); if (token.Status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded) @@ -926,6 +932,20 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons _sslAuthenticationOptions); } } + consumed += tmpConsumed; + inputBuffer = inputBuffer.Slice(tmpConsumed); + + if (token.Status.ErrorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired) + { + await Task.Yield(); + if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertValidationDelegate, _sslAuthenticationOptions.CertificateContext?.Trust, ref token, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + { + ProcessFailedCertificateValidation(_sslAuthenticationOptions, ref token, sslPolicyErrors, chainStatus); + } + + goto retry; + + } } while (cachedCreds && _credentialsHandle == null); } finally @@ -957,7 +977,7 @@ private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int cons } } - return token; + return (token, consumed); } internal ProtocolToken Renegotiate() @@ -1216,12 +1236,12 @@ private ProtocolToken CreateShutdownToken() return default; } - return GenerateToken(default, out _); + return GenerateToken(default).GetAwaiter().GetResult().Item1; } private ProtocolToken GenerateAlertToken() { - return GenerateToken(default, out _); + return GenerateToken(default).GetAwaiter().GetResult().Item1; } private static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain) 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 7c2897d0e9b139..04ddca13d38207 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 @@ -96,6 +96,7 @@ public ReadOnlySpan DecryptedReadOnlySpanSliced(int length) public Span EncryptedSpanSliced(int length) => _buffer.ActiveSpan.Slice(_decryptedLength + _decryptedPadding, length); public ReadOnlySpan EncryptedReadOnlySpan => _buffer.ActiveSpan.Slice(_decryptedLength + _decryptedPadding); + public ReadOnlyMemory EncryptedReadOnlyMemory => _buffer.ActiveMemory.Slice(_decryptedLength + _decryptedPadding); public int EncryptedLength => _buffer.ActiveLength - _decryptedPadding - _decryptedLength; 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 700531aa49d35d..d74a47bfd66a37 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 @@ -207,6 +207,16 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); } + if (errorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired) + { + token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.PeerCertVerifyRequired); + return token; + // System.Console.WriteLine($"Peer certificate verification required. setting VerifyResult to an error"); + // Interop.Ssl.SslSetVerifyResult((SafeSslHandle)context, 0); + // // continue with the handshake, the callback will be invoked later. + // errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); + } + // sometimes during renegotiation processing message 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 (token.Size == 0 && Interop.Ssl.IsSslRenegotiatePending((SafeSslHandle)context)) @@ -239,6 +249,9 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context public static SecurityStatusPal ApplyAlertToken(SafeDeleteContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage) { + // All the alerts that we manually propagate do with certificate validation + Interop.Ssl.SslSetVerifyResult((SafeSslHandle)securityContext!, 28); + // There doesn't seem to be an exposed API for writing an alert, // the API seems to assume that all alerts are generated internally by // SSLHandshake. diff --git a/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs b/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs index f4d79d578c8f2c..3bb93d27a2c02b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs +++ b/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs @@ -34,6 +34,7 @@ internal enum SecurityStatusPalErrorCode Renegotiate, TryAgain, HandshakeStarted, + PeerCertVerifyRequired, // Errors OutOfMemory, diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 09fd43b529bd3e..757fed0c665746 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -358,6 +358,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxDestroy) DllImportEntry(CryptoNative_SslCtxGetData) DllImportEntry(CryptoNative_SslCtxSetAlpnSelectCb) + DllImportEntry(CryptoNative_SslCtxSetCertVerifyCallback) DllImportEntry(CryptoNative_SslCtxSetData) DllImportEntry(CryptoNative_SslCtxSetProtocolOptions) DllImportEntry(CryptoNative_SslCtxSetQuietShutdown) @@ -379,6 +380,8 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetServerName) DllImportEntry(CryptoNative_SslGetSession) DllImportEntry(CryptoNative_SslGetVersion) + DllImportEntry(CryptoNative_SslSetVerifyResult) + DllImportEntry(CryptoNative_SslGetVerifyResult) DllImportEntry(CryptoNative_SslRead) DllImportEntry(CryptoNative_SslSessionFree) DllImportEntry(CryptoNative_SslSessionGetHostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 82023bab1b7af2..edd85c01d52c7e 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -303,6 +303,7 @@ extern bool g_libSslUses32BitTime; #define FOR_ALL_OPENSSL_FUNCTIONS \ REQUIRED_FUNCTION(a2d_ASN1_OBJECT) \ REQUIRED_FUNCTION(ASN1_d2i_bio) \ + REQUIRED_FUNCTION(ASYNC_pause_job) \ REQUIRED_FUNCTION(ASN1_i2d_bio) \ REQUIRED_FUNCTION(ASN1_GENERALIZEDTIME_free) \ REQUIRED_FUNCTION(ASN1_INTEGER_get) \ @@ -323,6 +324,8 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(BIO_gets) \ REQUIRED_FUNCTION(BIO_new) \ REQUIRED_FUNCTION(BIO_new_file) \ + REQUIRED_FUNCTION(BIO_new_fp) \ + REQUIRED_FUNCTION(BIO_printf) \ REQUIRED_FUNCTION(BIO_read) \ FALLBACK_FUNCTION(BIO_up_ref) \ REQUIRED_FUNCTION(BIO_s_mem) \ @@ -689,6 +692,7 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_ctrl) \ REQUIRED_FUNCTION(SSL_add_client_CA) \ REQUIRED_FUNCTION(SSL_set_alpn_protos) \ + REQUIRED_FUNCTION(SSL_set_info_callback) \ REQUIRED_FUNCTION(SSL_set_quiet_shutdown) \ REQUIRED_FUNCTION(SSL_CTX_callback_ctrl) \ REQUIRED_FUNCTION(SSL_CTX_check_private_key) \ @@ -713,6 +717,7 @@ extern bool g_libSslUses32BitTime; FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ REQUIRED_FUNCTION(SSL_CTX_set_session_id_context) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ + REQUIRED_FUNCTION(SSL_CTX_set_cert_verify_callback) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ REQUIRED_FUNCTION(SSL_do_handshake) \ @@ -722,6 +727,7 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_get_current_cipher) \ REQUIRED_FUNCTION(SSL_get_error) \ REQUIRED_FUNCTION(SSL_get_ex_data) \ + REQUIRED_FUNCTION(SSL_get_ex_data_X509_STORE_CTX_idx) \ REQUIRED_FUNCTION(SSL_get_finished) \ REQUIRED_FUNCTION(SSL_get_peer_cert_chain) \ REQUIRED_FUNCTION(SSL_get_peer_finished) \ @@ -735,6 +741,9 @@ extern bool g_libSslUses32BitTime; LEGACY_FUNCTION(SSL_load_error_strings) \ REQUIRED_FUNCTION(SSL_new) \ REQUIRED_FUNCTION(SSL_peek) \ + REQUIRED_FUNCTION(SSL_state_string_long) \ + REQUIRED_FUNCTION(SSL_alert_desc_string_long) \ + REQUIRED_FUNCTION(SSL_alert_type_string_long) \ REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ @@ -753,6 +762,8 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_set_ex_data) \ FALLBACK_FUNCTION(SSL_set_options) \ REQUIRED_FUNCTION(SSL_set_session) \ + REQUIRED_FUNCTION(SSL_set_verify_result) \ + REQUIRED_FUNCTION(SSL_get_verify_result) \ REQUIRED_FUNCTION(SSL_get_session) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ @@ -765,9 +776,12 @@ extern bool g_libSslUses32BitTime; LIGHTUP_FUNCTION(SSL_verify_client_post_handshake) \ LIGHTUP_FUNCTION(SSL_set_post_handshake_auth) \ REQUIRED_FUNCTION(SSL_version) \ + REQUIRED_FUNCTION(SSL_want) \ + REQUIRED_FUNCTION(SSL_trace) \ + REQUIRED_FUNCTION(SSL_set_msg_callback) \ REQUIRED_FUNCTION(UI_create_method) \ REQUIRED_FUNCTION(UI_destroy_method) \ - FALLBACK_FUNCTION(X509_check_host) \ +FALLBACK_FUNCTION(X509_check_host) \ REQUIRED_FUNCTION(X509_check_purpose) \ REQUIRED_FUNCTION(X509_cmp_time) \ REQUIRED_FUNCTION(X509_CRL_free) \ @@ -836,6 +850,7 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(X509_STORE_CTX_set_verify_cb) \ REQUIRED_FUNCTION(X509_STORE_CTX_set_ex_data) \ REQUIRED_FUNCTION(X509_STORE_CTX_get_ex_data) \ + REQUIRED_FUNCTION(X509_STORE_CTX_set_error) \ REQUIRED_FUNCTION(X509_STORE_free) \ FALLBACK_FUNCTION(X509_STORE_get0_param) \ REQUIRED_FUNCTION(X509_STORE_new) \ @@ -875,6 +890,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define a2d_ASN1_OBJECT a2d_ASN1_OBJECT_ptr #define ASN1_GENERALIZEDTIME_free ASN1_GENERALIZEDTIME_free_ptr #define ASN1_d2i_bio ASN1_d2i_bio_ptr +#define ASYNC_pause_job ASYNC_pause_job_ptr #define ASN1_i2d_bio ASN1_i2d_bio_ptr #define ASN1_INTEGER_get ASN1_INTEGER_get_ptr #define ASN1_OBJECT_free ASN1_OBJECT_free_ptr @@ -894,6 +910,8 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define BIO_gets BIO_gets_ptr #define BIO_new BIO_new_ptr #define BIO_new_file BIO_new_file_ptr +#define BIO_new_fp BIO_new_fp_ptr +#define BIO_printf BIO_printf_ptr #define BIO_read BIO_read_ptr #define BIO_up_ref BIO_up_ref_ptr #define BIO_s_mem BIO_s_mem_ptr @@ -1265,6 +1283,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_ctrl SSL_ctrl_ptr #define SSL_add_client_CA SSL_add_client_CA_ptr #define SSL_set_alpn_protos SSL_set_alpn_protos_ptr +#define SSL_set_info_callback SSL_set_info_callback_ptr #define SSL_set_quiet_shutdown SSL_set_quiet_shutdown_ptr #define SSL_CTX_callback_ctrl SSL_CTX_callback_ctrl_ptr #define SSL_CTX_check_private_key SSL_CTX_check_private_key_ptr @@ -1288,6 +1307,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_CTX_set_security_level SSL_CTX_set_security_level_ptr #define SSL_CTX_set_session_id_context SSL_CTX_set_session_id_context_ptr #define SSL_CTX_set_verify SSL_CTX_set_verify_ptr +#define SSL_CTX_set_cert_verify_callback SSL_CTX_set_cert_verify_callback_ptr #define SSL_CTX_use_certificate SSL_CTX_use_certificate_ptr #define SSL_CTX_use_PrivateKey SSL_CTX_use_PrivateKey_ptr #define SSL_do_handshake SSL_do_handshake_ptr @@ -1298,6 +1318,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_get_current_cipher SSL_get_current_cipher_ptr #define SSL_get_error SSL_get_error_ptr #define SSL_get_ex_data SSL_get_ex_data_ptr +#define SSL_get_ex_data_X509_STORE_CTX_idx SSL_get_ex_data_X509_STORE_CTX_idx_ptr #define SSL_get_finished SSL_get_finished_ptr #define SSL_get_peer_cert_chain SSL_get_peer_cert_chain_ptr #define SSL_get_peer_finished SSL_get_peer_finished_ptr @@ -1312,6 +1333,8 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_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_alert_type_string_long SSL_alert_type_string_long_ptr +#define SSL_alert_desc_string_long SSL_alert_desc_string_long_ptr #define SSL_read SSL_read_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr @@ -1330,6 +1353,8 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_set_ex_data SSL_set_ex_data_ptr #define SSL_set_options SSL_set_options_ptr #define SSL_set_session SSL_set_session_ptr +#define SSL_set_verify_result SSL_set_verify_result_ptr +#define SSL_get_verify_result SSL_get_verify_result_ptr #define SSL_get_session SSL_get_session_ptr #define SSL_set_verify SSL_set_verify_ptr #define SSL_shutdown SSL_shutdown_ptr @@ -1341,6 +1366,9 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_verify_client_post_handshake SSL_verify_client_post_handshake_ptr #define SSL_set_post_handshake_auth SSL_set_post_handshake_auth_ptr #define SSL_version SSL_version_ptr +#define SSL_want SSL_want_ptr +#define SSL_trace SSL_trace_ptr +#define SSL_set_msg_callback SSL_set_msg_callback_ptr #define TLS_method TLS_method_ptr #define UI_create_method UI_create_method_ptr #define UI_destroy_method UI_destroy_method_ptr @@ -1413,6 +1441,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define X509_STORE_CTX_set_verify_cb X509_STORE_CTX_set_verify_cb_ptr #define X509_STORE_CTX_set_ex_data X509_STORE_CTX_set_ex_data_ptr #define X509_STORE_CTX_get_ex_data X509_STORE_CTX_get_ex_data_ptr +#define X509_STORE_CTX_set_error X509_STORE_CTX_set_error_ptr #define X509_STORE_free X509_STORE_free_ptr #define X509_STORE_get0_param X509_STORE_get0_param_ptr #define X509_STORE_new X509_STORE_new_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 93d352702748f6..cf44c7b8d1bcc2 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -214,6 +214,7 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) // The other .NET platforms are server-preference, and the common consensus seems // to be to use server preference (as of June 2020), so just always assert that. SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION | SSL_OP_CIPHER_SERVER_PREFERENCE); + SSL_CTX_set_mode(ctx, SSL_MODE_ASYNC); #ifdef NEED_OPENSSL_3_0 if (CryptoNative_OpenSslVersionNumber() >= OPENSSL_VERSION_3_0_RTM) @@ -396,10 +397,54 @@ void CryptoNative_SslCtxDestroy(SSL_CTX* ctx) } } +static BIO* bio_stdout = NULL; + +static void apps_ssl_info_callback(const SSL *s, int where, int ret) +{ + const char *str; + int w = where & ~SSL_ST_MASK; + + if (w & SSL_ST_CONNECT) + str = "SSL_connect"; + else if (w & SSL_ST_ACCEPT) + str = "SSL_accept"; + else + str = "undefined"; + + if (where & SSL_CB_LOOP) { + BIO_printf(bio_stdout, "%s:%s\n", str, SSL_state_string_long(s)); + } else if (where & SSL_CB_ALERT) { + str = (where & SSL_CB_READ) ? "read" : "write"; + BIO_printf(bio_stdout, "SSL3 alert %s:%s:%s\n", str, + SSL_alert_type_string_long(ret), + SSL_alert_desc_string_long(ret)); + } else if (where & SSL_CB_EXIT) { + if (ret == 0) { + BIO_printf(bio_stdout, "%s:failed in %s\n", + str, SSL_state_string_long(s)); + } else if (ret < 0) { + BIO_printf(bio_stdout, "%s:error in %s\n", + str, SSL_state_string_long(s)); + } + } +} + void CryptoNative_SslSetConnectState(SSL* ssl) { // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_connect_state(ssl); + // SSL_set_msg_callback(ssl, SSL_trace); + // if (bio_stdout == NULL) + // { + // bio_stdout = BIO_new_fp(stdout, BIO_NOCLOSE); + // if (bio_stdout == NULL) + // { + // ERR_clear_error(); + // return; + // } + // } + // SSL_set_msg_callback_arg(ssl, bio_stdout); + // SSL_set_info_callback(ssl, apps_ssl_info_callback); } void CryptoNative_SslSetAcceptState(SSL* ssl) @@ -479,6 +524,7 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX* store) { (void)preverify_ok; (void)store; + printf("Store: %p, preverify_ok: %d\n", (void*)store, preverify_ok); // We don't care. Real verification happens in managed code. return 1; } @@ -514,7 +560,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) int pending = SSL_renegotiate_pending(ssl); if (!pending) { - SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); + CryptoNative_SslSetVerifyPeer(ssl); int ret = SSL_renegotiate(ssl); if(ret != 1) { @@ -657,7 +703,7 @@ X509NameStack* CryptoNative_SslGetClientCAList(SSL* ssl) void CryptoNative_SslSetVerifyPeer(SSL* ssl) { // void shim functions don't lead to exceptions, so skip the unconditional error clearing. - SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); + SSL_set_verify(ssl, SSL_VERIFY_PEER, NULL); } int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, uint8_t* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) @@ -1307,3 +1353,49 @@ void CryptoNative_SslStapleOcsp(SSL* ssl, uint8_t* buf, int32_t len) OPENSSL_free(copy); } } + +static int CertVerifyCallback(X509_STORE_CTX* store, void* param) +{ + (void)param; + // SslCtxCertValidationCallback callback = (SslCtxCertValidationCallback) param; + SSL *ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()); + + printf("Pausing job\n"); + ASYNC_pause_job(); + printf("Resumed job\n"); + + int verifyResult = (int)SSL_get_verify_result(ssl); + printf("SSL_get_verify_result(%p) == %d\n", (void*) ssl, verifyResult); + if (verifyResult < 0) + { + int ret = SSL_set_retry_verify(ssl); + printf("SSL_set_retry_verify(%p) == %d\n", (void*) ssl, ret); + return ret; + } + + X509_STORE_CTX_set_error(store, verifyResult); + return verifyResult == X509_V_OK; +} + +void CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxCertValidationCallback callback) +{ + if (ctx != NULL) + { + SSL_CTX_set_cert_verify_callback(ctx, CertVerifyCallback, (void*)callback); + } +} + +void CryptoNative_SslSetVerifyResult(SSL* ssl, int64_t verifyResult) +{ + (void)ssl; + (void)verifyResult; + SSL_set_verify_result(ssl, verifyResult); +} + +/* +Shims the SSL_get_verify_result method. +*/ +int64_t CryptoNative_SslGetVerifyResult(SSL* ssl) +{ + return SSL_get_verify_result(ssl); +} diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 8566c7b8ff9b35..f516ab08720b16 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -131,6 +131,9 @@ typedef void (*SslCtxRemoveSessionCallback)(SSL_CTX* ctx, SSL_SESSION* session); // the function pointer for keylog typedef void (*SslCtxSetKeylogCallback)(const SSL* ssl, const char *line); +// the function for remote certificate validation.. +typedef int32_t (*SslCtxCertValidationCallback)(SSL* ssl, X509_STORE_CTX* store); + /* Ensures that libssl is correctly initialized and ready to use. */ @@ -363,8 +366,6 @@ Returns 1 upon success, otherwise 0. */ PALEXPORT int32_t CryptoNative_SslUsePrivateKey(SSL* ssl, EVP_PKEY* pkey); - - /* Shims the SSL_CTX_use_certificate method. @@ -542,3 +543,18 @@ PALEXPORT int32_t CryptoNative_OpenSslGetProtocolSupport(SslProtocols protocol); Staples an encoded OCSP response onto the TLS session */ PALEXPORT void CryptoNative_SslStapleOcsp(SSL* ssl, uint8_t* buf, int32_t len); + +/* +Sets the certificate verification callback for the SSL_CTX. +*/ +PALEXPORT void CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxCertValidationCallback callback); + +/* +Shims the SSL_set_verify_result method. +*/ +PALEXPORT void CryptoNative_SslSetVerifyResult(SSL* ssl, int64_t verifyResult); + +/* +Shims the SSL_get_verify_result method. +*/ +PALEXPORT int64_t CryptoNative_SslGetVerifyResult(SSL* ssl); From b84be9d68277251af2212a3d604fb301c6856433 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 4 Jun 2025 09:28:15 +0200 Subject: [PATCH 02/16] WIP --- .../Interop.OpenSsl.cs | 14 ----- .../Interop.Ssl.cs | 9 --- .../tests/Functional/SmtpClientTest.cs | 44 +++++++++++++++ .../src/System/Net/Security/SslStream.IO.cs | 55 +++++++++---------- .../System/Net/Security/SslStream.Protocol.cs | 42 ++++---------- .../System/Net/Security/SslStreamPal.Unix.cs | 13 ----- .../src/System/Net/SecurityStatusPal.cs | 1 - .../pal_ssl.c | 15 +---- 8 files changed, 84 insertions(+), 109 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 13c33e2eefbd40..5a55430b226753 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 @@ -439,9 +439,6 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth // Client side always verifies the server's certificate. Ssl.SslSetVerifyPeer(sslHandle); - // HACK: set a bogus code to indicate that we did not perform the validation yet - // Ssl.SslSetVerifyResult(sslHandle, -1); - if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !IPAddress.IsValid(sslAuthenticationOptions.TargetHost)) { // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. @@ -474,8 +471,6 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth if (sslAuthenticationOptions.RemoteCertRequired) { Ssl.SslSetVerifyPeer(sslHandle); - // HACK: set a bogus code to indicate that we did not perform the validation yet - Ssl.SslSetVerifyResult(sslHandle, -1); } if (sslAuthenticationOptions.CertificateContext != null) @@ -552,8 +547,6 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, } } -#pragma warning disable CS0618 - System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_do_handshake"); int retVal = Ssl.SslDoHandshake(context, out Ssl.SslErrorCode errorCode); if (retVal != 1) { @@ -562,13 +555,6 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, return SecurityStatusPalErrorCode.CredentialsNeeded; } - if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_ASYNC) - // if (errorCode == Ssl.SslErrorCode.SSL_ERROR_WANT_RETRY_VERIFY) - { - System.Console.WriteLine($"[{AppDomain.GetCurrentThreadId()}] SSL_ERROR_WANT_ASYNC"); - return SecurityStatusPalErrorCode.PeerCertVerifyRequired; - } - if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ)) { Exception? innerError = GetSslError(retVal, errorCode); 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 fa56c48e390d4f..65c0d37e3d5a30 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 @@ -204,12 +204,6 @@ internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl) [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetData")] internal static partial void SslSessionSetData(IntPtr session, IntPtr val); - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyResult")] - internal static partial void SslSetVerifyResult(SafeSslHandle ssl, long verifyResult); - - [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetVerifyResult")] - internal static partial long SslGetVerifyResult(SafeSslHandle ssl); - internal static class Capabilities { // needs separate type (separate static cctor) to be sure OpenSSL is initialized. @@ -346,9 +340,6 @@ internal enum SslErrorCode SSL_ERROR_SYSCALL = 5, SSL_ERROR_ZERO_RETURN = 6, - SSL_ERROR_WANT_ASYNC = 9, - SSL_ERROR_WANT_RETRY_VERIFY = 12, - // NOTE: this SslErrorCode value doesn't exist in OpenSSL, but // we use it to distinguish when a renegotiation is pending. // Choosing an arbitrarily large value that shouldn't conflict diff --git a/src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs b/src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs index a3b25b27d790a9..b9a53267012b95 100644 --- a/src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs +++ b/src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs @@ -9,6 +9,7 @@ // (C) 2006 John Luke // +using System.ComponentModel; using System.Collections.Generic; using System.Globalization; using System.IO; @@ -338,6 +339,49 @@ public async Task SendMailAsync_CanBeCanceled_CancellationToken() Assert.Equal(GetClientDomain(), server.ClientDomain); } + [Fact] + public async Task SendAsync_CanBeCanceled_SendAsyncCancel() + { + using var server = new LoopbackSmtpServer(_output); + using SmtpClient client = server.CreateClient(); + + server.ReceiveMultipleConnections = true; + + bool first = true; + + server.OnConnected += _ => + { + if (first) + { + first = false; + client.SendAsyncCancel(); + } + }; + + var message = new MailMessage("foo@internet.com", "bar@internet.com", "Foo", "Bar"); + + TaskCompletionSource tcs = new TaskCompletionSource(); + client.SendCompleted += (s, e) => + { + tcs.SetResult(e); + }; + + client.SendAsync(message, null); + AsyncCompletedEventArgs e = await tcs.Task.WaitAsync(TestHelper.PassingTestTimeout); + Assert.True(e.Cancelled, "SendAsync should have been canceled"); + _output.WriteLine(e.Error?.ToString() ?? "No error"); + Assert.IsAssignableFrom(e.Error.InnerException); + + // We should still be able to send mail on the SmtpClient instance + await client.SendMailAsync(message).WaitAsync(TestHelper.PassingTestTimeout); + + Assert.Equal("", server.MailFrom); + Assert.Equal("", Assert.Single(server.MailTo)); + Assert.Equal("Foo", server.Message.Subject); + Assert.Equal("Bar", server.Message.Body); + Assert.Equal(GetClientDomain(), server.ClientDomain); + } + private static string GetClientDomain() => IPGlobalProperties.GetIPGlobalProperties().HostName.Trim().ToLower(); } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 6dc57a5a8769ca..eac06538b92119 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -297,8 +297,7 @@ private async Task ForceAuthenticationAsync(bool receiveFirst, byte[ { if (!receiveFirst) { - int consumed; - (token, consumed) = await NextMessage(reAuthenticationData).ConfigureAwait(false); + token = NextMessage(reAuthenticationData, out int consumed); Debug.Assert(consumed == (reAuthenticationData?.Length ?? 0)); if (token.Size > 0) @@ -490,14 +489,14 @@ private ProtocolToken ProcessTlsFrame(int frameSize) { int chunkSize = frameSize; - ReadOnlyMemory availableData = _buffer.EncryptedReadOnlyMemory; + ReadOnlySpan availableData = _buffer.EncryptedReadOnlySpan; // Often more TLS messages fit into same packet. Get as many complete frames as we can. while (_buffer.EncryptedLength - chunkSize > TlsFrameHelper.HeaderSize) { TlsFrameHeader nextHeader = default; - if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize).Span, ref nextHeader)) + if (!TlsFrameHelper.TryGetFrameHeader(availableData.Slice(chunkSize), ref nextHeader)) { break; } @@ -515,7 +514,7 @@ private ProtocolToken ProcessTlsFrame(int frameSize) chunkSize += frameSize; } - (ProtocolToken token, int consumed) = NextMessage(availableData.Slice(0, chunkSize)).GetAwaiter().GetResult(); + ProtocolToken token = NextMessage(availableData.Slice(0, chunkSize), out int consumed); _buffer.DiscardEncrypted(consumed); return token; } @@ -524,15 +523,20 @@ private ProtocolToken ProcessTlsFrame(int frameSize) // This is to reset auth state on remote side. // If this write succeeds we will allow auth retrying. // - private void SendAuthResetAndThrow(ReadOnlySpan alert, ExceptionDispatchInfo exception) + private void SendAuthResetSignal(ReadOnlySpan alert, ExceptionDispatchInfo exception) { SetException(exception.SourceException); - if (alert.Length >= 0) + if (alert.Length == 0) { - InnerStream.Write(alert); + // + // We don't have an alert to send so cannot retry and fail prematurely. + // + exception.Throw(); } + InnerStream.Write(alert); + exception.Throw(); } @@ -587,26 +591,21 @@ private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions ProtocolToken alertToken = default; if (!CompleteHandshake(ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) { - ProcessFailedCertificateValidation(sslAuthenticationOptions, ref alertToken, sslPolicyErrors, chainStatus); - } - } - - private void ProcessFailedCertificateValidation(SslAuthenticationOptions sslAuthenticationOptions, ref ProtocolToken alertToken, SslPolicyErrors sslPolicyErrors, 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. - SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), 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. - SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); - } - else - { - // Simple add sslPolicyErrors as crude info. - SendAuthResetAndThrow(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); + 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(new ReadOnlySpan(alertToken.Payload), 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(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); + } + else + { + // Simple add sslPolicyErrors as crude info. + SendAuthResetSignal(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 8db6358d22bb09..1e0a8ed0e2a756 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -11,7 +11,6 @@ using System.Security.Authentication.ExtendedProtection; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; -using System.Threading.Tasks; namespace System.Net.Security { @@ -814,10 +813,9 @@ static DateTime GetExpiryTimestamp(SslStreamCertificateContext certificateContex } // - internal async Task<(ProtocolToken, int)> NextMessage(ReadOnlyMemory incomingBuffer) + internal ProtocolToken NextMessage(ReadOnlySpan incomingBuffer, out int consumed) { - (ProtocolToken token, int consumed) = await GenerateToken(incomingBuffer).ConfigureAwait(false); - + ProtocolToken token = GenerateToken(incomingBuffer, out consumed); if (NetEventSource.Log.IsEnabled()) { if (token.Failed) @@ -826,7 +824,7 @@ static DateTime GetExpiryTimestamp(SslStreamCertificateContext certificateContex } } - return (token, consumed); + return token; } /*++ @@ -842,7 +840,7 @@ generates a set of bytes that will be sent next to Return: token - ProtocolToken with status and optionally buffer. --*/ - private async Task<(ProtocolToken, int)> GenerateToken(ReadOnlyMemory inputBuffer) + private ProtocolToken GenerateToken(ReadOnlySpan inputBuffer, out int consumed) { bool cachedCreds = false; bool sendTrustList = false; @@ -851,9 +849,6 @@ generates a set of bytes that will be sent next to ProtocolToken token = default; token.RentBuffer = true; - int consumed = 0; - int tmpConsumed; - // We need to try get credentials at the beginning. // _credentialsHandle may be always null on some platforms but // _securityContext will be allocated on first call. @@ -866,7 +861,6 @@ generates a set of bytes that will be sent next to { do { - retry: thumbPrint = null; if (refreshCredentialNeeded) { @@ -882,8 +876,8 @@ generates a set of bytes that will be sent next to token = SslStreamPal.AcceptSecurityContext( ref _credentialsHandle!, ref _securityContext, - inputBuffer.Span, - out tmpConsumed, + inputBuffer, + out consumed, _sslAuthenticationOptions); if (token.Status.ErrorCode == SecurityStatusPalErrorCode.HandshakeStarted) { @@ -911,8 +905,8 @@ generates a set of bytes that will be sent next to ref _credentialsHandle!, ref _securityContext, hostName, - inputBuffer.Span, - out tmpConsumed, + inputBuffer, + out consumed, _sslAuthenticationOptions); if (token.Status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded) @@ -932,20 +926,6 @@ generates a set of bytes that will be sent next to _sslAuthenticationOptions); } } - consumed += tmpConsumed; - inputBuffer = inputBuffer.Slice(tmpConsumed); - - if (token.Status.ErrorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired) - { - await Task.Yield(); - if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertValidationDelegate, _sslAuthenticationOptions.CertificateContext?.Trust, ref token, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) - { - ProcessFailedCertificateValidation(_sslAuthenticationOptions, ref token, sslPolicyErrors, chainStatus); - } - - goto retry; - - } } while (cachedCreds && _credentialsHandle == null); } finally @@ -977,7 +957,7 @@ generates a set of bytes that will be sent next to } } - return (token, consumed); + return token; } internal ProtocolToken Renegotiate() @@ -1236,12 +1216,12 @@ private ProtocolToken CreateShutdownToken() return default; } - return GenerateToken(default).GetAwaiter().GetResult().Item1; + return GenerateToken(default, out _); } private ProtocolToken GenerateAlertToken() { - return GenerateToken(default).GetAwaiter().GetResult().Item1; + return GenerateToken(default, out _); } private static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain) 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 d74a47bfd66a37..700531aa49d35d 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 @@ -207,16 +207,6 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); } - if (errorCode == SecurityStatusPalErrorCode.PeerCertVerifyRequired) - { - token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.PeerCertVerifyRequired); - return token; - // System.Console.WriteLine($"Peer certificate verification required. setting VerifyResult to an error"); - // Interop.Ssl.SslSetVerifyResult((SafeSslHandle)context, 0); - // // continue with the handshake, the callback will be invoked later. - // errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); - } - // sometimes during renegotiation processing message 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 (token.Size == 0 && Interop.Ssl.IsSslRenegotiatePending((SafeSslHandle)context)) @@ -249,9 +239,6 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context public static SecurityStatusPal ApplyAlertToken(SafeDeleteContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage) { - // All the alerts that we manually propagate do with certificate validation - Interop.Ssl.SslSetVerifyResult((SafeSslHandle)securityContext!, 28); - // There doesn't seem to be an exposed API for writing an alert, // the API seems to assume that all alerts are generated internally by // SSLHandshake. diff --git a/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs b/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs index 3bb93d27a2c02b..f4d79d578c8f2c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs +++ b/src/libraries/System.Net.Security/src/System/Net/SecurityStatusPal.cs @@ -34,7 +34,6 @@ internal enum SecurityStatusPalErrorCode Renegotiate, TryAgain, HandshakeStarted, - PeerCertVerifyRequired, // Errors OutOfMemory, diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index cf44c7b8d1bcc2..c220954aaafaee 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -1357,21 +1357,10 @@ void CryptoNative_SslStapleOcsp(SSL* ssl, uint8_t* buf, int32_t len) static int CertVerifyCallback(X509_STORE_CTX* store, void* param) { (void)param; - // SslCtxCertValidationCallback callback = (SslCtxCertValidationCallback) param; + SslCtxCertValidationCallback callback = (SslCtxCertValidationCallback) param; SSL *ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()); - printf("Pausing job\n"); - ASYNC_pause_job(); - printf("Resumed job\n"); - - int verifyResult = (int)SSL_get_verify_result(ssl); - printf("SSL_get_verify_result(%p) == %d\n", (void*) ssl, verifyResult); - if (verifyResult < 0) - { - int ret = SSL_set_retry_verify(ssl); - printf("SSL_set_retry_verify(%p) == %d\n", (void*) ssl, ret); - return ret; - } + int verifyResult = callback(ssl, store); X509_STORE_CTX_set_error(store, verifyResult); return verifyResult == X509_V_OK; From e4e866a309d1fe4b9a9af07b5e82638641cee47e Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 24 Jun 2025 15:13:14 +0200 Subject: [PATCH 03/16] revert unneeded change --- .../src/System/Net/CertificateValidationPal.Unix.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs index 360123ca0e0f08..6fda4835d4ae2e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs @@ -35,18 +35,8 @@ internal static SslPolicyErrors VerifyCertificateProperties( return null; } - IntPtr remoteCertificate = IntPtr.Zero; - { - using var chainStack = Interop.OpenSsl.GetPeerCertificateChain((SafeSslHandle)securityContext); - int count = Interop.Crypto.GetX509StackFieldCount(chainStack); - if (count > 0) - { - remoteCertificate = Interop.Crypto.GetX509StackField(chainStack, 0); - } - } - X509Certificate2? result = null; - + IntPtr remoteCertificate = Interop.OpenSsl.GetPeerCertificate((SafeSslHandle)securityContext); try { if (remoteCertificate == IntPtr.Zero) From d5568d02ce28b2ace563d93fe34151764dc41b22 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 24 Jun 2025 15:13:43 +0200 Subject: [PATCH 04/16] WIP --- .../Interop.OpenSsl.cs | 155 +++++++++++++----- .../Interop.Ssl.cs | 9 +- .../Interop.SslCtx.cs | 16 -- .../Net/Security/SslAuthenticationOptions.cs | 4 + .../System/Net/Security/SslStream.Protocol.cs | 4 +- .../src/System/Net/Security/SslStream.cs | 4 + .../System/Net/Security/SslStreamPal.Unix.cs | 3 +- .../entrypoints.c | 1 + .../pal_ssl.c | 10 +- .../pal_ssl.h | 5 + 10 files changed, 146 insertions(+), 65 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 f9e1cdd41cd7e6..712b8e0d6e06ed 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 @@ -216,7 +216,7 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication throw CreateSslException(SR.net_allocate_ssl_context_failed); } - Ssl.SslCtxSetCertVerifyCallback(sslCtx, &Ssl.CertVerifyCallback); + Ssl.SslCtxSetCertVerifyCallback(sslCtx, &CertVerifyCallback); Ssl.SslCtxSetProtocolOptions(sslCtx, protocols); @@ -389,7 +389,7 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth // Dispose() here will not close the handle. using SafeSslContextHandle sslCtxHandle = GetOrCreateSslContextHandle(sslAuthenticationOptions, cacheSslContext); - GCHandle alpnHandle = default; + GCHandle authOptionsHandle = default; try { sslHandle = SafeSslHandle.Create(sslCtxHandle, sslAuthenticationOptions.IsServer); @@ -419,6 +419,10 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth sslHandle.SslContextHandle = sslCtxHandle; } + authOptionsHandle = GCHandle.Alloc(sslAuthenticationOptions); + Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(authOptionsHandle)); + sslHandle.AuthOptionsHandle = authOptionsHandle; + if (!sslAuthenticationOptions.AllowRsaPssPadding || !sslAuthenticationOptions.AllowRsaPkcs1Padding) { ConfigureSignatureAlgorithms(sslHandle, sslAuthenticationOptions.AllowRsaPssPadding, sslAuthenticationOptions.AllowRsaPkcs1Padding); @@ -426,14 +430,7 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { - if (sslAuthenticationOptions.IsServer) - { - Debug.Assert(Interop.Ssl.SslGetData(sslHandle) == IntPtr.Zero); - alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); - Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(alpnHandle)); - sslHandle.AlpnHandle = alpnHandle; - } - else + if (!sslAuthenticationOptions.IsServer) { if (Interop.Ssl.SslSetAlpnProtos(sslHandle, sslAuthenticationOptions.ApplicationProtocols) != 0) { @@ -518,9 +515,9 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth } catch { - if (alpnHandle.IsAllocated) + if (authOptionsHandle.IsAllocated) { - alpnHandle.Free(); + authOptionsHandle.Free(); } throw; @@ -862,18 +859,97 @@ private static void QueryUniqueChannelBinding(SafeSslHandle context, SafeChannel bindingHandle.SetCertHashLength(certHashLength); } -#pragma warning disable IDE0060 [UnmanagedCallersOnly] - private static int VerifyClientCertificate(int preverify_ok, IntPtr x509_ctx_ptr) + internal static int CertVerifyCallback(IntPtr ssl, IntPtr store) { - // Full validation is handled after the handshake in VerifyCertificateProperties and the - // user callback. It's also up to those handlers to decide if a null certificate - // is appropriate. So just return success to tell OpenSSL that the cert is acceptable, - // we'll process it after the handshake finishes. - const int OpenSslSuccess = 1; - return OpenSslSuccess; + IntPtr data = Ssl.SslGetData(ssl); + Debug.Assert(data != IntPtr.Zero, "Expected non-null data pointer from SslGetData"); + GCHandle gch = GCHandle.FromIntPtr(data); + SslAuthenticationOptions options = (SslAuthenticationOptions)gch.Target!; + + using SafeX509StoreCtxHandle storeHandle = new(store, ownsHandle: false); + + X509Chain chain = new X509Chain(); + X509Certificate2? certificate = null; + + using (SafeSharedX509StackHandle chainStack = Interop.Crypto.X509StoreCtxGetSharedUntrusted(storeHandle)) + { + if (!chainStack.IsInvalid) + { + int count = Interop.Crypto.GetX509StackFieldCount(chainStack); + + for (int i = 0; i < count; i++) + { + IntPtr certPtr = Interop.Crypto.GetX509StackField(chainStack, i); + + if (certPtr != IntPtr.Zero) + { + // X509Certificate2(IntPtr) calls X509_dup, so the reference is appropriately tracked. + X509Certificate2 chainCert = new X509Certificate2(certPtr); + chain.ChainPolicy.ExtraStore.Add(chainCert); + + // first cert in the stack is the leaf cert + certificate ??= chainCert; + } + } + } + } + + Debug.Assert(certificate != null, "Certificate should not be null here."); + + SslCertificateTrust? trust = options.CertificateContext?.Trust; + SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None; + + if (options.CertificateChainPolicy != null) + { + chain.ChainPolicy = options.CertificateChainPolicy; + } + else + { + chain.ChainPolicy.RevocationMode = options.CertificateRevocationCheckMode; + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; + + if (trust != null) + { + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + if (trust._store != null) + { + chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); + } + if (trust._trustList != null) + { + chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); + } + } + } + + // set ApplicationPolicy unless already provided. + if (chain.ChainPolicy.ApplicationPolicy.Count == 0) + { + // Authenticate the remote party: (e.g. when operating in server mode, authenticate the client). + chain.ChainPolicy.ApplicationPolicy.Add(options.IsServer ? SslStream.s_clientAuthOid : SslStream.s_serverAuthOid); + } + + sslPolicyErrors |= CertificateValidation.BuildChainAndVerifyProperties( + chain, + certificate, + options.CheckCertName, + options.IsServer, + TargetHostNameHelper.NormalizeHostName(options.TargetHost)); + + bool success = sslPolicyErrors == SslPolicyErrors.None; + + if (options.CertValidationDelegate != null) + { + success = options.CertValidationDelegate( + options.SslStream!, + certificate, + chain, + sslPolicyErrors); + } + + return success ? 0 : 27 /* BAD_CERT */; } -#pragma warning restore IDE0060 [UnmanagedCallersOnly] private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte* outlen, byte* inp, uint inlen, IntPtr arg) @@ -887,8 +963,8 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; } - GCHandle protocolHandle = GCHandle.FromIntPtr(sslData); - if (!(protocolHandle.Target is List protocolList)) + GCHandle authOptionsHandle = GCHandle.FromIntPtr(sslData); + if (!((authOptionsHandle.Target as SslAuthenticationOptions)?.ApplicationProtocols is List protocolList)) { return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -915,17 +991,9 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte } catch { - // No common application protocol was negotiated, set the target on the alpnHandle to null. - // It is ok to clear the handle value here, this results in handshake failure, so the SslStream object is disposed. - protocolHandle.Target = null; - - return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; } - // No common application protocol was negotiated, set the target on the alpnHandle to null. - // It is ok to clear the handle value here, this results in handshake failure, so the SslStream object is disposed. - protocolHandle.Target = null; - + // No common application protocol was negotiated return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; } @@ -946,20 +1014,25 @@ private static unsafe int NewSessionCallback(IntPtr ssl, IntPtr session) IntPtr cert = Interop.Ssl.SslGetCertificate(ssl); Interop.Ssl.SslSessionSetData(session, cert); - IntPtr ptr = Ssl.SslGetData(ssl); + IntPtr ctx = Ssl.SslGetSslCtx(ssl); + IntPtr ptr = Ssl.SslCtxGetData(ctx); + // while SSL_CTX is kept alive by reference from SSL, the same is not true + // for the stored GCHandle pointing to SafeSslContextHandle, which if (ptr != IntPtr.Zero) { GCHandle gch = GCHandle.FromIntPtr(ptr); - IntPtr name = Ssl.SslGetServerName(ssl); - Debug.Assert(name != IntPtr.Zero); - SafeSslContextHandle? ctxHandle = gch.Target as SafeSslContextHandle; - // There is no relation between SafeSslContextHandle and SafeSslHandle so the handle - // may be released while the ssl session is still active. - if (ctxHandle != null && ctxHandle.TryAddSession(name, session)) + + if (ctxHandle != null) { - // offered session was stored in our cache. - return 1; + IntPtr name = Ssl.SslGetServerName(ssl); + Debug.Assert(name != IntPtr.Zero); + + if (ctxHandle.TryAddSession(name, session)) + { + // offered session was stored in our cache. + return 1; + } } } 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 cdac694697f017..5d3bd4e7913304 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 @@ -228,6 +228,9 @@ internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl) [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetData")] internal static partial void SslSessionSetData(IntPtr session, IntPtr val); + [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetSslCtx")] + internal static partial IntPtr SslGetSslCtx(IntPtr ssl); + internal static class Capabilities { // needs separate type (separate static cctor) to be sure OpenSSL is initialized. @@ -382,7 +385,7 @@ internal sealed class SafeSslHandle : SafeDeleteSslContext private bool _isServer; private bool _handshakeCompleted; - public GCHandle AlpnHandle; + public GCHandle AuthOptionsHandle; // Reference to the parent SSL_CTX handle in the SSL_CTX is being cached. Only used for // refcount management. public SafeSslContextHandle? SslContextHandle; @@ -480,10 +483,10 @@ protected override bool ReleaseHandle() SslContextHandle?.Dispose(); - if (AlpnHandle.IsAllocated) + if (AuthOptionsHandle.IsAllocated) { Interop.Ssl.SslSetData(handle, IntPtr.Zero); - AlpnHandle.Free(); + AuthOptionsHandle.Free(); } IntPtr h = handle; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs index 9511b361da4ec1..014eda084874b5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs @@ -65,17 +65,6 @@ internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, ReadOnl [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCertVerifyCallback")] internal static unsafe partial void SslCtxSetCertVerifyCallback(SafeSslContextHandle ctx, delegate* unmanaged callback); - - [UnmanagedCallersOnly] - internal static int CertVerifyCallback(IntPtr ssl, IntPtr store) - { - System.Console.WriteLine($"CertVerifyCallback called with store: {store:x8}, ssl: {ssl:x8}"); - IntPtr data = Ssl.SslGetData(ssl); - System.Console.WriteLine($"SSL data: {data:x8}"); - GCHandle gch = GCHandle.FromIntPtr(data); - System.Console.WriteLine($"SSL data handle: {gch.Target}"); - return 0; - } } } @@ -267,11 +256,6 @@ internal bool TrySetSession(SafeSslHandle sslHandle, string name) return false; } - // even if we don't have matching session, we can get new one and we need - // way how to link SSL back to `this`. - Debug.Assert(Interop.Ssl.SslGetData(sslHandle) == IntPtr.Zero); - Interop.Ssl.SslSetData(sslHandle, (IntPtr)_gch); - lock (_sslSessions) { if (_sslSessions.TryGetValue(name, out IntPtr session)) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs index 0236ff306152bd..b729bbfef8032e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs @@ -221,5 +221,9 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto #if TARGET_ANDROID internal SslStream.JavaProxy? SslStreamProxy { get; set; } #endif + +#if !TARGET_WINDOWS && !SYSNETSECURITY_NO_OPENSSL + internal SslStream? SslStream { get; set; } +#endif } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 4e9f4327ee9e22..b291ecab293f5e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -65,8 +65,8 @@ internal static bool DisableTlsResume private int _trailerSize = 16; private int _maxDataSize = 16354; - private static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); - private static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2"); + internal static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); + internal static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2"); // // Protocol properties 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 04ddca13d38207..51a16fbff94862 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 @@ -221,6 +221,10 @@ public SslStream(Stream innerStream, bool leaveInnerStreamOpen, RemoteCertificat _sslAuthenticationOptions.SslStreamProxy = new SslStream.JavaProxy(sslStream: this); #endif +#if !TARGET_WINDOWS && !SYSNETSECURITY_NO_OPENSSL + _sslAuthenticationOptions.SslStream = this; +#endif + if (NetEventSource.Log.IsEnabled()) NetEventSource.Log.SslStreamCtor(this, innerStream); } 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 700531aa49d35d..ccceda69097d15 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 @@ -220,8 +220,7 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context // whereas ALPN is supported from version 1.0.2. SafeSslHandle sslContext = (SafeSslHandle)context; if (errorCode == SecurityStatusPalErrorCode.OK && sslAuthenticationOptions.IsServer - && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0 - && sslContext.AlpnHandle.IsAllocated && sslContext.AlpnHandle.Target == null) + && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, Interop.OpenSsl.CreateSslException(SR.net_alpn_failed)); return token; diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index dd53bb638b4bec..d2fcef85530b43 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -373,6 +373,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetClientCAList) DllImportEntry(CryptoNative_SslGetCurrentCipherId) DllImportEntry(CryptoNative_SslGetData) + DllImportEntry(CryptoNative_SslGetSslCtx) DllImportEntry(CryptoNative_SslGetError) DllImportEntry(CryptoNative_SslGetFinished) DllImportEntry(CryptoNative_SslGetPeerCertChain) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 7df8469bb1d4aa..55fe856394a08a 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -214,7 +214,6 @@ SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) // The other .NET platforms are server-preference, and the common consensus seems // to be to use server preference (as of June 2020), so just always assert that. SSL_CTX_set_options(ctx, SSL_OP_NO_COMPRESSION | SSL_OP_CIPHER_SERVER_PREFERENCE); - SSL_CTX_set_mode(ctx, SSL_MODE_ASYNC); #ifdef NEED_OPENSSL_3_0 if (CryptoNative_OpenSslVersionNumber() >= OPENSSL_VERSION_3_0_RTM) @@ -1486,3 +1485,12 @@ int64_t CryptoNative_SslGetVerifyResult(SSL* ssl) { return SSL_get_verify_result(ssl); } + +/* +Shims SSL_get_SSL_CTX to retrieve the SSL_CTX from the SSL. +*/ +PALEXPORT SSL_CTX* CryptoNative_SslGetSslCtx(SSL* ssl) +{ + // No error queue impact. + return SSL_get_SSL_CTX(ssl); +} diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index ffb77c3fb3dbc3..4ee55e8df6d114 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -429,6 +429,11 @@ Shims SSL_CTX_get_ex_data to retrieve application context. */ PALEXPORT void* CryptoNative_SslCtxGetData(SSL_CTX* ctx); +/* +Shims SSL_get_SSL_CTX to retrieve the SSL_CTX from the SSL. +*/ +PALEXPORT SSL_CTX* CryptoNative_SslGetSslCtx(SSL* ssl); + /* Sets the specified encryption policy on the SSL_CTX. From 8960d4503b29316dd97f6c35a1c4a172c580ad4c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 24 Jun 2025 16:05:33 +0200 Subject: [PATCH 05/16] Reuse cert validation code --- .../Interop.OpenSsl.cs | 72 ++++++++----------- .../Interop.SslCtx.cs | 2 +- .../System/Net/Security/SslStream.Android.cs | 1 - .../src/System/Net/Security/SslStream.IO.cs | 2 +- .../System/Net/Security/SslStream.Protocol.cs | 22 ++++-- 5 files changed, 49 insertions(+), 50 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 712b8e0d6e06ed..51ece81751f416 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 @@ -860,7 +860,7 @@ private static void QueryUniqueChannelBinding(SafeSslHandle context, SafeChannel } [UnmanagedCallersOnly] - internal static int CertVerifyCallback(IntPtr ssl, IntPtr store) + internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback(IntPtr ssl, IntPtr store) { IntPtr data = Ssl.SslGetData(ssl); Debug.Assert(data != IntPtr.Zero, "Expected non-null data pointer from SslGetData"); @@ -898,57 +898,47 @@ internal static int CertVerifyCallback(IntPtr ssl, IntPtr store) Debug.Assert(certificate != null, "Certificate should not be null here."); SslCertificateTrust? trust = options.CertificateContext?.Trust; - SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None; - if (options.CertificateChainPolicy != null) + ProtocolToken alertToken = default; + + if (options.SslStream!.VerifyRemoteCertificate(certificate, chain, trust, ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) { - chain.ChainPolicy = options.CertificateChainPolicy; + // success + return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_OK; } - else - { - chain.ChainPolicy.RevocationMode = options.CertificateRevocationCheckMode; - chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; - if (trust != null) - { - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - if (trust._store != null) - { - chain.ChainPolicy.CustomTrustStore.AddRange(trust._store.Certificates); - } - if (trust._trustList != null) - { - chain.ChainPolicy.CustomTrustStore.AddRange(trust._trustList); - } - } + if (options.CertValidationDelegate != null) + { + // rejected by user validation callback + return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_APPLICATION_VERIFICATION; } - // set ApplicationPolicy unless already provided. - if (chain.ChainPolicy.ApplicationPolicy.Count == 0) + TlsAlertMessage alert; + if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None) { - // Authenticate the remote party: (e.g. when operating in server mode, authenticate the client). - chain.ChainPolicy.ApplicationPolicy.Add(options.IsServer ? SslStream.s_clientAuthOid : SslStream.s_serverAuthOid); + alert = SslStream.GetAlertMessageFromChain(chain); } - - sslPolicyErrors |= CertificateValidation.BuildChainAndVerifyProperties( - chain, - certificate, - options.CheckCertName, - options.IsServer, - TargetHostNameHelper.NormalizeHostName(options.TargetHost)); - - bool success = sslPolicyErrors == SslPolicyErrors.None; - - if (options.CertValidationDelegate != null) + else if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != SslPolicyErrors.None) + { + alert = TlsAlertMessage.BadCertificate; + } + else { - success = options.CertValidationDelegate( - options.SslStream!, - certificate, - chain, - sslPolicyErrors); + alert = TlsAlertMessage.CertificateUnknown; } - return success ? 0 : 27 /* BAD_CERT */; + // since we can't set the alert directly, we pick one of the error verify statuses + // which will result in the same alert being sent + + return alert switch + { + TlsAlertMessage.BadCertificate => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, + TlsAlertMessage.UnknownCA => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT, + TlsAlertMessage.CertificateRevoked => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REVOKED, + TlsAlertMessage.CertificateExpired => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_HAS_EXPIRED, + TlsAlertMessage.UnsupportedCert => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_INVALID_PURPOSE, + _ => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, + }; } [UnmanagedCallersOnly] diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs index 014eda084874b5..c67f4ca4c1f6e6 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs @@ -64,7 +64,7 @@ internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, ReadOnl } [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCertVerifyCallback")] - internal static unsafe partial void SslCtxSetCertVerifyCallback(SafeSslContextHandle ctx, delegate* unmanaged callback); + internal static unsafe partial void SslCtxSetCertVerifyCallback(SafeSslContextHandle ctx, delegate* unmanaged callback); } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs index 317375cec0b1d4..8be1ff74e78bf9 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Android.cs @@ -16,7 +16,6 @@ private JavaProxy.RemoteCertificateValidationResult VerifyRemoteCertificate() { ProtocolToken alertToken = default; var isValid = VerifyRemoteCertificate( - _sslAuthenticationOptions.CertValidationDelegate, _sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out SslPolicyErrors sslPolicyErrors, diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index eac06538b92119..0f0889eb24fe44 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -576,7 +576,7 @@ private bool CompleteHandshake(ref ProtocolToken alertToken, out SslPolicyErrors } #endif - if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertValidationDelegate, _sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) + if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) { _handshakeCompleted = false; return false; diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index b291ecab293f5e..e5260f68adb59a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Runtime.ExceptionServices; using System.Security; using System.Security.Authentication; @@ -1039,18 +1040,23 @@ internal SecurityStatusPal Decrypt(Span buffer, out int outputOffset, out --*/ //This method validates a remote certificate. - internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remoteCertValidationCallback, SslCertificateTrust? trust, ref ProtocolToken alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus) + internal bool VerifyRemoteCertificate(SslCertificateTrust? trust, ref ProtocolToken alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus) + { + X509Chain? chain = null; + X509Certificate2? certificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, ref chain, _sslAuthenticationOptions.CertificateChainPolicy); + + return VerifyRemoteCertificate(certificate, chain, trust, ref alertToken, out sslPolicyErrors, out chainStatus); + } + + internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? chain, SslCertificateTrust? trust, ref ProtocolToken alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus) { sslPolicyErrors = SslPolicyErrors.None; chainStatus = X509ChainStatusFlags.NoError; - // We don't catch exceptions in this method, so it's safe for "accepted" be initialized with true. bool success = false; - X509Chain? chain = null; try { - X509Certificate2? certificate = CertificateValidationPal.GetRemoteCertificate(_securityContext, ref chain, _sslAuthenticationOptions.CertificateChainPolicy); if (_remoteCertificate != null && certificate != null && certificate.RawDataMemory.Span.SequenceEqual(_remoteCertificate.RawDataMemory.Span)) @@ -1113,6 +1119,7 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot _remoteCertificate = certificate; + RemoteCertificateValidationCallback? remoteCertValidationCallback = _sslAuthenticationOptions.CertValidationDelegate; if (remoteCertValidationCallback != null) { success = remoteCertValidationCallback(this, certificate, chain, sslPolicyErrors); @@ -1135,7 +1142,10 @@ internal bool VerifyRemoteCertificate(RemoteCertificateValidationCallback? remot if (!success) { - CreateFatalHandshakeAlertToken(sslPolicyErrors, chain!, ref alertToken); + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + CreateFatalHandshakeAlertToken(sslPolicyErrors, chain!, ref alertToken); + } if (chain != null) { foreach (X509ChainStatus status in chain.ChainStatus) @@ -1229,7 +1239,7 @@ private ProtocolToken GenerateAlertToken() return GenerateToken(default, out _); } - private static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain) + internal static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain) { foreach (X509ChainStatus chainStatus in chain.ChainStatus) { From 7cb0edd0d5bb72ff7cc6b7705aab29cd188cb5c2 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 1 Jul 2025 18:12:08 +0200 Subject: [PATCH 06/16] Propagate validation exception back to managed code --- .../Interop.OpenSsl.cs | 86 +++++++++++-------- .../Interop.Ssl.cs | 4 + .../src/System/Net/Security/SslStream.IO.cs | 51 +++++++---- .../System/Net/Security/SslStream.Protocol.cs | 10 ++- .../Net/Security/SslStreamPal.Android.cs | 1 + .../System/Net/Security/SslStreamPal.OSX.cs | 1 + .../System/Net/Security/SslStreamPal.Unix.cs | 18 ++-- .../Net/Security/SslStreamPal.Windows.cs | 1 + .../ServerAsyncAuthenticateTest.cs | 4 +- .../Fakes/FakeSslStream.Implementation.cs | 19 ++++ .../System.Net.Security.Unit.Tests.csproj | 10 +++ 11 files changed, 135 insertions(+), 70 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 51ece81751f416..eb5f27813bdd18 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 @@ -11,6 +11,7 @@ using System.IO; using System.Net; using System.Net.Security; +using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Authentication.ExtendedProtection; @@ -695,7 +696,11 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, return SecurityStatusPalErrorCode.CredentialsNeeded; } - if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ)) + if (errorCode == Ssl.SslErrorCode.SSL_ERROR_SSL && context.CertificateValidationException is Exception ex) + { + handshakeException = ex; + } + else if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ)) { Exception? innerError = GetSslError(retVal, errorCode); @@ -732,7 +737,7 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, if (handshakeException != null) { - throw handshakeException; + ExceptionDispatchInfo.Throw(handshakeException); } // in case of TLS 1.3 post-handshake authentication, SslDoHandhaske @@ -869,8 +874,10 @@ internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback( using SafeX509StoreCtxHandle storeHandle = new(store, ownsHandle: false); + // the chain will get properly disposed inside the VerifyRemoteCertificate call X509Chain chain = new X509Chain(); X509Certificate2? certificate = null; + SafeSslHandle sslHandle = (SafeSslHandle)options.SslStream!._securityContext!; using (SafeSharedX509StackHandle chainStack = Interop.Crypto.X509StoreCtxGetSharedUntrusted(storeHandle)) { @@ -901,44 +908,55 @@ internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback( ProtocolToken alertToken = default; - if (options.SslStream!.VerifyRemoteCertificate(certificate, chain, trust, ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + try { - // success - return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_OK; - } + if (options.SslStream!.VerifyRemoteCertificate(certificate, chain, trust, ref alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus)) + { + // success + return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_OK; + } - if (options.CertValidationDelegate != null) - { - // rejected by user validation callback - return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_APPLICATION_VERIFICATION; - } + sslHandle.CertificateValidationException = SslStream.CreateCertificateValidationException(options, sslPolicyErrors, chainStatus); - TlsAlertMessage alert; - if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None) - { - alert = SslStream.GetAlertMessageFromChain(chain); - } - else if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != SslPolicyErrors.None) - { - alert = TlsAlertMessage.BadCertificate; - } - else - { - alert = TlsAlertMessage.CertificateUnknown; - } + if (options.CertValidationDelegate != null) + { + // rejected by user validation callback + return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_APPLICATION_VERIFICATION; + } - // since we can't set the alert directly, we pick one of the error verify statuses - // which will result in the same alert being sent + TlsAlertMessage alert; + if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateChainErrors) != SslPolicyErrors.None) + { + alert = SslStream.GetAlertMessageFromChain(chain); + } + else if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != SslPolicyErrors.None) + { + alert = TlsAlertMessage.BadCertificate; + } + else + { + alert = TlsAlertMessage.CertificateUnknown; + } + + // since we can't set the alert directly, we pick one of the error verify statuses + // which will result in the same alert being sent - return alert switch + return alert switch + { + TlsAlertMessage.BadCertificate => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, + TlsAlertMessage.UnknownCA => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT, + TlsAlertMessage.CertificateRevoked => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REVOKED, + TlsAlertMessage.CertificateExpired => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_HAS_EXPIRED, + TlsAlertMessage.UnsupportedCert => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_INVALID_PURPOSE, + _ => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, + }; + } + catch (Exception ex) { - TlsAlertMessage.BadCertificate => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, - TlsAlertMessage.UnknownCA => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT, - TlsAlertMessage.CertificateRevoked => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REVOKED, - TlsAlertMessage.CertificateExpired => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_HAS_EXPIRED, - TlsAlertMessage.UnsupportedCert => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_INVALID_PURPOSE, - _ => Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_CERT_REJECTED, - }; + sslHandle.CertificateValidationException = ex; + + return Interop.Crypto.X509VerifyStatusCodeUniversal.X509_V_ERR_UNSPECIFIED; + } } [UnmanagedCallersOnly] 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 5d3bd4e7913304..ce2211ff6ba66f 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 @@ -390,6 +390,10 @@ internal sealed class SafeSslHandle : SafeDeleteSslContext // refcount management. public SafeSslContextHandle? SslContextHandle; + // Storage for the exception that occurred during certificate validation callback so that + // we may rethrow it after returning to managed code. + public Exception? CertificateValidationException; + public bool IsServer { get { return _isServer; } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 0f0889eb24fe44..bd32eee6030386 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -576,11 +576,21 @@ private bool CompleteHandshake(ref ProtocolToken alertToken, out SslPolicyErrors } #endif - if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) +#pragma warning disable CS0162 // unreachable code on some platforms + if (!SslStreamPal.CertValidationInCallback) { - _handshakeCompleted = false; - return false; + if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) + { + _handshakeCompleted = false; + return false; + } + } + else + { + sslPolicyErrors = SslPolicyErrors.None; + chainStatus = X509ChainStatusFlags.NoError; } +#pragma warning restore CS0162 // unreachable code on some platforms _handshakeCompleted = true; return true; @@ -591,21 +601,26 @@ private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions ProtocolToken alertToken = default; 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(new ReadOnlySpan(alertToken.Payload), 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(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null))); - } - else - { - // Simple add sslPolicyErrors as crude info. - SendAuthResetSignal(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null))); - } + SendAuthResetSignal(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(CreateCertificateValidationException(sslAuthenticationOptions, sslPolicyErrors, chainStatus))); + } + } + + internal static Exception CreateCertificateValidationException(SslAuthenticationOptions options, SslPolicyErrors sslPolicyErrors, X509ChainStatusFlags chainStatus) + { + if (options.CertValidationDelegate != null) + { + // there may be some chain errors but the decision was made by custom callback. Details should be tracing if enabled. + return ExceptionDispatchInfo.SetCurrentStackTrace(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. + return ExceptionDispatchInfo.SetCurrentStackTrace(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_chain_validation, chainStatus), null)); + } + else + { + // Simple add sslPolicyErrors as crude info. + return ExceptionDispatchInfo.SetCurrentStackTrace(new AuthenticationException(SR.Format(SR.net_ssl_io_cert_validation, sslPolicyErrors), null)); } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index e5260f68adb59a..166f3d427b485c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -51,7 +51,7 @@ internal static bool DisableTlsResume private SafeFreeCredentials? _credentialsHandle; - private SafeDeleteSslContext? _securityContext; + internal SafeDeleteSslContext? _securityContext; private SslConnectionInfo _connectionInfo; private X509Certificate? _selectedClientCertificate; @@ -1131,7 +1131,7 @@ internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? sslPolicyErrors &= ~SslPolicyErrors.RemoteCertificateNotAvailable; } - success = (sslPolicyErrors == SslPolicyErrors.None); + success = sslPolicyErrors == SslPolicyErrors.None; } if (NetEventSource.Log.IsEnabled()) @@ -1142,10 +1142,14 @@ internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? if (!success) { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) +#pragma warning disable CS0162 // unreachable code on some platforms + // avoid reentrant call to GenerateToken if validation was called from a callback + if (!SslStreamPal.CertValidationInCallback) { CreateFatalHandshakeAlertToken(sslPolicyErrors, chain!, ref alertToken); } +#pragma warning restore CS0162 // unreachable code on some platforms + if (chain != null) { foreach (X509ChainStatus status in chain.ChainStatus) 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 4a5de17a2a221f..8fc47799c8cfe8 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 @@ -20,6 +20,7 @@ public static Exception GetException(SecurityStatusPal status) } internal const bool StartMutualAuthAsAnonymous = false; + internal const bool CertValidationInCallback = false; internal const bool CanEncryptEmptyMessage = false; public static void VerifyPackageInfo() 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 add3454a6f14eb..e3456d97295489 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 @@ -23,6 +23,7 @@ public static Exception GetException(SecurityStatusPal status) } internal const bool StartMutualAuthAsAnonymous = true; + internal const bool CertValidationInCallback = false; // SecureTransport is okay with a 0 byte input, but it produces a 0 byte output. // Since ST is not producing the framed empty message just call this false and avoid the 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 ccceda69097d15..ab13f96af3f812 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 @@ -18,6 +18,7 @@ public static Exception GetException(SecurityStatusPal status) } internal const bool StartMutualAuthAsAnonymous = false; + internal const bool CertValidationInCallback = true; internal const bool CanEncryptEmptyMessage = false; public static void VerifyPackageInfo() @@ -214,18 +215,6 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context errorCode = Interop.OpenSsl.DoSslHandshake((SafeSslHandle)context, ReadOnlySpan.Empty, ref token); } - // 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. - // We have this workaround, as openssl supports terminating handshake only from version 1.1.0, - // whereas ALPN is supported from version 1.0.2. - SafeSslHandle sslContext = (SafeSslHandle)context; - if (errorCode == SecurityStatusPalErrorCode.OK && sslAuthenticationOptions.IsServer - && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) - { - token.Status = new SecurityStatusPal(SecurityStatusPalErrorCode.InternalError, Interop.OpenSsl.CreateSslException(SR.net_alpn_failed)); - return token; - } - token.Status = new SecurityStatusPal(errorCode); } catch (Exception exc) @@ -236,7 +225,10 @@ private static ProtocolToken HandshakeInternal(ref SafeDeleteSslContext? context return token; } - public static SecurityStatusPal ApplyAlertToken(SafeDeleteContext? securityContext, TlsAlertType alertType, TlsAlertMessage alertMessage) + public static SecurityStatusPal ApplyAlertToken( + SafeDeleteContext? securityContext, + TlsAlertType alertType, + TlsAlertMessage alertMessage) { // There doesn't seem to be an exposed API for writing an alert, // the API seems to assume that all alerts are generated internally by 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 61bc2fd2e71c04..02da9c62346816 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 @@ -44,6 +44,7 @@ public static Exception GetException(SecurityStatusPal status) } internal const bool StartMutualAuthAsAnonymous = true; + internal const bool CertValidationInCallback = false; internal const bool CanEncryptEmptyMessage = true; private static readonly byte[] s_sessionTokenBuffer = InitSessionTokenBuffer(); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 44861f71ee3082..ef59c802863a53 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -209,7 +209,7 @@ public async Task ServerAsyncAuthenticate_VerificationDelegate_Success() { bool validationCallbackCalled = false; var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate, ClientCertificateRequired = true, }; - var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false), AllowTlsResume = false }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; serverOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => { @@ -241,7 +241,7 @@ public async Task ServerAsyncAuthenticate_ConstructorVerificationDelegate_Succes { bool validationCallbackCalled = false; var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate, ClientCertificateRequired = true, }; - var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false), AllowTlsResume = false }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); 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 31346cc9715e52..ad47676ffb1636 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 @@ -24,6 +24,7 @@ private class FakeOptions public RemoteCertificateValidationCallback? CertValidationDelegate; public LocalCertificateSelectionCallback? CertSelectionDelegate; public X509RevocationMode CertificateRevocationCheckMode; + public SslStream? SslStream; public void UpdateOptions(SslServerAuthenticationOptions sslServerAuthenticationOptions) { @@ -40,6 +41,7 @@ internal void UpdateOptions(ServerOptionsSelectionCallback optionCallback, objec private FakeOptions _sslAuthenticationOptions = new FakeOptions(); private SslConnectionInfo _connectionInfo; + internal SafeDeleteSslContext? _securityContext; internal ChannelBinding? GetChannelBinding(ChannelBindingKind kind) => null; private bool _remoteCertificateExposed; private X509Certificate2? LocalClientCertificate; @@ -103,6 +105,23 @@ private ProtocolToken CreateShutdownToken() { return null; } + + internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? chain, SslCertificateTrust? trust, ref ProtocolToken alertToken, out SslPolicyErrors sslPolicyErrors, out X509ChainStatusFlags chainStatus) + { + chainStatus = X509ChainStatusFlags.NoError; + sslPolicyErrors = SslPolicyErrors.None; + return true; + } + + internal static Exception CreateCertificateValidationException(SslAuthenticationOptions options, SslPolicyErrors sslPolicyErrors, X509ChainStatusFlags chainStatus) + { + return new Exception(); + } + + internal static TlsAlertMessage GetAlertMessageFromChain(X509Chain chain) + { + return TlsAlertMessage.CloseNotify; + } } internal class ProtocolToken diff --git a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj index 42381dbab6c6b8..e18995da0a54ba 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj @@ -130,10 +130,20 @@ + + + + + + From 7a4d6744758605bf881f4050c3c3fb99e4776961 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 13:05:44 +0100 Subject: [PATCH 07/16] Fix build --- .../src/System/Net/Security/SslStream.Protocol.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 30fb12a116434a..5916e65bdbefc7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -1090,6 +1090,7 @@ internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? // validation. // TODO: this forces allocation of X509Certificate2Collection int preexistingExtraCertsCount = _sslAuthenticationOptions.CertificateChainPolicy?.ExtraStore?.Count ?? 0; + RemoteCertificateValidationCallback? remoteCertValidationCallback = _sslAuthenticationOptions.CertValidationDelegate; try { @@ -1160,7 +1161,6 @@ internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? _remoteCertificate = certificate; - RemoteCertificateValidationCallback? remoteCertValidationCallback = _sslAuthenticationOptions.CertValidationDelegate; if (remoteCertValidationCallback != null) { success = remoteCertValidationCallback(this, certificate, chain, sslPolicyErrors); From 1087f6379cdb0b9e330792015988dc24b59719e7 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 14:02:28 +0100 Subject: [PATCH 08/16] Fix inline cert validation for resumed sessions and missing certs When CertValidationInCallback is true (Linux/Unix), the CertVerifyCallback is not called by OpenSSL in two cases: 1. Session resumption - cert was already validated in the original session 2. Peer didn't provide a certificate Previously, CompleteHandshake skipped VerifyRemoteCertificate entirely when CertValidationInCallback was true, which caused: - _remoteCertificate to remain null on resumed sessions, breaking IsMutuallyAuthenticated - User's RemoteCertificateValidationCallback to never be invoked when the peer didn't provide a certificate Fix: When _remoteCertificate is null after inline validation: - For resumed sessions: retrieve the cert from the SSL handle - For no-cert case: fall through to VerifyRemoteCertificate to invoke the user callback with RemoteCertificateNotAvailable Also skip ConnectionInfoInCallback test on Linux since connection info is not available during the inline cert validation callback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.IO.cs | 27 +++++++++++++++++++ .../ClientAsyncAuthenticateTest.cs | 1 + 2 files changed, 28 insertions(+) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index eff00cb63ea53d..ac11c6f71e60e4 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -610,6 +610,33 @@ private bool CompleteHandshake(ref ProtocolToken alertToken, out SslPolicyErrors return false; } } + else if (_remoteCertificate is null) + { + // CertVerifyCallback was not called during the handshake. This happens when: + // 1. The session was resumed — the cert is available from the SSL handle + // but OpenSSL skips the verify callback. + // 2. The peer didn't provide a certificate at all. + + X509Certificate2? remoteCert = CertificateValidationPal.GetRemoteCertificate(_securityContext); + if (remoteCert is not null) + { + // Resumed session: the cert was already validated in the original handshake. + _remoteCertificate = remoteCert; + sslPolicyErrors = SslPolicyErrors.None; + chainStatus = X509ChainStatusFlags.NoError; + } + else + { + // No certificate was provided by the peer. Run verification so that + // the user's RemoteCertificateValidationCallback is invoked with + // SslPolicyErrors.RemoteCertificateNotAvailable. + if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) + { + _handshakeCompleted = false; + return false; + } + } + } else { sslPolicyErrors = SslPolicyErrors.None; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs index addde8483c84a9..8ab246e9fcb4bf 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs @@ -29,6 +29,7 @@ public async Task ClientAsyncAuthenticate_ServerRequireEncryption_ConnectWithEnc [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/115467", TestPlatforms.Android)] + [SkipOnPlatform(TestPlatforms.Linux, "With inline cert validation, connection info is not yet available during the validation callback.")] public async Task ClientAsyncAuthenticate_ConnectionInfoInCallback_DoesNotThrow() { await ClientAsyncSslHelper(EncryptionPolicy.RequireEncryption, SslProtocols.Tls12, SslProtocolSupport.DefaultSslProtocols, AllowAnyServerCertificateAndVerifyConnectionInfo); From 4013d8e8b2e1dd9760d2c20a0ea9b781622e49b1 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 14:38:19 +0100 Subject: [PATCH 09/16] Fix GCHandle double-free in AllocateSslHandle error path When an exception occurs in AllocateSslHandle after the AuthOptionsHandle GCHandle is allocated and stored on the SafeSslHandle, the catch block frees the local copy of the GCHandle but the copy on sslHandle retains the stale IntPtr. When sslHandle is later finalized, ReleaseHandle() calls AuthOptionsHandle.Free() again, double-freeing the GCHandle. This corrupts the GCHandle table and causes crashes in unrelated code (e.g., SafeWaitHandle finalization in WaitSubsystem) when the freed GCHandle slot is reused. --- .../Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs | 1 + 1 file changed, 1 insertion(+) 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 fcc8fcd8ffcd48..6c40dcf44031a0 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 @@ -424,6 +424,7 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth authOptionsHandle = GCHandle.Alloc(sslAuthenticationOptions); Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(authOptionsHandle)); sslHandle.AuthOptionsHandle = authOptionsHandle; + authOptionsHandle = default; // the ownership is transferred to sslHandle, so we should not free it in the caller anymore. if (!sslAuthenticationOptions.AllowRsaPssPadding || !sslAuthenticationOptions.AllowRsaPkcs1Padding) { From e9fa3c8b9ebc09ba43ac3eb6753e709f40420e59 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 15:11:07 +0100 Subject: [PATCH 10/16] Enable connection info during inline cert validation callback During the cert verify callback (TLS 1.2), SSL_get_current_cipher() returns NULL because session->cipher is only set at ChangeCipherSpec. Fall back to SSL_get_pending_cipher() which returns s3.tmp.new_cipher, available as soon as ServerHello is processed. Also populate _connectionInfo before invoking the user's RemoteCertificateValidationCallback so that properties like SslProtocol and CipherAlgorithm are accessible during inline cert validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.Protocol.cs | 9 +++++++++ .../tests/FunctionalTests/ClientAsyncAuthenticateTest.cs | 1 - .../System.Security.Cryptography.Native/opensslshim.h | 2 ++ .../libs/System.Security.Cryptography.Native/pal_ssl.c | 8 ++++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 5916e65bdbefc7..d144a6a00d068b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -1163,6 +1163,15 @@ internal bool VerifyRemoteCertificate(X509Certificate2? certificate, X509Chain? if (remoteCertValidationCallback != null) { + // Ensure connection info is populated before calling the user callback, + // which may access properties like SslProtocol or CipherAlgorithm. + // During inline cert validation the handshake hasn't completed yet, so + // _connectionInfo may not have been set by ProcessHandshakeSuccess. + if (_connectionInfo.Protocol == 0 && _securityContext is not null) + { + SslStreamPal.QueryContextConnectionInfo(_securityContext, ref _connectionInfo); + } + success = remoteCertValidationCallback(this, certificate, chain, sslPolicyErrors); } else diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs index 8ab246e9fcb4bf..addde8483c84a9 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs @@ -29,7 +29,6 @@ public async Task ClientAsyncAuthenticate_ServerRequireEncryption_ConnectWithEnc [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/115467", TestPlatforms.Android)] - [SkipOnPlatform(TestPlatforms.Linux, "With inline cert validation, connection info is not yet available during the validation callback.")] public async Task ClientAsyncAuthenticate_ConnectionInfoInCallback_DoesNotThrow() { await ClientAsyncSslHelper(EncryptionPolicy.RequireEncryption, SslProtocols.Tls12, SslProtocolSupport.DefaultSslProtocols, AllowAnyServerCertificateAndVerifyConnectionInfo); diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 8a40ca1aca34a5..7b728b539e9d9c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -688,6 +688,7 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_get_error) \ REQUIRED_FUNCTION(SSL_get_ex_data) \ REQUIRED_FUNCTION(SSL_get_ex_data_X509_STORE_CTX_idx) \ + REQUIRED_FUNCTION(SSL_get_pending_cipher) \ REQUIRED_FUNCTION(SSL_get_finished) \ REQUIRED_FUNCTION(SSL_get_peer_cert_chain) \ REQUIRED_FUNCTION(SSL_get_peer_finished) \ @@ -1270,6 +1271,7 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_get_finished SSL_get_finished_ptr #define SSL_get_peer_cert_chain SSL_get_peer_cert_chain_ptr #define SSL_get_peer_finished SSL_get_peer_finished_ptr +#define SSL_get_pending_cipher SSL_get_pending_cipher_ptr #define SSL_get_servername SSL_get_servername_ptr #define SSL_get_SSL_CTX SSL_get_SSL_CTX_ptr #define SSL_get_version SSL_get_version_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 800e55996abfe3..5f134ddbc0823c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -1084,6 +1084,14 @@ int32_t CryptoNative_SslGetCurrentCipherId(SSL* ssl, int32_t* cipherId) const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl); if (!cipher) + { + // During the handshake (e.g. inside the cert verify callback), + // the current cipher may not be set yet (TLS 1.2 sets it at + // ChangeCipherSpec). Fall back to the pending cipher which is + // available as soon as ServerHello is processed. + cipher = SSL_get_pending_cipher(ssl); + } + if (!cipher) { *cipherId = -1; return 0; From ac88dca893be9dc5585cdc529903310f6f819682 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 15:21:26 +0100 Subject: [PATCH 11/16] Fix issues found in code review - Fix Debug.Assert that incorrectly asserts certificate is non-null in CertVerifyCallback. On the server side, the client may not send a certificate, which is a valid scenario handled by VerifyRemoteCertificate. - Remove debug printf in verify_callback (pal_ssl.c). --- .../Interop.OpenSsl.cs | 2 +- .../libs/System.Security.Cryptography.Native/pal_ssl.c | 9 --------- 2 files changed, 1 insertion(+), 10 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 6c40dcf44031a0..9d8d00401c5165 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 @@ -904,7 +904,7 @@ internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback( } } - Debug.Assert(certificate != null, "Certificate should not be null here."); + Debug.Assert(certificate != null, "OpenSSL only calls the callback if the certificate is present."); SslCertificateTrust? trust = options.CertificateContext?.Trust; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 5f134ddbc0823c..ef30dfb65e9b1b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -466,15 +466,6 @@ int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error) return result; } -static int verify_callback(int preverify_ok, X509_STORE_CTX* store) -{ - (void)preverify_ok; - (void)store; - printf("Store: %p, preverify_ok: %d\n", (void*)store, preverify_ok); - // We don't care. Real verification happens in managed code. - return 1; -} - int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) { ERR_clear_error(); From 57d5d1b41782c1ae0476a4d0f68f8391e9d41468 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 18:29:52 +0100 Subject: [PATCH 12/16] Send TLS alert when client cert is required but not provided Add failIfNoPeerCert parameter to SslSetVerifyPeer. When the server requires a client certificate and no user RemoteCertificateValidation callback is registered, set SSL_VERIFY_FAIL_IF_NO_PEER_CERT so that OpenSSL sends the appropriate TLS alert (certificate_required for TLS 1.3, handshake_failure for TLS 1.2). When a user callback IS registered, only SSL_VERIFY_PEER is set, preserving the ability for the callback to accept connections without a client certificate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Interop.OpenSsl.cs | 11 +++++++++-- .../Interop.Ssl.cs | 2 +- .../System.Security.Cryptography.Native/pal_ssl.c | 13 +++++++++---- .../System.Security.Cryptography.Native/pal_ssl.h | 2 +- 4 files changed, 20 insertions(+), 8 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 9d8d00401c5165..dca1be067ee107 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 @@ -445,7 +445,7 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth if (sslAuthenticationOptions.IsClient) { // Client side always verifies the server's certificate. - Ssl.SslSetVerifyPeer(sslHandle); + Ssl.SslSetVerifyPeer(sslHandle, failIfNoPeerCert: false); if (!string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && !IPAddress.IsValid(sslAuthenticationOptions.TargetHost)) { @@ -478,7 +478,14 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth { if (sslAuthenticationOptions.RemoteCertRequired) { - Ssl.SslSetVerifyPeer(sslHandle); + // When no user callback is registered, also set + // SSL_VERIFY_FAIL_IF_NO_PEER_CERT so that OpenSSL sends the + // appropriate TLS alert when the client doesn't provide a + // certificate. When a callback IS registered, the application + // may choose to accept connections without a client certificate, + // so we only set SSL_VERIFY_PEER and let managed code handle it. + bool failIfNoPeerCert = sslAuthenticationOptions.CertValidationDelegate is null; + Ssl.SslSetVerifyPeer(sslHandle, failIfNoPeerCert); } if (sslAuthenticationOptions.CertificateContext != 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 d9b752e342d025..c48c4272ab44ac 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 @@ -185,7 +185,7 @@ internal static SafeSharedX509StackHandle SslGetPeerCertChain(SafeSslHandle ssl) internal static unsafe partial bool SslSetCiphers(SafeSslHandle ssl, byte* cipherList, byte* cipherSuites); [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyPeer")] - internal static partial void SslSetVerifyPeer(SafeSslHandle ssl); + internal static partial void SslSetVerifyPeer(SafeSslHandle ssl, [MarshalAs(UnmanagedType.Bool)] bool failIfNoPeerCert); [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetData")] internal static partial IntPtr SslGetData(IntPtr ssl); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index ef30dfb65e9b1b..91b78bd7affcb6 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -478,7 +478,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) if (SSL_version(ssl) == TLS1_3_VERSION) { // Post-handshake auth reqires SSL_VERIFY_PEER to be set - CryptoNative_SslSetVerifyPeer(ssl); + CryptoNative_SslSetVerifyPeer(ssl, 0); return SSL_verify_client_post_handshake(ssl); } #endif @@ -489,7 +489,7 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) int pending = SSL_renegotiate_pending(ssl); if (!pending) { - CryptoNative_SslSetVerifyPeer(ssl); + CryptoNative_SslSetVerifyPeer(ssl, 0); int ret = SSL_renegotiate(ssl); if(ret != 1) { @@ -629,10 +629,15 @@ X509NameStack* CryptoNative_SslGetClientCAList(SSL* ssl) return SSL_get_client_CA_list(ssl); } -void CryptoNative_SslSetVerifyPeer(SSL* ssl) +void CryptoNative_SslSetVerifyPeer(SSL* ssl, int32_t failIfNoPeerCert) { // void shim functions don't lead to exceptions, so skip the unconditional error clearing. - SSL_set_verify(ssl, SSL_VERIFY_PEER, NULL); + int mode = SSL_VERIFY_PEER; + if (failIfNoPeerCert) + { + mode |= SSL_VERIFY_FAIL_IF_NO_PEER_CERT; + } + SSL_set_verify(ssl, mode, NULL); } int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, uint8_t* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 4ee55e8df6d114..787c8e56dac1e9 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -407,7 +407,7 @@ PALEXPORT X509NameStack* CryptoNative_SslGetClientCAList(SSL* ssl); /* Shims the SSL_set_verify method. */ -PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl); +PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl, int32_t failIfNoPeerCert); /* Shims SSL_set_ex_data to attach application context. From a28c862063248487276b4b4de7df1575b5d0cb9c Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 9 Mar 2026 18:44:48 +0100 Subject: [PATCH 13/16] Remove unused experimental code Remove debugging and experimental leftovers: - bio_stdout, apps_ssl_info_callback, and commented-out debug logging in CryptoNative_SslSetConnectState (pal_ssl.c) - CryptoNative_SslSetVerifyResult and CryptoNative_SslGetVerifyResult (unused native functions with no managed P/Invoke) - Unused opensslshim.h entries: ASYNC_pause_job, BIO_new_fp, BIO_printf, SSL_set_info_callback, SSL_want, SSL_trace, SSL_set_msg_callback - Unused EncryptedReadOnlyMemory property (SslStream.cs) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.cs | 1 - .../entrypoints.c | 2 - .../opensslshim.h | 14 ----- .../pal_ssl.c | 59 ------------------- .../pal_ssl.h | 9 --- 5 files changed, 85 deletions(-) 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 172c9dbe4ab5d2..41019a73cd6103 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 @@ -96,7 +96,6 @@ public ReadOnlySpan DecryptedReadOnlySpanSliced(int length) public Span EncryptedSpanSliced(int length) => _buffer.ActiveSpan.Slice(_decryptedLength + _decryptedPadding, length); public ReadOnlySpan EncryptedReadOnlySpan => _buffer.ActiveSpan.Slice(_decryptedLength + _decryptedPadding); - public ReadOnlyMemory EncryptedReadOnlyMemory => _buffer.ActiveMemory.Slice(_decryptedLength + _decryptedPadding); public int EncryptedLength => _buffer.ActiveLength - _decryptedPadding - _decryptedLength; diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 8a4865567cda48..9923a4ffd80bd0 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -390,8 +390,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetServerName) DllImportEntry(CryptoNative_SslGetSession) DllImportEntry(CryptoNative_SslGetVersion) - DllImportEntry(CryptoNative_SslSetVerifyResult) - DllImportEntry(CryptoNative_SslGetVerifyResult) DllImportEntry(CryptoNative_SslRead) DllImportEntry(CryptoNative_SslSessionFree) DllImportEntry(CryptoNative_SslSessionGetHostname) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 7b728b539e9d9c..e42d55b8335403 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -267,7 +267,6 @@ extern bool g_libSslUses32BitTime; #define FOR_ALL_OPENSSL_FUNCTIONS \ REQUIRED_FUNCTION(a2d_ASN1_OBJECT) \ REQUIRED_FUNCTION(ASN1_d2i_bio) \ - REQUIRED_FUNCTION(ASYNC_pause_job) \ REQUIRED_FUNCTION(ASN1_i2d_bio) \ REQUIRED_FUNCTION(ASN1_GENERALIZEDTIME_free) \ REQUIRED_FUNCTION(ASN1_INTEGER_get) \ @@ -288,8 +287,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(BIO_gets) \ REQUIRED_FUNCTION(BIO_new) \ REQUIRED_FUNCTION(BIO_new_file) \ - REQUIRED_FUNCTION(BIO_new_fp) \ - REQUIRED_FUNCTION(BIO_printf) \ REQUIRED_FUNCTION(BIO_read) \ REQUIRED_FUNCTION(BIO_up_ref) \ REQUIRED_FUNCTION(BIO_s_mem) \ @@ -651,7 +648,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_ctrl) \ REQUIRED_FUNCTION(SSL_add_client_CA) \ REQUIRED_FUNCTION(SSL_set_alpn_protos) \ - REQUIRED_FUNCTION(SSL_set_info_callback) \ REQUIRED_FUNCTION(SSL_set_quiet_shutdown) \ REQUIRED_FUNCTION(SSL_CTX_callback_ctrl) \ REQUIRED_FUNCTION(SSL_CTX_check_private_key) \ @@ -733,9 +729,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_verify_client_post_handshake) \ REQUIRED_FUNCTION(SSL_set_post_handshake_auth) \ REQUIRED_FUNCTION(SSL_version) \ - REQUIRED_FUNCTION(SSL_want) \ - REQUIRED_FUNCTION(SSL_trace) \ - REQUIRED_FUNCTION(SSL_set_msg_callback) \ REQUIRED_FUNCTION(UI_create_method) \ REQUIRED_FUNCTION(UI_destroy_method) \ REQUIRED_FUNCTION(X509_check_host) \ @@ -843,7 +836,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define a2d_ASN1_OBJECT a2d_ASN1_OBJECT_ptr #define ASN1_GENERALIZEDTIME_free ASN1_GENERALIZEDTIME_free_ptr #define ASN1_d2i_bio ASN1_d2i_bio_ptr -#define ASYNC_pause_job ASYNC_pause_job_ptr #define ASN1_i2d_bio ASN1_i2d_bio_ptr #define ASN1_INTEGER_get ASN1_INTEGER_get_ptr #define ASN1_OBJECT_free ASN1_OBJECT_free_ptr @@ -863,8 +855,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define BIO_gets BIO_gets_ptr #define BIO_new BIO_new_ptr #define BIO_new_file BIO_new_file_ptr -#define BIO_new_fp BIO_new_fp_ptr -#define BIO_printf BIO_printf_ptr #define BIO_read BIO_read_ptr #define BIO_up_ref BIO_up_ref_ptr #define BIO_s_mem BIO_s_mem_ptr @@ -1231,7 +1221,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_ctrl SSL_ctrl_ptr #define SSL_add_client_CA SSL_add_client_CA_ptr #define SSL_set_alpn_protos SSL_set_alpn_protos_ptr -#define SSL_set_info_callback SSL_set_info_callback_ptr #define SSL_set_quiet_shutdown SSL_set_quiet_shutdown_ptr #define SSL_CTX_callback_ctrl SSL_CTX_callback_ctrl_ptr #define SSL_CTX_check_private_key SSL_CTX_check_private_key_ptr @@ -1312,9 +1301,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; #define SSL_verify_client_post_handshake SSL_verify_client_post_handshake_ptr #define SSL_set_post_handshake_auth SSL_set_post_handshake_auth_ptr #define SSL_version SSL_version_ptr -#define SSL_want SSL_want_ptr -#define SSL_trace SSL_trace_ptr -#define SSL_set_msg_callback SSL_set_msg_callback_ptr #define TLS_method TLS_method_ptr #define UI_create_method UI_create_method_ptr #define UI_destroy_method UI_destroy_method_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 91b78bd7affcb6..c690b6fc94a2c7 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -343,54 +343,10 @@ void CryptoNative_SslCtxDestroy(SSL_CTX* ctx) } } -static BIO* bio_stdout = NULL; - -static void apps_ssl_info_callback(const SSL *s, int where, int ret) -{ - const char *str; - int w = where & ~SSL_ST_MASK; - - if (w & SSL_ST_CONNECT) - str = "SSL_connect"; - else if (w & SSL_ST_ACCEPT) - str = "SSL_accept"; - else - str = "undefined"; - - if (where & SSL_CB_LOOP) { - BIO_printf(bio_stdout, "%s:%s\n", str, SSL_state_string_long(s)); - } else if (where & SSL_CB_ALERT) { - str = (where & SSL_CB_READ) ? "read" : "write"; - BIO_printf(bio_stdout, "SSL3 alert %s:%s:%s\n", str, - SSL_alert_type_string_long(ret), - SSL_alert_desc_string_long(ret)); - } else if (where & SSL_CB_EXIT) { - if (ret == 0) { - BIO_printf(bio_stdout, "%s:failed in %s\n", - str, SSL_state_string_long(s)); - } else if (ret < 0) { - BIO_printf(bio_stdout, "%s:error in %s\n", - str, SSL_state_string_long(s)); - } - } -} - void CryptoNative_SslSetConnectState(SSL* ssl) { // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_connect_state(ssl); - // SSL_set_msg_callback(ssl, SSL_trace); - // if (bio_stdout == NULL) - // { - // bio_stdout = BIO_new_fp(stdout, BIO_NOCLOSE); - // if (bio_stdout == NULL) - // { - // ERR_clear_error(); - // return; - // } - // } - // SSL_set_msg_callback_arg(ssl, bio_stdout); - // SSL_set_info_callback(ssl, apps_ssl_info_callback); } void CryptoNative_SslSetAcceptState(SSL* ssl) @@ -1378,21 +1334,6 @@ void CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxCertValidation } } -void CryptoNative_SslSetVerifyResult(SSL* ssl, int64_t verifyResult) -{ - (void)ssl; - (void)verifyResult; - SSL_set_verify_result(ssl, verifyResult); -} - -/* -Shims the SSL_get_verify_result method. -*/ -int64_t CryptoNative_SslGetVerifyResult(SSL* ssl) -{ - return SSL_get_verify_result(ssl); -} - /* Shims SSL_get_SSL_CTX to retrieve the SSL_CTX from the SSL. */ diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 787c8e56dac1e9..23f0874d22606a 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -569,12 +569,3 @@ Sets the certificate verification callback for the SSL_CTX. */ PALEXPORT void CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxCertValidationCallback callback); -/* -Shims the SSL_set_verify_result method. -*/ -PALEXPORT void CryptoNative_SslSetVerifyResult(SSL* ssl, int64_t verifyResult); - -/* -Shims the SSL_get_verify_result method. -*/ -PALEXPORT int64_t CryptoNative_SslGetVerifyResult(SSL* ssl); From 73ecb79ddbe62fb4d15904df6bbac26a3d33f596 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 10 Mar 2026 10:24:27 +0100 Subject: [PATCH 14/16] Fix build --- src/native/libs/System.Security.Cryptography.Native/pal_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index c690b6fc94a2c7..f096f30aebd322 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -1318,7 +1318,7 @@ static int CertVerifyCallback(X509_STORE_CTX* store, void* param) { (void)param; SslCtxCertValidationCallback callback = (SslCtxCertValidationCallback) param; - SSL *ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()); + SSL* ssl = (SSL*) X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx()); int verifyResult = callback(ssl, store); From 0e60f87c86b7b2726940b454fdda22fa1dcedc34 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Wed, 11 Mar 2026 13:13:50 +0100 Subject: [PATCH 15/16] Revalidate certificate after TLS session resumption When CertValidationInCallback is true and the CertVerifyCallback was not called (resumed session or no peer cert), always fall through to VerifyRemoteCertificate instead of skipping validation for resumed sessions. This ensures the user's RemoteCertificateValidationCallback is invoked and full chain validation is performed on every connection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/System/Net/Security/SslStream.IO.cs | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index ac11c6f71e60e4..beedabafa4fbc1 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -616,25 +616,12 @@ private bool CompleteHandshake(ref ProtocolToken alertToken, out SslPolicyErrors // 1. The session was resumed — the cert is available from the SSL handle // but OpenSSL skips the verify callback. // 2. The peer didn't provide a certificate at all. - - X509Certificate2? remoteCert = CertificateValidationPal.GetRemoteCertificate(_securityContext); - if (remoteCert is not null) - { - // Resumed session: the cert was already validated in the original handshake. - _remoteCertificate = remoteCert; - sslPolicyErrors = SslPolicyErrors.None; - chainStatus = X509ChainStatusFlags.NoError; - } - else + // In both cases, run VerifyRemoteCertificate to invoke the user's callback + // and perform full validation. + if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) { - // No certificate was provided by the peer. Run verification so that - // the user's RemoteCertificateValidationCallback is invoked with - // SslPolicyErrors.RemoteCertificateNotAvailable. - if (!VerifyRemoteCertificate(_sslAuthenticationOptions.CertificateContext?.Trust, ref alertToken, out sslPolicyErrors, out chainStatus)) - { - _handshakeCompleted = false; - return false; - } + _handshakeCompleted = false; + return false; } } else From e9399fd75b9436ff5b1ea81d37146375098a7ca3 Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Mon, 16 Mar 2026 10:38:04 +0100 Subject: [PATCH 16/16] Address PR review feedback - Restore SslCtxSetQuietShutdown call (wfurt, Copilot) - Clear CertificateValidationException after consuming to prevent stale exceptions on SSL handle reuse (Copilot) - Separate leaf cert from ExtraStore in CertVerifyCallback to prevent double-dispose when VerifyRemoteCertificate cleans up ExtraStore (Copilot) - Apply user's CertificateChainPolicy to the chain before adding intermediates so they aren't discarded when the policy is set (Copilot) - Complete truncated comment in NewSessionCallback explaining the SafeSslContextHandle lifetime invariant (Copilot) - Remove unused System.Runtime.InteropServices using (Copilot) - Revert s_serverAuthOid/s_clientAuthOid to private (wfurt) - Keep _securityContext private on TARGET_APPLE (wfurt) - Clear SslStream reference on SslAuthenticationOptions after handshake to avoid retaining the SslStream through the GCHandle chain (wfurt/rzikm) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Interop.OpenSsl.cs | 24 +++++++++++++++---- .../src/System/Net/Security/SslStream.IO.cs | 7 ++++++ .../System/Net/Security/SslStream.Protocol.cs | 7 +++--- 3 files changed, 29 insertions(+), 9 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 dca1be067ee107..2975df40fb3163 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 @@ -254,7 +254,7 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication // If you find yourself wanting to remove this line to enable bidirectional // close-notify, you'll probably need to rewrite SafeSslHandle.Disconnect(). // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html - // Ssl.SslCtxSetQuietShutdown(sslCtx); + Ssl.SslCtxSetQuietShutdown(sslCtx); if (enableResume) { @@ -708,6 +708,7 @@ internal static SecurityStatusPalErrorCode DoSslHandshake(SafeSslHandle context, if (errorCode == Ssl.SslErrorCode.SSL_ERROR_SSL && context.CertificateValidationException is Exception ex) { handshakeException = ex; + context.CertificateValidationException = null; } else if ((retVal != -1) || (errorCode != Ssl.SslErrorCode.SSL_ERROR_WANT_READ)) { @@ -885,6 +886,10 @@ internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback( // the chain will get properly disposed inside the VerifyRemoteCertificate call X509Chain chain = new X509Chain(); + if (options.CertificateChainPolicy is not null) + { + chain.ChainPolicy = options.CertificateChainPolicy; + } X509Certificate2? certificate = null; SafeSslHandle sslHandle = (SafeSslHandle)options.SslStream!._securityContext!; @@ -902,10 +907,16 @@ internal static Interop.Crypto.X509VerifyStatusCodeUniversal CertVerifyCallback( { // X509Certificate2(IntPtr) calls X509_dup, so the reference is appropriately tracked. X509Certificate2 chainCert = new X509Certificate2(certPtr); - chain.ChainPolicy.ExtraStore.Add(chainCert); - // first cert in the stack is the leaf cert - certificate ??= chainCert; + if (certificate is null) + { + // First cert in the stack is the leaf cert. + certificate = chainCert; + } + else + { + chain.ChainPolicy.ExtraStore.Add(chainCert); + } } } } @@ -1047,7 +1058,10 @@ private static unsafe int NewSessionCallback(IntPtr ssl, IntPtr session) IntPtr ctx = Ssl.SslGetSslCtx(ssl); IntPtr ptr = Ssl.SslCtxGetData(ctx); // while SSL_CTX is kept alive by reference from SSL, the same is not true - // for the stored GCHandle pointing to SafeSslContextHandle, which + // for the stored GCHandle pointing to SafeSslContextHandle. The managed + // SafeSslContextHandle may have been disposed and its GCHandle freed, in + // which case SslCtxGetData returns IntPtr.Zero and no GCHandle should be + // reconstructed. if (ptr != IntPtr.Zero) { GCHandle gch = GCHandle.FromIntPtr(ptr); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index beedabafa4fbc1..90dce59217d16d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -642,6 +642,13 @@ private void CompleteHandshake(SslAuthenticationOptions sslAuthenticationOptions { SendAuthResetSignal(new ReadOnlySpan(alertToken.Payload), ExceptionDispatchInfo.Capture(CreateCertificateValidationException(sslAuthenticationOptions, sslPolicyErrors, chainStatus))); } + +#if !TARGET_WINDOWS && !SYSNETSECURITY_NO_OPENSSL + // Release the SslStream reference that was needed during the handshake + // for the CertVerifyCallback. Clearing it avoids retaining the SslStream + // through the SslAuthenticationOptions -> GCHandle -> SSL chain. + _sslAuthenticationOptions.SslStream = null; +#endif } internal static Exception CreateCertificateValidationException(SslAuthenticationOptions options, SslPolicyErrors sslPolicyErrors, X509ChainStatusFlags chainStatus) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index d144a6a00d068b..5bde47cdaaa69c 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Runtime.ExceptionServices; using System.Security; using System.Security.Authentication; @@ -84,7 +83,7 @@ internal static bool EnableServerAiaDownloads #if TARGET_APPLE // on OSX, we have two implementations of SafeDeleteContext, so store a reference to the base class - internal SafeDeleteContext? _securityContext; + private SafeDeleteContext? _securityContext; #else internal SafeDeleteSslContext? _securityContext; #endif @@ -102,8 +101,8 @@ internal static bool EnableServerAiaDownloads private int _trailerSize = 16; private int _maxDataSize = 16354; - internal static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); - internal static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2"); + private static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); + private static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2"); // // Protocol properties