From 0bff3d74b4a3679945b32bff5fdd732d178eb174 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Sun, 13 Sep 2026 14:06:49 +0200 Subject: [PATCH 1/2] dedup Session.Connect{Async} --- src/Renci.SshNet/Session.cs | 145 +++++++----------------------------- 1 file changed, 27 insertions(+), 118 deletions(-) diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index ac548b27d..875c7fb0f 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -544,117 +544,7 @@ internal Session(ConnectionInfo connectionInfo, IServiceFactory serviceFactory, /// Failed to establish proxy connection. public void Connect() { - if (IsConnected) - { - return; - } - - _connectLock.Wait(); - - try - { - if (IsConnected) - { - return; - } - - // Reset connection specific information - Reset(); - - // Build list of available messages while connecting - _sshMessageFactory = new SshMessageFactory(); - - _socket = _serviceFactory.CreateConnector(ConnectionInfo, _socketFactory) - .Connect(ConnectionInfo); - - var serverIdentification = _serviceFactory.CreateProtocolVersionExchange() - .Start(ClientVersion, _socket, ConnectionInfo.Timeout); - - // Set connection versions - ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); - - _logger.LogInformation("Server version '{ServerIdentification}'.", serverIdentification); - - if (!(serverIdentification.ProtocolVersion.Equals("2.0") || serverIdentification.ProtocolVersion.Equals("1.99"))) - { - throw new SshConnectionException(string.Format(CultureInfo.CurrentCulture, "Server version '{0}' is not supported.", serverIdentification.ProtocolVersion), - DisconnectReason.ProtocolVersionNotSupported); - } - - ServerIdentificationReceived?.Invoke(this, new SshIdentificationEventArgs(serverIdentification)); - - // Register Transport response messages - RegisterMessage("SSH_MSG_DISCONNECT"); - RegisterMessage("SSH_MSG_IGNORE"); - RegisterMessage("SSH_MSG_UNIMPLEMENTED"); - RegisterMessage("SSH_MSG_DEBUG"); - RegisterMessage("SSH_MSG_SERVICE_ACCEPT"); - RegisterMessage("SSH_MSG_KEXINIT"); - RegisterMessage("SSH_MSG_NEWKEYS"); - - // Some server implementations might sent this message first, prior to establishing encryption algorithm - RegisterMessage("SSH_MSG_USERAUTH_BANNER"); - - // Send our key exchange init. - // We need to do this before starting the message listener to avoid the case where we receive the server - // key exchange init and we continue the key exchange before having sent our own init. - _isInitialKex = true; - ClientInitMessage = BuildClientInitMessage(includeStrictKexPseudoAlgorithm: true); - SendMessage(ClientInitMessage); - - // Mark the message listener threads as started - _ = _messageListenerCompleted.Reset(); - - // Start incoming request listener - // ToDo: Make message pump async, to not consume a thread for every session - _ = ThreadAbstraction.ExecuteThreadLongRunning(MessageListener); - - // Wait for key exchange to be completed - WaitOnHandle(_keyExchangeCompletedWaitHandle.WaitHandle); - - // If sessionId is not set then its not connected - if (SessionId is null) - { - Disconnect(); - return; - } - - // Request user authorization service - SendMessage(new ServiceRequestMessage(ServiceName.UserAuthentication)); - - // Wait for service to be accepted - WaitOnHandle(_serviceAccepted); - - if (string.IsNullOrEmpty(ConnectionInfo.Username)) - { - throw new SshException("Username is not specified."); - } - - // Some servers send a global request immediately after successful authentication - // Avoid race condition by already enabling SSH_MSG_GLOBAL_REQUEST before authentication - RegisterMessage("SSH_MSG_GLOBAL_REQUEST"); - - ConnectionInfo.Authenticate(this, _serviceFactory); - _isAuthenticated = true; - - // Register Connection messages - RegisterMessage("SSH_MSG_REQUEST_SUCCESS"); - RegisterMessage("SSH_MSG_REQUEST_FAILURE"); - RegisterMessage("SSH_MSG_CHANNEL_OPEN_CONFIRMATION"); - RegisterMessage("SSH_MSG_CHANNEL_OPEN_FAILURE"); - RegisterMessage("SSH_MSG_CHANNEL_WINDOW_ADJUST"); - RegisterMessage("SSH_MSG_CHANNEL_EXTENDED_DATA"); - RegisterMessage("SSH_MSG_CHANNEL_REQUEST"); - RegisterMessage("SSH_MSG_CHANNEL_SUCCESS"); - RegisterMessage("SSH_MSG_CHANNEL_FAILURE"); - RegisterMessage("SSH_MSG_CHANNEL_DATA"); - RegisterMessage("SSH_MSG_CHANNEL_EOF"); - RegisterMessage("SSH_MSG_CHANNEL_CLOSE"); - } - finally - { - _ = _connectLock.Release(); - } + ConnectCore(isAsync: false, CancellationToken.None).GetAwaiter().GetResult(); } /// @@ -666,15 +556,28 @@ public void Connect() /// SSH session could not be established. /// Authentication of SSH session failed. /// Failed to establish proxy connection. - public async Task ConnectAsync(CancellationToken cancellationToken) + public Task ConnectAsync(CancellationToken cancellationToken) + { + return ConnectCore(isAsync: true, cancellationToken); + } + + private async Task ConnectCore(bool isAsync, CancellationToken cancellationToken) { - // If connected don't connect again if (IsConnected) { return; } - await _connectLock.WaitAsync(cancellationToken).ConfigureAwait(false); + if (isAsync) + { + await _connectLock.WaitAsync(cancellationToken).ConfigureAwait(false); + } + else + { + Debug.Assert(cancellationToken == default); + + _connectLock.Wait(CancellationToken.None); + } try { @@ -689,11 +592,17 @@ public async Task ConnectAsync(CancellationToken cancellationToken) // Build list of available messages while connecting _sshMessageFactory = new SshMessageFactory(); - _socket = await _serviceFactory.CreateConnector(ConnectionInfo, _socketFactory) - .ConnectAsync(ConnectionInfo, cancellationToken).ConfigureAwait(false); + var connector = _serviceFactory.CreateConnector(ConnectionInfo, _socketFactory); + + _socket = isAsync + ? await connector.ConnectAsync(ConnectionInfo, cancellationToken).ConfigureAwait(false) + : connector.Connect(ConnectionInfo); + + var protocolVersionExchange = _serviceFactory.CreateProtocolVersionExchange(); - var serverIdentification = await _serviceFactory.CreateProtocolVersionExchange() - .StartAsync(ClientVersion, _socket, cancellationToken).ConfigureAwait(false); + var serverIdentification = isAsync + ? await protocolVersionExchange.StartAsync(ClientVersion, _socket, cancellationToken).ConfigureAwait(false) + : protocolVersionExchange.Start(ClientVersion, _socket, ConnectionInfo.Timeout); // Set connection versions ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); From e9a7acd184470b9410739abb153f0ab2dd738036 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Sun, 21 Dec 2025 19:52:06 +0000 Subject: [PATCH 2/2] Inline ProtocolVersionExchange into Session Two motivating reasons: 1. Rather than reading one byte at a time from the socket, we can use the receive buffer to read as much as is available. Once we've read out the SSH identification, leftover bytes are processed as the kexinit messages as normal. 2. To replace explicit tests and mocks on internals (ProtocolVersionExchange class) with tests on public API. This is a first attempt in that direction. The logic is also rewritten to use spans on bytes rather than materialising strings and using regex. --- .../Abstractions/ArrayExtensions.cs | 17 ++ .../Abstractions/SocketAbstraction.Async.cs | 50 ---- .../Abstractions/SocketExtensions.cs | 55 ++-- src/Renci.SshNet/Common/ArrayBuffer.cs | 24 +- src/Renci.SshNet/Common/Extensions.cs | 17 +- .../Connection/IProtocolVersionExchange.cs | 36 --- .../Connection/ProtocolVersionExchange.cs | 252 ------------------ .../Connection/SshIdentification.cs | 18 +- src/Renci.SshNet/IServiceFactory.cs | 9 - src/Renci.SshNet/ServiceFactory.cs | 12 - src/Renci.SshNet/Session.cs | 191 ++++++++++++- ...ectionClosedByServer_NoDataSentByServer.cs | 47 +--- ...est_ServerResponseContainsNullCharacter.cs | 53 +--- ...entificationOnlyContainsProtocolVersion.cs | 48 +--- ...changeTest_ServerResponseValid_Comments.cs | 70 ++--- ...erverResponseValid_EmptySoftwareVersion.cs | 65 ++--- ...angeTest_ServerResponseValid_NoComments.cs | 70 ++--- ...rminatedByLineFeedWithoutCarriageReturn.cs | 70 ++--- ...Test_TimeoutReadingIdentificationString.cs | 38 +-- .../Classes/SessionTest_ConnectedBase.cs | 30 ++- ...Connected_ServerAndClientDisconnectRace.cs | 10 +- .../Classes/SessionTest_ConnectingBase.cs | 32 ++- ...est_SocketConnected_BadPacketAndDispose.cs | 10 +- .../Common/AssertExtensions.cs | 29 ++ .../Renci.SshNet.Tests.csproj | 3 + 25 files changed, 465 insertions(+), 791 deletions(-) create mode 100644 src/Renci.SshNet/Abstractions/ArrayExtensions.cs delete mode 100644 src/Renci.SshNet/Abstractions/SocketAbstraction.Async.cs delete mode 100644 src/Renci.SshNet/Connection/IProtocolVersionExchange.cs delete mode 100644 src/Renci.SshNet/Connection/ProtocolVersionExchange.cs create mode 100644 test/Renci.SshNet.Tests/Common/AssertExtensions.cs diff --git a/src/Renci.SshNet/Abstractions/ArrayExtensions.cs b/src/Renci.SshNet/Abstractions/ArrayExtensions.cs new file mode 100644 index 000000000..7b80894c4 --- /dev/null +++ b/src/Renci.SshNet/Abstractions/ArrayExtensions.cs @@ -0,0 +1,17 @@ +#nullable enable +#if !NET + +namespace System +{ + internal static class ArrayExtensions + { + extension(Array) + { + internal static int MaxLength + { + get { return 0X7FFFFFC7; } + } + } + } +} +#endif diff --git a/src/Renci.SshNet/Abstractions/SocketAbstraction.Async.cs b/src/Renci.SshNet/Abstractions/SocketAbstraction.Async.cs deleted file mode 100644 index 5d13b09d9..000000000 --- a/src/Renci.SshNet/Abstractions/SocketAbstraction.Async.cs +++ /dev/null @@ -1,50 +0,0 @@ -#if NET - -using System; -using System.Diagnostics; -using System.Net.Sockets; -using System.Threading; -using System.Threading.Tasks; - -namespace Renci.SshNet.Abstractions -{ - internal static partial class SocketAbstraction - { - public static ValueTask ReadAsync(Socket socket, byte[] buffer, CancellationToken cancellationToken) - { - return socket.ReceiveAsync(buffer, SocketFlags.None, cancellationToken); - } - - public static ValueTask SendAsync(Socket socket, ReadOnlyMemory data, CancellationToken cancellationToken = default) - { - Debug.Assert(socket != null); - Debug.Assert(data.Length > 0); - - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } - - return SendAsyncCore(socket, data, cancellationToken); - - static async ValueTask SendAsyncCore(Socket socket, ReadOnlyMemory data, CancellationToken cancellationToken) - { - do - { - try - { - var bytesSent = await socket.SendAsync(data, SocketFlags.None, cancellationToken).ConfigureAwait(false); - data = data.Slice(bytesSent); - } - catch (SocketException ex) when (IsErrorResumable(ex.SocketErrorCode)) - { - // Buffer may be full; attempt a short delay and retry - await Task.Delay(30, cancellationToken).ConfigureAwait(false); - } - } - while (data.Length > 0); - } - } - } -} -#endif // NET diff --git a/src/Renci.SshNet/Abstractions/SocketExtensions.cs b/src/Renci.SshNet/Abstractions/SocketExtensions.cs index 85a595ceb..92deb7a6f 100644 --- a/src/Renci.SshNet/Abstractions/SocketExtensions.cs +++ b/src/Renci.SshNet/Abstractions/SocketExtensions.cs @@ -26,19 +26,6 @@ public static Task ConnectAsync(this Socket socket, EndPoint remoteEndpoint, Can return connectTask.IsCompleted || !cancellationToken.CanBeCanceled ? connectTask : WaitWithCancellation(connectTask, socket, cancellationToken); - - static async Task WaitWithCancellation(Task connectTask, Socket socket, CancellationToken cancellationToken) - { - using (cancellationToken.Register(static s => ((Socket)s).Dispose(), socket, useSynchronizationContext: false)) - { - await connectTask.ConfigureAwait(false); - } - } - } - - public static Task ReceiveAsync(this Socket socket, byte[] buffer, CancellationToken cancellationToken) - { - return ReceiveAsync(socket, buffer, 0, buffer.Length, cancellationToken); } public static Task ReceiveAsync(this Socket socket, byte[] buffer, int offset, int length, CancellationToken cancellationToken) @@ -58,13 +45,45 @@ public static Task ReceiveAsync(this Socket socket, byte[] buffer, int offs return receiveTask.IsCompleted || !cancellationToken.CanBeCanceled ? receiveTask : WaitWithCancellation(receiveTask, socket, cancellationToken); + } + + public static Task SendAsync(this Socket socket, byte[] buffer, CancellationToken cancellationToken) + { + return SendAsync(socket, buffer, 0, buffer.Length, cancellationToken); + } + + public static Task SendAsync(this Socket socket, byte[] buffer, int offset, int length, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + + var sendTask = Task.Factory.FromAsync( + static (s, buffer, callback, state) => s.BeginSend(buffer.Array, buffer.Offset, buffer.Count, SocketFlags.None, callback, state), + static result => ((Socket)result.AsyncState).EndSend(result), + socket, + new ArraySegment(buffer, offset, length), + state: socket); + + return sendTask.IsCompleted || !cancellationToken.CanBeCanceled + ? sendTask + : WaitWithCancellation(sendTask, socket, cancellationToken); + } - static async Task WaitWithCancellation(Task receiveTask, Socket socket, CancellationToken cancellationToken) + private static async Task WaitWithCancellation(Task task, Socket socket, CancellationToken cancellationToken) + { + using (cancellationToken.Register(static s => ((Socket)s).Dispose(), socket, useSynchronizationContext: false)) + { + await task.ConfigureAwait(false); + } + } + + private static async Task WaitWithCancellation(Task task, Socket socket, CancellationToken cancellationToken) + { + using (cancellationToken.Register(static s => ((Socket)s).Dispose(), socket, useSynchronizationContext: false)) { - using (cancellationToken.Register(static s => ((Socket)s).Dispose(), socket, useSynchronizationContext: false)) - { - return await receiveTask.ConfigureAwait(false); - } + return await task.ConfigureAwait(false); } } } diff --git a/src/Renci.SshNet/Common/ArrayBuffer.cs b/src/Renci.SshNet/Common/ArrayBuffer.cs index bde8b93bb..f247c8727 100644 --- a/src/Renci.SshNet/Common/ArrayBuffer.cs +++ b/src/Renci.SshNet/Common/ArrayBuffer.cs @@ -1,5 +1,5 @@ #pragma warning disable -// Copied verbatim from https://github.com/dotnet/runtime/blob/d2650b6ae7023a2d9d2c74c56116f1f18472ab04/src/libraries/Common/src/System/Net/ArrayBuffer.cs +// Copied verbatim from https://github.com/dotnet/runtime/blob/591622c69134f9a790f902e364c7a6a5c572b12d/src/libraries/Common/src/System/Net/ArrayBuffer.cs // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. @@ -24,12 +24,6 @@ namespace System.Net [StructLayout(LayoutKind.Auto)] internal struct ArrayBuffer : IDisposable { -#if NET - private static int ArrayMaxLength => Array.MaxLength; -#else - private const int ArrayMaxLength = 0X7FFFFFC7; -#endif - private readonly bool _usePool; private byte[] _bytes; private int _activeStart; @@ -62,8 +56,7 @@ public ArrayBuffer(byte[] buffer) public void Dispose() { - _activeStart = 0; - _availableStart = 0; + DiscardAll(); byte[] array = _bytes; _bytes = null!; @@ -80,8 +73,7 @@ public void ClearAndReturnBuffer() Debug.Assert(_usePool); Debug.Assert(_bytes is not null); - _activeStart = 0; - _availableStart = 0; + DiscardAll(); byte[] bufferToReturn = _bytes; _bytes = Array.Empty(); @@ -115,6 +107,12 @@ public void Discard(int byteCount) } } + public void DiscardAll() + { + _activeStart = 0; + _availableStart = 0; + } + public void Commit(int byteCount) { Debug.Assert(byteCount <= AvailableLength); @@ -155,13 +153,13 @@ private void EnsureAvailableSpaceCore(int byteCount) int desiredSize = ActiveLength + byteCount; - if ((uint)desiredSize > ArrayMaxLength) + if ((uint)desiredSize > Array.MaxLength) { throw new OutOfMemoryException(); } // Double the existing buffer size (capped at Array.MaxLength). - int newSize = Math.Max(desiredSize, (int)Math.Min(ArrayMaxLength, 2 * (uint)_bytes.Length)); + int newSize = Math.Max(desiredSize, (int)Math.Min(Array.MaxLength, 2 * (uint)_bytes.Length)); byte[] newBytes = _usePool ? ArrayPool.Shared.Rent(newSize) : diff --git a/src/Renci.SshNet/Common/Extensions.cs b/src/Renci.SshNet/Common/Extensions.cs index 34b9d3e7a..1d5d9d95b 100644 --- a/src/Renci.SshNet/Common/Extensions.cs +++ b/src/Renci.SshNet/Common/Extensions.cs @@ -12,6 +12,9 @@ using System.Numerics; using System.Runtime.CompilerServices; using System.Security.Cryptography; +#if !NET +using System.Text; +#endif using System.Threading; using Renci.SshNet.Messages; @@ -419,14 +422,6 @@ async Task WaitCore() } } - extension(Array) - { - internal static int MaxLength - { - get { return 0X7FFFFFC7; } - } - } - extension(Task t) { internal bool IsCompletedSuccessfully @@ -460,5 +455,11 @@ public static bool TryComputeHash( return true; #endif } +#if !NET + internal static string GetString(this Encoding encoding, ReadOnlySpan bytes) + { + return encoding.GetString(bytes.ToArray()); + } +#endif } } diff --git a/src/Renci.SshNet/Connection/IProtocolVersionExchange.cs b/src/Renci.SshNet/Connection/IProtocolVersionExchange.cs deleted file mode 100644 index 1cbe3d7f2..000000000 --- a/src/Renci.SshNet/Connection/IProtocolVersionExchange.cs +++ /dev/null @@ -1,36 +0,0 @@ -using System; -using System.Net.Sockets; -using System.Threading; -using System.Threading.Tasks; - -namespace Renci.SshNet.Connection -{ - /// - /// Handles the SSH protocol version exchange. - /// - internal interface IProtocolVersionExchange - { - /// - /// Performs the SSH protocol version exchange. - /// - /// The identification string of the SSH client. - /// A connected to the server. - /// The maximum time to wait for the server to respond. - /// - /// The SSH identification of the server. - /// - SshIdentification Start(string clientVersion, Socket socket, TimeSpan timeout); - - /// - /// Asynchronously performs the SSH protocol version exchange. - /// - /// The identification string of the SSH client. - /// A connected to the server. - /// The token to monitor for cancellation requests. - /// - /// A task that represents the SSH protocol version exchange. The value of its - /// contains the SSH identification of the server. - /// - Task StartAsync(string clientVersion, Socket socket, CancellationToken cancellationToken); - } -} diff --git a/src/Renci.SshNet/Connection/ProtocolVersionExchange.cs b/src/Renci.SshNet/Connection/ProtocolVersionExchange.cs deleted file mode 100644 index c0ff064d7..000000000 --- a/src/Renci.SshNet/Connection/ProtocolVersionExchange.cs +++ /dev/null @@ -1,252 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using System.Net.Sockets; -using System.Text; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; - -using Renci.SshNet.Abstractions; -using Renci.SshNet.Common; -using Renci.SshNet.Messages.Transport; - -namespace Renci.SshNet.Connection -{ - /// - /// Handles the SSH protocol version exchange. - /// - /// - /// https://tools.ietf.org/html/rfc4253#section-4.2. - /// -#pragma warning disable MA0204 // Remove unnecessary partial modifier; not true for all targets - internal sealed partial class ProtocolVersionExchange : IProtocolVersionExchange - { - private const byte Null = 0x00; - private const string ServerVersionPattern = "^SSH-(?[^-]+)-(?.*?)([ ](?.+))?$"; - - private const int MaximumBannerLines = 1024; - private const int MaximumBannerLineLength = 8192; - -#if NET - private static readonly Regex ServerVersionRegex = GetServerVersionRegex(); - - [GeneratedRegex(ServerVersionPattern, RegexOptions.ExplicitCapture)] - private static partial Regex GetServerVersionRegex(); -#else - private static readonly Regex ServerVersionRegex = new Regex(ServerVersionPattern, RegexOptions.Compiled | RegexOptions.ExplicitCapture); -#endif - - /// - /// Performs the SSH protocol version exchange. - /// - /// The identification string of the SSH client. - /// A connected to the server. - /// The maximum time to wait for the server to respond. - /// - /// The SSH identification of the server. - /// - public SshIdentification Start(string clientVersion, Socket socket, TimeSpan timeout) - { - // Immediately send the identification string since the spec states both sides MUST send an identification string - // when the connection has been established - SocketAbstraction.Send(socket, Encoding.UTF8.GetBytes(clientVersion + "\x0D\x0A")); - - var bytesReceived = new List(); - - // Get server version from the server, - // ignore text lines which are sent before if any - for (var n = 0; n < MaximumBannerLines; n++) - { - var line = SocketReadLine(socket, timeout, bytesReceived); - - var identificationMatch = ServerVersionRegex.Match(line); - if (identificationMatch.Success) - { - return new SshIdentification(GetGroupValue(identificationMatch, "protoversion"), - GetGroupValue(identificationMatch, "softwareversion"), - GetGroupValue(identificationMatch, "comments")); - } - } - - throw CreateTooManyLinesReceivedException(); - } - - /// - /// Asynchronously performs the SSH protocol version exchange. - /// - /// The identification string of the SSH client. - /// A connected to the server. - /// The token to monitor for cancellation requests. - /// - /// A task that represents the SSH protocol version exchange. The value of its - /// contains the SSH identification of the server. - /// - public async Task StartAsync(string clientVersion, Socket socket, CancellationToken cancellationToken) - { - // Immediately send the identification string since the spec states both sides MUST send an identification string - // when the connection has been established -#if NET - await SocketAbstraction.SendAsync(socket, Encoding.UTF8.GetBytes(clientVersion + "\x0D\x0A"), cancellationToken).ConfigureAwait(false); -#else - SocketAbstraction.Send(socket, Encoding.UTF8.GetBytes(clientVersion + "\x0D\x0A")); -#endif // NET - - var bytesReceived = new List(); - - // Get server version from the server, - // ignore text lines which are sent before if any - for (var n = 0; n < MaximumBannerLines; n++) - { - var line = await SocketReadLineAsync(socket, bytesReceived, cancellationToken).ConfigureAwait(false); - - var identificationMatch = ServerVersionRegex.Match(line); - if (identificationMatch.Success) - { - return new SshIdentification(GetGroupValue(identificationMatch, "protoversion"), - GetGroupValue(identificationMatch, "softwareversion"), - GetGroupValue(identificationMatch, "comments")); - } - } - - throw CreateTooManyLinesReceivedException(); - } - - private static string GetGroupValue(Match match, string groupName) - { - var commentsGroup = match.Groups[groupName]; - if (commentsGroup.Success) - { - return commentsGroup.Value; - } - - return null; - } - - /// - /// Performs a blocking read on the socket until a line is read. - /// - /// The to read from. - /// A that represents the time to wait until a line is read. - /// A to which read bytes will be added. - /// The read has timed-out. - /// An error occurred when trying to access the socket. - private static string SocketReadLine(Socket socket, TimeSpan timeout, List buffer) - { - var data = new byte[1]; - - buffer.Clear(); - - while (buffer.Count < MaximumBannerLineLength) - { - var bytesRead = SocketAbstraction.Read(socket, data, 0, data.Length, timeout); - if (bytesRead == 0) - { - throw CreateConnectionLostException(); - } - - var byteRead = data[0]; - buffer.Add(byteRead); - - // The null character MUST NOT be sent - if (byteRead is Null) - { - throw CreateServerResponseContainsNullCharacterException(); - } - - if (byteRead == Session.LineFeed) - { - if (buffer.Count >= 2 && buffer[buffer.Count - 2] == Session.CarriageReturn) - { - // Return current line without CRLF - return Encoding.UTF8.GetString(buffer.ToArray(), 0, buffer.Count - 2); - } - - // Even though RFC4253 clearly indicates that the identification string should be terminated - // by a CR LF we also support banners and identification strings that are terminated by a LF - - // Return current line without LF - return Encoding.UTF8.GetString(buffer.ToArray(), 0, buffer.Count - 1); - } - } - - throw CreateBannerLineTooLongException(); - } - - private static async Task SocketReadLineAsync(Socket socket, List buffer, CancellationToken cancellationToken) - { - var data = new byte[1]; - - buffer.Clear(); - - while (buffer.Count < MaximumBannerLineLength) - { - var bytesRead = await socket.ReceiveAsync(data, cancellationToken).ConfigureAwait(false); - if (bytesRead == 0) - { - throw CreateConnectionLostException(); - } - - var byteRead = data[0]; - buffer.Add(byteRead); - - // The null character MUST NOT be sent - if (byteRead is Null) - { - throw CreateServerResponseContainsNullCharacterException(); - } - - if (byteRead == Session.LineFeed) - { - if (buffer.Count >= 2 && buffer[buffer.Count - 2] == Session.CarriageReturn) - { - // Return current line without CRLF - return Encoding.UTF8.GetString(buffer.ToArray(), 0, buffer.Count - 2); - } - - // Even though RFC4253 clearly indicates that the identification string should be terminated - // by a CR LF we also support banners and identification strings that are terminated by a LF - - // Return current line without LF - return Encoding.UTF8.GetString(buffer.ToArray(), 0, buffer.Count - 1); - } - } - - throw CreateBannerLineTooLongException(); - } - - private static SshConnectionException CreateConnectionLostException() - { - return new SshConnectionException( - "The connection to the remote server was closed before a valid SSH identification string was received.", - DisconnectReason.ConnectionLost); - } - - private static SshConnectionException CreateServerResponseContainsNullCharacterException() - { - return new SshConnectionException( - "The server response contained an invalid null character before a valid SSH identification string was received.", - DisconnectReason.ProtocolError); - } - - private static SshConnectionException CreateTooManyLinesReceivedException() - { - var message = string.Format( - CultureInfo.InvariantCulture, - "The server response exceeded the maximum allowed number of banner lines ({0}) before a valid SSH identification string was received.", - MaximumBannerLines); - - return new SshConnectionException(message, DisconnectReason.ProtocolError); - } - - private static SshConnectionException CreateBannerLineTooLongException() - { - var message = string.Format( - CultureInfo.InvariantCulture, - "The server returned a banner line that exceeds the maximum allowed length of {0} bytes.", - MaximumBannerLineLength); - - return new SshConnectionException(message, DisconnectReason.ProtocolError); - } - } -} diff --git a/src/Renci.SshNet/Connection/SshIdentification.cs b/src/Renci.SshNet/Connection/SshIdentification.cs index 25e84d03d..cd9df10d5 100644 --- a/src/Renci.SshNet/Connection/SshIdentification.cs +++ b/src/Renci.SshNet/Connection/SshIdentification.cs @@ -1,4 +1,5 @@ -using System; +#nullable enable +using System; namespace Renci.SshNet.Connection { @@ -29,7 +30,7 @@ public SshIdentification(string protocolVersion, string softwareVersion) /// The comments. /// is . /// is . - public SshIdentification(string protocolVersion, string softwareVersion, string comments) + public SshIdentification(string protocolVersion, string softwareVersion, string? comments) { ArgumentNullException.ThrowIfNull(protocolVersion); ArgumentNullException.ThrowIfNull(softwareVersion); @@ -69,7 +70,7 @@ public SshIdentification(string protocolVersion, string softwareVersion, string /// should contain additional information that might be useful /// in solving user problems. /// - public string Comments { get; } + public string? Comments { get; } /// /// Returns the SSH identification string. @@ -79,14 +80,9 @@ public SshIdentification(string protocolVersion, string softwareVersion, string /// public override string ToString() { - var identificationString = "SSH-" + ProtocolVersion + "-" + SoftwareVersion; - - if (Comments != null) - { - identificationString += " " + Comments; - } - - return identificationString; + return Comments != null + ? $"SSH-{ProtocolVersion}-{SoftwareVersion} {Comments}" + : $"SSH-{ProtocolVersion}-{SoftwareVersion}"; } } } diff --git a/src/Renci.SshNet/IServiceFactory.cs b/src/Renci.SshNet/IServiceFactory.cs index 6830c7baf..562d2cc6c 100644 --- a/src/Renci.SshNet/IServiceFactory.cs +++ b/src/Renci.SshNet/IServiceFactory.cs @@ -151,15 +151,6 @@ ShellStream CreateShellStream(ISession session, /// The value of is not supported. IConnector CreateConnector(IConnectionInfo connectionInfo, ISocketFactory socketFactory); - /// - /// Creates an that deals with the SSH protocol - /// version exchange. - /// - /// - /// An . - /// - IProtocolVersionExchange CreateProtocolVersionExchange(); - /// /// Creates a factory to create instances. /// diff --git a/src/Renci.SshNet/ServiceFactory.cs b/src/Renci.SshNet/ServiceFactory.cs index e560b4335..b1db5c629 100644 --- a/src/Renci.SshNet/ServiceFactory.cs +++ b/src/Renci.SshNet/ServiceFactory.cs @@ -174,18 +174,6 @@ public IConnector CreateConnector(IConnectionInfo connectionInfo, ISocketFactory } } - /// - /// Creates an that deals with the SSH protocol - /// version exchange. - /// - /// - /// An . - /// - public IProtocolVersionExchange CreateProtocolVersionExchange() - { - return new ProtocolVersionExchange(); - } - /// /// Creates a factory to create instances. /// diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 875c7fb0f..c8288e080 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -6,6 +6,7 @@ using System.Net; using System.Net.Sockets; using System.Security.Cryptography; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -33,6 +34,18 @@ public sealed class Session : ISession internal const byte CarriageReturn = 0x0d; internal const byte LineFeed = 0x0a; + /// + /// Maximum number of lines that we permit from the server when waiting + /// for a valid SSH identification string. + /// + private const int MaximumBannerLines = 1024; + + /// + /// Maximum length of a line we permit from the server when waiting for + /// a valid SSH identification string. + /// + private const int MaximumBannerLineLength = 8192; + /// /// Specifies maximum packet size defined by the protocol. /// @@ -72,6 +85,8 @@ public sealed class Session : ISession internal static readonly string ClientVersionString = "SSH-2.0-Renci.SshNet.SshClient." + ThisAssembly.NuGetPackageVersion.Replace('-', '_'); + internal static readonly byte[] ClientVersionBytes = [.. Encoding.ASCII.GetBytes(ClientVersionString), (byte)'\r', (byte)'\n']; + /// /// Holds the factory to use for creating new services. /// @@ -594,15 +609,18 @@ private async Task ConnectCore(bool isAsync, CancellationToken cancellationToken var connector = _serviceFactory.CreateConnector(ConnectionInfo, _socketFactory); - _socket = isAsync - ? await connector.ConnectAsync(ConnectionInfo, cancellationToken).ConfigureAwait(false) - : connector.Connect(ConnectionInfo); - - var protocolVersionExchange = _serviceFactory.CreateProtocolVersionExchange(); + SshIdentification serverIdentification; - var serverIdentification = isAsync - ? await protocolVersionExchange.StartAsync(ClientVersion, _socket, cancellationToken).ConfigureAwait(false) - : protocolVersionExchange.Start(ClientVersion, _socket, ConnectionInfo.Timeout); + if (isAsync) + { + _socket = await connector.ConnectAsync(ConnectionInfo, cancellationToken).ConfigureAwait(false); + serverIdentification = await DoProtocolVersionExchange(_socket, ConnectionInfo.Timeout, isAsync: true, cancellationToken).ConfigureAwait(false); + } + else + { + _socket = connector.Connect(ConnectionInfo); + serverIdentification = DoProtocolVersionExchange(_socket, ConnectionInfo.Timeout, isAsync: false, CancellationToken.None).GetAwaiter().GetResult(); + } // Set connection versions ServerVersion = ConnectionInfo.ServerVersion = serverIdentification.ToString(); @@ -691,6 +709,162 @@ private async Task ConnectCore(bool isAsync, CancellationToken cancellationToken } } + private async Task DoProtocolVersionExchange( + Socket socket, + TimeSpan timeout, + bool isAsync, + CancellationToken cancellationToken) + { + if (isAsync) + { + _ = await socket.SendAsync(ClientVersionBytes, cancellationToken).ConfigureAwait(false); + } + else + { + socket.SendTimeout = socket.ReceiveTimeout = timeout.AsTimeout(); + + _ = socket.Send(ClientVersionBytes); + } + + for (var n = 0; n < MaximumBannerLines; n++) + { + var bytesObserved = 0; + int indexOfLf; + + while (true) + { + var boundedLength = Math.Min(_receiveBuffer.ActiveLength, MaximumBannerLineLength); + + // Bound the search space below by what we have already observed, in order + // to avoid O(n^2) behavior when the data comes in piecewise, and above by + // the maximum line length to also avoid redundant work since we will throw + // if we exceed that anyway. + var relativeIndexOfNullOrLf = _receiveBuffer.ActiveReadOnlySpan + .Slice(bytesObserved, boundedLength - bytesObserved) + .IndexOfAny((byte)0, (byte)'\n'); + + if (relativeIndexOfNullOrLf >= 0) + { + var indexOfNullOrLf = bytesObserved + relativeIndexOfNullOrLf; + + if (_receiveBuffer.ActiveReadOnlySpan[indexOfNullOrLf] == 0) + { + throw new SshConnectionException( + "The server response contained an invalid null character before a valid SSH identification string was received.", + DisconnectReason.ProtocolError); + } + + indexOfLf = indexOfNullOrLf; + break; + } + + if (_receiveBuffer.ActiveLength >= MaximumBannerLineLength) + { + throw new SshConnectionException( + string.Format( + CultureInfo.InvariantCulture, + "The server returned a banner line that exceeds the maximum allowed length of {0} bytes.", + MaximumBannerLineLength), + DisconnectReason.ProtocolError); + } + + bytesObserved = _receiveBuffer.ActiveLength; + + _receiveBuffer.EnsureAvailableSpace(1); + + int bytesRead; + if (isAsync) + { +#if NET + bytesRead = await socket.ReceiveAsync(_receiveBuffer.AvailableMemory, cancellationToken).ConfigureAwait(false); +#else + bytesRead = await socket.ReceiveAsync( + buffer: _receiveBuffer.DangerousGetUnderlyingBuffer(), + offset: _receiveBuffer.ActiveStartOffset + _receiveBuffer.ActiveLength, + length: _receiveBuffer.AvailableLength, + cancellationToken).ConfigureAwait(false); +#endif + } + else + { + try + { + bytesRead = socket.Receive( + buffer: _receiveBuffer.DangerousGetUnderlyingBuffer(), + offset: _receiveBuffer.ActiveStartOffset + _receiveBuffer.ActiveLength, + size: _receiveBuffer.AvailableLength, + SocketFlags.None); + } + catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut) + { + throw new SshOperationTimeoutException( + string.Format(CultureInfo.InvariantCulture, + "Socket read operation has timed out after {0:F0} milliseconds.", + timeout.TotalMilliseconds), + ex); + } + } + + _receiveBuffer.Commit(bytesRead); + + if (bytesRead == 0) + { + throw new SshConnectionException( + "The connection to the remote server was closed before a valid SSH identification string was received.", + DisconnectReason.ConnectionLost); + } + } + + var currentLine = _receiveBuffer.ActiveReadOnlySpan.Slice(0, indexOfLf); + + if (!currentLine.IsEmpty && currentLine[^1] == (byte)'\r') + { + currentLine = currentLine[..^1]; + } + + int indexOfDash; + + if (!currentLine.StartsWith("SSH-"u8) || + (indexOfDash = currentLine.Slice(4).IndexOf((byte)'-')) < 0) + { + // This line is not interesting to us, go for a new one. + _receiveBuffer.Discard(indexOfLf + 1); + continue; + } + + var protocolVersion = Encoding.ASCII.GetString(_receiveBuffer.ActiveReadOnlySpan.Slice(4, indexOfDash)); + + var softwareVersionAndComments = currentLine.Slice(4 + indexOfDash + 1); + + var indexOfSpace = softwareVersionAndComments.IndexOf((byte)' '); + + string softwareVersion; + string comments; + + if (indexOfSpace >= 0) + { + softwareVersion = Encoding.ASCII.GetString(softwareVersionAndComments.Slice(0, indexOfSpace)); + comments = Encoding.UTF8.GetString(softwareVersionAndComments.Slice(indexOfSpace + 1)); + } + else + { + softwareVersion = Encoding.ASCII.GetString(softwareVersionAndComments); + comments = null; + } + + _receiveBuffer.Discard(indexOfLf + 1); + + return new SshIdentification(protocolVersion, softwareVersion, comments); + } + + throw new SshConnectionException( + string.Format( + CultureInfo.InvariantCulture, + "The server response exceeded the maximum allowed number of banner lines ({0}) before a valid SSH identification string was received.", + MaximumBannerLines), + DisconnectReason.ProtocolError); + } + /// /// Disconnects from the server. /// @@ -1915,6 +2089,7 @@ private void Reset() _isDisconnecting = false; _isAuthenticated = false; _exception = null; + _receiveBuffer.DiscardAll(); } private static SshConnectionException CreateConnectionAbortedByServerException() diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer.cs index 5380ca968..563eb7fac 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer.cs @@ -7,7 +7,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; -using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Connection @@ -16,13 +15,10 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ConnectionClosedByServer_NoDataSentByServer { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshConnectionException _actualException; [TestInitialize] @@ -38,17 +34,12 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "\uD55C"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); @@ -60,19 +51,16 @@ protected void Arrange() _dataReceivedByServer.AddRange(bytes); socket.Shutdown(SocketShutdown.Send); }; - _server.Disconnected += (socket) => _clientDisconnected = true; - - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { try { - _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.Connect(); Assert.Fail(); } catch (SshConnectionException ex) @@ -85,7 +73,7 @@ protected void Act() } [TestMethod] - public void StartShouldHaveThrownSshConnectionException() + public void ConnectShouldHaveThrownSshConnectionException() { Assert.IsInstanceOfType(_actualException); Assert.IsNull(_actualException.InnerException); @@ -95,26 +83,7 @@ public void StartShouldHaveThrownSshConnectionException() [TestMethod] public void ClientIdentificationWasSentToServer() { - Assert.HasCount(5, _dataReceivedByServer); - - Assert.AreEqual(0xed, _dataReceivedByServer[0]); - Assert.AreEqual(0x95, _dataReceivedByServer[1]); - Assert.AreEqual(0x9c, _dataReceivedByServer[2]); - Assert.AreEqual(0x0d, _dataReceivedByServer[3]); - Assert.AreEqual(0x0a, _dataReceivedByServer[4]); - } - - [TestMethod] - public void ConnectionIsClosedByServer() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - - var bytesReceived = _client.Receive(new byte[1]); - Assert.AreEqual(0, bytesReceived); - - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); + Assert.AreSequenceEqual(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter.cs index 370000cfe..b583600e9 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter.cs @@ -1,15 +1,12 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; -using System.Net.Sockets; using System.Text; using System.Threading; using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; -using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Connection @@ -18,14 +15,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseContainsNullCharacter { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshConnectionException _actualException; [TestInitialize] @@ -41,17 +35,12 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); @@ -64,19 +53,16 @@ protected void Arrange() { _dataReceivedByServer.AddRange(bytes); }; - _server.Disconnected += (socket) => _clientDisconnected = true; - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { try { - _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.Connect(); Assert.Fail(); } catch (SshConnectionException ex) @@ -89,7 +75,7 @@ protected void Act() } [TestMethod] - public void StartShouldHaveThrownSshConnectionException() + public void ConnectShouldHaveThrownSshConnectionException() { Assert.IsInstanceOfType(_actualException); Assert.IsNull(_actualException.InnerException); @@ -99,30 +85,7 @@ public void StartShouldHaveThrownSshConnectionException() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ClientRemainsConnected() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - } - - [TestMethod] - public void ClientHasNotReadPastNullCharacter() - { - var buffer = new byte[1]; - - var bytesReceived = _client.Receive(buffer); - Assert.AreEqual(1, bytesReceived); - Assert.AreEqual(0xed, buffer[0]); + Assert.AreSequenceEqual(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion.cs index e9985bcab..4012995de 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -9,7 +8,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; -using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Connection @@ -18,14 +16,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseInvalid_SshIdentificationOnlyContainsProtocolVersion { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshConnectionException _actualException; [TestInitialize] @@ -41,17 +36,12 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); @@ -67,19 +57,16 @@ protected void Arrange() socket.Shutdown(SocketShutdown.Send); }; - _server.Disconnected += (socket) => _clientDisconnected = true; - - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { try { - _ = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.Connect(); Assert.Fail(); } catch (SshConnectionException ex) @@ -92,7 +79,7 @@ protected void Act() } [TestMethod] - public void StartShouldHaveThrownSshConnectionException() + public void ConnectShouldHaveThrownSshConnectionException() { Assert.IsInstanceOfType(_actualException); Assert.IsNull(_actualException.InnerException); @@ -102,26 +89,7 @@ public void StartShouldHaveThrownSshConnectionException() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ConnectionIsClosedByServer() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - - var bytesReceived = _client.Receive(new byte[1]); - Assert.AreEqual(0, bytesReceived); - - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); + Assert.AreSequenceEqual(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_Comments.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_Comments.cs index 9875bdb39..7b409760c 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_Comments.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_Comments.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -8,6 +7,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Renci.SshNet.Common; using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; @@ -17,14 +17,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseValid_Comments { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshIdentification _actual; [TestInitialize] @@ -40,51 +37,53 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); - _serverIdentification = Encoding.UTF8.GetBytes("\r\nWelcome stranger!\r\n\r\nSSH-ABC2.0-OurSSHAppliance-1.4.7 Use at own risk.\uD55C\r\n!"); + _serverIdentification = Encoding.UTF8.GetBytes("\r\nWelcome stranger!\r\n\r\nSSH-2.0-OurSSHAppliance-1.4.7 Use at own risk.\uD55C\r\n!"); _server = new AsyncSocketListener(_serverEndPoint); _server.Start(); _server.BytesReceived += (bytes, socket) => { + var isFirstReceive = _dataReceivedByServer.Count == 0; + _dataReceivedByServer.AddRange(bytes); - socket.Send(_serverIdentification); - socket.Shutdown(SocketShutdown.Send); - }; - _server.Disconnected += (socket) => _clientDisconnected = true; - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); + if (isFirstReceive) + { + socket.Send(_serverIdentification); + socket.Shutdown(SocketShutdown.Send); + } + }; - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { - _actual = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.ServerIdentificationReceived += (sender, e) => _actual = e.SshIdentification; + + // Since the server is not a full implementation, connecting fails after the + // identification has been received. + _ = Assert.ThrowsExactly(_client.Connect); // Give some time to process all messages Thread.Sleep(200); } [TestMethod] - public void StartShouldReturnIdentificationOfServer() + public void ConnectShouldReturnIdentificationOfServer() { Assert.IsNotNull(_actual); - Assert.AreEqual("ABC2.0", _actual.ProtocolVersion); + Assert.AreEqual("2.0", _actual.ProtocolVersion); Assert.AreEqual("OurSSHAppliance-1.4.7", _actual.SoftwareVersion); Assert.AreEqual("Use at own risk.\uD55C", _actual.Comments); } @@ -92,30 +91,7 @@ public void StartShouldReturnIdentificationOfServer() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ClientRemainsConnected() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - } - - [TestMethod] - public void ClientDidNotReadPastIdentification() - { - var buffer = new byte[1]; - - var bytesReceived = _client.Receive(buffer); - Assert.AreEqual(1, bytesReceived); - Assert.AreEqual(0x21, buffer[0]); + Assert.SequenceStartsWith(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_EmptySoftwareVersion.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_EmptySoftwareVersion.cs index eff1b4e45..11fb22464 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_EmptySoftwareVersion.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_EmptySoftwareVersion.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -18,14 +17,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseValid_EmptySoftwareVersion { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshIdentification _actual; [TestInitialize] @@ -41,53 +37,53 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); - _serverIdentification = Encoding.UTF8.GetBytes("SSH-78.5-\r\n"); + _serverIdentification = Encoding.UTF8.GetBytes("SSH-2.0-\r\n"); _server = new AsyncSocketListener(_serverEndPoint); _server.Start(); _server.BytesReceived += (bytes, socket) => { - _dataReceivedByServer.AddRange(bytes); + var isFirstReceive = _dataReceivedByServer.Count == 0; - _ = socket.Send(_serverIdentification); + _dataReceivedByServer.AddRange(bytes); - socket.Shutdown(SocketShutdown.Send); + if (isFirstReceive) + { + _ = socket.Send(_serverIdentification); + socket.Shutdown(SocketShutdown.Send); + } }; - _server.Disconnected += (socket) => _clientDisconnected = true; - - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { - _actual = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.ServerIdentificationReceived += (sender, e) => _actual = e.SshIdentification; + + // Since the server is not a full implementation, connecting fails after the + // identification has been received. + _ = Assert.ThrowsExactly(_client.Connect); // Give some time to process all messages Thread.Sleep(200); } [TestMethod] - public void StartShouldReturnIdentificationOfServer() + public void ConnectShouldReturnIdentificationOfServer() { Assert.IsNotNull(_actual); - Assert.AreEqual("78.5", _actual.ProtocolVersion); + Assert.AreEqual("2.0", _actual.ProtocolVersion); Assert.AreEqual(string.Empty, _actual.SoftwareVersion); Assert.IsNull(_actual.Comments); } @@ -95,26 +91,7 @@ public void StartShouldReturnIdentificationOfServer() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ConnectionIsClosedByServer() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - - var bytesReceived = _client.Receive(new byte[1]); - Assert.AreEqual(0, bytesReceived); - - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); + Assert.SequenceStartsWith(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_NoComments.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_NoComments.cs index f5ff62308..9b7cc2d08 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_NoComments.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_NoComments.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -8,6 +7,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Renci.SshNet.Common; using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; @@ -17,14 +17,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseValid_NoComments { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshIdentification _actual; [TestInitialize] @@ -40,51 +37,53 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); - _serverIdentification = Encoding.UTF8.GetBytes("Welcome stranger!\r\n\r\nSSH-Zero-OurSSHAppliance\r\n!"); + _serverIdentification = Encoding.UTF8.GetBytes("Welcome stranger!\r\n\r\nSSH-2.0-OurSSHAppliance\r\n!"); _server = new AsyncSocketListener(_serverEndPoint); _server.Start(); _server.BytesReceived += (bytes, socket) => { + var isFirstReceive = _dataReceivedByServer.Count == 0; + _dataReceivedByServer.AddRange(bytes); - socket.Send(_serverIdentification); - socket.Shutdown(SocketShutdown.Send); - }; - _server.Disconnected += (socket) => _clientDisconnected = true; - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); + if (isFirstReceive) + { + socket.Send(_serverIdentification); + socket.Shutdown(SocketShutdown.Send); + } + }; - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { - _actual = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.ServerIdentificationReceived += (sender, e) => _actual = e.SshIdentification; + + // Since the server is not a full implementation, connecting fails after the + // identification has been received. + _ = Assert.ThrowsExactly(_client.Connect); // Give some time to process all messages Thread.Sleep(200); } [TestMethod] - public void StartShouldReturnIdentificationOfServer() + public void ConnectShouldReturnIdentificationOfServer() { Assert.IsNotNull(_actual); - Assert.AreEqual("Zero", _actual.ProtocolVersion); + Assert.AreEqual("2.0", _actual.ProtocolVersion); Assert.AreEqual("OurSSHAppliance", _actual.SoftwareVersion); Assert.IsNull(_actual.Comments); } @@ -92,30 +91,7 @@ public void StartShouldReturnIdentificationOfServer() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ClientRemainsConnected() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - } - - [TestMethod] - public void ClientDidNotReadPastIdentification() - { - var buffer = new byte[1]; - - var bytesReceived = _client.Receive(buffer); - Assert.AreEqual(1, bytesReceived); - Assert.AreEqual(0x21, buffer[0]); + Assert.SequenceStartsWith(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_TerminatedByLineFeedWithoutCarriageReturn.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_TerminatedByLineFeedWithoutCarriageReturn.cs index d9df1e9f9..c5fa35b99 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_TerminatedByLineFeedWithoutCarriageReturn.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_ServerResponseValid_TerminatedByLineFeedWithoutCarriageReturn.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; @@ -8,6 +7,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Renci.SshNet.Common; using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; @@ -17,14 +17,11 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_ServerResponseValid_TerminatedByLineFeedWithoutCarriageReturn { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; private byte[] _serverIdentification; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshIdentification _actual; [TestInitialize] @@ -40,53 +37,53 @@ public void Cleanup() _server?.Dispose(); _server = null; - if (_client != null) - { - _client.Shutdown(SocketShutdown.Both); - _client.Close(); - _client = null; - } + _client?.Dispose(); + _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromSeconds(5); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); - _serverIdentification = Encoding.UTF8.GetBytes("Welcome stranger!\n\nSSH-Zero-OurSSHAppliance\n\0"); + _serverIdentification = Encoding.UTF8.GetBytes("Welcome stranger!\n\nSSH-2.0-OurSSHAppliance\n\0"); _server = new AsyncSocketListener(_serverEndPoint); _server.Start(); _server.BytesReceived += (bytes, socket) => { - _dataReceivedByServer.AddRange(bytes); + var isFirstReceive = _dataReceivedByServer.Count == 0; - _ = socket.Send(_serverIdentification); + _dataReceivedByServer.AddRange(bytes); - socket.Shutdown(SocketShutdown.Send); + if (isFirstReceive) + { + _ = socket.Send(_serverIdentification); + socket.Shutdown(SocketShutdown.Send); + } }; - _server.Disconnected += (socket) => _clientDisconnected = true; - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { - _actual = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.ServerIdentificationReceived += (sender, e) => _actual = e.SshIdentification; + + // Since the server is not a full implementation, connecting fails after the + // identification has been received. + _ = Assert.ThrowsExactly(_client.Connect); // Give some time to process all messages Thread.Sleep(200); } [TestMethod] - public void StartShouldReturnIdentificationOfServer() + public void ConnectShouldReturnIdentificationOfServer() { Assert.IsNotNull(_actual); - Assert.AreEqual("Zero", _actual.ProtocolVersion); + Assert.AreEqual("2.0", _actual.ProtocolVersion); Assert.AreEqual("OurSSHAppliance", _actual.SoftwareVersion); Assert.IsNull(_actual.Comments); } @@ -94,30 +91,7 @@ public void StartShouldReturnIdentificationOfServer() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ClientRemainsConnected() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); - } - - [TestMethod] - public void ClientDidNotReadPastIdentification() - { - var buffer = new byte[1]; - - var bytesReceived = _client.Receive(buffer); - Assert.AreEqual(1, bytesReceived); - Assert.AreEqual(0x00, buffer[0]); + Assert.SequenceStartsWith(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_TimeoutReadingIdentificationString.cs b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_TimeoutReadingIdentificationString.cs index 9dde3563f..77389794f 100644 --- a/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_TimeoutReadingIdentificationString.cs +++ b/test/Renci.SshNet.Tests/Classes/Connection/ProtocolVersionExchangeTest_TimeoutReadingIdentificationString.cs @@ -1,15 +1,12 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Net; -using System.Net.Sockets; using System.Text; using System.Threading; using Microsoft.VisualStudio.TestTools.UnitTesting; using Renci.SshNet.Common; -using Renci.SshNet.Connection; using Renci.SshNet.Tests.Common; namespace Renci.SshNet.Tests.Classes.Connection @@ -18,13 +15,10 @@ namespace Renci.SshNet.Tests.Classes.Connection public class ProtocolVersionExchangeTest_TimeoutReadingIdentificationString { private AsyncSocketListener _server; - private ProtocolVersionExchange _protocolVersionExchange; - private string _clientVersion; private TimeSpan _timeout; private IPEndPoint _serverEndPoint; private List _dataReceivedByServer; - private bool _clientDisconnected; - private Socket _client; + private SshClient _client; private SshOperationTimeoutException _actualException; [TestInitialize] @@ -40,17 +34,15 @@ public void Cleanup() _server?.Dispose(); _server = null; - _client?.Close(); + _client?.Dispose(); _client = null; } protected void Arrange() { - _clientVersion = "SSH-2.0-Renci.SshNet.SshClient.0.0.1"; _timeout = TimeSpan.FromMilliseconds(200); _serverEndPoint = new IPEndPoint(IPAddress.Loopback, 8122); _dataReceivedByServer = new List(); - _clientDisconnected = false; _server = new AsyncSocketListener(_serverEndPoint); _server.Start(); @@ -60,19 +52,16 @@ protected void Arrange() _ = socket.Send(Encoding.UTF8.GetBytes("Welcome!\r\n")); }; - _server.Disconnected += (socket) => _clientDisconnected = true; - _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - _client.Connect(_serverEndPoint); - - _protocolVersionExchange = new ProtocolVersionExchange(); + _client = new SshClient(_serverEndPoint.Address.ToString(), _serverEndPoint.Port, "user", "password"); + _client.ConnectionInfo.Timeout = _timeout; } protected void Act() { try { - _ = _protocolVersionExchange.Start(_clientVersion, _client, _timeout); + _client.Connect(); Assert.Fail(); } catch (SshOperationTimeoutException ex) @@ -85,7 +74,7 @@ protected void Act() } [TestMethod] - public void StartShouldHaveThrownSshOperationTimeoutException() + public void ConnectShouldHaveThrownSshOperationTimeoutException() { Assert.IsNotNull(_actualException); Assert.AreEqual(string.Format("Socket read operation has timed out after {0} milliseconds.", _timeout.TotalMilliseconds), _actualException.Message); @@ -94,20 +83,7 @@ public void StartShouldHaveThrownSshOperationTimeoutException() [TestMethod] public void ClientIdentificationWasSentToServer() { - var expected = Encoding.UTF8.GetBytes(_clientVersion); - - Assert.HasCount(expected.Length + 2, _dataReceivedByServer); - - CollectionAssert.AreEqual(expected, _dataReceivedByServer.Take(expected.Length).ToArray()); - Assert.AreEqual(Session.CarriageReturn, _dataReceivedByServer[_dataReceivedByServer.Count - 2]); - Assert.AreEqual(Session.LineFeed, _dataReceivedByServer[_dataReceivedByServer.Count - 1]); - } - - [TestMethod] - public void ClientSocketShouldBeConnected() - { - Assert.IsTrue(_client.Connected); - Assert.IsFalse(_clientDisconnected); + Assert.SequenceStartsWith(Session.ClientVersionBytes, _dataReceivedByServer); } } } diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs index c8ff510e9..8b3bc1174 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Sockets; using System.Security.Cryptography; +using System.Text; using System.Threading; using Microsoft.Extensions.Logging.Abstractions; @@ -28,12 +29,12 @@ public abstract class SessionTest_ConnectedBase internal Mock SocketFactoryMock { get; private set; } internal Mock ConnectorMock { get; private set; } - private Mock _protocolVersionExchangeMock; private Mock _keyExchangeMock; private Mock _clientAuthenticationMock; private IPEndPoint _serverEndPoint; private string _keyExchangeAlgorithm; private bool _authenticationStarted; + private bool _clientIdentificationReceived; private SocketFactory _socketFactory; protected Random Random { get; private set; } @@ -106,6 +107,7 @@ protected virtual void SetupData() FirstKexReceived = new ManualResetEventSlim(); ServerIdentification = new SshIdentification("2.0", "OurServerStub"); _authenticationStarted = false; + _clientIdentificationReceived = false; _socketFactory = new SocketFactory(); Session = new Session(ConnectionInfo, ServiceFactoryMock.Object, SocketFactoryMock.Object); @@ -136,8 +138,7 @@ protected virtual void SetupData() { ServerSocket = socket; - // Since we're mocking the protocol version exchange, we'll immediately start KEX upon - // having established the connection instead of when the client has been identified + _ = ServerSocket.Send(Encoding.ASCII.GetBytes(ServerIdentification + "\r\n")); if (!WaitForClientKeyExchangeInit) { @@ -146,6 +147,24 @@ protected virtual void SetupData() }; ServerListener.BytesReceived += (received, socket) => { + if (!_clientIdentificationReceived) + { + var indexOfLf = Array.IndexOf(received, (byte)'\n'); + if (indexOfLf < 0) + { + return; + } + + _clientIdentificationReceived = true; + + if (indexOfLf == received.Length - 1) + { + return; + } + + received = received.AsSpan(indexOfLf + 1).ToArray(); + } + ServerBytesReceivedRegister.Add(received); if (received.Length > 5 && received[5] == 20) @@ -189,7 +208,6 @@ private void CreateMocks() ServiceFactoryMock = new Mock(MockBehavior.Strict); SocketFactoryMock = new Mock(MockBehavior.Strict); ConnectorMock = new Mock(MockBehavior.Strict); - _protocolVersionExchangeMock = new Mock(MockBehavior.Strict); _keyExchangeMock = new Mock(MockBehavior.Strict); _clientAuthenticationMock = new Mock(MockBehavior.Strict); } @@ -200,10 +218,6 @@ private void SetupMocks() .Returns(ConnectorMock.Object); _ = ConnectorMock.Setup(p => p.Connect(ConnectionInfo)) .Returns(ClientSocket); - _ = ServiceFactoryMock.Setup(p => p.CreateProtocolVersionExchange()) - .Returns(_protocolVersionExchangeMock.Object); - _ = _protocolVersionExchangeMock.Setup(p => p.Start(Session.ClientVersion, ClientSocket, ConnectionInfo.Timeout)) - .Returns(ServerIdentification); _ = ServiceFactoryMock.Setup(p => p.CreateKeyExchange(ConnectionInfo.KeyExchangeAlgorithms, new[] { _keyExchangeAlgorithm })).Returns(_keyExchangeMock.Object); _ = _keyExchangeMock.Setup(p => p.Name) .Returns(_keyExchangeAlgorithm); diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs index 9114ae151..b7f403fbc 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Sockets; using System.Security.Cryptography; +using System.Text; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -27,7 +28,6 @@ public class SessionTest_Connected_ServerAndClientDisconnectRace private Mock _serviceFactoryMock; private Mock _socketFactoryMock; private Mock _connectorMock; - private Mock _protocolVersionExchangeMock; private Mock _keyExchangeMock; private Mock _clientAuthenticationMock; private IPEndPoint _serverEndPoint; @@ -107,8 +107,7 @@ protected virtual void SetupData() { ServerSocket = socket; - // Since we're mocking the protocol version exchange, we'll immediately stat KEX upon - // having established the connection instead of when the client has been identified + _ = ServerSocket.Send(Encoding.ASCII.GetBytes(ServerIdentification + "\r\n")); var keyExchangeInitMessage = new KeyExchangeInitMessage { @@ -141,7 +140,6 @@ private void CreateMocks() _serviceFactoryMock = new Mock(MockBehavior.Strict); _socketFactoryMock = new Mock(MockBehavior.Strict); _connectorMock = new Mock(MockBehavior.Strict); - _protocolVersionExchangeMock = new Mock(MockBehavior.Strict); _keyExchangeMock = new Mock(MockBehavior.Strict); _clientAuthenticationMock = new Mock(MockBehavior.Strict); } @@ -152,10 +150,6 @@ private void SetupMocks() .Returns(_connectorMock.Object); _ = _connectorMock.Setup(p => p.Connect(ConnectionInfo)) .Returns(ClientSocket); - _ = _serviceFactoryMock.Setup(p => p.CreateProtocolVersionExchange()) - .Returns(_protocolVersionExchangeMock.Object); - _ = _protocolVersionExchangeMock.Setup(p => p.Start(Session.ClientVersion, ClientSocket, ConnectionInfo.Timeout)) - .Returns(ServerIdentification); _ = _serviceFactoryMock.Setup(p => p.CreateKeyExchange(ConnectionInfo.KeyExchangeAlgorithms, new[] { _keyExchangeAlgorithm })) .Returns(_keyExchangeMock.Object); _ = _keyExchangeMock.Setup(p => p.Name) diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectingBase.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectingBase.cs index d425afdb3..78328ce03 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectingBase.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectingBase.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Sockets; using System.Security.Cryptography; +using System.Text; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -27,12 +28,12 @@ public abstract class SessionTest_ConnectingBase internal Mock SocketFactoryMock { get; private set; } internal Mock ConnectorMock { get; private set; } - private Mock _protocolVersionExchangeMock; private Mock _keyExchangeMock; private Mock _clientAuthenticationMock; private IPEndPoint _serverEndPoint; private string[] _keyExchangeAlgorithms; private bool _authenticationStarted; + private bool _clientIdentificationReceived; private SocketFactory _socketFactory; protected Random Random { get; private set; } @@ -116,6 +117,7 @@ protected virtual void SetupData() ServerBytesReceivedRegister = new List(); ServerIdentification = new SshIdentification("2.0", "OurServerStub"); _authenticationStarted = false; + _clientIdentificationReceived = false; _socketFactory = new SocketFactory(); Session = new Session(ConnectionInfo, ServiceFactoryMock.Object, SocketFactoryMock.Object); @@ -130,6 +132,7 @@ protected virtual void SetupData() ServerListener.Connected += socket => { ServerSocket = socket; + _ = ServerSocket.Send(Encoding.ASCII.GetBytes(ServerIdentification + "\r\n")); ActionBeforeKexInit(); var keyExchangeInitMessage = new KeyExchangeInitMessage { @@ -150,6 +153,24 @@ protected virtual void SetupData() }; ServerListener.BytesReceived += (received, socket) => { + if (!_clientIdentificationReceived) + { + var indexOfLf = Array.IndexOf(received, (byte)'\n'); + if (indexOfLf < 0) + { + return; + } + + _clientIdentificationReceived = true; + + if (indexOfLf == received.Length - 1) + { + return; + } + + received = received.AsSpan(indexOfLf + 1).ToArray(); + } + ServerBytesReceivedRegister.Add(received); if (received.Length > 5 && received[5] == 20) @@ -189,8 +210,6 @@ protected virtual void SetupData() } }; ServerListener.Start(); - - ClientSocket = new DirectConnector(_socketFactory, NullLoggerFactory.Instance).Connect(ConnectionInfo); } private void CreateMocks() @@ -198,7 +217,6 @@ private void CreateMocks() ServiceFactoryMock = new Mock(MockBehavior.Strict); SocketFactoryMock = new Mock(MockBehavior.Strict); ConnectorMock = new Mock(MockBehavior.Strict); - _protocolVersionExchangeMock = new Mock(MockBehavior.Strict); _keyExchangeMock = new Mock(MockBehavior.Strict); _clientAuthenticationMock = new Mock(MockBehavior.Strict); } @@ -208,11 +226,7 @@ private void SetupMocks() _ = ServiceFactoryMock.Setup(p => p.CreateConnector(ConnectionInfo, SocketFactoryMock.Object)) .Returns(ConnectorMock.Object); _ = ConnectorMock.Setup(p => p.Connect(ConnectionInfo)) - .Returns(ClientSocket); - _ = ServiceFactoryMock.Setup(p => p.CreateProtocolVersionExchange()) - .Returns(_protocolVersionExchangeMock.Object); - _ = _protocolVersionExchangeMock.Setup(p => p.Start(Session.ClientVersion, ClientSocket, ConnectionInfo.Timeout)) - .Returns(() => ServerIdentification); + .Returns(() => ClientSocket = new DirectConnector(_socketFactory, NullLoggerFactory.Instance).Connect(ConnectionInfo)); _ = ServiceFactoryMock.Setup(p => p.CreateKeyExchange(ConnectionInfo.KeyExchangeAlgorithms, _keyExchangeAlgorithms)).Returns(_keyExchangeMock.Object); _ = _keyExchangeMock.Setup(p => p.Name) diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_SocketConnected_BadPacketAndDispose.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_SocketConnected_BadPacketAndDispose.cs index b3d75d5c5..18d75c185 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_SocketConnected_BadPacketAndDispose.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_SocketConnected_BadPacketAndDispose.cs @@ -1,6 +1,7 @@ using System; using System.Net; using System.Net.Sockets; +using System.Text; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -20,7 +21,6 @@ public class SessionTest_SocketConnected_BadPacketAndDispose private Mock _serviceFactoryMock; private Mock _socketFactoryMock; private Mock _connectorMock; - private Mock _protocolVersionExchangeMock; private ConnectionInfo _connectionInfo; private Session _session; private AsyncSocketListener _serverListener; @@ -48,7 +48,6 @@ protected void CreateMocks() _serviceFactoryMock = new Mock(MockBehavior.Strict); _socketFactoryMock = new Mock(MockBehavior.Strict); _connectorMock = new Mock(MockBehavior.Strict); - _protocolVersionExchangeMock = new Mock(MockBehavior.Strict); } protected void SetupData() @@ -66,8 +65,7 @@ protected void SetupData() { _serverSocket = socket; - // Since we're mocking the protocol version exchange, we can immediately send the bad - // packet upon establishing the connection + _ = _serverSocket.Send(Encoding.ASCII.GetBytes("SSH-2.0-XXX\r\n")); var badPacket = new byte[] { 0x0a, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05, 0x05 }; @@ -88,10 +86,6 @@ protected void SetupMocks() .Returns(_connectorMock.Object); _ = _connectorMock.Setup(p => p.Connect(_connectionInfo)) .Returns(_clientSocket); - _ = _serviceFactoryMock.Setup(p => p.CreateProtocolVersionExchange()) - .Returns(_protocolVersionExchangeMock.Object); - _ = _protocolVersionExchangeMock.Setup(p => p.Start(_session.ClientVersion, _clientSocket, _connectionInfo.Timeout)) - .Returns(new SshIdentification("2.0", "XXX")); } protected void Arrange() diff --git a/test/Renci.SshNet.Tests/Common/AssertExtensions.cs b/test/Renci.SshNet.Tests/Common/AssertExtensions.cs new file mode 100644 index 000000000..cd303ecae --- /dev/null +++ b/test/Renci.SshNet.Tests/Common/AssertExtensions.cs @@ -0,0 +1,29 @@ +#nullable enable +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; + +namespace Microsoft.VisualStudio.TestTools.UnitTesting +{ + internal static class AssertExtensions + { + extension(Assert) + { + public static void SequenceStartsWith( + T[] expectedPrefix, + IEnumerable? actual, + string? message = "", + [CallerArgumentExpression(nameof(expectedPrefix))] string expectedExpression = "", + [CallerArgumentExpression(nameof(actual))] string actualExpression = "") + { + Assert.IsNotNull(actual, valueExpression: actualExpression); + Assert.AreSequenceEqual( + expectedPrefix, + actual.Take(expectedPrefix.Length), + message, + expectedExpression, + actualExpression); + } + } + } +} diff --git a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index 25ffbd8f9..ae6efb88e 100644 --- a/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/test/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -23,6 +23,9 @@ build; native; contentfiles; analyzers; buildtransitive all + + runtime; build; native; contentfiles; analyzers; buildtransitive +