Fix matching of pubkeys to key algorithms (#607)

* Fix matching of pubkeys to key algorithms

Allow all configured key algorithms for pubkey authentication, even if
these algorithms are not supported as host key algorithms by the
server.

Preference is given to the modern rsa-sha2-* signature algorithms if
the server indicates support for them as host keys signature
algorithms.

* Replace Boolean with primitive boolean

* Add integration tests for ecdsa-sha2-nistp384/521

* Remove redundant import

* Clean up Transport interface

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>
This commit is contained in:
Fabian Henneke
2020-07-28 12:30:29 +02:00
committed by GitHub
parent 4b1619d54f
commit 6becee176a
13 changed files with 86 additions and 34 deletions

View File

@@ -5,3 +5,5 @@ ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJ8ww4hJG/gHJYdkjTTBDF1GNz+228nuWprPV+NbQauA
ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOaWrwt3drIOjeBq2LSHRavxAT7ja2f+5soOUJl/zKSI ajvanerp@Heimdall.xebialabs.com
ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAoZ9l6Tkm2aL1tSBy2yw4xU5s8BE9MfqS/4J7DzvsYJxF6oQmTIjmStuhH/CT7UjuDtKXdXZUsIhKtafiizxGO8kHSzKDeitpth2RSr8ddMzZKyD6RNs7MfsgjA3UTtrrSrCXEY6O43S2cnuJrWzkPxtwxaQ3zOvDbS2tiulzyq0VzYmuhA/a4CyuQtJBuu+P2oqmu6pU/VB6IzONpvBvYbNPsH1WDmP7zko5wHPihXPCliztspKxS4DRtOZ7BGXyvg44UmIy0Kf4jOkaBV/eCCA4qH7ZHz71/5ceMOpszPcNOEmLGGYhwI+P3OuGMpkrSAv1f8IY6R8spZNncP6UaQ== no-passphrase
ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDKRyZAtOJJfAhPU6xE6ZXY564vwErAI3n3Yn4lTHL9bxev9Ily6eCqPLcV0WbSV04pztngFn9MjT7yb8mcXheHpIaWEH569sMpmpOtyfn4p68SceuXBGyyPGMIcfOTknkASd1JYSD4EPkd9rZmCzcx3vEnLu8ChnA/G221xSVQ5VC/jD/c/CgNUayhQ+xbn57qHKKtZwfTa21QmwIabGYJNwlVjlKTCdddeVnZfKqKrG7cxHQApsxd21rhM9IT/C/f4Y/Tx3WUUVeam0iZ265oiPHoPALqJIWSQIUheRYAxYAQqJwSQ0Or9MM8XXun2Iy3RUSGk6eIvrCsFbNURsHNs7Pu0UnpYv6FZ3vCkFep/1pAT6fQvY7pDOOWDHKXArD4watc9gIWaQBH73wDW/KgBcnMRSoGWgQjsYqIamP4oV1+HqUI3lRAsXZaX+eiBGt3+3A5KebP27UJ1YUwhwlzs7wzTKaCu0OaL+hOsP1F2AxAa995bgFksMd23645ux3YCJKXG4sGpJ1Z/Hs49K72gv+QjLZVxXqY623c8+3OUhlixqoEFd4iG7UMc5a552ch/VA+jaspmLZoFhPz99aBRVb1oCSPxSwLw+Q/wxv6pZmT+14rqTzY2farjU53hM+CsUPh7dnWXhGG7RuA5wCdeOXOYjuksfzAoHIZhPqTgQ== ajvanerp@Heimdall.local
ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBMvfRYSe44VQGwxexOMibcM3+fWeUP1jrBofOxFDRRrzRF8dK/vll2svqTPXMRnITnT1UoemEcB5OHtvH4hzfh/HFeDxJ5S7UncYxoClTSa8MeMFG2Zj9CoUZs1SHbwSGg== root@sshj
ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBAHquUYgkU9wJrcxDWVtdqtfqf6SBDdPDRxwDl7OCohV2UNu2KdjJwSj8j0fsPeMdHjSiv9OCnHYrVilQ+W5WW5q5wGXwk10oIcV0JJscohLA0nS7mKinBrxUwVHnNZbPExFciicnEArcYRb1BuT7HF8hfjuSSpWS0rob6kloSSi/jV7ZA== root@sshj

View File

@@ -77,6 +77,8 @@ class IntegrationSpec extends IntegrationBaseSpec {
"id_ed25519_opensshv1_protected" | "sshjtest"
"id_rsa" | null
"id_rsa_opensshv1" | null
"id_ecdsa_nistp384_opensshv1" | null
"id_ecdsa_nistp521_opensshv1" | null
}
def "should not authenticate with wrong key"() {

View File

@@ -0,0 +1,10 @@
-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAiAAAABNlY2RzYS
1zaGEyLW5pc3RwMzg0AAAACG5pc3RwMzg0AAAAYQTL30WEnuOFUBsMXsTjIm3DN/n1nlD9
Y6waHzsRQ0Ua80RfHSv75ZdrL6kz1zEZyE509VKHphHAeTh7bx+Ic34fxxXg8SeUu1J3GM
aApU0mvDHjBRtmY/QqFGbNUh28EhoAAADYHWlHLx1pRy8AAAATZWNkc2Etc2hhMi1uaXN0
cDM4NAAAAAhuaXN0cDM4NAAAAGEEy99FhJ7jhVAbDF7E4yJtwzf59Z5Q/WOsGh87EUNFGv
NEXx0r++WXay+pM9cxGchOdPVSh6YRwHk4e28fiHN+H8cV4PEnlLtSdxjGgKVNJrwx4wUb
ZmP0KhRmzVIdvBIaAAAAMQD3sx28SrtkuhN+Yu06BAoFLMMgneIqguM3jowaz0LWfP1Nhx
Rnh9tNKM6YYvygCggAAAAPZmhlbm5la2VATGFwdG9w
-----END OPENSSH PRIVATE KEY-----

View File

@@ -0,0 +1,12 @@
-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAArAAAABNlY2RzYS
1zaGEyLW5pc3RwNTIxAAAACG5pc3RwNTIxAAAAhQQB6rlGIJFPcCa3MQ1lbXarX6n+kgQ3
Tw0ccA5ezgqIVdlDbtinYycEo/I9H7D3jHR40or/Tgpx2K1YpUPluVluaucBl8JNdKCHFd
CSbHKISwNJ0u5iopwa8VMFR5zWWzxMRXIonJxAK3GEW9Qbk+xxfIX47kkqVktK6G+pJaEk
ov41e2QAAAEQwljQZcJY0GUAAAATZWNkc2Etc2hhMi1uaXN0cDUyMQAAAAhuaXN0cDUyMQ
AAAIUEAeq5RiCRT3AmtzENZW12q1+p/pIEN08NHHAOXs4KiFXZQ27Yp2MnBKPyPR+w94x0
eNKK/04KcditWKVD5blZbmrnAZfCTXSghxXQkmxyiEsDSdLuYqKcGvFTBUec1ls8TEVyKJ
ycQCtxhFvUG5PscXyF+O5JKlZLSuhvqSWhJKL+NXtkAAAAQVXt20tSeLzMU1U2nMv8CEEY
Oyl1WIkGAcRatDBAfsE0+NcJ/eSbPXywWAqCzOElQ5ftNFz9t1kNXwW5qiLaaIBpAAAAD2
ZoZW5uZWtlQExhcHRvcAECAwQ=
-----END OPENSSH PRIVATE KEY-----

View File

@@ -22,8 +22,13 @@ import net.schmizz.sshj.signature.SignatureDSA;
import net.schmizz.sshj.signature.SignatureECDSA;
import net.schmizz.sshj.signature.SignatureRSA;
import java.util.Arrays;
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); }

