Skip to content

Fall back to TLSv1.2 when System.Security.Authentication.SslProtocols.None is disabled/unavailable #766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,6 @@ public IProtocol Protocol
/// </summary>
public SslOption Ssl { get; set; }

public SslOption Tls {
get { return this.Ssl; }
set { this.Ssl = value; }
}

/// <summary>
/// Construct an instance from a protocol and an address in "hostname:port" format.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,6 @@ public TimeSpan ContinuationTimeout
/// </summary>
public SslOption Ssl { get; set; } = new SslOption();

/// <summary>
/// TLS options setting.
/// </summary>
public SslOption Tls {
get { return this.Ssl; }
set { this.Ssl = value; }
}

/// <summary>
/// Set to false to make automatic connection recovery not recover topology (exchanges, queues, bindings, etc).
/// Defaults to true.
Expand Down
26 changes: 19 additions & 7 deletions projects/client/RabbitMQ.Client/src/client/api/SslHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
using System;
using System.IO;
using System.Net.Security;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;

namespace RabbitMQ.Client
Expand All @@ -59,21 +60,32 @@ private SslHelper(SslOption sslOption)
}

/// <summary>
/// Upgrade a Tcp stream to an Ssl stream using the SSL options provided.
/// Upgrade a Tcp stream to an Ssl stream using the TLS options provided.
/// </summary>
public static Stream TcpUpgrade(Stream tcpStream, SslOption sslOption)
public static Stream TcpUpgrade(Stream tcpStream, SslOption options)
{
var helper = new SslHelper(sslOption);
var helper = new SslHelper(options);

RemoteCertificateValidationCallback remoteCertValidator =
sslOption.CertificateValidationCallback ?? helper.CertificateValidationCallback;
options.CertificateValidationCallback ?? helper.CertificateValidationCallback;
LocalCertificateSelectionCallback localCertSelector =
sslOption.CertificateSelectionCallback ?? helper.CertificateSelectionCallback;
options.CertificateSelectionCallback ?? helper.CertificateSelectionCallback;

var sslStream = new SslStream(tcpStream, false, remoteCertValidator, localCertSelector);

sslStream.AuthenticateAsClientAsync(sslOption.ServerName, sslOption.Certs, sslOption.Version,
sslOption.CheckCertificateRevocation).GetAwaiter().GetResult();
Action<SslOption> TryAuthenticating = (SslOption opts) => {
sslStream.AuthenticateAsClientAsync(opts.ServerName, opts.Certs, opts.Version,
opts.CheckCertificateRevocation).GetAwaiter().GetResult();
};
try {
TryAuthenticating(options);
} catch(ArgumentException e) when (e.ParamName == "sslProtocolType" && options.Version == SslProtocols.None) {
// SslProtocols.None is dysfunctional in this environment, possibly due to TLS version restrictions
// in the app context, system or .NET version-specific behavior. See rabbitmq/rabbitmq-dotnet-client#764
// for background.
options.UseFallbackTlsVersions();
TryAuthenticating(options);
}

return sslStream;
}
Expand Down
21 changes: 16 additions & 5 deletions projects/client/RabbitMQ.Client/src/client/api/SslOption.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public SslOption()
}

/// <summary>
/// Retrieve or set the set of ssl policy errors that are deemed acceptable.
/// Retrieve or set the set of TLS policy errors that are deemed acceptable.
/// </summary>
public SslPolicyErrors AcceptablePolicyErrors { get; set; }

Expand All @@ -90,13 +90,13 @@ public SslOption()
public string CertPath { get; set; }

/// <summary>
/// An optional client specified SSL certificate selection callback. If this is not specified,
/// An optional client specified TLS certificate selection callback. If this is not specified,
/// the first valid certificate found will be used.
/// </summary>
public LocalCertificateSelectionCallback CertificateSelectionCallback { get; set; }

/// <summary>
/// An optional client specified SSL certificate validation callback. If this is not specified,
/// An optional client specified TLS certificate validation callback. If this is not specified,
/// the default callback will be used in conjunction with the <see cref="AcceptablePolicyErrors"/> property to
/// determine if the remote server certificate is valid.
/// </summary>
Expand Down Expand Up @@ -135,19 +135,30 @@ public X509CertificateCollection Certs
public bool CheckCertificateRevocation { get; set; }

/// <summary>
/// Flag specifying if Ssl should indeed be used.
/// Flag specifying if TLS should indeed be used.
/// </summary>
public bool Enabled { get; set; }

/// <summary>
/// Retrieve or set server's Canonical Name.
/// This MUST match the CN on the Certificate else the SSL connection will fail.
/// This MUST match the Subject Alternative Name or CN on the Certificate else the TLS connection will fail.
/// </summary>
public string ServerName { get; set; }

/// <summary>
/// Retrieve or set the Ssl protocol version.
/// </summary>
public SslProtocols Version { get; set; }

/// <summary>
/// Reconfigures the instance to enable/use TLSv1.2.
/// Only used in environments where System.Security.Authentication.SslProtocols.None
/// is unavailable or effectively disabled, as reported by System.Net.ServicePointManager.
/// </summary>
internal SslProtocols UseFallbackTlsVersions()
{
this.Version = SslProtocols.Tls12;
return Version;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ namespace RabbitMQ.Client
}
public class SslHelper
{
public static System.IO.Stream TcpUpgrade(System.IO.Stream tcpStream, RabbitMQ.Client.SslOption sslOption) { }
public static System.IO.Stream TcpUpgrade(System.IO.Stream tcpStream, RabbitMQ.Client.SslOption options) { }
}
public class SslOption
{
Expand Down
4 changes: 2 additions & 2 deletions projects/client/Unit/src/unit/TestAmqpUri.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ private static void AssertUriPartEquivalence(ConnectionFactory cf, string user,
Assert.AreEqual(password, cf.Password);
Assert.AreEqual(port, cf.Port);
Assert.AreEqual(vhost, cf.VirtualHost);
Assert.AreEqual(tlsEnabled, cf.Tls.Enabled);
Assert.AreEqual(tlsEnabled, cf.Ssl.Enabled);

Assert.AreEqual(port, cf.Endpoint.Port);
Assert.AreEqual(tlsEnabled, cf.Endpoint.Tls.Enabled);
Assert.AreEqual(tlsEnabled, cf.Endpoint.Ssl.Enabled);
}

private void ParseFailWith<T>(string uri) where T : Exception
Expand Down