diff --git a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs
index 2e2c7527e..94cac4d98 100644
--- a/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs
+++ b/src/Renci.SshNet/Channels/ChannelDirectTcpip.cs
@@ -1,4 +1,5 @@
using System;
+using System.Collections.Generic;
using System.Net;
using System.Net.Sockets;
using System.Threading;
@@ -23,6 +24,34 @@ internal sealed class ChannelDirectTcpip : ClientChannel, IChannelDirectTcpip
private IForwardedPort _forwardedPort;
private Socket _socket;
+ ///
+ /// Holds a value indicating whether data received on the channel may be written to
+ /// .
+ ///
+ ///
+ /// until releases the relay; see
+ /// for why it is held back.
+ ///
+ private bool _relaying;
+
+ ///
+ /// Holds the data received on the channel before the relay was released, or
+ /// when there is none.
+ ///
+ private Queue _pendingData;
+
+ ///
+ /// Holds the shutdown that was requested before the relay was released, or
+ /// when none was.
+ ///
+ private SocketShutdown? _pendingShutdown;
+
+ ///
+ /// Holds a value indicating whether must be closed as soon as the relay
+ /// is released.
+ ///
+ private bool _pendingClose;
+
///
/// Initializes a new instance of the class.
///
@@ -47,6 +76,18 @@ public override ChannelTypes ChannelType
get { return ChannelTypes.DirectTcpip; }
}
+ ///
+ /// Opens the channel for the specified remote host and port.
+ ///
+ /// The name of the remote host to forward to.
+ /// The port of the remote host to forward to.
+ /// The forwarded port for which the channel is opened.
+ /// The socket to receive requests from, and send responses from the remote host to.
+ ///
+ /// The channel takes ownership of here, but does not yet write to it:
+ /// the caller may still have its own reply to send first. Data received before
+ /// is buffered rather than written. See .
+ ///
public void Open(string remoteHost, uint port, IForwardedPort forwardedPort, Socket socket)
{
if (IsOpen)
@@ -80,13 +121,20 @@ public void Open(string remoteHost, uint port, IForwardedPort forwardedPort, Soc
///
private void ForwardedPort_Closing(object sender, EventArgs eventArgs)
{
- // signal to the client that we will not send anything anymore; this should also interrupt the
- // blocking receive in Bind if the client sends FIN/ACK in time
- ShutdownSocket(SocketShutdown.Send);
+ // The port is going away, so this is an abort rather than an orderly close: act on the
+ // socket right away even when the relay has not been released, discarding anything that
+ // is still buffered. There is nobody left to relay it for, and the whole point of this
+ // handler is to interrupt a blocking receive.
+ lock (_socketLock)
+ {
+ // signal to the client that we will not send anything anymore; this should also interrupt the
+ // blocking receive in Bind if the client sends FIN/ACK in time
+ ShutdownSocketCore(SocketShutdown.Send);
- // if the FIN/ACK is not sent in time by the remote client, then interrupt the blocking receive
- // by closing the socket
- CloseSocket();
+ // if the FIN/ACK is not sent in time by the remote client, then interrupt the blocking receive
+ // by closing the socket
+ CloseSocketCore();
+ }
}
///
@@ -94,15 +142,25 @@ private void ForwardedPort_Closing(object sender, EventArgs eventArgs)
///
public void Bind()
{
+ // Release the data that arrived while the forwarded port was still writing its own reply
+ // to the client, and only then start pumping in the other direction.
+ StartRelay();
+
// Cannot bind if channel is not open
if (!IsOpen)
{
return;
}
+ var socket = _socket;
+ if (socket is null)
+ {
+ return;
+ }
+
var buffer = new byte[RemotePacketSize];
- SocketAbstraction.ReadContinuous(_socket, buffer, 0, buffer.Length, SendData);
+ SocketAbstraction.ReadContinuous(socket, buffer, 0, buffer.Length, SendData);
// even though the client has disconnected, we still want to properly close the
// channel
@@ -113,55 +171,163 @@ public void Bind()
}
///
- /// Closes the socket, hereby interrupting the blocking receive in .
+ /// Starts writing data received on the channel to the socket, first flushing whatever arrived
+ /// while the relay was held back.
///
- private void CloseSocket()
+ ///
+ ///
+ /// A forwarded port writes its own reply to the client between and
+ /// - for that is the SOCKS reply, which
+ /// RFC 1928 s6 requires to precede any byte from the target. runs on the
+ /// session's message-receive thread, so without this gate the two would write to the same
+ /// socket from two threads with no ordering between them, and a target that speaks first - SSH,
+ /// SMTP, IMAP, POP3, FTP, MySQL, PostgreSQL, Redis - would land its greeting where the reply
+ /// belonged.
+ ///
+ ///
+ /// A shutdown or close requested while the relay was closed is applied here too, after the
+ /// buffered data, so that a target which speaks and then immediately hangs up still gets its
+ /// bytes to the client in the right order.
+ ///
+ ///
+ private void StartRelay()
{
- if (_socket is null)
+ lock (_socketLock)
{
- return;
+ if (_relaying)
+ {
+ return;
+ }
+
+ _relaying = true;
+
+ var pending = _pendingData;
+ _pendingData = null;
+
+ if (pending is not null)
+ {
+ while (pending.Count > 0)
+ {
+ var data = pending.Dequeue();
+ SendToSocket(data, 0, data.Length);
+ }
+ }
+
+ if (_pendingShutdown is SocketShutdown pendingShutdown)
+ {
+ _pendingShutdown = null;
+ ShutdownSocketCore(pendingShutdown);
+ }
+
+ if (_pendingClose)
+ {
+ _pendingClose = false;
+ CloseSocketCore();
+ }
}
+ }
+
+ ///
+ /// Writes data to the socket, if there still is one and it is connected.
+ ///
+ /// An array of containing the data to write.
+ /// The zero-based offset in at which to begin taking data from.
+ /// The number of bytes of to write.
+ ///
+ /// The caller must hold .
+ ///
+ private void SendToSocket(byte[] data, int offset, int count)
+ {
+ var socket = _socket;
+ if (socket.IsConnected())
+ {
+ SocketAbstraction.Send(socket, data, offset, count);
+ }
+ }
+
+ ///
+ /// Closes the socket, hereby interrupting the blocking receive in .
+ ///
+ private void CloseSocket()
+ {
lock (_socketLock)
{
- if (_socket is null)
+ if (!_relaying)
{
+ // Disposing the socket now would strand both the reply the forwarded port is about
+ // to write and anything OnData has buffered. StartRelay applies this afterwards.
+ _pendingClose = true;
return;
}
- // closing a socket actually disposes the socket, so we can safely dereference
- // the field to avoid entering the lock again later
- _socket.Dispose();
- _socket = null;
+ CloseSocketCore();
}
}
+ ///
+ /// Disposes the socket.
+ ///
+ ///
+ /// The caller must hold .
+ ///
+ private void CloseSocketCore()
+ {
+ // closing a socket actually disposes the socket, so we can safely dereference
+ // the field to avoid entering the lock again later
+ _socket?.Dispose();
+ _socket = null;
+
+ // there is no longer anywhere to write buffered data to
+ _pendingData = null;
+ }
+
///
/// Shuts down the socket.
///
/// One of the values that specifies the operation that will no longer be allowed.
private void ShutdownSocket(SocketShutdown how)
{
- if (_socket is null)
- {
- return;
- }
-
lock (_socketLock)
{
- if (!_socket.IsConnected())
+ if (!_relaying)
{
+ // Shutting the send side down now would break the reply the forwarded port is
+ // about to write, and would discard whatever OnData has buffered. StartRelay
+ // applies this after the buffer has been drained.
+ _pendingShutdown = _pendingShutdown is null || _pendingShutdown == how
+ ? how
+ : SocketShutdown.Both;
return;
}
- try
- {
- _socket.Shutdown(how);
- }
- catch (SocketException ex)
- {
- _logger.LogInformation(ex, "Failure shutting down socket");
- }
+ ShutdownSocketCore(how);
+ }
+ }
+
+ ///
+ /// Shuts down the socket, without regard for the relay.
+ ///
+ /// One of the values that specifies the operation that will no longer be allowed.
+ ///
+ /// The caller must hold .
+ ///
+ private void ShutdownSocketCore(SocketShutdown how)
+ {
+ var socket = _socket;
+
+ if (!socket.IsConnected())
+ {
+ return;
+ }
+
+ try
+ {
+ socket.Shutdown(how);
+ }
+ catch (SocketException ex)
+ {
+ _logger.LogInformation(ex, "Failure shutting down socket");
}
}
@@ -198,15 +364,28 @@ protected override void OnData(ArraySegment data)
{
base.OnData(data);
- if (_socket != null)
+ lock (_socketLock)
{
- lock (_socketLock)
+ if (_socket is null)
{
- if (_socket.IsConnected())
- {
- SocketAbstraction.Send(_socket, data.Array, data.Offset, data.Count);
- }
+ return;
+ }
+
+ if (!_relaying)
+ {
+ // We are on the session's message-receive thread and the forwarded port has not
+ // finished writing its own reply to the client yet. Hold on to the data instead of
+ // racing that thread for the socket; StartRelay writes it out in order. The array
+ // belongs to the message being processed, so it has to be copied.
+ var buffered = new byte[data.Count];
+ Buffer.BlockCopy(data.Array, data.Offset, buffered, 0, data.Count);
+
+ _pendingData ??= new Queue();
+ _pendingData.Enqueue(buffered);
+ return;
}
+
+ SendToSocket(data.Array, data.Offset, data.Count);
}
}
@@ -288,17 +467,9 @@ protected override void Dispose(bool disposing)
if (disposing)
{
- if (_socket != null)
+ lock (_socketLock)
{
- lock (_socketLock)
- {
- var socket = _socket;
- if (socket != null)
- {
- _socket = null;
- socket.Dispose();
- }
- }
+ CloseSocketCore();
}
var channelOpen = _channelOpen;
diff --git a/src/Renci.SshNet/ForwardedPortDynamic.cs b/src/Renci.SshNet/ForwardedPortDynamic.cs
index 7210b403a..b4db9cb30 100644
--- a/src/Renci.SshNet/ForwardedPortDynamic.cs
+++ b/src/Renci.SshNet/ForwardedPortDynamic.cs
@@ -504,19 +504,13 @@ private bool HandleSocks4(Socket socket, IChannelDirectTcpip channel, TimeSpan t
channel.Open(host, port, this, socket);
- SocketAbstraction.SendByte(socket, 0x00);
+ var channelOpen = channel.IsOpen;
- if (channel.IsOpen)
- {
- SocketAbstraction.SendByte(socket, 0x5a);
- SocketAbstraction.Send(socket, portBuffer, 0, portBuffer.Length);
- SocketAbstraction.Send(socket, ipBuffer, 0, ipBuffer.Length);
- return true;
- }
+ var socksReply = CreateSocks4Reply(channelOpen, portBuffer, ipBuffer);
+
+ SocketAbstraction.Send(socket, socksReply, 0, socksReply.Length);
- // signal that request was rejected or failed
- SocketAbstraction.SendByte(socket, 0x5b);
- return false;
+ return channelOpen;
}
private bool HandleSocks5(Socket socket, IChannelDirectTcpip channel, TimeSpan timeout)
@@ -671,6 +665,52 @@ private static string GetSocks5Host(int addressType, Socket socket, TimeSpan tim
}
}
+ ///
+ /// Creates the reply to a SOCKS4 request.
+ ///
+ /// if the channel was opened; otherwise, .
+ /// The destination port from the request.
+ /// The destination address from the request.
+ ///
+ /// An array of containing the reply.
+ ///
+ ///
+ /// The reply is eight bytes whether or not the request was granted. The destination port and
+ /// address are defined to be ignored by the client, but they are not optional: a client that
+ /// reads the reply as the fixed-width record it is - which includes the SOCKS4 client in this
+ /// library - would block waiting for the remaining six bytes of a rejection.
+ ///
+ private static byte[] CreateSocks4Reply(bool channelOpen, byte[] portBuffer, byte[] ipBuffer)
+ {
+ var socksReply = new byte[// Reply version; fixed: 0x00
+ 1 +
+
+ // Reply code
+ 1 +
+
+ // Destination port; ignored by the client
+ 2 +
+
+ // Destination IPv4 address; ignored by the client
+ 4];
+
+ socksReply[0] = 0x00;
+
+ if (channelOpen)
+ {
+ socksReply[1] = 0x5a; // request granted
+ }
+ else
+ {
+ socksReply[1] = 0x5b; // request rejected or failed
+ }
+
+ Buffer.BlockCopy(portBuffer, 0, socksReply, 2, portBuffer.Length);
+ Buffer.BlockCopy(ipBuffer, 0, socksReply, 4, ipBuffer.Length);
+
+ return socksReply;
+ }
+
private static byte[] CreateSocks5Reply(bool channelOpen)
{
var socksReply = new byte[// SOCKS version
diff --git a/test/Renci.SshNet.Tests/Classes/Channels/ChannelDirectTcpipTest_ReplyOrdering.cs b/test/Renci.SshNet.Tests/Classes/Channels/ChannelDirectTcpipTest_ReplyOrdering.cs
new file mode 100644
index 000000000..c364d010f
--- /dev/null
+++ b/test/Renci.SshNet.Tests/Classes/Channels/ChannelDirectTcpipTest_ReplyOrdering.cs
@@ -0,0 +1,357 @@
+using System;
+using System.Net;
+using System.Net.Sockets;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Moq;
+
+using Renci.SshNet.Channels;
+using Renci.SshNet.Messages;
+using Renci.SshNet.Messages.Connection;
+using Renci.SshNet.Tests.Common;
+
+namespace Renci.SshNet.Tests.Classes.Channels
+{
+ ///
+ /// A forwarded port owns the client socket between and
+ /// so that it can write its own reply first —
+ /// writes the SOCKS reply there, and RFC 1928 §6 requires that
+ /// reply to precede any byte from the target.
+ ///
+ /// runs on the session's message-receive thread, so if it
+ /// wrote to the socket during that window the two threads would race and a target that speaks
+ /// first — SSH, SMTP, IMAP, POP3, FTP, MySQL, PostgreSQL, Redis — could land its greeting where
+ /// the reply belonged. These tests pin the ordering, and the disposal of the data and of the
+ /// socket itself when the target speaks and then immediately hangs up.
+ ///
+ [TestClass]
+ public class ChannelDirectTcpipTest_ReplyOrdering : TestBase
+ {
+ private const string Reply = "";
+ private const string TargetGreeting = "SSH-2.0-OpenSSH_10.2p1\r\n";
+
+ private Mock _sessionMock;
+ private Mock _forwardedPortMock;
+ private Mock _connectionInfoMock;
+ private uint _localChannelNumber;
+ private uint _remoteChannelNumber;
+ private uint _remoteWindowSize;
+ private uint _remotePacketSize;
+
+ ///
+ /// The socket the channel owns and writes to, i.e. the one the forwarded port accepted.
+ ///
+ private Socket _serverSide;
+
+ ///
+ /// The socket the SOCKS client would be holding.
+ ///
+ private Socket _clientSide;
+
+ private Socket _listener;
+
+ protected override void OnInit()
+ {
+ base.OnInit();
+
+ var random = new Random();
+ _localChannelNumber = (uint)random.Next(0, int.MaxValue);
+ _remoteChannelNumber = (uint)random.Next(0, int.MaxValue);
+ _remoteWindowSize = 0x100000;
+ _remotePacketSize = 0x8000;
+
+ _sessionMock = new Mock(MockBehavior.Strict);
+ _forwardedPortMock = new Mock(MockBehavior.Strict);
+ _connectionInfoMock = new Mock(MockBehavior.Strict);
+
+ _ = _sessionMock.Setup(p => p.SessionLoggerFactory)
+ .Returns(NullLoggerFactory.Instance);
+ _ = _sessionMock.Setup(p => p.IsConnected)
+ .Returns(true);
+ _ = _sessionMock.Setup(p => p.ConnectionInfo)
+ .Returns(_connectionInfoMock.Object);
+ _ = _connectionInfoMock.Setup(p => p.ChannelCloseTimeout)
+ .Returns(TimeSpan.FromSeconds(10));
+ _ = _sessionMock.Setup(p => p.WaitOnHandle(It.IsAny()))
+ .Callback(handle => handle.WaitOne());
+ _ = _sessionMock.Setup(p => p.TryWait(It.IsAny(), It.IsAny()))
+ .Returns(WaitResult.Success);
+ _ = _sessionMock.Setup(p => p.TrySendMessage(It.IsAny()))
+ .Returns(true);
+
+ // Confirm the channel open as soon as it is requested, the way a server would. Any other
+ // message the channel sends (a window adjust, say) is simply accepted.
+ _ = _sessionMock.Setup(p => p.SendMessage(It.IsAny()))
+ .Callback(message =>
+ {
+ if (message is ChannelOpenMessage open)
+ {
+ _sessionMock.Raise(p => p.ChannelOpenConfirmationReceived += null,
+ new MessageEventArgs(
+ new ChannelOpenConfirmationMessage(open.LocalChannelNumber,
+ _remoteWindowSize,
+ _remotePacketSize,
+ _remoteChannelNumber)));
+ }
+ });
+
+ _listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
+ _listener.Bind(new IPEndPoint(IPAddress.Loopback, 0));
+ _listener.Listen(1);
+
+ _clientSide = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp) { NoDelay = true };
+ _clientSide.Connect(_listener.LocalEndPoint);
+ _serverSide = _listener.Accept();
+ _serverSide.NoDelay = true;
+ }
+
+ protected override void OnCleanup()
+ {
+ _clientSide?.Dispose();
+ _serverSide?.Dispose();
+ _listener?.Dispose();
+
+ base.OnCleanup();
+ }
+
+ ///
+ /// The defect itself: data that arrives while the port is still composing its reply must not
+ /// be written to the client ahead of that reply.
+ ///
+ [TestMethod]
+ public void TargetDataArrivingBeforeBindIsWrittenAfterThePortsOwnReply()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ RaiseData(TargetGreeting);
+
+ // Nothing may reach the client yet: the port has not written its reply.
+ Assert.AreEqual(0, BytesWaitingForClient(), "the target's greeting was written to the client before the port's reply");
+
+ // The forwarded port writes its reply, then hands the socket over.
+ _ = _serverSide.Send(Encoding.ASCII.GetBytes(Reply));
+
+ var bind = RunBind(channel);
+
+ Assert.AreEqual(Reply + TargetGreeting, ReadFromClient(Reply.Length + TargetGreeting.Length));
+
+ FinishBind(bind);
+ }
+ }
+
+ ///
+ /// Everything buffered has to come out, once, in the order it arrived.
+ ///
+ [TestMethod]
+ public void EveryChunkBufferedBeforeBindIsDeliveredOnceAndInOrder()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ var expected = new StringBuilder(Reply);
+
+ for (var i = 0; i < 25; i++)
+ {
+ var chunk = "chunk-" + i.ToString("D2") + ";";
+ RaiseData(chunk);
+ _ = expected.Append(chunk);
+ }
+
+ Assert.AreEqual(0, BytesWaitingForClient());
+
+ _ = _serverSide.Send(Encoding.ASCII.GetBytes(Reply));
+
+ var bind = RunBind(channel);
+
+ Assert.AreEqual(expected.ToString(), ReadFromClient(expected.Length));
+
+ FinishBind(bind);
+ }
+ }
+
+ ///
+ /// A target that speaks and hangs up at once — sshd refusing a connection, say — must still
+ /// get its bytes to the client, after the reply, before the socket is shut down.
+ ///
+ [TestMethod]
+ public void EofArrivingBeforeBindDoesNotDiscardBufferedDataOrPreEmptTheReply()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ RaiseData(TargetGreeting);
+ _sessionMock.Raise(p => p.ChannelEofReceived += null,
+ new MessageEventArgs(new ChannelEofMessage(_localChannelNumber)));
+
+ // The reply must still be writable: the send side may not have been shut down yet.
+ _ = _serverSide.Send(Encoding.ASCII.GetBytes(Reply));
+
+ var bind = RunBind(channel);
+
+ Assert.AreEqual(Reply + TargetGreeting, ReadFromClient(Reply.Length + TargetGreeting.Length));
+
+ // and only then the end of the stream
+ Assert.AreEqual(0, _clientSide.Receive(new byte[16], 0, 16, SocketFlags.None));
+
+ FinishBind(bind);
+ }
+ }
+
+ ///
+ /// Same again, but with the whole channel closed before the port got its reply out. The socket
+ /// may not be disposed from under the reply.
+ ///
+ [TestMethod]
+ public void ChannelClosedBeforeBindDoesNotDiscardBufferedDataOrPreEmptTheReply()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ RaiseData(TargetGreeting);
+ _sessionMock.Raise(p => p.ChannelEofReceived += null,
+ new MessageEventArgs(new ChannelEofMessage(_localChannelNumber)));
+ _sessionMock.Raise(p => p.ChannelCloseReceived += null,
+ new MessageEventArgs(new ChannelCloseMessage(_localChannelNumber)));
+
+ Assert.IsFalse(channel.IsOpen, "the channel should have been closed");
+
+ _ = _serverSide.Send(Encoding.ASCII.GetBytes(Reply));
+
+ channel.Bind();
+
+ Assert.AreEqual(Reply + TargetGreeting, ReadFromClient(Reply.Length + TargetGreeting.Length));
+ Assert.AreEqual(0, _clientSide.Receive(new byte[16], 0, 16, SocketFlags.None));
+ }
+ }
+
+ ///
+ /// Once the relay is running, data goes straight to the socket — no buffering, no delay until
+ /// some later event.
+ ///
+ [TestMethod]
+ public void DataArrivingAfterBindIsWrittenStraightToTheClient()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ _ = _serverSide.Send(Encoding.ASCII.GetBytes(Reply));
+
+ var bind = RunBind(channel);
+
+ Assert.AreEqual(Reply, ReadFromClient(Reply.Length));
+
+ RaiseData(TargetGreeting);
+
+ Assert.AreEqual(TargetGreeting, ReadFromClient(TargetGreeting.Length));
+
+ FinishBind(bind);
+ }
+ }
+
+ ///
+ /// The port going away is an abort, not an orderly close: it must still take the socket down
+ /// immediately, which is what unblocks a forwarded port that is stopping.
+ ///
+ [TestMethod]
+ public void ForwardedPortClosingBeforeBindStillTakesTheSocketDown()
+ {
+ using (var channel = CreateOpenChannel())
+ {
+ RaiseData(TargetGreeting);
+
+ _forwardedPortMock.Raise(p => p.Closing += null, EventArgs.Empty);
+
+ // FIN, not the buffered greeting
+ Assert.AreEqual(0, _clientSide.Receive(new byte[64], 0, 64, SocketFlags.None));
+
+ // and Bind must not block on a socket that is gone
+ channel.Bind();
+ }
+ }
+
+ private ChannelDirectTcpip CreateOpenChannel()
+ {
+ var channel = new ChannelDirectTcpip(_sessionMock.Object,
+ _localChannelNumber,
+ localWindowSize: 0x100000,
+ localPacketSize: 0x8000);
+ channel.Open("target.example.com", 22, _forwardedPortMock.Object, _serverSide);
+ Assert.IsTrue(channel.IsOpen);
+ return channel;
+ }
+
+ private void RaiseData(string text)
+ {
+ // The array belongs to the message and is reused by the session, so hand over a distinct
+ // one each time — a channel that kept a reference rather than a copy must not pass.
+ var payload = Encoding.ASCII.GetBytes(text);
+ _sessionMock.Raise(p => p.ChannelDataReceived += null,
+ new MessageEventArgs(new ChannelDataMessage(_localChannelNumber, payload)));
+ Array.Clear(payload, 0, payload.Length);
+ }
+
+ ///
+ /// Bind blocks receiving from the client, so it runs on its own thread.
+ ///
+ private static Task RunBind(ChannelDirectTcpip channel)
+ {
+ var started = new ManualResetEventSlim(false);
+ var task = Task.Factory.StartNew(() =>
+ {
+ started.Set();
+ channel.Bind();
+ }, TaskCreationOptions.LongRunning);
+ _ = started.Wait(TimeSpan.FromSeconds(5));
+ return task;
+ }
+
+ private void FinishBind(Task bind)
+ {
+ _clientSide.Shutdown(SocketShutdown.Send);
+ Assert.IsTrue(bind.Wait(TimeSpan.FromSeconds(10)), "Bind did not return after the client shut down its send side");
+ }
+
+ ///
+ /// Returns how many bytes are readable by the client, after giving a wrongly-ordered write
+ /// long enough to show up. Loopback delivery is immediate, so this does not race the fault it
+ /// is looking for; it only gives it time.
+ ///
+ private int BytesWaitingForClient()
+ {
+ var deadline = DateTime.UtcNow.AddMilliseconds(250);
+ while (DateTime.UtcNow < deadline)
+ {
+ if (_clientSide.Available > 0)
+ {
+ return _clientSide.Available;
+ }
+
+ Thread.Sleep(10);
+ }
+
+ return _clientSide.Available;
+ }
+
+ private string ReadFromClient(int count)
+ {
+ var buffer = new byte[count];
+ var read = 0;
+
+ _clientSide.ReceiveTimeout = 10000;
+
+ while (read < count)
+ {
+ var bytes = _clientSide.Receive(buffer, read, count - read, SocketFlags.None);
+ if (bytes == 0)
+ {
+ break;
+ }
+
+ read += bytes;
+ }
+
+ return Encoding.ASCII.GetString(buffer, 0, read);
+ }
+ }
+}
diff --git a/test/Renci.SshNet.Tests/Classes/ForwardedPortDynamicTest_Started_Socks4Reply.cs b/test/Renci.SshNet.Tests/Classes/ForwardedPortDynamicTest_Started_Socks4Reply.cs
new file mode 100644
index 000000000..17d46b624
--- /dev/null
+++ b/test/Renci.SshNet.Tests/Classes/ForwardedPortDynamicTest_Started_Socks4Reply.cs
@@ -0,0 +1,183 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Net;
+using System.Net.Sockets;
+using System.Text;
+
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Moq;
+
+using Renci.SshNet.Channels;
+using Renci.SshNet.Common;
+using Renci.SshNet.Tests.Common;
+
+namespace Renci.SshNet.Tests.Classes
+{
+ ///
+ /// A SOCKS4 reply is a fixed-width eight byte record - version, code, destination port,
+ /// destination address - whether or not the request was granted. The last six fields are defined
+ /// as ignored by the client, but reading them is not optional: a client that reads the record as
+ /// a record, which SSH.NET's own does on the
+ /// granted path, blocks waiting for bytes that never come if a rejection is short.
+ ///
+ [TestClass]
+ public class ForwardedPortDynamicTest_Started_Socks4Reply
+ {
+ private const string UserName = "socks-user";
+
+ private Mock _sessionMock;
+ private Mock _channelMock;
+ private Mock _connectionInfoMock;
+ private ForwardedPortDynamic _forwardedPort;
+ private Socket _client;
+ private IPEndPoint _remoteEndpoint;
+ private IList _exceptionRegister;
+
+ [TestCleanup]
+ public void Cleanup()
+ {
+ if (_forwardedPort is not null && _forwardedPort.IsStarted)
+ {
+ _forwardedPort.Stop();
+ }
+
+ _client?.Dispose();
+ _client = null;
+ _forwardedPort?.Dispose();
+ _forwardedPort = null;
+ }
+
+ ///
+ /// The case that was short: the channel could not be opened, so the request is rejected.
+ ///
+ [TestMethod]
+ public void RejectedRequestShouldBeAnsweredWithAFullEightByteReply()
+ {
+ Arrange(channelOpen: false);
+
+ var reply = RequestConnect();
+
+ Assert.AreEqual(8, reply.Length, "a SOCKS4 reply is eight bytes, granted or not");
+ Assert.AreEqual(0x00, reply[0], "reply version");
+ Assert.AreEqual(0x5b, reply[1], "request rejected or failed");
+ AssertEchoesTheRequest(reply);
+ }
+
+ ///
+ /// And the granted case still is what it was.
+ ///
+ [TestMethod]
+ public void GrantedRequestShouldBeAnsweredWithAFullEightByteReply()
+ {
+ Arrange(channelOpen: true);
+
+ var reply = RequestConnect();
+
+ Assert.AreEqual(8, reply.Length);
+ Assert.AreEqual(0x00, reply[0], "reply version");
+ Assert.AreEqual(0x5a, reply[1], "request granted");
+ AssertEchoesTheRequest(reply);
+ }
+
+ private void AssertEchoesTheRequest(byte[] reply)
+ {
+ var address = _remoteEndpoint.Address.GetAddressBytes();
+
+ Assert.AreEqual((byte)(_remoteEndpoint.Port >> 8), reply[2], "destination port, high byte");
+ Assert.AreEqual((byte)(_remoteEndpoint.Port & 0xFF), reply[3], "destination port, low byte");
+ CollectionAssert.AreEqual(address, new[] { reply[4], reply[5], reply[6], reply[7] }, "destination address");
+ }
+
+ private void Arrange(bool channelOpen)
+ {
+ _exceptionRegister = new List();
+ _remoteEndpoint = new IPEndPoint(IPAddress.Parse("193.168.1.5"), 8122);
+
+ _sessionMock = new Mock(MockBehavior.Strict);
+ _channelMock = new Mock(MockBehavior.Strict);
+ _connectionInfoMock = new Mock(MockBehavior.Strict);
+
+ _ = _sessionMock.Setup(p => p.SessionLoggerFactory).Returns(NullLoggerFactory.Instance);
+ _ = _sessionMock.Setup(p => p.IsConnected).Returns(true);
+ _ = _sessionMock.Setup(p => p.ConnectionInfo).Returns(_connectionInfoMock.Object);
+ _ = _sessionMock.Setup(p => p.CreateChannelDirectTcpip()).Returns(_channelMock.Object);
+ _ = _connectionInfoMock.Setup(p => p.Timeout).Returns(TimeSpan.FromSeconds(5));
+ _ = _channelMock.Setup(p => p.Open(_remoteEndpoint.Address.ToString(),
+ (uint)_remoteEndpoint.Port,
+ It.IsAny(),
+ It.IsAny()));
+ _ = _channelMock.Setup(p => p.IsOpen).Returns(channelOpen);
+ _ = _channelMock.Setup(p => p.Bind());
+ _ = _channelMock.Setup(p => p.Dispose());
+
+ // Bind to an ephemeral port rather than a fixed one, so that the test does not collide
+ // with anything else that happens to be listening.
+ _forwardedPort = new ForwardedPortDynamic("127.0.0.1", 0);
+ _forwardedPort.Exception += (sender, args) => _exceptionRegister.Add(args);
+ _forwardedPort.Session = _sessionMock.Object;
+ _forwardedPort.Start();
+
+ _client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)
+ {
+ ReceiveTimeout = 10000
+ };
+ _client.Connect(new IPEndPoint(IPAddress.Loopback, (int)_forwardedPort.BoundPort));
+ }
+
+ ///
+ /// Sends a SOCKS4 CONNECT and reads back exactly what the protocol says the reply is, without
+ /// giving up early. A reply that is short shows up here as a receive timeout, which is what a
+ /// real client would see too.
+ ///
+ private byte[] RequestConnect()
+ {
+ var user = Encoding.ASCII.GetBytes(UserName);
+ var address = _remoteEndpoint.Address.GetAddressBytes();
+ var request = new byte[8 + user.Length + 1];
+
+ request[0] = 0x04; // SOCKS version
+ request[1] = 0x01; // CONNECT
+ request[2] = (byte)(_remoteEndpoint.Port >> 8);
+ request[3] = (byte)(_remoteEndpoint.Port & 0xFF);
+ Buffer.BlockCopy(address, 0, request, 4, address.Length);
+ Buffer.BlockCopy(user, 0, request, 8, user.Length);
+ request[request.Length - 1] = 0x00;
+
+ _ = _client.Send(request, 0, request.Length, SocketFlags.None);
+
+ var reply = new byte[8];
+ var received = 0;
+
+ try
+ {
+ while (received < reply.Length)
+ {
+ var bytes = _client.Receive(reply, received, reply.Length - received, SocketFlags.None);
+ if (bytes == 0)
+ {
+ break;
+ }
+
+ received += bytes;
+ }
+ }
+ catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
+ {
+ Assert.Fail(string.Format(CultureInfo.InvariantCulture,
+ "timed out after {0} of {1} reply bytes: {2}",
+ received,
+ reply.Length,
+ BitConverter.ToString(reply, 0, received)));
+ }
+
+ Assert.AreEqual(0, _exceptionRegister.Count, _exceptionRegister.AsString());
+
+ var actual = new byte[received];
+ Buffer.BlockCopy(reply, 0, actual, 0, received);
+ return actual;
+ }
+ }
+}