Skip to content

Commit a32e9b6

Browse files
vanitasvitaevanitasvitae
committed
Fix conversion of ECDSA keys
1 parent 8e6038e commit a32e9b6

File tree

3 files changed

+100
-53
lines changed

3 files changed

+100
-53
lines changed

‎pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java

Lines changed: 60 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
import java.security.spec.RSAPublicKeySpec;
3030
import java.security.spec.X509EncodedKeySpec;
3131
import java.util.Date;
32+
import java.util.Enumeration;
3233

3334
import javax.crypto.interfaces.DHPrivateKey;
3435
import javax.crypto.interfaces.DHPublicKey;
3536
import javax.crypto.spec.DHParameterSpec;
3637
import javax.crypto.spec.DHPrivateKeySpec;
3738
import javax.crypto.spec.DHPublicKeySpec;
3839

40+
import org.bouncycastle.asn1.ASN1Encodable;
3941
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
4042
import org.bouncycastle.asn1.ASN1OctetString;
4143
import org.bouncycastle.asn1.DEROctetString;
@@ -49,6 +51,7 @@
4951
import org.bouncycastle.asn1.x9.X9ECParameters;
5052
import org.bouncycastle.asn1.x9.X9ECParametersHolder;
5153
import org.bouncycastle.asn1.x9.X9ECPoint;
54+
import org.bouncycastle.asn1.x9.X9ObjectIdentifiers;
5255
import org.bouncycastle.bcpg.BCPGKey;
5356
import org.bouncycastle.bcpg.DSAPublicBCPGKey;
5457
import org.bouncycastle.bcpg.DSASecretBCPGKey;
@@ -72,6 +75,7 @@
7275
import org.bouncycastle.bcpg.X25519SecretBCPGKey;
7376
import org.bouncycastle.bcpg.X448PublicBCPGKey;
7477
import org.bouncycastle.bcpg.X448SecretBCPGKey;
78+
import org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey;
7579
import org.bouncycastle.jcajce.util.DefaultJcaJceHelper;
7680
import org.bouncycastle.jcajce.util.NamedJcaJceHelper;
7781
import org.bouncycastle.jcajce.util.ProviderJcaJceHelper;
@@ -549,47 +553,58 @@ private BCPGKey getPublicBCPGKey(int algorithm, PGPAlgorithmParameters algorithm
549553
SubjectPublicKeyInfo keyInfo = SubjectPublicKeyInfo.getInstance(pubKey.getEncoded());
550554

551555
// TODO: should probably match curve by comparison as well
552-
ASN1ObjectIdentifier curveOid = ASN1ObjectIdentifier.getInstance(keyInfo.getAlgorithm().getParameters());
553-
if (curveOid == null)
556+
ASN1Encodable enc = keyInfo.getAlgorithm().getAlgorithm();
557+
ASN1ObjectIdentifier curveOid;
558+
curveOid = ASN1ObjectIdentifier.getInstance(enc);
559+
560+
// ecPublicKey is not specific enough. Drill down to find proper OID.
561+
if (X9ObjectIdentifiers.id_ecPublicKey.equals(curveOid))
554562
{
555-
// Legacy XDH on Curve25519 (legacy X25519)
556-
// 1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110
557-
if (pubKey.getAlgorithm().regionMatches(true, 0, "X2", 0, 2))
563+
enc = getCurveOIDForBCECKey((BCECPublicKey) pubKey);
564+
ASN1ObjectIdentifier nCurveOid = ASN1ObjectIdentifier.getInstance(enc);
565+
if (nCurveOid != null)
558566
{
559-
PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters);
567+
curveOid = nCurveOid;
568+
}
569+
}
560570

561-
return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)),
571+
// Legacy XDH on Curve25519 (legacy X25519)
572+
// 1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110
573+
if (pubKey.getAlgorithm().regionMatches(true, 0, "X2", 0, 2))
574+
{
575+
PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters);
576+
577+
return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)),
562578
kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm());
563-
}
564-
// Legacy X448 (1.3.101.111)
565-
if (pubKey.getAlgorithm().regionMatches(true, 0, "X4", 0, 2))
566-
{
579+
}
580+
// Legacy X448 (1.3.101.111)
581+
if (pubKey.getAlgorithm().regionMatches(true, 0, "X4", 0, 2))
582+
{
567583

568-
PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters);
584+
PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters);
569585

