From e52396641a86e1238523071730b9e74aab5d59c4 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Tue, 26 May 2020 14:40:39 +0200 Subject: [PATCH 1/3] Make KeyType compatible with Android Keystore Android Keystore private keys do not implement PrivateKey since the raw key material is not available to applications. With this commit, sshj's KeyType correctly detects the algorithm associated with Android Keystore keys, which makes them usable for SSH authentication. --- .../java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java | 2 +- src/main/java/net/schmizz/sshj/common/KeyType.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java index 8e7d80032..c1bf89e82 100644 --- a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java +++ b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java @@ -103,7 +103,7 @@ static void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { } static boolean isECKeyWithFieldSize(Key key, int fieldSize) { - return "ECDSA".equals(key.getAlgorithm()) + return ("ECDSA".equals(key.getAlgorithm()) || "EC".equals(key.getAlgorithm())) && fieldSizeFromKey((ECKey) key) == fieldSize; } diff --git a/src/main/java/net/schmizz/sshj/common/KeyType.java b/src/main/java/net/schmizz/sshj/common/KeyType.java index 2e8520f45..393c85edb 100644 --- a/src/main/java/net/schmizz/sshj/common/KeyType.java +++ b/src/main/java/net/schmizz/sshj/common/KeyType.java @@ -66,7 +66,7 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { @Override protected boolean isMyType(Key key) { - return (key instanceof RSAPublicKey || key instanceof RSAPrivateKey); + return "RSA".equals(key.getAlgorithm()); } }, @@ -99,7 +99,7 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { @Override protected boolean isMyType(Key key) { - return (key instanceof DSAPublicKey || key instanceof DSAPrivateKey); + return "DSA".equals(key.getAlgorithm()); } }, From 07afbbc05209e3f86f1b063671ae1d97a741038f Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Thu, 28 May 2020 08:01:09 +0200 Subject: [PATCH 2/3] Extract RSA, DSA, ECDSA and EC into constants --- .../com/hierynomus/sshj/common/KeyAlgorithm.java | 16 ++++++++++++++++ .../keyprovider/OpenSSHKeyV1KeyFile.java | 5 +++-- .../sshj/common/ECDSAVariationsAdapter.java | 5 +++-- .../java/net/schmizz/sshj/common/KeyType.java | 11 +++++------ .../schmizz/sshj/transport/kex/Curve25519DH.java | 3 ++- .../net/schmizz/sshj/transport/kex/ECDH.java | 3 ++- .../verification/OpenSSHKnownHosts.java | 3 ++- .../sshj/userauth/keyprovider/PKCS5KeyFile.java | 5 +++-- .../sshj/userauth/keyprovider/PuTTYKeyFile.java | 5 +++-- .../sshj/signature/SignatureDSASpec.groovy | 3 ++- .../schmizz/sshj/signature/SignatureDSATest.java | 3 ++- src/test/java/net/schmizz/sshj/util/KeyUtil.java | 9 +++++---- 12 files changed, 48 insertions(+), 23 deletions(-) create mode 100644 src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java diff --git a/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java b/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java new file mode 100644 index 000000000..b874ee726 --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java @@ -0,0 +1,16 @@ +package com.hierynomus.sshj.common; + +public class KeyAlgorithm { + + public static final String RSA = "RSA"; + public static final String DSA = "DSA"; + + /** Elliptic curve signature key algorithm for use with BouncyCastle **/ + public static final String ECDSA = "ECDSA"; + + /** General elliptic curve algorithm identifier for use with BouncyCastle **/ + public static final String EC_BC = "EC"; + + /** General elliptic curve algorithm identifier for use with the Android Keystore **/ + public static final String EC_KEYSTORE = "EC"; +} diff --git a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java index b2b8b46ea..bdc5d717c 100644 --- a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java +++ b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java @@ -15,6 +15,7 @@ */ package com.hierynomus.sshj.userauth.keyprovider; +import com.hierynomus.sshj.common.KeyAlgorithm; import com.hierynomus.sshj.transport.cipher.BlockCiphers; import net.i2p.crypto.eddsa.EdDSAPrivateKey; import net.i2p.crypto.eddsa.spec.EdDSANamedCurveTable; @@ -207,7 +208,7 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub keyBuffer.readMPInt(); // iqmp (q^-1 mod p) keyBuffer.readMPInt(); // p (Prime 1) keyBuffer.readMPInt(); // q (Prime 2) - kp = new KeyPair(publicKey, SecurityUtils.getKeyFactory("RSA").generatePrivate(new RSAPrivateKeySpec(n, d))); + kp = new KeyPair(publicKey, SecurityUtils.getKeyFactory(KeyAlgorithm.RSA).generatePrivate(new RSAPrivateKeySpec(n, d))); break; case ECDSA256: kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, "P-256")); @@ -239,7 +240,7 @@ private PrivateKey createECDSAPrivateKey(KeyType kt, PlainBuffer buffer, String X9ECParameters ecParams = NISTNamedCurves.getByName(name); ECNamedCurveSpec ecCurveSpec = new ECNamedCurveSpec(name, ecParams.getCurve(), ecParams.getG(), ecParams.getN()); ECPrivateKeySpec pks = new ECPrivateKeySpec(s, ecCurveSpec); - return SecurityUtils.getKeyFactory("ECDSA").generatePrivate(pks); + return SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA).generatePrivate(pks); } } diff --git a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java index c1bf89e82..aba0c7485 100644 --- a/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java +++ b/src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.common; +import com.hierynomus.sshj.common.KeyAlgorithm; import com.hierynomus.sshj.secg.SecgUtils; import org.bouncycastle.asn1.nist.NISTNamedCurves; import org.bouncycastle.asn1.x9.X9ECParameters; @@ -87,7 +88,7 @@ static PublicKey readPubKeyFromBuffer(Buffer buf, String variation) throws Ge ECPoint p = new ECPoint(bigX, bigY); ECPublicKeySpec publicKeySpec = new ECPublicKeySpec(p, ecCurveSpec); - KeyFactory keyFactory = KeyFactory.getInstance("ECDSA"); + KeyFactory keyFactory = KeyFactory.getInstance(KeyAlgorithm.ECDSA); return keyFactory.generatePublic(publicKeySpec); } catch (Exception ex) { throw new GeneralSecurityException(ex); @@ -103,7 +104,7 @@ static void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { } static boolean isECKeyWithFieldSize(Key key, int fieldSize) { - return ("ECDSA".equals(key.getAlgorithm()) || "EC".equals(key.getAlgorithm())) + return (KeyAlgorithm.ECDSA.equals(key.getAlgorithm()) || KeyAlgorithm.EC_KEYSTORE.equals(key.getAlgorithm())) && fieldSizeFromKey((ECKey) key) == fieldSize; } diff --git a/src/main/java/net/schmizz/sshj/common/KeyType.java b/src/main/java/net/schmizz/sshj/common/KeyType.java index 393c85edb..6e841c529 100644 --- a/src/main/java/net/schmizz/sshj/common/KeyType.java +++ b/src/main/java/net/schmizz/sshj/common/KeyType.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.common; +import com.hierynomus.sshj.common.KeyAlgorithm; import com.hierynomus.sshj.signature.Ed25519PublicKey; import com.hierynomus.sshj.userauth.certificate.Certificate; import net.i2p.crypto.eddsa.EdDSAPublicKey; @@ -30,9 +31,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.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; import java.security.spec.DSAPublicKeySpec; import java.security.spec.RSAPublicKeySpec; @@ -53,7 +52,7 @@ public PublicKey readPubKeyFromBuffer(Buffer buf) } catch (Buffer.BufferException be) { throw new GeneralSecurityException(be); } - final KeyFactory keyFactory = SecurityUtils.getKeyFactory("RSA"); + final KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.RSA); return keyFactory.generatePublic(new RSAPublicKeySpec(n, e)); } @@ -66,7 +65,7 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { @Override protected boolean isMyType(Key key) { - return "RSA".equals(key.getAlgorithm()); + return KeyAlgorithm.RSA.equals(key.getAlgorithm()); } }, @@ -84,7 +83,7 @@ public PublicKey readPubKeyFromBuffer(Buffer buf) } catch (Buffer.BufferException be) { throw new GeneralSecurityException(be); } - final KeyFactory keyFactory = SecurityUtils.getKeyFactory("DSA"); + final KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.DSA); return keyFactory.generatePublic(new DSAPublicKeySpec(y, p, q, g)); } @@ -99,7 +98,7 @@ protected void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer buf) { @Override protected boolean isMyType(Key key) { - return "DSA".equals(key.getAlgorithm()); + return KeyAlgorithm.DSA.equals(key.getAlgorithm()); } }, diff --git a/src/main/java/net/schmizz/sshj/transport/kex/Curve25519DH.java b/src/main/java/net/schmizz/sshj/transport/kex/Curve25519DH.java index 01bd22381..a82971a27 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/Curve25519DH.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/Curve25519DH.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.transport.kex; +import com.hierynomus.sshj.common.KeyAlgorithm; import net.schmizz.sshj.common.Factory; import net.schmizz.sshj.transport.random.Random; import org.bouncycastle.asn1.x9.X9ECParameters; @@ -31,7 +32,7 @@ public class Curve25519DH extends DHBase { private byte[] secretKey; public Curve25519DH() { - super("ECDSA", "ECDH"); + super(KeyAlgorithm.ECDSA, "ECDH"); } @Override diff --git a/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java b/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java index 0f35a83e1..42eb322dd 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.transport.kex; +import com.hierynomus.sshj.common.KeyAlgorithm; import net.schmizz.sshj.common.Factory; import net.schmizz.sshj.common.SecurityUtils; import net.schmizz.sshj.transport.random.Random; @@ -54,7 +55,7 @@ public void init(AlgorithmParameterSpec params, Factory randomFactory) t @Override public void computeK(byte[] f) throws GeneralSecurityException { - KeyFactory keyFactory = SecurityUtils.getKeyFactory("EC"); + KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.EC_BC); ECPublicKeySpec keySpec = new ECPublicKeySpec(getDecoded(f, ecParameterSpec.getCurve()), ecParameterSpec); PublicKey yourPubKey = keyFactory.generatePublic(keySpec); agreement.doPhase(yourPubKey, true); diff --git a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java index b04f95932..742e5f147 100644 --- a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java +++ b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.transport.verification; +import com.hierynomus.sshj.common.KeyAlgorithm; import com.hierynomus.sshj.transport.verification.KnownHostMatchers; import net.schmizz.sshj.common.*; import org.slf4j.Logger; @@ -239,7 +240,7 @@ public KnownHostEntry parseEntry(String line) final BigInteger e = new BigInteger(split[i++]); final BigInteger n = new BigInteger(split[i++]); try { - final KeyFactory keyFactory = SecurityUtils.getKeyFactory("RSA"); + final KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.RSA); key = keyFactory.generatePublic(new RSAPublicKeySpec(n, e)); } catch (Exception ex) { log.error("Error reading entry `{}`, could not create key", line, ex); diff --git a/src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS5KeyFile.java b/src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS5KeyFile.java index 8f4068d48..fe8eb9f58 100644 --- a/src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS5KeyFile.java +++ b/src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS5KeyFile.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.userauth.keyprovider; +import com.hierynomus.sshj.common.KeyAlgorithm; import com.hierynomus.sshj.transport.cipher.BlockCiphers; import net.schmizz.sshj.common.Base64; import net.schmizz.sshj.common.ByteArrayUtils; @@ -140,7 +141,7 @@ protected KeyPair readKeyPair() ASN1Data asn = new ASN1Data(data = decrypt(Base64.decode(sb.toString()), cipher, iv)); switch (type) { case RSA: { - KeyFactory factory = KeyFactory.getInstance("RSA"); + KeyFactory factory = KeyFactory.getInstance(KeyAlgorithm.RSA); asn.readNext(); BigInteger modulus = asn.readNext(); BigInteger pubExp = asn.readNext(); @@ -150,7 +151,7 @@ protected KeyPair readKeyPair() return new KeyPair(pubKey, prvKey); } case DSA: { - KeyFactory factory = KeyFactory.getInstance("DSA"); + KeyFactory factory = KeyFactory.getInstance(KeyAlgorithm.DSA); asn.readNext(); BigInteger p = asn.readNext(); BigInteger q = asn.readNext(); diff --git a/src/main/java/net/schmizz/sshj/userauth/keyprovider/PuTTYKeyFile.java b/src/main/java/net/schmizz/sshj/userauth/keyprovider/PuTTYKeyFile.java index 5a8ec6819..b36fa774e 100644 --- a/src/main/java/net/schmizz/sshj/userauth/keyprovider/PuTTYKeyFile.java +++ b/src/main/java/net/schmizz/sshj/userauth/keyprovider/PuTTYKeyFile.java @@ -15,6 +15,7 @@ */ package net.schmizz.sshj.userauth.keyprovider; +import com.hierynomus.sshj.common.KeyAlgorithm; import net.schmizz.sshj.common.Base64; import net.schmizz.sshj.common.KeyType; import net.schmizz.sshj.userauth.password.PasswordUtils; @@ -114,7 +115,7 @@ protected KeyPair readKeyPair() throws IOException { final KeyFactory factory; try { - factory = KeyFactory.getInstance("RSA"); + factory = KeyFactory.getInstance(KeyAlgorithm.RSA); } catch (NoSuchAlgorithmException s) { throw new IOException(s.getMessage(), s); } @@ -141,7 +142,7 @@ protected KeyPair readKeyPair() throws IOException { final KeyFactory factory; try { - factory = KeyFactory.getInstance("DSA"); + factory = KeyFactory.getInstance(KeyAlgorithm.DSA); } catch (NoSuchAlgorithmException s) { throw new IOException(s.getMessage(), s); } diff --git a/src/test/groovy/net/schmizz/sshj/signature/SignatureDSASpec.groovy b/src/test/groovy/net/schmizz/sshj/signature/SignatureDSASpec.groovy index 8913b7f5c..3032b2a73 100644 --- a/src/test/groovy/net/schmizz/sshj/signature/SignatureDSASpec.groovy +++ b/src/test/groovy/net/schmizz/sshj/signature/SignatureDSASpec.groovy @@ -30,6 +30,7 @@ */ package net.schmizz.sshj.signature +import com.hierynomus.sshj.common.KeyAlgorithm import spock.lang.Unroll; import java.math.BigInteger; @@ -47,7 +48,7 @@ import spock.lang.Specification class SignatureDSASpec extends Specification { - def keyFactory = KeyFactory.getInstance("DSA") + def keyFactory = KeyFactory.getInstance(KeyAlgorithm.DSA) private PublicKey createPublicKey(final byte[] y, final byte[] p, final byte[] q, final byte[] g) throws Exception { final BigInteger publicKey = new BigInteger(y); diff --git a/src/test/java/net/schmizz/sshj/signature/SignatureDSATest.java b/src/test/java/net/schmizz/sshj/signature/SignatureDSATest.java index 339e13a97..37c7bfd46 100644 --- a/src/test/java/net/schmizz/sshj/signature/SignatureDSATest.java +++ b/src/test/java/net/schmizz/sshj/signature/SignatureDSATest.java @@ -22,6 +22,7 @@ import java.security.spec.DSAPublicKeySpec; import java.util.Arrays; +import com.hierynomus.sshj.common.KeyAlgorithm; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -35,7 +36,7 @@ public class SignatureDSATest { @Before public void setUp() throws NoSuchAlgorithmException { - keyFactory = KeyFactory.getInstance("DSA"); + keyFactory = KeyFactory.getInstance(KeyAlgorithm.DSA); } @Test diff --git a/src/test/java/net/schmizz/sshj/util/KeyUtil.java b/src/test/java/net/schmizz/sshj/util/KeyUtil.java index b7a27c89e..56c0f1ac4 100644 --- a/src/test/java/net/schmizz/sshj/util/KeyUtil.java +++ b/src/test/java/net/schmizz/sshj/util/KeyUtil.java @@ -30,6 +30,7 @@ */ package net.schmizz.sshj.util; +import com.hierynomus.sshj.common.KeyAlgorithm; import net.schmizz.sshj.common.SecurityUtils; import java.math.BigInteger; @@ -46,7 +47,7 @@ public class KeyUtil { /** Creates a DSA private key. */ public static PrivateKey newDSAPrivateKey(String x, String p, String q, String g) throws GeneralSecurityException { - return SecurityUtils.getKeyFactory("DSA").generatePrivate(new DSAPrivateKeySpec(new BigInteger(x, 16), + return SecurityUtils.getKeyFactory(KeyAlgorithm.DSA).generatePrivate(new DSAPrivateKeySpec(new BigInteger(x, 16), new BigInteger(p, 16), new BigInteger(q, 16), new BigInteger(g, 16)) @@ -56,7 +57,7 @@ public static PrivateKey newDSAPrivateKey(String x, String p, String q, String g /** Creates a DSA public key. */ public static PublicKey newDSAPublicKey(String y, String p, String q, String g) throws GeneralSecurityException { - return SecurityUtils.getKeyFactory("DSA").generatePublic(new DSAPublicKeySpec(new BigInteger(y, 16), + return SecurityUtils.getKeyFactory(KeyAlgorithm.DSA).generatePublic(new DSAPublicKeySpec(new BigInteger(y, 16), new BigInteger(p, 16), new BigInteger(q, 16), new BigInteger(g, 16)) @@ -66,7 +67,7 @@ public static PublicKey newDSAPublicKey(String y, String p, String q, String g) /** Creates an RSA private key. */ public static PrivateKey newRSAPrivateKey(String modulus, String exponent) throws GeneralSecurityException { - return SecurityUtils.getKeyFactory("RSA").generatePrivate(new RSAPrivateKeySpec(new BigInteger(modulus, 16), + return SecurityUtils.getKeyFactory(KeyAlgorithm.RSA).generatePrivate(new RSAPrivateKeySpec(new BigInteger(modulus, 16), new BigInteger(exponent, 16)) ); } @@ -74,7 +75,7 @@ public static PrivateKey newRSAPrivateKey(String modulus, String exponent) /** Creates an RSA public key. */ public static PublicKey newRSAPublicKey(String modulus, String exponent) throws GeneralSecurityException { - return SecurityUtils.getKeyFactory("RSA").generatePublic(new RSAPublicKeySpec(new BigInteger(modulus, 16), + return SecurityUtils.getKeyFactory(KeyAlgorithm.RSA).generatePublic(new RSAPublicKeySpec(new BigInteger(modulus, 16), new BigInteger(exponent, 16))); } From 87d9aa3b3a937366182dabecb11e78f31a301c33 Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Thu, 28 May 2020 08:06:12 +0200 Subject: [PATCH 3/3] Fix license lint issue --- .../com/hierynomus/sshj/common/KeyAlgorithm.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java b/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java index b874ee726..852e05d1a 100644 --- a/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java +++ b/src/main/java/com/hierynomus/sshj/common/KeyAlgorithm.java @@ -1,3 +1,18 @@ +/* + * 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 com.hierynomus.sshj.common; public class KeyAlgorithm {