From c6c9a3f6a81c733be09038352b853d77c3e4891a Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Tue, 5 Sep 2017 15:23:47 +0200 Subject: [PATCH] Correctly determine KeyType for ECDSA public key (Fixes #356) --- .../sshj/common/ECDSAVariationsAdapter.java | 16 ++++++---- .../java/net/schmizz/sshj/common/KeyType.java | 12 +++----- .../hierynomus/sshj/common/KeyTypeSpec.groovy | 30 +++++++++++++++++++ .../password/ConsolePasswordFinderSpec.groovy | 22 ++++++++++++++ 4 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 src/test/groovy/com/hierynomus/sshj/common/KeyTypeSpec.groovy create mode 100644 src/test/groovy/net/schmizz/sshj/userauth/password/ConsolePasswordFinderSpec.groovy diff --git a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java index bf810fc7..770825f2 100644 --- a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java +++ b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java @@ -17,8 +17,10 @@ package net.schmizz.sshj.common; import java.math.BigInteger; import java.security.GeneralSecurityException; +import java.security.Key; import java.security.KeyFactory; import java.security.PublicKey; +import java.security.interfaces.ECKey; import java.security.interfaces.ECPublicKey; import java.util.Arrays; import java.util.HashMap; @@ -34,7 +36,7 @@ import org.slf4j.LoggerFactory; import com.hierynomus.sshj.secg.SecgUtils; -public class ECDSAVariationsAdapter { +class ECDSAVariationsAdapter { private final static String BASE_ALGORITHM_NAME = "ecdsa-sha2-nistp"; @@ -53,7 +55,7 @@ public class ECDSAVariationsAdapter { SUPPORTED_CURVES.put("521", "nistp521"); } - public static PublicKey readPubKeyFromBuffer(Buffer buf, String variation) throws GeneralSecurityException { + static PublicKey readPubKeyFromBuffer(Buffer buf, String variation) throws GeneralSecurityException { String algorithm = BASE_ALGORITHM_NAME + variation; if (!SecurityUtils.isBouncyCastleRegistered()) { throw new GeneralSecurityException("BouncyCastle is required to read a key of type " + algorithm); @@ -92,7 +94,7 @@ public class ECDSAVariationsAdapter { } } - public static void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { + static void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { final ECPublicKey ecdsa = (ECPublicKey) pk; byte[] encoded = SecgUtils.getEncoded(ecdsa.getW(), ecdsa.getParams().getCurve()); @@ -100,8 +102,12 @@ public class ECDSAVariationsAdapter { .putBytes(encoded); } - public static int fieldSizeFromKey(ECPublicKey ecPublicKey) { - return ecPublicKey.getParams().getCurve().getField().getFieldSize(); + static boolean isECKeyWithFieldSize(Key key, int fieldSize) { + return "ECDSA".equals(key.getAlgorithm()) + && fieldSizeFromKey((ECKey) key) == fieldSize; } + private static int fieldSizeFromKey(ECKey ecPublicKey) { + return ecPublicKey.getParams().getCurve().getField().getFieldSize(); + } } diff --git a/src/main/java/net/schmizz/sshj/common/KeyType.java b/src/main/java/net/schmizz/sshj/common/KeyType.java index a54620f7..c382a3ef 100644 --- a/src/main/java/net/schmizz/sshj/common/KeyType.java +++ b/src/main/java/net/schmizz/sshj/common/KeyType.java @@ -20,11 +20,7 @@ import java.security.GeneralSecurityException; import java.security.Key; import java.security.KeyFactory; import java.security.PublicKey; -import java.security.interfaces.DSAPrivateKey; -import java.security.interfaces.DSAPublicKey; -import java.security.interfaces.ECPublicKey; -import java.security.interfaces.RSAPrivateKey; -import java.security.interfaces.RSAPublicKey; +import java.security.interfaces.*; import java.security.spec.DSAPublicKeySpec; import java.security.spec.RSAPublicKeySpec; import java.util.ArrayList; @@ -130,7 +126,7 @@ public enum KeyType { @Override protected boolean isMyType(Key key) { - return ("ECDSA".equals(key.getAlgorithm()) && ECDSAVariationsAdapter.fieldSizeFromKey((ECPublicKey) key) == 256); + return ECDSAVariationsAdapter.isECKeyWithFieldSize(key, 256); } }, @@ -151,7 +147,7 @@ public enum KeyType { @Override protected boolean isMyType(Key key) { - return ("ECDSA".equals(key.getAlgorithm()) && ECDSAVariationsAdapter.fieldSizeFromKey((ECPublicKey) key) == 384); + return ECDSAVariationsAdapter.isECKeyWithFieldSize(key, 384); } }, @@ -172,7 +168,7 @@ public enum KeyType { @Override protected boolean isMyType(Key key) { - return ("ECDSA".equals(key.getAlgorithm()) && ECDSAVariationsAdapter.fieldSizeFromKey((ECPublicKey) key) == 521); + return ECDSAVariationsAdapter.isECKeyWithFieldSize(key, 521); } }, diff --git a/src/test/groovy/com/hierynomus/sshj/common/KeyTypeSpec.groovy b/src/test/groovy/com/hierynomus/sshj/common/KeyTypeSpec.groovy new file mode 100644 index 00000000..3faa9294 --- /dev/null +++ b/src/test/groovy/com/hierynomus/sshj/common/KeyTypeSpec.groovy @@ -0,0 +1,30 @@ +package com.hierynomus.sshj.common + +import net.schmizz.sshj.common.KeyType +import net.schmizz.sshj.userauth.keyprovider.OpenSSHKeyFile +import spock.lang.Specification +import spock.lang.Unroll + +class KeyTypeSpec extends Specification { + + @Unroll + def "should determine correct keytype for #type key"() { + given: + OpenSSHKeyFile kf = new OpenSSHKeyFile() + kf.init(privKey, pubKey) + + expect: + KeyType.fromKey(kf.getPublic()) == type + KeyType.fromKey(kf.getPrivate()) == type + + where: + privKey << ["""-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIGhcvG8anyHew/xZJfozh5XIc1kmZZs6o2f0l3KFs4jgoAoGCCqGSM49 +AwEHoUQDQgAEDUA1JYVD7URSoOGdwPxjea+ETD6IABMD9CWfk3NVTNkdu/Ksn7uX +cLTQhx4N16z1IgW2bRbSbsmM++UKXmeWyg== +-----END EC PRIVATE KEY-----"""] + pubKey << ["""ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBA1ANSWFQ+1EUqDhncD8Y3mvhEw+iAATA/Qln5NzVUzZHbvyrJ+7l3C00IceDdes9SIFtm0W0m7JjPvlCl5nlso= SSH Key"""] + type << [KeyType.ECDSA256] + + } +} \ No newline at end of file diff --git a/src/test/groovy/net/schmizz/sshj/userauth/password/ConsolePasswordFinderSpec.groovy b/src/test/groovy/net/schmizz/sshj/userauth/password/ConsolePasswordFinderSpec.groovy new file mode 100644 index 00000000..88d68c3e --- /dev/null +++ b/src/test/groovy/net/schmizz/sshj/userauth/password/ConsolePasswordFinderSpec.groovy @@ -0,0 +1,22 @@ +package net.schmizz.sshj.userauth.password + +import spock.lang.Specification + +class ConsolePasswordFinderSpec extends Specification { + + def "should read password from console"() { + given: + def console = Mock(Console) { + readPassword(*_) >> "password".toCharArray() + } + def cpf = new ConsolePasswordFinder(console) + def resource = new AccountResource("test", "localhost") + + when: + def password = cpf.reqPassword(resource) + + then: + password == "password".toCharArray() + + } +}