View File

@@ -231,12 +231,9 @@ final class KeyExchanger
}
kex = Factory.Named.Util.create(transport.getConfig().getKeyExchangeFactories(),
negotiatedAlgs.getKeyExchangeAlgorithm());
List<KeyAlgorithm> keyAlgorithms = new ArrayList<KeyAlgorithm>();
for (String signatureAlgorithm : negotiatedAlgs.getSignatureAlgorithms()) {
keyAlgorithms.add(Factory.Named.Util.create(transport.getConfig().getKeyAlgorithms(), signatureAlgorithm));
}
transport.setKeyAlgorithms(keyAlgorithms);
transport.setHostKeyAlgorithm(Factory.Named.Util.create(transport.getConfig().getKeyAlgorithms(),
negotiatedAlgs.getSignatureAlgorithm()));
transport.setRSASHA2Support(negotiatedAlgs.getRSASHA2Support());
try {
kex.init(transport,

View File

@@ -15,12 +15,10 @@
*/
package net.schmizz.sshj.transport;
import java.util.List;
public final class NegotiatedAlgorithms {
private final String kex;
private final List<String> availableSigs;
private final String sig;
private final String c2sCipher;
private final String s2cCipher;
private final String c2sMAC;
@@ -28,24 +26,27 @@ public final class NegotiatedAlgorithms {
private final String c2sComp;
private final String s2cComp;
NegotiatedAlgorithms(String kex, List<String> availableSigs, String c2sCipher, String s2cCipher, String c2sMAC, String s2cMAC,
String c2sComp, 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) {
this.kex = kex;
this.availableSigs = availableSigs;
this.sig = sig;
this.c2sCipher = c2sCipher;
this.s2cCipher = s2cCipher;
this.c2sMAC = c2sMAC;
this.s2cMAC = s2cMAC;
this.c2sComp = c2sComp;
this.s2cComp = s2cComp;
this.rsaSHA2Support = rsaSHA2Support;
}
public String getKeyExchangeAlgorithm() {
return kex;
}
public List<String> getSignatureAlgorithms() {
return availableSigs;
public String getSignatureAlgorithm() {
return sig;
}
public String getClient2ServerCipherAlgorithm() {
@@ -72,17 +73,22 @@ public final class NegotiatedAlgorithms {
return s2cComp;
}
public boolean getRSASHA2Support() {
return rsaSHA2Support;
}
@Override
public String toString() {
return ("[ " +
"kex=" + kex + "; " +
"availableSigs=" + availableSigs + "; " +
"sig=" + sig + "; " +
"c2sCipher=" + c2sCipher + "; " +
"s2cCipher=" + s2cCipher + "; " +
"c2sMAC=" + c2sMAC + "; " +
"s2cMAC=" + s2cMAC + "; " +
"c2sComp=" + c2sComp + "; " +
"s2cComp=" + s2cComp +
"s2cComp=" + s2cComp + "; " +
"rsaSHA2Support=" + rsaSHA2Support +
" ]");
}

View File

@@ -15,6 +15,7 @@
*/
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;
@@ -91,7 +92,7 @@ class Proposal {
return kex;
}
public List<String> getSignatureAlgorithms() {
public List<String> getHostKeyAlgorithms() {
return sig;
}
@@ -127,13 +128,14 @@ class Proposal {
throws TransportException {
return new NegotiatedAlgorithms(
firstMatch(this.getKeyExchangeAlgorithms(), other.getKeyExchangeAlgorithms()),
allMatch(this.getSignatureAlgorithms(), other.getSignatureAlgorithms()),
firstMatch(this.getHostKeyAlgorithms(), other.getHostKeyAlgorithms()),
firstMatch(this.getClient2ServerCipherAlgorithms(), other.getClient2ServerCipherAlgorithms()),
firstMatch(this.getServer2ClientCipherAlgorithms(), other.getServer2ClientCipherAlgorithms()),
firstMatch(this.getClient2ServerMACAlgorithms(), other.getClient2ServerMACAlgorithms()),
firstMatch(this.getServer2ClientMACAlgorithms(), other.getServer2ClientMACAlgorithms()),
firstMatch(this.getClient2ServerCompressionAlgorithms(), other.getClient2ServerCompressionAlgorithms()),
firstMatch(this.getServer2ClientCompressionAlgorithms(), other.getServer2ClientCompressionAlgorithms())
firstMatch(this.getServer2ClientCompressionAlgorithms(), other.getServer2ClientCompressionAlgorithms()),
other.getHostKeyAlgorithms().containsAll(KeyAlgorithms.SSH_RSA_SHA2_ALGORITHMS)
);
}

View File

@@ -238,5 +238,6 @@ public interface Transport
*/
void die(Exception e);
KeyAlgorithm getKeyAlgorithm(KeyType keyType) throws TransportException;
KeyAlgorithm getHostKeyAlgorithm();
KeyAlgorithm getClientKeyAlgorithm(KeyType keyType) throws TransportException;
}

View File

@@ -16,6 +16,7 @@
package net.schmizz.sshj.transport;
import com.hierynomus.sshj.key.KeyAlgorithm;
import com.hierynomus.sshj.key.KeyAlgorithms;
import com.hierynomus.sshj.transport.IdentificationStringParser;
import net.schmizz.concurrent.ErrorDeliveryUtil;
import net.schmizz.concurrent.Event;
@@ -89,7 +90,9 @@ public final class TransportImpl
private final Decoder decoder;
private List<KeyAlgorithm> keyAlgorithms;
private KeyAlgorithm hostKeyAlgorithm;
private boolean rsaSHA2Support;
private final Event<TransportException> serviceAccept;
@@ -657,18 +660,30 @@ public final class TransportImpl
return connInfo;
}
@Override
public KeyAlgorithm getKeyAlgorithm(KeyType keyType) throws TransportException {
for (KeyAlgorithm ka : keyAlgorithms) {
if (ka.getKeyFormat().equals(keyType)) {
return ka;
}
public void setHostKeyAlgorithm(KeyAlgorithm keyAlgorithm) {
this.hostKeyAlgorithm = keyAlgorithm;
}
@Override
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<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()))
return f.create();
throw new TransportException("Cannot find an available KeyAlgorithm for type " + keyType);
}
public void setKeyAlgorithms(List<KeyAlgorithm> keyAlgorithms) {
this.keyAlgorithms = keyAlgorithms;
}
}

View File

@@ -79,7 +79,7 @@ public abstract class AbstractDHG extends AbstractDH
H = digest.digest();
Signature signature = trans.getKeyAlgorithm(KeyType.fromKey(hostKey)).newSignature();
Signature signature = trans.getHostKeyAlgorithm().newSignature();
signature.initVerify(hostKey);
signature.update(H, 0, H.length);
if (!signature.verify(sig))

View File

@@ -85,7 +85,7 @@ public abstract class AbstractDHGex extends AbstractDH {
.putMPInt(k);
digest.update(buf.array(), buf.rpos(), buf.available());
H = digest.digest();
KeyAlgorithm keyAlgorithm = trans.getKeyAlgorithm(KeyType.fromKey(hostKey));
KeyAlgorithm keyAlgorithm = trans.getHostKeyAlgorithm();
Signature signature = keyAlgorithm.newSignature();
signature.initVerify(hostKey);
signature.update(H, 0, H.length);

View File

@@ -50,7 +50,7 @@ public abstract class KeyedAuthMethod
// public key as 2 strings: [ key type | key blob ]
KeyType keyType = KeyType.fromKey(key);
try {
KeyAlgorithm ka = params.getTransport().getKeyAlgorithm(keyType);
KeyAlgorithm ka = params.getTransport().getClientKeyAlgorithm(keyType);
reqBuf.putString(ka.getKeyAlgorithm())
.putString(new Buffer.PlainBuffer().putPublicKey(key).getCompactData());
return reqBuf;
@@ -71,7 +71,7 @@ public abstract class KeyedAuthMethod
final KeyType kt = KeyType.fromKey(key);
Signature signature;
try {
signature = params.getTransport().getKeyAlgorithm(kt).newSignature();
signature = params.getTransport().getClientKeyAlgorithm(kt).newSignature();
} catch (TransportException e) {
throw new UserAuthException("No KeyAlgorithm configured for key " + kt);
}