From bac489e5c231849be0236701ae06384d7fc094fe Mon Sep 17 00:00:00 2001 From: Vladimir Lagunov Date: Thu, 11 Nov 2021 16:07:44 +0700 Subject: [PATCH 1/3] Improve SshdContainer: log `docker build` to stdout, don't wait too long if container exited --- build.gradle | 2 +- .../sshj/SshServerWaitStrategy.java | 2 +- .../com/hierynomus/sshj/SshdContainer.java | 30 ++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 777cb3d3..9fd32081 100644 --- a/build.gradle +++ b/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' diff --git a/src/itest/groovy/com/hierynomus/sshj/SshServerWaitStrategy.java b/src/itest/groovy/com/hierynomus/sshj/SshServerWaitStrategy.java index 3dec0336..3942e4b5 100644 --- a/src/itest/groovy/com/hierynomus/sshj/SshServerWaitStrategy.java +++ b/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]; diff --git a/src/itest/groovy/com/hierynomus/sshj/SshdContainer.java b/src/itest/groovy/com/hierynomus/sshj/SshdContainer.java index 9aa8f1b3..fda929e3 100644 --- a/src/itest/groovy/com/hierynomus/sshj/SshdContainer.java +++ b/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 { + /** + * 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 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 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 { From f6a78459a0888972ec4c426dd547c7867c66abc3 Mon Sep 17 00:00:00 2001 From: Vladimir Lagunov Date: Thu, 11 Nov 2021 15:53:39 +0700 Subject: [PATCH 2/3] Fix #740: Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa Previously, there was a heuristic that was choosing rsa-sha2-512 after receiving a host key of type RSA. It didn't work well when a server doesn't have an RSA host key. OpenSSH 8.8 introduced a breaking change: it removed ssh-rsa from the default list of supported public key signature algorithms. SSHJ was unable to connect to OpenSSH 8.8 server if the server has an EcDSA or Ed25519 host key. Current behaviour behaves the same as OpenSSH 8.8 client does. SSHJ doesn't try to determine rsa-sha2-* support on the fly. Instead, it looks only on `Config.getKeyAlgorithms()`, which may or may not contain ssh-rsa and rsa-sha2-* in any order. Sorry, this commit mostly reverts changes from #607. --- .../sshj/RsaShaKeySignatureTest.groovy | 159 ++++++++++++++++++ .../hierynomus/sshj/key/KeyAlgorithms.java | 6 +- .../schmizz/sshj/transport/KeyExchanger.java | 1 - .../sshj/transport/NegotiatedAlgorithms.java | 10 +- .../net/schmizz/sshj/transport/Proposal.java | 5 +- .../schmizz/sshj/transport/TransportImpl.java | 15 +- .../sshj/userauth/method/KeyedAuthMethod.java | 11 +- .../keyprovider/FileKeyProviderSpec.groovy | 13 +- 8 files changed, 189 insertions(+), 31 deletions(-) create mode 100644 src/itest/groovy/com/hierynomus/sshj/RsaShaKeySignatureTest.groovy diff --git a/src/itest/groovy/com/hierynomus/sshj/RsaShaKeySignatureTest.groovy b/src/itest/groovy/com/hierynomus/sshj/RsaShaKeySignatureTest.groovy new file mode 100644 index 00000000..a039b4c2 --- /dev/null +++ b/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 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() + } +} diff --git a/src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java b/src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java index 2c4c4edf..4b232e7d 100644 --- a/src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java +++ b/src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java @@ -27,8 +27,6 @@ public class KeyAlgorithms { - public static List 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); diff --git a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java index f8ad845a..6705519f 100644 --- a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java +++ b/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, diff --git a/src/main/java/net/schmizz/sshj/transport/NegotiatedAlgorithms.java b/src/main/java/net/schmizz/sshj/transport/NegotiatedAlgorithms.java index b1c97b53..824cb0f7 100644 --- a/src/main/java/net/schmizz/sshj/transport/NegotiatedAlgorithms.java +++ b/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 + " ]"); } diff --git a/src/main/java/net/schmizz/sshj/transport/Proposal.java b/src/main/java/net/schmizz/sshj/transport/Proposal.java index 299cd57d..5f5f8a1f 100644 --- a/src/main/java/net/schmizz/sshj/transport/Proposal.java +++ b/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 filterKnownHostKeyAlgorithms(List configuredKeyAlgorithms, List knownHostKeyAlgorithms) { diff --git a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java index 57effbbd..6195704a 100644 --- a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java +++ b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java @@ -86,8 +86,6 @@ static final class ConnInfo { private KeyAlgorithm hostKeyAlgorithm; - private boolean rsaSHA2Support; - private final Event serviceAccept; private final Event close; @@ -626,20 +624,15 @@ public KeyAlgorithm getHostKeyAlgorithm() { return this.hostKeyAlgorithm; } - public void setRSASHA2Support(boolean rsaSHA2Support) { - this.rsaSHA2Support = rsaSHA2Support; - } - @Override public KeyAlgorithm getClientKeyAlgorithm(KeyType keyType) throws TransportException { - if (keyType != KeyType.RSA || !rsaSHA2Support) { - return Factory.Named.Util.create(getConfig().getKeyAlgorithms(), keyType.toString()); - } - List> factories = getConfig().getKeyAlgorithms(); if (factories != null) for (Factory.Named f : factories) - if (f.getName().equals("ssh-rsa") || KeyAlgorithms.SSH_RSA_SHA2_ALGORITHMS.contains(f.getName())) + if ( + f instanceof KeyAlgorithms.Factory && ((KeyAlgorithms.Factory) f).getKeyType().equals(keyType) + || !(f instanceof KeyAlgorithms.Factory) && f.getName().equals(keyType.toString()) + ) return f.create(); throw new TransportException("Cannot find an available KeyAlgorithm for type " + keyType); } diff --git a/src/main/java/net/schmizz/sshj/userauth/method/KeyedAuthMethod.java b/src/main/java/net/schmizz/sshj/userauth/method/KeyedAuthMethod.java index 38ae608e..3ec23e76 100644 --- a/src/main/java/net/schmizz/sshj/userauth/method/KeyedAuthMethod.java +++ b/src/main/java/net/schmizz/sshj/userauth/method/KeyedAuthMethod.java @@ -51,12 +51,15 @@ protected SSHPacket putPubKey(SSHPacket reqBuf) KeyType keyType = KeyType.fromKey(key); try { KeyAlgorithm ka = params.getTransport().getClientKeyAlgorithm(keyType); - reqBuf.putString(ka.getKeyAlgorithm()) - .putString(new Buffer.PlainBuffer().putPublicKey(key).getCompactData()); - return reqBuf; + if (ka != null) { + reqBuf.putString(ka.getKeyAlgorithm()) + .putString(new Buffer.PlainBuffer().putPublicKey(key).getCompactData()); + return reqBuf; + } } catch (IOException ioe) { - throw new UserAuthException("No KeyAlgorithm configured for key " + keyType); + throw new UserAuthException("No KeyAlgorithm configured for key " + keyType, ioe); } + throw new UserAuthException("No KeyAlgorithm configured for key " + keyType); } protected SSHPacket putSig(SSHPacket reqBuf) diff --git a/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy b/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy index e81b6e2c..06b48af6 100644 --- a/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy +++ b/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy @@ -16,6 +16,7 @@ package com.hierynomus.sshj.userauth.keyprovider import com.hierynomus.sshj.test.SshFixture +import net.schmizz.sshj.DefaultConfig import net.schmizz.sshj.SSHClient import net.schmizz.sshj.userauth.keyprovider.KeyFormat import org.apache.sshd.server.auth.pubkey.AcceptAllPublickeyAuthenticator @@ -39,7 +40,17 @@ class FileKeyProviderSpec extends Specification { @Unroll def "should have #format FileKeyProvider enabled by default"() { given: - SSHClient client = fixture.setupConnectedDefaultClient() + // `fixture` is backed by Apache SSHD server. Looks like it doesn't support rsa-sha2-512 public key signature. + // Changing the default config to prioritize the former default implementation of RSA signature. + def config = new DefaultConfig() + def sshRsaFactory = config.keyAlgorithms.find {it.name == "ssh-rsa" } + if (sshRsaFactory == null) + throw new IllegalStateException("ssh-rsa was removed from the default config, the test should be refactored") + config.keyAlgorithms = [sshRsaFactory] + config.keyAlgorithms.grep { it != sshRsaFactory } + + and: + SSHClient client = fixture.setupClient(config) + fixture.connectClient(client) when: client.authPublickey("jeroen", keyfile) From 592b612ddec552120c87d81690ca0708101386b6 Mon Sep 17 00:00:00 2001 From: Vladimir Lagunov Date: Mon, 6 Dec 2021 16:31:55 +0700 Subject: [PATCH 3/3] Introduce ConfigImpl.prioritizeSshRsaKeyAlgorithm to deal with broken backward compatibility --- .../java/net/schmizz/sshj/ConfigImpl.java | 27 ++++++ .../keyprovider/FileKeyProviderSpec.groovy | 7 +- .../net/schmizz/sshj/ConfigImplSpec.groovy | 87 +++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy diff --git a/src/main/java/net/schmizz/sshj/ConfigImpl.java b/src/main/java/net/schmizz/sshj/ConfigImpl.java index 836f2161..67243cb3 100644 --- a/src/main/java/net/schmizz/sshj/ConfigImpl.java +++ b/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> 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; + } + } + } } diff --git a/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy b/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy index 06b48af6..3a10e338 100644 --- a/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy +++ b/src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy @@ -43,10 +43,7 @@ class FileKeyProviderSpec extends Specification { // `fixture` is backed by Apache SSHD server. Looks like it doesn't support rsa-sha2-512 public key signature. // Changing the default config to prioritize the former default implementation of RSA signature. def config = new DefaultConfig() - def sshRsaFactory = config.keyAlgorithms.find {it.name == "ssh-rsa" } - if (sshRsaFactory == null) - throw new IllegalStateException("ssh-rsa was removed from the default config, the test should be refactored") - config.keyAlgorithms = [sshRsaFactory] + config.keyAlgorithms.grep { it != sshRsaFactory } + config.prioritizeSshRsaKeyAlgorithm() and: SSHClient client = fixture.setupClient(config) @@ -59,7 +56,7 @@ class FileKeyProviderSpec extends Specification { client.isAuthenticated() cleanup: - client.disconnect() + client?.disconnect() where: format | keyfile diff --git a/src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy b/src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy new file mode 100644 index 00000000..5395e673 --- /dev/null +++ b/src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy @@ -0,0 +1,87 @@ +/* + * 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 net.schmizz.sshj + +import com.hierynomus.sshj.key.KeyAlgorithms +import spock.lang.Specification + +class ConfigImplSpec extends Specification { + static def ECDSA = KeyAlgorithms.ECDSASHANistp521() + static def ED25519 = KeyAlgorithms.EdDSA25519() + static def RSA_SHA_256 = KeyAlgorithms.RSASHA256() + static def RSA_SHA_512 = KeyAlgorithms.RSASHA512() + static def SSH_RSA = KeyAlgorithms.SSHRSA() + + def "prioritizeSshRsaKeyAlgorithm does nothing if there is no ssh-rsa"() { + given: + def config = new DefaultConfig() + config.keyAlgorithms = [RSA_SHA_512, ED25519] + + when: + config.prioritizeSshRsaKeyAlgorithm() + + then: + config.keyAlgorithms == [RSA_SHA_512, ED25519] + } + + def "prioritizeSshRsaKeyAlgorithm does nothing if there is no rsa-sha2-any"() { + given: + def config = new DefaultConfig() + config.keyAlgorithms = [ED25519, SSH_RSA, ECDSA] + + when: + config.prioritizeSshRsaKeyAlgorithm() + + then: + config.keyAlgorithms == [ED25519, SSH_RSA, ECDSA] + } + + def "prioritizeSshRsaKeyAlgorithm does nothing if ssh-rsa already has higher priority"() { + given: + def config = new DefaultConfig() + config.keyAlgorithms = [ED25519, SSH_RSA, RSA_SHA_512, ECDSA] + + when: + config.prioritizeSshRsaKeyAlgorithm() + + then: + config.keyAlgorithms == [ED25519, SSH_RSA, RSA_SHA_512, ECDSA] + } + + def "prioritizeSshRsaKeyAlgorithm prioritizes ssh-rsa if there is one rsa-sha2-any is prioritized"() { + given: + def config = new DefaultConfig() + config.keyAlgorithms = [ED25519, RSA_SHA_512, ECDSA, SSH_RSA] + + when: + config.prioritizeSshRsaKeyAlgorithm() + + then: + config.keyAlgorithms == [ED25519, SSH_RSA, RSA_SHA_512, ECDSA] + } + + def "prioritizeSshRsaKeyAlgorithm prioritizes ssh-rsa if there are two rsa-sha2-any is prioritized"() { + given: + def config = new DefaultConfig() + config.keyAlgorithms = [ED25519, RSA_SHA_512, ECDSA, RSA_SHA_256, SSH_RSA] + + when: + config.prioritizeSshRsaKeyAlgorithm() + + then: + config.keyAlgorithms == [ED25519, SSH_RSA, RSA_SHA_512, ECDSA, RSA_SHA_256] + } +}