Skip to content

Commit 5ff27ec

Browse files
exceptionfactoryexceptionfactory
authored
Refactored ECDSA Key Handling using Java Security (#983)
- Replaced Bouncy Castle with Java Security components for ECDSA Key Specifications
1 parent 31ed354 commit 5ff27ec

File tree

7 files changed

+160
-57
lines changed

7 files changed

+160
-57
lines changed

‎src/main/java/com/hierynomus/sshj/common/KeyDecryptionFailedException.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
package com.hierynomus.sshj.common;
1717

18-
import org.bouncycastle.openssl.EncryptionException;
19-
2018
import java.io.IOException;
2119

2220
/**
@@ -32,7 +30,7 @@ public KeyDecryptionFailedException() {
3230
super(MESSAGE);
3331
}
3432

35-
public KeyDecryptionFailedException(EncryptionException cause) {
33+
public KeyDecryptionFailedException(IOException cause) {
3634
super(MESSAGE, cause);
3735
}
3836

‎src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@
3131
import net.schmizz.sshj.userauth.keyprovider.FileKeyProvider;
3232
import net.schmizz.sshj.userauth.keyprovider.KeyFormat;
3333
import net.schmizz.sshj.userauth.password.PasswordFinder;
34-
import org.bouncycastle.asn1.nist.NISTNamedCurves;
35-
import org.bouncycastle.asn1.x9.X9ECParameters;
36-
import org.bouncycastle.jce.spec.ECNamedCurveSpec;
37-
import org.bouncycastle.openssl.EncryptionException;
3834
import org.slf4j.Logger;
3935
import org.slf4j.LoggerFactory;
4036

@@ -47,15 +43,14 @@
4743
import java.security.KeyPair;
4844
import java.security.PrivateKey;
4945
import java.security.PublicKey;
50-
import java.security.spec.ECPrivateKeySpec;
5146
import java.security.spec.RSAPrivateCrtKeySpec;
5247
import java.util.Arrays;
5348
import java.util.HashMap;
5449
import java.util.Map;
5550

5651
/**
5752
* Reads a key file in the new OpenSSH format.
58-
* The format is described in the following document: https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.key
53+
* The format is described in the following document: <a href="https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.key">Key Protocol</a>
5954
*/
6055
public class OpenSSHKeyV1KeyFile extends BaseFileKeyProvider {
6156
private static final String BEGIN = "-----BEGIN ";
@@ -244,7 +239,7 @@ private PlainBuffer decryptPrivateKey(final byte[] privateKey, final int private
244239
cipher.update(privateKey, 0, privateKeyLength);
245240
} catch (final SSHRuntimeException e) {
246241
final String message = String.format("OpenSSH Private Key decryption failed with cipher [%s]", cipherName);
247-
throw new KeyDecryptionFailedException(new EncryptionException(message, e));
242+
throw new KeyDecryptionFailedException(new IOException(message, e));
248243
}
249244
final PlainBuffer decryptedPrivateKey = new PlainBuffer(privateKeyLength);
250245
decryptedPrivateKey.putRawBytes(privateKey, 0, privateKeyLength);
@@ -343,7 +338,7 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub
343338
int checkInt1 = keyBuffer.readUInt32AsInt(); // uint32 checkint1
344339
int checkInt2 = keyBuffer.readUInt32AsInt(); // uint32 checkint2
345340
if (checkInt1 != checkInt2) {
346-
throw new KeyDecryptionFailedException(new EncryptionException("OpenSSH Private Key integer comparison failed"));
341+
throw new KeyDecryptionFailedException(new IOException("OpenSSH Private Key integer comparison failed"));
347342
}
348343
// The private key section contains both the public key and the private key
349344
String keyType = keyBuffer.readString(); // string keytype
@@ -365,13 +360,13 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub
365360
kp = new KeyPair(publicKey, privateKey);
366361
break;
367362
case ECDSA256:
368-
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, "P-256"));
363+
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, ECDSACurve.SECP256R1));
369364
break;
370365
case ECDSA384:
371-
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, "P-384"));
366+
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, ECDSACurve.SECP384R1));
372367
break;
373368
case ECDSA521:
374-
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, "P-521"));
369+
kp = new KeyPair(publicKey, createECDSAPrivateKey(kt, keyBuffer, ECDSACurve.SECP521R1));
375370
break;
376371

377372
default:
@@ -388,13 +383,10 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub
388383
return kp;
389384
}
390385

