From caac95c836dc58b7de4db46a451ce1b5ab250e20 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Fri, 6 Jun 2025 07:34:06 +0200 Subject: [PATCH] Check host key algorithms before continuing key exchange (#1642) The library currently does not check for matching host key algorithms until needed at the end of the key exchange, in contrast to other algorithm types which are checked beforehand. This leads to confusing or uninformative errors, normally from the server (correctly) closing the connection. This change moves that check alongside the rest of them, and also improves the error messages that arise from no matching algorithms. --- src/Renci.SshNet/Security/KeyExchange.cs | 101 ++++++++++-------- src/Renci.SshNet/ServiceFactory.cs | 2 +- .../ConnectivityTests.cs | 28 +++-- 3 files changed, 74 insertions(+), 57 deletions(-) diff --git a/src/Renci.SshNet/Security/KeyExchange.cs b/src/Renci.SshNet/Security/KeyExchange.cs index 8ae5c30a..dc442156 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -19,6 +19,7 @@ namespace Renci.SshNet.Security public abstract class KeyExchange : Algorithm, IKeyExchange { private readonly ILogger _logger; + private Func _hostKeyAlgorithmFactory; private CipherInfo _clientCipherInfo; private CipherInfo _serverCipherInfo; private HashInfo _clientHashInfo; @@ -81,6 +82,33 @@ namespace Renci.SshNet.Security SendMessage(session.ClientInitMessage); } + // Determine host key algorithm + var hostKeyAlgorithmName = (from b in session.ConnectionInfo.HostKeyAlgorithms.Keys + from a in message.ServerHostKeyAlgorithms + where a == b + select a).FirstOrDefault(); + + if (_logger.IsEnabled(LogLevel.Trace)) + { + _logger.LogTrace("[{SessionId}] Host key algorithm: we offer {WeOffer}", + Session.SessionIdHex, + session.ConnectionInfo.HostKeyAlgorithms.Keys.Join(",")); + + _logger.LogTrace("[{SessionId}] Host key algorithm: they offer {TheyOffer}", + Session.SessionIdHex, + message.ServerHostKeyAlgorithms.Join(",")); + } + + if (hostKeyAlgorithmName is null) + { + throw new SshConnectionException( + $"No matching host key algorithm (server offers {message.ServerHostKeyAlgorithms.Join(",")})", + DisconnectReason.KeyExchangeFailed); + } + + session.ConnectionInfo.CurrentHostKeyAlgorithm = hostKeyAlgorithmName; + _hostKeyAlgorithmFactory = session.ConnectionInfo.HostKeyAlgorithms[hostKeyAlgorithmName]; + // Determine client encryption algorithm var clientEncryptionAlgorithmName = (from b in session.ConnectionInfo.Encryptions.Keys from a in message.EncryptionAlgorithmsClientToServer @@ -98,9 +126,11 @@ namespace Renci.SshNet.Security message.EncryptionAlgorithmsClientToServer.Join(",")); } - if (string.IsNullOrEmpty(clientEncryptionAlgorithmName)) + if (clientEncryptionAlgorithmName is null) { - throw new SshConnectionException("Client encryption algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching client encryption algorithm (server offers {message.EncryptionAlgorithmsClientToServer.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentClientEncryption = clientEncryptionAlgorithmName; @@ -123,9 +153,11 @@ namespace Renci.SshNet.Security message.EncryptionAlgorithmsServerToClient.Join(",")); } - if (string.IsNullOrEmpty(serverDecryptionAlgorithmName)) + if (serverDecryptionAlgorithmName is null) { - throw new SshConnectionException("Server decryption algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching server encryption algorithm (server offers {message.EncryptionAlgorithmsServerToClient.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentServerEncryption = serverDecryptionAlgorithmName; @@ -150,9 +182,11 @@ namespace Renci.SshNet.Security message.MacAlgorithmsClientToServer.Join(",")); } - if (string.IsNullOrEmpty(clientHmacAlgorithmName)) + if (clientHmacAlgorithmName is null) { - throw new SshConnectionException("Client HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching client MAC algorithm (server offers {message.MacAlgorithmsClientToServer.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentClientHmacAlgorithm = clientHmacAlgorithmName; @@ -178,9 +212,11 @@ namespace Renci.SshNet.Security message.MacAlgorithmsServerToClient.Join(",")); } - if (string.IsNullOrEmpty(serverHmacAlgorithmName)) + if (serverHmacAlgorithmName is null) { - throw new SshConnectionException("Server HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching server MAC algorithm (server offers {message.MacAlgorithmsServerToClient.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentServerHmacAlgorithm = serverHmacAlgorithmName; @@ -204,9 +240,11 @@ namespace Renci.SshNet.Security message.CompressionAlgorithmsClientToServer.Join(",")); } - if (string.IsNullOrEmpty(compressionAlgorithmName)) + if (compressionAlgorithmName is null) { - throw new SshConnectionException("Compression algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching client compression algorithm (server offers {message.CompressionAlgorithmsClientToServer.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentClientCompressionAlgorithm = compressionAlgorithmName; @@ -229,9 +267,11 @@ namespace Renci.SshNet.Security message.CompressionAlgorithmsServerToClient.Join(",")); } - if (string.IsNullOrEmpty(decompressionAlgorithmName)) + if (decompressionAlgorithmName is null) { - throw new SshConnectionException("Decompression algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException( + $"No matching server compression algorithm (server offers {message.CompressionAlgorithmsServerToClient.Join(",")})", + DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentServerCompressionAlgorithm = decompressionAlgorithmName; @@ -245,7 +285,7 @@ namespace Renci.SshNet.Security { if (!ValidateExchangeHash()) { - throw new SshConnectionException("Key exchange negotiation failed.", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException("Host key could not be verified.", DisconnectReason.KeyExchangeFailed); } SendMessage(new NewKeysMessage()); @@ -449,40 +489,9 @@ namespace Renci.SshNet.Security { var exchangeHash = CalculateHash(); - // We need to inspect both the key and signature format identifers to find the correct - // HostAlgorithm instance. Example cases: + var keyAlgorithm = _hostKeyAlgorithmFactory(encodedKey); - // Key identifier Signature identifier | Algorithm name - // ssh-rsa ssh-rsa | ssh-rsa - // ssh-rsa rsa-sha2-256 | rsa-sha2-256 - // ssh-rsa-cert-v01@openssh.com ssh-rsa | ssh-rsa-cert-v01@openssh.com - // ssh-rsa-cert-v01@openssh.com rsa-sha2-256 | rsa-sha2-256-cert-v01@openssh.com - - var signatureData = new KeyHostAlgorithm.SignatureKeyData(); - signatureData.Load(encodedSignature); - - string keyName; - using (var keyReader = new SshDataStream(encodedKey)) - { - keyName = keyReader.ReadString(); - } - - string algorithmName; - - if (signatureData.AlgorithmName.StartsWith("rsa-sha2", StringComparison.Ordinal)) - { - algorithmName = keyName.Replace("ssh-rsa", signatureData.AlgorithmName); - } - else - { - algorithmName = keyName; - } - - var keyAlgorithm = Session.ConnectionInfo.HostKeyAlgorithms[algorithmName](encodedKey); - - Session.ConnectionInfo.CurrentHostKeyAlgorithm = algorithmName; - - return keyAlgorithm.VerifySignatureBlob(exchangeHash, signatureData.Signature) && CanTrustHostKey(keyAlgorithm); + return keyAlgorithm.VerifySignature(exchangeHash, encodedSignature) && CanTrustHostKey(keyAlgorithm); } /// diff --git a/src/Renci.SshNet/ServiceFactory.cs b/src/Renci.SshNet/ServiceFactory.cs index 3dc5ec34..79662907 100644 --- a/src/Renci.SshNet/ServiceFactory.cs +++ b/src/Renci.SshNet/ServiceFactory.cs @@ -101,7 +101,7 @@ namespace Renci.SshNet if (keyExchangeAlgorithmFactory is null) { - throw new SshConnectionException("Failed to negotiate key exchange algorithm.", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException($"No matching key exchange algorithm (server offers {serverAlgorithms.Join(",")})", DisconnectReason.KeyExchangeFailed); } return keyExchangeAlgorithmFactory(); diff --git a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs index 3fae0a8d..bbe6f906 100644 --- a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs +++ b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs @@ -415,16 +415,24 @@ namespace Renci.SshNet.IntegrationTests { client.HostKeyReceived += (sender, e) => { e.CanTrust = false; }; - try - { - client.Connect(); - Assert.Fail(); - } - catch (SshConnectionException ex) - { - Assert.IsNull(ex.InnerException); - Assert.AreEqual("Key exchange negotiation failed.", ex.Message); - } + var ex = Assert.Throws(client.Connect); + + Assert.AreEqual(DisconnectReason.KeyExchangeFailed, ex.DisconnectReason); + Assert.AreEqual("Host key could not be verified.", ex.Message); + } + } + + [TestMethod] + public void Common_HostKeyAlgorithms_NoMatch() + { + using (var client = new SshClient(_connectionInfoFactory.Create())) + { + client.ConnectionInfo.HostKeyAlgorithms.Clear(); + + var ex = Assert.Throws(client.Connect); + + Assert.AreEqual(DisconnectReason.KeyExchangeFailed, ex.DisconnectReason); + Assert.IsTrue(ex.Message.StartsWith("No matching host key algorithm"), ex.Message); } }