Skip to content

Commit 70a0a08

Browse files
mus65Rob-Hague
andauthored
Handle unknown channel messages correctly (#1363)
* Handle unknown channel messages correctly See discussion #1218 . Some servers send custom channel messages like '[email protected]' as keep alive messages. This currently causes a NotSupportedException. According to the spec https://datatracker.ietf.org/doc/html/rfc4254#section-5.4 : "If the request is not recognized or is not supported for the channel, SSH_MSG_CHANNEL_FAILURE is returned." Send a failure message back instead of throwing an exception. * consider WantReply before sending failure reply * Use RemoteChannelNumber for failure message * fix wrong ChannelNumber in SshCommand Channel Response not directly related to the PR, was noticed during Code Review. --------- Co-authored-by: Rob Hague <[email protected]>
1 parent 8bd08ed commit 70a0a08

File tree

6 files changed

+131
-5
lines changed

6 files changed

+131
-5
lines changed

src/Renci.SshNet/Channels/Channel.cs

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Globalization;
32
using System.Net.Sockets;
43
using System.Threading;
54

@@ -715,8 +714,14 @@ private void OnChannelRequest(object sender, MessageEventArgs<ChannelRequestMess
715714
}
716715
else
717716
{
718-
// TODO: we should also send a SSH_MSG_CHANNEL_FAILURE message
719-
throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, "Request '{0}' is not supported.", e.Message.RequestName));
717+
var unknownRequestInfo = new UnknownRequestInfo(e.Message.RequestName);
718+
unknownRequestInfo.Load(e.Message.RequestData);
719+
720+
if (unknownRequestInfo.WantReply)
721+
{
722+
var reply = new ChannelFailureMessage(RemoteChannelNumber);
723+
SendMessage(reply);
724+
}
720725
}
721726
}
722727
catch (Exception ex)

src/Renci.SshNet/Channels/IChannel.cs

+5
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ internal interface IChannel : IDisposable
5959
/// </remarks>
6060
uint LocalPacketSize { get; }
6161

62+
/// <summary>
63+
/// Gets the remote channel number.
64+
/// </summary>
65+
uint RemoteChannelNumber { get; }
66+
6267
/// <summary>
6368
/// Gets the maximum size of a data packet that can be sent using the channel.
6469
/// </summary>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
namespace Renci.SshNet.Messages.Connection
2+
{
3+
/// <summary>
4+
/// Represents an unknown request information that we can't handle.
5+
/// </summary>
6+
internal sealed class UnknownRequestInfo : RequestInfo
7+
{
8+
/// <summary>
9+
/// Gets the name of the request.
10+
/// </summary>
11+
public override string RequestName { get; }
12+
13+
/// <summary>
14+
/// Initializes a new instance of the <see cref="UnknownRequestInfo"/> class.
15+
/// <paramref name="requestName">The name of the unknown request.</paramref>
16+
/// </summary>
17+
internal UnknownRequestInfo(string requestName)
18+
{
19+
RequestName = requestName;
20+
}
21+
}
22+
}

src/Renci.SshNet/SshCommand.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -464,15 +464,15 @@ private void Channel_RequestReceived(object sender, ChannelRequestEventArgs e)
464464