391-
private PrivateKey createECDSAPrivateKey(KeyType kt, PlainBuffer buffer, String name) throws GeneralSecurityException, Buffer.BufferException {
386+
private PrivateKey createECDSAPrivateKey(KeyType kt, PlainBuffer buffer, ECDSACurve ecdsaCurve) throws GeneralSecurityException, Buffer.BufferException {
392387
kt.readPubKeyFromBuffer(buffer); // Public key
393-
BigInteger s = new BigInteger(1, buffer.readBytes());
394-
X9ECParameters ecParams = NISTNamedCurves.getByName(name);
395-
ECNamedCurveSpec ecCurveSpec = new ECNamedCurveSpec(name, ecParams.getCurve(), ecParams.getG(), ecParams.getN());
396-
ECPrivateKeySpec pks = new ECPrivateKeySpec(s, ecCurveSpec);
397-
return SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA).generatePrivate(pks);
388+
final BigInteger s = new BigInteger(1, buffer.readBytes());
389+
return ECDSAKeyFactory.getPrivateKey(s, ecdsaCurve);
398390
}
399391

400392
/**

‎src/main/java/net/schmizz/sshj/common/ECDSACurve.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright (C)2009 - SSHJ Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package net.schmizz.sshj.common;
17+
18+
/**
19+
* Enumeration of supported ECDSA Curves with corresponding algorithm parameter names
20+
*/
21+
public enum ECDSACurve {
22+
/** NIST P-256 */
23+
SECP256R1("secp256r1"),
24+
25+
/** NIST P-384 */
26+
SECP384R1("secp384r1"),
27+
28+
/** NIST P-521 */
29+
SECP521R1("secp521r1");
30+
31+
private final String curveName;
32+
33+
ECDSACurve(final String curveName) {
34+
this.curveName = curveName;
35+
}
36+
37+
/**
38+
* Get Curve Name for use with Java Cryptography Architecture components
39+
*
40+
* @return Curve Name
41+
*/
42+
public String getCurveName() {
43+
return curveName;
44+
}
45+
}

‎src/main/java/net/schmizz/sshj/common/ECDSAKeyFactory.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright (C)2009 - SSHJ Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package net.schmizz.sshj.common;
17+
18+
import com.hierynomus.sshj.common.KeyAlgorithm;
19+
20+
import java.math.BigInteger;
21+
import java.security.AlgorithmParameters;
22+
import java.security.GeneralSecurityException;
23+
import java.security.KeyFactory;
24+
import java.security.PrivateKey;
25+
import java.security.PublicKey;
26+
import java.security.spec.ECGenParameterSpec;
27+
import java.security.spec.ECParameterSpec;
28+
import java.security.spec.ECPoint;
29+
import java.security.spec.ECPrivateKeySpec;
30+
import java.security.spec.ECPublicKeySpec;
31+
import java.util.Objects;
32+
33+
/**
34+
* Factory for generating Elliptic Curve Keys using Java Security components for NIST Curves
35+
*/
36+
public class ECDSAKeyFactory {
37+
38+
private ECDSAKeyFactory() {
39+
40+
}
41+
42+
/**
43+
* Get Elliptic Curve Private Key for private key value and Curve Name
44+
*
45+
* @param privateKeyInteger Private Key
46+
* @param ecdsaCurve Elliptic Curve
47+
* @return Elliptic Curve Private Key
48+
* @throws GeneralSecurityException Thrown on failure to create parameter specification
49+
*/
50+
public static PrivateKey getPrivateKey(final BigInteger privateKeyInteger, final ECDSACurve ecdsaCurve) throws GeneralSecurityException {
51+
Objects.requireNonNull(privateKeyInteger, "Private Key integer required");
52+
Objects.requireNonNull(ecdsaCurve, "Curve required");
53+
54+
final ECParameterSpec parameterSpec = getParameterSpec(ecdsaCurve);
55+
final ECPrivateKeySpec privateKeySpec = new ECPrivateKeySpec(privateKeyInteger, parameterSpec);
56+
57+
final KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA);
58+
return keyFactory.generatePrivate(privateKeySpec);
59+
}
60+
61+
/**
62+
* Get Elliptic Curve Public Key for public key value and Curve Name
63+
*
64+
* @param point Public Key point
65+
* @param ecdsaCurve Elliptic Curve
66+
* @return Elliptic Curve Public Key
67+
* @throws GeneralSecurityException Thrown on failure to create parameter specification
68+
*/
69+
public static PublicKey getPublicKey(final ECPoint point, final ECDSACurve ecdsaCurve) throws GeneralSecurityException {
70+
Objects.requireNonNull(point, "Elliptic Curve Point required");
71+
Objects.requireNonNull(ecdsaCurve, "Curve required");
72+
73+
final ECParameterSpec parameterSpec = getParameterSpec(ecdsaCurve);
74+
final ECPublicKeySpec publicKeySpec = new ECPublicKeySpec(point, parameterSpec);
75+
76+
final KeyFactory keyFactory = SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA);
77+
return keyFactory.generatePublic(publicKeySpec);
78+
}
79+
80+
private static ECParameterSpec getParameterSpec(final ECDSACurve ecdsaCurve) throws GeneralSecurityException {
81+
final ECGenParameterSpec genParameterSpec = new ECGenParameterSpec(ecdsaCurve.getCurveName());
82+
final AlgorithmParameters algorithmParameters = AlgorithmParameters.getInstance(KeyAlgorithm.EC_KEYSTORE);
83+
algorithmParameters.init(genParameterSpec);
84+
return algorithmParameters.getParameterSpec(ECParameterSpec.class);
85+
}
86+
}

