Use the best TLS version by default.

Specifying SslProtocols.None allows the .NET Framework to choose the best TLS version supported by the OS (on .NET 4.7 and later). However, negotiating TLS 1.2 with a Windows Schannel client against a yaSSL-based MySQL Server will fail; automatically try with a lower TLS version if it's possible we're in this scenario.
mysql-net · Mar 16, 2018 · e4c8aec · e4c8aec
1 parent 59c450c
commit e4c8aec
Show file tree

Hide file tree

Showing 11 changed files with 164 additions and 56 deletions.
diff --git a/.ci/config/config.compression+ssl.json b/.ci/config/config.compression+ssl.json
@@ -3,7 +3,7 @@
       "ConnectionString": "server=127.0.0.1;user id=ssltest;password=test;port=3306;database=mysqltest;ssl mode=required;use compression=true;DefaultCommandTimeout=3600",
       "PasswordlessUser": "no_password",
       "SecondaryDatabase": "testdb2",
-      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password",
+      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password,Tls12",
       "MySqlBulkLoaderLocalCsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.CSV",
       "MySqlBulkLoaderLocalTsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.TSV",
       "CertificatesPath": "../../../../../.ci/server/certs"

diff --git a/.ci/config/config.compression.json b/.ci/config/config.compression.json
@@ -3,7 +3,7 @@
       "ConnectionString": "server=127.0.0.1;user id=mysqltest;password='test;key=\"val';port=3306;database=mysqltest;ssl mode=none;UseCompression=true;DefaultCommandTimeout=3600",
       "PasswordlessUser": "no_password",
       "SecondaryDatabase": "testdb2",
-      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password",
+      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password,Tls12",
       "MySqlBulkLoaderLocalCsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.CSV",
       "MySqlBulkLoaderLocalTsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.TSV"
     }

diff --git a/.ci/config/config.json b/.ci/config/config.json
@@ -3,7 +3,7 @@
       "ConnectionString": "server=127.0.0.1;user id=mysqltest;password='test;key=\"val';port=3306;database=mysqltest;ssl mode=none;Use Affected Rows=true;DefaultCommandTimeout=3600",
       "PasswordlessUser": "no_password",
       "SecondaryDatabase": "testdb2",
-      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password",
+      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password,Tls12",
       "MySqlBulkLoaderLocalCsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.CSV",
       "MySqlBulkLoaderLocalTsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.TSV"
     }

diff --git a/.ci/config/config.ssl.json b/.ci/config/config.ssl.json
@@ -3,7 +3,7 @@
       "ConnectionString": "server=127.0.0.1;user id=ssltest;password=test;port=3306;database=mysqltest;ssl mode=required;certificate file=../../../../../.ci/server/certs/ssl-client.pfx;DefaultCommandTimeout=3600",
       "PasswordlessUser": "no_password",
       "SecondaryDatabase": "testdb2",
-      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password",
+      "UnsupportedFeatures": "RsaEncryption,CachingSha2Password,Tls12",
       "MySqlBulkLoaderLocalCsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.CSV",
       "MySqlBulkLoaderLocalTsvFile": "../../../../TestData/LoadData_UTF8_BOM_Unix.TSV",
       "CertificatesPath": "../../../../../.ci/server/certs"

diff --git a/.travis.yml b/.travis.yml
@@ -5,10 +5,10 @@ services: docker
 env:
   - IMAGE=mysql:5.6
     NAME=mysql
-    OMIT_FEATURES=Json,Sha256Password,RsaEncryption,LargePackets,CachingSha2Password,SessionTrack
+    OMIT_FEATURES=Json,Sha256Password,RsaEncryption,LargePackets,CachingSha2Password,SessionTrack,Tls11,Tls12
   - IMAGE=mysql:5.7
     NAME=mysql
-    OMIT_FEATURES=RsaEncryption,CachingSha2Password
+    OMIT_FEATURES=RsaEncryption,CachingSha2Password,Tls12
   - IMAGE=mysql:8.0
     NAME=mysql
     OMIT_FEATURES=None

diff --git a/src/MySqlConnector/Core/ConnectionPool.cs b/src/MySqlConnector/Core/ConnectionPool.cs
@@ -2,6 +2,7 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
+using System.Security.Authentication;
 using System.Threading;
 using System.Threading.Tasks;
 using MySql.Data.MySqlClient;
@@ -17,6 +18,8 @@ internal sealed class ConnectionPool
 
 		public ConnectionSettings ConnectionSettings { get; }
 
