Skip to content

Commit 213297a

Browse files
jill-kleiberdghgit
authored andcommitted
pr-1716-improve-PGPPadding
1 parent 5d3edd0 commit 213297a

File tree

6 files changed

+250
-7
lines changed

6 files changed

+250
-7
lines changed

pg/src/main/java/org/bouncycastle/bcpg/PaddingPacket.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ public PaddingPacket(int octetLen, SecureRandom random)
3939

4040
private static byte[] randomBytes(int octetCount, SecureRandom random)
4141
{
42+
if (octetCount <= 0)
43+
{
44+
throw new IllegalArgumentException("Octet count MUST NOT be 0 nor negative.");
45+
}
4246
byte[] bytes = new byte[octetCount];
4347
random.nextBytes(bytes);
4448
return bytes;

pg/src/main/java/org/bouncycastle/openpgp/PGPKeyRing.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.bouncycastle.bcpg.BCPGInputStream;
1212
import org.bouncycastle.bcpg.Packet;
13+
import org.bouncycastle.bcpg.PacketFormat;
1314
import org.bouncycastle.bcpg.PacketTags;
1415
import org.bouncycastle.bcpg.SignaturePacket;
1516
import org.bouncycastle.bcpg.TrustPacket;
@@ -144,6 +145,9 @@ public abstract void encode(OutputStream outStream)
144145
public abstract byte[] getEncoded()
145146
throws IOException;
146147

148+
public abstract byte[] getEncoded(PacketFormat format)
149+
throws IOException;
150+
147151
private static boolean isUserTag(int tag)
148152
{
149153
switch (tag)

pg/src/main/java/org/bouncycastle/openpgp/PGPPadding.java

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
package org.bouncycastle.openpgp;
22

3+
import java.io.ByteArrayOutputStream;
34
import java.io.IOException;
5+
import java.io.OutputStream;
6+
import java.security.SecureRandom;
47

58
import org.bouncycastle.bcpg.BCPGInputStream;
9+
import org.bouncycastle.bcpg.BCPGOutputStream;
610
import org.bouncycastle.bcpg.Packet;
11+
import org.bouncycastle.bcpg.PacketFormat;
712
import org.bouncycastle.bcpg.PaddingPacket;
13+
import org.bouncycastle.crypto.CryptoServicesRegistrar;
814

915
/**
1016
* The PGPPadding contains random data, and can be used to defend against traffic analysis on version 2 SEIPD messages
@@ -16,10 +22,25 @@ public class PGPPadding
1622
{
1723
private PaddingPacket p;
1824

25+
/**
26+
* Minimum random padding length in octets.
27+
* Chosen totally arbitrarily.
28+
*/
29+
public static final int MIN_PADDING_LEN = 16;
30+
31+
/**
32+
* Maximum random padding length.
33+
* Chosen somewhat arbitrarily, as SSH also uses max 255 bytes for random padding.
34+
*
35+
* @see <a href="https://www.rfc-editor.org/rfc/rfc4253.html#section-6">
36+
* rfc4253 - Binary Packet Protocol</a>
37+
*/
38+
public static final int MAX_PADDING_LEN = 255;
39+
1940
/**
2041
* Default constructor.
2142
*
22-
* @param in
43+
* @param in packet input stream
2344
* @throws IOException
2445
*/
2546
public PGPPadding(
@@ -34,8 +55,78 @@ public PGPPadding(
3455
p = (PaddingPacket)packet;
3556
}
3657

58+
/**
59+
* Generate a new, random {@link PGPPadding} object.
60+
* The padding consists of n random bytes, where n is a number between (inclusive) {@link #MIN_PADDING_LEN}
61+
* and {@link #MAX_PADDING_LEN}.
62+
*/
63+
public PGPPadding()
64+
{
65+
this(CryptoServicesRegistrar.getSecureRandom());
66+
}
67+
68+
/**
69+
* Generate a new, random {@link PGPPadding} object.
70+
* The padding consists of n random bytes, where n is a number between (inclusive) {@link #MIN_PADDING_LEN}
71+
* and {@link #MAX_PADDING_LEN}.
72+
*
73+
* @param random random number generator instance
74+
*/
75+
public PGPPadding(SecureRandom random)
76+
{
77+
this(MIN_PADDING_LEN + random.nextInt(MAX_PADDING_LEN - MIN_PADDING_LEN + 1), random);
78+
}
79+
80+
/**
81+
* Generate a new, random {@link PGPPadding} object.
82+
* The padding consists of <pre>len</pre> random bytes.
83+
*/
84+
public PGPPadding(int len)
85+
{
86+
this(len, CryptoServicesRegistrar.getSecureRandom());
87+
}
88+
89+
/**
90+
* Generate a new, random {@link PGPPadding} object.
91+
* The padding consists of <pre>len</pre> random bytes.
92+
*
93+
* @param len number of random octets
94+
* @param random random number generator instance
95+
*/
96+
public PGPPadding(int len, SecureRandom random)
97+
{
98+
this.p = new PaddingPacket(len, random);
99+
}
100+
101+
/**
102+
* Return the padding octets as a byte array.
103+
* @return padding octets
104+
*/
37105
public byte[] getPadding()
38106
{
39107
return p.getPadding();
40108
}
109+
110+
public void encode(OutputStream outStream)
111+
throws IOException
112+
{
113+
BCPGOutputStream pOut = BCPGOutputStream.wrap(outStream);
114+
p.encode(pOut);
115+
}
116+
117+
public byte[] getEncoded()
118+
throws IOException
119+
{
120+
return getEncoded(PacketFormat.ROUNDTRIP);
121+
}
122+
123+
public byte[] getEncoded(PacketFormat format)
124+
throws IOException
125+
{
126+
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
127+
BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
128+
encode(pOut);
129+
pOut.close();
130+
return bOut.toByteArray();
131+
}
41132
}

pg/src/main/java/org/bouncycastle/openpgp/PGPPublicKeyRing.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717

1818
import org.bouncycastle.bcpg.ArmoredInputException;
1919
import org.bouncycastle.bcpg.BCPGInputStream;
20+
import org.bouncycastle.bcpg.BCPGOutputStream;
2021
import org.bouncycastle.bcpg.Packet;
22+
import org.bouncycastle.bcpg.PacketFormat;
2123
import org.bouncycastle.bcpg.PacketTags;
2224
import org.bouncycastle.bcpg.PublicKeyPacket;
2325
import org.bouncycastle.bcpg.TrustPacket;
@@ -238,10 +240,16 @@ public Iterator<PGPPublicKey> iterator()
238240
public byte[] getEncoded()
239241
throws IOException
240242
{
241-
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
242-
243-
this.encode(bOut);
243+
return getEncoded(PacketFormat.ROUNDTRIP);
244+
}
244245

246+
@Override
247+
public byte[] getEncoded(PacketFormat format) throws IOException
248+
{
249+
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
250+
BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
251+
this.encode(pOut);
252+
pOut.close();
245253
return bOut.toByteArray();
246254
}
247255

pg/src/main/java/org/bouncycastle/openpgp/PGPSecretKeyRing.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import org.bouncycastle.bcpg.ArmoredInputException;
1717
import org.bouncycastle.bcpg.BCPGInputStream;
18+
import org.bouncycastle.bcpg.BCPGOutputStream;
19+
import org.bouncycastle.bcpg.PacketFormat;
1820
import org.bouncycastle.bcpg.PacketTags;
1921
import org.bouncycastle.bcpg.PublicSubkeyPacket;
2022
import org.bouncycastle.bcpg.SecretKeyPacket;
@@ -389,10 +391,16 @@ public int size()
389391
public byte[] getEncoded()
390392
throws IOException
391393
{
392-
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
393-
394-
this.encode(bOut);
394+
return getEncoded(PacketFormat.ROUNDTRIP);
395+
}
395396

397+
@Override
398+
public byte[] getEncoded(PacketFormat format) throws IOException
399+
{
400+
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
401+
BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
402+
this.encode(pOut);
403+
pOut.close();
396404
return bOut.toByteArray();
397405
}
398406

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package org.bouncycastle.openpgp.test;
2+
3+
import org.bouncycastle.bcpg.ArmoredInputStream;
4+
import org.bouncycastle.bcpg.ArmoredOutputStream;
5+
import org.bouncycastle.bcpg.BCPGInputStream;
6+
import org.bouncycastle.bcpg.BCPGOutputStream;
7+
import org.bouncycastle.bcpg.HashAlgorithmTags;
8+
import org.bouncycastle.bcpg.PacketFormat;
9+
import org.bouncycastle.bcpg.PublicKeyAlgorithmTags;
10+
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
11+
import org.bouncycastle.crypto.CryptoServicesRegistrar;
12+
import org.bouncycastle.crypto.generators.Ed25519KeyPairGenerator;
13+
import org.bouncycastle.crypto.generators.X25519KeyPairGenerator;
14+
import org.bouncycastle.crypto.params.Ed25519KeyGenerationParameters;
15+
import org.bouncycastle.crypto.params.X25519KeyGenerationParameters;
16+
import org.bouncycastle.openpgp.PGPException;
17+
import org.bouncycastle.openpgp.PGPKeyPair;
18+
import org.bouncycastle.openpgp.PGPPadding;
19+
import org.bouncycastle.openpgp.PGPPublicKeyRing;
20+
import org.bouncycastle.openpgp.PGPSecretKey;
21+
import org.bouncycastle.openpgp.operator.PGPDigestCalculator;
22+
import org.bouncycastle.openpgp.operator.bc.BcKeyFingerprintCalculator;
23+
import org.bouncycastle.openpgp.operator.bc.BcPGPDigestCalculatorProvider;
24+
import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair;
25+
import org.bouncycastle.util.test.SimpleTest;
26+
27+
import java.io.ByteArrayInputStream;
28+
import java.io.ByteArrayOutputStream;
29+
import java.io.IOException;
30+
import java.util.Arrays;
31+
import java.util.Date;
32+
33+
public class PGPPaddingTest
34+
extends SimpleTest
35+
{
36+
@Override
37+
public String getName()
38+
{
39+
return "PGPPaddingTest";
40+
}
41+
42+
@Override
43+
public void performTest()
44+
throws Exception
45+
{
46+
randomPaddingIsInBounds();
47+
fixedLenPaddingIsCorrectLength();
48+
negativePaddingLengthThrows();
49+
zeroPaddingLengthThrows();
50+
51+
parsePaddedCertificate();
52+
}
53+
54+
private void randomPaddingIsInBounds()
55+
{
56+
for (int i = 0; i < 10; i++)
57+
{
58+
PGPPadding padding = new PGPPadding();
59+
int len = padding.getPadding().length;
60+
isTrue("Padding length exceeds bounds. Min: " + PGPPadding.MIN_PADDING_LEN +
61+
", Max: " + PGPPadding.MAX_PADDING_LEN + ", Actual: " + len ,
62+
len >= PGPPadding.MIN_PADDING_LEN && len <= PGPPadding.MAX_PADDING_LEN);
63+
}
64+
}
65+
66+
private void fixedLenPaddingIsCorrectLength()
67+
{
68+
PGPPadding padding = new PGPPadding(42);
69+
isEquals("Padding length mismatch", 42, padding.getPadding().length);
70+
}
71+
72+
private void negativePaddingLengthThrows()
73+
{
74+
testException(null, "IllegalArgumentException", () -> new PGPPadding(-1));
75+
}
76+
77+
private void zeroPaddingLengthThrows()
78+
{
79+
testException(null, "IllegalArgumentException", () -> new PGPPadding(0));
80+
}
81+
82+
private void parsePaddedCertificate()
83+
throws PGPException, IOException
84+
{
85+
PGPDigestCalculator digestCalc = new BcPGPDigestCalculatorProvider().get(HashAlgorithmTags.SHA1);
86+
87+
Date creationTime = new Date(1000 * (new Date().getTime() / 1000));
88+
Ed25519KeyPairGenerator edGen = new Ed25519KeyPairGenerator();
89+
edGen.init(new Ed25519KeyGenerationParameters(CryptoServicesRegistrar.getSecureRandom()));
90+
AsymmetricCipherKeyPair edPair = edGen.generateKeyPair();
91+
92+
X25519KeyPairGenerator xGen = new X25519KeyPairGenerator();
93+
xGen.init(new X25519KeyGenerationParameters(CryptoServicesRegistrar.getSecureRandom()));
94+
AsymmetricCipherKeyPair xPair = xGen.generateKeyPair();
95+
96+
PGPKeyPair primaryKeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.Ed25519, edPair, creationTime);
97+
PGPKeyPair subKeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.X25519, xPair, creationTime);
98+
99+
PGPSecretKey secretPrimaryKey = new PGPSecretKey(primaryKeyPair.getPrivateKey(), primaryKeyPair.getPublicKey(), digestCalc, true, null);
100+
PGPSecretKey secretSubKey = new PGPSecretKey(subKeyPair.getPrivateKey(), subKeyPair.getPublicKey(), digestCalc, false, null);
101+
102+
PGPPublicKeyRing certificate = new PGPPublicKeyRing(Arrays.asList(secretPrimaryKey.getPublicKey(), secretSubKey.getPublicKey()));
103+
PGPPadding padding = new PGPPadding();
104+
105+
ByteArrayOutputStream bOut = new ByteArrayOutputStream();
106+
ArmoredOutputStream aOut = ArmoredOutputStream.builder().clearHeaders().build(bOut);
107+
BCPGOutputStream pOut = new BCPGOutputStream(aOut, PacketFormat.CURRENT);
108+
certificate.encode(pOut);
109+
padding.encode(pOut);
110+
111+
pOut.close();
112+
aOut.close();
113+
114+
ByteArrayInputStream bIn = new ByteArrayInputStream(bOut.toByteArray());
115+
ArmoredInputStream aIn = new ArmoredInputStream(bIn);
116+
BCPGInputStream pIn = new BCPGInputStream(aIn);
117+
118+
PGPPublicKeyRing parsed = new PGPPublicKeyRing(pIn, new BcKeyFingerprintCalculator());
119+
isTrue(org.bouncycastle.util.Arrays.areEqual(
120+
certificate.getEncoded(PacketFormat.CURRENT),
121+
parsed.getEncoded(PacketFormat.CURRENT)));
122+
}
123+
124+
public static void main(String[] args)
125+
{
126+
runTest(new PGPPaddingTest());
127+
}
128+
}

0 commit comments

Comments
 (0)