Skip to content

Commit

Permalink
Raise warning message for insecure protocols (#591)
Browse files Browse the repository at this point in the history
  • Loading branch information
DavoudEshtehari authored Jun 10, 2020
1 parent 4258e4b commit e601e20
Show file tree
Hide file tree
Showing 24 changed files with 344 additions and 21 deletions.
6 changes: 6 additions & 0 deletions doc/snippets/Microsoft.Data.SqlClient/SqlClientLogger.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,11 @@
<summary>Logs information through a specified method of the current instance type.</summary>
<remarks>To be added.</remarks>
</LogInfo>
<LogWarning>
<param name="type">The type to be logged.</param>
<param name="method">The logging method.</param>
<param name="message">The message to be logged.</param>
<summary>Logs warning through a specified method of the current instance type.</summary>
</LogWarning>
</members>
</docs>
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ internal struct SNI_Error
internal static extern uint SNITerminate();

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIWaitForSSLHandshakeToCompleteWrapper")]
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds);
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds, out uint pProtocolVersion);

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl)]
internal static extern uint UnmanagedIsTokenRestricted([In] IntPtr token, [MarshalAs(UnmanagedType.Bool)] out bool isRestricted);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ internal abstract class SNIHandle

public abstract void ReturnPacket(SNIPacket packet);

/// <summary>
/// Gets a value that indicates the security protocol used to authenticate this connection.
/// </summary>
public virtual int ProtocolVersion { get; } = 0;
#if DEBUG
/// <summary>
/// Test handle for killing underlying connection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public Guid ConnectionId
}
}

public int ProtocolVersion => _lowerHandle.ProtocolVersion;

/// <summary>
/// Constructor
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ internal sealed class SNIMarsHandle : SNIHandle

public override int ReserveHeaderSize => SNISMUXHeader.HEADER_LENGTH;

public override int ProtocolVersion => _connection.ProtocolVersion;

/// <summary>
/// Dispose object
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,21 @@ public override uint Status
}
}

public override int ProtocolVersion
{
get
{
try
{
return (int)_sslStream.SslProtocol;
}
catch
{
return base.ProtocolVersion;
}
}
}

public override uint CheckConnection()
{
long scopeID = SqlClientEventSource.Log.SNIScopeEnterEvent("<sc.SNI.SNINpHandle.CheckConnection |SNI|INFO>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,21 @@ public override uint Status
}
}

public override int ProtocolVersion
{
get
{
try
{
return (int)_sslStream.SslProtocol;
}
catch
{
return base.ProtocolVersion;
}
}
}

/// <summary>
/// Constructor
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ private void LoadSSPILibrary()
// No - Op
}

private void WaitForSSLHandShakeToComplete(ref uint error)
private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion)
{
// No - Op

}

private SNIErrorDetails GetSniErrorDetails()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,13 @@ private void LoadSSPILibrary()
}
}

