From 8e6038e16dab7cb28f0053ffb5f335c962415268 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 13 May 2024 15:35:26 +0200 Subject: [PATCH 1/4] Add test for EC curve key conversion --- .../openpgp/test/AbstractPgpKeyPairTest.java | 3 +- .../openpgp/test/ECDSAKeyPairTest.java | 256 ++++++++++++++++++ 2 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/AbstractPgpKeyPairTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/AbstractPgpKeyPairTest.java index 507c85c153..b4060b2584 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/test/AbstractPgpKeyPairTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/AbstractPgpKeyPairTest.java @@ -2,6 +2,7 @@ import org.bouncycastle.bcpg.test.AbstractPacketTest; import org.bouncycastle.crypto.AsymmetricCipherKeyPair; +import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.operator.bc.BcPGPKeyConverter; import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair; @@ -35,7 +36,7 @@ public BcPGPKeyPair toBcKeyPair(JcaPGPKeyPair keyPair) public JcaPGPKeyPair toJcaKeyPair(BcPGPKeyPair keyPair) throws PGPException { - JcaPGPKeyConverter c = new JcaPGPKeyConverter(); + JcaPGPKeyConverter c = new JcaPGPKeyConverter().setProvider(new BouncyCastleProvider()); return new JcaPGPKeyPair(keyPair.getPublicKey().getAlgorithm(), new KeyPair( c.getPublicKey(keyPair.getPublicKey()), diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java new file mode 100644 index 0000000000..4a7a725064 --- /dev/null +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java @@ -0,0 +1,256 @@ +package org.bouncycastle.openpgp.test; + +import org.bouncycastle.bcpg.*; +import org.bouncycastle.crypto.AsymmetricCipherKeyPair; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.jce.spec.ECNamedCurveGenParameterSpec; +import org.bouncycastle.openpgp.*; +import org.bouncycastle.openpgp.bc.BcPGPSecretKeyRing; +import org.bouncycastle.openpgp.jcajce.JcaPGPSecretKeyRing; +import org.bouncycastle.openpgp.operator.bc.BcPGPKeyConverter; +import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair; +import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyConverter; +import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyPair; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.*; +import java.util.Date; + +public class ECDSAKeyPairTest extends AbstractPgpKeyPairTest +{ + + private static final String PRIME256v1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lHcEZkH7VRMIKoZIzj0DAQcCAwQee5wkHVVrG7u7CcrHoZOaC+reK0wn2Y5XPJoU\n" + + "O6geh1j2qXHj4+f+a6lav5hzKIJZHkgBYcS0aeABgWNjKsHbAAD/b4K93MJF7c2l\n" + + "4Y7ojBqTuZAOOD0Dyqe8MTXXyDUWN/0R/w==\n" + + "=mPB9\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + private static final String SECP384r1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lKQEZkH7VhMFK4EEACIDAwQgkKs+EzJaFLgMZH5Fp1S8DCXZC0OildnuQX6F7Jzt\n" + + "BgkYyfDZ/F2KNistCqfsmxWnwAxtdRuuY2PfehWktQBQaID0OfXUnOC2E5961b3/\n" + + "7xoZU26T0npmTqX0P/wuXawAAX9S2V72/xeShrcIwIwy2QvCcsW9ATBSQ6U+T7KZ\n" + + "zzFisUiqCgYa/9hoSNnu7iNrnrcYlQ==\n" + + "=SyFg\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + private static final String SECP521r1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lNkEZkH7VhMFK4EEACMEIwQBxt7DenSWrjuJGR0ouSwylW3ZC6mX4S+A5Cav7nz3\n" + + "DninA8Rdt3Cd5sHQ1IWea+J05NUZDKbOL417lUSPkAVLot0B/Qis90wODcGnAXbc\n" + + "m+m7rN2/Waryj/EsxLxub4UNtyZ405C8dDo9ch2JRfHiH6R1dwyqD9+yY2lOPYO+\n" + + "tn5fx/4AAgIDG9+DPtDf91tBMhBKc0f++t6aV115HLlyIpnEipThSwMTgzWm0uPZ\n" + + "KD3CifJeUU/TMk9IGFYvRlaWBQfrB3V/Ahz4\n" + + "=DD95\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + private static final String BRAINPOOLP256r1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lHgEZkH7VhMJKyQDAwIIAQEHAgMEj7YxVg4/2p4uuhcpRqGl2i+vDhjx8YhUUNJX\n" + + "RNFozBuIWJ6zkW3wRKdD/7Y7tzKNwyHmZ4FBFCcUoLliLeD4SAABAIkEm4iT1g0B\n" + + "Bo9vkUrUcP2b+vtOuwtmrvGrT0VzVXYlD5M=\n" + + "=vZRh\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + private static final String BRAINPOOLP384r1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lKgEZkH7VhMJKyQDAwIIAQELAwMEYm1fhilklF53Pj91awsoO0aZsppmPk9KNESD\n" + + "H7/gSK86gl+yhf4/oKSxeOFDHCU2es6Iijq/TCIaAjeFH3ITEyQ4tPdnDqQSz2xq\n" + + "o6wtRTW3cRD9oyoOT8bAMdm+RYpJAAF5AXAfxp3VtxqVVxnR1mC3Z3nL25zmvdu1\n" + + "oPRvA9fenVxTOlyU6X9qCycSuxamkPO7Gic=\n" + + "=2eJn\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + private static final String BRAINPOOLP521r1 = "" + + "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" + + "Version: BCPG v@RELEASE_NAME@\n" + + "\n" + + "lNgEZkH7VhMJKyQDAwIIAQENBAMEbSjn4lQKNnC50PzeUtenikvF62KR7HfOLJTA\n" + + "r/T17tFx3Qb6Ek/xQWIJ5nIHroOrduZjLigPOXqQ+GNhCgdNPGUqAWw1sfQ86nrx\n" + + "jqlr67na3F3eaTJr9ajr2V37/5uHnuryJnkyy2laFdOGD0Ad9/bQkvXYoWVm0P07\n" + + "uCPnexEAAgCSUoeS3c+DAZlWETdyuSDyvHK7GLO67+CgVsEyqBF/Kch/vhBZFWXA\n" + + "Cs9lph8la5B0faKH5XSbeReudKGh/MjfIJo=\n" + + "=MZeT\n" + + "-----END PGP PRIVATE KEY BLOCK-----"; + + @Override + public String getName() + { + return "ECDSAKeyPairTest"; + } + + @Override + public void performTest() + throws Exception + { + Security.addProvider(new BouncyCastleProvider()); + testConversionOfParsedJcaKeyPair(); + testConversionOfParsedBcKeyPair(); + + testConversionOfFreshJcaKeyPair(); + } + + private void testConversionOfParsedJcaKeyPair() throws PGPException, IOException { + // parseAndConvertJca(PRIME256v1); + // parseAndConvertJca(SECP384r1); + // parseAndConvertJca(SECP521r1); + parseAndConvertJca(BRAINPOOLP256r1); + parseAndConvertJca(BRAINPOOLP384r1); + parseAndConvertJca(BRAINPOOLP521r1); + } + + private void parseAndConvertJca(String curve) throws IOException, PGPException { + JcaPGPKeyConverter c = new JcaPGPKeyConverter(); + PGPKeyPair parsed = parseJca(curve); + byte[] pubEnc = parsed.getPublicKey().getEncoded(); + byte[] privEnc = parsed.getPrivateKey().getPrivateKeyDataPacket().getEncoded(); + + JcaPGPKeyPair j1 = new JcaPGPKeyPair( + parsed.getPublicKey().getAlgorithm(), + new KeyPair(c.getPublicKey(parsed.getPublicKey()), + c.getPrivateKey(parsed.getPrivateKey())), + parsed.getPublicKey().getCreationTime()); + isEncodingEqual(pubEnc, j1.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, j1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + BcPGPKeyPair b1 = toBcKeyPair(j1); + isEncodingEqual(pubEnc, b1.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + JcaPGPKeyPair j2 = toJcaKeyPair(b1); + isEncodingEqual(pubEnc, j2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, j2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + BcPGPKeyPair b2 = toBcKeyPair(j2); + isEncodingEqual(pubEnc, b2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + } + + private void testConversionOfParsedBcKeyPair() throws PGPException, IOException { + parseAndConvertBc(PRIME256v1); + parseAndConvertBc(SECP384r1); + parseAndConvertBc(SECP521r1); + parseAndConvertBc(BRAINPOOLP256r1); + parseAndConvertBc(BRAINPOOLP384r1); + parseAndConvertBc(BRAINPOOLP521r1); + } + + private void parseAndConvertBc(String curve) throws IOException, PGPException { + BcPGPKeyConverter c = new BcPGPKeyConverter(); + PGPKeyPair parsed = parseBc(curve); + byte[] pubEnc = parsed.getPublicKey().getEncoded(); + byte[] privEnc = parsed.getPrivateKey().getPrivateKeyDataPacket().getEncoded(); + + BcPGPKeyPair b1 = new BcPGPKeyPair( + parsed.getPublicKey().getAlgorithm(), + new AsymmetricCipherKeyPair( + c.getPublicKey(parsed.getPublicKey()), + c.getPrivateKey(parsed.getPrivateKey())), + parsed.getPublicKey().getCreationTime()); + isEncodingEqual(pubEnc, b1.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + JcaPGPKeyPair j1 = toJcaKeyPair(b1); + isEncodingEqual(pubEnc, j1.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, j1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + BcPGPKeyPair b2 = toBcKeyPair(j1); + isEncodingEqual(pubEnc, b2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + JcaPGPKeyPair j2 = toJcaKeyPair(b2); + isEncodingEqual(pubEnc, j2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, j2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + + } + + private PGPKeyPair parseJca(String armored) throws IOException, PGPException { + ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); + ArmoredInputStream aIn = new ArmoredInputStream(bIn); + BCPGInputStream pIn = new BCPGInputStream(aIn); + JcaPGPSecretKeyRing ring = new JcaPGPSecretKeyRing(pIn); + PGPSecretKey sk = ring.getSecretKey(); + return new PGPKeyPair(sk.getPublicKey(), sk.extractPrivateKey(null)); + } + + private PGPKeyPair parseBc(String armored) throws IOException, PGPException { + ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); + ArmoredInputStream aIn = new ArmoredInputStream(bIn); + BCPGInputStream pIn = new BCPGInputStream(aIn); + BcPGPSecretKeyRing ring = new BcPGPSecretKeyRing(pIn); + PGPSecretKey sk = ring.getSecretKey(); + return new PGPKeyPair(sk.getPublicKey(), sk.extractPrivateKey(null)); + } + + private void testConversionOfFreshJcaKeyPair() + throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, PGPException, IOException + { + for (String curve : new String[] {"prime256v1", "secp384r1", "secp521r1", "brainpoolP256r1", "brainpoolP384r1", "brainpoolP512r1"}) + { + try { + testConversionOfFreshJcaKeyPair(curve); + } catch (Exception e) { + + } + } + + } + + private void testConversionOfFreshJcaKeyPair(String curve) + throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException, PGPException + { + Date date = currentTimeRounded(); + KeyPairGenerator gen = KeyPairGenerator.getInstance("ECDSA", new BouncyCastleProvider()); + gen.initialize(new ECNamedCurveGenParameterSpec(curve)); + KeyPair kp = gen.generateKeyPair(); + + JcaPGPKeyPair j1 = new JcaPGPKeyPair(PublicKeyAlgorithmTags.ECDSA, kp, date); + byte[] pubEnc = j1.getPublicKey().getEncoded(); + byte[] privEnc = j1.getPrivateKey().getPrivateKeyDataPacket().getEncoded(); + isTrue("Legacy ECDSA public key MUST be instanceof ECDSAPublicBCPGKey", + j1.getPublicKey().getPublicKeyPacket().getKey() instanceof ECDSAPublicBCPGKey); + isTrue("Legacy ECDSA secret key MUST be instanceof ECSecretBCPGKey", + j1.getPrivateKey().getPrivateKeyDataPacket() instanceof ECSecretBCPGKey); + + BcPGPKeyPair b1 = toBcKeyPair(j1); + isEncodingEqual(pubEnc, b1.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + isTrue("Legacy ECDSA public key MUST be instanceof ECDSAPublicBCPGKey", + b1.getPublicKey().getPublicKeyPacket().getKey() instanceof ECDSAPublicBCPGKey); + isTrue(" Legacy ECDSA secret key MUST be instanceof ECSecretBCPGKey", + b1.getPrivateKey().getPrivateKeyDataPacket() instanceof ECSecretBCPGKey); + + JcaPGPKeyPair j2 = toJcaKeyPair(b1); + isEncodingEqual(pubEnc, j2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, j2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + isTrue("Legacy ECDSA public key MUST be instanceof ECDSAPublicBCPGKey", + j2.getPublicKey().getPublicKeyPacket().getKey() instanceof ECDSAPublicBCPGKey); + isTrue("Legacy ECDSA secret key MUST be instanceof ECSecretBCPGKey", + j2.getPrivateKey().getPrivateKeyDataPacket() instanceof ECSecretBCPGKey); + + BcPGPKeyPair b2 = toBcKeyPair(j2); + isEncodingEqual(pubEnc, b2.getPublicKey().getEncoded()); + isEncodingEqual(privEnc, b2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); + isTrue("Legacy ECDSA public key MUST be instanceof ECDSAPublicBCPGKey", + b2.getPublicKey().getPublicKeyPacket().getKey() instanceof ECDSAPublicBCPGKey); + isTrue("Legacy ECDSA secret key MUST be instanceof ECSecretBCPGKey", + b2.getPrivateKey().getPrivateKeyDataPacket() instanceof ECSecretBCPGKey); + + isEquals("Creation time is preserved", + date.getTime(), b2.getPublicKey().getCreationTime().getTime()); + } + + public static void main(String[] args) + { + runTest(new ECDSAKeyPairTest()); + } +} From a32e9b63efcd491fe4c291038407b5b2ee1d913d Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 20 May 2024 11:12:41 +0200 Subject: [PATCH 2/4] Fix conversion of ECDSA keys --- .../operator/jcajce/JcaPGPKeyConverter.java | 89 +++++++++++++------ .../openpgp/test/ECDSAKeyPairTest.java | 61 ++++++++----- .../openpgp/test/RegressionTest.java | 3 +- 3 files changed, 100 insertions(+), 53 deletions(-) diff --git a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java index 64779da731..e4ed38340d 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java @@ -29,6 +29,7 @@ import java.security.spec.RSAPublicKeySpec; import java.security.spec.X509EncodedKeySpec; import java.util.Date; +import java.util.Enumeration; import javax.crypto.interfaces.DHPrivateKey; import javax.crypto.interfaces.DHPublicKey; @@ -36,6 +37,7 @@ import javax.crypto.spec.DHPrivateKeySpec; import javax.crypto.spec.DHPublicKeySpec; +import org.bouncycastle.asn1.ASN1Encodable; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.ASN1OctetString; import org.bouncycastle.asn1.DEROctetString; @@ -49,6 +51,7 @@ import org.bouncycastle.asn1.x9.X9ECParameters; import org.bouncycastle.asn1.x9.X9ECParametersHolder; import org.bouncycastle.asn1.x9.X9ECPoint; +import org.bouncycastle.asn1.x9.X9ObjectIdentifiers; import org.bouncycastle.bcpg.BCPGKey; import org.bouncycastle.bcpg.DSAPublicBCPGKey; import org.bouncycastle.bcpg.DSASecretBCPGKey; @@ -72,6 +75,7 @@ import org.bouncycastle.bcpg.X25519SecretBCPGKey; import org.bouncycastle.bcpg.X448PublicBCPGKey; import org.bouncycastle.bcpg.X448SecretBCPGKey; +import org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPublicKey; import org.bouncycastle.jcajce.util.DefaultJcaJceHelper; import org.bouncycastle.jcajce.util.NamedJcaJceHelper; import org.bouncycastle.jcajce.util.ProviderJcaJceHelper; @@ -549,47 +553,58 @@ private BCPGKey getPublicBCPGKey(int algorithm, PGPAlgorithmParameters algorithm SubjectPublicKeyInfo keyInfo = SubjectPublicKeyInfo.getInstance(pubKey.getEncoded()); // TODO: should probably match curve by comparison as well - ASN1ObjectIdentifier curveOid = ASN1ObjectIdentifier.getInstance(keyInfo.getAlgorithm().getParameters()); - if (curveOid == null) + ASN1Encodable enc = keyInfo.getAlgorithm().getAlgorithm(); + ASN1ObjectIdentifier curveOid; + curveOid = ASN1ObjectIdentifier.getInstance(enc); + + // ecPublicKey is not specific enough. Drill down to find proper OID. + if (X9ObjectIdentifiers.id_ecPublicKey.equals(curveOid)) { - // Legacy XDH on Curve25519 (legacy X25519) - // 1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110 - if (pubKey.getAlgorithm().regionMatches(true, 0, "X2", 0, 2)) + enc = getCurveOIDForBCECKey((BCECPublicKey) pubKey); + ASN1ObjectIdentifier nCurveOid = ASN1ObjectIdentifier.getInstance(enc); + if (nCurveOid != null) { - PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters); + curveOid = nCurveOid; + } + } - return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)), + // Legacy XDH on Curve25519 (legacy X25519) + // 1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110 + if (pubKey.getAlgorithm().regionMatches(true, 0, "X2", 0, 2)) + { + PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters); + + return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)), kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm()); - } - // Legacy X448 (1.3.101.111) - if (pubKey.getAlgorithm().regionMatches(true, 0, "X4", 0, 2)) - { + } + // Legacy X448 (1.3.101.111) + if (pubKey.getAlgorithm().regionMatches(true, 0, "X4", 0, 2)) + { - PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters); + PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters); - return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)), + return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)), kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm()); - } - // sun.security.ec.XDHPublicKeyImpl returns "XDH" for getAlgorithm() - // In this case we need to determine the curve by looking at the length of the encoding :/ - else + } + // sun.security.ec.XDHPublicKeyImpl returns "XDH" for getAlgorithm() + // In this case we need to determine the curve by looking at the length of the encoding :/ + else if (pubKey.getAlgorithm().regionMatches(true, 0, "XDH", 0, 3)) + { + // Legacy X25519 (1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110) + if (X25519.SCALAR_SIZE + 12 == pubKey.getEncoded().length) // + 12 for some reason { - // Legacy X25519 (1.3.6.1.4.1.3029.1.5.1 & 1.3.101.110) - if (X25519.SCALAR_SIZE + 12 == pubKey.getEncoded().length) // + 12 for some reason - { - PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters); + PGPKdfParameters kdfParams = implGetKdfParameters(CryptlibObjectIdentifiers.curvey25519, algorithmParameters); - return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)), + return new ECDHPublicBCPGKey(CryptlibObjectIdentifiers.curvey25519, new BigInteger(1, getPointEncUncompressed(pubKey, X25519.SCALAR_SIZE)), kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm()); - } - // Legacy X448 (1.3.101.111) - else - { - PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters); + } + // Legacy X448 (1.3.101.111) + else + { + PGPKdfParameters kdfParams = implGetKdfParameters(EdECObjectIdentifiers.id_X448, algorithmParameters); - return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)), + return new ECDHPublicBCPGKey(EdECObjectIdentifiers.id_X448, new BigInteger(1, getPointEncUncompressed(pubKey, X448.SCALAR_SIZE)), kdfParams.getHashAlgorithm(), kdfParams.getSymmetricWrapAlgorithm()); - } } } @@ -670,6 +685,22 @@ private BCPGKey getPublicBCPGKey(int algorithm, PGPAlgorithmParameters algorithm } } + private ASN1Encodable getCurveOIDForBCECKey(BCECPublicKey pubKey) + { + // Iterate through all registered curves to find applicable OID + Enumeration names = ECNamedCurveTable.getNames(); + while (names.hasMoreElements()) + { + String name = (String) names.nextElement(); + X9ECParameters parms = ECNamedCurveTable.getByName(name); + if (pubKey.getParameters().getCurve().equals(parms.getCurve())) + { + return ECNamedCurveTable.getOID(name); + } + } + return null; + } + @FunctionalInterface private interface BCPGKeyOperation { diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java index 4a7a725064..bdefb35220 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java @@ -18,7 +18,8 @@ import java.security.*; import java.util.Date; -public class ECDSAKeyPairTest extends AbstractPgpKeyPairTest +public class ECDSAKeyPairTest + extends AbstractPgpKeyPairTest { private static final String PRIME256v1 = "" + @@ -93,23 +94,27 @@ public void performTest() throws Exception { Security.addProvider(new BouncyCastleProvider()); + testConversionOfFreshJcaKeyPair(); testConversionOfParsedJcaKeyPair(); testConversionOfParsedBcKeyPair(); - testConversionOfFreshJcaKeyPair(); } - private void testConversionOfParsedJcaKeyPair() throws PGPException, IOException { - // parseAndConvertJca(PRIME256v1); - // parseAndConvertJca(SECP384r1); - // parseAndConvertJca(SECP521r1); + private void testConversionOfParsedJcaKeyPair() + throws PGPException, IOException + { parseAndConvertJca(BRAINPOOLP256r1); parseAndConvertJca(BRAINPOOLP384r1); parseAndConvertJca(BRAINPOOLP521r1); + parseAndConvertJca(PRIME256v1); + parseAndConvertJca(SECP384r1); + parseAndConvertJca(SECP521r1); } - private void parseAndConvertJca(String curve) throws IOException, PGPException { - JcaPGPKeyConverter c = new JcaPGPKeyConverter(); + private void parseAndConvertJca(String curve) + throws IOException, PGPException + { + JcaPGPKeyConverter c = new JcaPGPKeyConverter().setProvider(new BouncyCastleProvider()); PGPKeyPair parsed = parseJca(curve); byte[] pubEnc = parsed.getPublicKey().getEncoded(); byte[] privEnc = parsed.getPrivateKey().getPrivateKeyDataPacket().getEncoded(); @@ -119,7 +124,7 @@ private void parseAndConvertJca(String curve) throws IOException, PGPException { new KeyPair(c.getPublicKey(parsed.getPublicKey()), c.getPrivateKey(parsed.getPrivateKey())), parsed.getPublicKey().getCreationTime()); - isEncodingEqual(pubEnc, j1.getPublicKey().getEncoded()); + isEncodingEqual("ECDSA Public key (" + curve + ") encoding mismatch", pubEnc, j1.getPublicKey().getEncoded()); isEncodingEqual(privEnc, j1.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); BcPGPKeyPair b1 = toBcKeyPair(j1); @@ -135,16 +140,20 @@ private void parseAndConvertJca(String curve) throws IOException, PGPException { isEncodingEqual(privEnc, b2.getPrivateKey().getPrivateKeyDataPacket().getEncoded()); } - private void testConversionOfParsedBcKeyPair() throws PGPException, IOException { - parseAndConvertBc(PRIME256v1); - parseAndConvertBc(SECP384r1); - parseAndConvertBc(SECP521r1); + private void testConversionOfParsedBcKeyPair() + throws PGPException, IOException + { parseAndConvertBc(BRAINPOOLP256r1); parseAndConvertBc(BRAINPOOLP384r1); parseAndConvertBc(BRAINPOOLP521r1); + parseAndConvertBc(PRIME256v1); + parseAndConvertBc(SECP384r1); + parseAndConvertBc(SECP521r1); } - private void parseAndConvertBc(String curve) throws IOException, PGPException { + private void parseAndConvertBc(String curve) + throws IOException, PGPException + { BcPGPKeyConverter c = new BcPGPKeyConverter(); PGPKeyPair parsed = parseBc(curve); byte[] pubEnc = parsed.getPublicKey().getEncoded(); @@ -173,7 +182,9 @@ private void parseAndConvertBc(String curve) throws IOException, PGPException { } - private PGPKeyPair parseJca(String armored) throws IOException, PGPException { + private PGPKeyPair parseJca(String armored) + throws IOException, PGPException + { ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); ArmoredInputStream aIn = new ArmoredInputStream(bIn); BCPGInputStream pIn = new BCPGInputStream(aIn); @@ -182,7 +193,9 @@ private PGPKeyPair parseJca(String armored) throws IOException, PGPException { return new PGPKeyPair(sk.getPublicKey(), sk.extractPrivateKey(null)); } - private PGPKeyPair parseBc(String armored) throws IOException, PGPException { + private PGPKeyPair parseBc(String armored) + throws IOException, PGPException + { ByteArrayInputStream bIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); ArmoredInputStream aIn = new ArmoredInputStream(bIn); BCPGInputStream pIn = new BCPGInputStream(aIn); @@ -194,15 +207,17 @@ private PGPKeyPair parseBc(String armored) throws IOException, PGPException { private void testConversionOfFreshJcaKeyPair() throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, PGPException, IOException { - for (String curve : new String[] {"prime256v1", "secp384r1", "secp521r1", "brainpoolP256r1", "brainpoolP384r1", "brainpoolP512r1"}) + for (String curve : new String[] { + "prime256v1", + "secp384r1", + "secp521r1", + "brainpoolP256r1", + "brainpoolP384r1", + "brainpoolP512r1" + }) { - try { - testConversionOfFreshJcaKeyPair(curve); - } catch (Exception e) { - - } + testConversionOfFreshJcaKeyPair(curve); } - } private void testConversionOfFreshJcaKeyPair(String curve) diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java index e9a4c11e3c..e110c16ba2 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java @@ -74,7 +74,8 @@ public class RegressionTest new LegacyX25519KeyPairTest(), new LegacyX448KeyPairTest(), - new Curve25519PrivateKeyEncodingTest() + new Curve25519PrivateKeyEncodingTest(), + new ECDSAKeyPairTest() }; public static void main(String[] args) From 8004fb31210c45a95634daf20a8021e0fea09ec7 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 23 May 2024 09:59:53 +0200 Subject: [PATCH 3/4] Rename named curve lookup method --- .../openpgp/operator/jcajce/JcaPGPKeyConverter.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java index e4ed38340d..a12d9cbbb1 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaPGPKeyConverter.java @@ -557,10 +557,10 @@ private BCPGKey getPublicBCPGKey(int algorithm, PGPAlgorithmParameters algorithm ASN1ObjectIdentifier curveOid; curveOid = ASN1ObjectIdentifier.getInstance(enc); - // ecPublicKey is not specific enough. Drill down to find proper OID. + // BCECPublicKey uses explicit parameter encoding, so we need to find the named curve manually if (X9ObjectIdentifiers.id_ecPublicKey.equals(curveOid)) { - enc = getCurveOIDForBCECKey((BCECPublicKey) pubKey); + enc = getNamedCurveOID((BCECPublicKey) pubKey); ASN1ObjectIdentifier nCurveOid = ASN1ObjectIdentifier.getInstance(enc); if (nCurveOid != null) { @@ -685,7 +685,7 @@ else if (pubKey.getAlgorithm().regionMatches(true, 0, "XDH", 0, 3)) } } - private ASN1Encodable getCurveOIDForBCECKey(BCECPublicKey pubKey) + private ASN1Encodable getNamedCurveOID(BCECPublicKey pubKey) { // Iterate through all registered curves to find applicable OID Enumeration names = ECNamedCurveTable.getNames(); From 897caef2bcfdbd369fcd0692b923dc95df328260 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 23 May 2024 10:39:49 +0200 Subject: [PATCH 4/4] Do not register BouncyCastleProvider in ECDSAKeyPairTest --- .../java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java index bdefb35220..111f8fd853 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/ECDSAKeyPairTest.java @@ -93,7 +93,6 @@ public String getName() public void performTest() throws Exception { - Security.addProvider(new BouncyCastleProvider()); testConversionOfFreshJcaKeyPair(); testConversionOfParsedJcaKeyPair(); testConversionOfParsedBcKeyPair();