‎src/main/java/net/schmizz/sshj/common/ECDSAVariationsAdapter.java‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,16 @@
1717

1818
import com.hierynomus.sshj.common.KeyAlgorithm;
1919
import com.hierynomus.sshj.secg.SecgUtils;
20-
import org.bouncycastle.asn1.nist.NISTNamedCurves;
21-
import org.bouncycastle.asn1.x9.X9ECParameters;
22-
import org.bouncycastle.jce.spec.ECNamedCurveSpec;
2320
import org.slf4j.Logger;
2421
import org.slf4j.LoggerFactory;
2522

2623
import java.math.BigInteger;
2724
import java.security.GeneralSecurityException;
2825
import java.security.Key;
29-
import java.security.KeyFactory;
3026
import java.security.PublicKey;
3127
import java.security.interfaces.ECKey;
3228
import java.security.interfaces.ECPublicKey;
3329
import java.security.spec.ECPoint;
34-
import java.security.spec.ECPublicKeySpec;
3530
import java.util.Arrays;
3631
import java.util.HashMap;
3732
import java.util.Map;
@@ -42,13 +37,13 @@ class ECDSAVariationsAdapter {
4237

4338
private final static Logger log = LoggerFactory.getLogger(ECDSAVariationsAdapter.class);
4439

45-
public final static Map<String, String> SUPPORTED_CURVES = new HashMap<String, String>();
46-
public final static Map<String, String> NIST_CURVES_NAMES = new HashMap<String, String>();
40+
public final static Map<String, String> SUPPORTED_CURVES = new HashMap<>();
41+
public final static Map<String, ECDSACurve> NIST_CURVES = new HashMap<>();
4742

4843
static {
49-
NIST_CURVES_NAMES.put("256", "p-256");
50-
NIST_CURVES_NAMES.put("384", "p-384");
51-
NIST_CURVES_NAMES.put("521", "p-521");
44+
NIST_CURVES.put("256", ECDSACurve.SECP256R1);
45+
NIST_CURVES.put("384", ECDSACurve.SECP384R1);
46+
NIST_CURVES.put("521", ECDSACurve.SECP521R1);
5247

5348
SUPPORTED_CURVES.put("256", "nistp256");
5449
SUPPORTED_CURVES.put("384", "nistp384");
@@ -72,21 +67,15 @@ static PublicKey readPubKeyFromBuffer(Buffer<?> buf, String variation) throws Ge
7267
algorithm, curveName, keyLen, x04, Arrays.toString(x), Arrays.toString(y)));
7368
}
7469

