Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix matching of pubkeys to key algorithms #607

Merged
merged 9 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/itest/docker-image/authorized_keys
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions src/itest/groovy/com/hierynomus/sshj/IntegrationSpec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -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"() {
Expand Down
10 changes: 10 additions & 0 deletions src/itest/resources/keyfiles/id_ecdsa_nistp384_opensshv1
Original file line number Diff line number Diff line change
@@ -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-----
12 changes: 12 additions & 0 deletions src/itest/resources/keyfiles/id_ecdsa_nistp521_opensshv1
Original file line number Diff line number Diff line change
@@ -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-----
5 changes: 5 additions & 0 deletions src/main/java/com/hierynomus/sshj/key/KeyAlgorithms.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@
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); }
Expand Down
9 changes: 3 additions & 6 deletions src/main/java/net/schmizz/sshj/transport/KeyExchanger.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,9 @@ private void gotKexInit(SSHPacket buf)
}
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,38 @@
*/
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;
private final String s2cMAC;
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() {
Expand All @@ -72,17 +73,22 @@ public String getServer2ClientCompressionAlgorithm() {
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 +
" ]");
}

Expand Down
8 changes: 5 additions & 3 deletions src/main/java/net/schmizz/sshj/transport/Proposal.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -91,7 +92,7 @@ public List<String> getKeyExchangeAlgorithms() {
return kex;
}

public List<String> getSignatureAlgorithms() {
public List<String> getHostKeyAlgorithms() {
return sig;
}

Expand Down Expand Up @@ -127,13 +128,14 @@ public NegotiatedAlgorithms negotiate(Proposal other)
throws TransportException {
return new NegotiatedAlgorithms(
firstMatch(this.getKeyExchangeAlgorithms(), other.getKeyExchangeAlgorithms()),
allMatch(this.getSignatureAlgorithms(), other.getSignatureAlgorithms()),
firstMatch(this.getHostKeyAlgorithms(), other.getHostKeyAlgorithms()),
fmeum marked this conversation as resolved.
Show resolved Hide resolved
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)
);
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/net/schmizz/sshj/transport/Transport.java
Original file line number Diff line number Diff line change
Expand Up @@ -238,5 +238,6 @@ long write(SSHPacket payload)
*/
void die(Exception e);

KeyAlgorithm getKeyAlgorithm(KeyType keyType) throws TransportException;
KeyAlgorithm getHostKeyAlgorithm();
KeyAlgorithm getClientKeyAlgorithm(KeyType keyType) throws TransportException;
}
35 changes: 25 additions & 10 deletions src/main/java/net/schmizz/sshj/transport/TransportImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -89,7 +90,9 @@ static final class ConnInfo {

private final Decoder decoder;

private List<KeyAlgorithm> keyAlgorithms;
private KeyAlgorithm hostKeyAlgorithm;

private boolean rsaSHA2Support;

private final Event<TransportException> serviceAccept;

Expand Down Expand Up @@ -657,18 +660,30 @@ ConnInfo getConnInfo() {
return connInfo;
}

public void setHostKeyAlgorithm(KeyAlgorithm keyAlgorithm) {
this.hostKeyAlgorithm = keyAlgorithm;
}

@Override
public KeyAlgorithm getKeyAlgorithm(KeyType keyType) throws TransportException {
for (KeyAlgorithm ka : keyAlgorithms) {
if (ka.getKeyFormat().equals(keyType)) {
return ka;
}
}
public KeyAlgorithm getHostKeyAlgorithm() {
return this.hostKeyAlgorithm;
}

throw new TransportException("Cannot find an available KeyAlgorithm for type " + keyType);
public void setRSASHA2Support(boolean rsaSHA2Support) {
this.rsaSHA2Support = rsaSHA2Support;
}

public void setKeyAlgorithms(List<KeyAlgorithm> keyAlgorithms) {
this.keyAlgorithms = keyAlgorithms;
@Override
public KeyAlgorithm getClientKeyAlgorithm(KeyType keyType) throws TransportException {
fmeum marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public boolean next(Message msg, SSHPacket packet)
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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private boolean parseGexReply(SSHPacket buffer) throws Buffer.BufferException, G
.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected SSHPacket putPubKey(SSHPacket reqBuf)
// 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;
Expand All @@ -71,7 +71,7 @@ protected SSHPacket putSig(SSHPacket reqBuf)
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);
}
Expand Down