Skip to content

Commit c5d51b9

Browse files
driesengdrieseng
committed
Harden against "leaking" message registration.
Fixes issue #55.
1 parent 7e2836a commit c5d51b9

4 files changed

+67
-53
lines changed

‎src/Renci.SshNet/KeyboardInteractiveAuthenticationMethod.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,16 @@ public override AuthenticationResult Authenticate(Session session)
5757
session.UserAuthenticationFailureReceived += Session_UserAuthenticationFailureReceived;
5858
session.MessageReceived += Session_MessageReceived;
5959

60+
session.RegisterMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
61+
6062
try
6163
{
62-
session.RegisterMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
6364
session.SendMessage(_requestMessage);
6465
session.WaitOnHandle(_authenticationCompleted);
65-
session.UnRegisterMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
6666
}
6767
finally
6868
{
69+
session.UnRegisterMessage("SSH_MSG_USERAUTH_INFO_REQUEST");
6970
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
7071
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
7172
session.MessageReceived -= Session_MessageReceived;

‎src/Renci.SshNet/PasswordAuthenticationMethod.cs

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public override AuthenticationResult Authenticate(Session session)
8989
}
9090
finally
9191
{
92+
session.UnRegisterMessage("SSH_MSG_USERAUTH_PASSWD_CHANGEREQ");
9293
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
9394
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
9495
session.MessageReceived -= Session_MessageReceived;

‎src/Renci.SshNet/PrivateKeyAuthenticationMethod.cs

+45-36
Original file line numberDiff line numberDiff line change
@@ -60,55 +60,65 @@ public override AuthenticationResult Authenticate(Session session)
6060

6161
session.RegisterMessage("SSH_MSG_USERAUTH_PK_OK");
6262

63-
foreach (var keyFile in KeyFiles)
63+
try
6464
{
65-
_authenticationCompleted.Reset();
66-
_isSignatureRequired = false;
65+
foreach (var keyFile in KeyFiles)
66+
{
67+
_authenticationCompleted.Reset();
68+
_isSignatureRequired = false;
6769

68-
var message = new RequestMessagePublicKey(ServiceName.Connection, Username, keyFile.HostKey.Name, keyFile.HostKey.Data);
70+
var message = new RequestMessagePublicKey(ServiceName.Connection,
71+
Username,
72+
keyFile.HostKey.Name,
73+
keyFile.HostKey.Data);
6974

70-
if (KeyFiles.Count < 2)
71-
{
72-
// If only one key file provided then send signature for very first request
73-
var signatureData = new SignatureData(message, session.SessionId).GetBytes();
75+
if (KeyFiles.Count < 2)
76+
{
77+
// If only one key file provided then send signature for very first request
78+
var signatureData = new SignatureData(message, session.SessionId).GetBytes();
7479

75-
message.Signature = keyFile.HostKey.Sign(signatureData);
76-
}
80+
message.Signature = keyFile.HostKey.Sign(signatureData);
81+
}
7782

78-
// Send public key authentication request
79-
session.SendMessage(message);
83+
// Send public key authentication request
84+
session.SendMessage(message);
8085

81-
session.WaitOnHandle(_authenticationCompleted);
86+
session.WaitOnHandle(_authenticationCompleted);
8287

83-
if (_isSignatureRequired)
84-
{
85-
_authenticationCompleted.Reset();
88+
if (_isSignatureRequired)
89+
{
90+
_authenticationCompleted.Reset();
8691

87-
var signatureMessage = new RequestMessagePublicKey(ServiceName.Connection, Username, keyFile.HostKey.Name, keyFile.HostKey.Data);
92+
var signatureMessage = new RequestMessagePublicKey(ServiceName.Connection,
93+
Username,
94+
keyFile.HostKey.Name,
95+
keyFile.HostKey.Data);
8896

89-
var signatureData = new SignatureData(message, session.SessionId).GetBytes();
97+
var signatureData = new SignatureData(message, session.SessionId).GetBytes();
9098

91-
signatureMessage.Signature = keyFile.HostKey.Sign(signatureData);
99+
signatureMessage.Signature = keyFile.HostKey.Sign(signatureData);
92100

93-
// Send public key authentication request with signature
94-
session.SendMessage(signatureMessage);
95-
}
101+
// Send public key authentication request with signature
102+
session.SendMessage(signatureMessage);
103+
}
96104

97-
session.WaitOnHandle(_authenticationCompleted);
105+
session.WaitOnHandle(_authenticationCompleted);
98106

99-
if (_authenticationResult == AuthenticationResult.Success)
100-
{
101-
break;
107+
if (_authenticationResult == AuthenticationResult.Success)
108+
{
109+
break;
110+
}
102111
}
103-
}
104-
105-
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
106-
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
107-
session.MessageReceived -= Session_MessageReceived;
108-
109-
session.UnRegisterMessage("SSH_MSG_USERAUTH_PK_OK");
110112

111-
return _authenticationResult;
113+
return _authenticationResult;
114+
}
115+
finally
116+
{
117+
session.UserAuthenticationSuccessReceived -= Session_UserAuthenticationSuccessReceived;
118+
session.UserAuthenticationFailureReceived -= Session_UserAuthenticationFailureReceived;
119+
session.MessageReceived -= Session_MessageReceived;
120+
session.UnRegisterMessage("SSH_MSG_USERAUTH_PK_OK");
121+
}
112122
}
113123

