When connections fail due to an algorithm negotiation failure, throw …

…a JSchAlgoNegoFailException that extends JSchException. The new JSchAlgoNegoFailException details which specific algorithm negotiation failed, along with what both JSch and the server proposed.
mwiede · Sep 6, 2022 · 82878e7 · 82878e7
1 parent 4d57f2c
commit 82878e7
Show file tree

Hide file tree

Showing 5 changed files with 187 additions and 6 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,3 +1,6 @@
+* [0.2.4](https://github.com/mwiede/jsch/releases/tag/jsch-0.2.4)
+  * When connections fail due to an algorithm negotiation failure, throw a `JSchAlgoNegoFailException` that extends `JSchException`.
+    * The new `JSchAlgoNegoFailException` details which specific algorithm negotiation failed, along with what both JSch and the server proposed.
 * [0.2.3](https://github.com/mwiede/jsch/releases/tag/jsch-0.2.3)
   * #188 fix private key length checks for ssh-ed25519 & ssh-ed448. by @norrisjeremy in https://github.com/mwiede/jsch/pull/189
 * [0.2.2](https://github.com/mwiede/jsch/releases/tag/jsch-0.2.2)

diff --git a/src/main/java/com/jcraft/jsch/JSchAlgoNegoFailException.java b/src/main/java/com/jcraft/jsch/JSchAlgoNegoFailException.java
@@ -0,0 +1,69 @@
+package com.jcraft.jsch;
+
+/**
+ * Extension of {@link JSchException} to indicate when a connection fails during algorithm
+ * negotiation.
+ */
+public class JSchAlgoNegoFailException extends JSchException {
+
+  private static final long serialVersionUID = -1L;
+
+  private final String algorithmName;
+  private final String jschProposal;
+  private final String serverProposal;
+
+  JSchAlgoNegoFailException(int algorithmIndex, String jschProposal, String serverProposal) {
+    super(failString(algorithmIndex, jschProposal, serverProposal));
+    algorithmName = algorithmNameFromIndex(algorithmIndex);
+    this.jschProposal = jschProposal;
+    this.serverProposal = serverProposal;
+  }
+
+  /** Get the algorithm name. */
+  public String getAlgorithmName() {
+    return algorithmName;
+  }
+
+  /** Get the JSch algorithm proposal. */
+  public String getJSchProposal() {
+    return jschProposal;
+  }
+
+  /** Get the server algorithm proposal. */
+  public String getServerProposal() {
+    return serverProposal;
+  }
+
+  private static String failString(int algorithmIndex, String jschProposal, String serverProposal) {
+    return String.format(
+        "Algorithm negotiation fail: algorithmName=\"%s\" jschProposal=\"%s\" serverProposal=\"%s\"",
+        algorithmNameFromIndex(algorithmIndex), jschProposal, serverProposal);
+  }
+
+  private static String algorithmNameFromIndex(int algorithmIndex) {
+    switch (algorithmIndex) {
+      case KeyExchange.PROPOSAL_KEX_ALGS:
+        return "kex";
+      case KeyExchange.PROPOSAL_SERVER_HOST_KEY_ALGS:
+        return "server_host_key";
+      case KeyExchange.PROPOSAL_ENC_ALGS_CTOS:
+        return "cipher.c2s";
+      case KeyExchange.PROPOSAL_ENC_ALGS_STOC:
+        return "cipher.s2c";
+      case KeyExchange.PROPOSAL_MAC_ALGS_CTOS:
+        return "mac.c2s";
+      case KeyExchange.PROPOSAL_MAC_ALGS_STOC:
+        return "mac.s2c";
+      case KeyExchange.PROPOSAL_COMP_ALGS_CTOS:
+        return "compression.c2s";
+      case KeyExchange.PROPOSAL_COMP_ALGS_STOC:
+        return "compression.s2c";
+      case KeyExchange.PROPOSAL_LANG_CTOS:
+        return "lang.c2s";
+      case KeyExchange.PROPOSAL_LANG_STOC:
+        return "lang.s2c";
+      default:
+        return "";
+    }
+  }
+}
diff --git a/src/main/java/com/jcraft/jsch/KeyExchange.java b/src/main/java/com/jcraft/jsch/KeyExchange.java
@@ -123,13 +123,13 @@ protected static String[] guess(Session session, byte[]I_S, byte[]I_C) throws Ex
       loop:
       while(j<cp.length){
         while(j<cp.length && cp[j]!=',')j++; 
-        if(k==j) return null;
+        if(k==j) throw new JSchAlgoNegoFailException(i, Util.byte2str(cp), Util.byte2str(sp));
         String algorithm=Util.byte2str(cp, k, j-k);
         int l=0;
         int m=0;
         while(l<sp.length){
           while(l<sp.length && sp[l]!=',')l++; 
-          if(m==l) return null;
+          if(m==l) throw new JSchAlgoNegoFailException(i, Util.byte2str(cp), Util.byte2str(sp));
           if(algorithm.equals(Util.byte2str(sp, m, l-m))){
             guess[i]=algorithm;
             break loop;
@@ -144,7 +144,7 @@ protected static String[] guess(Session session, byte[]I_S, byte[]I_C) throws Ex
         guess[i]="";
       }
       else if(guess[i]==null){
-        return null;
+        throw new JSchAlgoNegoFailException(i, Util.byte2str(cp), Util.byte2str(sp));
       }
     }
 

diff --git a/src/main/java/com/jcraft/jsch/Session.java b/src/main/java/com/jcraft/jsch/Session.java
@@ -600,9 +600,6 @@ private KeyExchange receive_kexinit(Buffer buf) throws Exception {
    }
 
     guess=KeyExchange.guess(this, I_S, I_C);
-    if(guess==null){
-      throw new JSchException("Algorithm negotiation fail");
-    }
 
     if(guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("ext-info-c") ||
        guess[KeyExchange.PROPOSAL_KEX_ALGS].equals("ext-info-s")){

diff --git a/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java b/src/test/java/com/jcraft/jsch/JSchAlgoNegoFailExceptionIT.java
@@ -0,0 +1,112 @@
+package com.jcraft.jsch;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.commons.codec.binary.Base64.decodeBase64;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.List;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+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 JSchAlgoNegoFailExceptionIT {
+
+  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);
+
+  @ParameterizedTest
+  @CsvSource(
+      delimiter = '|',
+      value = {
+        "kex|curve25519-sha256,curve25519-sha256@libssh.org,ecdh-sha2-nistp521,ecdh-sha2-nistp384,ecdh-sha2-nistp256,diffie-hellman-group18-sha512,diffie-hellman-group16-sha512,diffie-hellman-group14-sha256,diffie-hellman-group-exchange-sha256,diffie-hellman-group-exchange-sha1,diffie-hellman-group14-sha1,diffie-hellman-group1-sha1",
+        "server_host_key|ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-ed25519,ssh-rsa,rsa-sha2-512,rsa-sha2-256,ssh-dss",
+        "cipher.c2s|chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr,aes256-cbc,aes192-cbc,aes128-cbc,3des-cbc,blowfish-cbc,arcfour,arcfour256,arcfour128,rijndael-cbc@lysator.liu.se,cast128-cbc",
+        "cipher.s2c|chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr,aes256-cbc,aes192-cbc,aes128-cbc,3des-cbc,blowfish-cbc,arcfour,arcfour256,arcfour128,rijndael-cbc@lysator.liu.se,cast128-cbc",
+        "mac.c2s|hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha1-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,hmac-sha1,hmac-sha1-96-etm@openssh.com,hmac-sha1-96,hmac-md5-etm@openssh.com,hmac-md5,hmac-md5-96-etm@openssh.com,hmac-md5-96,hmac-ripemd160,hmac-ripemd160@openssh.com,hmac-ripemd160-etm@openssh.com",
+        "mac.s2c|hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha1-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,hmac-sha1,hmac-sha1-96-etm@openssh.com,hmac-sha1-96,hmac-md5-etm@openssh.com,hmac-md5,hmac-md5-96-etm@openssh.com,hmac-md5-96,hmac-ripemd160,hmac-ripemd160@openssh.com,hmac-ripemd160-etm@openssh.com",
+        "compression.c2s|none,zlib@openssh.com",
+        "compression.s2c|none,zlib@openssh.com",
+        "lang.c2s|''",
+        "lang.s2c|''"
+      })
+  public void testJSchAlgoNegoFailException(String algorithmName, String serverProposal)
+      throws Exception {
+    String jschProposal = "foo";
+    JSch ssh = createRSAIdentity();
+    Session session = createSession(ssh);
+    session.setConfig(algorithmName, jschProposal);
+    session.setTimeout(timeout);
+
+    JSchAlgoNegoFailException e = assertThrows(JSchAlgoNegoFailException.class, session::connect);
+
+    if (algorithmName.equals("kex")) {
+      jschProposal += ",ext-info-c";
+    }
+    String message =
+        String.format(
+            "Algorithm negotiation fail: algorithmName=\"%s\" jschProposal=\"%s\" serverProposal=\"%s\"",
+            algorithmName, jschProposal, serverProposal);
+
+    assertEquals(message, e.getMessage());
+    assertEquals(algorithmName, e.getAlgorithmName());
+    assertEquals(jschProposal, e.getJSchProposal());
+    assertEquals(serverProposal, e.getServerProposal());
+  }
+
+  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("[%s]:%d", sshd.getHost(), sshd.getFirstMappedPort());
+    return new HostKey(hostname, decodeBase64(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);
+  }
+}