Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Remove Client property from TcpClient and UdpClient
Browse files Browse the repository at this point in the history 
The property leaks through the abstraction.  It's rarely used.  And it complicates things on Unix due to exposing the underlying socket instance combined with difficulties with multiple connect calls on a single socket instance.

Until we have good reason to expose it in the contract, we're removing it from the contract before it's stable.
  • Loading branch information
@stephentoub
stephentoub committed Feb 7, 2016
1 parent 1bc77b7 commit 30abd30
Show file tree
Hide file tree
Showing 18 changed files with 130 additions and 272 deletions.
93 changes: 75 additions & 18 deletions src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs
View file
Expand Up @@ -6,6 +6,8 @@
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Runtime.ExceptionServices;
using System.Runtime.InteropServices;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;
using System.Threading.Tasks;
Expand All @@ -25,7 +27,6 @@ internal class SNITCPHandle : SNIHandle
private readonly TaskFactory _writeTaskFactory;

private Stream _stream;
private TcpClient _tcpClient;
private SslStream _sslStream;
private SslOverTdsStream _sslOverTdsStream;
private SNIAsyncCallback _receiveCallback;
Expand Down Expand Up @@ -56,12 +57,6 @@ public override void Dispose()
_sslStream.Dispose();
_sslStream = null;
}

if (_tcpClient != null)
{
_tcpClient.Dispose();
_tcpClient = null;
}
}
}

Expand Down Expand Up @@ -103,8 +98,6 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba

try
{
_tcpClient = new TcpClient();

TimeSpan ts;

// In case the Timeout is Infinite, we will receive the max value of Int64 as the tick count
Expand All @@ -116,7 +109,7 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba
ts = ts.Ticks < 0 ? TimeSpan.FromTicks(0) : ts;
}

Task connectTask;
Task<Socket> connectTask;
if (parallel)
{
Task<IPAddress[]> serverAddrTask = Dns.GetHostAddressesAsync(serverName);
Expand All @@ -130,11 +123,11 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba
return;
}

connectTask = _tcpClient.ConnectAsync(serverAddresses, port);
connectTask = ConnectAsync(serverAddresses, port);
}
else
{
connectTask = _tcpClient.ConnectAsync(serverName, port);
connectTask = ConnectAsync(serverName, port);
}

if (!(isInfiniteTimeOut ? connectTask.Wait(-1) : connectTask.Wait(ts)))
Expand All @@ -143,9 +136,9 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba
return;
}

_tcpClient.NoDelay = true;
_tcpStream = _tcpClient.GetStream();
_socket = _tcpClient.Client;
_socket = connectTask.Result;
_socket.NoDelay = true;
_tcpStream = new NetworkStream(_socket, true);

_sslOverTdsStream = new SslOverTdsStream(_tcpStream);
_sslStream = new SslStream(_sslOverTdsStream, true, new RemoteCertificateValidationCallback(ValidateServerCertificate), null);
Expand All @@ -165,6 +158,70 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba
_status = TdsEnums.SNI_SUCCESS;
}

private static async Task<Socket> ConnectAsync(string serverName, int port)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
await socket.ConnectAsync(serverName, port).ConfigureAwait(false);
return socket;
}

// On unix we can't use the instance Socket methods that take multiple endpoints

IPAddress[] addresses = await Dns.GetHostAddressesAsync(serverName).ConfigureAwait(false);
return await ConnectAsync(addresses, port).ConfigureAwait(false);
}

private static async Task<Socket> ConnectAsync(IPAddress[] serverAddresses, int port)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
await socket.ConnectAsync(serverAddresses, port).ConfigureAwait(false);
return socket;
}

// On unix we can't use the instance Socket methods that take multiple endpoints

if (serverAddresses == null)
{
throw new ArgumentNullException("serverAddresses");
}
if (serverAddresses.Length == 0)
{
throw new ArgumentOutOfRangeException("serverAddresses");
}

