Implement OpenSSH strict kex extension for mitigating Terrapin attack. (

#263)

src/Tmds.Ssh/AlgorithmNames.cs

@@ -86,4 +86,9 @@ static class AlgorithmNames // TODO: rename to KnownNames
     private static readonly byte[] PublicKeyBytes = "publickey"u8.ToArray();
     public static Name PublicKey => new Name(PublicKeyBytes);
 
+    // Strict key exchange
+    private static readonly byte[] ClientStrictKexBytes = "kex-strict-c-v00@openssh.com"u8.ToArray();
+    public static Name ClientStrictKex => new Name(ClientStrictKexBytes);
+    private static readonly byte[] PServerStrictKexBytes = "kex-strict-s-v00@openssh.com"u8.ToArray();
+    public static Name ServerStrictKex => new Name(PServerStrictKexBytes);
 }

src/Tmds.Ssh/KeyExchangeContext.cs

@@ -7,13 +7,12 @@ sealed class KeyExchangeContext
 {
     private readonly SshConnection _connection;
     private readonly SshSession _session;
-    private readonly bool _isInitialKex;
 
     public KeyExchangeContext(SshConnection connection, SshSession session, bool isInitialKex = true)
     {
         _connection = connection;
         _session = session;
-        _isInitialKex = isInitialKex;
+        IsInitialKex = isInitialKex;
     }
 
     public SequencePool SequencePool => _connection.SequencePool;
@@ -60,7 +59,7 @@ private bool CheckPacketForReturn(MessageId expected, Packet packet)
         {
             return true;
         }
-        else if (_isInitialKex)
+        else if (IsInitialKex)
         {
             // During the initial kex, only permit the strictly required kex packets.
             packet.Dispose();
@@ -77,8 +76,8 @@ private bool CheckPacketForReturn(MessageId expected, Packet packet)
     public ValueTask SendPacketAsync(Packet packet, CancellationToken ct)
         => _connection.SendPacketAsync(packet, ct);
 
-    public void SetEncryptorDecryptor(IPacketEncryptor encryptor, IPacketDecryptor decryptor)
-        => _connection.SetEncryptorDecryptor(encryptor, decryptor);
+    public void SetEncryptorDecryptor(IPacketEncryptor encryptor, IPacketDecryptor decryptor, bool resetSequenceNumbers, bool throwIfReceiveSNZero)
+        => _connection.SetEncryptorDecryptor(encryptor, decryptor, resetSequenceNumbers, throwIfReceiveSNZero);
 
     public required List<Name> KeyExchangeAlgorithms { get; init; }
     public required List<Name> ServerHostKeyAlgorithms { get; init; }
@@ -92,4 +91,9 @@ public void SetEncryptorDecryptor(IPacketEncryptor encryptor, IPacketDecryptor d
     public required List<Name> LanguagesServerToClient { get; init; }
     public required IHostKeyVerification HostKeyVerification { get; init; }
     public required int MinimumRSAKeySize { get; init; }
+    public bool IsInitialKex { get; }
+
+    // Unconditionally enable strict key exchange on the first key exchange as described in https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.
+    // This mitigates the Terrapin attack (CVE-2023-48795, https://terrapin-attack.com/).
+    public bool NegotiateStrictKex => IsInitialKex;
 }

src/Tmds.Ssh/SocketSshConnection.cs

@@ -212,12 +212,22 @@ public override async ValueTask WriteLineAsync(string line, CancellationToken ct
         await _socket.SendAsync(Encoding.UTF8.GetBytes(line), SocketFlags.None, ct).ConfigureAwait(false);
     }
 
-    public override void SetEncryptorDecryptor(IPacketEncryptor packetEncoder, IPacketDecryptor packetDecoder)
+    public override void SetEncryptorDecryptor(IPacketEncryptor packetEncoder, IPacketDecryptor packetDecoder, bool resetSequenceNumbers, bool throwIfReceiveSNZero)
     {
         _encryptor?.Dispose();
         _decryptor?.Dispose();
         _encryptor = packetEncoder;
         _decryptor = packetDecoder;
+
+        if (resetSequenceNumbers)
+        {
+            if (_receiveSequenceNumber == 0 && throwIfReceiveSNZero)
+            {
+                ThrowHelper.ThrowProtocolValueOutOfRange();
+            }
+            _sendSequenceNumber = 0;
+            _receiveSequenceNumber = 0;
+        }
     }
 
     public override void Dispose()

src/Tmds.Ssh/SshClient.cs

@@ -351,4 +351,14 @@ internal void ForceConnectionClose()
         Debug.Assert(_session is not null);
         _session.ForceConnectionClose();
     }
+
+    // For testing.
+    internal SshConnectionInfo ConnectionInfo
+    {
+        get
+        {
+            Debug.Assert(_session is not null);
+            return _session.ConnectionInfo;
+        }
+    }
 }

