diff --git a/README.md b/README.md index 9ab4978f2..e39e4f298 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,12 @@ Private keys can be encrypted using one of the following cipher methods: * hmac-sha2-512-96 * hmac-ripemd160 * hmac-ripemd160@openssh.com +* hmac-md5-etm@openssh.com +* hmac-md5-96-etm@openssh.com +* hmac-sha1-etm@openssh.com +* hmac-sha1-96-etm@openssh.com +* hmac-sha2-256-etm@openssh.com +* hmac-sha2-512-etm@openssh.com ## Framework Support **SSH.NET** supports the following target frameworks: diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index b4cb0fc85..d57c08860 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -376,16 +376,24 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy #pragma warning disable IDE0200 // Remove unnecessary lambda expression; We want to prevent instantiating the HashAlgorithm objects. HmacAlgorithms = new Dictionary { - { "hmac-sha2-256", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key)) }, - { "hmac-sha2-512", new HashInfo(64 * 8, key => CryptoAbstraction.CreateHMACSHA512(key)) }, - { "hmac-sha2-512-96", new HashInfo(64 * 8, key => CryptoAbstraction.CreateHMACSHA512(key, 96)) }, - { "hmac-sha2-256-96", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, 96)) }, - { "hmac-ripemd160", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key)) }, - { "hmac-ripemd160@openssh.com", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key)) }, - { "hmac-sha1", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key)) }, - { "hmac-sha1-96", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96)) }, - { "hmac-md5", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key)) }, - { "hmac-md5-96", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96)) }, + /* Encrypt-and-MAC (encrypt-and-authenticate) variants */ + { "hmac-sha2-256", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key), isEncryptThenMAC: false) }, + { "hmac-sha2-512", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key), isEncryptThenMAC: false) }, + { "hmac-sha2-512-96", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, 96), isEncryptThenMAC: false) }, + { "hmac-sha2-256-96", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, 96), isEncryptThenMAC: false) }, + { "hmac-ripemd160", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key), isEncryptThenMAC: false) }, + { "hmac-ripemd160@openssh.com", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key), isEncryptThenMAC: false) }, + { "hmac-sha1", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key), isEncryptThenMAC: false) }, + { "hmac-sha1-96", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96), isEncryptThenMAC: false) }, + { "hmac-md5", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key), isEncryptThenMAC: false) }, + { "hmac-md5-96", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96), isEncryptThenMAC: false) }, + /* Encrypt-then-MAC variants */ + { "hmac-sha2-256-etm@openssh.com", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key), isEncryptThenMAC: true) }, + { "hmac-sha2-512-etm@openssh.com", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key), isEncryptThenMAC: true) }, + { "hmac-sha1-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key), isEncryptThenMAC: true) }, + { "hmac-sha1-96-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96), isEncryptThenMAC: true) }, + { "hmac-md5-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key), isEncryptThenMAC: true) }, + { "hmac-md5-96-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96), isEncryptThenMAC: true) }, }; #pragma warning restore IDE0200 // Remove unnecessary lambda expression diff --git a/src/Renci.SshNet/HashInfo.cs b/src/Renci.SshNet/HashInfo.cs index cbbbf5fe7..5412f0764 100644 --- a/src/Renci.SshNet/HashInfo.cs +++ b/src/Renci.SshNet/HashInfo.cs @@ -1,5 +1,6 @@ using System; using System.Security.Cryptography; + using Renci.SshNet.Common; namespace Renci.SshNet @@ -17,6 +18,14 @@ public class HashInfo /// public int KeySize { get; private set; } + /// + /// Gets a value indicating whether enable encrypt-then-MAC or use encrypt-and-MAC. + /// + /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. + /// + public bool IsEncryptThenMAC { get; private set; } + /// /// Gets the cipher. /// @@ -27,10 +36,12 @@ public class HashInfo /// /// Size of the key. /// The hash algorithm to use for a given key. - public HashInfo(int keySize, Func hash) + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. + public HashInfo(int keySize, Func hash, bool isEncryptThenMAC = false) { KeySize = keySize; HashAlgorithm = key => hash(key.Take(KeySize / 8)); + IsEncryptThenMAC = isEncryptThenMAC; } } } diff --git a/src/Renci.SshNet/Messages/Message.cs b/src/Renci.SshNet/Messages/Message.cs index eab59815d..fa3ba5f72 100644 --- a/src/Renci.SshNet/Messages/Message.cs +++ b/src/Renci.SshNet/Messages/Message.cs @@ -37,7 +37,7 @@ protected override void WriteBytes(SshDataStream stream) base.WriteBytes(stream); } - internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor) + internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor, bool isEncryptThenMAC = false) { const int outboundPacketSequenceSize = 4; @@ -78,7 +78,9 @@ internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor) var packetLength = messageLength + 4 + 1; // determine the padding length - var paddingLength = GetPaddingLength(paddingMultiplier, packetLength); + // in Encrypt-then-MAC mode, the length field is not encrypted, so we should keep it out of the + // padding length calculation + var paddingLength = GetPaddingLength(paddingMultiplier, isEncryptThenMAC ? packetLength - 4 : packetLength); // add padding bytes var paddingBytes = new byte[paddingLength]; @@ -104,7 +106,9 @@ internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor) var packetLength = messageLength + 4 + 1; // determine the padding length - var paddingLength = GetPaddingLength(paddingMultiplier, packetLength); + // in Encrypt-then-MAC mode, the length field is not encrypted, so we should keep it out of the + // padding length calculation + var paddingLength = GetPaddingLength(paddingMultiplier, isEncryptThenMAC ? packetLength - 4 : packetLength); var packetDataLength = GetPacketDataLength(messageLength, paddingLength); diff --git a/src/Renci.SshNet/Security/IKeyExchange.cs b/src/Renci.SshNet/Security/IKeyExchange.cs index 7ffd2f465..c8f04b219 100644 --- a/src/Renci.SshNet/Security/IKeyExchange.cs +++ b/src/Renci.SshNet/Security/IKeyExchange.cs @@ -66,18 +66,20 @@ public interface IKeyExchange : IDisposable /// /// Creates the server-side hash algorithm to use. /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. /// /// The server hash algorithm. /// - HashAlgorithm CreateServerHash(); + HashAlgorithm CreateServerHash(out bool isEncryptThenMAC); /// /// Creates the client-side hash algorithm to use. /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. /// /// The client hash algorithm. /// - HashAlgorithm CreateClientHash(); + HashAlgorithm CreateClientHash(out bool isEncryptThenMAC); /// /// Creates the compression algorithm to use to deflate data. diff --git a/src/Renci.SshNet/Security/KeyExchange.cs b/src/Renci.SshNet/Security/KeyExchange.cs index f24dcafe1..10f7e0f8a 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -103,7 +103,7 @@ from a in message.MacAlgorithmsClientToServer select a).FirstOrDefault(); if (string.IsNullOrEmpty(clientHmacAlgorithmName)) { - throw new SshConnectionException("Server HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); + throw new SshConnectionException("Client HMAC algorithm not found", DisconnectReason.KeyExchangeFailed); } session.ConnectionInfo.CurrentClientHmacAlgorithm = clientHmacAlgorithmName; @@ -218,11 +218,14 @@ public Cipher CreateClientCipher() /// /// Creates the server side hash algorithm to use. /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. /// /// The server-side hash algorithm. /// - public HashAlgorithm CreateServerHash() + public HashAlgorithm CreateServerHash(out bool isEncryptThenMAC) { + isEncryptThenMAC = _serverHashInfo.IsEncryptThenMAC; + // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; @@ -241,11 +244,14 @@ public HashAlgorithm CreateServerHash() /// /// Creates the client side hash algorithm to use. /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. /// /// The client-side hash algorithm. /// - public HashAlgorithm CreateClientHash() + public HashAlgorithm CreateClientHash(out bool isEncryptThenMAC) { + isEncryptThenMAC = _clientHashInfo.IsEncryptThenMAC; + // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 193b10028..37b7f2db3 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -176,6 +176,10 @@ public class Session : ISession private HashAlgorithm _clientMac; + private bool _serverEtm; + + private bool _clientEtm; + private Cipher _clientCipher; private Cipher _serverCipher; @@ -1054,7 +1058,7 @@ internal void SendMessage(Message message) DiagnosticAbstraction.Log(string.Format("[{0}] Sending message '{1}' to server: '{2}'.", ToHex(SessionId), message.GetType().Name, message)); var paddingMultiplier = _clientCipher is null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize); - var packetData = message.GetPacket(paddingMultiplier, _clientCompression); + var packetData = message.GetPacket(paddingMultiplier, _clientCompression, _clientMac != null && _clientEtm); // take a write lock to ensure the outbound packet sequence number is incremented // atomically, and only after the packet has actually been sent @@ -1063,7 +1067,7 @@ internal void SendMessage(Message message) byte[] hash = null; var packetDataOffset = 4; // first four bytes are reserved for outbound packet sequence - if (_clientMac != null) + if (_clientMac != null && !_clientEtm) { // write outbound packet sequence to start of packet data Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); @@ -1075,8 +1079,29 @@ internal void SendMessage(Message message) // Encrypt packet data if (_clientCipher != null) { - packetData = _clientCipher.Encrypt(packetData, packetDataOffset, packetData.Length - packetDataOffset); - packetDataOffset = 0; + if (_clientMac != null && _clientEtm) + { + // The length of the "packet length" field in bytes + const int packetLengthFieldLength = 4; + + var encryptedData = _clientCipher.Encrypt(packetData, packetDataOffset + packetLengthFieldLength, packetData.Length - packetDataOffset - packetLengthFieldLength); + + Array.Resize(ref packetData, packetDataOffset + packetLengthFieldLength + encryptedData.Length); + + // write outbound packet sequence to start of packet data + Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); + + // write encrypted data + Buffer.BlockCopy(encryptedData, 0, packetData, packetDataOffset + packetLengthFieldLength, encryptedData.Length); + + // calculate packet hash + hash = _clientMac.ComputeHash(packetData); + } + else + { + packetData = _clientCipher.Encrypt(packetData, packetDataOffset, packetData.Length - packetDataOffset); + packetDataOffset = 0; + } } if (packetData.Length > MaximumSshPacketSize) @@ -1194,8 +1219,22 @@ private Message ReceiveMessage(Socket socket) // The length of the "padding length" field in bytes const int paddingLengthFieldLength = 1; - // Determine the size of the first block, which is 8 or cipher block size (whichever is larger) bytes - var blockSize = _serverCipher is null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize); + int blockSize; + + // Determine the size of the first block which is 8 or cipher block size (whichever is larger) bytes + // The "packet length" field is not encrypted in ETM. + if (_serverMac != null && _serverEtm) + { + blockSize = (byte) 4; + } + else if (_serverCipher != null) + { + blockSize = Math.Max((byte) 8, _serverCipher.MinimumSize); + } + else + { + blockSize = (byte) 8; + } var serverMacLength = _serverMac != null ? _serverMac.HashSize/8 : 0; @@ -1215,7 +1254,7 @@ private Message ReceiveMessage(Socket socket) return null; } - if (_serverCipher != null) + if (_serverCipher != null && (_serverMac == null || !_serverEtm)) { firstBlock = _serverCipher.Decrypt(firstBlock); } @@ -1257,6 +1296,20 @@ private Message ReceiveMessage(Socket socket) } } + // validate encrypted message against MAC + if (_serverMac != null && _serverEtm) + { + var clientHash = _serverMac.ComputeHash(data, 0, data.Length - serverMacLength); + var serverHash = data.Take(data.Length - serverMacLength, serverMacLength); + + // TODO Add IsEqualTo overload that takes left+right index and number of bytes to compare. + // TODO That way we can eliminate the extra allocation of the Take above. + if (!serverHash.IsEqualTo(clientHash)) + { + throw new SshConnectionException("MAC error", DisconnectReason.MacError); + } + } + if (_serverCipher != null) { var numberOfBytesToDecrypt = data.Length - (blockSize + inboundPacketSequenceLength + serverMacLength); @@ -1271,8 +1324,8 @@ private Message ReceiveMessage(Socket socket) var messagePayloadLength = (int) packetLength - paddingLength - paddingLengthFieldLength; var messagePayloadOffset = inboundPacketSequenceLength + packetLengthFieldLength + paddingLengthFieldLength; - // validate message against MAC - if (_serverMac != null) + // validate decrypted message against MAC + if (_serverMac != null && !_serverEtm) { var clientHash = _serverMac.ComputeHash(data, 0, data.Length - serverMacLength); var serverHash = data.Take(data.Length - serverMacLength, serverMacLength); @@ -1472,8 +1525,8 @@ internal void OnNewKeysReceived(NewKeysMessage message) // Update negotiated algorithms _serverCipher = _keyExchange.CreateServerCipher(); _clientCipher = _keyExchange.CreateClientCipher(); - _serverMac = _keyExchange.CreateServerHash(); - _clientMac = _keyExchange.CreateClientHash(); + _serverMac = _keyExchange.CreateServerHash(out _serverEtm); + _clientMac = _keyExchange.CreateClientHash(out _clientEtm); _clientCompression = _keyExchange.CreateCompressor(); _serverDecompression = _keyExchange.CreateDecompressor(); diff --git a/test/Renci.SshNet.IntegrationTests/HmacTests.cs b/test/Renci.SshNet.IntegrationTests/HmacTests.cs index 993e5ec98..e7876bc16 100644 --- a/test/Renci.SshNet.IntegrationTests/HmacTests.cs +++ b/test/Renci.SshNet.IntegrationTests/HmacTests.cs @@ -58,6 +58,42 @@ public void HmacSha2_512() DoTest(MessageAuthenticationCodeAlgorithm.HmacSha2_512); } + [TestMethod] + public void HmacMd5_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacMd5Etm); + } + + [TestMethod] + public void HmacMd5_96_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacMd5_96_Etm); + } + + [TestMethod] + public void HmacSha1_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacSha1Etm); + } + + [TestMethod] + public void HmacSha1_96_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacSha1_96_Etm); + } + + [TestMethod] + public void HmacSha2_256_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacSha2_256_Etm); + } + + [TestMethod] + public void HmacSha2_512_Etm() + { + DoTest(MessageAuthenticationCodeAlgorithm.HmacSha2_512_Etm); + } + private void DoTest(MessageAuthenticationCodeAlgorithm macAlgorithm) { _remoteSshdConfig.ClearMessageAuthenticationCodeAlgorithms() diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs index 6331f7b9c..5ce0d4675 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs @@ -215,10 +215,18 @@ private void SetupMocks() .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); - _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HashAlgorithm) null); - _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HashAlgorithm) null); + _ = _keyExchangeMock.Setup(p => p.CreateServerHash(out It.Ref.IsAny)) + .Returns((ref bool serverEtm) => + { + serverEtm = false; + return (HashAlgorithm) null; + }); + _ = _keyExchangeMock.Setup(p => p.CreateClientHash(out It.Ref.IsAny)) + .Returns((ref bool clientEtm) => + { + clientEtm = false; + return (HashAlgorithm) null; + }); _ = _keyExchangeMock.Setup(p => p.CreateCompressor()) .Returns((Compressor) null); _ = _keyExchangeMock.Setup(p => p.CreateDecompressor()) diff --git a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs index 11cda2d90..c75fa32a3 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Sockets; using System.Security.Cryptography; + using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using Renci.SshNet.Common; @@ -163,10 +164,18 @@ private void SetupMocks() .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); - _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HashAlgorithm) null); - _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HashAlgorithm) null); + _ = _keyExchangeMock.Setup(p => p.CreateServerHash(out It.Ref.IsAny)) + .Returns((ref bool serverEtm) => + { + serverEtm = false; + return (HashAlgorithm) null; + }); + _ = _keyExchangeMock.Setup(p => p.CreateClientHash(out It.Ref.IsAny)) + .Returns((ref bool clientEtm) => + { + clientEtm = false; + return (HashAlgorithm) null; + }); _ = _keyExchangeMock.Setup(p => p.CreateCompressor()) .Returns((Compressor) null); _ = _keyExchangeMock.Setup(p => p.CreateDecompressor())