Correctly determine KeyType for ECDSA public key (Fixes #356)

hierynomus · Sep 5, 2017 · c6c9a3f · c6c9a3f
1 parent 0918bc6
commit c6c9a3f
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 13 deletions.
diff --git a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java
@@ -17,8 +17,10 @@
 
 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 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,16 +94,20 @@ public static PublicKey readPubKeyFromBuffer(Buffer<?> buf, String variation) th
         }
     }
 
-    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());
 
         buf.putString("nistp" + Integer.toString(fieldSizeFromKey(ecdsa)))
             .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
@@ -20,11 +20,7 @@
 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 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer<?> buf) {
 
         @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 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer<?> buf) {
 
         @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 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer<?> buf) {
 
         @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
@@ -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]
+
+    }
+}
diff --git a/src/test/groovy/net/schmizz/sshj/userauth/password/ConsolePasswordFinderSpec.groovy 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()
+
+    }
+}