src/Tmds.Ssh/SshConnection.cs

@@ -23,5 +23,5 @@ protected SshConnection(SequencePool sequencePool)
     public abstract ValueTask<Packet> ReceivePacketAsync(CancellationToken ct, int maxLength = Constants.PreAuthMaxPacketLength);
     public abstract ValueTask SendPacketAsync(Packet packet, CancellationToken ct);
     public abstract void Dispose();
-    public abstract void SetEncryptorDecryptor(IPacketEncryptor packetEncoder, IPacketDecryptor packetDecoder);
+    public abstract void SetEncryptorDecryptor(IPacketEncryptor packetEncoder, IPacketDecryptor packetDecoder, bool resetSequenceNumbers, bool throwIfReceiveSNZero);
 }

src/Tmds.Ssh/SshConnectionInfo.cs

@@ -12,7 +12,7 @@ internal SshConnectionInfo() { }
     public string HostName { get; internal set; } = string.Empty;
     public int Port { get; internal set; }
 
-    // Managed
+    internal bool UseStrictKex { get; set; }
     internal byte[]? SessionId { get; set; }
     internal string? ClientIdentificationString { get; set; }
     internal string? ServerIdentificationString { get; set; }

src/Tmds.Ssh/SshSession.KeyExchange.cs

@@ -153,7 +153,18 @@ private async Task PerformKeyExchangeAsync(KeyExchangeContext context, ReadOnlyP
         IPacketEncryptor encryptor = encC2SAlg.CreatePacketEncryptor(keyExchangeOutput.EncryptionKeyC2S, keyExchangeOutput.InitialIVC2S, hmacC2SAlg, keyExchangeOutput.IntegrityKeyC2S);
         IPacketDecryptor decryptor = encS2CAlg.CreatePacketDecryptor(sequencePool, keyExchangeOutput.EncryptionKeyS2C, keyExchangeOutput.InitialIVS2C, hmacS2CAlg, keyExchangeOutput.IntegrityKeyS2C);
 
-        context.SetEncryptorDecryptor(encryptor, decryptor);
+        // Strict key exchange.
+        if (context.NegotiateStrictKex)
+        {
+            ConnectionInfo.UseStrictKex = remoteInit.kex_algorithms.Contains(AlgorithmNames.ServerStrictKex);
+        }
+        bool resetSequenceNumbers = ConnectionInfo.UseStrictKex;
+        // The sequence numbers get reset to ensure the next message is the first message that was encrypted by the server.
+        // This checks that a MitM did not compromise the sequence numbers by injecting/dropping messages.
+        // If we are already at zero on the first kex then the sequence numbers were compromised.
+        bool throwIfReceiveSNZero = resetSequenceNumbers && context.IsInitialKex;
+
+        context.SetEncryptorDecryptor(encryptor, decryptor, resetSequenceNumbers, throwIfReceiveSNZero);
 
         static Name ChooseAlgorithm(List<Name> localList, Name[] remoteList)
         {
@@ -312,11 +323,16 @@ private TrustedHostKeys GetKnownHostKeys()
 
     private Packet CreateKeyExchangeInitMessage(KeyExchangeContext context)
     {
+        List<Name> kexAlgorithms = context.KeyExchangeAlgorithms;
+        if (context.NegotiateStrictKex)
+        {
+            kexAlgorithms = [..kexAlgorithms, AlgorithmNames.ClientStrictKex];
+        }
         using var packet = _sequencePool.RentPacket();
         var writer = packet.GetWriter();
         writer.WriteMessageId(MessageId.SSH_MSG_KEXINIT);
         writer.WriteRandomBytes(16);
-        writer.WriteNameList(context.KeyExchangeAlgorithms);
+        writer.WriteNameList(kexAlgorithms);
         writer.WriteNameList(context.ServerHostKeyAlgorithms);
         writer.WriteNameList(context.EncryptionAlgorithmsClientToServer);
         writer.WriteNameList(context.EncryptionAlgorithmsServerToClient);
@@ -330,7 +346,7 @@ private Packet CreateKeyExchangeInitMessage(KeyExchangeContext context)
         writer.WriteUInt32(0);
 
         Logger.ClientKexInit(
-            context.KeyExchangeAlgorithms,
+            kexAlgorithms,
             context.ServerHostKeyAlgorithms,
             context.EncryptionAlgorithmsClientToServer,
             context.EncryptionAlgorithmsServerToClient,

test/Tmds.Ssh.Tests/ConnectTests.cs

@@ -17,7 +17,9 @@ public ConnectTests(SshServer sshServer)
     [Fact]
     public async Task ConnectSuccess()
     {
-        using var _ = await _sshServer.CreateClientAsync();
+        using var client = await _sshServer.CreateClientAsync();
+
+        Assert.True(client.ConnectionInfo.UseStrictKex);
     }
 
     [Fact]