75-
if (!SUPPORTED_CURVES.values().contains(curveName)) {
70+
if (!SUPPORTED_CURVES.containsValue(curveName)) {
7671
throw new GeneralSecurityException(String.format("Unknown curve %s", curveName));
7772
}
7873

79-
BigInteger bigX = new BigInteger(1, x);
80-
BigInteger bigY = new BigInteger(1, y);
81-
82-
String name = NIST_CURVES_NAMES.get(variation);
83-
X9ECParameters ecParams = NISTNamedCurves.getByName(name);
84-
ECNamedCurveSpec ecCurveSpec = new ECNamedCurveSpec(name, ecParams.getCurve(), ecParams.getG(), ecParams.getN());
85-
ECPoint p = new ECPoint(bigX, bigY);
86-
ECPublicKeySpec publicKeySpec = new ECPublicKeySpec(p, ecCurveSpec);
87-
88-
KeyFactory keyFactory = KeyFactory.getInstance(KeyAlgorithm.ECDSA);
89-
return keyFactory.generatePublic(publicKeySpec);
74+
final BigInteger bigX = new BigInteger(1, x);
75+
final BigInteger bigY = new BigInteger(1, y);
76+
final ECPoint point = new ECPoint(bigX, bigY);
77+
final ECDSACurve ecdsaCurve = NIST_CURVES.get(variation);
78+
return ECDSAKeyFactory.getPublicKey(point, ecdsaCurve);
9079
} catch (Exception ex) {
9180
throw new GeneralSecurityException(ex);
9281
}
@@ -96,7 +85,7 @@ static void writePubKeyContentsIntoBuffer(PublicKey pk, Buffer<?> buf) {
9685
final ECPublicKey ecdsa = (ECPublicKey) pk;
9786
byte[] encoded = SecgUtils.getEncoded(ecdsa.getW(), ecdsa.getParams().getCurve());
9887

99-
buf.putString("nistp" + Integer.toString(fieldSizeFromKey(ecdsa)))
88+
buf.putString("nistp" + (fieldSizeFromKey(ecdsa)))
10089
.putBytes(encoded);
10190
}
10291

‎src/main/java/net/schmizz/sshj/userauth/keyprovider/PuTTYKeyFile.java‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,8 @@
2424
import net.i2p.crypto.eddsa.spec.EdDSAPublicKeySpec;
2525
import net.schmizz.sshj.common.*;
2626
import net.schmizz.sshj.userauth.password.PasswordUtils;
27-
import org.bouncycastle.asn1.nist.NISTNamedCurves;
28-
import org.bouncycastle.asn1.x9.X9ECParameters;
2927
import org.bouncycastle.crypto.generators.Argon2BytesGenerator;
3028
import org.bouncycastle.crypto.params.Argon2Parameters;
31-
import org.bouncycastle.jce.spec.ECNamedCurveSpec;
3229
import org.bouncycastle.util.encoders.Hex;
3330

3431
import javax.crypto.Cipher;
@@ -173,29 +170,26 @@ protected KeyPair readKeyPair() throws IOException {
173170
EdDSAPrivateKeySpec privateSpec = new EdDSAPrivateKeySpec(privateKeyReader.readBytes(), ed25519);
174171
return new KeyPair(new EdDSAPublicKey(publicSpec), new EdDSAPrivateKey(privateSpec));
175172
}
176-
final String ecdsaCurve;
173+
final ECDSACurve ecdsaCurve;
177174
switch (keyType) {
178175
case ECDSA256:
179-
ecdsaCurve = "P-256";
176+
ecdsaCurve = ECDSACurve.SECP256R1;
180177
break;
181178
case ECDSA384:
182-
ecdsaCurve = "P-384";
179+
ecdsaCurve = ECDSACurve.SECP384R1;
183180
break;
184181
case ECDSA521:
185-
ecdsaCurve = "P-521";
182+
ecdsaCurve = ECDSACurve.SECP521R1;
186183
break;
187184
default:
188185
ecdsaCurve = null;
189186
break;
190187
}
191188
if (ecdsaCurve != null) {
192-
BigInteger s = new BigInteger(1, privateKeyReader.readBytes());
193-
X9ECParameters ecParams = NISTNamedCurves.getByName(ecdsaCurve);
194-
ECNamedCurveSpec ecCurveSpec = new ECNamedCurveSpec(ecdsaCurve, ecParams.getCurve(), ecParams.getG(),
195-
ecParams.getN());
196-
ECPrivateKeySpec pks = new ECPrivateKeySpec(s, ecCurveSpec);
189+
final BigInteger s = new BigInteger(1, privateKeyReader.readBytes());
190+
197191
try {
198-
PrivateKey privateKey = SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA).generatePrivate(pks);
192+
final PrivateKey privateKey = ECDSAKeyFactory.getPrivateKey(s, ecdsaCurve);
199193
return new KeyPair(keyType.readPubKeyFromBuffer(publicKeyReader), privateKey);
200194
} catch (GeneralSecurityException e) {
201195
throw new IOException(e.getMessage(), e);

‎src/test/java/net/schmizz/sshj/keyprovider/PuTTYKeyFileTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,6 @@ public void testEcDsa256() throws Exception {
425425

426426
PKCS8KeyFile referenceKey = new PKCS8KeyFile();
427427
referenceKey.init(new File("src/test/resources/keytypes/test_ecdsa_nistp256"));
428-
assertEquals(key.getPrivate(), referenceKey.getPrivate());
429428
assertEquals(key.getPublic(), referenceKey.getPublic());
430429
}
431430

0 commit comments

Comments
 (0)