Merge branch 'pr-1716-improve-PGPPadding' into 'main'

pr-1716-improve-PGPPadding

See merge request root/bc-java!18

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

@@ -39,6 +39,10 @@ public PaddingPacket(int octetLen, SecureRandom random)
 
     private static byte[] randomBytes(int octetCount, SecureRandom random)
     {
+        if (octetCount <= 0)
+        {
+            throw new IllegalArgumentException("Octet count MUST NOT be 0 nor negative.");
+        }
         byte[] bytes = new byte[octetCount];
         random.nextBytes(bytes);
         return bytes;

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

@@ -10,6 +10,7 @@
 
 import org.bouncycastle.bcpg.BCPGInputStream;
 import org.bouncycastle.bcpg.Packet;
+import org.bouncycastle.bcpg.PacketFormat;
 import org.bouncycastle.bcpg.PacketTags;
 import org.bouncycastle.bcpg.SignaturePacket;
 import org.bouncycastle.bcpg.TrustPacket;
@@ -144,6 +145,9 @@ public abstract void encode(OutputStream outStream)
     public abstract byte[] getEncoded()
         throws IOException;
 
+    public abstract byte[] getEncoded(PacketFormat format)
+        throws IOException;
+
     private static boolean isUserTag(int tag)
     {
         switch (tag)

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

@@ -1,10 +1,16 @@
 package org.bouncycastle.openpgp;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
+import java.security.SecureRandom;
 
 import org.bouncycastle.bcpg.BCPGInputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
 import org.bouncycastle.bcpg.Packet;
+import org.bouncycastle.bcpg.PacketFormat;
 import org.bouncycastle.bcpg.PaddingPacket;
+import org.bouncycastle.crypto.CryptoServicesRegistrar;
 
 /**
  * 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
 {
     private PaddingPacket p;
 
+    /**
+     * Minimum random padding length in octets.
+     * Chosen totally arbitrarily.
+     */
+    public static final int MIN_PADDING_LEN = 16;
+
+    /**
+     * Maximum random padding length.
+     * Chosen somewhat arbitrarily, as SSH also uses max 255 bytes for random padding.
+     *
+     * @see <a href="https://www.rfc-editor.org/rfc/rfc4253.html#section-6">
+     *     rfc4253 - Binary Packet Protocol</a>
+     */
+    public static final int MAX_PADDING_LEN = 255;
+
     /**
      * Default constructor.
      *
-     * @param in
+     * @param in packet input stream
      * @throws IOException
      */
     public PGPPadding(
@@ -34,8 +55,78 @@ public PGPPadding(
         p = (PaddingPacket)packet;
     }
 
+    /**
+     * Generate a new, random {@link PGPPadding} object.
+     * The padding consists of n random bytes, where n is a number between (inclusive) {@link #MIN_PADDING_LEN}
+     * and {@link #MAX_PADDING_LEN}.
+     */
+    public PGPPadding()
+    {
+        this(CryptoServicesRegistrar.getSecureRandom());
+    }
+
+    /**
+     * Generate a new, random {@link PGPPadding} object.
+     * The padding consists of n random bytes, where n is a number between (inclusive) {@link #MIN_PADDING_LEN}
+     * and {@link #MAX_PADDING_LEN}.
+     *
+     * @param random random number generator instance
+     */
+    public PGPPadding(SecureRandom random)
+    {
+        this(MIN_PADDING_LEN + random.nextInt(MAX_PADDING_LEN - MIN_PADDING_LEN + 1), random);
+    }
+
+    /**
+     * Generate a new, random {@link PGPPadding} object.
+     * The padding consists of <pre>len</pre> random bytes.
+     */
+    public PGPPadding(int len)
+    {
+        this(len, CryptoServicesRegistrar.getSecureRandom());
+    }
+
+    /**
+     * Generate a new, random {@link PGPPadding} object.
+     * The padding consists of <pre>len</pre> random bytes.
+     *
+     * @param len number of random octets
+     * @param random random number generator instance
+     */
+    public PGPPadding(int len, SecureRandom random)
+    {
+        this.p = new PaddingPacket(len, random);
+    }
+
+    /**
+     * Return the padding octets as a byte array.
+     * @return padding octets
+     */
     public byte[] getPadding()
     {
         return p.getPadding();
     }
+
+    public void encode(OutputStream outStream)
+        throws IOException
+    {
+        BCPGOutputStream pOut = BCPGOutputStream.wrap(outStream);
+        p.encode(pOut);
+    }
+
+    public byte[] getEncoded()
+        throws IOException
+    {
+        return getEncoded(PacketFormat.ROUNDTRIP);
+    }
+
+    public byte[] getEncoded(PacketFormat format)
+        throws IOException
+    {
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
+        encode(pOut);
+        pOut.close();
+        return bOut.toByteArray();
+    }
 }

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

@@ -17,7 +17,9 @@
 
 import org.bouncycastle.bcpg.ArmoredInputException;
 import org.bouncycastle.bcpg.BCPGInputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
 import org.bouncycastle.bcpg.Packet;
+import org.bouncycastle.bcpg.PacketFormat;
 import org.bouncycastle.bcpg.PacketTags;
 import org.bouncycastle.bcpg.PublicKeyPacket;
 import org.bouncycastle.bcpg.TrustPacket;
@@ -238,10 +240,16 @@ public Iterator<PGPPublicKey> iterator()
     public byte[] getEncoded()
         throws IOException
     {
-        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
-
-        this.encode(bOut);
+        return getEncoded(PacketFormat.ROUNDTRIP);
+    }
 
+    @Override
+    public byte[] getEncoded(PacketFormat format) throws IOException
+    {
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
+        this.encode(pOut);
+        pOut.close();
         return bOut.toByteArray();
     }
 

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

@@ -15,6 +15,8 @@
 
 import org.bouncycastle.bcpg.ArmoredInputException;
 import org.bouncycastle.bcpg.BCPGInputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
+import org.bouncycastle.bcpg.PacketFormat;
 import org.bouncycastle.bcpg.PacketTags;
 import org.bouncycastle.bcpg.PublicSubkeyPacket;
 import org.bouncycastle.bcpg.SecretKeyPacket;
@@ -389,10 +391,16 @@ public int size()
     public byte[] getEncoded()
         throws IOException
     {
-        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
-
-        this.encode(bOut);
+        return getEncoded(PacketFormat.ROUNDTRIP);
+    }
 
+    @Override
+    public byte[] getEncoded(PacketFormat format) throws IOException
+    {
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        BCPGOutputStream pOut = new BCPGOutputStream(bOut, format);
+        this.encode(pOut);
+        pOut.close();
         return bOut.toByteArray();
     }
 

pg/src/test/java/org/bouncycastle/openpgp/test/PGPPaddingTest.java

@@ -0,0 +1,128 @@
+package org.bouncycastle.openpgp.test;
+
+import org.bouncycastle.bcpg.ArmoredInputStream;
+import org.bouncycastle.bcpg.ArmoredOutputStream;
+import org.bouncycastle.bcpg.BCPGInputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
+import org.bouncycastle.bcpg.HashAlgorithmTags;
+import org.bouncycastle.bcpg.PacketFormat;
+import org.bouncycastle.bcpg.PublicKeyAlgorithmTags;
+import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
+import org.bouncycastle.crypto.CryptoServicesRegistrar;
+import org.bouncycastle.crypto.generators.Ed25519KeyPairGenerator;
+import org.bouncycastle.crypto.generators.X25519KeyPairGenerator;
+import org.bouncycastle.crypto.params.Ed25519KeyGenerationParameters;
+import org.bouncycastle.crypto.params.X25519KeyGenerationParameters;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPKeyPair;
+import org.bouncycastle.openpgp.PGPPadding;
+import org.bouncycastle.openpgp.PGPPublicKeyRing;
+import org.bouncycastle.openpgp.PGPSecretKey;
+import org.bouncycastle.openpgp.operator.PGPDigestCalculator;
+import org.bouncycastle.openpgp.operator.bc.BcKeyFingerprintCalculator;
+import org.bouncycastle.openpgp.operator.bc.BcPGPDigestCalculatorProvider;
+import org.bouncycastle.openpgp.operator.bc.BcPGPKeyPair;
+import org.bouncycastle.util.test.SimpleTest;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Date;
+
+public class PGPPaddingTest
+    extends SimpleTest
+{
+    @Override
+    public String getName()
+    {
+        return "PGPPaddingTest";
+    }
+
+    @Override
+    public void performTest()
+        throws Exception
+    {
+        randomPaddingIsInBounds();
+        fixedLenPaddingIsCorrectLength();
+        negativePaddingLengthThrows();
+        zeroPaddingLengthThrows();
+
+        parsePaddedCertificate();
+    }
+
+    private void randomPaddingIsInBounds()
+    {
+        for (int i = 0; i < 10; i++)
+        {
+            PGPPadding padding = new PGPPadding();
+            int len = padding.getPadding().length;
+            isTrue("Padding length exceeds bounds. Min: " + PGPPadding.MIN_PADDING_LEN +
+                    ", Max: " + PGPPadding.MAX_PADDING_LEN + ", Actual: " + len ,
+                len >= PGPPadding.MIN_PADDING_LEN && len <= PGPPadding.MAX_PADDING_LEN);
+        }
+    }
+
+    private void fixedLenPaddingIsCorrectLength()
+    {
+        PGPPadding padding = new PGPPadding(42);
+        isEquals("Padding length mismatch", 42, padding.getPadding().length);
+    }
+
+    private void negativePaddingLengthThrows()
+    {
+        testException(null, "IllegalArgumentException", () -> new PGPPadding(-1));
+    }
+
+    private void zeroPaddingLengthThrows()
+    {
+        testException(null, "IllegalArgumentException", () -> new PGPPadding(0));
+    }
+
+    private void parsePaddedCertificate()
+        throws PGPException, IOException
+    {
+        PGPDigestCalculator digestCalc = new BcPGPDigestCalculatorProvider().get(HashAlgorithmTags.SHA1);
+
+        Date creationTime = new Date(1000 * (new Date().getTime() / 1000));
+        Ed25519KeyPairGenerator edGen = new Ed25519KeyPairGenerator();
+        edGen.init(new Ed25519KeyGenerationParameters(CryptoServicesRegistrar.getSecureRandom()));
+        AsymmetricCipherKeyPair edPair = edGen.generateKeyPair();
+
+        X25519KeyPairGenerator xGen = new X25519KeyPairGenerator();
+        xGen.init(new X25519KeyGenerationParameters(CryptoServicesRegistrar.getSecureRandom()));
+        AsymmetricCipherKeyPair xPair = xGen.generateKeyPair();
+
+        PGPKeyPair primaryKeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.Ed25519, edPair, creationTime);
+        PGPKeyPair subKeyPair = new BcPGPKeyPair(PublicKeyAlgorithmTags.X25519, xPair, creationTime);
+
+        PGPSecretKey secretPrimaryKey = new PGPSecretKey(primaryKeyPair.getPrivateKey(), primaryKeyPair.getPublicKey(), digestCalc, true, null);
+        PGPSecretKey secretSubKey = new PGPSecretKey(subKeyPair.getPrivateKey(), subKeyPair.getPublicKey(), digestCalc, false, null);
+
+        PGPPublicKeyRing certificate = new PGPPublicKeyRing(Arrays.asList(secretPrimaryKey.getPublicKey(), secretSubKey.getPublicKey()));
+        PGPPadding padding = new PGPPadding();
+
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        ArmoredOutputStream aOut = ArmoredOutputStream.builder().clearHeaders().build(bOut);
+        BCPGOutputStream pOut = new BCPGOutputStream(aOut, PacketFormat.CURRENT);
+        certificate.encode(pOut);
+        padding.encode(pOut);
+
+        pOut.close();
+        aOut.close();
+
+        ByteArrayInputStream bIn = new ByteArrayInputStream(bOut.toByteArray());
+        ArmoredInputStream aIn = new ArmoredInputStream(bIn);
+        BCPGInputStream pIn = new BCPGInputStream(aIn);
+
+        PGPPublicKeyRing parsed = new PGPPublicKeyRing(pIn, new BcKeyFingerprintCalculator());
+        isTrue(org.bouncycastle.util.Arrays.areEqual(
+            certificate.getEncoded(PacketFormat.CURRENT),
+            parsed.getEncoded(PacketFormat.CURRENT)));
+    }
+
+    public static void main(String[] args)
+    {
+        runTest(new PGPPaddingTest());
+    }
+}