570-
return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)),
586+
return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)),
571587
kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm());
572-
}
573-
// sun.security.ec.XDHPublicKeyImpl returns "XDH" for getAlgorithm()
574-
// In this case we need to determine the curve by looking at the length of the encoding :/
575-
else
588+
}
589+
// sun.security.ec.XDHPublicKeyImpl returns "XDH" for getAlgorithm()
590+
// In this case we need to determine the curve by looking at the length of the encoding :/
591+
else if (pubKey.getAlgorithm().regionMatches(true, 0, "XDH", 0, 3))
592+
{
593+
// Legacy X25519 (1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110)
594+
if (X25519.SCALAR_SIZE + 12 == pubKey.getEncoded().length) // + 12 for some reason
576595
{
577-
// Legacy X25519 (1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110)
578-
if (X25519.SCALAR_SIZE + 12 == pubKey.getEncoded().length) // + 12 for some reason
579-
{
580-
PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters);
596+
PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters);
581597

582-
return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)),
598+
return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)),
583599
kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm());
584-
}
585-
// Legacy X448 (1.3.101.111)
586-
else
587-
{
588-
PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters);
600+
}
601+
// Legacy X448 (1.3.101.111)
602+
else
603+
{
604+
PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters);
589605

590-
return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)),
606+
return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)),
591607
kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm());
592-
}
593608
}
594609
}
595610

@@ -670,6 +685,22 @@ private BCPGKey getPublicBCPGKey(int algorithm, PGPAlgorithmParameters algorithm
670685
}
671686
}
672687

