CVE-2023-48795: Improper Validation of Integrity Check Value (CWE-354)
value: The SSH transport protocol with certain OpenSSH extensions, found in OpenSSH before 9.6 and other products, allows remote attackers to bypass integrity checks such that some packets are omitted (from the extension negotiation message), and a client and server may consequently end up with a connection for which some security features have been downgraded or disabled, aka a Terrapin attack. This occurs because the SSH Binary Packet Protocol (BPP), implemented by these extensions, mishandles the handshake phase and mishandles use of sequence numbers. For example, there is an effective attack against SSH's use of ChaCha20-Poly1305 (and CBC with Encrypt-then-MAC). The bypass occurs in chacha20-poly1305@openssh.com and (if CBC is used) the -etm@openssh.com MAC algorithms. This also affects Maverick Synergy Java SSH API before 3.1.0-SNAPSHOT, Dropbear through 2022.83, Ssh before 5.1.1 in Erlang/OTP, PuTTY before 0.80, AsyncSSH before 2.14.2, golang.org/x/crypto before 0.17.0, libssh before 0.10.6, libssh2 through 1.11.0, Thorn Tech SFTP Gateway before 3.4.6, Tera Term before 5.1, Paramiko before 3.4.0, jsch before 0.2.15, SFTPGo before 2.5.6, Netgate pfSense Plus through 23.09.1, Netgate pfSense CE through 2.7.2, HPN-SSH through 18.2.0, ProFTPD before 1.3.8b (and before 1.3.9rc2), ORYX CycloneSSH before 2.3.4, NetSarang XShell 7 before Build 0144, CrushFTP before 10.6.0, ConnectBot SSH library before 2.2.22, Apache MINA sshd through 2.11.0, sshj through 0.37.0, TinySSH through 20230101, trilead-ssh2 6401, LANCOM LCOS and LANconfig, FileZilla before 3.66.4, Nova before 11.8, PKIX-SSH before 14.4, SecureCRT before 9.4.3, Transmit5 before 5.10.4, Win32-OpenSSH before 9.5.0.0p1-Beta, WinSCP before 6.2.2, Bitvise SSH Server before 9.32, Bitvise SSH Client before 9.33, KiTTY through 0.76.1.13, the net-ssh gem 7.2.0 for Ruby, the mscdex ssh2 module before 1.15.0 for Node.js, the thrussh library before 0.35.1 for Rust, and the Russh crate before 0.40.2 for Rust.
CWE description: The product does not validate or incorrectly validates the integrity check values or "checksums" of a message. This may prevent it from detecting if the data has been modified or corrupted in transmission.
CVSS v3 base score: 5.9
Published: 2023-12-18T16:15Z
Repository: https://github.com/connectbot/sshlib
Language: Java
Files changed: src/main/java/com/trilead/ssh2/transport/KexManager.java, src/main/java/com/trilead/ssh2/transport/NegotiatedParameters.java, src/main/java/com/trilead/ssh2/transport/TransportConnection.java, src/main/java/com/trilead/ssh2/transport/TransportManager.java
Commit date: 2023-12-19 00:14:04 -0800
Commit message: Implement kex-strict from OpenSSH

Implement's OpenSSH's mitigation for the Terrapin attack.

Diff: diff --git a/src/main/java/com/trilead/ssh2/transport/KexManager.java b/src/main/java/com/trilead/ssh2/transport/KexManager.java
index fd7cc38..1fe6354 100644
--- a/src/main/java/com/trilead/ssh2/transport/KexManager.java
+++ b/src/main/java/com/trilead/ssh2/transport/KexManager.java
@@ -101,10 +101,13 @@ public class KexManager
 	}
 
 	/** RFC 8308 Section 2 */
 	private static final String EXT_INFO_C = "ext-info-c";
 
+	private static final String KEX_STRICT_C_OPENSSH = "kex-strict-c-v00@openssh.com";
+	private static final String KEX_STRICT_S_OPENSSH = "kex-strict-s-v00@openssh.com";
+
 	private KexState kxs;
 	private int kexCount = 0;
 	private KeyMaterial km;
 	byte[] sessionId;
 	private ClientServerHello csh;
@@ -191,10 +194,23 @@ public class KexManager
 			return false;
 
 		return (a[0].equals(b[0]));
 	}
 
