Key exchange negotiation failed when speak to OpenSSH 6.5 and 6.6

[scott-xu]

OpenSSH 6.5 and 6.6 has a bug that causes ~0.2% of connections using the [email protected] KEX exchange method to fail when connecting with something that implements the spec properly, for instance, SSH.NET

The bug is fixed in OpenSSH 6.6.1 and 6.7 onwards, see openssh/openssh-portable@adbfdbb

They also implemented a compatibility logic in newer OpenSSH so that when newer OpenSSH client speaks to OpenSSH 6.5/6.6, the client will NOT propose curve25519-sha256 nor [email protected] key exchange method.

Currently, when SSH.NET speaks to OpenSSH 6.5/6.6, ~0.2% of connections will throw below exception:

Renci.SshNet.Common.SshConnectionException: Key exchange negotiation failed.
at Renci.SshNet.Session.WaitOnHandle(WaitHandle waitHandle, TimeSpan timeout) in \SSH.NET-rsa-sha2-256\src\Renci.SshNet\Session.cs:line 977
at Renci.SshNet.Session.WaitOnHandle(WaitHandle waitHandle) in \SSH.NET-rsa-sha2-256\src\Renci.SshNet\Session.cs:line 874
at Renci.SshNet.Session.ConnectAsync(CancellationToken cancellationToken) in \SSH.NET-rsa-sha2-256\src\Renci.SshNet\Session.cs:line 739
at Renci.SshNet.BaseClient.CreateAndConnectSessionAsync(CancellationToken cancellationToken) in \SSH.NET-rsa-sha2-256\src\Renci.SshNet\BaseClient.cs:line 545
at Renci.SshNet.BaseClient.ConnectAsync(CancellationToken cancellationToken) in \SSH.NET-rsa-sha2-256\src\Renci.SshNet\BaseClient.cs:line 284

Options:

• We could consider removing curve25519-sha256 and [email protected] key exchange algorithms when and only when speak to OpenSSH 6.5 and 6.6.
• We could expose some event during negotiation so that user can attach a handler to the event and filter the key exchange algorithems based on server identification. For example: event EventHandler<SshIdentificationEventArgs> ServerIdentificationReceived
• Or we could do both.

[scott-xu]

Here's the test cases:

[TestMethod]
[DataRow("OpenSSH_6.5")]
[DataRow("OpenSSH_6.5p1")]
[DataRow("OpenSSH_6.5 PKIX")]
[DataRow("OpenSSH_6.6")]
[DataRow("OpenSSH_6.6p1")]
[DataRow("OpenSSH_6.6 PKIX")]
public void ShouldExcludeCurve25519Kex(string serverSoftwareVersion)
{
    ServerIdentification = new SshIdentification("2.0", serverSoftwareVersion);

    Assert.IsFalse(ConnectionInfo.KeyExchangeAlgorithms.Keys.Contains("curve25519-sha256"));
    Assert.IsFalse(ConnectionInfo.KeyExchangeAlgorithms.Keys.Contains("[email protected]"));
}

[TestMethod]
[DataRow("OpenSSH_6.6.1")]
[DataRow("OpenSSH_6.6.1p1")]
[DataRow("OpenSSH_6.6.1 PKIX")]
[DataRow("OpenSSH_6.7")]
[DataRow("OpenSSH_6.7p1")]
[DataRow("OpenSSH_6.7 PKIX")]
public void ShouldIncludeCurve25519Kex(string serverSoftwareVersion)
{
    ServerIdentification = new SshIdentification("2.0", serverSoftwareVersion);

    Assert.IsTrue(ConnectionInfo.KeyExchangeAlgorithms.Keys.Contains("curve25519-sha256"));
    Assert.IsTrue(ConnectionInfo.KeyExchangeAlgorithms.Keys.Contains("[email protected]"));
}

[scott-xu]

I think it would be better to provide a generic solution. Here I propose a new event in BaseClient named as SshIdentificationReceived. Lib user can update ConnectionInfo based on the server identification.

[Rob-Hague]

Sounds good to me. Did you have problems with the previous version of your PR? It seems reasonable to have both

[scott-xu]

The previous version of the PR is specific to fix the issue of OpenSSH 6.5/6.6. The updated PR is generic and can potentially benefit to PR #972

[WojciechNagorski]

The 2023.0.1 version has been released to Nuget: https://www.nuget.org/packages/SSH.NET/2023.0.1