From 922e2070b5b4c281c2622bca61458c8c6439518d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 2 Oct 2026 14:41:16 +0100 Subject: [PATCH 1/8] Report the outcome of every physical connection attempt, identifying the client certificate used Adds ConfigurationOptions.ConnectionAttemptCompleted, raised exactly once per physical connection attempt (initial connect or reconnect, interactive or subscription): success once the Redis handshake completes, or failure if the attempt ends first. Unlike ConnectionFailed, failed reconnects are not suppressed while a server stays unreachable. The args identify the client certificate by subject, issuer and SHA-1/SHA-256 thumbprint, snapshotted rather than retained: the certificate actually sent after a completed handshake, or the one the selection callback returned when the handshake itself failed. See #3252. --- docs/Authentication.md | 20 ++ .../ConfigurationOptions.cs | 19 ++ .../ConnectionAttemptEventArgs.cs | 118 +++++++++++ .../ConnectionMultiplexer.Events.cs | 11 + src/StackExchange.Redis/PhysicalBridge.cs | 2 + src/StackExchange.Redis/PhysicalConnection.cs | 43 +++- .../PublicAPI/PublicAPI.Unshipped.txt | 12 ++ .../ConnectionAttemptUnitTests.cs | 197 ++++++++++++++++++ .../InProcessTestServer.cs | 34 ++- 9 files changed, 450 insertions(+), 6 deletions(-) create mode 100644 src/StackExchange.Redis/ConnectionAttemptEventArgs.cs create mode 100644 tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs diff --git a/docs/Authentication.md b/docs/Authentication.md index 15a673d198..76f2bdbca9 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -92,6 +92,26 @@ can be used; this uses the normal [`LocalCertificateSelectionCallback`](https://learn.microsoft.com/dotnet/api/system.net.security.remotecertificatevalidationcallback) API. +If you rotate client certificates and need to know whether a particular certificate worked (for example, to fall back to a +known-good certificate), subscribe to `options.ConnectionAttemptCompleted` *before* connecting. It is raised once for every +physical connection attempt, including every failed reconnect (unlike `ConnectionFailed`, which is raised once until the +connection is restored), and identifies the client certificate used for that attempt by subject, issuer and thumbprint: + +``` csharp +options.ConnectionAttemptCompleted += (sender, e) => +{ + if (e.ClientCertificateThumbprintSha256 is { } thumbprint) + { + if (e.IsSuccess) MarkHealthy(thumbprint); + else MarkSuspect(thumbprint, e.FailureType, e.Exception); + } +}; +``` + +Note that a rejected client certificate does not necessarily surface as an authentication failure: with TLS 1.3 the client +handshake completes before the server rejects the certificate, so the attempt typically fails as `SocketClosed` during the +Redis handshake that follows. + ## User certificates with implicit user authentication Historically, the client certificate only provided access to the server, but as the `default` user. From 8.6, diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs index 75a23c6309..e83e516268 100644 --- a/src/StackExchange.Redis/ConfigurationOptions.cs +++ b/src/StackExchange.Redis/ConfigurationOptions.cs @@ -385,6 +385,18 @@ private void SetWithValue(OptionFlags hasValue, ref T storage, T value) where [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1009:DeclareEventHandlersCorrectly", Justification = "Existing compatibility")] public event RemoteCertificateValidationCallback? CertificateValidation; + /// + /// Raised once for every physical connection attempt (initial connect or reconnect, interactive or subscription), reporting + /// whether that attempt completed the Redis handshake, and identifying the client certificate selected for it, if any; note + /// that this cannot be specified in the configuration-string. + /// + /// + /// Unlike , this is not suppressed while a server remains unreachable: every + /// failed reconnect attempt is reported. Subscribe before connecting, so that initial attempts are observed. Handlers are invoked + /// on a worker thread, not on the connection's own thread. + /// + public event EventHandler? ConnectionAttemptCompleted; + /// /// The default (not explicitly configured) options for this connection, fetched based on our parsed endpoints. /// @@ -1032,6 +1044,12 @@ internal RemoteCertificateValidationCallback? CertificateValidationCallback private set => CertificateValidation = value; } + internal EventHandler? ConnectionAttemptCompletedHandler + { + get => ConnectionAttemptCompleted; + private set => ConnectionAttemptCompleted = value; + } + /// /// How often to re-check the replication role of each connected server, in seconds (every minute by /// default), or 0 to never do so. @@ -1123,6 +1141,7 @@ public static ConfigurationOptions Parse(string configuration, bool ignoreUnknow commandMap = commandMap, CertificateValidationCallback = CertificateValidationCallback, CertificateSelectionCallback = CertificateSelectionCallback, + ConnectionAttemptCompletedHandler = ConnectionAttemptCompletedHandler, ChannelPrefix = ChannelPrefix.Clone(), #pragma warning disable CS0618 // Type or member is obsolete SocketManager = SocketManager, diff --git a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs new file mode 100644 index 0000000000..1819754f98 --- /dev/null +++ b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs @@ -0,0 +1,118 @@ +using System; +using System.Net; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Text; + +namespace StackExchange.Redis; + +/// +/// Describes the outcome of a single physical connection attempt; see . +/// +/// +/// Each physical connection attempt (initial connect or reconnect, interactive or subscription) reports exactly one outcome: +/// success once the Redis handshake has completed, or failure if the attempt ends before that point. A connection that fails +/// after it was established is a disconnect, not a failed attempt, and does not report here. +/// +public sealed class ConnectionAttemptEventArgs : EventArgs, ICompletable +{ + private readonly EventHandler? handler; + private readonly object sender; + private readonly string _physicalName; + + internal ConnectionAttemptEventArgs( + EventHandler? handler, + object sender, + EndPoint? endPoint, + ConnectionType connectionType, + bool isSuccess, + ConnectionFailureType failureType, + Exception? exception, + X509Certificate? clientCertificate, + string? physicalName) + { + this.handler = handler; + this.sender = sender; + EndPoint = endPoint; + ConnectionType = connectionType; + IsSuccess = isSuccess; + FailureType = failureType; + Exception = exception; + _physicalName = physicalName ?? GetType().Name; + + // Snapshot rather than retain: this is delivered on a worker after the attempt has finished, by which time + // whoever supplied the certificate may have disposed it - and we have no business holding it (or its key). + if (clientCertificate is not null) + { + ClientCertificateSubject = clientCertificate.Subject; + ClientCertificateIssuer = clientCertificate.Issuer; + ClientCertificateThumbprint = clientCertificate.GetCertHashString(); + using var sha256 = SHA256.Create(); + ClientCertificateThumbprintSha256 = ToHex(sha256.ComputeHash(clientCertificate.GetRawCertData())); + } + } + + private static string ToHex(byte[] value) => BitConverter.ToString(value).Replace("-", ""); + + /// + /// Gets the server endpoint of the attempt. + /// + public EndPoint? EndPoint { get; } + + /// + /// Gets the connection-type of the attempt. + /// + public ConnectionType ConnectionType { get; } + + /// + /// Whether the attempt completed the Redis handshake. + /// + public bool IsSuccess { get; } + + /// + /// The type of failure, or on success. + /// + public ConnectionFailureType FailureType { get; } + + /// + /// Gets the exception if available (this can be null, and is always null on success). + /// + public Exception? Exception { get; } + + /// + /// The subject of the client certificate used for this attempt, if any. + /// + /// + /// If the TLS handshake completed, this is the certificate actually sent to the server (null if the server did not ask for one). + /// If the handshake itself failed, this is the certificate returned by , when that + /// callback ran; a certificate supplied via SslClientAuthenticationOptions is only identified after a completed handshake. + /// This is only known when the library performs TLS itself; it is always null when a tunnel supplies the transport. + /// The same applies to the other ClientCertificate* members. + /// + public string? ClientCertificateSubject { get; } + + /// + /// The issuer of the client certificate used for this attempt, if any. + /// + public string? ClientCertificateIssuer { get; } + + /// + /// The SHA-1 thumbprint (upper-case hex, as ) of the client certificate used for this attempt, if any. + /// + public string? ClientCertificateThumbprint { get; } + + /// + /// The SHA-256 thumbprint (upper-case hex) of the client certificate used for this attempt, if any. + /// + public string? ClientCertificateThumbprintSha256 { get; } + + void ICompletable.AppendStormLog(StringBuilder sb) => + sb.Append("event, connection-attempt: ").Append(EndPoint != null ? Format.ToString(EndPoint) : "n/a"); + + bool ICompletable.TryComplete(bool isAsync) => ConnectionMultiplexer.TryCompleteHandler(handler, sender, this, isAsync); + + /// + /// Returns the physical name of the connection. + /// + public override string ToString() => _physicalName; +} diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs index fd5bd7c7f1..56f8d0541e 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs @@ -1,12 +1,23 @@ using System; using System.Net; using System.Runtime.CompilerServices; +using System.Security.Cryptography.X509Certificates; using StackExchange.Redis.Maintenance; namespace StackExchange.Redis; public partial class ConnectionMultiplexer { + internal void OnConnectionAttemptCompleted(EndPoint endpoint, ConnectionType connectionType, bool isSuccess, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, string? physicalName) + { + if (_isDisposed) return; + var handler = RawConfig.ConnectionAttemptCompletedHandler; + if (handler != null) + { + CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, failureType, exception, clientCertificate, physicalName)); + } + } + /// /// Raised whenever a physical connection fails. /// diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 7a8dbacbcd..9e94cde129 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -549,6 +549,8 @@ private void AbandonPendingBacklog(Exception ex, PhysicalConnection? connection) internal void OnFullyEstablished(PhysicalConnection connection, string source) { Trace("OnFullyEstablished"); + // the attempt itself succeeded (handshake complete), even if this bridge has since moved on and discards it below + connection.ReportAttemptOutcome(isSuccess: true, ConnectionFailureType.None, null); connection.SetIdle(); if (physical == connection && !isDisposed && ChangeState(State.ConnectedEstablishing, State.ConnectedEstablished)) { diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index cbedd42259..714330e682 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -95,6 +95,21 @@ private static readonly Message private int failureReported; + // one terminal outcome per physical attempt (see ConfigurationOptions.ConnectionAttemptCompleted); deliberately + // separate from failureReported, which only fires for the bridge's *current* connection + private int _attemptOutcomeReported; + private X509Certificate? _clientCertificate; // only captured when someone is listening for attempt outcomes + + internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failureType, Exception? exception) + { + if (Interlocked.CompareExchange(ref _attemptOutcomeReported, 1, 0) != 0) return; + var clientCertificate = Interlocked.Exchange(ref _clientCertificate, null); // nothing to gain from keeping it + if (BridgeCouldBeNull is { } bridge) + { + bridge.Multiplexer.OnConnectionAttemptCompleted(bridge.ServerEndPoint.EndPoint, connectionType, isSuccess, failureType, exception, clientCertificate, _physicalName); + } + } + private int clientSentQuit; private int lastWriteTickCount, lastReadTickCount, lastBeatTickCount; @@ -435,6 +450,7 @@ public void Dispose() Trace("Disconnected"); RecordConnectionFailed(ConnectionFailureType.ConnectionDisposed); } + ReportAttemptOutcome(isSuccess: false, ConnectionFailureType.ConnectionDisposed, null); // backstop for attempts abandoned without a recorded failure OnCloseEcho(); // ReSharper disable once GCSuppressFinalizeForTypeWithoutDestructor GC.SuppressFinalize(this); @@ -484,6 +500,7 @@ public void RecordConnectionFailed( Exception? outerException = innerException; IdentifyFailureType(innerException, ref failureType); + ReportAttemptOutcome(isSuccess: false, failureType, innerException); // no-op if already established (or already reported) var bridge = BridgeCouldBeNull; Message? nextMessage; @@ -1229,11 +1246,23 @@ static Stream DemandSocketStream(Socket? socket) var host = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); stream ??= DemandSocketStream(socket); + var observeAttempt = config.ConnectionAttemptCompletedHandler is not null; + var certificateSelection = config.CertificateSelectionCallback ?? GetAmbientClientCertificateCallback(); + if (observeAttempt && certificateSelection is { } selector) + { + // capture what was selected, so that a failed handshake can still say which certificate it used + certificateSelection = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => + { + var selected = selector(sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers); + _clientCertificate = selected; + return selected; + }; + } var ssl = new SslStream( innerStream: stream, leaveInnerStreamOpen: false, userCertificateValidationCallback: config.CertificateValidationCallback ?? GetAmbientIssuerCertificateCallback(), - userCertificateSelectionCallback: config.CertificateSelectionCallback ?? GetAmbientClientCertificateCallback(), + userCertificateSelectionCallback: certificateSelection, encryptionPolicy: EncryptionPolicy.RequireEncryption); try { @@ -1260,6 +1289,18 @@ static Stream DemandSocketStream(Socket? socket) bridge.Multiplexer.Logger?.LogErrorConnectionIssue(ex, ex.Message); throw; } + if (observeAttempt) + { + // after a successful handshake, the stream knows the certificate actually *sent*, which supersedes + // what the callback selected: the callback can run even when the server never asks for a certificate, + // in which case this is null. It also covers certificates supplied via SslClientAuthenticationOptions, + // which never pass through our callback. + try + { + _clientCertificate = ssl.LocalCertificate; + } + catch { /* best effort only */ } + } // Note on the "assert ssl.IsEncrypted after the handshake" advice: on every TFM we // target, SslStream.IsEncrypted (and IsSigned) is literally an alias for // IsAuthenticated, i.e. "handshake completed, no exception" - which is exactly what diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index ab058de62d..b141622d7c 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -1 +1,13 @@ #nullable enable +StackExchange.Redis.ConfigurationOptions.ConnectionAttemptCompleted -> System.EventHandler? +StackExchange.Redis.ConnectionAttemptEventArgs +StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateIssuer.get -> string? +StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateSubject.get -> string? +StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateThumbprint.get -> string? +StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateThumbprintSha256.get -> string? +StackExchange.Redis.ConnectionAttemptEventArgs.ConnectionType.get -> StackExchange.Redis.ConnectionType +StackExchange.Redis.ConnectionAttemptEventArgs.EndPoint.get -> System.Net.EndPoint? +StackExchange.Redis.ConnectionAttemptEventArgs.Exception.get -> System.Exception? +StackExchange.Redis.ConnectionAttemptEventArgs.FailureType.get -> StackExchange.Redis.ConnectionFailureType +StackExchange.Redis.ConnectionAttemptEventArgs.IsSuccess.get -> bool +override StackExchange.Redis.ConnectionAttemptEventArgs.ToString() -> string! diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs new file mode 100644 index 0000000000..3a7b0f53b6 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -0,0 +1,197 @@ +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Net; +using System.Net.Sockets; +#if !NETFRAMEWORK +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +#endif +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests; + +public class ConnectionAttemptUnitTests(ITestOutputHelper output) +{ + [Fact] + public async Task SuccessfulAttemptsAreReported() + { + using var server = new InProcessTestServer(output); + var options = server.GetClientConfig(); + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + + await WaitForAsync(() => attempts.Count(a => a.IsSuccess) >= 2); + var interactive = Assert.Single(attempts, a => a.ConnectionType == ConnectionType.Interactive); + var subscription = Assert.Single(attempts, a => a.ConnectionType == ConnectionType.Subscription); + foreach (var attempt in new[] { interactive, subscription }) + { + Assert.True(attempt.IsSuccess); + Assert.Equal(ConnectionFailureType.None, attempt.FailureType); + Assert.Null(attempt.Exception); + Assert.Null(attempt.ClientCertificateSubject); + Assert.Null(attempt.ClientCertificateThumbprint); + } + } + + [Fact] + public async Task EveryFailedReconnectIsReported() + { + // a port with nothing listening: every attempt is refused, so the server never becomes reachable, + // which is exactly when ConnectionFailed stops reporting + var endpoint = new IPEndPoint(IPAddress.Loopback, GetUnusedPort()); + var options = new ConfigurationOptions + { + EndPoints = { endpoint }, + AbortOnConnectFail = false, + ConnectTimeout = 1000, + HeartbeatInterval = TimeSpan.FromMilliseconds(100), + ReconnectRetryPolicy = new LinearRetry(50), + }; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + + await WaitForAsync(() => attempts.Count(a => a.ConnectionType == ConnectionType.Interactive) >= 4); + var interactive = attempts.Where(a => a.ConnectionType == ConnectionType.Interactive).ToArray(); + output.WriteLine($"interactive attempts: {interactive.Length}"); + Assert.All(interactive, a => + { + Assert.False(a.IsSuccess); + Assert.NotEqual(ConnectionFailureType.None, a.FailureType); + Assert.Equal(endpoint, a.EndPoint); + }); + } + +#if !NETFRAMEWORK // the in-process server only does TLS on .NET + [Fact] + public async Task AcceptedClientCertificateIsIdentified() + { + using var good = CreateClientCertificate("good-client"); + using var server = new InProcessTestServer(output, useSsl: true) + { + ClientCertificateValidator = cert => cert is not null && cert.GetCertHashString() == good.Thumbprint, + }; + var options = server.GetClientConfig(withPubSub: false); + options.CertificateSelection += (_, _, _, _, _) => good; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.True(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = Assert.Single(attempts); + Assert.True(attempt.IsSuccess); + Assert.Equal(good.Subject, attempt.ClientCertificateSubject); + Assert.Equal(good.Issuer, attempt.ClientCertificateIssuer); + Assert.Equal(good.Thumbprint, attempt.ClientCertificateThumbprint); + Assert.Equal(Sha256(good), attempt.ClientCertificateThumbprintSha256); + } + + [Fact] + public async Task RejectedClientCertificateIsIdentified() + { + using var good = CreateClientCertificate("good-client"); + using var bad = CreateClientCertificate("bad-client"); + using var server = new InProcessTestServer(output, useSsl: true) + { + ClientCertificateValidator = cert => cert is not null && cert.GetCertHashString() == good.Thumbprint, + }; + var options = server.GetClientConfig(withPubSub: false); + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + options.CertificateSelection += (_, _, _, _, _) => bad; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.False(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = attempts.First(); + output.WriteLine($"rejected attempt: {attempt.FailureType}: {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}"); + Assert.False(attempt.IsSuccess); + Assert.NotEqual(ConnectionFailureType.None, attempt.FailureType); + Assert.Equal(bad.Subject, attempt.ClientCertificateSubject); + Assert.Equal(bad.Thumbprint, attempt.ClientCertificateThumbprint); + Assert.Equal(Sha256(bad), attempt.ClientCertificateThumbprintSha256); + } + + [Fact] + public async Task ClientCertificateNotRequestedByServer() + { + // the selection callback still runs when the server does not ask for a certificate, but nothing is sent, + // so nothing should be reported + using var cert = CreateClientCertificate("unrequested-client"); + using var server = new InProcessTestServer(output, useSsl: true); + var options = server.GetClientConfig(withPubSub: false); + int selections = 0; + options.CertificateSelection += (_, _, _, _, _) => + { + System.Threading.Interlocked.Increment(ref selections); + return cert; + }; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.True(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = Assert.Single(attempts); + output.WriteLine($"selection callback invoked {selections} time(s); reported thumbprint: {attempt.ClientCertificateThumbprint ?? "(null)"}"); + Assert.True(attempt.IsSuccess); + Assert.Null(attempt.ClientCertificateThumbprint); + } + + private static X509Certificate2 CreateClientCertificate(string name) + { + using var rsa = RSA.Create(2048); + var request = new CertificateRequest($"CN={name}", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(new OidCollection { new Oid("1.3.6.1.5.5.7.3.2") }, false)); + var now = DateTimeOffset.UtcNow; + using var certificate = request.CreateSelfSigned(now.AddMinutes(-5), now.AddDays(1)); +#pragma warning disable SYSLIB0057 + return new X509Certificate2(certificate.Export(X509ContentType.Pfx)); +#pragma warning restore SYSLIB0057 + } + + private static string Sha256(X509Certificate2 certificate) + { + using var sha256 = SHA256.Create(); + return Convert.ToHexString(sha256.ComputeHash(certificate.RawData)); + } +#endif + + private static ConcurrentQueue Observe(ConfigurationOptions options) + { + var attempts = new ConcurrentQueue(); + options.ConnectionAttemptCompleted += (_, e) => attempts.Enqueue(e); + return attempts; + } + + private static async Task WaitForAsync(Func condition, int timeoutMilliseconds = 10000) + { + var giveUp = DateTime.UtcNow.AddMilliseconds(timeoutMilliseconds); + while (!condition()) + { + if (DateTime.UtcNow > giveUp) Assert.Fail("Condition not met before timeout"); + await Task.Delay(20); + } + await Task.Delay(100); // let any stragglers (e.g. a duplicate report, if there were one) arrive before asserting + } + + private static int GetUnusedPort() + { + var listener = new TcpListener(IPAddress.Loopback, 0); // not IDisposable on net481 + listener.Start(); + try + { + return ((IPEndPoint)listener.LocalEndpoint).Port; + } + finally + { + listener.Stop(); + } + } +} diff --git a/tests/StackExchange.Redis.Tests/InProcessTestServer.cs b/tests/StackExchange.Redis.Tests/InProcessTestServer.cs index 2276500bd2..06eb6c57b0 100644 --- a/tests/StackExchange.Redis.Tests/InProcessTestServer.cs +++ b/tests/StackExchange.Redis.Tests/InProcessTestServer.cs @@ -59,6 +59,14 @@ public InProcessTestServer(ITestOutputHelper? log = null, EndPoint? endpoint = n /// public RedisProtocol MaxProtocolVersion { get; set; } = RedisProtocol.Resp3; +#if !NETFRAMEWORK + /// + /// When set (and TLS is enabled), the server requires a client certificate, and accepts the handshake only if + /// this returns true for the certificate presented (which is null if the client sent none). + /// + public Func? ClientCertificateValidator { get; set; } +#endif + protected override RedisProtocol MaxProtocol => MaxProtocolVersion; public Task ConnectAsync(bool withPubSub = true, bool defaultOnly = false, WriteMode writeMode = WriteMode.Default, TextWriter? log = null) @@ -394,11 +402,27 @@ private sealed class InProcTunnel( async () => { using var ssl = new SslStream(serverTransport, leaveInnerStreamOpen: false); - await ssl.AuthenticateAsServerAsync( - server.GetServerCertificate(), - clientCertificateRequired: false, - enabledSslProtocols: SslProtocols.None, - checkCertificateRevocation: false).ConfigureAwait(false); + if (server.ClientCertificateValidator is { } validator) + { + await ssl.AuthenticateAsServerAsync( + new SslServerAuthenticationOptions + { + ServerCertificate = server.GetServerCertificate(), + ClientCertificateRequired = true, + EnabledSslProtocols = SslProtocols.None, + CertificateRevocationCheckMode = X509RevocationMode.NoCheck, + RemoteCertificateValidationCallback = (_, certificate, _, _) => validator(certificate), + }, + cancellationToken).ConfigureAwait(false); + } + else + { + await ssl.AuthenticateAsServerAsync( + server.GetServerCertificate(), + clientCertificateRequired: false, + enabledSslProtocols: SslProtocols.None, + checkCertificateRevocation: false).ConfigureAwait(false); + } var serverSide = new StreamDuplexPipe(ssl); await server.RunClientAsync(serverSide, node: node, state: null).ConfigureAwait(false); }, From 6d1c556d293a79b281b15ec1b3805709edadedf6 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 2 Oct 2026 14:49:35 +0100 Subject: [PATCH 2/8] Report the TLS host name per connection attempt; list the new ConfigurationOptions field in ExpectedFields TlsHostName is what the library asked TLS to authenticate (and sent as SNI): the resolved host, or the caller's TargetHost when SslClientAuthenticationOptions is used; null when the library did not perform TLS. --- src/StackExchange.Redis/ConnectionAttemptEventArgs.cs | 10 ++++++++++ .../ConnectionMultiplexer.Events.cs | 4 ++-- src/StackExchange.Redis/PhysicalConnection.cs | 6 ++++-- .../PublicAPI/PublicAPI.Unshipped.txt | 1 + tests/StackExchange.Redis.Tests/ConfigTests.cs | 1 + .../ConnectionAttemptUnitTests.cs | 3 +++ 6 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs index 1819754f98..55cbb264a1 100644 --- a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs +++ b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs @@ -29,6 +29,7 @@ internal ConnectionAttemptEventArgs( ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, + string? tlsHostName, string? physicalName) { this.handler = handler; @@ -38,6 +39,7 @@ internal ConnectionAttemptEventArgs( IsSuccess = isSuccess; FailureType = failureType; Exception = exception; + TlsHostName = tlsHostName; _physicalName = physicalName ?? GetType().Name; // Snapshot rather than retain: this is delivered on a worker after the attempt has finished, by which time @@ -74,6 +76,14 @@ internal ConnectionAttemptEventArgs( /// public ConnectionFailureType FailureType { get; } + /// + /// The host name the library asked TLS to authenticate for this attempt (sent as SNI, and used to validate the server + /// certificate), or null if the library did not perform TLS. This can differ from , for example when + /// is configured or a host name is inferred for an address endpoint. + /// + /// When SslClientAuthenticationOptions is used, this is its TargetHost, as supplied by the caller. + public string? TlsHostName { get; } + /// /// Gets the exception if available (this can be null, and is always null on success). /// diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs index 56f8d0541e..bc76631df9 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs @@ -8,13 +8,13 @@ namespace StackExchange.Redis; public partial class ConnectionMultiplexer { - internal void OnConnectionAttemptCompleted(EndPoint endpoint, ConnectionType connectionType, bool isSuccess, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, string? physicalName) + internal void OnConnectionAttemptCompleted(EndPoint endpoint, ConnectionType connectionType, bool isSuccess, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, string? tlsHostName, string? physicalName) { if (_isDisposed) return; var handler = RawConfig.ConnectionAttemptCompletedHandler; if (handler != null) { - CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, failureType, exception, clientCertificate, physicalName)); + CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, failureType, exception, clientCertificate, tlsHostName, physicalName)); } } diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 714330e682..228d928fd9 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -99,6 +99,7 @@ private static readonly Message // separate from failureReported, which only fires for the bridge's *current* connection private int _attemptOutcomeReported; private X509Certificate? _clientCertificate; // only captured when someone is listening for attempt outcomes + private string? _tlsHostName; // the host we asked TLS to authenticate (and send as SNI), if we performed TLS internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failureType, Exception? exception) { @@ -106,7 +107,7 @@ internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failure var clientCertificate = Interlocked.Exchange(ref _clientCertificate, null); // nothing to gain from keeping it if (BridgeCouldBeNull is { } bridge) { - bridge.Multiplexer.OnConnectionAttemptCompleted(bridge.ServerEndPoint.EndPoint, connectionType, isSuccess, failureType, exception, clientCertificate, _physicalName); + bridge.Multiplexer.OnConnectionAttemptCompleted(bridge.ServerEndPoint.EndPoint, connectionType, isSuccess, failureType, exception, clientCertificate, _tlsHostName, _physicalName); } } @@ -1243,7 +1244,7 @@ static Stream DemandSocketStream(Socket? socket) if (config.Ssl) { log?.LogInformationConfiguringTLS(); - var host = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); + var host = _tlsHostName = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); stream ??= DemandSocketStream(socket); var observeAttempt = config.ConnectionAttemptCompletedHandler is not null; @@ -1272,6 +1273,7 @@ static Stream DemandSocketStream(Socket? socket) var configOptions = config.SslClientAuthenticationOptions?.Invoke(host); if (configOptions is not null) { + _tlsHostName = configOptions.TargetHost; // the caller's options decide what is actually sent await ssl.AuthenticateAsClientAsync(configOptions).ForAwait(); } else diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index b141622d7c..83da525535 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -10,4 +10,5 @@ StackExchange.Redis.ConnectionAttemptEventArgs.EndPoint.get -> System.Net.EndPoi StackExchange.Redis.ConnectionAttemptEventArgs.Exception.get -> System.Exception? StackExchange.Redis.ConnectionAttemptEventArgs.FailureType.get -> StackExchange.Redis.ConnectionFailureType StackExchange.Redis.ConnectionAttemptEventArgs.IsSuccess.get -> bool +StackExchange.Redis.ConnectionAttemptEventArgs.TlsHostName.get -> string? override StackExchange.Redis.ConnectionAttemptEventArgs.ToString() -> string! diff --git a/tests/StackExchange.Redis.Tests/ConfigTests.cs b/tests/StackExchange.Redis.Tests/ConfigTests.cs index e0cbb8c0ae..6b487dbdab 100644 --- a/tests/StackExchange.Redis.Tests/ConfigTests.cs +++ b/tests/StackExchange.Redis.Tests/ConfigTests.cs @@ -79,6 +79,7 @@ orderby name "commandMap", "configChannel", "configCheckSeconds", + "ConnectionAttemptCompleted", "connectRetry", "connectTimeout", "defaultDatabase", diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index 3a7b0f53b6..d2d219dd47 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -33,6 +33,7 @@ public async Task SuccessfulAttemptsAreReported() Assert.Null(attempt.Exception); Assert.Null(attempt.ClientCertificateSubject); Assert.Null(attempt.ClientCertificateThumbprint); + Assert.Null(attempt.TlsHostName); // no TLS } } @@ -88,6 +89,8 @@ public async Task AcceptedClientCertificateIsIdentified() Assert.Equal(good.Issuer, attempt.ClientCertificateIssuer); Assert.Equal(good.Thumbprint, attempt.ClientCertificateThumbprint); Assert.Equal(Sha256(good), attempt.ClientCertificateThumbprintSha256); + Assert.Equal(options.ResolveTlsHostName(attempt.EndPoint!), attempt.TlsHostName); + Assert.False(string.IsNullOrEmpty(attempt.TlsHostName)); } [Fact] From be00860b0f58d60f71cbb4d4ab59a148700d08d8 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 2 Oct 2026 14:58:57 +0100 Subject: [PATCH 3/8] Report the stage reached and the server certificate check per connection attempt ConnectionAttemptStage (Connect, Tunnel, Tls, Handshake, Established; spaced by 100 to leave room) says how far an attempt got. ServerCertificatePolicyErrors/ChainStatus/Accepted capture the server certificate validation, observed via a wrapping callback (platform-default semantics when none is configured); when that injected callback rejects, the AuthenticationException is re-described so observing does not cost the platform's error detail. The TLS block now resolves SslClientAuthenticationOptions before constructing the SslStream, and only wraps callbacks the options do not supply: SslStream rejects a constructor callback that differs from one in the options. --- docs/Authentication.md | 7 +- .../ConnectionAttemptEventArgs.cs | 43 +++++++ .../ConnectionAttemptStage.cs | 40 +++++++ .../ConnectionMultiplexer.Events.cs | 14 ++- src/StackExchange.Redis/PhysicalConnection.cs | 108 +++++++++++++++--- .../PublicAPI/PublicAPI.Unshipped.txt | 11 ++ .../ConnectionAttemptUnitTests.cs | 95 ++++++++++++++- .../InProcessTestServer.cs | 9 +- 8 files changed, 305 insertions(+), 22 deletions(-) create mode 100644 src/StackExchange.Redis/ConnectionAttemptStage.cs diff --git a/docs/Authentication.md b/docs/Authentication.md index 76f2bdbca9..f69921c087 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -110,7 +110,12 @@ options.ConnectionAttemptCompleted += (sender, e) => Note that a rejected client certificate does not necessarily surface as an authentication failure: with TLS 1.3 the client handshake completes before the server rejects the certificate, so the attempt typically fails as `SocketClosed` during the -Redis handshake that follows. +Redis handshake that follows. `e.Stage` says how far the attempt got (`Connect`, `Tunnel`, `Tls`, `Handshake`, or +`Established` on success), so that case reports `Handshake`. + +The same event also describes the server side of TLS: `e.TlsHostName` is the host name sent as SNI (which can differ from +`e.EndPoint`), and `e.ServerCertificatePolicyErrors`, `e.ServerCertificateChainStatus` and `e.ServerCertificateAccepted` report +what the platform found when validating the server certificate, and what was decided. ## User certificates with implicit user authentication diff --git a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs index 55cbb264a1..5845ed6dc1 100644 --- a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs +++ b/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs @@ -1,5 +1,6 @@ using System; using System.Net; +using System.Net.Security; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Text; @@ -26,10 +27,12 @@ internal ConnectionAttemptEventArgs( EndPoint? endPoint, ConnectionType connectionType, bool isSuccess, + ConnectionAttemptStage stage, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, string? tlsHostName, + ServerCertificateCheck? serverCertificateCheck, string? physicalName) { this.handler = handler; @@ -37,9 +40,16 @@ internal ConnectionAttemptEventArgs( EndPoint = endPoint; ConnectionType = connectionType; IsSuccess = isSuccess; + Stage = stage; FailureType = failureType; Exception = exception; TlsHostName = tlsHostName; + if (serverCertificateCheck is not null) + { + ServerCertificatePolicyErrors = serverCertificateCheck.PolicyErrors; + ServerCertificateChainStatus = serverCertificateCheck.ChainStatus; + ServerCertificateAccepted = serverCertificateCheck.Accepted; + } _physicalName = physicalName ?? GetType().Name; // Snapshot rather than retain: this is delivered on a worker after the attempt has finished, by which time @@ -71,6 +81,11 @@ internal ConnectionAttemptEventArgs( /// public bool IsSuccess { get; } + /// + /// How far the attempt got: on success, otherwise the stage during which it failed. + /// + public ConnectionAttemptStage Stage { get; } + /// /// The type of failure, or on success. /// @@ -84,6 +99,27 @@ internal ConnectionAttemptEventArgs( /// When SslClientAuthenticationOptions is used, this is its TargetHost, as supplied by the caller. public string? TlsHostName { get; } + /// + /// The errors the platform found when validating the server certificate, if the library observed that validation; null otherwise + /// (for example: TLS was not used, the handshake failed before validation, or SslClientAuthenticationOptions supplied its + /// own validation callback). + /// + /// Non-empty errors do not imply rejection: a configured callback + /// may accept the certificate regardless; see . + public SslPolicyErrors? ServerCertificatePolicyErrors { get; } + + /// + /// The combined status flags of the server certificate chain, under the same conditions as ; + /// this gives the detail behind (for example, an untrusted root or an expired certificate). + /// + public X509ChainStatusFlags? ServerCertificateChainStatus { get; } + + /// + /// Whether the server certificate was accepted (by the configured validation callback, or by the platform default when none is + /// configured), under the same conditions as . + /// + public bool? ServerCertificateAccepted { get; } + /// /// Gets the exception if available (this can be null, and is always null on success). /// @@ -126,3 +162,10 @@ void ICompletable.AppendStormLog(StringBuilder sb) => /// public override string ToString() => _physicalName; } + +internal sealed class ServerCertificateCheck(SslPolicyErrors policyErrors, X509ChainStatusFlags chainStatus, bool accepted) +{ + public SslPolicyErrors PolicyErrors { get; } = policyErrors; + public X509ChainStatusFlags ChainStatus { get; } = chainStatus; + public bool Accepted { get; } = accepted; +} diff --git a/src/StackExchange.Redis/ConnectionAttemptStage.cs b/src/StackExchange.Redis/ConnectionAttemptStage.cs new file mode 100644 index 0000000000..35a328e751 --- /dev/null +++ b/src/StackExchange.Redis/ConnectionAttemptStage.cs @@ -0,0 +1,40 @@ +namespace StackExchange.Redis; + +/// +/// How far a physical connection attempt got; see . +/// +/// Values are spaced apart, so that finer-grained stages can be added later without renumbering; compare stages by order, not equality, where "at least as far as" is meant. +public enum ConnectionAttemptStage +{ + /// + /// The stage is not known. + /// + Unknown = 0, + + /// + /// Establishing the connection: resolving and connecting the socket, or (when a tunnel supplies the entire transport, + /// including any TLS) creating that transport. + /// + Connect = 100, + + /// + /// Pre-authentication negotiation by the configured (for example, an HTTP proxy CONNECT). + /// + Tunnel = 200, + + /// + /// The TLS handshake performed by the library. + /// + Tls = 300, + + /// + /// The Redis handshake (for example HELLO, AUTH, and the initial configuration queries). Note that with TLS 1.3, a + /// client certificate rejected by the server is typically observed here, rather than during . + /// + Handshake = 400, + + /// + /// The attempt completed successfully. + /// + Established = 500, +} diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs index bc76631df9..4bb4f4667c 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs @@ -8,13 +8,23 @@ namespace StackExchange.Redis; public partial class ConnectionMultiplexer { - internal void OnConnectionAttemptCompleted(EndPoint endpoint, ConnectionType connectionType, bool isSuccess, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate, string? tlsHostName, string? physicalName) + internal void OnConnectionAttemptCompleted( + EndPoint endpoint, + ConnectionType connectionType, + bool isSuccess, + ConnectionAttemptStage stage, + ConnectionFailureType failureType, + Exception? exception, + X509Certificate? clientCertificate, + string? tlsHostName, + ServerCertificateCheck? serverCertificateCheck, + string? physicalName) { if (_isDisposed) return; var handler = RawConfig.ConnectionAttemptCompletedHandler; if (handler != null) { - CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, failureType, exception, clientCertificate, tlsHostName, physicalName)); + CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, stage, failureType, exception, clientCertificate, tlsHostName, serverCertificateCheck, physicalName)); } } diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 228d928fd9..93c9aa36b6 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -100,6 +100,8 @@ private static readonly Message private int _attemptOutcomeReported; private X509Certificate? _clientCertificate; // only captured when someone is listening for attempt outcomes private string? _tlsHostName; // the host we asked TLS to authenticate (and send as SNI), if we performed TLS + private ServerCertificateCheck? _serverCertificateCheck; // only captured when someone is listening for attempt outcomes + private volatile ConnectionAttemptStage _attemptStage = ConnectionAttemptStage.Connect; internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failureType, Exception? exception) { @@ -107,10 +109,57 @@ internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failure var clientCertificate = Interlocked.Exchange(ref _clientCertificate, null); // nothing to gain from keeping it if (BridgeCouldBeNull is { } bridge) { - bridge.Multiplexer.OnConnectionAttemptCompleted(bridge.ServerEndPoint.EndPoint, connectionType, isSuccess, failureType, exception, clientCertificate, _tlsHostName, _physicalName); + var stage = isSuccess ? ConnectionAttemptStage.Established : _attemptStage; + bridge.Multiplexer.OnConnectionAttemptCompleted( + bridge.ServerEndPoint.EndPoint, + connectionType, + isSuccess, + stage, + failureType, + exception, + clientCertificate, + _tlsHostName, + Volatile.Read(ref _serverCertificateCheck), + _physicalName); } } + // capture what was selected, so that a failed handshake can still say which certificate it used + private LocalCertificateSelectionCallback ObserveClientCertificate(LocalCertificateSelectionCallback selector) + => (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => + { + var selected = selector(sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers); + _clientCertificate = selected; + return selected; + }; + + // capture what the platform thought of the server certificate, and what was decided; SslStream does not expose + // this afterwards, and an AuthenticationException only describes it in (TFM-specific) message text + private RemoteCertificateValidationCallback ObserveServerCertificate(RemoteCertificateValidationCallback? validator) + => (sender, certificate, chain, sslPolicyErrors) => + { + var chainStatus = X509ChainStatusFlags.NoError; + if (chain is not null) + { + foreach (var status in chain.ChainStatus) + { + chainStatus |= status.Status; + } + } + + // with no callback configured, this is the platform default: accept only a certificate without errors + var accepted = validator is null ? sslPolicyErrors == SslPolicyErrors.None : validator(sender, certificate, chain, sslPolicyErrors); + Volatile.Write(ref _serverCertificateCheck, new ServerCertificateCheck(sslPolicyErrors, chainStatus, accepted)); + return accepted; + }; + + private static void OnTlsFault(PhysicalBridge bridge, Exception ex) + { + Debug.WriteLine(ex.Message); + bridge.Multiplexer.SetAuthSuspect(ex); + bridge.Multiplexer.Logger?.LogErrorConnectionIssue(ex, ex.Message); + } + private int clientSentQuit; private int lastWriteTickCount, lastReadTickCount, lastBeatTickCount; @@ -1228,6 +1277,7 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log) StartTransportReading(transport); log?.LogInformationTransportConnected(bridge.Name, transport.IsEncrypted); + _attemptStage = ConnectionAttemptStage.Handshake; await bridge.OnConnectedAsync(this, log).ForAwait(); return true; } @@ -1235,6 +1285,7 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log) var tunnel = config.Tunnel; if (tunnel is not null) { + _attemptStage = ConnectionAttemptStage.Tunnel; stream = await tunnel.BeforeAuthenticateAsync(bridge.ServerEndPoint.EndPoint, bridge.ConnectionType, socket, CancellationToken.None).ForAwait(); } @@ -1243,26 +1294,46 @@ static Stream DemandSocketStream(Socket? socket) if (config.Ssl) { + _attemptStage = ConnectionAttemptStage.Tls; log?.LogInformationConfiguringTLS(); var host = _tlsHostName = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); stream ??= DemandSocketStream(socket); - var observeAttempt = config.ConnectionAttemptCompletedHandler is not null; + var certificateValidation = config.CertificateValidationCallback ?? GetAmbientIssuerCertificateCallback(); var certificateSelection = config.CertificateSelectionCallback ?? GetAmbientClientCertificateCallback(); - if (observeAttempt && certificateSelection is { } selector) + bool optionsValidate = false, optionsSelect = false; +#if NET + SslClientAuthenticationOptions? configOptions; + try { - // capture what was selected, so that a failed handshake can still say which certificate it used - certificateSelection = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) => - { - var selected = selector(sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers); - _clientCertificate = selected; - return selected; - }; + configOptions = config.SslClientAuthenticationOptions?.Invoke(host); + } + catch (Exception ex) + { + OnTlsFault(bridge, ex); + throw; + } + if (configOptions is not null) + { + _tlsHostName = configOptions.TargetHost; // the caller's options decide what is actually sent + optionsValidate = configOptions.RemoteCertificateValidationCallback is not null; + optionsSelect = configOptions.LocalCertificateSelectionCallback is not null; + } +#endif + var observeAttempt = config.ConnectionAttemptCompletedHandler is not null; + var observingDefaultValidation = false; // i.e. we supplied the only validation callback + if (observeAttempt) + { + observingDefaultValidation = !optionsValidate && certificateValidation is null; + // SslStream refuses a constructor callback that differs from one also supplied via the options, so we + // only observe through the callbacks that the options leave to us + if (!optionsValidate) certificateValidation = ObserveServerCertificate(certificateValidation); + if (!optionsSelect && certificateSelection is not null) certificateSelection = ObserveClientCertificate(certificateSelection); } var ssl = new SslStream( innerStream: stream, leaveInnerStreamOpen: false, - userCertificateValidationCallback: config.CertificateValidationCallback ?? GetAmbientIssuerCertificateCallback(), + userCertificateValidationCallback: certificateValidation, userCertificateSelectionCallback: certificateSelection, encryptionPolicy: EncryptionPolicy.RequireEncryption); try @@ -1270,10 +1341,8 @@ static Stream DemandSocketStream(Socket? socket) try { #if NET - var configOptions = config.SslClientAuthenticationOptions?.Invoke(host); if (configOptions is not null) { - _tlsHostName = configOptions.TargetHost; // the caller's options decide what is actually sent await ssl.AuthenticateAsClientAsync(configOptions).ForAwait(); } else @@ -1286,9 +1355,7 @@ static Stream DemandSocketStream(Socket? socket) } catch (Exception ex) { - Debug.WriteLine(ex.Message); - bridge.Multiplexer.SetAuthSuspect(ex); - bridge.Multiplexer.Logger?.LogErrorConnectionIssue(ex, ex.Message); + OnTlsFault(bridge, ex); throw; } if (observeAttempt) @@ -1320,6 +1387,14 @@ static Stream DemandSocketStream(Socket? socket) } catch (AuthenticationException authexception) { + if (observingDefaultValidation && Volatile.Read(ref _serverCertificateCheck) is { Accepted: false } check) + { + // Supplying a callback (only to observe) changes the platform's message from describing the errors to + // "rejected by the provided callback"; restore that detail, rather than lose it by observing. + authexception = new AuthenticationException( + $"The remote certificate is invalid: {check.PolicyErrors} (chain status: {check.ChainStatus}).", + authexception); + } RecordConnectionFailed(ConnectionFailureType.AuthenticationFailure, authexception, isInitialConnect: true); bridge.Multiplexer.Trace("Encryption failure"); return false; @@ -1334,6 +1409,7 @@ static Stream DemandSocketStream(Socket? socket) log?.LogInformationConnected(bridge.Name); + _attemptStage = ConnectionAttemptStage.Handshake; await bridge.OnConnectedAsync(this, log).ForAwait(); return true; } diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index 83da525535..4e2cc10709 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -10,5 +10,16 @@ StackExchange.Redis.ConnectionAttemptEventArgs.EndPoint.get -> System.Net.EndPoi StackExchange.Redis.ConnectionAttemptEventArgs.Exception.get -> System.Exception? StackExchange.Redis.ConnectionAttemptEventArgs.FailureType.get -> StackExchange.Redis.ConnectionFailureType StackExchange.Redis.ConnectionAttemptEventArgs.IsSuccess.get -> bool +StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificateAccepted.get -> bool? +StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificateChainStatus.get -> System.Security.Cryptography.X509Certificates.X509ChainStatusFlags? +StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificatePolicyErrors.get -> System.Net.Security.SslPolicyErrors? +StackExchange.Redis.ConnectionAttemptEventArgs.Stage.get -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Connect = 100 -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Established = 500 -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Handshake = 400 -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Tls = 300 -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Tunnel = 200 -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConnectionAttemptStage.Unknown = 0 -> StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptEventArgs.TlsHostName.get -> string? override StackExchange.Redis.ConnectionAttemptEventArgs.ToString() -> string! diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index d2d219dd47..c2a533e8c3 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Sockets; #if !NETFRAMEWORK +using System.Net.Security; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; #endif @@ -29,11 +30,14 @@ public async Task SuccessfulAttemptsAreReported() foreach (var attempt in new[] { interactive, subscription }) { Assert.True(attempt.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Established, attempt.Stage); Assert.Equal(ConnectionFailureType.None, attempt.FailureType); Assert.Null(attempt.Exception); Assert.Null(attempt.ClientCertificateSubject); Assert.Null(attempt.ClientCertificateThumbprint); Assert.Null(attempt.TlsHostName); // no TLS + Assert.Null(attempt.ServerCertificatePolicyErrors); + Assert.Null(attempt.ServerCertificateAccepted); } } @@ -61,6 +65,7 @@ public async Task EveryFailedReconnectIsReported() Assert.All(interactive, a => { Assert.False(a.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Connect, a.Stage); Assert.NotEqual(ConnectionFailureType.None, a.FailureType); Assert.Equal(endpoint, a.EndPoint); }); @@ -91,6 +96,13 @@ public async Task AcceptedClientCertificateIsIdentified() Assert.Equal(Sha256(good), attempt.ClientCertificateThumbprintSha256); Assert.Equal(options.ResolveTlsHostName(attempt.EndPoint!), attempt.TlsHostName); Assert.False(string.IsNullOrEmpty(attempt.TlsHostName)); + Assert.Equal(ConnectionAttemptStage.Established, attempt.Stage); + + // the test server's certificate is self-signed, and accepted by the test's own validation callback regardless + output.WriteLine($"server certificate: {attempt.ServerCertificatePolicyErrors} / {attempt.ServerCertificateChainStatus}, accepted: {attempt.ServerCertificateAccepted}"); + Assert.True(attempt.ServerCertificateAccepted); + Assert.NotNull(attempt.ServerCertificatePolicyErrors); + Assert.NotNull(attempt.ServerCertificateChainStatus); } [Fact] @@ -113,8 +125,10 @@ public async Task RejectedClientCertificateIsIdentified() await WaitForAsync(() => !attempts.IsEmpty); var attempt = attempts.First(); - output.WriteLine($"rejected attempt: {attempt.FailureType}: {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}"); + output.WriteLine($"rejected attempt: {attempt.Stage}, {attempt.FailureType}: {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}"); Assert.False(attempt.IsSuccess); + Assert.True(attempt.Stage is ConnectionAttemptStage.Tls or ConnectionAttemptStage.Handshake); // TLS 1.3: Handshake, TLS 1.2: Tls + Assert.True(attempt.ServerCertificateAccepted); // the failure is the client certificate, not the server's Assert.NotEqual(ConnectionFailureType.None, attempt.FailureType); Assert.Equal(bad.Subject, attempt.ClientCertificateSubject); Assert.Equal(bad.Thumbprint, attempt.ClientCertificateThumbprint); @@ -147,6 +161,85 @@ public async Task ClientCertificateNotRequestedByServer() Assert.Null(attempt.ClientCertificateThumbprint); } + [Fact] + public async Task ServerCertificateRejectedByPlatformDefault() + { + // no validation callback configured: the library observes validation through its own callback, which must keep the + // platform-default semantics (reject a certificate with any errors - here, a self-signed one) + using var server = new InProcessTestServer(output, useSsl: true); + var options = server.GetClientConfig(withPubSub: false, validateServerCertificate: false); + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.False(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = attempts.First(); + output.WriteLine($"server certificate: {attempt.ServerCertificatePolicyErrors} / {attempt.ServerCertificateChainStatus}; {attempt.FailureType}: {attempt.Exception?.Message}"); + Assert.False(attempt.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Tls, attempt.Stage); + Assert.Equal(ConnectionFailureType.AuthenticationFailure, attempt.FailureType); + Assert.False(attempt.ServerCertificateAccepted); + Assert.True(attempt.ServerCertificatePolicyErrors!.Value.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors)); + Assert.NotEqual(X509ChainStatusFlags.NoError, attempt.ServerCertificateChainStatus); + + // observing must not cost the detail that the platform's own message would have given + Assert.Contains(nameof(SslPolicyErrors.RemoteCertificateChainErrors), attempt.Exception?.Message); + } + + [Fact] + public async Task ServerCertificateRejectedByCallback() + { + using var server = new InProcessTestServer(output, useSsl: true); + var options = server.GetClientConfig(withPubSub: false, validateServerCertificate: false); + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + options.CertificateValidation += (_, _, _, _) => false; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.False(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = attempts.First(); + Assert.False(attempt.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Tls, attempt.Stage); + Assert.False(attempt.ServerCertificateAccepted); + Assert.NotNull(attempt.ServerCertificatePolicyErrors); + } + + [Fact] + public async Task OptionsValidationCallbackIsNotDisplaced() + { + // SslStream throws if a constructor callback differs from one in SslClientAuthenticationOptions; observing must + // not introduce a constructor callback when the options supply their own + using var server = new InProcessTestServer(output, useSsl: true); + var options = server.GetClientConfig(withPubSub: false, validateServerCertificate: false); + int validations = 0; + options.SslClientAuthenticationOptions = host => new SslClientAuthenticationOptions + { + TargetHost = host, + RemoteCertificateValidationCallback = (_, _, _, _) => + { + System.Threading.Interlocked.Increment(ref validations); + return true; + }, + }; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.True(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = Assert.Single(attempts); + Assert.True(attempt.IsSuccess); + Assert.True(validations > 0); + Assert.Null(attempt.ServerCertificateAccepted); // the options own validation, so the library did not observe it + Assert.Equal(options.ResolveTlsHostName(attempt.EndPoint!), attempt.TlsHostName); + } + private static X509Certificate2 CreateClientCertificate(string name) { using var rsa = RSA.Create(2048); diff --git a/tests/StackExchange.Redis.Tests/InProcessTestServer.cs b/tests/StackExchange.Redis.Tests/InProcessTestServer.cs index 06eb6c57b0..047b6d04de 100644 --- a/tests/StackExchange.Redis.Tests/InProcessTestServer.cs +++ b/tests/StackExchange.Redis.Tests/InProcessTestServer.cs @@ -102,7 +102,12 @@ public override TypedRedisValue Execute(RedisClient client, in RedisRequest requ return result; } - public ConfigurationOptions GetClientConfig(bool withPubSub = true, bool defaultOnly = false, WriteMode writeMode = WriteMode.Default) + /// Whether to enable the pub/sub commands. + /// Whether to connect only to the default endpoint. + /// The write mode to use. + /// When using TLS, whether to configure a validation callback that trusts this server's + /// (self-signed) certificate; when false, the platform default applies, which rejects it. + public ConfigurationOptions GetClientConfig(bool withPubSub = true, bool defaultOnly = false, WriteMode writeMode = WriteMode.Default, bool validateServerCertificate = true) { var commands = GetCommands(); if (!withPubSub) @@ -135,7 +140,7 @@ public ConfigurationOptions GetClientConfig(bool withPubSub = true, bool default TestConfig.ApplyMaintenanceDefault(config); if (!string.IsNullOrEmpty(Password)) config.Password = Password; config.Ssl = UseSsl; // explicitly, ignore provider defaults - if (UseSsl) + if (UseSsl && validateServerCertificate) { #if !NETFRAMEWORK config.CertificateValidation += _certificateValidationCallback; From f873a26d7eef09c8f9a0c773c25af4e7283bbaa4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 2 Oct 2026 16:02:40 +0100 Subject: [PATCH 4/8] Address review: never let observing break a connection; classify AUTH failures correctly - Guard the certificate snapshot per value, and the whole reporting path: reading a certificate disposed mid-attempt throws (measured: CryptographicException), and this runs on the connection's own path, including establishing a healthy connection. - Rename ConnectionAttemptEventArgs to ConnectionAttemptCompletedEventArgs, matching the event. - Add SequenceNumber and CompletedTimeUtc, since handlers on worker threads can observe outcomes out of order. - Document the classification: a wrong password is Handshake + AuthenticationFailure (the server replied), a TLS 1.3 client-certificate rejection is Handshake + SocketClosed (the server hung up); plus guidance on corroboration, retrying the primary, fallback expiry and ordering; SHA-1 and PII notes. - Tests: wrong password (plaintext, and TLS with an accepted certificate), the documented classifier, a disposed certificate, and outcome sequencing. --- docs/Authentication.md | 43 ++++-- .../ConfigurationOptions.cs | 4 +- ...=> ConnectionAttemptCompletedEventArgs.cs} | 57 ++++++-- .../ConnectionAttemptStage.cs | 2 +- .../ConnectionMultiplexer.Events.cs | 20 ++- src/StackExchange.Redis/PhysicalConnection.cs | 14 ++ .../PublicAPI/PublicAPI.Unshipped.txt | 36 ++--- .../ConnectionAttemptUnitTests.cs | 131 +++++++++++++++++- 8 files changed, 265 insertions(+), 42 deletions(-) rename src/StackExchange.Redis/{ConnectionAttemptEventArgs.cs => ConnectionAttemptCompletedEventArgs.cs} (74%) diff --git a/docs/Authentication.md b/docs/Authentication.md index f69921c087..869155bfc4 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -100,23 +100,48 @@ connection is restored), and identifies the client certificate used for that att ``` csharp options.ConnectionAttemptCompleted += (sender, e) => { - if (e.ClientCertificateThumbprintSha256 is { } thumbprint) - { - if (e.IsSuccess) MarkHealthy(thumbprint); - else MarkSuspect(thumbprint, e.FailureType, e.Exception); - } + if (e.ClientCertificateThumbprintSha256 is not { } thumbprint) return; // no client certificate involved + + if (e.IsSuccess) RecordSuccess(thumbprint, e.SequenceNumber); + else if (IsLikelyClientCertificateRejection(e)) RecordSuspectedRejection(thumbprint, e.SequenceNumber); + // anything else (socket connect, server certificate, a wrong password, ...) says nothing about the client certificate }; + +static bool IsLikelyClientCertificateRejection(ConnectionAttemptCompletedEventArgs e) + => !e.IsSuccess + && e.ServerCertificateAccepted == true // the server certificate was fine... + && ((e.Stage == ConnectionAttemptStage.Tls && e.FailureType == ConnectionFailureType.AuthenticationFailure) // TLS 1.2 + || (e.Stage == ConnectionAttemptStage.Handshake && e.FailureType == ConnectionFailureType.SocketClosed)); // TLS 1.3 ``` -Note that a rejected client certificate does not necessarily surface as an authentication failure: with TLS 1.3 the client -handshake completes before the server rejects the certificate, so the attempt typically fails as `SocketClosed` during the -Redis handshake that follows. `e.Stage` says how far the attempt got (`Connect`, `Tunnel`, `Tls`, `Handshake`, or -`Established` on success), so that case reports `Handshake`. +`e.Stage` says how far the attempt got: `Connect`, `Tunnel`, `Tls`, `Handshake`, or `Established` on success. Telling a rejected +client certificate apart from other failures needs both the stage and the failure type: + +- With TLS 1.2, the server rejects the certificate during the TLS handshake: `Tls` + `AuthenticationFailure`. +- With TLS 1.3, the client's TLS handshake completes *before* the server rejects the certificate, and the server then hangs up + during the Redis handshake that follows: `Handshake` + `SocketClosed`. +- A wrong password or ACL problem is the server *replying* with an error during the Redis handshake: `Handshake` + + `AuthenticationFailure`. That says nothing about the certificate, and must not count against it. + +Even so, `SocketClosed` is a weak signal: server restarts, connection resets, `maxclients` and network interruptions look the +same. If you act on it, for example by falling back to an older certificate: + +- **require corroboration**, such as several consecutive suspected rejections of the same certificate, rather than reacting to + one; +- **keep retrying the primary certificate** periodically, so that a fallback is never permanent; +- **check the fallback certificate's expiry** before using it, and consider whether falling back is acceptable at all: certificates + are often rotated because the old one is expiring or compromised; +- **don't assume ordering**: handlers run on worker threads, so outcomes can arrive out of order, concurrently, or after the next + attempt has already selected its certificate. Use `e.SequenceNumber` (or `e.CompletedTimeUtc`) to order them. The same event also describes the server side of TLS: `e.TlsHostName` is the host name sent as SNI (which can differ from `e.EndPoint`), and `e.ServerCertificatePolicyErrors`, `e.ServerCertificateChainStatus` and `e.ServerCertificateAccepted` report what the platform found when validating the server certificate, and what was decided. +Two cautions: the SHA-1 `e.ClientCertificateThumbprint` identifies a certificate but is not a basis for trust decisions (prefer +`e.ClientCertificateThumbprintSha256`), and certificate subjects can contain tenant or personal identifiers, so consider that +before logging them. + ## User certificates with implicit user authentication Historically, the client certificate only provided access to the server, but as the `default` user. From 8.6, diff --git a/src/StackExchange.Redis/ConfigurationOptions.cs b/src/StackExchange.Redis/ConfigurationOptions.cs index e83e516268..686723c71b 100644 --- a/src/StackExchange.Redis/ConfigurationOptions.cs +++ b/src/StackExchange.Redis/ConfigurationOptions.cs @@ -395,7 +395,7 @@ private void SetWithValue(OptionFlags hasValue, ref T storage, T value) where /// failed reconnect attempt is reported. Subscribe before connecting, so that initial attempts are observed. Handlers are invoked /// on a worker thread, not on the connection's own thread. /// - public event EventHandler? ConnectionAttemptCompleted; + public event EventHandler? ConnectionAttemptCompleted; /// /// The default (not explicitly configured) options for this connection, fetched based on our parsed endpoints. @@ -1044,7 +1044,7 @@ internal RemoteCertificateValidationCallback? CertificateValidationCallback private set => CertificateValidation = value; } - internal EventHandler? ConnectionAttemptCompletedHandler + internal EventHandler? ConnectionAttemptCompletedHandler { get => ConnectionAttemptCompleted; private set => ConnectionAttemptCompleted = value; diff --git a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs similarity index 74% rename from src/StackExchange.Redis/ConnectionAttemptEventArgs.cs rename to src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs index 5845ed6dc1..9e85d3fefb 100644 --- a/src/StackExchange.Redis/ConnectionAttemptEventArgs.cs +++ b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs @@ -15,14 +15,14 @@ namespace StackExchange.Redis; /// success once the Redis handshake has completed, or failure if the attempt ends before that point. A connection that fails /// after it was established is a disconnect, not a failed attempt, and does not report here. /// -public sealed class ConnectionAttemptEventArgs : EventArgs, ICompletable +public sealed class ConnectionAttemptCompletedEventArgs : EventArgs, ICompletable { - private readonly EventHandler? handler; + private readonly EventHandler? handler; private readonly object sender; private readonly string _physicalName; - internal ConnectionAttemptEventArgs( - EventHandler? handler, + internal ConnectionAttemptCompletedEventArgs( + EventHandler? handler, object sender, EndPoint? endPoint, ConnectionType connectionType, @@ -33,6 +33,8 @@ internal ConnectionAttemptEventArgs( X509Certificate? clientCertificate, string? tlsHostName, ServerCertificateCheck? serverCertificateCheck, + long sequenceNumber, + DateTime completedTimeUtc, string? physicalName) { this.handler = handler; @@ -44,6 +46,8 @@ internal ConnectionAttemptEventArgs( FailureType = failureType; Exception = exception; TlsHostName = tlsHostName; + SequenceNumber = sequenceNumber; + CompletedTimeUtc = completedTimeUtc; if (serverCertificateCheck is not null) { ServerCertificatePolicyErrors = serverCertificateCheck.PolicyErrors; @@ -54,18 +58,47 @@ internal ConnectionAttemptEventArgs( // Snapshot rather than retain: this is delivered on a worker after the attempt has finished, by which time // whoever supplied the certificate may have disposed it - and we have no business holding it (or its key). + // This runs on the connection's own path, so each read is guarded: a certificate disposed while the attempt + // was in flight (plausible mid-rotation) throws when read, and observing must never affect the connection. if (clientCertificate is not null) { - ClientCertificateSubject = clientCertificate.Subject; - ClientCertificateIssuer = clientCertificate.Issuer; - ClientCertificateThumbprint = clientCertificate.GetCertHashString(); - using var sha256 = SHA256.Create(); - ClientCertificateThumbprintSha256 = ToHex(sha256.ComputeHash(clientCertificate.GetRawCertData())); + ClientCertificateSubject = TryRead(clientCertificate, static c => c.Subject); + ClientCertificateIssuer = TryRead(clientCertificate, static c => c.Issuer); + ClientCertificateThumbprint = TryRead(clientCertificate, static c => c.GetCertHashString()); + ClientCertificateThumbprintSha256 = TryRead(clientCertificate, static c => + { + using var sha256 = SHA256.Create(); + return ToHex(sha256.ComputeHash(c.GetRawCertData())); + }); + } + } + + private static string? TryRead(X509Certificate certificate, Func read) + { + try + { + return read(certificate); + } + catch + { + return null; // e.g. disposed; the identity is then simply unknown } } private static string ToHex(byte[] value) => BitConverter.ToString(value).Replace("-", ""); + /// + /// Identifies this outcome among those reported by the same multiplexer: it increases with every outcome, in the order they + /// were recorded. Handlers are invoked on worker threads, so they can observe outcomes out of order, or concurrently; use this + /// (rather than arrival order) to tell which outcome is the more recent. + /// + public long SequenceNumber { get; } + + /// + /// When this outcome was recorded (UTC). + /// + public DateTime CompletedTimeUtc { get; } + /// /// Gets the server endpoint of the attempt. /// @@ -133,7 +166,9 @@ internal ConnectionAttemptEventArgs( /// If the handshake itself failed, this is the certificate returned by , when that /// callback ran; a certificate supplied via SslClientAuthenticationOptions is only identified after a completed handshake. /// This is only known when the library performs TLS itself; it is always null when a tunnel supplies the transport. - /// The same applies to the other ClientCertificate* members. + /// The same applies to the other ClientCertificate* members. Any of them may also be null if the certificate could + /// not be read (for example, because it was disposed while the attempt was in flight). + /// Certificate subjects and issuers can contain tenant or personal identifiers; consider that before logging them. /// public string? ClientCertificateSubject { get; } @@ -145,6 +180,8 @@ internal ConnectionAttemptEventArgs( /// /// The SHA-1 thumbprint (upper-case hex, as ) of the client certificate used for this attempt, if any. /// + /// This identifies a certificate; it is not suitable as a basis for trust decisions, given SHA-1's weaknesses. Prefer + /// where possible. public string? ClientCertificateThumbprint { get; } /// diff --git a/src/StackExchange.Redis/ConnectionAttemptStage.cs b/src/StackExchange.Redis/ConnectionAttemptStage.cs index 35a328e751..7ff49bef8e 100644 --- a/src/StackExchange.Redis/ConnectionAttemptStage.cs +++ b/src/StackExchange.Redis/ConnectionAttemptStage.cs @@ -1,7 +1,7 @@ namespace StackExchange.Redis; /// -/// How far a physical connection attempt got; see . +/// How far a physical connection attempt got; see . /// /// Values are spaced apart, so that finer-grained stages can be added later without renumbering; compare stages by order, not equality, where "at least as far as" is meant. public enum ConnectionAttemptStage diff --git a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs index 4bb4f4667c..91c0497e20 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs @@ -2,6 +2,7 @@ using System.Net; using System.Runtime.CompilerServices; using System.Security.Cryptography.X509Certificates; +using System.Threading; using StackExchange.Redis.Maintenance; namespace StackExchange.Redis; @@ -24,10 +25,27 @@ internal void OnConnectionAttemptCompleted( var handler = RawConfig.ConnectionAttemptCompletedHandler; if (handler != null) { - CompleteAsWorker(new ConnectionAttemptEventArgs(handler, this, endpoint, connectionType, isSuccess, stage, failureType, exception, clientCertificate, tlsHostName, serverCertificateCheck, physicalName)); + var sequenceNumber = Interlocked.Increment(ref _connectionAttemptSequence); + CompleteAsWorker(new ConnectionAttemptCompletedEventArgs( + handler, + this, + endpoint, + connectionType, + isSuccess, + stage, + failureType, + exception, + clientCertificate, + tlsHostName, + serverCertificateCheck, + sequenceNumber, + DateTime.UtcNow, + physicalName)); } } + private long _connectionAttemptSequence; + /// /// Raised whenever a physical connection fails. /// diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 93c9aa36b6..f338ab4055 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -107,6 +107,20 @@ internal void ReportAttemptOutcome(bool isSuccess, ConnectionFailureType failure { if (Interlocked.CompareExchange(ref _attemptOutcomeReported, 1, 0) != 0) return; var clientCertificate = Interlocked.Exchange(ref _clientCertificate, null); // nothing to gain from keeping it + // This runs on the connection's own path (including establishing a healthy connection), so nothing that merely + // *observes* the attempt may be allowed to throw into it. + try + { + ReportAttemptOutcomeCore(isSuccess, failureType, exception, clientCertificate); + } + catch (Exception ex) + { + Debug.WriteLine(ex.Message); + } + } + + private void ReportAttemptOutcomeCore(bool isSuccess, ConnectionFailureType failureType, Exception? exception, X509Certificate? clientCertificate) + { if (BridgeCouldBeNull is { } bridge) { var stage = isSuccess ? ConnectionAttemptStage.Established : _attemptStage; diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index 4e2cc10709..bdea01e8cb 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -1,19 +1,21 @@ #nullable enable -StackExchange.Redis.ConfigurationOptions.ConnectionAttemptCompleted -> System.EventHandler? -StackExchange.Redis.ConnectionAttemptEventArgs -StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateIssuer.get -> string? -StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateSubject.get -> string? -StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateThumbprint.get -> string? -StackExchange.Redis.ConnectionAttemptEventArgs.ClientCertificateThumbprintSha256.get -> string? -StackExchange.Redis.ConnectionAttemptEventArgs.ConnectionType.get -> StackExchange.Redis.ConnectionType -StackExchange.Redis.ConnectionAttemptEventArgs.EndPoint.get -> System.Net.EndPoint? -StackExchange.Redis.ConnectionAttemptEventArgs.Exception.get -> System.Exception? -StackExchange.Redis.ConnectionAttemptEventArgs.FailureType.get -> StackExchange.Redis.ConnectionFailureType -StackExchange.Redis.ConnectionAttemptEventArgs.IsSuccess.get -> bool -StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificateAccepted.get -> bool? -StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificateChainStatus.get -> System.Security.Cryptography.X509Certificates.X509ChainStatusFlags? -StackExchange.Redis.ConnectionAttemptEventArgs.ServerCertificatePolicyErrors.get -> System.Net.Security.SslPolicyErrors? -StackExchange.Redis.ConnectionAttemptEventArgs.Stage.get -> StackExchange.Redis.ConnectionAttemptStage +StackExchange.Redis.ConfigurationOptions.ConnectionAttemptCompleted -> System.EventHandler? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ClientCertificateIssuer.get -> string? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ClientCertificateSubject.get -> string? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ClientCertificateThumbprint.get -> string? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ClientCertificateThumbprintSha256.get -> string? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ConnectionType.get -> StackExchange.Redis.ConnectionType +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.EndPoint.get -> System.Net.EndPoint? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.Exception.get -> System.Exception? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.FailureType.get -> StackExchange.Redis.ConnectionFailureType +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.IsSuccess.get -> bool +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.SequenceNumber.get -> long +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.CompletedTimeUtc.get -> System.DateTime +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ServerCertificateAccepted.get -> bool? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ServerCertificateChainStatus.get -> System.Security.Cryptography.X509Certificates.X509ChainStatusFlags? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ServerCertificatePolicyErrors.get -> System.Net.Security.SslPolicyErrors? +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.Stage.get -> StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptStage.Connect = 100 -> StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptStage.Established = 500 -> StackExchange.Redis.ConnectionAttemptStage @@ -21,5 +23,5 @@ StackExchange.Redis.ConnectionAttemptStage.Handshake = 400 -> StackExchange.Redi StackExchange.Redis.ConnectionAttemptStage.Tls = 300 -> StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptStage.Tunnel = 200 -> StackExchange.Redis.ConnectionAttemptStage StackExchange.Redis.ConnectionAttemptStage.Unknown = 0 -> StackExchange.Redis.ConnectionAttemptStage -StackExchange.Redis.ConnectionAttemptEventArgs.TlsHostName.get -> string? -override StackExchange.Redis.ConnectionAttemptEventArgs.ToString() -> string! +StackExchange.Redis.ConnectionAttemptCompletedEventArgs.TlsHostName.get -> string? +override StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ToString() -> string! diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index c2a533e8c3..8fc2ec6046 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -62,6 +62,17 @@ public async Task EveryFailedReconnectIsReported() await WaitForAsync(() => attempts.Count(a => a.ConnectionType == ConnectionType.Interactive) >= 4); var interactive = attempts.Where(a => a.ConnectionType == ConnectionType.Interactive).ToArray(); output.WriteLine($"interactive attempts: {interactive.Length}"); + + // handlers can run out of order, so the sequence (not arrival) is what orders outcomes + var all = attempts.ToArray(); + Assert.Equal(all.Length, all.Select(a => a.SequenceNumber).Distinct().Count()); + Assert.All(all, a => Assert.True(a.SequenceNumber > 0)); + var ordered = all.OrderBy(a => a.SequenceNumber).ToArray(); + for (int i = 1; i < ordered.Length; i++) + { + Assert.True(ordered[i].CompletedTimeUtc >= ordered[i - 1].CompletedTimeUtc); + } + Assert.All(all, a => Assert.True(a.CompletedTimeUtc > DateTime.UtcNow.AddMinutes(-1) && a.CompletedTimeUtc <= DateTime.UtcNow)); Assert.All(interactive, a => { Assert.False(a.IsSuccess); @@ -71,6 +82,27 @@ public async Task EveryFailedReconnectIsReported() }); } + [Fact] + public async Task WrongPasswordIsAnAuthenticationFailureDuringTheHandshake() + { + using var server = new InProcessTestServer(output) { Password = "right" }; + var options = server.GetClientConfig(withPubSub: false); + options.Password = "wrong"; + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.False(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = attempts.First(); + output.WriteLine($"wrong password: {attempt.Stage}, {attempt.FailureType}: {attempt.Exception?.Message}"); + Assert.False(attempt.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Handshake, attempt.Stage); + Assert.Equal(ConnectionFailureType.AuthenticationFailure, attempt.FailureType); + } + #if !NETFRAMEWORK // the in-process server only does TLS on .NET [Fact] public async Task AcceptedClientCertificateIsIdentified() @@ -135,6 +167,101 @@ public async Task RejectedClientCertificateIsIdentified() Assert.Equal(Sha256(bad), attempt.ClientCertificateThumbprintSha256); } + [Fact] + public async Task WrongPasswordIsNotMistakenForARejectedClientCertificate() + { + // the certificate is fine, the password is not: anything acting on certificate health must leave this alone + using var good = CreateClientCertificate("good-client"); + using var server = new InProcessTestServer(output, useSsl: true) + { + Password = "right", + ClientCertificateValidator = cert => cert is not null && cert.GetCertHashString() == good.Thumbprint, + }; + var options = server.GetClientConfig(withPubSub: false); + options.Password = "wrong"; + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + options.CertificateSelection += (_, _, _, _, _) => good; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + Assert.False(conn.IsConnected); + + await WaitForAsync(() => !attempts.IsEmpty); + var attempt = attempts.First(); + Assert.False(attempt.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Handshake, attempt.Stage); + Assert.Equal(ConnectionFailureType.AuthenticationFailure, attempt.FailureType); + Assert.True(attempt.ServerCertificateAccepted); + Assert.Equal(good.Thumbprint, attempt.ClientCertificateThumbprint); + Assert.False(IsLikelyClientCertificateRejection(attempt)); + } + + [Fact] + public async Task RejectedClientCertificateMatchesTheDocumentedClassification() + { + using var good = CreateClientCertificate("good-client"); + using var bad = CreateClientCertificate("bad-client"); + using var server = new InProcessTestServer(output, useSsl: true) + { + ClientCertificateValidator = cert => cert is not null && cert.GetCertHashString() == good.Thumbprint, + }; + var options = server.GetClientConfig(withPubSub: false); + options.AbortOnConnectFail = false; + options.ConnectTimeout = 2000; + options.CertificateSelection += (_, _, _, _, _) => bad; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + + await WaitForAsync(() => !attempts.IsEmpty); + Assert.True(IsLikelyClientCertificateRejection(attempts.First())); + } + + // mirrors the classification documented in docs/Authentication.md; keep the two in step + private static bool IsLikelyClientCertificateRejection(ConnectionAttemptCompletedEventArgs e) + => !e.IsSuccess + && e.ServerCertificateAccepted == true + && ((e.Stage == ConnectionAttemptStage.Tls && e.FailureType == ConnectionFailureType.AuthenticationFailure) + || (e.Stage == ConnectionAttemptStage.Handshake && e.FailureType == ConnectionFailureType.SocketClosed)); + + [Fact] + public void DisposedClientCertificateDoesNotThrow() + { + // the snapshot runs on the connection's own path; a certificate disposed mid-attempt must not break it + var cert = CreateClientCertificate("disposed-client"); + cert.Dispose(); + + string? subjectAfterDispose; + try + { + subjectAfterDispose = cert.Subject; + } + catch (Exception ex) + { + subjectAfterDispose = $"(throws {ex.GetType().Name})"; + } + output.WriteLine($"disposed certificate Subject: {subjectAfterDispose}"); + + var args = new ConnectionAttemptCompletedEventArgs( + handler: null, + sender: this, + endPoint: null, + connectionType: ConnectionType.Interactive, + isSuccess: true, + stage: ConnectionAttemptStage.Established, + failureType: ConnectionFailureType.None, + exception: null, + clientCertificate: cert, + tlsHostName: null, + serverCertificateCheck: null, + sequenceNumber: 1, + completedTimeUtc: DateTime.UtcNow, + physicalName: null); + output.WriteLine($"snapshot: subject={args.ClientCertificateSubject ?? "(null)"}, sha256={args.ClientCertificateThumbprintSha256 ?? "(null)"}"); + Assert.True(args.IsSuccess); + } + [Fact] public async Task ClientCertificateNotRequestedByServer() { @@ -259,9 +386,9 @@ private static string Sha256(X509Certificate2 certificate) } #endif - private static ConcurrentQueue Observe(ConfigurationOptions options) + private static ConcurrentQueue Observe(ConfigurationOptions options) { - var attempts = new ConcurrentQueue(); + var attempts = new ConcurrentQueue(); options.ConnectionAttemptCompleted += (_, e) => attempts.Enqueue(e); return attempts; } From bd38631dcf37210e3408ddc9f7986becc3eaedc8 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 5 Oct 2026 14:59:40 +0100 Subject: [PATCH 5/8] Compute the SHA-256 client certificate thumbprint without copying on net8.0+ Use X509Certificate2.RawDataMemory, SHA256.HashData into a stack buffer and Convert.ToHexString, rather than GetRawCertData() + a SHA256 instance; down-level TFMs are unchanged. --- .../ConnectionAttemptCompletedEventArgs.cs | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs index 9e85d3fefb..929a8cf298 100644 --- a/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs +++ b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs @@ -65,11 +65,7 @@ internal ConnectionAttemptCompletedEventArgs( ClientCertificateSubject = TryRead(clientCertificate, static c => c.Subject); ClientCertificateIssuer = TryRead(clientCertificate, static c => c.Issuer); ClientCertificateThumbprint = TryRead(clientCertificate, static c => c.GetCertHashString()); - ClientCertificateThumbprintSha256 = TryRead(clientCertificate, static c => - { - using var sha256 = SHA256.Create(); - return ToHex(sha256.ComputeHash(c.GetRawCertData())); - }); + ClientCertificateThumbprintSha256 = TryRead(clientCertificate, GetSha256Thumbprint); } } @@ -85,7 +81,22 @@ internal ConnectionAttemptCompletedEventArgs( } } - private static string ToHex(byte[] value) => BitConverter.ToString(value).Replace("-", ""); + private static string GetSha256Thumbprint(X509Certificate certificate) + { +#if NET8_0_OR_GREATER + // RawDataMemory is a view over the certificate's own (cached) encoding, unlike RawData / GetRawCertData(), + // which copy on every call; the span is only used for the duration of the hash, never retained + ReadOnlySpan raw = certificate is X509Certificate2 cert2 + ? cert2.RawDataMemory.Span + : certificate.GetRawCertData(); // base type only: no non-copying accessor + Span hash = stackalloc byte[SHA256.HashSizeInBytes]; + SHA256.HashData(raw, hash); + return Convert.ToHexString(hash); // upper-case, matching Thumbprint +#else + using var sha256 = SHA256.Create(); + return BitConverter.ToString(sha256.ComputeHash(certificate.GetRawCertData())).Replace("-", ""); +#endif + } /// /// Identifies this outcome among those reported by the same multiplexer: it increases with every outcome, in the order they From 59eb937ddf0b5e9b2c9c5fa29e7a6464ac8e8496 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 8 Oct 2026 13:04:36 +0100 Subject: [PATCH 6/8] Report abandoned connection attempts as such; keep refused-connect exceptions; fix attempt-order race in tests - ResetNonConnected (the initial-connect retry path) abandons any attempt in flight; report that attempt as ConnectionDisposed with an explanatory exception, rather than UnableToConnect with no exception, since it says nothing about the server. ConnectionFailed is unchanged. - BeginConnectAsync's catch blocks now pass the caught exception to RecordConnectionFailed, so a refused connection carries its SocketException. - Tests: outcomes are raised on workers, so arrival order is not attempt order; select the initial attempt by SequenceNumber rather than ConcurrentQueue.First() (the cause of the CI failure in RejectedClientCertificateMatchesTheDocumentedClassification). Add coverage for abandoned attempts and for refused attempts carrying an exception. - Docs: abandoned attempts say nothing about the certificate; the classification is a lower bound. --- docs/Authentication.md | 5 ++ src/StackExchange.Redis/PhysicalBridge.cs | 6 ++ src/StackExchange.Redis/PhysicalConnection.cs | 4 +- .../ConnectionAttemptUnitTests.cs | 62 +++++++++++++++---- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/docs/Authentication.md b/docs/Authentication.md index 869155bfc4..2969ba0549 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -122,6 +122,11 @@ client certificate apart from other failures needs both the stage and the failur during the Redis handshake that follows: `Handshake` + `SocketClosed`. - A wrong password or ACL problem is the server *replying* with an error during the Redis handshake: `Handshake` + `AuthenticationFailure`. That says nothing about the certificate, and must not count against it. +- An attempt that the library abandons itself, for example to retry the initial connect, reports `ConnectionDisposed` at + whatever stage it had reached. That also says nothing about the certificate. + +Treat the classification as a lower bound: if the library abandons an attempt at the same moment the server rejects the +certificate, the attempt is reported as abandoned, and the rejection is not seen. Even so, `SocketClosed` is a weak signal: server restarts, connection resets, `maxclients` and network interruptions look the same. If you act on it, for example by falling back to an older certificate: diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 9e94cde129..3d421035ad 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -454,6 +454,12 @@ internal void ResetNonConnected() var tmp = physical; if (tmp != null && state != (int)State.ConnectedEstablished) { + // we are abandoning any attempt in flight in order to retry; that says nothing about the server, so report + // it as such (reporting is once-only, so the failure recorded below does not report the attempt again) + tmp.ReportAttemptOutcome( + isSuccess: false, + ConnectionFailureType.ConnectionDisposed, + new RedisConnectionException(ConnectionFailureType.ConnectionDisposed, CommandFlags.None, "The connection attempt was abandoned so that it could be retried")); tmp.RecordConnectionFailed(ConnectionFailureType.UnableToConnect); } TryConnect(null); diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index f338ab4055..fc53e20e71 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -420,7 +420,7 @@ internal async Task BeginConnectAsync(ILogger? log) catch (ObjectDisposedException ex) { log?.LogErrorSocketShutdown(ex, new(endpoint)); - try { RecordConnectionFailed(ConnectionFailureType.UnableToConnect, isInitialConnect: true); } + try { RecordConnectionFailed(ConnectionFailureType.UnableToConnect, ex, isInitialConnect: true); } catch (Exception inner) { ConnectionMultiplexer.TraceWithoutContext(inner.Message); @@ -429,7 +429,7 @@ internal async Task BeginConnectAsync(ILogger? log) catch (Exception outer) { ConnectionMultiplexer.TraceWithoutContext(outer.Message); - try { RecordConnectionFailed(ConnectionFailureType.UnableToConnect, isInitialConnect: true); } + try { RecordConnectionFailed(ConnectionFailureType.UnableToConnect, outer, isInitialConnect: true); } catch (Exception inner) { ConnectionMultiplexer.TraceWithoutContext(inner.Message); diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index 8fc2ec6046..f6dda79ebf 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -78,10 +78,46 @@ public async Task EveryFailedReconnectIsReported() Assert.False(a.IsSuccess); Assert.Equal(ConnectionAttemptStage.Connect, a.Stage); Assert.NotEqual(ConnectionFailureType.None, a.FailureType); + Assert.NotNull(a.Exception); // the refusal itself, not just its classification Assert.Equal(endpoint, a.EndPoint); }); } + [Fact] + public async Task AbandonedAttemptIsNotReportedAsAFailureToConnect() + { + // accepts TCP (via the backlog) but never answers the TLS handshake, so every attempt stays in flight at Tls + // until the library abandons it; that is the library's decision, and says nothing about the server + var listener = new TcpListener(IPAddress.Loopback, 0); // not IDisposable on net481 + listener.Start(); + try + { + var endpoint = (IPEndPoint)listener.LocalEndpoint; + var options = new ConfigurationOptions + { + EndPoints = { endpoint }, + Ssl = true, + AbortOnConnectFail = false, + ConnectTimeout = 1000, + }; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + conn.GetServerSnapshot()[0].ResetNonConnected(); + + await WaitForAsync(() => attempts.Any(a => a.FailureType == ConnectionFailureType.ConnectionDisposed)); + var abandoned = attempts.First(a => a.FailureType == ConnectionFailureType.ConnectionDisposed); + Assert.False(abandoned.IsSuccess); + Assert.Equal(ConnectionAttemptStage.Tls, abandoned.Stage); + Assert.Contains("abandoned", abandoned.Exception?.Message); + Assert.DoesNotContain(attempts, a => a.FailureType == ConnectionFailureType.UnableToConnect && a.Exception is null); + } + finally + { + listener.Stop(); + } + } + [Fact] public async Task WrongPasswordIsAnAuthenticationFailureDuringTheHandshake() { @@ -95,8 +131,7 @@ public async Task WrongPasswordIsAnAuthenticationFailureDuringTheHandshake() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); Assert.False(conn.IsConnected); - await WaitForAsync(() => !attempts.IsEmpty); - var attempt = attempts.First(); + var attempt = await FirstAttemptAsync(attempts); output.WriteLine($"wrong password: {attempt.Stage}, {attempt.FailureType}: {attempt.Exception?.Message}"); Assert.False(attempt.IsSuccess); Assert.Equal(ConnectionAttemptStage.Handshake, attempt.Stage); @@ -155,8 +190,7 @@ public async Task RejectedClientCertificateIsIdentified() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); Assert.False(conn.IsConnected); - await WaitForAsync(() => !attempts.IsEmpty); - var attempt = attempts.First(); + var attempt = await FirstAttemptAsync(attempts); output.WriteLine($"rejected attempt: {attempt.Stage}, {attempt.FailureType}: {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}"); Assert.False(attempt.IsSuccess); Assert.True(attempt.Stage is ConnectionAttemptStage.Tls or ConnectionAttemptStage.Handshake); // TLS 1.3: Handshake, TLS 1.2: Tls @@ -187,8 +221,7 @@ public async Task WrongPasswordIsNotMistakenForARejectedClientCertificate() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); Assert.False(conn.IsConnected); - await WaitForAsync(() => !attempts.IsEmpty); - var attempt = attempts.First(); + var attempt = await FirstAttemptAsync(attempts); Assert.False(attempt.IsSuccess); Assert.Equal(ConnectionAttemptStage.Handshake, attempt.Stage); Assert.Equal(ConnectionFailureType.AuthenticationFailure, attempt.FailureType); @@ -214,8 +247,7 @@ public async Task RejectedClientCertificateMatchesTheDocumentedClassification() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); - await WaitForAsync(() => !attempts.IsEmpty); - Assert.True(IsLikelyClientCertificateRejection(attempts.First())); + Assert.True(IsLikelyClientCertificateRejection(await FirstAttemptAsync(attempts))); } // mirrors the classification documented in docs/Authentication.md; keep the two in step @@ -302,8 +334,7 @@ public async Task ServerCertificateRejectedByPlatformDefault() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); Assert.False(conn.IsConnected); - await WaitForAsync(() => !attempts.IsEmpty); - var attempt = attempts.First(); + var attempt = await FirstAttemptAsync(attempts); output.WriteLine($"server certificate: {attempt.ServerCertificatePolicyErrors} / {attempt.ServerCertificateChainStatus}; {attempt.FailureType}: {attempt.Exception?.Message}"); Assert.False(attempt.IsSuccess); Assert.Equal(ConnectionAttemptStage.Tls, attempt.Stage); @@ -329,8 +360,7 @@ public async Task ServerCertificateRejectedByCallback() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); Assert.False(conn.IsConnected); - await WaitForAsync(() => !attempts.IsEmpty); - var attempt = attempts.First(); + var attempt = await FirstAttemptAsync(attempts); Assert.False(attempt.IsSuccess); Assert.Equal(ConnectionAttemptStage.Tls, attempt.Stage); Assert.False(attempt.ServerCertificateAccepted); @@ -393,6 +423,14 @@ private static ConcurrentQueue Observe(Conf return attempts; } + // outcomes are raised on workers, so arrival order is not attempt order; the initial attempt is always sequence 1 + private static async Task FirstAttemptAsync(ConcurrentQueue attempts) + { + ConnectionAttemptCompletedEventArgs? first = null; + await WaitForAsync(() => (first = attempts.FirstOrDefault(a => a.SequenceNumber == 1)) is not null); + return first!; + } + private static async Task WaitForAsync(Func condition, int timeoutMilliseconds = 10000) { var giveUp = DateTime.UtcNow.AddMilliseconds(timeoutMilliseconds); From d208648223727fca8d092d560b55f63297ca9eea Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 8 Oct 2026 14:03:31 +0100 Subject: [PATCH 7/8] Report heartbeat connect timeouts with their exception; log the classified attempt - OnHeartbeat abandons a connect that has outlived the retry policy; report that attempt as UnableToConnect with the ConnectTimeout exception it builds, rather than letting Dispose report it as ConnectionDisposed with no exception. Seen on Windows CI, where a refused loopback connect takes long enough for the heartbeat to fire first. - Tests: add TimedOutAttemptCarriesTheTimeout (forces the heartbeat path on any OS); drop incidental stage assertions that raced the reset; log the attempt in the classification test, which still fails on Windows. --- src/StackExchange.Redis/PhysicalBridge.cs | 1 + .../ConnectionAttemptUnitTests.cs | 41 +++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 3d421035ad..83d04c8897 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -627,6 +627,7 @@ internal void OnHeartbeat(bool ifConnectedOnly) Trace("Aborting connect"); // abort and reconnect var snapshot = physical; + snapshot?.ReportAttemptOutcome(isSuccess: false, ConnectionFailureType.UnableToConnect, ex); // else Dispose reports it without the timeout OnDisconnected(ConnectionFailureType.UnableToConnect, snapshot, out bool isCurrent, out State oldState); snapshot?.Dispose(); // Cleanup the existing connection/socket if any, otherwise it will wait reading indefinitely TryConnect(null); diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index f6dda79ebf..bc249cd872 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -83,10 +83,44 @@ public async Task EveryFailedReconnectIsReported() }); } + [Fact] + public async Task TimedOutAttemptCarriesTheTimeout() + { + // never answers the TLS handshake; with a short retry policy the heartbeat gives up on each attempt, which must be + // reported with the timeout rather than as a bare disposal + var listener = new TcpListener(IPAddress.Loopback, 0); // not IDisposable on net481 + listener.Start(); + try + { + var options = new ConfigurationOptions + { + EndPoints = { (IPEndPoint)listener.LocalEndpoint }, + Ssl = true, + AbortOnConnectFail = false, + ConnectTimeout = 1000, + HeartbeatInterval = TimeSpan.FromMilliseconds(100), + ReconnectRetryPolicy = new LinearRetry(50), + }; + var attempts = Observe(options); + + await using var conn = await ConnectionMultiplexer.ConnectAsync(options); + + await WaitForAsync(() => attempts.Any(a => a.FailureType == ConnectionFailureType.UnableToConnect)); + var timedOut = attempts.First(a => a.FailureType == ConnectionFailureType.UnableToConnect); + Assert.False(timedOut.IsSuccess); + Assert.NotNull(timedOut.Exception); + Assert.All(attempts, a => Assert.NotNull(a.Exception)); + } + finally + { + listener.Stop(); + } + } + [Fact] public async Task AbandonedAttemptIsNotReportedAsAFailureToConnect() { - // accepts TCP (via the backlog) but never answers the TLS handshake, so every attempt stays in flight at Tls + // accepts TCP (via the backlog) but never answers the TLS handshake, so every attempt stays in flight // until the library abandons it; that is the library's decision, and says nothing about the server var listener = new TcpListener(IPAddress.Loopback, 0); // not IDisposable on net481 listener.Start(); @@ -108,7 +142,6 @@ public async Task AbandonedAttemptIsNotReportedAsAFailureToConnect() await WaitForAsync(() => attempts.Any(a => a.FailureType == ConnectionFailureType.ConnectionDisposed)); var abandoned = attempts.First(a => a.FailureType == ConnectionFailureType.ConnectionDisposed); Assert.False(abandoned.IsSuccess); - Assert.Equal(ConnectionAttemptStage.Tls, abandoned.Stage); Assert.Contains("abandoned", abandoned.Exception?.Message); Assert.DoesNotContain(attempts, a => a.FailureType == ConnectionFailureType.UnableToConnect && a.Exception is null); } @@ -247,7 +280,9 @@ public async Task RejectedClientCertificateMatchesTheDocumentedClassification() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); - Assert.True(IsLikelyClientCertificateRejection(await FirstAttemptAsync(attempts))); + var attempt = await FirstAttemptAsync(attempts); + output.WriteLine($"{attempt.Stage} / {attempt.FailureType}; server certificate accepted: {attempt.ServerCertificateAccepted}; {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}; inner {attempt.Exception?.InnerException?.GetType().Name}: {attempt.Exception?.InnerException?.Message}"); + Assert.True(IsLikelyClientCertificateRejection(attempt)); } // mirrors the classification documented in docs/Authentication.md; keep the two in step From 4aef3b1733f3fe4404d2997d0052f0c9fc440787 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 8 Oct 2026 15:51:29 +0100 Subject: [PATCH 8/8] Classify Handshake + SocketFailure as a likely client-certificate rejection Measured on Windows CI (Windows Server 2025, net10.0): with TLS 1.3, SChannel surfaces the server's rejection of the client certificate as an IOException on the first read after the TLS handshake (inner Win32Exception describing the alert), so the attempt reports Handshake + SocketFailure rather than the Handshake + SocketClosed seen on Linux. - Docs and the test's mirror of the classification accept either failure type at Handshake; both are weak signals. - The classification test also logs the inner exception's HResult / NativeErrorCode, to judge whether the Windows alert can be recognised specifically. --- docs/Authentication.md | 13 ++++++++----- .../ConnectionAttemptUnitTests.cs | 6 ++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/Authentication.md b/docs/Authentication.md index 2969ba0549..d81589566d 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -111,15 +111,18 @@ static bool IsLikelyClientCertificateRejection(ConnectionAttemptCompletedEventAr => !e.IsSuccess && e.ServerCertificateAccepted == true // the server certificate was fine... && ((e.Stage == ConnectionAttemptStage.Tls && e.FailureType == ConnectionFailureType.AuthenticationFailure) // TLS 1.2 - || (e.Stage == ConnectionAttemptStage.Handshake && e.FailureType == ConnectionFailureType.SocketClosed)); // TLS 1.3 + || (e.Stage == ConnectionAttemptStage.Handshake // TLS 1.3 + && e.FailureType is ConnectionFailureType.SocketClosed or ConnectionFailureType.SocketFailure)); ``` `e.Stage` says how far the attempt got: `Connect`, `Tunnel`, `Tls`, `Handshake`, or `Established` on success. Telling a rejected client certificate apart from other failures needs both the stage and the failure type: - With TLS 1.2, the server rejects the certificate during the TLS handshake: `Tls` + `AuthenticationFailure`. -- With TLS 1.3, the client's TLS handshake completes *before* the server rejects the certificate, and the server then hangs up - during the Redis handshake that follows: `Handshake` + `SocketClosed`. +- With TLS 1.3, the client's TLS handshake completes *before* the server rejects the certificate, so the rejection surfaces + during the Redis handshake that follows. How it surfaces depends on the platform's TLS stack: as the server hanging up, + `Handshake` + `SocketClosed` (seen on Linux), or as a failed read, `Handshake` + `SocketFailure`, whose exception describes + the server's TLS alert (seen on Windows). - A wrong password or ACL problem is the server *replying* with an error during the Redis handshake: `Handshake` + `AuthenticationFailure`. That says nothing about the certificate, and must not count against it. - An attempt that the library abandons itself, for example to retry the initial connect, reports `ConnectionDisposed` at @@ -128,8 +131,8 @@ client certificate apart from other failures needs both the stage and the failur Treat the classification as a lower bound: if the library abandons an attempt at the same moment the server rejects the certificate, the attempt is reported as abandoned, and the rejection is not seen. -Even so, `SocketClosed` is a weak signal: server restarts, connection resets, `maxclients` and network interruptions look the -same. If you act on it, for example by falling back to an older certificate: +Even so, `SocketClosed` and `SocketFailure` are weak signals: server restarts, connection resets, `maxclients` and network +interruptions look the same. If you act on it, for example by falling back to an older certificate: - **require corroboration**, such as several consecutive suspected rejections of the same certificate, rather than reacting to one; diff --git a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs index bc249cd872..cb675fb29c 100644 --- a/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.ComponentModel; using System.Linq; using System.Net; using System.Net.Sockets; @@ -281,7 +282,8 @@ public async Task RejectedClientCertificateMatchesTheDocumentedClassification() await using var conn = await ConnectionMultiplexer.ConnectAsync(options); var attempt = await FirstAttemptAsync(attempts); - output.WriteLine($"{attempt.Stage} / {attempt.FailureType}; server certificate accepted: {attempt.ServerCertificateAccepted}; {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}; inner {attempt.Exception?.InnerException?.GetType().Name}: {attempt.Exception?.InnerException?.Message}"); + var inner = attempt.Exception?.InnerException; + output.WriteLine($"{attempt.Stage} / {attempt.FailureType}; server certificate accepted: {attempt.ServerCertificateAccepted}; {attempt.Exception?.GetType().Name}: {attempt.Exception?.Message}; inner {inner?.GetType().Name} (HResult 0x{inner?.HResult:X8}, native {(inner as Win32Exception)?.NativeErrorCode:X8}): {inner?.Message}"); Assert.True(IsLikelyClientCertificateRejection(attempt)); } @@ -290,7 +292,7 @@ private static bool IsLikelyClientCertificateRejection(ConnectionAttemptComplete => !e.IsSuccess && e.ServerCertificateAccepted == true && ((e.Stage == ConnectionAttemptStage.Tls && e.FailureType == ConnectionFailureType.AuthenticationFailure) - || (e.Stage == ConnectionAttemptStage.Handshake && e.FailureType == ConnectionFailureType.SocketClosed)); + || (e.Stage == ConnectionAttemptStage.Handshake && e.FailureType is ConnectionFailureType.SocketClosed or ConnectionFailureType.SocketFailure)); [Fact] public void DisposedClientCertificateDoesNotThrow()