diff --git a/docs/Authentication.md b/docs/Authentication.md index 15a673d19..d81589566 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -92,6 +92,64 @@ 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 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 // 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, 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 + 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` 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; +- **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 b6ad77317..4ea52bc04 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/ConnectionAttemptCompletedEventArgs.cs b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs new file mode 100644 index 000000000..929a8cf29 --- /dev/null +++ b/src/StackExchange.Redis/ConnectionAttemptCompletedEventArgs.cs @@ -0,0 +1,219 @@ +using System; +using System.Net; +using System.Net.Security; +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 ConnectionAttemptCompletedEventArgs : EventArgs, ICompletable +{ + private readonly EventHandler? handler; + private readonly object sender; + private readonly string _physicalName; + + internal ConnectionAttemptCompletedEventArgs( + EventHandler? handler, + object sender, + EndPoint? endPoint, + ConnectionType connectionType, + bool isSuccess, + ConnectionAttemptStage stage, + ConnectionFailureType failureType, + Exception? exception, + X509Certificate? clientCertificate, + string? tlsHostName, + ServerCertificateCheck? serverCertificateCheck, + long sequenceNumber, + DateTime completedTimeUtc, + string? physicalName) + { + this.handler = handler; + this.sender = sender; + EndPoint = endPoint; + ConnectionType = connectionType; + IsSuccess = isSuccess; + Stage = stage; + FailureType = failureType; + Exception = exception; + TlsHostName = tlsHostName; + SequenceNumber = sequenceNumber; + CompletedTimeUtc = completedTimeUtc; + 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 + // 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 = TryRead(clientCertificate, static c => c.Subject); + ClientCertificateIssuer = TryRead(clientCertificate, static c => c.Issuer); + ClientCertificateThumbprint = TryRead(clientCertificate, static c => c.GetCertHashString()); + ClientCertificateThumbprintSha256 = TryRead(clientCertificate, GetSha256Thumbprint); + } + } + + 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 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 + /// 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. + /// + 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; } + + /// + /// 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. + /// + 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; } + + /// + /// 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). + /// + 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. 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; } + + /// + /// 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. + /// + /// 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; } + + /// + /// 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; +} + +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 000000000..7ff49bef8 --- /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 fd5bd7c7f..91c0497e2 100644 --- a/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs +++ b/src/StackExchange.Redis/ConnectionMultiplexer.Events.cs @@ -1,12 +1,51 @@ using System; using System.Net; using System.Runtime.CompilerServices; +using System.Security.Cryptography.X509Certificates; +using System.Threading; using StackExchange.Redis.Maintenance; namespace StackExchange.Redis; public partial class ConnectionMultiplexer { + 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) + { + 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/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 7a8dbacbc..83d04c889 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); @@ -549,6 +555,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)) { @@ -619,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/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index cbedd4225..fc53e20e7 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -95,6 +95,85 @@ 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 + 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) + { + 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; + 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; @@ -341,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); @@ -350,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); @@ -435,6 +514,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 +564,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; @@ -1210,6 +1291,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; } @@ -1217,6 +1299,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(); } @@ -1225,22 +1308,53 @@ static Stream DemandSocketStream(Socket? socket) if (config.Ssl) { + _attemptStage = ConnectionAttemptStage.Tls; log?.LogInformationConfiguringTLS(); - var host = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); + var host = _tlsHostName = config.ResolveTlsHostName(bridge.ServerEndPoint.EndPoint); stream ??= DemandSocketStream(socket); + var certificateValidation = config.CertificateValidationCallback ?? GetAmbientIssuerCertificateCallback(); + var certificateSelection = config.CertificateSelectionCallback ?? GetAmbientClientCertificateCallback(); + bool optionsValidate = false, optionsSelect = false; +#if NET + SslClientAuthenticationOptions? configOptions; + try + { + 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(), - userCertificateSelectionCallback: config.CertificateSelectionCallback ?? GetAmbientClientCertificateCallback(), + userCertificateValidationCallback: certificateValidation, + userCertificateSelectionCallback: certificateSelection, encryptionPolicy: EncryptionPolicy.RequireEncryption); try { try { #if NET - var configOptions = config.SslClientAuthenticationOptions?.Invoke(host); if (configOptions is not null) { await ssl.AuthenticateAsClientAsync(configOptions).ForAwait(); @@ -1255,11 +1369,21 @@ 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) + { + // 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 @@ -1277,6 +1401,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; @@ -1291,6 +1423,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 444a5d1fb..2bfa4e24f 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -1,4 +1,30 @@ #nullable enable +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 +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.ConnectionAttemptCompletedEventArgs.TlsHostName.get -> string? +override StackExchange.Redis.ConnectionAttemptCompletedEventArgs.ToString() -> string! override StackExchange.Redis.Configuration.AzureManagedRedisOptionsProvider.MaintenanceNotifications.get -> StackExchange.Redis.MaintenanceNotificationMode override StackExchange.Redis.Configuration.RedisCloudOptionsProvider.MaintenanceNotifications.get -> StackExchange.Redis.MaintenanceNotificationMode override StackExchange.Redis.Configuration.RedisEnterpriseOptionsProvider.MaintenanceNotifications.get -> StackExchange.Redis.MaintenanceNotificationMode diff --git a/tests/StackExchange.Redis.Tests/ConfigTests.cs b/tests/StackExchange.Redis.Tests/ConfigTests.cs index e0cbb8c0a..6b487dbda 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 new file mode 100644 index 000000000..cb675fb29 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/ConnectionAttemptUnitTests.cs @@ -0,0 +1,495 @@ +using System; +using System.Collections.Concurrent; +using System.ComponentModel; +using System.Linq; +using System.Net; +using System.Net.Sockets; +#if !NETFRAMEWORK +using System.Net.Security; +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(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); + } + } + + [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}"); + + // 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); + 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 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 + // 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.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() + { + 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); + + 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); + Assert.Equal(ConnectionFailureType.AuthenticationFailure, attempt.FailureType); + } + +#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); + 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] + 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); + + 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 + 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); + 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); + + var attempt = await FirstAttemptAsync(attempts); + 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); + + var attempt = await FirstAttemptAsync(attempts); + 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)); + } + + // 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 is ConnectionFailureType.SocketClosed or ConnectionFailureType.SocketFailure)); + + [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() + { + // 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); + } + + [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); + + 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); + 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); + + var attempt = await FirstAttemptAsync(attempts); + 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); + 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; + } + + // 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); + 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 2276500bd..047b6d04d 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) @@ -94,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) @@ -127,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; @@ -394,11 +407,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); },