private void WaitForSSLHandShakeToComplete(ref uint error)
private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion)
{
if (TdsParserStateObjectFactory.UseManagedSNI)
return;
// in the case where an async connection is made, encryption is used and Windows Authentication is used,
// wait for SSL handshake to complete, so that the SSL context is fully negotiated before we try to use its
// Channel Bindings as part of the Windows Authentication context build (SSL handshake must complete
// before calling SNISecGenClientContext).
error = _physicalStateObj.WaitForSSLHandShakeToComplete();
error = _physicalStateObj.WaitForSSLHandShakeToComplete(out protocolVersion);
if (error != TdsEnums.SNI_SUCCESS)
{
_physicalStateObj.AddError(ProcessSNIError(_physicalStateObj));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Reflection;
using System.Security.Authentication;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -19,6 +21,7 @@
using Microsoft.Data.SqlClient.DataClassification;
using Microsoft.Data.SqlClient.Server;
using Microsoft.Data.SqlTypes;
using Newtonsoft.Json.Serialization;

namespace Microsoft.Data.SqlClient
{
Expand All @@ -39,6 +42,9 @@ internal struct SNIErrorDetails
internal sealed partial class TdsParser
{
private static int _objectTypeCount; // EventSource counter
private readonly SqlClientLogger _logger = new SqlClientLogger();
private readonly string _typeName;

internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount);
internal int ObjectID => _objectID;

Expand Down Expand Up @@ -174,6 +180,7 @@ internal TdsParser(bool MARS, bool fAsynchronous)

_physicalStateObj = TdsParserStateObjectFactory.Singleton.CreateTdsParserStateObject(this);
DataClassificationVersion = TdsEnums.DATA_CLASSIFICATION_NOT_ENABLED;
_typeName = GetType().Name;
}

internal SqlInternalConnectionTds Connection
Expand Down Expand Up @@ -906,7 +913,15 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake(bool encrypt, bool trus
ThrowExceptionAndWarning(_physicalStateObj);
}

WaitForSSLHandShakeToComplete(ref error);
int protocolVersion = 0;
WaitForSSLHandShakeToComplete(ref error, ref protocolVersion);

SslProtocols protocol = (SslProtocols)protocolVersion;
string warningMessage = protocol.GetProtocolWarning();
if(!string.IsNullOrEmpty(warningMessage))
{
_logger.LogWarning(_typeName, MethodBase.GetCurrentMethod().Name, warningMessage);
}

// create a new packet encryption changes the internal packet size
_physicalStateObj.ClearAllWritePackets();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Diagnostics;
using System.Globalization;
using System.Security;
using System.Security.Authentication;
using System.Text;
using Microsoft.Data.Common;
using Microsoft.Data.SqlTypes;
Expand Down Expand Up @@ -917,4 +918,64 @@ private void ParseMultipartName()

internal static readonly MultiPartTableName Null = new MultiPartTableName(new string[] { null, null, null, null });
}

internal static class SslProtocolsHelper
{
private static string ToFriendlyName(this SslProtocols protocol)
{
string name;

/* The SslProtocols.Tls13 is supported by netcoreapp3.1 and later
* This driver does not support this version yet!
if ((protocol & SslProtocols.Tls13) == SslProtocols.Tls13)
{
name = "TLS 1.3";
}*/
if((protocol & SslProtocols.Tls12) == SslProtocols.Tls12)
{
name = "TLS 1.2";
}
else if ((protocol & SslProtocols.Tls11) == SslProtocols.Tls11)
{
name = "TLS 1.1";
}
else if ((protocol & SslProtocols.Tls) == SslProtocols.Tls)
{
name = "TLS 1.0";
}
#pragma warning disable CS0618 // Type or member is obsolete: SSL is depricated
else if ((protocol & SslProtocols.Ssl3) == SslProtocols.Ssl3)
{
name = "SSL 3.0";
}
else if ((protocol & SslProtocols.Ssl2) == SslProtocols.Ssl2)
#pragma warning restore CS0618 // Type or member is obsolete: SSL is depricated
{
name = "SSL 2.0";
}
else
{
name = protocol.ToString();
}

return name;
}

/// <summary>
/// check the negotiated secure protocol if it's under TLS 1.2
/// </summary>
/// <param name="protocol"></param>
/// <returns>Localized warning message</returns>
public static string GetProtocolWarning(this SslProtocols protocol)
{
string message = string.Empty;
#pragma warning disable CS0618 // Type or member is obsolete : SSL is depricated
if ((protocol & (SslProtocols.Ssl2 | SslProtocols.Ssl3 | SslProtocols.Tls | SslProtocols.Tls11)) != SslProtocols.None)
#pragma warning restore CS0618 // Type or member is obsolete : SSL is depricated
{
message = SRHelper.Format(SR.SEC_ProtocolWarning, protocol.ToFriendlyName());
}
return message;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ private void ResetCancelAndProcessAttention()

internal abstract uint EnableSsl(ref uint info);

internal abstract uint WaitForSSLHandShakeToComplete();
internal abstract uint WaitForSSLHandShakeToComplete(out int protocolVersion);

internal abstract void Dispose();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ internal override uint GenerateSspiClientContext(byte[] receivedBuff, uint recei
return 0;
}

internal override uint WaitForSSLHandShakeToComplete() => 0;
internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion)
{
protocolVersion = Handle.ProtocolVersion;
return 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,33 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Security.Authentication;
using System.Threading.Tasks;
using Microsoft.Data.Common;

namespace Microsoft.Data.SqlClient
{
internal class TdsParserStateObjectNative : TdsParserStateObject
{
// protocol versions from native sni
[Flags]
private enum NativeProtocols
{
SP_PROT_SSL2_SERVER = 0x00000004,
SP_PROT_SSL2_CLIENT = 0x00000008,
SP_PROT_SSL3_SERVER = 0x00000010,
SP_PROT_SSL3_CLIENT = 0x00000020,
SP_PROT_TLS1_0_SERVER = 0x00000040,
SP_PROT_TLS1_0_CLIENT = 0x00000080,
SP_PROT_TLS1_1_SERVER = 0x00000100,
SP_PROT_TLS1_1_CLIENT = 0x00000200,
SP_PROT_TLS1_2_SERVER = 0x00000400,
SP_PROT_TLS1_2_CLIENT = 0x00000800,
SP_PROT_TLS1_3_SERVER = 0x00001000,
SP_PROT_TLS1_3_CLIENT = 0x00002000,
SP_PROT_NONE = 0x0
}

private SNIHandle _sessionHandle = null; // the SNI handle we're to work on

private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS
Expand Down Expand Up @@ -309,8 +329,49 @@ internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize)
internal override uint GenerateSspiClientContext(byte[] receivedBuff, uint receivedLength, ref byte[] sendBuff, ref uint sendLength, byte[] _sniSpnBuffer)
=> SNINativeMethodWrapper.SNISecGenClientContext(Handle, receivedBuff, receivedLength, sendBuff, ref sendLength, _sniSpnBuffer);

internal override uint WaitForSSLHandShakeToComplete()
=> SNINativeMethodWrapper.SNIWaitForSSLHandshakeToComplete(Handle, GetTimeoutRemaining());
internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion)
{
uint returnValue = SNINativeMethodWrapper.SNIWaitForSSLHandshakeToComplete(Handle, GetTimeoutRemaining(), out uint nativeProtocolVersion);
var nativeProtocol = (NativeProtocols)nativeProtocolVersion;

/* The SslProtocols.Tls13 is supported by netcoreapp3.1 and later
* This driver does not support this version yet!
if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_3_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_3_SERVER))
{
protocolVersion = (int)SslProtocols.Tls13;
}*/
if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_2_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_2_SERVER))
{
protocolVersion = (int)SslProtocols.Tls12;
}
else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_1_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_1_SERVER))
{
protocolVersion = (int)SslProtocols.Tls11;
}
else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_0_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_0_SERVER))
{
protocolVersion = (int)SslProtocols.Tls;
}
else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_SERVER))
{
#pragma warning disable CS0618 // Type or member is obsolete : SSL is depricated
protocolVersion = (int)SslProtocols.Ssl3;
}
else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL2_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL2_SERVER))
{
protocolVersion = (int)SslProtocols.Ssl2;
#pragma warning restore CS0618 // Type or member is obsolete : SSL is depricated
}
else if(nativeProtocol.HasFlag(NativeProtocols.SP_PROT_NONE))
{
protocolVersion = (int)SslProtocols.None;
}
else
{
throw new ArgumentException(SRHelper.Format(SRHelper.net_invalid_enum, nameof(NativeProtocols)), nameof(NativeProtocols));
}
return returnValue;
}

