Skip to content

Commit

Permalink
#457 address CVE-2023-48795 by adding support for new strict key exch…
Browse files Browse the repository at this point in the history
…ange extension.
  • Loading branch information
norrisjeremy committed Dec 18, 2023
1 parent 533c437 commit 6214da9
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 4 deletions.
2 changes: 2 additions & 0 deletions src/main/java/com/jcraft/jsch/JSch.java
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
39 changes: 39 additions & 0 deletions src/main/java/com/jcraft/jsch/JSchStrictKexException.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
89 changes: 86 additions & 3 deletions src/main/java/com/jcraft/jsch/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -565,14 +574,31 @@ 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();
}

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]);
}

Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\"",
Expand Down
91 changes: 91 additions & 0 deletions src/test/java/com/jcraft/jsch/JSchStrictKexExceptionIT.java
Original file line number Diff line number Diff line change
@@ -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<String> 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);
}
}

0 comments on commit 6214da9

Please sign in to comment.