From 12984cca3077ecdd32e27b730ba69d5a72ad624e Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 12 Feb 2024 21:15:25 +0800 Subject: [PATCH 1/7] Support ETM (Encrypt-then-MAC) variants for HMAC --- .../Abstractions/CryptoAbstraction.cs | 58 ++++++++++-------- src/Renci.SshNet/ConnectionInfo.cs | 4 ++ src/Renci.SshNet/HashInfo.cs | 13 ++-- .../Security/Cryptography/HMAC.cs | 49 +++++++++++++++ src/Renci.SshNet/Security/IKeyExchange.cs | 5 +- src/Renci.SshNet/Security/KeyExchange.cs | 11 ++-- src/Renci.SshNet/Session.cs | 60 +++++++++++++++---- .../HmacTests.cs | 12 ++++ .../Classes/SessionTest_ConnectedBase.cs | 5 +- ...Connected_ServerAndClientDisconnectRace.cs | 5 +- 10 files changed, 167 insertions(+), 55 deletions(-) create mode 100644 src/Renci.SshNet/Security/Cryptography/HMAC.cs diff --git a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs index ca187833f..8456b37a8 100644 --- a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs +++ b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs @@ -84,75 +84,85 @@ public static System.Security.Cryptography.RIPEMD160 CreateRIPEMD160() } #endif // FEATURE_HASH_RIPEMD160 - public static System.Security.Cryptography.HMACMD5 CreateHMACMD5(byte[] key) + public static HMAC CreateHMACMD5(byte[] key) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new System.Security.Cryptography.HMACMD5(key); + return new HMAC(new System.Security.Cryptography.HMACMD5(key)); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static HMACMD5 CreateHMACMD5(byte[] key, int hashSize) + public static HMAC CreateHMACMD5(byte[] key, int hashSize) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new HMACMD5(key, hashSize); + return new HMAC(new HMACMD5(key, hashSize)); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static System.Security.Cryptography.HMACSHA1 CreateHMACSHA1(byte[] key) + public static HMAC CreateHMACSHA1(byte[] key) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new System.Security.Cryptography.HMACSHA1(key); + return new HMAC(new System.Security.Cryptography.HMACSHA1(key)); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static HMACSHA1 CreateHMACSHA1(byte[] key, int hashSize) + public static HMAC CreateHMACSHA1(byte[] key, int hashSize) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMACSHA1(key, hashSize); + return new HMAC(new HMACSHA1(key, hashSize)); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static System.Security.Cryptography.HMACSHA256 CreateHMACSHA256(byte[] key) + public static HMAC CreateHMACSHA256(byte[] key) { - return new System.Security.Cryptography.HMACSHA256(key); + return new HMAC(new System.Security.Cryptography.HMACSHA256(key)); } - public static HMACSHA256 CreateHMACSHA256(byte[] key, int hashSize) + public static HMAC CreateHMACSHA256(byte[] key, int hashSize) { - return new HMACSHA256(key, hashSize); + return new HMAC(new HMACSHA256(key, hashSize)); } - public static System.Security.Cryptography.HMACSHA384 CreateHMACSHA384(byte[] key) + public static HMAC CreateHMACSHA256(byte[] key, bool etm) { - return new System.Security.Cryptography.HMACSHA384(key); + return new HMAC(new System.Security.Cryptography.HMACSHA256(key), etm); } - public static HMACSHA384 CreateHMACSHA384(byte[] key, int hashSize) + public static HMAC CreateHMACSHA384(byte[] key) { - return new HMACSHA384(key, hashSize); + return new HMAC(new System.Security.Cryptography.HMACSHA384(key)); } - public static System.Security.Cryptography.HMACSHA512 CreateHMACSHA512(byte[] key) + public static HMAC CreateHMACSHA384(byte[] key, int hashSize) { - return new System.Security.Cryptography.HMACSHA512(key); + return new HMAC(new HMACSHA384(key, hashSize)); } - public static HMACSHA512 CreateHMACSHA512(byte[] key, int hashSize) + public static HMAC CreateHMACSHA512(byte[] key) { - return new HMACSHA512(key, hashSize); + return new HMAC(new System.Security.Cryptography.HMACSHA512(key)); + } + + public static HMAC CreateHMACSHA512(byte[] key, int hashSize) + { + return new HMAC(new HMACSHA512(key, hashSize)); + } + + public static HMAC CreateHMACSHA512(byte[] key, bool etm) + { + return new HMAC(new System.Security.Cryptography.HMACSHA512(key), etm); } #if FEATURE_HMAC_RIPEMD160 - public static System.Security.Cryptography.HMACRIPEMD160 CreateHMACRIPEMD160(byte[] key) + public static HMAC CreateHMACRIPEMD160(byte[] key) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new System.Security.Cryptography.HMACRIPEMD160(key); + return new HMAC(new System.Security.Cryptography.HMACRIPEMD160(key)); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } #else - public static global::SshNet.Security.Cryptography.HMACRIPEMD160 CreateHMACRIPEMD160(byte[] key) + public static HMAC CreateHMACRIPEMD160(byte[] key) { - return new global::SshNet.Security.Cryptography.HMACRIPEMD160(key); + return new HMAC(new global::SshNet.Security.Cryptography.HMACRIPEMD160(key)); } #endif // FEATURE_HMAC_RIPEMD160 } diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index b4cb0fc85..8abf0a75b 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -376,6 +376,7 @@ 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 { + /* Encrypt-and-MAC (encrypt-and-authenticate) variants */ { "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)) }, @@ -386,6 +387,9 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy { "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-then-MAC variants */ + { "hmac-sha2-256-etm@openssh.com", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, etm: true)) }, + { "hmac-sha2-512-etm@openssh.com", new HashInfo(64 * 8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: 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..a7f0e9375 100644 --- a/src/Renci.SshNet/HashInfo.cs +++ b/src/Renci.SshNet/HashInfo.cs @@ -1,6 +1,6 @@ using System; -using System.Security.Cryptography; using Renci.SshNet.Common; +using Renci.SshNet.Security.Cryptography; namespace Renci.SshNet { @@ -20,17 +20,22 @@ public class HashInfo /// /// Gets the cipher. /// - public Func HashAlgorithm { get; private set; } + public Func HMAC { get; private set; } + + /// + /// Gets a value indicating whether Encrypt-then-MAC or not. + /// + public bool ETM { get; private set; } /// /// Initializes a new instance of the class. /// /// Size of the key. /// The hash algorithm to use for a given key. - public HashInfo(int keySize, Func hash) + public HashInfo(int keySize, Func hash) { KeySize = keySize; - HashAlgorithm = key => hash(key.Take(KeySize / 8)); + HMAC = key => hash(key.Take(KeySize / 8)); } } } diff --git a/src/Renci.SshNet/Security/Cryptography/HMAC.cs b/src/Renci.SshNet/Security/Cryptography/HMAC.cs new file mode 100644 index 000000000..079089b89 --- /dev/null +++ b/src/Renci.SshNet/Security/Cryptography/HMAC.cs @@ -0,0 +1,49 @@ +using System; +using System.Security.Cryptography; + +namespace Renci.SshNet.Security.Cryptography +{ + /// + /// Represents the info for Message Authentication Code (MAC). + /// + public sealed class HMAC : IDisposable + { + /// + /// Initializes a new instance of the class. + /// + /// The hash algorithm. + public HMAC(HashAlgorithm hashAlgorithm) + : this(hashAlgorithm, etm: false) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The hash algorithm. + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. + public HMAC( + HashAlgorithm hashAlgorithm, + bool etm) + { + HashAlgorithm = hashAlgorithm; + ETM = etm; + } + + /// + public void Dispose() + { + HashAlgorithm?.Dispose(); + } + + /// + /// Gets the hash algorithem. + /// + public HashAlgorithm HashAlgorithm { get; private set; } + + /// + /// Gets a value indicating whether enable encryption-to-mac or encryption-then-mac. + /// + public bool ETM { get; private set; } + } +} diff --git a/src/Renci.SshNet/Security/IKeyExchange.cs b/src/Renci.SshNet/Security/IKeyExchange.cs index 7ffd2f465..545a5f872 100644 --- a/src/Renci.SshNet/Security/IKeyExchange.cs +++ b/src/Renci.SshNet/Security/IKeyExchange.cs @@ -1,5 +1,4 @@ using System; -using System.Security.Cryptography; using Renci.SshNet.Common; using Renci.SshNet.Compression; @@ -69,7 +68,7 @@ public interface IKeyExchange : IDisposable /// /// The server hash algorithm. /// - HashAlgorithm CreateServerHash(); + HMAC CreateServerHash(); /// /// Creates the client-side hash algorithm to use. @@ -77,7 +76,7 @@ public interface IKeyExchange : IDisposable /// /// The client hash algorithm. /// - HashAlgorithm CreateClientHash(); + HMAC CreateClientHash(); /// /// 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..d1544ad33 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Security.Cryptography; using Renci.SshNet.Abstractions; using Renci.SshNet.Common; @@ -103,7 +102,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; @@ -221,7 +220,7 @@ public Cipher CreateClientCipher() /// /// The server-side hash algorithm. /// - public HashAlgorithm CreateServerHash() + public HMAC CreateServerHash() { // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; @@ -235,7 +234,7 @@ public HashAlgorithm CreateServerHash() Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentServerHmacAlgorithm)); - return _serverHashInfo.HashAlgorithm(serverKey); + return _serverHashInfo.HMAC(serverKey); } /// @@ -244,7 +243,7 @@ public HashAlgorithm CreateServerHash() /// /// The client-side hash algorithm. /// - public HashAlgorithm CreateClientHash() + public HMAC CreateClientHash() { // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; @@ -258,7 +257,7 @@ public HashAlgorithm CreateClientHash() Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentClientHmacAlgorithm)); - return _clientHashInfo.HashAlgorithm(clientKey); + return _clientHashInfo.HMAC(clientKey); } /// diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 193b10028..54551adea 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -3,7 +3,6 @@ using System.Globalization; using System.Linq; using System.Net.Sockets; -using System.Security.Cryptography; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -172,9 +171,9 @@ public class Session : ISession private IKeyExchange _keyExchange; - private HashAlgorithm _serverMac; + private HMAC _serverMac; - private HashAlgorithm _clientMac; + private HMAC _clientMac; private Cipher _clientCipher; @@ -1063,20 +1062,43 @@ 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 && !_clientMac.ETM) { // write outbound packet sequence to start of packet data Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); // calculate packet hash - hash = _clientMac.ComputeHash(packetData); + hash = _clientMac.HashAlgorithm.ComputeHash(packetData); } // Encrypt packet data if (_clientCipher != null) { - packetData = _clientCipher.Encrypt(packetData, packetDataOffset, packetData.Length - packetDataOffset); - packetDataOffset = 0; + if (_clientMac != null && _clientMac.ETM) + { + var packetDataLengthFieldSize = 4; + + var encryptedData = _clientCipher.Encrypt(packetData, packetDataOffset + packetDataLengthFieldSize, packetData.Length - packetDataOffset - packetDataLengthFieldSize); + + packetData = new byte[packetDataOffset + packetDataLengthFieldSize + encryptedData.Length]; + + // write outbound packet sequence to start of packet data + Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); + + // write packet data length field + Pack.UInt32ToBigEndian((uint) encryptedData.Length, packetData, packetDataOffset); + + // write encrypted data + Buffer.BlockCopy(encryptedData, 0, packetData, packetDataOffset + packetDataLengthFieldSize, encryptedData.Length); + + // calculate packet hash + hash = _clientMac.HashAlgorithm.ComputeHash(packetData); + } + else + { + packetData = _clientCipher.Encrypt(packetData, packetDataOffset, packetData.Length - packetDataOffset); + packetDataOffset = 0; + } } if (packetData.Length > MaximumSshPacketSize) @@ -1197,7 +1219,7 @@ private Message ReceiveMessage(Socket socket) // 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); - var serverMacLength = _serverMac != null ? _serverMac.HashSize/8 : 0; + var serverMacLength = _serverMac != null ? _serverMac.HashAlgorithm.HashSize/8 : 0; byte[] data; uint packetLength; @@ -1215,7 +1237,7 @@ private Message ReceiveMessage(Socket socket) return null; } - if (_serverCipher != null) + if (_serverCipher != null && !_serverMac.ETM) { firstBlock = _serverCipher.Decrypt(firstBlock); } @@ -1257,6 +1279,20 @@ private Message ReceiveMessage(Socket socket) } } + // validate encrypted message against MAC + if (_serverMac != null && _serverMac.ETM) + { + var clientHash = _serverMac.HashAlgorithm.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,10 +1307,10 @@ 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 && !_serverMac.ETM) { - var clientHash = _serverMac.ComputeHash(data, 0, data.Length - serverMacLength); + var clientHash = _serverMac.HashAlgorithm.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. diff --git a/test/Renci.SshNet.IntegrationTests/HmacTests.cs b/test/Renci.SshNet.IntegrationTests/HmacTests.cs index 993e5ec98..9a89c7089 100644 --- a/test/Renci.SshNet.IntegrationTests/HmacTests.cs +++ b/test/Renci.SshNet.IntegrationTests/HmacTests.cs @@ -58,6 +58,18 @@ public void HmacSha2_512() DoTest(MessageAuthenticationCodeAlgorithm.HmacSha2_512); } + [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..3ed966253 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs @@ -3,7 +3,6 @@ using System.Globalization; using System.Net; using System.Net.Sockets; -using System.Security.Cryptography; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -216,9 +215,9 @@ private void SetupMocks() _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HashAlgorithm) null); + .Returns((HMAC) null); _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HashAlgorithm) null); + .Returns((HMAC) 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..7e4fed4fc 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs @@ -3,7 +3,6 @@ using System.Globalization; using System.Net; using System.Net.Sockets; -using System.Security.Cryptography; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using Renci.SshNet.Common; @@ -164,9 +163,9 @@ private void SetupMocks() _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HashAlgorithm) null); + .Returns((HMAC) null); _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HashAlgorithm) null); + .Returns((HMAC) null); _ = _keyExchangeMock.Setup(p => p.CreateCompressor()) .Returns((Compressor) null); _ = _keyExchangeMock.Setup(p => p.CreateDecompressor()) From 9c908abdd9f5240a679ce10aabbb44ac356c823d Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 13 Feb 2024 07:43:42 +0800 Subject: [PATCH 2/7] Support ETM (Encrypt-then-MAC) variants for HMAC --- src/Renci.SshNet/Messages/Message.cs | 10 +++++--- src/Renci.SshNet/Session.cs | 34 +++++++++++++++++++--------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/Renci.SshNet/Messages/Message.cs b/src/Renci.SshNet/Messages/Message.cs index eab59815d..99e9077dd 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 etm = 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, etm ? 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, etm ? packetLength - 4 : packetLength); var packetDataLength = GetPacketDataLength(messageLength, paddingLength); diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index 54551adea..c8d6d87b6 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -1053,7 +1053,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 && _clientMac.ETM); // take a write lock to ensure the outbound packet sequence number is incremented // atomically, and only after the packet has actually been sent @@ -1076,20 +1076,18 @@ internal void SendMessage(Message message) { if (_clientMac != null && _clientMac.ETM) { - var packetDataLengthFieldSize = 4; + // The length of the "packet length" field in bytes + const int packetLengthFieldLength = 4; - var encryptedData = _clientCipher.Encrypt(packetData, packetDataOffset + packetDataLengthFieldSize, packetData.Length - packetDataOffset - packetDataLengthFieldSize); + var encryptedData = _clientCipher.Encrypt(packetData, packetDataOffset + packetLengthFieldLength, packetData.Length - packetDataOffset - packetLengthFieldLength); - packetData = new byte[packetDataOffset + packetDataLengthFieldSize + encryptedData.Length]; + Array.Resize(ref packetData, packetDataOffset + packetLengthFieldLength + encryptedData.Length); // write outbound packet sequence to start of packet data Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); - // write packet data length field - Pack.UInt32ToBigEndian((uint) encryptedData.Length, packetData, packetDataOffset); - // write encrypted data - Buffer.BlockCopy(encryptedData, 0, packetData, packetDataOffset + packetDataLengthFieldSize, encryptedData.Length); + Buffer.BlockCopy(encryptedData, 0, packetData, packetDataOffset + packetLengthFieldLength, encryptedData.Length); // calculate packet hash hash = _clientMac.HashAlgorithm.ComputeHash(packetData); @@ -1216,8 +1214,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 && _serverMac.ETM) + { + blockSize = (byte) 4; + } + else if (_serverCipher != null) + { + blockSize = Math.Max((byte) 8, _serverCipher.MinimumSize); + } + else + { + blockSize = (byte) 8; + } var serverMacLength = _serverMac != null ? _serverMac.HashAlgorithm.HashSize/8 : 0; @@ -1237,7 +1249,7 @@ private Message ReceiveMessage(Socket socket) return null; } - if (_serverCipher != null && !_serverMac.ETM) + if (_serverCipher != null && (_serverMac == null || !_serverMac.ETM)) { firstBlock = _serverCipher.Decrypt(firstBlock); } From bb84fd724565974133bdb5194b4df4543620c5a7 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 13 Feb 2024 21:36:14 +0800 Subject: [PATCH 3/7] Remove `ETM` property from `HashInfo` --- src/Renci.SshNet/HashInfo.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Renci.SshNet/HashInfo.cs b/src/Renci.SshNet/HashInfo.cs index a7f0e9375..63015036a 100644 --- a/src/Renci.SshNet/HashInfo.cs +++ b/src/Renci.SshNet/HashInfo.cs @@ -22,11 +22,6 @@ public class HashInfo /// public Func HMAC { get; private set; } - /// - /// Gets a value indicating whether Encrypt-then-MAC or not. - /// - public bool ETM { get; private set; } - /// /// Initializes a new instance of the class. /// From b126f401efe7a70b0339f46652398eb903a7e884 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Tue, 13 Feb 2024 21:44:42 +0800 Subject: [PATCH 4/7] Add support for HmacMd5Etm, HmacMd5_96_Etm, HmacSha1Etm and HmacSha1_96_Etm Explicitly specify etm even if false --- .../Abstractions/CryptoAbstraction.cs | 54 ++++++++----------- src/Renci.SshNet/ConnectionInfo.cs | 26 +++++---- .../Security/Cryptography/HMAC.cs | 9 ---- .../HmacTests.cs | 24 +++++++++ 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs index 8456b37a8..749ac755d 100644 --- a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs +++ b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs @@ -84,85 +84,75 @@ public static System.Security.Cryptography.RIPEMD160 CreateRIPEMD160() } #endif // FEATURE_HASH_RIPEMD160 - public static HMAC CreateHMACMD5(byte[] key) + public static HMAC CreateHMACMD5(byte[] key, bool etm) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACMD5(key)); + return new HMAC(new System.Security.Cryptography.HMACMD5(key), etm); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static HMAC CreateHMACMD5(byte[] key, int hashSize) + public static HMAC CreateHMACMD5(byte[] key, int hashSize, bool etm) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new HMAC(new HMACMD5(key, hashSize)); + return new HMAC(new HMACMD5(key, hashSize), etm); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static HMAC CreateHMACSHA1(byte[] key) + public static HMAC CreateHMACSHA1(byte[] key, bool etm) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACSHA1(key)); + return new HMAC(new System.Security.Cryptography.HMACSHA1(key), etm); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static HMAC CreateHMACSHA1(byte[] key, int hashSize) + public static HMAC CreateHMACSHA1(byte[] key, int hashSize, bool etm) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new HMACSHA1(key, hashSize)); + return new HMAC(new HMACSHA1(key, hashSize), etm); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static HMAC CreateHMACSHA256(byte[] key) - { - return new HMAC(new System.Security.Cryptography.HMACSHA256(key)); - } - - public static HMAC CreateHMACSHA256(byte[] key, int hashSize) - { - return new HMAC(new HMACSHA256(key, hashSize)); - } - public static HMAC CreateHMACSHA256(byte[] key, bool etm) { return new HMAC(new System.Security.Cryptography.HMACSHA256(key), etm); } - public static HMAC CreateHMACSHA384(byte[] key) + public static HMAC CreateHMACSHA256(byte[] key, int hashSize, bool etm) { - return new HMAC(new System.Security.Cryptography.HMACSHA384(key)); + return new HMAC(new HMACSHA256(key, hashSize), etm); } - public static HMAC CreateHMACSHA384(byte[] key, int hashSize) + public static HMAC CreateHMACSHA384(byte[] key, bool etm) { - return new HMAC(new HMACSHA384(key, hashSize)); + return new HMAC(new System.Security.Cryptography.HMACSHA384(key), etm); } - public static HMAC CreateHMACSHA512(byte[] key) + public static HMAC CreateHMACSHA384(byte[] key, int hashSize, bool etm) { - return new HMAC(new System.Security.Cryptography.HMACSHA512(key)); + return new HMAC(new HMACSHA384(key, hashSize), etm); } - public static HMAC CreateHMACSHA512(byte[] key, int hashSize) + public static HMAC CreateHMACSHA512(byte[] key, bool etm) { - return new HMAC(new HMACSHA512(key, hashSize)); + return new HMAC(new System.Security.Cryptography.HMACSHA512(key), etm); } - public static HMAC CreateHMACSHA512(byte[] key, bool etm) + public static HMAC CreateHMACSHA512(byte[] key, int hashSize, bool etm) { - return new HMAC(new System.Security.Cryptography.HMACSHA512(key), etm); + return new HMAC(new HMACSHA512(key, hashSize), etm); } #if FEATURE_HMAC_RIPEMD160 - public static HMAC CreateHMACRIPEMD160(byte[] key) + public static HMAC CreateHMACRIPEMD160(byte[] key, bool etm) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACRIPEMD160(key)); + return new HMAC(new System.Security.Cryptography.HMACRIPEMD160(key), etm); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } #else - public static HMAC CreateHMACRIPEMD160(byte[] key) + public static HMAC CreateHMACRIPEMD160(byte[] key, bool etm) { - return new HMAC(new global::SshNet.Security.Cryptography.HMACRIPEMD160(key)); + return new HMAC(new global::SshNet.Security.Cryptography.HMACRIPEMD160(key), etm); } #endif // FEATURE_HMAC_RIPEMD160 } diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index 8abf0a75b..c55d1108c 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -377,19 +377,23 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy HmacAlgorithms = new Dictionary { /* Encrypt-and-MAC (encrypt-and-authenticate) variants */ - { "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)) }, + { "hmac-sha2-256", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, etm: false)) }, + { "hmac-sha2-512", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: false)) }, + { "hmac-sha2-512-96", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, 96, etm: false)) }, + { "hmac-sha2-256-96", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, 96, etm: false)) }, + { "hmac-ripemd160", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key, etm: false)) }, + { "hmac-ripemd160@openssh.com", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key, etm: false)) }, + { "hmac-sha1", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, etm: false)) }, + { "hmac-sha1-96", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96, etm: false)) }, + { "hmac-md5", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, etm: false)) }, + { "hmac-md5-96", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96, etm: false)) }, /* Encrypt-then-MAC variants */ { "hmac-sha2-256-etm@openssh.com", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, etm: true)) }, - { "hmac-sha2-512-etm@openssh.com", new HashInfo(64 * 8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: true)) }, + { "hmac-sha2-512-etm@openssh.com", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: true)) }, + { "hmac-sha1-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, etm: true)) }, + { "hmac-sha1-96-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96, etm: true)) }, + { "hmac-md5-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, etm: true)) }, + { "hmac-md5-96-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96, etm: true)) }, }; #pragma warning restore IDE0200 // Remove unnecessary lambda expression diff --git a/src/Renci.SshNet/Security/Cryptography/HMAC.cs b/src/Renci.SshNet/Security/Cryptography/HMAC.cs index 079089b89..764ba0f49 100644 --- a/src/Renci.SshNet/Security/Cryptography/HMAC.cs +++ b/src/Renci.SshNet/Security/Cryptography/HMAC.cs @@ -8,15 +8,6 @@ namespace Renci.SshNet.Security.Cryptography /// public sealed class HMAC : IDisposable { - /// - /// Initializes a new instance of the class. - /// - /// The hash algorithm. - public HMAC(HashAlgorithm hashAlgorithm) - : this(hashAlgorithm, etm: false) - { - } - /// /// Initializes a new instance of the class. /// diff --git a/test/Renci.SshNet.IntegrationTests/HmacTests.cs b/test/Renci.SshNet.IntegrationTests/HmacTests.cs index 9a89c7089..e7876bc16 100644 --- a/test/Renci.SshNet.IntegrationTests/HmacTests.cs +++ b/test/Renci.SshNet.IntegrationTests/HmacTests.cs @@ -58,6 +58,30 @@ 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() { From 7712a629434cbe3b9335455d966078afa3757c8b Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Wed, 14 Feb 2024 08:52:24 +0800 Subject: [PATCH 5/7] Add Encrypt-then-MAC variants to README.md --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9ab4978f2..8791da8b3 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,13 @@ Private keys can be encrypted using one of the following cipher methods: * hmac-sha2-512 * hmac-sha2-512-96 * hmac-ripemd160 -* hmac-ripemd160@openssh.com +* 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: From 6bc85f27ed8bb6fd1d91e6351a9af708b21d4766 Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Wed, 14 Feb 2024 09:04:10 +0800 Subject: [PATCH 6/7] Add back empty span to prevent auto link --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 8791da8b3..e39e4f298 100644 --- a/README.md +++ b/README.md @@ -114,13 +114,13 @@ Private keys can be encrypted using one of the following cipher methods: * hmac-sha2-512 * 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 +* 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: From 2d991cc9963ea3c30465384031c4deb84383607f Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Fri, 16 Feb 2024 18:17:55 +0800 Subject: [PATCH 7/7] Store ETM in `HashInfo`; Change `HMAC Create[...]Hash()` to `HashAlgorithm Create[...]Hash(out bool isEncryptThenMAC)` --- .../Abstractions/CryptoAbstraction.cs | 48 +++++++++---------- src/Renci.SshNet/ConnectionInfo.cs | 32 ++++++------- src/Renci.SshNet/HashInfo.cs | 19 ++++++-- src/Renci.SshNet/Messages/Message.cs | 6 +-- .../Security/Cryptography/HMAC.cs | 40 ---------------- src/Renci.SshNet/Security/IKeyExchange.cs | 7 ++- src/Renci.SshNet/Security/KeyExchange.cs | 15 ++++-- src/Renci.SshNet/Session.cs | 37 +++++++------- .../Classes/SessionTest_ConnectedBase.cs | 17 +++++-- ...Connected_ServerAndClientDisconnectRace.cs | 18 +++++-- 10 files changed, 122 insertions(+), 117 deletions(-) delete mode 100644 src/Renci.SshNet/Security/Cryptography/HMAC.cs diff --git a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs index 749ac755d..ca187833f 100644 --- a/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs +++ b/src/Renci.SshNet/Abstractions/CryptoAbstraction.cs @@ -84,75 +84,75 @@ public static System.Security.Cryptography.RIPEMD160 CreateRIPEMD160() } #endif // FEATURE_HASH_RIPEMD160 - public static HMAC CreateHMACMD5(byte[] key, bool etm) + public static System.Security.Cryptography.HMACMD5 CreateHMACMD5(byte[] key) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACMD5(key), etm); + return new System.Security.Cryptography.HMACMD5(key); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static HMAC CreateHMACMD5(byte[] key, int hashSize, bool etm) + public static HMACMD5 CreateHMACMD5(byte[] key, int hashSize) { #pragma warning disable CA5351 // Do not use broken cryptographic algorithms - return new HMAC(new HMACMD5(key, hashSize), etm); + return new HMACMD5(key, hashSize); #pragma warning restore CA5351 // Do not use broken cryptographic algorithms } - public static HMAC CreateHMACSHA1(byte[] key, bool etm) + public static System.Security.Cryptography.HMACSHA1 CreateHMACSHA1(byte[] key) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACSHA1(key), etm); + return new System.Security.Cryptography.HMACSHA1(key); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static HMAC CreateHMACSHA1(byte[] key, int hashSize, bool etm) + public static HMACSHA1 CreateHMACSHA1(byte[] key, int hashSize) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new HMACSHA1(key, hashSize), etm); + return new HMACSHA1(key, hashSize); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } - public static HMAC CreateHMACSHA256(byte[] key, bool etm) + public static System.Security.Cryptography.HMACSHA256 CreateHMACSHA256(byte[] key) { - return new HMAC(new System.Security.Cryptography.HMACSHA256(key), etm); + return new System.Security.Cryptography.HMACSHA256(key); } - public static HMAC CreateHMACSHA256(byte[] key, int hashSize, bool etm) + public static HMACSHA256 CreateHMACSHA256(byte[] key, int hashSize) { - return new HMAC(new HMACSHA256(key, hashSize), etm); + return new HMACSHA256(key, hashSize); } - public static HMAC CreateHMACSHA384(byte[] key, bool etm) + public static System.Security.Cryptography.HMACSHA384 CreateHMACSHA384(byte[] key) { - return new HMAC(new System.Security.Cryptography.HMACSHA384(key), etm); + return new System.Security.Cryptography.HMACSHA384(key); } - public static HMAC CreateHMACSHA384(byte[] key, int hashSize, bool etm) + public static HMACSHA384 CreateHMACSHA384(byte[] key, int hashSize) { - return new HMAC(new HMACSHA384(key, hashSize), etm); + return new HMACSHA384(key, hashSize); } - public static HMAC CreateHMACSHA512(byte[] key, bool etm) + public static System.Security.Cryptography.HMACSHA512 CreateHMACSHA512(byte[] key) { - return new HMAC(new System.Security.Cryptography.HMACSHA512(key), etm); + return new System.Security.Cryptography.HMACSHA512(key); } - public static HMAC CreateHMACSHA512(byte[] key, int hashSize, bool etm) + public static HMACSHA512 CreateHMACSHA512(byte[] key, int hashSize) { - return new HMAC(new HMACSHA512(key, hashSize), etm); + return new HMACSHA512(key, hashSize); } #if FEATURE_HMAC_RIPEMD160 - public static HMAC CreateHMACRIPEMD160(byte[] key, bool etm) + public static System.Security.Cryptography.HMACRIPEMD160 CreateHMACRIPEMD160(byte[] key) { #pragma warning disable CA5350 // Do not use weak cryptographic algorithms - return new HMAC(new System.Security.Cryptography.HMACRIPEMD160(key), etm); + return new System.Security.Cryptography.HMACRIPEMD160(key); #pragma warning restore CA5350 // Do not use weak cryptographic algorithms } #else - public static HMAC CreateHMACRIPEMD160(byte[] key, bool etm) + public static global::SshNet.Security.Cryptography.HMACRIPEMD160 CreateHMACRIPEMD160(byte[] key) { - return new HMAC(new global::SshNet.Security.Cryptography.HMACRIPEMD160(key), etm); + return new global::SshNet.Security.Cryptography.HMACRIPEMD160(key); } #endif // FEATURE_HMAC_RIPEMD160 } diff --git a/src/Renci.SshNet/ConnectionInfo.cs b/src/Renci.SshNet/ConnectionInfo.cs index c55d1108c..d57c08860 100644 --- a/src/Renci.SshNet/ConnectionInfo.cs +++ b/src/Renci.SshNet/ConnectionInfo.cs @@ -377,23 +377,23 @@ public ConnectionInfo(string host, int port, string username, ProxyTypes proxyTy HmacAlgorithms = new Dictionary { /* Encrypt-and-MAC (encrypt-and-authenticate) variants */ - { "hmac-sha2-256", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, etm: false)) }, - { "hmac-sha2-512", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: false)) }, - { "hmac-sha2-512-96", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, 96, etm: false)) }, - { "hmac-sha2-256-96", new HashInfo(32*8, key => CryptoAbstraction.CreateHMACSHA256(key, 96, etm: false)) }, - { "hmac-ripemd160", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key, etm: false)) }, - { "hmac-ripemd160@openssh.com", new HashInfo(160, key => CryptoAbstraction.CreateHMACRIPEMD160(key, etm: false)) }, - { "hmac-sha1", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, etm: false)) }, - { "hmac-sha1-96", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96, etm: false)) }, - { "hmac-md5", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, etm: false)) }, - { "hmac-md5-96", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96, etm: false)) }, + { "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, etm: true)) }, - { "hmac-sha2-512-etm@openssh.com", new HashInfo(64*8, key => CryptoAbstraction.CreateHMACSHA512(key, etm: true)) }, - { "hmac-sha1-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, etm: true)) }, - { "hmac-sha1-96-etm@openssh.com", new HashInfo(20*8, key => CryptoAbstraction.CreateHMACSHA1(key, 96, etm: true)) }, - { "hmac-md5-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, etm: true)) }, - { "hmac-md5-96-etm@openssh.com", new HashInfo(16*8, key => CryptoAbstraction.CreateHMACMD5(key, 96, etm: true)) }, + { "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 63015036a..5412f0764 100644 --- a/src/Renci.SshNet/HashInfo.cs +++ b/src/Renci.SshNet/HashInfo.cs @@ -1,6 +1,7 @@ using System; +using System.Security.Cryptography; + using Renci.SshNet.Common; -using Renci.SshNet.Security.Cryptography; namespace Renci.SshNet { @@ -17,20 +18,30 @@ 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. /// - public Func HMAC { get; private set; } + public Func HashAlgorithm { get; private set; } /// /// Initializes a new instance of the class. /// /// 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; - HMAC = key => hash(key.Take(KeySize / 8)); + 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 99e9077dd..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, bool etm = false) + internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor, bool isEncryptThenMAC = false) { const int outboundPacketSequenceSize = 4; @@ -80,7 +80,7 @@ internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor, bool et // determine the padding length // 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, etm ? packetLength - 4 : packetLength); + var paddingLength = GetPaddingLength(paddingMultiplier, isEncryptThenMAC ? packetLength - 4 : packetLength); // add padding bytes var paddingBytes = new byte[paddingLength]; @@ -108,7 +108,7 @@ internal byte[] GetPacket(byte paddingMultiplier, Compressor compressor, bool et // determine the padding length // 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, etm ? packetLength - 4 : packetLength); + var paddingLength = GetPaddingLength(paddingMultiplier, isEncryptThenMAC ? packetLength - 4 : packetLength); var packetDataLength = GetPacketDataLength(messageLength, paddingLength); diff --git a/src/Renci.SshNet/Security/Cryptography/HMAC.cs b/src/Renci.SshNet/Security/Cryptography/HMAC.cs deleted file mode 100644 index 764ba0f49..000000000 --- a/src/Renci.SshNet/Security/Cryptography/HMAC.cs +++ /dev/null @@ -1,40 +0,0 @@ -using System; -using System.Security.Cryptography; - -namespace Renci.SshNet.Security.Cryptography -{ - /// - /// Represents the info for Message Authentication Code (MAC). - /// - public sealed class HMAC : IDisposable - { - /// - /// Initializes a new instance of the class. - /// - /// The hash algorithm. - /// to enable encrypt-then-MAC, to use encrypt-and-MAC. - public HMAC( - HashAlgorithm hashAlgorithm, - bool etm) - { - HashAlgorithm = hashAlgorithm; - ETM = etm; - } - - /// - public void Dispose() - { - HashAlgorithm?.Dispose(); - } - - /// - /// Gets the hash algorithem. - /// - public HashAlgorithm HashAlgorithm { get; private set; } - - /// - /// Gets a value indicating whether enable encryption-to-mac or encryption-then-mac. - /// - public bool ETM { get; private set; } - } -} diff --git a/src/Renci.SshNet/Security/IKeyExchange.cs b/src/Renci.SshNet/Security/IKeyExchange.cs index 545a5f872..c8f04b219 100644 --- a/src/Renci.SshNet/Security/IKeyExchange.cs +++ b/src/Renci.SshNet/Security/IKeyExchange.cs @@ -1,4 +1,5 @@ using System; +using System.Security.Cryptography; using Renci.SshNet.Common; using Renci.SshNet.Compression; @@ -65,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. /// - HMAC 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. /// - HMAC 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 d1544ad33..10f7e0f8a 100644 --- a/src/Renci.SshNet/Security/KeyExchange.cs +++ b/src/Renci.SshNet/Security/KeyExchange.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; using Renci.SshNet.Abstractions; using Renci.SshNet.Common; @@ -217,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 HMAC CreateServerHash() + public HashAlgorithm CreateServerHash(out bool isEncryptThenMAC) { + isEncryptThenMAC = _serverHashInfo.IsEncryptThenMAC; + // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; @@ -234,17 +238,20 @@ public HMAC CreateServerHash() Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentServerHmacAlgorithm)); - return _serverHashInfo.HMAC(serverKey); + return _serverHashInfo.HashAlgorithm(serverKey); } /// /// Creates the client side hash algorithm to use. /// + /// to enable encrypt-then-MAC, to use encrypt-and-MAC. /// /// The client-side hash algorithm. /// - public HMAC CreateClientHash() + public HashAlgorithm CreateClientHash(out bool isEncryptThenMAC) { + isEncryptThenMAC = _clientHashInfo.IsEncryptThenMAC; + // Resolve Session ID var sessionId = Session.SessionId ?? ExchangeHash; @@ -257,7 +264,7 @@ public HMAC CreateClientHash() Session.ToHex(Session.SessionId), Session.ConnectionInfo.CurrentClientHmacAlgorithm)); - return _clientHashInfo.HMAC(clientKey); + return _clientHashInfo.HashAlgorithm(clientKey); } /// diff --git a/src/Renci.SshNet/Session.cs b/src/Renci.SshNet/Session.cs index c8d6d87b6..37b7f2db3 100644 --- a/src/Renci.SshNet/Session.cs +++ b/src/Renci.SshNet/Session.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Linq; using System.Net.Sockets; +using System.Security.Cryptography; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -171,9 +172,13 @@ public class Session : ISession private IKeyExchange _keyExchange; - private HMAC _serverMac; + private HashAlgorithm _serverMac; - private HMAC _clientMac; + private HashAlgorithm _clientMac; + + private bool _serverEtm; + + private bool _clientEtm; private Cipher _clientCipher; @@ -1053,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, _clientMac != null && _clientMac.ETM); + 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 @@ -1062,19 +1067,19 @@ internal void SendMessage(Message message) byte[] hash = null; var packetDataOffset = 4; // first four bytes are reserved for outbound packet sequence - if (_clientMac != null && !_clientMac.ETM) + if (_clientMac != null && !_clientEtm) { // write outbound packet sequence to start of packet data Pack.UInt32ToBigEndian(_outboundPacketSequence, packetData); // calculate packet hash - hash = _clientMac.HashAlgorithm.ComputeHash(packetData); + hash = _clientMac.ComputeHash(packetData); } // Encrypt packet data if (_clientCipher != null) { - if (_clientMac != null && _clientMac.ETM) + if (_clientMac != null && _clientEtm) { // The length of the "packet length" field in bytes const int packetLengthFieldLength = 4; @@ -1090,7 +1095,7 @@ internal void SendMessage(Message message) Buffer.BlockCopy(encryptedData, 0, packetData, packetDataOffset + packetLengthFieldLength, encryptedData.Length); // calculate packet hash - hash = _clientMac.HashAlgorithm.ComputeHash(packetData); + hash = _clientMac.ComputeHash(packetData); } else { @@ -1218,7 +1223,7 @@ private Message ReceiveMessage(Socket socket) // 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 && _serverMac.ETM) + if (_serverMac != null && _serverEtm) { blockSize = (byte) 4; } @@ -1231,7 +1236,7 @@ private Message ReceiveMessage(Socket socket) blockSize = (byte) 8; } - var serverMacLength = _serverMac != null ? _serverMac.HashAlgorithm.HashSize/8 : 0; + var serverMacLength = _serverMac != null ? _serverMac.HashSize/8 : 0; byte[] data; uint packetLength; @@ -1249,7 +1254,7 @@ private Message ReceiveMessage(Socket socket) return null; } - if (_serverCipher != null && (_serverMac == null || !_serverMac.ETM)) + if (_serverCipher != null && (_serverMac == null || !_serverEtm)) { firstBlock = _serverCipher.Decrypt(firstBlock); } @@ -1292,9 +1297,9 @@ private Message ReceiveMessage(Socket socket) } // validate encrypted message against MAC - if (_serverMac != null && _serverMac.ETM) + if (_serverMac != null && _serverEtm) { - var clientHash = _serverMac.HashAlgorithm.ComputeHash(data, 0, data.Length - serverMacLength); + 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. @@ -1320,9 +1325,9 @@ private Message ReceiveMessage(Socket socket) var messagePayloadOffset = inboundPacketSequenceLength + packetLengthFieldLength + paddingLengthFieldLength; // validate decrypted message against MAC - if (_serverMac != null && !_serverMac.ETM) + if (_serverMac != null && !_serverEtm) { - var clientHash = _serverMac.HashAlgorithm.ComputeHash(data, 0, data.Length - serverMacLength); + 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. @@ -1520,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.Tests/Classes/SessionTest_ConnectedBase.cs b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs index 3ed966253..5ce0d4675 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_ConnectedBase.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Net; using System.Net.Sockets; +using System.Security.Cryptography; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -214,10 +215,18 @@ private void SetupMocks() .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); - _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HMAC) null); - _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HMAC) 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 7e4fed4fc..c75fa32a3 100644 --- a/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs +++ b/test/Renci.SshNet.Tests/Classes/SessionTest_Connected_ServerAndClientDisconnectRace.cs @@ -3,6 +3,8 @@ using System.Globalization; using System.Net; using System.Net.Sockets; +using System.Security.Cryptography; + using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using Renci.SshNet.Common; @@ -162,10 +164,18 @@ private void SetupMocks() .Returns((Cipher) null); _ = _keyExchangeMock.Setup(p => p.CreateClientCipher()) .Returns((Cipher) null); - _ = _keyExchangeMock.Setup(p => p.CreateServerHash()) - .Returns((HMAC) null); - _ = _keyExchangeMock.Setup(p => p.CreateClientHash()) - .Returns((HMAC) 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())