+		public SslProtocols SslProtocols { get; set; }
+
 		public async ValueTask<ServerSession> GetSessionAsync(MySqlConnection connection, IOBehavior ioBehavior, CancellationToken cancellationToken)
 		{
 			cancellationToken.ThrowIfCancellationRequested();
@@ -411,6 +414,7 @@ private static IReadOnlyList<ConnectionPool> GetAllPools()
 		private ConnectionPool(ConnectionSettings cs)
 		{
 			ConnectionSettings = cs;
+			SslProtocols = Utility.GetDefaultSslProtocols();
 			m_generation = 0;
 			m_cleanSemaphore = new SemaphoreSlim(1);
 			m_sessionSemaphore = new SemaphoreSlim(cs.MaximumPoolSize);

diff --git a/src/MySqlConnector/Core/ServerSession.cs b/src/MySqlConnector/Core/ServerSession.cs
@@ -225,60 +225,93 @@ public async Task ConnectAsync(ConnectionSettings cs, ILoadBalancer loadBalancer
 					VerifyState(State.Created);
 					m_state = State.Connecting;
 				}
-				var connected = false;
-				if (cs.ConnectionType == ConnectionType.Tcp)
-					connected = await OpenTcpSocketAsync(cs, loadBalancer, ioBehavior, cancellationToken).ConfigureAwait(false);
-				else if (cs.ConnectionType == ConnectionType.Unix)
-					connected = await OpenUnixSocketAsync(cs, ioBehavior, cancellationToken).ConfigureAwait(false);
-				if (!connected)
+
+				// TLS negotiation should automatically fall back to the best version supported by client and server. However,
+				// Windows Schannel clients will fail to connect to a yaSSL-based MySQL Server if TLS 1.2 is requested and
+				// have to use only TLS 1.1: https://github.com/mysql-net/MySqlConnector/pull/101
+				// In order to use the best protocol possible (i.e., not always default to TLS 1.1), we try the OS-default protocol
+				// (which is SslProtocols.None; see https://docs.microsoft.com/en-us/dotnet/framework/network-programming/tls),
+				// then fall back to SslProtocols.Tls11 if that fails and it's possible that the cause is a yaSSL server.
+				bool shouldRetrySsl;
+				var sslProtocols = Pool?.SslProtocols ?? Utility.GetDefaultSslProtocols();
+				PayloadData payload;
+				InitialHandshakePayload initialHandshake;
+				do
 				{
-					lock (m_lock)
-						m_state = State.Failed;
-					Log.Error("{0} connecting failed", m_logArguments);
-					throw new MySqlException("Unable to connect to any of the specified MySQL hosts.");
-				}
+					shouldRetrySsl = (sslProtocols == SslProtocols.None || (sslProtocols & SslProtocols.Tls12) == SslProtocols.Tls12) && Utility.IsWindows();
+
+					var connected = false;
+					if (cs.ConnectionType == ConnectionType.Tcp)
+						connected = await OpenTcpSocketAsync(cs, loadBalancer, ioBehavior, cancellationToken).ConfigureAwait(false);
+					else if (cs.ConnectionType == ConnectionType.Unix)
+						connected = await OpenUnixSocketAsync(cs, ioBehavior, cancellationToken).ConfigureAwait(false);
+					if (!connected)
+					{
+						lock (m_lock)
+							m_state = State.Failed;
+						Log.Error("{0} connecting failed", m_logArguments);
+						throw new MySqlException("Unable to connect to any of the specified MySQL hosts.");
+					}
 
-				var byteHandler = new SocketByteHandler(m_socket);
-				m_payloadHandler = new StandardPayloadHandler(byteHandler);
+					var byteHandler = new SocketByteHandler(m_socket);
+					m_payloadHandler = new StandardPayloadHandler(byteHandler);
 
-				var payload = await ReceiveAsync(ioBehavior, cancellationToken).ConfigureAwait(false);
-				var initialHandshake = InitialHandshakePayload.Create(payload);
+					payload = await ReceiveAsync(ioBehavior, cancellationToken).ConfigureAwait(false);
+					initialHandshake = InitialHandshakePayload.Create(payload);
 
-				// if PluginAuth is supported, then use the specified auth plugin; else, fall back to protocol capabilities to determine the auth type to use
-				string authPluginName;
-				if ((initialHandshake.ProtocolCapabilities & ProtocolCapabilities.PluginAuth) != 0)
-					authPluginName = initialHandshake.AuthPluginName;
-				else
-					authPluginName = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.SecureConnection) == 0 ? "mysql_old_password" : "mysql_native_password";
-				m_logArguments[1] = authPluginName;
-				Log.Debug("{0} server sent auth_plugin_name '{1}'", m_logArguments);
-				if (authPluginName != "mysql_native_password" && authPluginName != "sha256_password" && authPluginName != "caching_sha2_password")
-				{
-					Log.Error("{0} unsupported authentication method '{1}'", m_logArguments);
-					throw new NotSupportedException("Authentication method '{0}' is not supported.".FormatInvariant(initialHandshake.AuthPluginName));
-				}
+					// if PluginAuth is supported, then use the specified auth plugin; else, fall back to protocol capabilities to determine the auth type to use
+					string authPluginName;
+					if ((initialHandshake.ProtocolCapabilities & ProtocolCapabilities.PluginAuth) != 0)
+						authPluginName = initialHandshake.AuthPluginName;
+					else
+						authPluginName = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.SecureConnection) == 0 ? "mysql_old_password" : "mysql_native_password";
+					m_logArguments[1] = authPluginName;
+					Log.Debug("{0} server sent auth_plugin_name '{1}'", m_logArguments);
+					if (authPluginName != "mysql_native_password" && authPluginName != "sha256_password" && authPluginName != "caching_sha2_password")
+					{
+						Log.Error("{0} unsupported authentication method '{1}'", m_logArguments);
+						throw new NotSupportedException("Authentication method '{0}' is not supported.".FormatInvariant(initialHandshake.AuthPluginName));
+					}
 