internal override void DisposePacketCache()
{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Resources/SR.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1860,6 +1860,9 @@
<data name="SQL_BulkLoadCannotConvertValueWithoutRowNo" xml:space="preserve">
<value>The given value{0} of type {1} from the data source cannot be converted to type {2} for Column {3} [{4}].</value>
</data>
<data name="SEC_ProtocolWarning" xml:space="preserve">
<value>Security Warning: The negotiated {0} is an insecure protocol and is supported for backward compatibility only. The recommended protocol version is TLS 1.2 and later.</value>
</data>
<data name="SQL_BulkLoadUnspecifiedSortOrder" xml:space="preserve">
<value>A column order hint cannot have an unspecified sort order.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal static class SNINativeManagedWrapperX64
internal static extern uint SNITerminate();

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIWaitForSSLHandshakeToCompleteWrapper")]
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds);
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds, out uint pProtocolVersion);

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl)]
internal static extern uint UnmanagedIsTokenRestricted([In] IntPtr token, [MarshalAs(UnmanagedType.Bool)] out bool isRestricted);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal static class SNINativeManagedWrapperX86
internal static extern uint SNITerminate();

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIWaitForSSLHandshakeToCompleteWrapper")]
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds);
internal static extern uint SNIWaitForSSLHandshakeToComplete([In] SNIHandle pConn, int dwMilliseconds, out uint pProtocolVersion);

[DllImport(SNI, CallingConvention = CallingConvention.Cdecl)]
internal static extern uint UnmanagedIsTokenRestricted([In] IntPtr token, [MarshalAs(UnmanagedType.Bool)] out bool isRestricted);
Expand Down
Loading

0 comments on commit e601e20

Please sign in to comment.