Skip to content

Commit f50b9f1

Browse files
committed
Do not allow simultaneously sending of a packet and disposing of the session.
Fixes variant of issue #32. Impact on performance still needs to be checked, so we may revert this later.
1 parent 009fbc3 commit f50b9f1

File tree

1 file changed

+49
-25
lines changed

1 file changed

+49
-25
lines changed

src/Renci.SshNet/Session.cs

+49-25
Original file line numberDiff line numberDiff line change
@@ -300,20 +300,20 @@ public Message ClientInitMessage
300300
if (_clientInitMessage == null)
301301
{
302302
_clientInitMessage = new KeyExchangeInitMessage
303-
{
304-
KeyExchangeAlgorithms = ConnectionInfo.KeyExchangeAlgorithms.Keys.ToArray(),
305-
ServerHostKeyAlgorithms = ConnectionInfo.HostKeyAlgorithms.Keys.ToArray(),
306-
EncryptionAlgorithmsClientToServer = ConnectionInfo.Encryptions.Keys.ToArray(),
307-
EncryptionAlgorithmsServerToClient = ConnectionInfo.Encryptions.Keys.ToArray(),
308-
MacAlgorithmsClientToServer = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
309-
MacAlgorithmsServerToClient = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
310-
CompressionAlgorithmsClientToServer = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
311-
CompressionAlgorithmsServerToClient = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
312-
LanguagesClientToServer = new[] {string.Empty},
313-
LanguagesServerToClient = new[] {string.Empty},
314-
FirstKexPacketFollows = false,
315-
Reserved = 0
316-
};
303+
{
304+
KeyExchangeAlgorithms = ConnectionInfo.KeyExchangeAlgorithms.Keys.ToArray(),
305+
ServerHostKeyAlgorithms = ConnectionInfo.HostKeyAlgorithms.Keys.ToArray(),
306+
EncryptionAlgorithmsClientToServer = ConnectionInfo.Encryptions.Keys.ToArray(),
307+
EncryptionAlgorithmsServerToClient = ConnectionInfo.Encryptions.Keys.ToArray(),
308+
MacAlgorithmsClientToServer = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
309+
MacAlgorithmsServerToClient = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
310+
CompressionAlgorithmsClientToServer = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
311+
CompressionAlgorithmsServerToClient = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
312+
LanguagesClientToServer = new[] {string.Empty},
313+
LanguagesServerToClient = new[] {string.Empty},
314+
FirstKexPacketFollows = false,
315+
Reserved = 0
316+
};
317317
}
318318
return _clientInitMessage;
319319
}
@@ -798,11 +798,11 @@ internal void WaitOnHandle(WaitHandle waitHandle, TimeSpan timeout)
798798
throw new ArgumentNullException("waitHandle");
799799

800800
var waitHandles = new[]
801-
{
802-
_exceptionWaitHandle,
803-
_messageListenerCompleted,
804-
waitHandle
805-
};
801+
{
802+
_exceptionWaitHandle,
803+
_messageListenerCompleted,
804+
waitHandle
805+
};
806806

807807
switch (WaitHandle.WaitAny(waitHandles, timeout))
808808
{
@@ -852,9 +852,6 @@ internal void SendMessage(Message message)
852852
// atomically, and only after the packet has actually been sent
853853
lock (_socketWriteLock)
854854
{
855-
if (!_socket.IsConnected())
856-
throw new SshConnectionException("Client not connected.");
857-
858855
byte[] hash = null;
859856
var packetDataOffset = 4; // first four bytes are reserved for outbound packet sequence
860857

@@ -881,15 +878,14 @@ internal void SendMessage(Message message)
881878
var packetLength = packetData.Length - packetDataOffset;
882879
if (hash == null)
883880
{
884-
SocketAbstraction.Send(_socket, packetData, packetDataOffset, packetLength);
881+
SendPacket(packetData, packetDataOffset, packetLength);
885882
}
886883
else
887884
{
888885
var data = new byte[packetLength + (_clientMac.HashSize/8)];
889886
Buffer.BlockCopy(packetData, packetDataOffset, data, 0, packetLength);
890887
Buffer.BlockCopy(hash, 0, data, packetLength, hash.Length);
891-
892-
SocketAbstraction.Send(_socket, data, 0, data.Length);
888+
SendPacket(data, 0, data.Length);
893889
}
894890

895891
// increment the packet sequence number only after we're sure the packet has
@@ -902,6 +898,34 @@ internal void SendMessage(Message message)
902898
}
903899
}
904900

901+
/// <summary>
902+
/// Sends an SSH packet to the server.
903+
/// </summary>
904+
/// <param name="packet">A byte array containing the packet to send.</param>
905+
/// <param name="offset">The offset of the packet.</param>
906+
/// <param name="length">The length of the packet.</param>
907+
/// <exception cref="SshConnectionException">Client is not connected to the server.</exception>
908+
/// <remarks>
909+
/// <para>
910+
/// The send is performed in a dispose lock to avoid <see cref="NullReferenceException"/>
911+
/// and/or <see cref="ObjectDisposedException"/> when sending the packet.
912+
/// </para>
913+
/// <para>
914+
/// This method is only to be used when the connection is established, as the locking
915+
/// overhead is not required while establising the connection.
916+
/// </para>
917+
/// </remarks>
918+
private void SendPacket(byte[] packet, int offset, int length)
919+
{
920+
lock (_socketDisposeLock)
921+
{
922+
if (!_socket.IsConnected())
923+
throw new SshConnectionException("Client not connected.");
924+
925+
SocketAbstraction.Send(_socket, packet, offset, length);
926+
}
927+
}
928+
905929
/// <summary>
906930
/// Sends a message to the server.
907931
/// </summary>

0 commit comments

Comments
 (0)