Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement OpenSSH strict key exchange extension #1366

Merged
merged 19 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Renci.SshNet/ConnectionInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy
{ "diffie-hellman-group14-sha256", () => new KeyExchangeDiffieHellmanGroup14Sha256() },
{ "diffie-hellman-group14-sha1", () => new KeyExchangeDiffieHellmanGroup14Sha1() },
{ "diffie-hellman-group1-sha1", () => new KeyExchangeDiffieHellmanGroup1Sha1() },
{ "kex-strict-c-v00@openssh.com", null },
scott-xu marked this conversation as resolved.
Show resolved Hide resolved
};

Encryptions = new Dictionary<string, CipherInfo>
Expand Down
79 changes: 58 additions & 21 deletions src/Renci.SshNet/Session.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,17 @@ public class Session : ISession
/// </summary>
private bool _isDisconnecting;

/// <summary>
/// Indicates whether it is the init kex.
/// </summary>
private bool _isInitialKex = true;

/// <summary>
/// Indicates whether server supports strict key exchange.
/// <see href="https://github.com/openssh/openssh-portable/blob/master/PROTOCOL"/> 1.10.
/// </summary>
private bool _isStrictKex;

private IKeyExchange _keyExchange;

private HashAlgorithm _serverMac;
Expand Down Expand Up @@ -288,20 +299,20 @@ public Message ClientInitMessage
get
{
_clientInitMessage ??= new KeyExchangeInitMessage
{
KeyExchangeAlgorithms = ConnectionInfo.KeyExchangeAlgorithms.Keys.ToArray(),
ServerHostKeyAlgorithms = ConnectionInfo.HostKeyAlgorithms.Keys.ToArray(),
EncryptionAlgorithmsClientToServer = ConnectionInfo.Encryptions.Keys.ToArray(),
EncryptionAlgorithmsServerToClient = ConnectionInfo.Encryptions.Keys.ToArray(),
MacAlgorithmsClientToServer = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
MacAlgorithmsServerToClient = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
CompressionAlgorithmsClientToServer = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
CompressionAlgorithmsServerToClient = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
LanguagesClientToServer = new[] { string.Empty },
LanguagesServerToClient = new[] { string.Empty },
FirstKexPacketFollows = false,
Reserved = 0
};
{
KeyExchangeAlgorithms = ConnectionInfo.KeyExchangeAlgorithms.Keys.ToArray(),
ServerHostKeyAlgorithms = ConnectionInfo.HostKeyAlgorithms.Keys.ToArray(),
EncryptionAlgorithmsClientToServer = ConnectionInfo.Encryptions.Keys.ToArray(),
EncryptionAlgorithmsServerToClient = ConnectionInfo.Encryptions.Keys.ToArray(),
MacAlgorithmsClientToServer = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
MacAlgorithmsServerToClient = ConnectionInfo.HmacAlgorithms.Keys.ToArray(),
CompressionAlgorithmsClientToServer = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
CompressionAlgorithmsServerToClient = ConnectionInfo.CompressionAlgorithms.Keys.ToArray(),
LanguagesClientToServer = new[] { string.Empty },
LanguagesServerToClient = new[] { string.Empty },
FirstKexPacketFollows = false,
Reserved = 0
};

return _clientInitMessage;
}
Expand Down Expand Up @@ -1106,13 +1117,20 @@ internal void SendMessage(Message message)
SendPacket(data, 0, data.Length);
}

// increment the packet sequence number only after we're sure the packet has
// been sent; even though it's only used for the MAC, it needs to be incremented
// for each package sent.
//
// the server will use it to verify the data integrity, and as such the order in
// which messages are sent must follow the outbound packet sequence number
_outboundPacketSequence++;
if (_isStrictKex && message is NewKeysMessage)
{
_outboundPacketSequence = 0;
}
else
{
// increment the packet sequence number only after we're sure the packet has
// been sent; even though it's only used for the MAC, it needs to be incremented
// for each package sent.
//
// the server will use it to verify the data integrity, and as such the order in
// which messages are sent must follow the outbound packet sequence number
_outboundPacketSequence++;
}
}
}

Expand Down Expand Up @@ -1446,6 +1464,18 @@ internal void OnKeyExchangeInitReceived(KeyExchangeInitMessage message)

_keyExchangeCompletedWaitHandle.Reset();

if (_isInitialKex && message.KeyExchangeAlgorithms.Contains("kex-strict-s-v00@openssh.com"))
scott-xu marked this conversation as resolved.
Show resolved Hide resolved
{
_isStrictKex = true;

DiagnosticAbstraction.Log(string.Format("[{0}] Enabling strict key exchange extension.", ToHex(SessionId)));

if (_inboundPacketSequence != 1)
{
throw new SshConnectionException("KEXINIT was not the first packet during strict key exchange", DisconnectReason.KeyExchangeFailed);
}
}

// Disable messages that are not key exchange related
_sshMessageFactory.DisableNonKeyExchangeMessages();
scott-xu marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -1522,6 +1552,13 @@ internal void OnNewKeysReceived(NewKeysMessage message)
// Enable activated messages that are not key exchange related
_sshMessageFactory.EnableActivatedMessages();

_isInitialKex = false;

if (_isStrictKex)
{
_inboundPacketSequence = 0;
}

NewKeysReceived?.Invoke(this, new MessageEventArgs<NewKeysMessage>(message));

// Signal that key exchange completed
Expand Down