114124
private void Session_UserAuthenticationSuccessReceived(object sender, MessageEventArgs<SuccessMessage> e)
@@ -168,8 +178,8 @@ protected virtual void Dispose(bool disposing)
168178
var authenticationCompleted = _authenticationCompleted;
169179
if (authenticationCompleted != null)
170180
{
171-
authenticationCompleted.Dispose();
172181
_authenticationCompleted = null;
182+
authenticationCompleted.Dispose();
173183
}
174184

175185
_isDisposed = true;
@@ -243,6 +253,5 @@ protected override void SaveData()
243253
WriteBinaryString(_message.PublicKeyData);
244254
}
245255
}
246-
247256
}
248257
}

‎src/Renci.SshNet/Security/KeyExchangeDiffieHellmanGroupExchangeShaBase.cs

+18-15
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ public override void Start(Session session, KeyExchangeInitMessage message)
5050
base.Start(session, message);
5151

5252
Session.RegisterMessage("SSH_MSG_KEX_DH_GEX_GROUP");
53-
Session.RegisterMessage("SSH_MSG_KEX_DH_GEX_REPLY");
5453

5554
Session.MessageReceived += Session_MessageReceived;
5655

57-
// 1. send SSH_MSG_KEY_DH_GEX_REQUEST
56+
// 1. client sends SSH_MSG_KEY_DH_GEX_REQUEST
5857
SendMessage(new KeyExchangeDhGroupExchangeRequest(MinimumGroupSize, PreferredGroupSize,
5958
MaximumProupSize));
6059
}
@@ -71,34 +70,38 @@ public override void Finish()
7170

7271
private void Session_MessageReceived(object sender, MessageEventArgs<Message> e)
7372
{
73+
// 2. server sends SSH_MSG_KEX_DH_GEX_GROUP
7474
var groupMessage = e.Message as KeyExchangeDhGroupExchangeGroup;
7575
if (groupMessage != null)
7676
{
77-
// Unregister message once received
77+
// Unregister SSH_MSG_KEX_DH_GEX_GROUP message once received
7878
Session.UnRegisterMessage("SSH_MSG_KEX_DH_GEX_GROUP");
79+
// Register in order to be able to receive SSH_MSG_KEX_DH_GEX_REPLY message
80+
Session.RegisterMessage("SSH_MSG_KEX_DH_GEX_REPLY");
7981

80-
// 2. Receive SSH_MSG_KEX_DH_GEX_GROUP
8182
_prime = groupMessage.SafePrime;
8283
_group = groupMessage.SubGroup;
8384

8485
PopulateClientExchangeValue();
8586

86-
// 3. Send SSH_MSG_KEX_DH_GEX_INIT
87+
// 3. client sends SSH_MSG_KEX_DH_GEX_INIT
8788
SendMessage(new KeyExchangeDhGroupExchangeInit(_clientExchangeValue));
89+
90+
// Skip further execution as we'll be waiting for the SSH_MSG_KEX_DH_GEX_REPLY message
91+
return;
8892
}
89-
else
93+
94+
// 4. server sends SSH_MSG_KEX_DH_GEX_REPLY
95+
var replyMessage = e.Message as KeyExchangeDhGroupExchangeReply;
96+
if (replyMessage != null)
9097
{
91-
var replyMessage = e.Message as KeyExchangeDhGroupExchangeReply;
92-
if (replyMessage != null)
93-
{
94-
// Unregister message once received
95-
Session.UnRegisterMessage("SSH_MSG_KEX_DH_GEX_REPLY");
98+
// Unregister SSH_MSG_KEX_DH_GEX_REPLY message once received
99+
Session.UnRegisterMessage("SSH_MSG_KEX_DH_GEX_REPLY");
96100

97-
HandleServerDhReply(replyMessage.HostKey, replyMessage.F, replyMessage.Signature);
101+
HandleServerDhReply(replyMessage.HostKey, replyMessage.F, replyMessage.Signature);
98102

99-
// When SSH_MSG_KEX_DH_GEX_REPLY received key exchange is completed
100-
Finish();
101-
}
103+
// When SSH_MSG_KEX_DH_GEX_REPLY received key exchange is completed
104+
Finish();
102105
}
103106
}
104107
}

0 commit comments

Comments
 (0)