Mirrors/sshj
1
0
Fork 0
mirror of https://github.com/hierynomus/sshj.git synced 2025-12-06 15:20:54 +03:00
Code Issues Actions Packages Projects Releases Wiki Activity

Lean on Config.keyAlgorithms choosing between rsa-sha2-* and ssh-rsa (#742)

Browse Source 
* Improve SshdContainer: log `docker build` to stdout, don't wait too long if container exited

* 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.

* Introduce ConfigImpl.prioritizeSshRsaKeyAlgorithm to deal with broken backward compatibility

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
This commit is contained in:
Vladimir Lagunov
2021-12-06 18:14:04 +07:00
committed by GitHub
parent d8697c2228
commit 624747c527
13 changed files with 332 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
build.gradle
View File

@@ -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'

159
src/itest/groovy/com/hierynomus/sshj/RsaShaKeySignatureTest.groovy Normal file
View File

@@ -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()
}
}

2
src/itest/groovy/com/hierynomus/sshj/SshServerWaitStrategy.java
View File

@@ -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];

30
src/itest/groovy/com/hierynomus/sshj/SshdContainer.java
View File

@@ -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 @@ import java.util.concurrent.Future;
* 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 class SshdContainer extends GenericContainer<SshdContainer> {
}
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 class SshdContainer extends GenericContainer<SshdContainer> {
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 {

6
src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java
View File

@@ -27,8 +27,6 @@ import java.util.List;
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 class KeyAlgorithms {
return algorithmName;
}
public KeyType getKeyType() {
return keyType;
}
@Override
public KeyAlgorithm create() {
return new BaseKeyAlgorithm(algorithmName, signatureFactory, keyType);

27
src/main/java/net/schmizz/sshj/ConfigImpl.java
View File

@@ -26,6 +26,7 @@ import net.schmizz.sshj.transport.mac.MAC;
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 class ConfigImpl
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;
}
}
}
}

1
src/main/java/net/schmizz/sshj/transport/KeyExchanger.java
View File

@@ -243,7 +243,6 @@ final class KeyExchanger
negotiatedAlgs.getKeyExchangeAlgorithm());
transport.setHostKeyAlgorithm(Factory.Named.Util.create(transport.getConfig().getKeyAlgorithms(),
negotiatedAlgs.getSignatureAlgorithm()));
transport.setRSASHA2Support(negotiatedAlgs.getRSASHA2Support());
try {
kex.init(transport,

10
src/main/java/net/schmizz/sshj/transport/NegotiatedAlgorithms.java
View File

@@ -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 final class NegotiatedAlgorithms {
return s2cComp;
}
public boolean getRSASHA2Support() {
return rsaSHA2Support;
}
@Override
public String toString() {
return ("[ " +
@@ -88,7 +81,6 @@ public final class NegotiatedAlgorithms {
"s2cMAC=" + s2cMAC + "; " +
"c2sComp=" + c2sComp + "; " +
"s2cComp=" + s2cComp + "; " +
"rsaSHA2Support=" + rsaSHA2Support +
" ]");
}

5
src/main/java/net/schmizz/sshj/transport/Proposal.java
View File

@@ -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 @@ class Proposal {
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) {

15
src/main/java/net/schmizz/sshj/transport/TransportImpl.java
View File

@@ -86,8 +86,6 @@ public final class TransportImpl
private KeyAlgorithm hostKeyAlgorithm;
private boolean rsaSHA2Support;
private final Event<TransportException> serviceAccept;
private final Event<TransportException> close;
@@ -626,20 +624,15 @@ public final class TransportImpl
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<Factory.Named<KeyAlgorithm>> factories = getConfig().getKeyAlgorithms();
if (factories != null)
for (Factory.Named<KeyAlgorithm> 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);
}

11
src/main/java/net/schmizz/sshj/userauth/method/KeyedAuthMethod.java
View File

@@ -51,12 +51,15 @@ public abstract class KeyedAuthMethod
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)

12
src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy
View File

@@ -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,14 @@ 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()
config.prioritizeSshRsaKeyAlgorithm()
and:
SSHClient client = fixture.setupClient(config)
fixture.connectClient(client)
when:
client.authPublickey("jeroen", keyfile)
@@ -48,7 +56,7 @@ class FileKeyProviderSpec extends Specification {
client.isAuthenticated()
cleanup:
client.disconnect()
client?.disconnect()
where:
format | keyfile

87
src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy Normal file
View File

@@ -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]
}
}