Skip to content

Commit d2d13bc

Browse files
NickCraverslorelloslorello89
authored
SSL/TLS: Add SslClientAuthenticationOptions configurability (#2224)
Solution for #2219, allowing explicit configuration of the SSL connection for advanced use cases. This needs some thought, but pitching the general idea here as an option available for the frameworks that support it. Co-authored-by: slorello <[email protected]> Co-authored-by: Steve Lorello <[email protected]>
1 parent 612d327 commit d2d13bc

27 files changed

+349
-66
lines changed

Directory.Packages.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<!-- Packages only used in the solution, upgrade at will -->
1212
<PackageVersion Include="BenchmarkDotNet" Version="0.13.1" />
1313
<PackageVersion Include="GitHubActionsTestLogger" Version="2.0.0-alpha" />
14-
<PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.3" />
14+
<PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4-beta1.22362.3" />
1515
<PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.2" />
1616
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
1717
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />

StackExchange.Redis.sln

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Basic", "Basic", "{38BDEEED
7272
tests\RedisConfigs\Basic\primary-6379.conf = tests\RedisConfigs\Basic\primary-6379.conf
7373
tests\RedisConfigs\Basic\replica-6380.conf = tests\RedisConfigs\Basic\replica-6380.conf
7474
tests\RedisConfigs\Basic\secure-6381.conf = tests\RedisConfigs\Basic\secure-6381.conf
75+
tests\RedisConfigs\Basic\tls-ciphers-6384.conf = tests\RedisConfigs\Basic\tls-ciphers-6384.conf
7576
EndProjectSection
7677
EndProject
7778
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BasicTestBaseline", "tests\BasicTestBaseline\BasicTestBaseline.csproj", "{8FDB623D-779B-4A84-BC6B-75106E41D8A4}"

docs/Configuration.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,26 +76,26 @@ The `ConfigurationOptions` object has a wide range of properties, all of which a
7676
| abortConnect={bool} | `AbortOnConnectFail` | `true` (`false` on Azure) | If true, `Connect` will not create a connection while no servers are available |
7777
| allowAdmin={bool} | `AllowAdmin` | `false` | Enables a range of commands that are considered risky |
7878
| channelPrefix={string} | `ChannelPrefix` | `null` | Optional channel prefix for all pub/sub operations |
79-
| checkCertificateRevocation={bool} | `CheckCertificateRevocation` | `true` | A Boolean value that specifies whether the certificate revocation list is checked during authentication. |
79+
| checkCertificateRevocation={bool} | `CheckCertificateRevocation` | `true` | A Boolean value that specifies whether the certificate revocation list is checked during authentication. |
8080
| connectRetry={int} | `ConnectRetry` | `3` | The number of times to repeat connect attempts during initial `Connect` |
8181
| connectTimeout={int} | `ConnectTimeout` | `5000` | Timeout (ms) for connect operations |
8282
| configChannel={string} | `ConfigurationChannel` | `__Booksleeve_MasterChanged` | Broadcast channel name for communicating configuration changes |
83-
| configCheckSeconds={int} | `ConfigCheckSeconds` | `60` | Time (seconds) to check configuration. This serves as a keep-alive for interactive sockets, if it is supported. |
83+
| configCheckSeconds={int} | `ConfigCheckSeconds` | `60` | Time (seconds) to check configuration. This serves as a keep-alive for interactive sockets, if it is supported. |
8484
| defaultDatabase={int} | `DefaultDatabase` | `null` | Default database index, from `0` to `databases - 1` |
8585
| keepAlive={int} | `KeepAlive` | `-1` | Time (seconds) at which to send a message to help keep sockets alive (60 sec default) |
8686
| name={string} | `ClientName` | `null` | Identification for the connection within redis |
8787
| password={string} | `Password` | `null` | Password for the redis server |
8888
| user={string} | `User` | `null` | User for the redis server (for use with ACLs on redis 6 and above) |
8989
| proxy={proxy type} | `Proxy` | `Proxy.None` | Type of proxy in use (if any); for example "twemproxy/envoyproxy" |
9090
| resolveDns={bool} | `ResolveDns` | `false` | Specifies that DNS resolution should be explicit and eager, rather than implicit |
91-
| serviceName={string} | `ServiceName` | `null` | Used for connecting to a sentinel primary service |
91+
| serviceName={string} | `ServiceName` | `null` | Used for connecting to a sentinel primary service |
9292
| ssl={bool} | `Ssl` | `false` | Specifies that SSL encryption should be used |
9393
| sslHost={string} | `SslHost` | `null` | Enforces a particular SSL host identity on the server's certificate |
9494
| sslProtocols={enum} | `SslProtocols` | `null` | Ssl/Tls versions supported when using an encrypted connection. Use '\|' to provide multiple values. |
9595
| syncTimeout={int} | `SyncTimeout` | `5000` | Time (ms) to allow for synchronous operations |
96-
| asyncTimeout={int} | `AsyncTimeout` | `SyncTimeout` | Time (ms) to allow for asynchronous operations |
97-
| tiebreaker={string} | `TieBreaker` | `__Booksleeve_TieBreak` | Key to use for selecting a server in an ambiguous primary scenario |
98-
| version={string} | `DefaultVersion` | (`3.0` in Azure, else `2.0`) | Redis version level (useful when the server does not make this available) |
96+
| asyncTimeout={int} | `AsyncTimeout` | `SyncTimeout` | Time (ms) to allow for asynchronous operations |
97+
| tiebreaker={string} | `TieBreaker` | `__Booksleeve_TieBreak` | Key to use for selecting a server in an ambiguous primary scenario |
98+
| version={string} | `DefaultVersion` | (`4.0` in Azure, else `2.0`) | Redis version level (useful when the server does not make this available) |
9999

100100

101101
Additional code-only options:
@@ -105,6 +105,8 @@ Additional code-only options:
105105
- Determines how commands will be queued (or not) during a disconnect, for sending when it's available again
106106
- BeforeSocketConnect - Default: `null`
107107
- Allows modifying a `Socket` before connecting (for advanced scenarios)
108+
- SslClientAuthenticationOptions (`netcooreapp3.1`/`net5.0` and higher) - Default: `null`
109+
- Allows specifying exact options for SSL/TLS authentication against a server (e.g. cipher suites, protocols, etc.) - overrides all other SSL configuration options. This is a `Func<string, SslClientAuthenticationOptions>` which receiveces the host (or `SslHost` if set) to get the options for. If `null` is returned from the `Func`, it's the same as this property not being set at all when connecting.
108110

109111
Tokens in the configuration string are comma-separated; any without an `=` sign are assumed to be redis server endpoints. Endpoints without an explicit port will use 6379 if ssl is not enabled, and 6380 if ssl is enabled.
110112
Tokens starting with `$` are taken to represent command maps, for example: `$config=cfg`.

docs/ReleaseNotes.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
- Performance: Optimization around timeout processing to reduce lock contention in the case of many items that haven't yet timed out during a heartbeat ([#2217 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2217))
1010
- Fix [#2223](https://github.com/StackExchange/StackExchange.Redis/issues/2223): Resolve sync-context issues (missing `ConfigureAwait(false)`) ([#2229 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2229))
1111
- Fix [#1968](https://github.com/StackExchange/StackExchange.Redis/issues/1968): Improved handling of EVAL scripts during server restarts and failovers, detecting and re-sending the script for a retry when needed ([#2170 by martintmk](https://github.com/StackExchange/StackExchange.Redis/pull/2170))
12+
- Adds: `ConfigurationOptions.SslClientAuthenticationOptions` (`netcoreapp3.1`/`net5.0`+ only) to give more control over SSL/TLS authentication ([#2224 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2224))
13+
1214

1315
## 2.6.48
1416

src/StackExchange.Redis/ConfigurationOptions.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,14 @@ public int ResponseTimeout
490490
/// </remarks>
491491
public SocketManager? SocketManager { get; set; }
492492

493+
#if NETCOREAPP3_1_OR_GREATER
494+
/// <summary>
495+
/// A <see cref="SslClientAuthenticationOptions"/> provider for a given host, for custom TLS connection options.
496+
/// Note: this overrides *all* other TLS and certificate settings, only for advanced use cases.
497+
/// </summary>
498+
public Func<string, SslClientAuthenticationOptions>? SslClientAuthenticationOptions { get; set; }
499+
#endif
500+
493501
/// <summary>
494502
/// Indicates whether the connection should be encrypted.
495503
/// </summary>
@@ -619,6 +627,9 @@ public static ConfigurationOptions Parse(string configuration, bool ignoreUnknow
619627
checkCertificateRevocation = checkCertificateRevocation,
620628
BeforeSocketConnect = BeforeSocketConnect,
621629
EndPoints = EndPoints.Clone(),
630+
#if NETCOREAPP3_1_OR_GREATER
631+
SslClientAuthenticationOptions = SslClientAuthenticationOptions,
632+
#endif
622633
};
623634

624635
/// <summary>

src/StackExchange.Redis/ConnectionMultiplexer.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,8 @@ private static bool AllComplete(Task[] tasks)
557557
return true;
558558
}
559559

560-
internal bool AuthSuspect { get; private set; }
561-
internal void SetAuthSuspect() => AuthSuspect = true;
560+
internal Exception? AuthException { get; private set; }
561+
internal void SetAuthSuspect(Exception authException) => AuthException = authException;
562562

563563
/// <summary>
564564
/// Creates a new <see cref="ConnectionMultiplexer"/> instance.

src/StackExchange.Redis/ExceptionFactory.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Reflection;
4+
using System.Security.Authentication;
35
using System.Text;
46
using System.Threading;
57

@@ -383,17 +385,29 @@ private static string GetLabel(bool includeDetail, RedisCommand command, Message
383385
internal static Exception UnableToConnect(ConnectionMultiplexer muxer, string? failureMessage = null)
384386
{
385387
var sb = new StringBuilder("It was not possible to connect to the redis server(s).");
386-
if (muxer != null)
388+
Exception? inner = null;
389+
if (muxer is not null)
387390
{
388-
if (muxer.AuthSuspect) sb.Append(" There was an authentication failure; check that passwords (or client certificates) are configured correctly.");
389-
else if (muxer.RawConfig.AbortOnConnectFail) sb.Append(" Error connecting right now. To allow this multiplexer to continue retrying until it's able to connect, use abortConnect=false in your connection string or AbortOnConnectFail=false; in your code.");
391+
if (muxer.AuthException is Exception aex)
392+
{
393+
sb.Append(" There was an authentication failure; check that passwords (or client certificates) are configured correctly: (").Append(aex.GetType().Name).Append(") ").Append(aex.Message);
394+
inner = aex;
395+
if (aex is AuthenticationException && aex.InnerException is Exception iaex)
396+
{
397+
sb.Append(" (Inner - ").Append(iaex.GetType().Name).Append(") ").Append(iaex.Message);
398+
}
399+
}
400+
else if (muxer.RawConfig.AbortOnConnectFail)
401+
{
402+
sb.Append(" Error connecting right now. To allow this multiplexer to continue retrying until it's able to connect, use abortConnect=false in your connection string or AbortOnConnectFail=false; in your code.");
403+
}
390404
}
391405
if (!failureMessage.IsNullOrWhiteSpace())
392406
{
393407
sb.Append(' ').Append(failureMessage.Trim());
394408
}
395409

396-
return new RedisConnectionException(ConnectionFailureType.UnableToConnect, sb.ToString());
410+
return new RedisConnectionException(ConnectionFailureType.UnableToConnect, sb.ToString(), inner);
397411
}
398412

399413
internal static Exception BeganProfilingWithDuplicateContext(object forContext)

src/StackExchange.Redis/PhysicalConnection.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1432,12 +1432,24 @@ internal async ValueTask<bool> ConnectedAsync(Socket socket, LogProxy? log, Sock
14321432
{
14331433
try
14341434
{
1435+
#if NETCOREAPP3_1_OR_GREATER
1436+
var configOptions = config.SslClientAuthenticationOptions?.Invoke(host);
1437+
if (configOptions is not null)
1438+
{
1439+
await ssl.AuthenticateAsClientAsync(configOptions);
1440+
}
1441+
else
1442+
{
1443+
ssl.AuthenticateAsClient(host, config.SslProtocols, config.CheckCertificateRevocation);
1444+
}
1445+
#else
14351446
ssl.AuthenticateAsClient(host, config.SslProtocols, config.CheckCertificateRevocation);
1447+
#endif
14361448
}
14371449
catch (Exception ex)
14381450
{
14391451
Debug.WriteLine(ex.Message);
1440-
bridge.Multiplexer?.SetAuthSuspect();
1452+
bridge.Multiplexer?.SetAuthSuspect(ex);
14411453
throw;
14421454
}
14431455
log?.WriteLine($"TLS connection established successfully using protocol: {ssl.SslProtocol}");
File renamed without changes.
File renamed without changes.

0 commit comments

Comments
 (0)