// Try each address in turn, and return the socket opened for the first one that works.
ExceptionDispatchInfo lastException = null;
foreach (IPAddress address in serverAddresses)
{
var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
try
{
await socket.ConnectAsync(address, port).ConfigureAwait(false);
return socket;
}
catch (Exception exc)
{
socket.Dispose();
lastException = ExceptionDispatchInfo.Capture(exc);
}
}

// Propagate the last failure that occurrred
if (lastException != null)
{
lastException.Throw();
}

// Should never get here. Either there will have been no addresses and we'll have thrown
// at the beginning, or one of the addresses will have worked and we'll have returned, or
// at least one of the addresses will failed, in which case we will have propagated that.
throw new ArgumentException();
}

/// <summary>
/// Enable SSL
/// </summary>
Expand Down Expand Up @@ -276,11 +333,11 @@ public override uint Receive(out SNIPacket packet, int timeoutInMilliseconds)
{
if (timeoutInMilliseconds > 0)
{
_tcpClient.ReceiveTimeout = timeoutInMilliseconds;
_socket.ReceiveTimeout = timeoutInMilliseconds;
}
else if (timeoutInMilliseconds == -1)
{ // SqlCient internally represents infinite timeout by -1, and for TcpClient this is translated to a timeout of 0
_tcpClient.ReceiveTimeout = 0;
_socket.ReceiveTimeout = 0;
}
else
{
Expand Down Expand Up @@ -320,7 +377,7 @@ public override uint Receive(out SNIPacket packet, int timeoutInMilliseconds)
}
finally
{
_tcpClient.ReceiveTimeout = 0;
_socket.ReceiveTimeout = 0;
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs
View file
Expand Up @@ -142,9 +142,8 @@ private Task ClientAsyncSslHelper(SslProtocols clientSslProtocols, SslProtocols
Assert.True(((IAsyncResult)async).AsyncWaitHandle.WaitOne(TestConfiguration.PassingTestTimeoutMilliseconds), "Timed Out");
async.GetAwaiter().GetResult();

_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
server.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.True(sslStream.CipherAlgorithm != CipherAlgorithmType.Null, "Cipher algorithm should not be NULL");
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand Down
10 changes: 4 additions & 6 deletions src/System.Net.Security/tests/FunctionalTests/ClientDefaultEncryptionTest.cs
View file
Expand Up @@ -45,9 +45,8 @@ public async Task ClientDefaultEncryption_ServerRequireEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverRequireEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.True(sslStream.CipherAlgorithm != CipherAlgorithmType.Null, "Cipher algorithm should not be NULL");
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand All @@ -66,9 +65,8 @@ public async Task ClientDefaultEncryption_ServerAllowNoEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverAllowNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.True(sslStream.CipherAlgorithm != CipherAlgorithmType.Null, "Cipher algorithm should not be NULL");
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand Down
11 changes: 4 additions & 7 deletions src/System.Net.Security/tests/FunctionalTests/DummyTcpServer.cs
View file
Expand Up @@ -96,8 +96,7 @@ private void OnAuthenticate(Task result, ClientState state)
try
{
result.GetAwaiter().GetResult();
_log.WriteLine("Server({0}) authenticated to client({1}) with encryption cipher: {2} {3}-bit strength",
state.TcpClient.Client.LocalEndPoint, state.TcpClient.Client.RemoteEndPoint,
_log.WriteLine("Server authenticated to client with encryption cipher: {0} {1}-bit strength",
sslStream.CipherAlgorithm, sslStream.CipherStrength);

// Start listening for data from the client connection.
Expand All @@ -106,15 +105,13 @@ private void OnAuthenticate(Task result, ClientState state)
catch (AuthenticationException authEx)
{
_log.WriteLine(
"Server({0}) disconnecting from client({1}) during authentication. No shared SSL/TLS algorithm. ({2})",
state.TcpClient.Client.LocalEndPoint,
state.TcpClient.Client.RemoteEndPoint,
"Server disconnecting from client during authentication. No shared SSL/TLS algorithm. ({0})",
authEx);
}
catch (Exception ex)
{
_log.WriteLine("Server({0}) disconnecting from client({1}) during authentication. Exception: {2}",
state.TcpClient.Client.LocalEndPoint, state.TcpClient.Client.RemoteEndPoint, ex.Message);
_log.WriteLine("Server disconnecting from client during authentication. Exception: {0}",
ex.Message);
}
finally
{
Expand Down
15 changes: 6 additions & 9 deletions src/System.Net.Security/tests/FunctionalTests/ServerAllowNoEncryptionTest.cs
View file
Expand Up @@ -44,9 +44,8 @@ public async Task ServerAllowNoEncryption_ClientRequireEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.RequireEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverAllowNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.NotEqual(CipherAlgorithmType.Null, sslStream.CipherAlgorithm);
Assert.True(sslStream.CipherStrength > 0);
}
Expand All @@ -65,9 +64,8 @@ public async Task ServerAllowNoEncryption_ClientAllowNoEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.AllowNoEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverAllowNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.NotEqual(CipherAlgorithmType.Null, sslStream.CipherAlgorithm);
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand All @@ -87,9 +85,8 @@ public async Task ServerAllowNoEncryption_ClientNoEncryption_ConnectWithNoEncryp
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.NoEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverAllowNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);

CipherAlgorithmType expected = CipherAlgorithmType.Null;
Assert.Equal(expected, sslStream.CipherAlgorithm);
Expand Down
5 changes: 2 additions & 3 deletions src/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs
View file
Expand Up @@ -171,9 +171,8 @@ private static IEnumerable<object[]> ProtocolMismatchData()
}

_log.WriteLine(
"Server({0}) authenticated client({1}) with encryption cipher: {2} {3}-bit strength",
serverConnection.Client.LocalEndPoint,
serverConnection.Client.RemoteEndPoint,
"Server({0}) authenticated with encryption cipher: {1} {2}-bit strength",
serverEndPoint,
sslServerStream.CipherAlgorithm,
sslServerStream.CipherStrength);

Expand Down
10 changes: 4 additions & 6 deletions src/System.Net.Security/tests/FunctionalTests/ServerNoEncryptionTest.cs
View file
Expand Up @@ -63,9 +63,8 @@ public async Task ServerNoEncryption_ClientAllowNoEncryption_ConnectWithNoEncryp
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);

_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);

CipherAlgorithmType expected = CipherAlgorithmType.Null;
Assert.Equal(expected, sslStream.CipherAlgorithm);
Expand All @@ -87,9 +86,8 @@ public async Task ServerNoEncryption_ClientNoEncryption_ConnectWithNoEncryption(
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.NoEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverNoEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);

CipherAlgorithmType expected = CipherAlgorithmType.Null;
Assert.Equal(expected, sslStream.CipherAlgorithm);
Expand Down
10 changes: 4 additions & 6 deletions src/System.Net.Security/tests/FunctionalTests/ServerRequireEncryptionTest.cs
View file
Expand Up @@ -44,9 +44,8 @@ public async Task ServerRequireEncryption_ClientRequireEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.RequireEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverRequireEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.True(sslStream.CipherAlgorithm != CipherAlgorithmType.Null, "Cipher algorithm should not be NULL");
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand All @@ -65,9 +64,8 @@ public async Task ServerRequireEncryption_ClientAllowNoEncryption_ConnectWithEnc
using (var sslStream = new SslStream(client.GetStream(), false, AllowAnyServerCertificate, null, EncryptionPolicy.AllowNoEncryption))
{
await sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false);
_log.WriteLine("Client({0}) authenticated to server({1}) with encryption cipher: {2} {3}-bit strength",
client.Client.LocalEndPoint, client.Client.RemoteEndPoint,
sslStream.CipherAlgorithm, sslStream.CipherStrength);
_log.WriteLine("Client authenticated to server({0}) with encryption cipher: {1} {2}-bit strength",
serverRequireEncryption.RemoteEndPoint, sslStream.CipherAlgorithm, sslStream.CipherStrength);
Assert.True(sslStream.CipherAlgorithm != CipherAlgorithmType.Null, "Cipher algorithm should not be NULL");
Assert.True(sslStream.CipherStrength > 0, "Cipher strength should be greater than 0");
}
Expand Down

0 comments on commit 30abd30

Please sign in to comment.