Skip to content

Commit 16004ef

Browse files
driesengdrieseng
authored
Add support for partial success limit to prevent stack overflow on badly behaving servers
Add support for partial success limit to prevent stack overflow on badly behaving servers
2 parents 69fb3be + c2e1de5 commit 16004ef

File tree

21 files changed

+1746
-147
lines changed

21 files changed

+1746
-147
lines changed

‎src/Renci.SshNet.Tests/Classes/ClientAuthenticationTest.cs

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,64 @@ public class ClientAuthenticationTest
1212
[TestInitialize]
1313
public void Init()
1414
{
15-
_clientAuthentication = new ClientAuthentication();
15+
_clientAuthentication = new ClientAuthentication(1);
1616
}
1717

18+
[TestMethod]
19+
public void Ctor_PartialSuccessLimit_Zero()
20+
{
21+
const int partialSuccessLimit = 0;
22+
23+
try
24+
{
25+
new ClientAuthentication(partialSuccessLimit);
26+
Assert.Fail();
27+
}
28+
catch (ArgumentOutOfRangeException ex)
29+
{
30+
Assert.IsNull(ex.InnerException);
31+
Assert.AreEqual(string.Format("Cannot be less than one.{0}Parameter name: {1}", Environment.NewLine, ex.ParamName), ex.Message);
32+
Assert.AreEqual("partialSuccessLimit", ex.ParamName);
33+
}
34+
}
35+
36+
[TestMethod]
37+
public void Ctor_PartialSuccessLimit_Negative()
38+
{
39+
var partialSuccessLimit = new Random().Next(int.MinValue, -1);
40+
41+
try
42+
{
43+
new ClientAuthentication(partialSuccessLimit);
44+
Assert.Fail();
45+
}
46+
catch (ArgumentOutOfRangeException ex)
47+
{
48+
Assert.IsNull(ex.InnerException);
49+
Assert.AreEqual(string.Format("Cannot be less than one.{0}Parameter name: {1}", Environment.NewLine, ex.ParamName), ex.Message);
50+
Assert.AreEqual("partialSuccessLimit", ex.ParamName);
51+
}
52+
}
53+
54+
[TestMethod]
55+
public void Ctor_PartialSuccessLimit_One()
56+
{
57+
const int partialSuccessLimit = 1;
58+
59+
var clientAuthentication = new ClientAuthentication(partialSuccessLimit);
60+
Assert.AreEqual(partialSuccessLimit, clientAuthentication.PartialSuccessLimit);
61+
}
62+
63+
[TestMethod]
64+
public void Ctor_PartialSuccessLimit_MaxValue()
65+
{
66+
const int partialSuccessLimit = int.MaxValue;
67+
68+
var clientAuthentication = new ClientAuthentication(partialSuccessLimit);
69+
Assert.AreEqual(partialSuccessLimit, clientAuthentication.PartialSuccessLimit);
70+
}
71+
72+
1873
[TestMethod]
1974
public void AuthenticateShouldThrowArgumentNullExceptionWhenConnectionInfoIsNull()
2075
{

‎src/Renci.SshNet.Tests/Classes/ClientAuthenticationTestBase.cs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ public abstract class ClientAuthenticationTestBase : TestBase
1313
internal Mock<IAuthenticationMethod> PasswordAuthenticationMethodMock { get; private set; }
1414
internal Mock<IAuthenticationMethod> PublicKeyAuthenticationMethodMock { get; private set; }
1515
internal Mock<IAuthenticationMethod> KeyboardInteractiveAuthenticationMethodMock { get; private set; }
16-
internal ClientAuthentication ClientAuthentication { get; private set; }
16+
17+
protected abstract void SetupData();
1718

1819
protected void CreateMocks()
1920
{
@@ -27,18 +28,20 @@ protected void CreateMocks()
2728

2829
protected abstract void SetupMocks();
2930

31+
protected virtual void Arrange()
32+
{
33+
SetupData();
34+
CreateMocks();
35+
SetupMocks();
36+
}
37+
3038
protected abstract void Act();
3139

32-
protected override void OnInit()
40+
protected sealed override void OnInit()
3341
{
3442
base.OnInit();
3543

36-
// Arrange
37-
CreateMocks();
38-
SetupMocks();
39-
ClientAuthentication = new ClientAuthentication();
40-
41-
// Act
44+
Arrange();
4245
Act();
4346
}
4447

‎src/Renci.SshNet.Tests/Classes/ClientAuthenticationTest_Failure_MultiList_AllAllowedAuthenticationsHaveReachedPartialSuccessLimit.cs

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
using System.Collections.Generic;
2+
using Microsoft.VisualStudio.TestTools.UnitTesting;
3+
using Moq;
4+
using Renci.SshNet.Common;
5+
6+
namespace Renci.SshNet.Tests.Classes
7+
{
8+
/// <summary>
9+
/// * ConnectionInfo provides the following authentication methods (in order):
10+
/// o publickey
11+
/// o password
12+
/// * Partial success limit is 2
13+
///
14+
/// none
15+
/// (1=FAIL)
16+
/// |
17+
/// +------------------------+------------------------+
18+
/// | | |
19+
/// password ◄--\ publickey keyboard-interactive
20+
/// (7=SKIP) | (2=PS)
21+
/// | |
22+
/// | password
23+
/// | (3=PS)
24+
/// | |
25+
/// | password
26+
/// | (4=PS)
27+
/// | |
28+
/// | publickey
29+
/// | (5=PS)
30+
/// | |
31+
/// \---- publickey
32+
/// (6=SKIP)
33+
/// </summary>
34+
[TestClass]
35+
public class ClientAuthenticationTest_Failure_MultiList_AllAllowedAuthenticationsHaveReachedPartialSuccessLimit : ClientAuthenticationTestBase
36+
{
37+
private int _partialSuccessLimit;
38+
private ClientAuthentication _clientAuthentication;
39+
private SshAuthenticationException _actualException;
40+
41+
protected override void SetupData()
42+
{
43+
_partialSuccessLimit = 2;
44+
}
45+
46+
protected override void SetupMocks()
47+
{
48+
var seq = new MockSequence();
49+
50+
SessionMock.InSequence(seq).Setup(p => p.RegisterMessage("SSH_MSG_USERAUTH_FAILURE"));
51+
SessionMock.InSequence(seq).Setup(p => p.RegisterMessage("SSH_MSG_USERAUTH_SUCCESS"));
52+
SessionMock.InSequence(seq).Setup(p => p.RegisterMessage("SSH_MSG_USERAUTH_BANNER"));
53+
54+
ConnectionInfoMock.InSequence(seq).Setup(p => p.CreateNoneAuthenticationMethod())
55+
.Returns(NoneAuthenticationMethodMock.Object);
56+
57+
/* 1 */
58+
59+
NoneAuthenticationMethodMock.InSequence(seq).Setup(p => p.Authenticate(SessionMock.Object))
60+
.Returns(AuthenticationResult.Failure);
61+
ConnectionInfoMock.InSequence(seq)
62+
.Setup(p => p.AuthenticationMethods)
63+
.Returns(new List<IAuthenticationMethod>
64+
{
65+
PublicKeyAuthenticationMethodMock.Object,
66+
PasswordAuthenticationMethodMock.Object
67+
});
68+
NoneAuthenticationMethodMock.InSequence(seq)
69+
.Setup(p => p.AllowedAuthentications)
70+
.Returns(new[] {"password", "publickey", "keyboard-interactive"});
71+
72+
/* Enumerate supported authentication methods */
73+
74+
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
75+
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
76+
77+
/* 2 */
78+
79+
PublicKeyAuthenticationMethodMock.InSequence(seq)
80+
.Setup(p => p.Authenticate(SessionMock.Object))
81+
.Returns(AuthenticationResult.PartialSuccess);
82+
PublicKeyAuthenticationMethodMock.InSequence(seq)
83+
.Setup(p => p.AllowedAuthentications)
84+
.Returns(new[] {"password"});
85+
86+
/* Enumerate supported authentication methods */
87+
88+
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
89+
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
90+
91+
/* 3 */
92+
93+
PasswordAuthenticationMethodMock.InSequence(seq)
94+
.Setup(p => p.Authenticate(SessionMock.Object))
95+
.Returns(AuthenticationResult.PartialSuccess);
96+
PasswordAuthenticationMethodMock.InSequence(seq)
97+
.Setup(p => p.AllowedAuthentications)
98+
.Returns(new[] {"password"});
99+
100+
/* Enumerate supported authentication methods */
101+
102+
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
103+
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
104+
105+
/* 4 */
106+
107+
PasswordAuthenticationMethodMock.InSequence(seq)
108+
.Setup(p => p.Authenticate(SessionMock.Object))
109+
.Returns(AuthenticationResult.PartialSuccess);
110+
PasswordAuthenticationMethodMock.InSequence(seq)
111+
.Setup(p => p.AllowedAuthentications)
112+
.Returns(new[] {"publickey"});
113+
114+
/* Enumerate supported authentication methods */
115+
116+
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
117+
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
118+
119+
/* 5 */
120+
121+
PublicKeyAuthenticationMethodMock.InSequence(seq)
122+
.Setup(p => p.Authenticate(SessionMock.Object))
123+
.Returns(AuthenticationResult.PartialSuccess);
124+
PublicKeyAuthenticationMethodMock.InSequence(seq)
125+
.Setup(p => p.AllowedAuthentications)
126+
.Returns(new[] {"publickey"});
127+
128+
/* Enumerate supported authentication methods */
129+
130+
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
131+
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
132+
133+
/* 6: Record partial success limit reached exception, and skip password authentication method */
134+
135+
PublicKeyAuthenticationMethodMock.InSequence(seq)
136+
.Setup(p => p.Name)
137+
.Returns("publickey-partial1");
138+
139+
/* 7: Record partial success limit reached exception, and skip password authentication method */
140+
141+
PasswordAuthenticationMethodMock.InSequence(seq)
142+
.Setup(p => p.Name)
143+
.Returns("password-partial1");
144+
145+
SessionMock.InSequence(seq).Setup(p => p.UnRegisterMessage("SSH_MSG_USERAUTH_FAILURE"));
146+
SessionMock.InSequence(seq).Setup(p => p.UnRegisterMessage("SSH_MSG_USERAUTH_SUCCESS"));
147+
SessionMock.InSequence(seq).Setup(p => p.UnRegisterMessage("SSH_MSG_USERAUTH_BANNER"));
148+
}
149+
150+
protected override void Arrange()
151+
{
152+
base.Arrange();
153+
154+
_clientAuthentication = new ClientAuthentication(_partialSuccessLimit);
155+
}
156+
157+
protected override void Act()
158+
{
159+
try
160+
{
161+
_clientAuthentication.Authenticate(ConnectionInfoMock.Object, SessionMock.Object);
162+
Assert.Fail();
163+
}
164+
catch (SshAuthenticationException ex)
165+
{
166+
_actualException = ex;
167+
}
168+
}
169+
170+
[TestMethod]
171+
public void AuthenticateOnPasswordAuthenticationMethodShouldHaveBeenInvokedTwice()
172+
{
173+
PasswordAuthenticationMethodMock.Verify(p => p.Authenticate(SessionMock.Object), Times.Exactly(2));
174+
}
175+
176+
[TestMethod]
177+
public void AuthenticateOnPublicKeyAuthenticationMethodShouldHaveBeenInvokedTwice()
178+
{
179+
PublicKeyAuthenticationMethodMock.Verify(p => p.Authenticate(SessionMock.Object), Times.Exactly(2));
180+
}
181+
182+
[TestMethod]
183+
public void AuthenticateShouldThrowSshAuthenticationException()
184+
{
185+
Assert.IsNotNull(_actualException);
186+
Assert.IsNull(_actualException.InnerException);
187+
Assert.AreEqual("Reached authentication attempt limit for method (password-partial1).", _actualException.Message);
188+
}
189+
}
190+
}

‎src/Renci.SshNet.Tests/Classes/ClientAuthenticationTest_Failure_SingleList_AuthenticationMethodFailed.cs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,15 @@ namespace Renci.SshNet.Tests.Classes
88
[TestClass]
99
public class ClientAuthenticationTest_Failure_SingleList_AuthenticationMethodFailed : ClientAuthenticationTestBase
1010
{
11+
private int _partialSuccessLimit;
12+
private ClientAuthentication _clientAuthentication;
1113
private SshAuthenticationException _actualException;
1214

15+
protected override void SetupData()
16+
{
17+
_partialSuccessLimit = 1;
18+
}
19+
1320
protected override void SetupMocks()
1421
{
1522
var seq = new MockSequence();
@@ -21,23 +28,26 @@ protected override void SetupMocks()
2128
ConnectionInfoMock.InSequence(seq).Setup(p => p.CreateNoneAuthenticationMethod())
2229
.Returns(NoneAuthenticationMethodMock.Object);
2330

24-
NoneAuthenticationMethodMock.InSequence(seq).Setup(p => p.Authenticate(SessionMock.Object))
25-
.Returns(AuthenticationResult.Failure);
26-
ConnectionInfoMock.InSequence(seq).Setup(p => p.AuthenticationMethods)
27-
.Returns(new List<IAuthenticationMethod>
28-
{
29-
PublicKeyAuthenticationMethodMock.Object,
30-
PasswordAuthenticationMethodMock.Object
31-
});
3231
NoneAuthenticationMethodMock.InSequence(seq)
33-
.Setup(p => p.AllowedAuthentications)
34-
.Returns(new[] { "password" });
32+
.Setup(p => p.Authenticate(SessionMock.Object))
33+
.Returns(AuthenticationResult.Failure);
34+
ConnectionInfoMock.InSequence(seq)
35+
.Setup(p => p.AuthenticationMethods)
36+
.Returns(new List<IAuthenticationMethod>
37+
{
38+
PublicKeyAuthenticationMethodMock.Object,
39+
PasswordAuthenticationMethodMock.Object
40+
});
41+
NoneAuthenticationMethodMock.InSequence(seq)
42+
.Setup(p => p.AllowedAuthentications)
43+
.Returns(new[] { "password" });
3544

3645
PublicKeyAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("publickey");
3746
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
3847

39-
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Authenticate(SessionMock.Object))
40-
.Returns(AuthenticationResult.Failure);
48+
PasswordAuthenticationMethodMock.InSequence(seq)
49+
.Setup(p => p.Authenticate(SessionMock.Object))
50+
.Returns(AuthenticationResult.Failure);
4151
// obtain name for inclusion in SshAuthenticationException
4252
PasswordAuthenticationMethodMock.InSequence(seq).Setup(p => p.Name).Returns("password");
4353

@@ -46,11 +56,18 @@ protected override void SetupMocks()
4656
SessionMock.InSequence(seq).Setup(p => p.UnRegisterMessage("SSH_MSG_USERAUTH_BANNER"));
4757
}
4858

59+
protected override void Arrange()
60+
{
61+
base.Arrange();
62+
63+
_clientAuthentication = new ClientAuthentication(_partialSuccessLimit);
64+
}
65+
4966
protected override void Act()
5067
{
5168
try
5269
{
53-
ClientAuthentication.Authenticate(ConnectionInfoMock.Object, SessionMock.Object);
70+
_clientAuthentication.Authenticate(ConnectionInfoMock.Object, SessionMock.Object);
5471
Assert.Fail();
5572
}
5673
catch (SshAuthenticationException ex)

0 commit comments

Comments
 (0)