Skip to content

Commit 22f6fec

Browse files
dghgitdghgit
committed
Merge branch 'pgpainless-fixKeyConversion'
2 parents 0588317 + 35f3089 commit 22f6fec

14 files changed

+1716
-165
lines changed

‎pg/src/main/java/org/bouncycastle/openpgp/operator/PGPKeyConverter.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ protected PGPKeyConverter()
7474
* <td>SHA2-256</td>
7575
* <td>AES-128</td>
7676
* </tr>
77+
* <tr>
78+
* <td>Curve448</td>
79+
* <td>SHA2-512</td>
80+
* <td>AES-256</td>
81+
* </tr>
7782
* </table>
7883
*/
7984
protected PGPKdfParameters implGetKdfParameters(ASN1ObjectIdentifier curveID, PGPAlgorithmParameters algorithmParameters)
@@ -89,7 +94,8 @@ else if (curveID.equals(SECObjectIdentifiers.secp384r1) || curveID.equals(TeleTr
8994
{
9095
return new PGPKdfParameters(HashAlgorithmTags.SHA384, SymmetricKeyAlgorithmTags.AES_192);
9196
}
92-
else if (curveID.equals(SECObjectIdentifiers.secp521r1) || curveID.equals(TeleTrusTObjectIdentifiers.brainpoolP512r1))
97+
else if (curveID.equals(SECObjectIdentifiers.secp521r1) || curveID.equals(TeleTrusTObjectIdentifiers.brainpoolP512r1)
98+
|| curveID.equals(EdECObjectIdentifiers.id_X448))
9399
{
94100
return new PGPKdfParameters(HashAlgorithmTags.SHA512, SymmetricKeyAlgorithmTags.AES_256);
95101
}

‎pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcPGPKeyConverter.java

+185-87
Large diffs are not rendered by default.

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

+221-76
Large diffs are not rendered by default.

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

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package org.bouncycastle.openpgp.test;
2+
3+
import org.bouncycastle.bcpg.test.AbstractPacketTest;
4+
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
5+
import org.bouncycastle.openpgp.PGPException;
6+
import org.bouncycastle.openpgp.operator.bc.BcPGPKeyConverter;
7+
import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair;
8+
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyConverter;
9+
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyPair;
10+
11+
import java.security.KeyPair;
12+
import java.util.Date;
13+
14+
public abstract class AbstractPgpKeyPairTest
15+
extends AbstractPacketTest
16+
{
17+
18+
public Date currentTimeRounded()
19+
{
20+
Date now = new Date();
21+
return new Date((now.getTime() / 1000) * 1000); // rounded to seconds
22+
}
23+
24+
public BcPGPKeyPair toBcKeyPair(JcaPGPKeyPair keyPair)
25+
throws PGPException
26+
{
27+
BcPGPKeyConverter c = new BcPGPKeyConverter();
28+
return new BcPGPKeyPair(keyPair.getPublicKey().getAlgorithm(),
29+
new AsymmetricCipherKeyPair(
30+
c.getPublicKey(keyPair.getPublicKey()),
31+
c.getPrivateKey(keyPair.getPrivateKey())),
32+
keyPair.getPublicKey().getCreationTime());
33+
}
34+
35+
public JcaPGPKeyPair toJcaKeyPair(BcPGPKeyPair keyPair)
36+
throws PGPException
37+
{
38+
JcaPGPKeyConverter c = new JcaPGPKeyConverter();
39+
return new JcaPGPKeyPair(keyPair.getPublicKey().getAlgorithm(),
40+
new KeyPair(
41+
c.getPublicKey(keyPair.getPublicKey()),
42+
c.getPrivateKey(keyPair.getPrivateKey())),
43+
keyPair.getPublicKey().getCreationTime());
44+
}
45+
}

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

+195
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
package org.bouncycastle.openpgp.test;
2+
3+
import org.bouncycastle.asn1.ASN1OctetString;
4+
import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
5+
import org.bouncycastle.bcpg.*;
6+
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
7+
import org.bouncycastle.crypto.generators.X25519KeyPairGenerator;
8+
import org.bouncycastle.crypto.params.X25519KeyGenerationParameters;
9+
import org.bouncycastle.crypto.params.X25519PrivateKeyParameters;
10+
import org.bouncycastle.jcajce.spec.XDHParameterSpec;
11+
import org.bouncycastle.jce.provider.BouncyCastleProvider;
12+
import org.bouncycastle.openpgp.*;
13+
import org.bouncycastle.openpgp.operator.bc.BcPGPKeyConverter;
14+
import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair;
15+
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyConverter;
16+
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyPair;
17+
import org.bouncycastle.util.Arrays;
18+
19+
import java.io.IOException;
20+
import java.security.*;
21+
import java.util.Date;
22+
23+
/**
24+
* Curve25519Legacy ECDH Secret Key Material uses big-endian MPI form,
25+
* while X25519 keys use little-endian native encoding.
26+
* This test verifies that legacy X25519 keys using ECDH are reverse-encoded,
27+
* while X25519 keys are natively encoded.
28+
*/
29+
public class Curve25519PrivateKeyEncodingTest
30+
extends AbstractPgpKeyPairTest
31+
{
32+
@Override
33+
public String getName()
34+
{
35+
return "Curve25519PrivateKeyEncodingTest";
36+
}
37+
38+
@Override
39+
public void performTest()
40+
throws Exception
41+
{
42+
containsTest();
43+
verifySecretKeyReverseEncoding();
44+
}
45+
46+
private void verifySecretKeyReverseEncoding()
47+
throws PGPException, IOException, InvalidAlgorithmParameterException, NoSuchAlgorithmException
48+
{
49+
bc_verifySecretKeyReverseEncoding();
50+
jca_verifySecretKeyReverseEncoding();
51+
}
52+
53+
/**
54+
* Verify that legacy ECDH keys over curve25519 encode the private key in reversed encoding,
55+
* while dedicated X25519 keys use native encoding for the private key material.
56+
* Test the JcaJce implementation.
57+
*
58+
* @throws NoSuchAlgorithmException
59+
* @throws InvalidAlgorithmParameterException
60+
* @throws PGPException
61+
* @throws IOException
62+
*/
63+
private void jca_verifySecretKeyReverseEncoding()
64+
throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, PGPException, IOException
65+
{
66+
JcaPGPKeyConverter c = new JcaPGPKeyConverter();
67+
68+
Date date = currentTimeRounded();
69+
KeyPairGenerator gen = KeyPairGenerator.getInstance("XDH", new BouncyCastleProvider());
70+
gen.initialize(new XDHParameterSpec("X25519"));
71+
KeyPair kp = gen.generateKeyPair();
72+
73+
byte[] rawPrivateKey = jcaNativePrivateKey(kp.getPrivate());
74+
75+
// Legacy key uses reversed encoding
76+
PGPKeyPair pgpECDHKeyPair = new JcaPGPKeyPair(PublicKeyAlgorithmTags.ECDH, kp, date);
77+
byte[] encodedECDHPrivateKey = pgpECDHKeyPair.getPrivateKey().getPrivateKeyDataPacket().getEncoded();
78+
isTrue("ECDH Curve25519Legacy (X25519) key MUST encode secret key in 'reverse' (big-endian MPI encoding) (JCE implementation)",
79+
containsSubsequence(encodedECDHPrivateKey, Arrays.reverse(rawPrivateKey)));
80+
81+
byte[] decodedECDHPrivateKey = jcaNativePrivateKey(c.getPrivateKey(pgpECDHKeyPair.getPrivateKey()));
82+
isEncodingEqual("Decoded ECDH Curve25519Legacy (X25519) key MUST match original raw key (JCE implementation)",
83+
decodedECDHPrivateKey, rawPrivateKey);
84+
85+
// X25519 key uses native encoding
86+
PGPKeyPair pgpX25519KeyPair = new JcaPGPKeyPair(PublicKeyAlgorithmTags.X25519, kp, date);
87+
byte[] encodedX25519PrivateKey = pgpX25519KeyPair.getPrivateKey().getPrivateKeyDataPacket().getEncoded();
88+
isTrue("X25519 key MUST use native encoding (little-endian) to encode the secret key material (JCE implementation)",
89+
containsSubsequence(encodedX25519PrivateKey, rawPrivateKey));
90+
91+
byte[] decodedX25519PrivateKey = jcaNativePrivateKey(c.getPrivateKey(pgpX25519KeyPair.getPrivateKey()));
92+
isEncodingEqual("Decoded X25519 key MUST match original raw key (JCE implementation)",
93+
rawPrivateKey, decodedX25519PrivateKey);
94+
}
95+
96+
/**
97+
* Return the native encoding of the given private key.
98+
* @param privateKey private key
99+
* @return native encoding
100+
* @throws IOException
101+
*/
102+
private byte[] jcaNativePrivateKey(PrivateKey privateKey)
103+
throws IOException
104+
{
105+
PrivateKeyInfo kInfo = PrivateKeyInfo.getInstance(privateKey.getEncoded());
106+
return ASN1OctetString.getInstance(kInfo.parsePrivateKey()).getOctets();
107+
}
108+
109+
/**
110+
* Verify that legacy ECDH keys over curve25519 encode the private key in reversed encoding,
111+
* while dedicated X25519 keys use native encoding for the private key material.
112+
* Test the BC implementation.
113+
*/
114+
private void bc_verifySecretKeyReverseEncoding()
115+
throws PGPException
116+
{
117+
BcPGPKeyConverter c = new BcPGPKeyConverter();
118+
119+
Date date = currentTimeRounded();
120+
X25519KeyPairGenerator gen = new X25519KeyPairGenerator();
121+
gen.init(new X25519KeyGenerationParameters(new SecureRandom()));
122+
AsymmetricCipherKeyPair kp = gen.generateKeyPair();
123+
124+
byte[] rawPrivateKey = ((X25519PrivateKeyParameters) kp.getPrivate()).getEncoded();
125+
126+
// Legacy key uses reversed encoding
127+
PGPKeyPair pgpECDHKeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.ECDH, kp, date);
128+
byte[] encodedECDHPrivateKey = pgpECDHKeyPair.getPrivateKey().getPrivateKeyDataPacket().getEncoded();
129+
isTrue("ECDH Curve25519Legacy (X25519) key MUST encode secret key in 'reverse' (big-endian MPI encoding) (BC implementation)",
130+
containsSubsequence(encodedECDHPrivateKey, Arrays.reverse(rawPrivateKey)));
131+
132+
byte[] decodedECDHPrivateKey = ((X25519PrivateKeyParameters) c.getPrivateKey(pgpECDHKeyPair.getPrivateKey())).getEncoded();
133+
isEncodingEqual("Decoded ECDH Curve25519Legacy (X25519) key MUST match original raw key (BC implementation)",
134+
decodedECDHPrivateKey, rawPrivateKey);
135+
136+
// X25519 key uses native encoding
137+
PGPKeyPair pgpX25519KeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.X25519, kp, date);
138+
byte[] encodedX25519PrivateKey = pgpX25519KeyPair.getPrivateKey().getPrivateKeyDataPacket().getEncoded();
139+
isTrue("X25519 key MUST use native encoding (little-endian) to encode the secret key material (BC implementation)",
140+
containsSubsequence(encodedX25519PrivateKey, rawPrivateKey));
141+
142+
byte[] decodedX25519PrivateKey = ((X25519PrivateKeyParameters) c.getPrivateKey(pgpX25519KeyPair.getPrivateKey())).getEncoded();
143+
isEncodingEqual("Decoded X25519 key MUST match original raw key (BC implementation)",
144+
rawPrivateKey, decodedX25519PrivateKey);
145+
}
146+
147+
/**
148+
* Return true, if the given sequence contains the given subsequence entirely.
149+
* @param sequence sequence
150+
* @param subsequence subsequence
151+
* @return true if subsequence is a subsequence of sequence
152+
*/
153+
public boolean containsSubsequence(byte[] sequence, byte[] subsequence)
154+
{
155+
outer: for (int i = 0; i < sequence.length - subsequence.length + 1; i++)
156+
{
157+
for (int j = 0; j < subsequence.length; j++)
158+
{
159+
if (sequence[i + j] != subsequence[j])
160+
{
161+
continue outer;
162+
}
163+
}
164+
return true;
165+
}
166+
return false;
167+
}
168+
169+
/**
170+
* Test proper functionality of the {@link #containsSubsequence(byte[], byte[])} method.
171+
*/
172+
private void containsTest() {
173+
// Make sure our containsSubsequence method functions correctly
174+
byte[] s = new byte[] {0x00, 0x01, 0x02, 0x03};
175+
isTrue(containsSubsequence(s, new byte[] {0x00, 0x01}));
176+
isTrue(containsSubsequence(s, new byte[] {0x01, 0x02}));
177+
isTrue(containsSubsequence(s, new byte[] {0x02, 0x03}));
178+
isTrue(containsSubsequence(s, new byte[] {0x00}));
179+
isTrue(containsSubsequence(s, new byte[] {0x03}));
180+
isTrue(containsSubsequence(s, new byte[] {0x00, 0x01, 0x02, 0x03}));
181+
isTrue(containsSubsequence(s, new byte[0]));
182+
isTrue(containsSubsequence(new byte[0], new byte[0]));
183+
184+
isFalse(containsSubsequence(s, new byte[] {0x00, 0x02}));
185+
isFalse(containsSubsequence(s, new byte[] {0x00, 0x00}));
186+
isFalse(containsSubsequence(s, new byte[] {0x00, 0x01, 0x02, 0x03, 0x04}));
187+
isFalse(containsSubsequence(s, new byte[] {0x04}));
188+
isFalse(containsSubsequence(new byte[0], new byte[] {0x00}));
189+
}
190+
191+
public static void main(String[] args)
192+
{
193+
runTest(new Curve25519PrivateKeyEncodingTest());
194+
}
195+
}

0 commit comments

Comments
 (0)