688+
private ASN1Encodable getCurveOIDForBCECKey(BCECPublicKey pubKey)
689+
{
690+
// Iterate through all registered curves to find applicable OID
691+
Enumeration names = ECNamedCurveTable.getNames();
692+
while (names.hasMoreElements())
693+
{
694+
String name = (String) names.nextElement();
695+
X9ECParameters parms = ECNamedCurveTable.getByName(name);
696+
if (pubKey.getParameters().getCurve().equals(parms.getCurve()))
697+
{
698+
return ECNamedCurveTable.getOID(name);
699+
}
700+
}
701+
return null;
702+
}
703+
673704
@FunctionalInterface
674705
private interface BCPGKeyOperation
675706
{

‎pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
import java.security.*;
1919
import java.util.Date;
2020

21-
public class ECDSAKeyPairTest extends AbstractPgpKeyPairTest
21+
public class ECDSAKeyPairTest
22+
extends AbstractPgpKeyPairTest
2223
{
2324

2425
private static final String PRIME256v1 = "" +
@@ -93,23 +94,27 @@ public void performTest()
9394
throws Exception
9495
{
9596
Security.addProvider(new BouncyCastleProvider());
97+
testConversionOfFreshJcaKeyPair();
9698
testConversionOfParsedJcaKeyPair();
9799
testConversionOfParsedBcKeyPair();
98100

99-
testConversionOfFreshJcaKeyPair();
100101
}
101102

102-
private void testConversionOfParsedJcaKeyPair() throws PGPException, IOException {
103-
// parseAndConvertJca(PRIME256v1);
104-
// parseAndConvertJca(SECP384r1);
105-
// parseAndConvertJca(SECP521r1);
103+
private void testConversionOfParsedJcaKeyPair()
104+
throws PGPException, IOException
105+
{
106106
parseAndConvertJca(BRAINPOOLP256r1);
107107
parseAndConvertJca(BRAINPOOLP384r1);
108108
parseAndConvertJca(BRAINPOOLP521r1);
109+
parseAndConvertJca(PRIME256v1);
110+
parseAndConvertJca(SECP384r1);
111+
parseAndConvertJca(SECP521r1);
109112
}
110113

111-
private void parseAndConvertJca(String curve) throws IOException, PGPException {
112-
JcaPGPKeyConverter c = new JcaPGPKeyConverter();
114+
private void parseAndConvertJca(String curve)
115+
throws IOException, PGPException
116+
{
117+
JcaPGPKeyConverter c = new JcaPGPKeyConverter().setProvider(new BouncyCastleProvider());
113118
PGPKeyPair parsed = parseJca(curve);
114119
byte[] pubEnc = parsed.getPublicKey().getEncoded();
115120
byte[] privEnc = parsed.getPrivateKey().getPrivateKeyDataPacket().getEncoded();
@@ -119,7 +124,7 @@ private void parseAndConvertJca(String curve) throws IOException, PGPException {
119124
new KeyPair(c.getPublicKey(parsed.getPublicKey()),
120125
c.getPrivateKey(parsed.getPrivateKey())),
121126
parsed.getPublicKey().getCreationTime());
122-
isEncodingEqual(pubEnc, j1.getPublicKey().getEncoded());
127+
isEncodingEqual("ECDSA Public key (" + curve + ") encoding mismatch", pubEnc, j1.getPublicKey().getEncoded());
123128
isEncodingEqual(privEnc, j1.getPrivateKey().getPrivateKeyDataPacket().getEncoded());
124129

125130
BcPGPKeyPair b1 = toBcKeyPair(j1);
@@ -135,16 +140,20 @@ private void parseAndConvertJca(String curve) throws IOException, PGPException {
135140
isEncodingEqual(privEnc, b2.getPrivateKey().getPrivateKeyDataPacket().getEncoded());
136141
}
137142

138-
private void testConversionOfParsedBcKeyPair() throws PGPException, IOException {
139-
parseAndConvertBc(PRIME256v1);
140-
parseAndConvertBc(SECP384r1);
141-
parseAndConvertBc(SECP521r1);
143+
private void testConversionOfParsedBcKeyPair()
144+
throws PGPException, IOException
145+
{
142146
parseAndConvertBc(BRAINPOOLP256r1);
143147
parseAndConvertBc(BRAINPOOLP384r1);
144148
parseAndConvertBc(BRAINPOOLP521r1);
149+
parseAndConvertBc(PRIME256v1);
150+
parseAndConvertBc(SECP384r1);
151+
parseAndConvertBc(SECP521r1);
145152
}
146153

147-
private void parseAndConvertBc(String curve) throws IOException, PGPException {
154+
private void parseAndConvertBc(String curve)
155+
throws IOException, PGPException
156+
{
148157
BcPGPKeyConverter c = new BcPGPKeyConverter();
149158
PGPKeyPair parsed = parseBc(curve);
150159
byte[] pubEnc = parsed.getPublicKey().getEncoded();
@@ -173,7 +182,9 @@ private void parseAndConvertBc(String curve) throws IOException, PGPException {
173182

174183
}
175184

176-
private PGPKeyPair parseJca(String armored) throws IOException, PGPException {
185+
private PGPKeyPair parseJca(String armored)
186+
throws IOException, PGPException
187+
{
177188
ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8));
178189
ArmoredInputStream aIn = new ArmoredInputStream(bIn);
179190
BCPGInputStream pIn = new BCPGInputStream(aIn);
@@ -182,7 +193,9 @@ private PGPKeyPair parseJca(String armored) throws IOException, PGPException {
182193
return new PGPKeyPair(sk.getPublicKey(), sk.extractPrivateKey(null));
183194
}
184195

185-
private PGPKeyPair parseBc(String armored) throws IOException, PGPException {
196+
private PGPKeyPair parseBc(String armored)
197+
throws IOException, PGPException
198+
{
186199
ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8));
187200
ArmoredInputStream aIn = new ArmoredInputStream(bIn);
188201
BCPGInputStream pIn = new BCPGInputStream(aIn);
@@ -194,15 +207,17 @@ private PGPKeyPair parseBc(String armored) throws IOException, PGPException {
194207
private void testConversionOfFreshJcaKeyPair()
195208
throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, PGPException, IOException
196209
{
197-
for (String curve : new String[] {"prime256v1", "secp384r1", "secp521r1", "brainpoolP256r1", "brainpoolP384r1", "brainpoolP512r1"})
210+
for (String curve : new String[] {
211+
"prime256v1",
212+
"secp384r1",
213+
"secp521r1",
214+
"brainpoolP256r1",
215+
"brainpoolP384r1",
216+
"brainpoolP512r1"
217+
})
198218
{
199-
try {
200-
testConversionOfFreshJcaKeyPair(curve);
201-
} catch (Exception e) {
202-
203-
}
219+
testConversionOfFreshJcaKeyPair(curve);
204220
}
205-
206221
}
207222

208223
private void testConversionOfFreshJcaKeyPair(String curve)

‎pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ public class RegressionTest
7474
new LegacyX25519KeyPairTest(),
7575
new LegacyX448KeyPairTest(),
7676

77-
new Curve25519PrivateKeyEncodingTest()
77+
new Curve25519PrivateKeyEncodingTest(),
78+
new ECDSAKeyPairTest()
7879
};
7980

8081
public static void main(String[] args)

0 commit comments

Comments
 (0)