-				ServerVersion = new ServerVersion(Encoding.ASCII.GetString(initialHandshake.ServerVersion));
-				ConnectionId = initialHandshake.ConnectionId;
-				AuthPluginData = initialHandshake.AuthPluginData;
-				m_useCompression = cs.UseCompression && (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.Compress) != 0;
+					ServerVersion = new ServerVersion(Encoding.ASCII.GetString(initialHandshake.ServerVersion));
+					ConnectionId = initialHandshake.ConnectionId;
+					AuthPluginData = initialHandshake.AuthPluginData;
+					m_useCompression = cs.UseCompression && (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.Compress) != 0;
 
-				m_supportsConnectionAttributes = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.ConnectionAttributes) != 0;
-				m_supportsDeprecateEof = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.DeprecateEof) != 0;
-				var serverSupportsSsl = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.Ssl) != 0;
+					m_supportsConnectionAttributes = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.ConnectionAttributes) != 0;
+					m_supportsDeprecateEof = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.DeprecateEof) != 0;
+					var serverSupportsSsl = (initialHandshake.ProtocolCapabilities & ProtocolCapabilities.Ssl) != 0;
 
-				Log.Info("{0} made connection; ServerVersion={1}; ConnectionId={2}; Flags: {3}{4}{5}{6}", m_logArguments[0], ServerVersion.OriginalString, ConnectionId,
-					m_useCompression ? "Cmp " :"", m_supportsConnectionAttributes ? "Attr " : "", m_supportsDeprecateEof ? "" : "Eof ", serverSupportsSsl ? "Ssl " : "");
+					Log.Info("{0} made connection; ServerVersion={1}; ConnectionId={2}; Flags: {3}{4}{5}{6}", m_logArguments[0], ServerVersion.OriginalString, ConnectionId,
+						m_useCompression ? "Cmp " : "", m_supportsConnectionAttributes ? "Attr " : "", m_supportsDeprecateEof ? "" : "Eof ", serverSupportsSsl ? "Ssl " : "");
 