465465
if (exitStatusInfo.WantReply)
466466
{
467-
var replyMessage = new ChannelSuccessMessage(_channel.LocalChannelNumber);
467+
var replyMessage = new ChannelSuccessMessage(_channel.RemoteChannelNumber);
468468
_session.SendMessage(replyMessage);
469469
}
470470
}
471471
else
472472
{
473473
if (e.Info.WantReply)
474474
{
475-
var replyMessage = new ChannelFailureMessage(_channel.LocalChannelNumber);
475+
var replyMessage = new ChannelFailureMessage(_channel.RemoteChannelNumber);
476476
_session.SendMessage(replyMessage);
477477
}
478478
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
using System;
2+
using System.Collections.Generic;
3+
4+
using Microsoft.VisualStudio.TestTools.UnitTesting;
5+
6+
using Moq;
7+
8+
using Renci.SshNet.Common;
9+
using Renci.SshNet.Messages;
10+
using Renci.SshNet.Messages.Connection;
11+
12+
namespace Renci.SshNet.Tests.Classes.Channels
13+
{
14+
[TestClass]
15+
public class ChannelTest_OnSessionChannelRequestReceived_HandleUnknownMessage : ChannelTestBase
16+
{
17+
private uint _localWindowSize;
18+
private uint _localPacketSize;
19+
private uint _localChannelNumber;
20+
private uint _remoteChannelNumber;
21+
private uint _remoteWindowSize;
22+
private uint _remotePacketSize;
23+
private ChannelStub _channel;
24+
private IList<ExceptionEventArgs> _channelExceptionRegister;
25+
private UnknownRequestInfoWithWantReply _requestInfo;
26+
27+
protected override void SetupData()
28+
{
29+
var random = new Random();
30+
31+
_localWindowSize = (uint) random.Next(1000, int.MaxValue);
32+
_localPacketSize = _localWindowSize - 1;
33+
_localChannelNumber = (uint) random.Next(0, int.MaxValue);
34+
_remoteChannelNumber = (uint) random.Next(0, int.MaxValue);
35+
_remoteWindowSize = (uint) random.Next(0, int.MaxValue);
36+
_remotePacketSize = (uint) random.Next(0, int.MaxValue);
37+
_channelExceptionRegister = new List<ExceptionEventArgs>();
38+
_requestInfo = new UnknownRequestInfoWithWantReply();
39+
}
40+
41+
protected override void SetupMocks()
42+
{
43+
_ = SessionMock.Setup(p => p.ConnectionInfo)
44+
.Returns(new ConnectionInfo("host", "user", new PasswordAuthenticationMethod("user", "password")));
45+
_ = SessionMock.Setup(p => p.SendMessage(It.IsAny<Message>()));
46+
}
47+
48+
protected override void Arrange()
49+
{
50+
base.Arrange();
51+
52+
_channel = new ChannelStub(SessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize);
53+
_channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize);
54+
_channel.SetIsOpen(true);
55+
_channel.Exception += (sender, args) => _channelExceptionRegister.Add(args);
56+
}
57+
58+
protected override void Act()
59+
{
60+
SessionMock.Raise(s => s.ChannelRequestReceived += null,
61+
new MessageEventArgs<ChannelRequestMessage>(new ChannelRequestMessage(_localChannelNumber, _requestInfo)));
62+
}
63+
64+
[TestMethod]
65+
public void FailureMessageWasSent()
66+
{
67+
SessionMock.Verify(p => p.SendMessage(It.Is<ChannelFailureMessage>(m => m.LocalChannelNumber == _channel.RemoteChannelNumber)), Times.Once);
68+
}
69+
70+
[TestMethod]
71+
public void NoExceptionShouldHaveFired()
72+
{
73+
Assert.AreEqual(0, _channelExceptionRegister.Count);
74+
}
75+
}
76+
77+
internal class UnknownRequestInfoWithWantReply : RequestInfo
78+
{
79+
public override string RequestName
80+
{
81+
get
82+
{
83+
return nameof(UnknownRequestInfoWithWantReply);
84+
}
85+
}
86+
87+
internal UnknownRequestInfoWithWantReply()
88+
{
89+
WantReply = true;
90+
}
91+
}
92+
}

test/Renci.SshNet.Tests/Classes/ShellStreamTest_ReadExpect.cs

+2
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,8 @@ public void Open()
370370

371371
public uint LocalPacketSize => throw new NotImplementedException();
372372

373+
public uint RemoteChannelNumber => throw new NotImplementedException();
374+
373375
public uint RemotePacketSize => throw new NotImplementedException();
374376

375377
public bool IsOpen => throw new NotImplementedException();

0 commit comments

Comments
 (0)