From a189305e656b283883436e1c3ad57b6cc857fb6f Mon Sep 17 00:00:00 2001 From: Jeremy Norris Date: Mon, 18 Dec 2023 14:00:02 -0600 Subject: [PATCH] #457 address CVE-2023-48795 by adding support for new strict key exchange extension. --- src/main/java/com/jcraft/jsch/JSch.java | 2 + .../jcraft/jsch/JSchStrictKexException.java | 39 ++++++++ src/main/java/com/jcraft/jsch/Session.java | 89 +++++++++++++++++- .../jsch/JSchAlgoNegoFailExceptionIT.java | 2 +- .../jcraft/jsch/JSchStrictKexExceptionIT.java | 91 +++++++++++++++++++ 5 files changed, 219 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/jcraft/jsch/JSchStrictKexException.java create mode 100644 src/test/java/com/jcraft/jsch/JSchStrictKexExceptionIT.java diff --git a/src/main/java/com/jcraft/jsch/JSch.java b/src/main/java/com/jcraft/jsch/JSch.java index c1335964..ae754305 100644 --- a/src/main/java/com/jcraft/jsch/JSch.java +++ b/src/main/java/com/jcraft/jsch/JSch.java @@ -45,6 +45,8 @@ public class JSch { "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,rsa-sha2-512,rsa-sha2-256")); config.put("prefer_known_host_key_types", Util.getSystemProperty("jsch.prefer_known_host_key_types", "yes")); + config.put("enable_strict_kex", Util.getSystemProperty("jsch.enable_strict_kex", "yes")); + config.put("require_strict_kex", Util.getSystemProperty("jsch.require_strict_kex", "no")); config.put("enable_server_sig_algs", Util.getSystemProperty("jsch.enable_server_sig_algs", "yes")); config.put("cipher.s2c", Util.getSystemProperty("jsch.cipher", diff --git a/src/main/java/com/jcraft/jsch/JSchStrictKexException.java b/src/main/java/com/jcraft/jsch/JSchStrictKexException.java new file mode 100644 index 00000000..3454c1d2 --- /dev/null +++ b/src/main/java/com/jcraft/jsch/JSchStrictKexException.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2002-2018 ymnk, JCraft,Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without modification, are permitted + * provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this list of conditions + * and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, this list of + * conditions and the following disclaimer in the documentation and/or other materials provided with + * the distribution. + * + * 3. The names of the authors may not be used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL JCRAFT, INC. OR ANY CONTRIBUTORS TO THIS SOFTWARE BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.jcraft.jsch; + +public class JSchStrictKexException extends JSchException { + private static final long serialVersionUID = -1L; + + JSchStrictKexException() { + super(); + } + + JSchStrictKexException(String s) { + super(s); + } +} diff --git a/src/main/java/com/jcraft/jsch/Session.java b/src/main/java/com/jcraft/jsch/Session.java index 963f43f9..ede905e5 100644 --- a/src/main/java/com/jcraft/jsch/Session.java +++ b/src/main/java/com/jcraft/jsch/Session.java @@ -117,6 +117,11 @@ public class Session { private volatile boolean isConnected = false; + private volatile boolean initialKex = true; + private volatile boolean doStrictKex = false; + private boolean enable_strict_kex = true; + private boolean require_strict_kex = false; + private volatile boolean isAuthed = false; private Thread connectThread = null; @@ -194,6 +199,7 @@ public void connect(int connectTimeout) throws JSchException { if (isConnected) { throw new JSchException("session is already connected"); } + initialKex = true; io = new IO(); if (random == null) { @@ -308,6 +314,8 @@ public void connect(int connectTimeout) throws JSchException { getLogger().log(Logger.INFO, "Local version string: " + Util.byte2str(V_C)); } + enable_strict_kex = getConfig("enable_strict_kex").equals("yes"); + require_strict_kex = getConfig("require_strict_kex").equals("yes"); send_kexinit(); buf = read(buf); @@ -365,6 +373,7 @@ public void connect(int connectTimeout) throws JSchException { } receive_newkeys(buf, kex); + initialKex = false; } else { in_kex = false; throw new JSchException("invalid protocol(newkyes): " + buf.getCommand()); @@ -565,6 +574,21 @@ private KeyExchange receive_kexinit(Buffer buf) throws Exception { } System.arraycopy(buf.buffer, buf.s, I_S, 0, I_S.length); + if ((enable_strict_kex || require_strict_kex) && initialKex) { + doStrictKex = checkServerStrictKex(); + if (doStrictKex) { + if (getLogger().isEnabled(Logger.INFO)) { + getLogger().log(Logger.INFO, "Doing strict KEX"); + } + + if (seqi != 1) { + throw new JSchStrictKexException("KEXINIT not first packet from server"); + } + } else if (require_strict_kex) { + throw new JSchStrictKexException("Strict KEX not supported by server"); + } + } + if (!in_kex) { // We are in rekeying activated by the remote! send_kexinit(); } @@ -572,7 +596,9 @@ private KeyExchange receive_kexinit(Buffer buf) throws Exception { guess = KeyExchange.guess(this, I_S, I_C); if (guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("ext-info-c") - || guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("ext-info-s")) { + || guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("ext-info-s") + || guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("kex-strict-c-v00@openssh.com") + || guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("kex-strict-s-v00@openssh.com")) { throw new JSchException("Invalid Kex negotiated: " + guess[KeyExchange.PROPOSAL_KEX_ALGS]); } @@ -595,6 +621,28 @@ private KeyExchange receive_kexinit(Buffer buf) throws Exception { return kex; } + private boolean checkServerStrictKex() { + Buffer sb = new Buffer(I_S); + sb.setOffSet(17); + byte[] sp = sb.getString(); // server proposal + + int l = 0; + int m = 0; + while (l < sp.length) { + while (l < sp.length && sp[l] != ',') + l++; + if (m == l) + continue; + if ("kex-strict-s-v00@openssh.com".equals(Util.byte2str(sp, m, l - m))) { + return true; + } + l++; + m = l; + } + + return false; + } + private volatile boolean in_kex = false; private volatile boolean in_prompt = false; private volatile String[] not_available_shks = null; @@ -683,6 +731,10 @@ private void send_kexinit() throws Exception { kex += ",ext-info-c"; } + if ((enable_strict_kex || require_strict_kex) && initialKex) { + kex += ",kex-strict-c-v00@openssh.com"; + } + String server_host_key = getConfig("server_host_key"); String[] not_available_shks = checkSignatures(getConfig("CheckSignatures")); // Cache for UserAuthPublicKey @@ -1177,7 +1229,9 @@ Buffer read(Buffer buf) throws Exception { } } - seqi++; + if (++seqi == 0 && (enable_strict_kex || require_strict_kex) && initialKex) { + throw new JSchStrictKexException("incoming sequence number wrapped during initial KEX"); + } if (inflater != null) { // inflater.uncompress(buf); @@ -1210,6 +1264,8 @@ Buffer read(Buffer buf) throws Exception { "SSH_MSG_DISCONNECT: " + reason_code + " " + description + " " + language_tag, reason_code, description, language_tag); // break; + } else if (initialKex && doStrictKex) { + break; } else if (type == SSH_MSG_IGNORE) { } else if (type == SSH_MSG_UNIMPLEMENTED) { buf.rewind(); @@ -1354,6 +1410,13 @@ byte[] getSessionId() { private void receive_newkeys(Buffer buf, KeyExchange kex) throws Exception { updateKeys(kex); in_kex = false; + if (doStrictKex) { + seqi = 0; + if (getLogger().isEnabled(Logger.INFO)) { + getLogger().log(Logger.INFO, + "Reset incoming sequence number after receiving SSH_MSG_NEWKEYS for strict KEX"); + } + } } private void updateKeys(KeyExchange kex) throws Exception { @@ -1621,13 +1684,29 @@ void write(Packet packet) throws Exception { } private void _write(Packet packet) throws Exception { + boolean initialKex = this.initialKex; + boolean doStrictKex = this.doStrictKex; + boolean enable_strict_kex = this.enable_strict_kex; + boolean require_strict_kex = this.require_strict_kex; + boolean resetSeqo = packet.buffer.getCommand() == SSH_MSG_NEWKEYS && doStrictKex; + synchronized (lock) { encode(packet); if (io != null) { io.put(packet); - seqo++; + if (++seqo == 0 && (enable_strict_kex || require_strict_kex) && initialKex) { + throw new JSchStrictKexException("outgoing sequence number wrapped during initial KEX"); + } + if (resetSeqo) { + seqo = 0; + } } } + + if (resetSeqo && io != null && getLogger().isEnabled(Logger.INFO)) { + getLogger().log(Logger.INFO, + "Reset outgoing sequence number after sending SSH_MSG_NEWKEYS for strict KEX"); + } } Runnable thread; @@ -2010,6 +2089,8 @@ public void disconnect() { // for the first packet during (re)connect. seqi = 0; seqo = 0; + initialKex = true; + doStrictKex = false; // synchronized(jsch.pool){ // jsch.pool.removeElement(this); @@ -3067,6 +3148,8 @@ private void applyConfig() throws JSchException { checkConfig(config, "kex"); checkConfig(config, "server_host_key"); checkConfig(config, "prefer_known_host_key_types"); + checkConfig(config, "enable_strict_kex"); + checkConfig(config, "require_strict_kex"); checkConfig(config, "enable_pubkey_auth_query"); checkConfig(config, "try_additional_pubkey_algorithms"); checkConfig(config, "enable_auth_none"); diff --git a/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java b/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java index e26ab53b..5bb4ec66 100644 --- a/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java +++ b/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java @@ -61,7 +61,7 @@ public void testJSchAlgoNegoFailException(String algorithmName, String serverPro JSchAlgoNegoFailException e = assertThrows(JSchAlgoNegoFailException.class, session::connect); if (algorithmName.equals("kex")) { - jschProposal += ",ext-info-c"; + jschProposal += ",ext-info-c,kex-strict-c-v00@openssh.com"; } String message = String.format(Locale.ROOT, "Algorithm negotiation fail: algorithmName=\"%s\" jschProposal=\"%s\" serverProposal=\"%s\"", diff --git a/src/test/java/com/jcraft/jsch/JSchStrictKexExceptionIT.java b/src/test/java/com/jcraft/jsch/JSchStrictKexExceptionIT.java new file mode 100644 index 00000000..e8a7fef6 --- /dev/null +++ b/src/test/java/com/jcraft/jsch/JSchStrictKexExceptionIT.java @@ -0,0 +1,91 @@ +package com.jcraft.jsch; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Base64; +import java.util.List; +import java.util.Locale; +import org.junit.jupiter.api.Test; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.images.builder.ImageFromDockerfile; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +@Testcontainers +public class JSchStrictKexExceptionIT { + + private static final int timeout = 2000; + + @Container + public GenericContainer sshd = new GenericContainer<>( + new ImageFromDockerfile().withFileFromClasspath("ssh_host_rsa_key", "docker/ssh_host_rsa_key") + .withFileFromClasspath("ssh_host_rsa_key.pub", "docker/ssh_host_rsa_key.pub") + .withFileFromClasspath("ssh_host_ecdsa256_key", "docker/ssh_host_ecdsa256_key") + .withFileFromClasspath("ssh_host_ecdsa256_key.pub", "docker/ssh_host_ecdsa256_key.pub") + .withFileFromClasspath("ssh_host_ecdsa384_key", "docker/ssh_host_ecdsa384_key") + .withFileFromClasspath("ssh_host_ecdsa384_key.pub", "docker/ssh_host_ecdsa384_key.pub") + .withFileFromClasspath("ssh_host_ecdsa521_key", "docker/ssh_host_ecdsa521_key") + .withFileFromClasspath("ssh_host_ecdsa521_key.pub", "docker/ssh_host_ecdsa521_key.pub") + .withFileFromClasspath("ssh_host_ed25519_key", "docker/ssh_host_ed25519_key") + .withFileFromClasspath("ssh_host_ed25519_key.pub", "docker/ssh_host_ed25519_key.pub") + .withFileFromClasspath("ssh_host_dsa_key", "docker/ssh_host_dsa_key") + .withFileFromClasspath("ssh_host_dsa_key.pub", "docker/ssh_host_dsa_key.pub") + .withFileFromClasspath("sshd_config", "docker/sshd_config") + .withFileFromClasspath("authorized_keys", "docker/authorized_keys") + .withFileFromClasspath("Dockerfile", "docker/Dockerfile")) + .withExposedPorts(22); + + @Test + public void testEnableStrictKexRequireStrictKex() throws Exception { + JSch ssh = createRSAIdentity(); + Session session = createSession(ssh); + session.setConfig("enable_strict_kex", "yes"); + session.setConfig("require_strict_kex", "yes"); + session.setTimeout(timeout); + + assertThrows(JSchStrictKexException.class, session::connect, + "Strict KEX not supported by server"); + } + + @Test + public void testNoEnableStrictKexRequireStrictKex() throws Exception { + JSch ssh = createRSAIdentity(); + Session session = createSession(ssh); + session.setConfig("enable_strict_kex", "no"); + session.setConfig("require_strict_kex", "yes"); + session.setTimeout(timeout); + + assertThrows(JSchStrictKexException.class, session::connect, + "Strict KEX not supported by server"); + } + + private JSch createRSAIdentity() throws Exception { + HostKey hostKey = readHostKey(getResourceFile("docker/ssh_host_rsa_key.pub")); + JSch ssh = new JSch(); + ssh.addIdentity(getResourceFile("docker/id_rsa"), getResourceFile("docker/id_rsa.pub"), null); + ssh.getHostKeyRepository().add(hostKey, null); + return ssh; + } + + private HostKey readHostKey(String fileName) throws Exception { + List lines = Files.readAllLines(Paths.get(fileName), UTF_8); + String[] split = lines.get(0).split("\\s+"); + String hostname = + String.format(Locale.ROOT, "[%s]:%d", sshd.getHost(), sshd.getFirstMappedPort()); + return new HostKey(hostname, Base64.getDecoder().decode(split[1])); + } + + private Session createSession(JSch ssh) throws Exception { + Session session = ssh.getSession("root", sshd.getHost(), sshd.getFirstMappedPort()); + session.setConfig("StrictHostKeyChecking", "yes"); + session.setConfig("PreferredAuthentications", "publickey"); + return session; + } + + private String getResourceFile(String fileName) { + return ResourceUtil.getResourceFile(getClass(), fileName); + } +}