-				if (cs.SslMode != MySqlSslMode.None && (cs.SslMode != MySqlSslMode.Preferred || serverSupportsSsl))
-				{
-					if (!serverSupportsSsl)
+					if (cs.SslMode != MySqlSslMode.None && (cs.SslMode != MySqlSslMode.Preferred || serverSupportsSsl))
 					{
-						Log.Error("{0} requires SSL but server doesn't support it", m_logArguments);
-						throw new MySqlException("Server does not support SSL");
+						if (!serverSupportsSsl)
+						{
+							Log.Error("{0} requires SSL but server doesn't support it", m_logArguments);
+							throw new MySqlException("Server does not support SSL");
+						}
+
+						try
+						{
+							await InitSslAsync(initialHandshake.ProtocolCapabilities, cs, sslProtocols, ioBehavior, cancellationToken).ConfigureAwait(false);
+							shouldRetrySsl = false;
+						}
+						catch (Exception ex) when (shouldRetrySsl && ((ex is MySqlException && ex.InnerException is IOException) || ex is IOException))
+						{
+							// negotiating TLS 1.2 with a yaSSL-based server throws an exception on Windows, see comment at top of method
+							Log.Warn(ex, "{0} failed negotiating TLS; falling back to TLS 1.1", m_logArguments);
+							sslProtocols = SslProtocols.Tls | SslProtocols.Tls11;
+							if (Pool != null)
+								Pool.SslProtocols = sslProtocols;
+						}
 					}
-					await InitSslAsync(initialHandshake.ProtocolCapabilities, cs, ioBehavior, cancellationToken).ConfigureAwait(false);
-				}
+					else
+					{
+						shouldRetrySsl = false;
+					}
+				} while (shouldRetrySsl);
 
 				if (m_supportsConnectionAttributes && s_connectionAttributes == null)
 					s_connectionAttributes = CreateConnectionAttributes();
@@ -772,7 +805,7 @@ private async Task<bool> OpenUnixSocketAsync(ConnectionSettings cs, IOBehavior i
 			return false;
 		}
 
-		private async Task InitSslAsync(ProtocolCapabilities serverCapabilities, ConnectionSettings cs, IOBehavior ioBehavior, CancellationToken cancellationToken)
+		private async Task InitSslAsync(ProtocolCapabilities serverCapabilities, ConnectionSettings cs, SslProtocols sslProtocols, IOBehavior ioBehavior, CancellationToken cancellationToken)
 		{
 			Log.Info("{0} initializing TLS connection", m_logArguments);
 			X509CertificateCollection clientCertificates = null;
@@ -861,11 +894,6 @@ bool ValidateRemoteCertificate(object rcbSender, X509Certificate rcbCertificate,
 			else
 				sslStream = new SslStream(m_networkStream, false, ValidateRemoteCertificate, ValidateLocalCertificate);
 
-			// SslProtocols.Tls1.2 throws an exception in Windows, see https://github.com/mysql-net/MySqlConnector/pull/101
-			var sslProtocols = SslProtocols.Tls | SslProtocols.Tls11;
-			if (!Utility.IsWindows())
-				sslProtocols |= SslProtocols.Tls12;
-
 			var checkCertificateRevocation = cs.SslMode == MySqlSslMode.VerifyFull;
 
 			var initSsl = HandshakeResponse41Payload.CreateWithSsl(serverCapabilities, cs, m_useCompression);
@@ -889,6 +917,8 @@ bool ValidateRemoteCertificate(object rcbSender, X509Certificate rcbCertificate,
 				m_payloadHandler.ByteHandler = sslByteHandler;
 				m_isSecureConnection = true;
 				m_sslStream = sslStream;
+				m_logArguments[1] = sslStream.SslProtocol;
+				Log.Info("{0} connected TLS with protocol {1}", m_logArguments);
 			}
 			catch (Exception ex)
 			{
@@ -1082,6 +1112,8 @@ private void VerifyState(State state1, State state2, State state3)
 
 		internal bool SslIsMutuallyAuthenticated => m_sslStream?.IsMutuallyAuthenticated ?? false;
 
+		internal SslProtocols SslProtocol => m_sslStream?.SslProtocol ?? SslProtocols.None;
+
 		private byte[] CreateConnectionAttributes()
 		{
 			Log.Debug("{0} creating connection attributes", m_logArguments);

diff --git a/src/MySqlConnector/MySql.Data.MySqlClient/MySqlConnection.cs b/src/MySqlConnector/MySql.Data.MySqlClient/MySqlConnection.cs
@@ -3,6 +3,7 @@
 using System.Data;
 using System.Data.Common;
 using System.Net.Sockets;
+using System.Security.Authentication;
 using System.Threading;
 using System.Threading.Tasks;
 using MySqlConnector.Core;
@@ -426,6 +427,8 @@ private async ValueTask<ServerSession> CreateSessionAsync(IOBehavior? ioBehavior
 
 		internal bool SslIsMutuallyAuthenticated => m_session.SslIsMutuallyAuthenticated;
 
+		internal SslProtocols SslProtocol => m_session.SslProtocol;
+
 		internal void SetState(ConnectionState newState)
 		{
 			if (m_connectionState != newState)

diff --git a/src/MySqlConnector/Utilities/Utility.cs b/src/MySqlConnector/Utilities/Utility.cs
@@ -2,7 +2,9 @@
 using System.Globalization;
 using System.IO;
 using System.Linq;
+using System.Net.Security;
 using System.Runtime.InteropServices;
+using System.Security.Authentication;
 using System.Security.Cryptography;
 using System.Text;
 using System.Threading.Tasks;
@@ -254,5 +256,41 @@ public static void GetOSDetails(out string os, out string osDescription, out str
 			architecture = RuntimeInformation.ProcessArchitecture.ToString();
 		}
 #endif
+
+#if NET45 || NET46
+		public static SslProtocols GetDefaultSslProtocols()
+		{
+			if (!s_defaultSslProtocols.HasValue)
+			{
+				try
+				{
+					using (var memoryStream = new MemoryStream())
+					using (var sslStream = new SslStream(memoryStream))
+					{
+						sslStream.AuthenticateAsClient("localhost", null, SslProtocols.None, false);
+					}
+				}
+				catch (ArgumentException ex) when (ex.ParamName == "sslProtocolType")
+				{
+					// Prior to .NET Framework 4.7, SslProtocols.None is not a valid argument to AuthenticateAsClientAsync.
+					// If the NET46 build is loaded by an application that targets. NET 4.7 (or later), or if app.config has set
+					// Switch.System.Net.DontEnableSystemDefaultTlsVersions to false, then SslProtocols.None will work; otherwise,
+					// if the application targets .NET 4.6.2 or earlier and hasn't changed the AppContext switch, then it will
+					// fail at runtime; we catch the exception and explicitly specify the protocols to use.
+					s_defaultSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12;
+				}
+				catch (Exception)
+				{
+					s_defaultSslProtocols = SslProtocols.None;
+				}
+			}
+
+			return s_defaultSslProtocols.Value;
+		}
+
+		static SslProtocols? s_defaultSslProtocols;
+#else
+		public static SslProtocols GetDefaultSslProtocols() => SslProtocols.None;
+#endif
 	}
 }
diff --git a/tests/SideBySide/ServerFeatures.cs b/tests/SideBySide/ServerFeatures.cs
@@ -16,5 +16,7 @@ public enum ServerFeatures
 		Timeout = 128,
 		ErrorCodes = 256,
 		KnownCertificateAuthority = 512,
+		Tls11 = 1024,
+		Tls12 = 2048,
 	}
 }
diff --git a/tests/SideBySide/SslTests.cs b/tests/SideBySide/SslTests.cs
@@ -1,4 +1,5 @@
 using System.IO;
+using System.Security.Authentication;
 using System.Threading.Tasks;
 using MySql.Data.MySqlClient;
 using Xunit;
@@ -118,6 +119,34 @@ public async Task ConnectSslBadCaCertificate()
 			}
 		}
 
