Skip to content

Commit

Permalink
Introduce ConfigImpl.prioritizeSshRsaKeyAlgorithm to deal with broken…
Browse files Browse the repository at this point in the history
… backward compatibility
  • Loading branch information
vladimirlagunov committed Dec 6, 2021
1 parent 7d01e7e commit 592b612
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 5 deletions.
27 changes: 27 additions & 0 deletions src/main/java/net/schmizz/sshj/ConfigImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -59,7 +56,7 @@ class FileKeyProviderSpec extends Specification {
client.isAuthenticated()

cleanup:
client.disconnect()
client?.disconnect()

where:
format | keyfile
Expand Down
87 changes: 87 additions & 0 deletions src/test/groovy/net/schmizz/sshj/ConfigImplSpec.groovy
Original file line number Diff line number Diff line change
@@ -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]
}
}

0 comments on commit 592b612

Please sign in to comment.