From 12984cca3077ecdd32e27b730ba69d5a72ad624e Mon Sep 17 00:00:00 2001 From: Scott Xu Date: Mon, 12 Feb 2024 21:15:25 +0800 Subject: [PATCH] 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())