+		[SkippableFact(ConfigSettings.RequiresSsl)]
+		public async Task ConnectSslTlsVersion()
+		{
+			using (var connection = new MySqlConnection(AppConfig.ConnectionString))
+			{
+				await connection.OpenAsync();
+#if BASELINE
+				var expectedProtocol = AppConfig.SupportedFeatures.HasFlag(ServerFeatures.Tls11) ? SslProtocols.Tls11 : SslProtocols.Tls;
+#else
+				var expectedProtocol = AppConfig.SupportedFeatures.HasFlag(ServerFeatures.Tls12) ? SslProtocols.Tls12 :
+					AppConfig.SupportedFeatures.HasFlag(ServerFeatures.Tls11) ? SslProtocols.Tls11 :
+					SslProtocols.Tls;
+#endif
+				var expectedProtocolString = expectedProtocol == SslProtocols.Tls12 ? "TLSv1.2" :
+					expectedProtocol == SslProtocols.Tls11 ? "TLSv1.1" : "TLSv1";
+
+#if !BASELINE
+				Assert.Equal(expectedProtocol, connection.SslProtocol);
+#endif
+				using (var cmd = new MySqlCommand("show status like 'Ssl_version';", connection))
+				using (var reader = await cmd.ExecuteReaderAsync())
+				{
+					Assert.True(reader.Read());
+					Assert.Equal(expectedProtocolString, reader.GetString(1));
+				}
+			}
+		}
+
 		readonly DatabaseFixture m_database;
 	}
 }