+	private boolean containsAlgo(String[] algos, String targetAlgo)
+	{
+		if (algos == null || targetAlgo == null)
+			return false;
+
+		for (String algo : algos) {
+			if (targetAlgo.equals(algo))
+				return true;
+		}
+
+		return false;
+	}
+
 	private boolean isGuessOK(KexParameters cpar, KexParameters spar)
 	{
 		if (cpar == null || spar == null)
 			throw new IllegalArgumentException();
 
@@ -212,10 +228,12 @@ public class KexManager
 
 		try
 		{
 			np.kex_algo = getFirstMatch(client.kex_algorithms, server.kex_algorithms);
 
+			np.isStrictKex = containsAlgo(server.kex_algorithms, KEX_STRICT_S_OPENSSH);
+
 			log.log(20, "kex_algo=" + np.kex_algo);
 
 			np.server_host_key_algo = getFirstMatch(client.server_host_key_algorithms,
 					server.server_host_key_algorithms);
 
@@ -302,17 +320,18 @@ public class KexManager
 	 * @param cwl the crypto wishlist to which the key exchange algo
 	 *            should be added.
 	 */
 	private static void addExtraKexAlgorithms(CryptoWishList cwl) {
 		String[] oldKexAlgorithms = cwl.kexAlgorithms;
-		List<String> kexAlgorithms = new ArrayList<>(oldKexAlgorithms.length + 1);
+		List<String> kexAlgorithms = new ArrayList<>(oldKexAlgorithms.length + 2);
 		for (String algo : oldKexAlgorithms)
 		{
-			if (!algo.equals(EXT_INFO_C))
+			if (!algo.equals(EXT_INFO_C) && !algo.equals(KEX_STRICT_C_OPENSSH))
 				kexAlgorithms.add(algo);
 		}
 		kexAlgorithms.add(EXT_INFO_C);
+		kexAlgorithms.add(KEX_STRICT_C_OPENSSH);
 		cwl.kexAlgorithms = kexAlgorithms.toArray(new String[0]);
 	}
 
 	/**
 	 * If the verifier can indicate which algorithms it knows about for this host, then
@@ -744,6 +763,10 @@ public class KexManager
 			}
 		}
 
 		throw new IllegalStateException("Unkown KEX method! (" + kxs.np.kex_algo + ")");
 	}
+
+	public boolean isStrictKex() {
+		return kxs.np.isStrictKex;
+	}
 }
diff --git a/src/main/java/com/trilead/ssh2/transport/NegotiatedParameters.java b/src/main/java/com/trilead/ssh2/transport/NegotiatedParameters.java
index 34ed34a..a3d7bc8 100644
--- a/src/main/java/com/trilead/ssh2/transport/NegotiatedParameters.java
+++ b/src/main/java/com/trilead/ssh2/transport/NegotiatedParameters.java
@@ -7,10 +7,11 @@ package com.trilead.ssh2.transport;
  * @version $Id: NegotiatedParameters.java,v 1.1 2007/10/15 12:49:57 cplattne Exp $
  */
 public class NegotiatedParameters
 {
 	public boolean guessOK;
+	public boolean isStrictKex;
 	public String kex_algo;
 	public String server_host_key_algo;
 	public String enc_algo_client_to_server;
 	public String enc_algo_server_to_client;
 	public String mac_algo_client_to_server;
diff --git a/src/main/java/com/trilead/ssh2/transport/TransportConnection.java b/src/main/java/com/trilead/ssh2/transport/TransportConnection.java
index 19f9cd7..18be7db 100644
--- a/src/main/java/com/trilead/ssh2/transport/TransportConnection.java
+++ b/src/main/java/com/trilead/ssh2/transport/TransportConnection.java
@@ -359,6 +359,20 @@ public class TransportConnection
 	 */
 	public void startCompression() {
 		can_recv_compress = true;
 		can_send_compress = true;
 	}
+
+	/**
+	 * Resets the send sequence number for MAC calculation.
+	 */
+	public void resetSendSequenceNumber() {
+		send_seq_number = 0;
+	}
+
+	/**
+	 * Resets the receive sequence number for MAC calculation.
+	 */
+	public void resetReceiveSequenceNumber() {
+		recv_seq_number = 0;
+	}
 }
diff --git a/src/main/java/com/trilead/ssh2/transport/TransportManager.java b/src/main/java/com/trilead/ssh2/transport/TransportManager.java
index ce33608..4bdb4f6 100644
--- a/src/main/java/com/trilead/ssh2/transport/TransportManager.java
+++ b/src/main/java/com/trilead/ssh2/transport/TransportManager.java
@@ -120,14 +120,15 @@ public class TransportManager
 
 	String hostname;
 	int port;
 	Socket sock;
 
-	Object connectionSemaphore = new Object();
+	private final Object connectionSemaphore = new Object();
 
 	boolean flagKexOngoing = false;
 	boolean connectionClosed = false;
+	boolean firstKexFinished = false;
 
 	Throwable reasonClosedCause = null;
 
 	TransportConnection tc;
 	KexManager km;
@@ -402,10 +403,12 @@ public class TransportManager
 			}
 		}
 	}
 
 	public void kexFinished() {
+		firstKexFinished = true;
+
 		synchronized (connectionSemaphore)
 		{
 			flagKexOngoing = false;
 			connectionSemaphore.notifyAll();
 		}
@@ -417,15 +420,19 @@ public class TransportManager
 	}
 
 	public void changeRecvCipher(BlockCipher bc, MAC mac)
 	{
 		tc.changeRecvCipher(bc, mac);
+		if (km.isStrictKex())
+			tc.resetReceiveSequenceNumber();
 	}
 
 	public void changeSendCipher(BlockCipher bc, MAC mac)
 	{
 		tc.changeSendCipher(bc, mac);
+		if (km.isStrictKex())
+			tc.resetSendSequenceNumber();
 	}
 
 	/**
 	 * @param comp
 	 */
