Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa

build.gradle

@@ -53,7 +53,7 @@ dependencies {
   testImplementation "org.apache.sshd:sshd-core:$sshdVersion"
   testImplementation "org.apache.sshd:sshd-sftp:$sshdVersion"
   testImplementation "org.apache.sshd:sshd-scp:$sshdVersion"
-  testRuntimeOnly "ch.qos.logback:logback-classic:1.2.6"
+  testImplementation "ch.qos.logback:logback-classic:1.2.6"
   testImplementation 'org.glassfish.grizzly:grizzly-http-server:2.4.4'
   testImplementation 'org.apache.httpcomponents:httpclient:4.5.9'
   testImplementation 'org.testcontainers:testcontainers:1.16.2'

src/itest/groovy/com/hierynomus/sshj/RsaShaKeySignatureTest.groovy

@@ -0,0 +1,159 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.sshj
+
+import com.hierynomus.sshj.key.KeyAlgorithms
+import net.schmizz.sshj.Config
+import net.schmizz.sshj.DefaultConfig
+import org.testcontainers.images.builder.dockerfile.DockerfileBuilder
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import java.nio.file.Paths
+
+/**
+ * Checks that SSHJ is able to work with OpenSSH 8.8, which removed ssh-rsa signature from the default setup.
+ */
+class RsaShaKeySignatureTest extends Specification {
+    private static final Map<String, KeyAlgorithms.Factory> SSH_HOST_KEYS_AND_FACTORIES = [
+        'ssh_host_ecdsa_256_key': KeyAlgorithms.ECDSASHANistp256(),
+        'ssh_host_ecdsa_384_key': KeyAlgorithms.ECDSASHANistp384(),
+        'ssh_host_ecdsa_521_key': KeyAlgorithms.ECDSASHANistp521(),
+        'ssh_host_ed25519_384_key': KeyAlgorithms.EdDSA25519(),
+        'ssh_host_rsa_2048_key': KeyAlgorithms.RSASHA512(),
+    ]
+
+    private static void dockerfileBuilder(DockerfileBuilder it, String hostKey, String pubkeyAcceptedAlgorithms) {
+        it.from("archlinux:base")
+        it.run('yes | pacman -Sy core/openssh' +
+                ' && (' +
+                '  V=$(echo $(/usr/sbin/sshd -h 2>&1) | grep -o \'OpenSSH_[0-9][0-9]*[.][0-9][0-9]*p[0-9]\');' +
+                '  if [[ "$V" < OpenSSH_8.8p1 ]]; then' +
+                '    echo $V is too old 1>&2;' +
+                '    exit 1;' +
+                '   fi' +
+                ')' +
+                ' && set -o pipefail ' +
+                ' && useradd --create-home sshj' +
+                ' && echo \"sshj:ultrapassword\" | chpasswd')
+        it.add("authorized_keys", "/home/sshj/.ssh/")
+        it.add(hostKey, '/etc/ssh/')
+        it.run('chmod go-rwx /etc/ssh/ssh_host_*' +
+                ' && chown -R sshj /home/sshj/.ssh' +
+                ' && chmod -R go-rwx /home/sshj/.ssh')
+        it.expose(22)
+
+        def cmd = [
+                '/usr/sbin/sshd',
+                '-D',
+                '-e',
+                '-f', '/dev/null',
+                '-o', 'LogLevel=DEBUG2',
+                '-o', "HostKey=/etc/ssh/$hostKey",
+        ]
+        if (pubkeyAcceptedAlgorithms != null) {
+            cmd += ['-o', "PubkeyAcceptedAlgorithms=$pubkeyAcceptedAlgorithms"]
+        }
+        it.cmd(cmd as String[])
+    }
+
+    private static SshdContainer makeSshdContainer(String hostKey, String pubkeyAcceptedAlgorithms) {
+        return new SshdContainer(new SshdContainer.DebugLoggingImageFromDockerfile()
+                .withFileFromPath("authorized_keys", Paths.get("src/itest/docker-image/authorized_keys"))
+                .withFileFromPath(hostKey, Paths.get("src/itest/docker-image/test-container/host_keys/$hostKey"))
+                .withDockerfileFromBuilder {
+                    dockerfileBuilder(it, hostKey, pubkeyAcceptedAlgorithms)
+                })
+    }
+
+    @Unroll
+    def "connect to a server with host key #hostKey that does not support ssh-rsa"() {
+        given:
+        SshdContainer sshd = makeSshdContainer(hostKey, "rsa-sha2-512,rsa-sha2-256,ssh-ed25519")
+        sshd.start()
+
+        and:
+        Config config = new DefaultConfig()
+        config.keyAlgorithms = [
+                KeyAlgorithms.RSASHA512(),
+                KeyAlgorithms.RSASHA256(),
+                SSH_HOST_KEYS_AND_FACTORIES[hostKey],
+        ]
+
+        when:
+        def sshClient = sshd.getConnectedClient(config)
+        sshClient.authPublickey("sshj", "src/itest/resources/keyfiles/id_rsa_opensshv1")
+
+        then:
+        sshClient.isAuthenticated()
+
+        cleanup:
+        sshClient?.disconnect()
+        sshd.stop()
+
+        where:
+        hostKey << SSH_HOST_KEYS_AND_FACTORIES.keySet()
+    }
+
+    @Unroll
+    def "connect to a default server with host key #hostKey using a default config"() {
+        given:
+        SshdContainer sshd = makeSshdContainer(hostKey, null)
+        sshd.start()
+
+        when:
+        def sshClient = sshd.getConnectedClient()
+        sshClient.authPublickey("sshj", "src/itest/resources/keyfiles/id_rsa_opensshv1")
+
+        then:
+        sshClient.isAuthenticated()
+
+        cleanup:
+        sshClient?.disconnect()
+        sshd.stop()
+
+        where:
+        hostKey << SSH_HOST_KEYS_AND_FACTORIES.keySet()
+    }
+
+    @Unroll
+    def "connect to a server with host key #hostkey that supports only ssh-rsa"() {
+        given:
+        SshdContainer sshd = makeSshdContainer(hostKey, "ssh-rsa,ssh-ed25519")
+        sshd.start()
+
+        and:
+        Config config = new DefaultConfig()
+        config.keyAlgorithms = [
+                KeyAlgorithms.SSHRSA(),
+                SSH_HOST_KEYS_AND_FACTORIES[hostKey],
+        ]
+
+        when:
+        def sshClient = sshd.getConnectedClient(config)
+        sshClient.authPublickey("sshj", "src/itest/resources/keyfiles/id_rsa_opensshv1")
+
+        then:
+        sshClient.isAuthenticated()
+
+        cleanup:
+        sshClient.disconnect()
+        sshd.stop()
+
+        where:
+        hostKey << SSH_HOST_KEYS_AND_FACTORIES.keySet()
+    }
+}

src/itest/groovy/com/hierynomus/sshj/SshServerWaitStrategy.java

@@ -35,7 +35,7 @@ public class SshServerWaitStrategy implements WaitStrategy {
     @Override
     public void waitUntilReady(WaitStrategyTarget waitStrategyTarget) {
         long expectedEnd = System.nanoTime() + startupTimeout.toNanos();
-        while (true) {
+        while (waitStrategyTarget.isRunning()) {
             long attemptStart = System.nanoTime();
             IOException error = null;
             byte[] buffer = new byte[7];

src/itest/groovy/com/hierynomus/sshj/SshdContainer.java

@@ -15,14 +15,18 @@
  */
 package com.hierynomus.sshj;
 
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.DefaultConfig;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.transport.verification.PromiscuousVerifier;
 import org.jetbrains.annotations.NotNull;
+import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.images.builder.ImageFromDockerfile;
 import org.testcontainers.images.builder.dockerfile.DockerfileBuilder;
+import org.testcontainers.utility.DockerLoggerFactory;
 
 import java.io.IOException;
 import java.nio.file.Paths;
@@ -32,6 +36,20 @@
  * A JUnit4 rule for launching a generic SSH server container.
  */
 public class SshdContainer extends GenericContainer<SshdContainer> {
+    /**
+     * A workaround for strange logger names of testcontainers. They contain no dots, but contain slashes,
+     * square brackets, and even emoji. It's uneasy to set the logging level via the XML file of logback, the
+     * result would be less readable than the code below.
+     */
+    public static class DebugLoggingImageFromDockerfile extends ImageFromDockerfile {
+        public DebugLoggingImageFromDockerfile() {
+            super();
+            Logger logger = (Logger) LoggerFactory.getILoggerFactory()
+                    .getLogger(DockerLoggerFactory.getLogger(getDockerImageName()).getName());
+            logger.setLevel(Level.DEBUG);
+        }
+    }
+
     public static class Builder {
         public static final String DEFAULT_SSHD_CONFIG = "" +
                 "PermitRootLogin yes\n" +
@@ -95,7 +113,7 @@ public static void defaultDockerfileBuilder(@NotNull DockerfileBuilder builder)
         }
 
         private @NotNull Future<String> buildInner() {
-            return new ImageFromDockerfile()
+            return new DebugLoggingImageFromDockerfile()
                     .withDockerfileFromBuilder(Builder::defaultDockerfileBuilder)
                     .withFileFromPath(".", Paths.get("src/itest/docker-image"))
                     .withFileFromString("sshd_config", sshdConfig);
@@ -111,6 +129,16 @@ public SshdContainer(@NotNull Future<String> future) {
         super(future);
         withExposedPorts(22);
         setWaitStrategy(new SshServerWaitStrategy());
+        withLogConsumer(outputFrame -> {
+            switch (outputFrame.getType()) {
+                case STDOUT:
+                    logger().info("sshd stdout: {}", outputFrame.getUtf8String().stripTrailing());
+                    break;
+                case STDERR:
+                    logger().info("sshd stderr: {}", outputFrame.getUtf8String().stripTrailing());
+                    break;
+            }
+        });
     }
 
     public SSHClient getConnectedClient(Config config) throws IOException {

src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java

@@ -27,8 +27,6 @@
 
 public class KeyAlgorithms {
 
-    public static List<String> SSH_RSA_SHA2_ALGORITHMS = Arrays.asList("rsa-sha2-512", "rsa-sha2-256");
-
     public static Factory SSHRSA() { return new Factory("ssh-rsa", new SignatureRSA.FactorySSHRSA(), KeyType.RSA); }
     public static Factory SSHRSACertV01() { return new Factory("ssh-rsa-cert-v01@openssh.com", new SignatureRSA.FactoryCERT(), KeyType.RSA_CERT); }
     public static Factory RSASHA256() { return new Factory("rsa-sha2-256", new SignatureRSA.FactoryRSASHA256(), KeyType.RSA); }
@@ -61,6 +59,10 @@ public String getName() {
             return algorithmName;
         }
 
+        public KeyType getKeyType() {
+            return keyType;
+        }
+
         @Override
         public KeyAlgorithm create() {
             return new BaseKeyAlgorithm(algorithmName, signatureFactory, keyType);

src/main/java/net/schmizz/sshj/ConfigImpl.java

@@ -26,6 +26,7 @@
 import net.schmizz.sshj.transport.random.Random;
 import net.schmizz.sshj.userauth.keyprovider.FileKeyProvider;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
@@ -188,4 +189,30 @@ public boolean isVerifyHostKeyCertificates() {
     public void setVerifyHostKeyCertificates(boolean value) {
         verifyHostKeyCertificates = value;
     }
+
+    /**
+     * Modern servers neglect the key algorithm ssh-rsa. OpenSSH 8.8 even dropped its support by default in favour
+     * of rsa-sha2-*. However, there are legacy servers like Apache SSHD that don't support the newer replacements
+     * for ssh-rsa.
+     *
+     * If ssh-rsa factory is in {@link #getKeyAlgorithms()}, this methods makes ssh-rsa key algorithm more preferred
+     * than any of rsa-sha2-*. Otherwise, nothing happens.
+     */
+    public void prioritizeSshRsaKeyAlgorithm() {
+        List<Factory.Named<KeyAlgorithm>> keyAlgorithms = getKeyAlgorithms();
+        for (int sshRsaIndex = 0; sshRsaIndex < keyAlgorithms.size(); ++ sshRsaIndex) {
+            if ("ssh-rsa".equals(keyAlgorithms.get(sshRsaIndex).getName())) {
+                for (int i = 0; i < sshRsaIndex; ++i) {
+                    final String algo = keyAlgorithms.get(i).getName();
+                    if ("rsa-sha2-256".equals(algo) || "rsa-sha2-512".equals(algo)) {
+                        keyAlgorithms = new ArrayList<>(keyAlgorithms);
+                        keyAlgorithms.add(i, keyAlgorithms.remove(sshRsaIndex));
+                        setKeyAlgorithms(keyAlgorithms);
+                        break;
+                    }
+                }
+                break;
+            }
+        }
+    }
 }

src/main/java/net/schmizz/sshj/transport/KeyExchanger.java

@@ -243,7 +243,6 @@ private void gotKexInit(SSHPacket buf)
                                         negotiatedAlgs.getKeyExchangeAlgorithm());
         transport.setHostKeyAlgorithm(Factory.Named.Util.create(transport.getConfig().getKeyAlgorithms(),
                                       negotiatedAlgs.getSignatureAlgorithm()));
-        transport.setRSASHA2Support(negotiatedAlgs.getRSASHA2Support());
 
         try {
             kex.init(transport,

src/main/java/net/schmizz/sshj/transport/NegotiatedAlgorithms.java

@@ -26,10 +26,8 @@ public final class NegotiatedAlgorithms {
     private final String c2sComp;
     private final String s2cComp;
 
-    private final boolean rsaSHA2Support;
-
     NegotiatedAlgorithms(String kex, String sig, String c2sCipher, String s2cCipher, String c2sMAC, String s2cMAC,
-                         String c2sComp, String s2cComp, boolean rsaSHA2Support) {
+                         String c2sComp, String s2cComp) {
         this.kex = kex;
         this.sig = sig;
         this.c2sCipher = c2sCipher;
@@ -38,7 +36,6 @@ public final class NegotiatedAlgorithms {
         this.s2cMAC = s2cMAC;
         this.c2sComp = c2sComp;
         this.s2cComp = s2cComp;
-        this.rsaSHA2Support = rsaSHA2Support;
     }
 
     public String getKeyExchangeAlgorithm() {
@@ -73,10 +70,6 @@ public String getServer2ClientCompressionAlgorithm() {
         return s2cComp;
     }
 
-    public boolean getRSASHA2Support() {
-        return rsaSHA2Support;
-    }
-
     @Override
     public String toString() {
         return ("[ " +
@@ -88,7 +81,6 @@ public String toString() {
                 "s2cMAC=" + s2cMAC + "; " +
                 "c2sComp=" + c2sComp + "; " +
                 "s2cComp=" + s2cComp + "; " +
-                "rsaSHA2Support=" + rsaSHA2Support +
                 " ]");
     }
 

src/main/java/net/schmizz/sshj/transport/Proposal.java

@@ -15,7 +15,6 @@
  */
 package net.schmizz.sshj.transport;
 
-import com.hierynomus.sshj.key.KeyAlgorithms;
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.common.Buffer;
 import net.schmizz.sshj.common.Factory;
@@ -140,8 +139,8 @@ public NegotiatedAlgorithms negotiate(Proposal other)
                 firstMatch("Client2ServerCompressionAlgorithms", this.getClient2ServerCompressionAlgorithms(),
                         other.getClient2ServerCompressionAlgorithms()),
                 firstMatch("Server2ClientCompressionAlgorithms", this.getServer2ClientCompressionAlgorithms(),
-                        other.getServer2ClientCompressionAlgorithms()),
-                other.getHostKeyAlgorithms().containsAll(KeyAlgorithms.SSH_RSA_SHA2_ALGORITHMS));
+                        other.getServer2ClientCompressionAlgorithms())
+        );
     }
 
     private List<String> filterKnownHostKeyAlgorithms(List<String> configuredKeyAlgorithms, List<String> knownHostKeyAlgorithms) {