@@ -529,42 +536,10 @@ public class TransportManager
 		{
 			int msglen = tc.receiveMessage(msg, 0, msg.length);
 
 			int type = msg[0] & 0xff;
 
-			if (type == Packets.SSH_MSG_IGNORE)
-				continue;
-
-			if (type == Packets.SSH_MSG_DEBUG)
-			{
-				if (log.isEnabled())
-				{
-					TypesReader tr = new TypesReader(msg, 0, msglen);
-					tr.readByte();
-					tr.readBoolean();
-					StringBuffer debugMessageBuffer = new StringBuffer();
-					debugMessageBuffer.append(tr.readString("UTF-8"));
-
-					for (int i = 0; i < debugMessageBuffer.length(); i++)
-					{
-						char c = debugMessageBuffer.charAt(i);
-
-						if ((c >= 32) && (c <= 126))
-							continue;
-						debugMessageBuffer.setCharAt(i, '\uFFFD');
-					}
-
-					log.log(50, "DEBUG Message from remote: '" + debugMessageBuffer.toString() + "'");
-				}
-				continue;
-			}
-
-			if (type == Packets.SSH_MSG_UNIMPLEMENTED)
-			{
-				throw new IOException("Peer sent UNIMPLEMENTED message, that should not happen.");
-			}
-
 			if (type == Packets.SSH_MSG_DISCONNECT)
 			{
 				TypesReader tr = new TypesReader(msg, 0, msglen);
 				tr.readByte();
 				int reason_code = tr.readUINT32();
@@ -613,10 +588,50 @@ public class TransportManager
 			{
 				km.handleMessage(msg, msglen);
 				continue;
 			}
 
+			/*
+			 * Any other packet should not be used when kex-strict is enabled.
+			 */
+			if (!firstKexFinished && km.isStrictKex())
+			{
+				throw new IOException("Unexpected packet received when kex-strict enabled");
+			}
+
+			if (type == Packets.SSH_MSG_IGNORE)
+				continue;
+
+			if (type == Packets.SSH_MSG_DEBUG)
+			{
+				if (log.isEnabled())
+				{
+					TypesReader tr = new TypesReader(msg, 0, msglen);
+					tr.readByte();
+					tr.readBoolean();
+					StringBuffer debugMessageBuffer = new StringBuffer();
+					debugMessageBuffer.append(tr.readString("UTF-8"));
+
+					for (int i = 0; i < debugMessageBuffer.length(); i++)
+					{
+						char c = debugMessageBuffer.charAt(i);
+
+						if ((c >= 32) && (c <= 126))
+							continue;
+						debugMessageBuffer.setCharAt(i, '\uFFFD');
+					}
+
+					log.log(50, "DEBUG Message from remote: '" + debugMessageBuffer.toString() + "'");
+				}
+				continue;
+			}
+
+			if (type == Packets.SSH_MSG_UNIMPLEMENTED)
+			{
+				throw new IOException("Peer sent UNIMPLEMENTED message, that should not happen.");
+			}
+
 			if (type == Packets.SSH_MSG_USERAUTH_SUCCESS) {
 				tc.startCompression();
 			}
 
 			if (type == Packets.SSH_MSG_EXT_INFO) {

Vulnerable code: 		List<String> kexAlgorithms = new ArrayList<>(oldKexAlgorithms.length + 1);
			if (!algo.equals(EXT_INFO_C))
	Object connectionSemaphore = new Object();
			if (type == Packets.SSH_MSG_IGNORE)
				continue;

			if (type == Packets.SSH_MSG_DEBUG)
			{
				if (log.isEnabled())
				{
					TypesReader tr = new TypesReader(msg, 0, msglen);
					tr.readByte();
					tr.readBoolean();
					StringBuffer debugMessageBuffer = new StringBuffer();
					debugMessageBuffer.append(tr.readString("UTF-8"));

					for (int i = 0; i < debugMessageBuffer.length(); i++)
					{
						char c = debugMessageBuffer.charAt(i);

						if ((c >= 32) && (c <= 126))
							continue;
						debugMessageBuffer.setCharAt(i, '\uFFFD');
					}

					log.log(50, "DEBUG Message from remote: '" + debugMessageBuffer.toString() + "'");
				}
				continue;
			}

			if (type == Packets.SSH_MSG_UNIMPLEMENTED)
			{
				throw new IOException("Peer sent UNIMPLEMENTED message, that should not happen.");
			}

Fixed code: 	private static final String KEX_STRICT_C_OPENSSH = "kex-strict-c-v00@openssh.com";
	private static final String KEX_STRICT_S_OPENSSH = "kex-strict-s-v00@openssh.com";

	private boolean containsAlgo(String[] algos, String targetAlgo)
	{
		if (algos == null || targetAlgo == null)
			return false;

		for (String algo : algos) {
			if (targetAlgo.equals(algo))
				return true;
		}

		return false;
	}

			np.isStrictKex = containsAlgo(server.kex_algorithms, KEX_STRICT_S_OPENSSH);

		List<String> kexAlgorithms = new ArrayList<>(oldKexAlgorithms.length + 2);
			if (!algo.equals(EXT_INFO_C) && !algo.equals(KEX_STRICT_C_OPENSSH))
		kexAlgorithms.add(KEX_STRICT_C_OPENSSH);

	public boolean isStrictKex() {
		return kxs.np.isStrictKex;
	}
	public boolean isStrictKex;

	/**
	 * Resets the send sequence number for MAC calculation.
	 */
	public void resetSendSequenceNumber() {
		send_seq_number = 0;
	}

	/**
	 * Resets the receive sequence number for MAC calculation.
	 */
	public void resetReceiveSequenceNumber() {
		recv_seq_number = 0;
	}
	private final Object connectionSemaphore = new Object();
	boolean firstKexFinished = false;
		firstKexFinished = true;

		if (km.isStrictKex())
			tc.resetReceiveSequenceNumber();
		if (km.isStrictKex())
			tc.resetSendSequenceNumber();
			/*
			 * Any other packet should not be used when kex-strict is enabled.
			 */
			if (!firstKexFinished && km.isStrictKex())
			{
				throw new IOException("Unexpected packet received when kex-strict enabled");
			}

			if (type == Packets.SSH_MSG_IGNORE)
				continue;

			if (type == Packets.SSH_MSG_DEBUG)
			{
				if (log.isEnabled())
				{
					TypesReader tr = new TypesReader(msg, 0, msglen);
					tr.readByte();
					tr.readBoolean();
					StringBuffer debugMessageBuffer = new StringBuffer();
					debugMessageBuffer.append(tr.readString("UTF-8"));

					for (int i = 0; i < debugMessageBuffer.length(); i++)
					{
						char c = debugMessageBuffer.charAt(i);

						if ((c >= 32) && (c <= 126))
							continue;
						debugMessageBuffer.setCharAt(i, '\uFFFD');
					}

					log.log(50, "DEBUG Message from remote: '" + debugMessageBuffer.toString() + "'");
				}
				continue;
			}

			if (type == Packets.SSH_MSG_UNIMPLEMENTED)
			{
				throw new IOException("Peer sent UNIMPLEMENTED message, that should not happen.");
			}