Make CipherOutputStream.close() idempotent in crypto.io and jcajce.io

Author: Arpan0995

core/src/main/java/org/bouncycastle/crypto/io/CipherOutputStream.java

@@ -27,6 +27,7 @@ public class CipherOutputStream
 
     private final byte[] oneByte = new byte[1];
     private byte[] buf;
+    private boolean closed;
 
     /**
      * Constructs a CipherOutputStream from an OutputStream and a
@@ -223,6 +224,12 @@ public void flush()
     public void close()
         throws IOException
     {
+        if (closed)
+        {
+            return;
+        }
+        closed = true;
+
         ensureCapacity(0, true);
         IOException error = null;
         try

core/src/test/java/org/bouncycastle/crypto/test/CipherStreamTest.java

@@ -494,6 +494,54 @@ private void testReadWrite(Object cipher, CipherParameters params, boolean block
         }
     }
 
+    private void testDoubleClose()
+        throws Exception
+    {
+        KeyParameter key = new KeyParameter(new byte[16]);
+
+        testDoubleClose("AES/CBC/PKCS7", new PaddedBufferedBlockCipher(CBCBlockCipher.newInstance(AESEngine.newInstance()), new PKCS7Padding()),
+            new ParametersWithIV(key, new byte[16]));
+        testDoubleClose("AES/EAX", new EAXBlockCipher(AESEngine.newInstance()), new ParametersWithIV(key, new byte[16]));
+        testDoubleClose("AES/GCM", GCMBlockCipher.newInstance(AESEngine.newInstance()), new ParametersWithIV(key, new byte[12]));
+    }
+
+    private void testDoubleClose(String label, Object cipher, CipherParameters params)
+        throws Exception
+    {
+        byte[] data = new byte[33];
+
+        init(cipher, true, params);
+
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        OutputStream cOut = createCipherOutputStream(bOut, cipher);
+
+        cOut.write(data);
+        cOut.close();
+
+        byte[] expected = bOut.toByteArray();
+
+        cOut.close();
+
+        if (!Arrays.areEqual(expected, bOut.toByteArray()))
+        {
+            fail("second close changed the output for " + label);
+        }
+
+        init(cipher, false, params);
+
+        ByteArrayOutputStream pOut = new ByteArrayOutputStream();
+        OutputStream dOut = createCipherOutputStream(pOut, cipher);
+
+        dOut.write(expected);
+        dOut.close();
+        dOut.close();
+
+        if (!Arrays.areEqual(data, pOut.toByteArray()))
+        {
+            fail("double closed decryption failed for " + label);
+        }
+    }
+
     public void performTest()
         throws Exception
     {
@@ -503,6 +551,8 @@ public void performTest()
             this.streamSize = testSizes[i];
             performTests();
         }
+
+        testDoubleClose();
     }
 
     private void performTests()

docs/releasenotes.md

@@ -63,6 +63,7 @@ Date: 2026, TBD
 - The ML-KEM KeyGenerator (KEMGenerateSpec/KEMExtractSpec), Cipher (wrap/unwrap), javax.crypto.KEM and KeyFactory.translateKey services accepted only BC's own ML-KEM key objects, and the KeyGenerator failed with a ClassCastException at generateKey() rather than at init, so an ML-KEM key from another provider could not be used with BC even though its standard encoding was one BC reads. This broke BCJSSE handshakes over the ML-KEM and hybrid groups whenever another provider ahead of BC decoded the peer's key or generated the ephemeral key pair. A foreign key is now converted from its X.509 or PKCS#8 encoding, with the usual parameter-set checks, and an unusable one is rejected at init (github #2466).
 - The raw JCA provider bounded the PBKDF2 iteration count taken from an encoding (org.bouncycastle.pbe.max_iteration_count, default 10,000,000) but not the counts of the legacy PBES1 (PKCS#5 scheme 1) and PKCS#12 PBE families beside it. Their AlgorithmParameters (PKCS12PBE and its OID aliases, PBKDF1) accepted any count, narrowing one beyond the int range with intValue() so that 2^32 arrived as 0, and every Cipher, Mac and SecretKeyFactory derivation ran with whatever count it was given - including a count decoded by another provider's AlgorithmParameters, as when javax.crypto.EncryptedPrivateKeyInfo.getKeySpec() decrypts a PKCS#12 PBE-protected key with BC. As these schemes carry the count in unauthenticated parameters and derive before anything can be checked, a supplied blob could hold a derivation for tens of minutes. The parameter parse now rejects a negative, beyond-int or over-limit count, and the derivations reject a negative or over-limit count, under the same property as PBKDF2. The PKCS#12 key store derives through the same code, so a org.bouncycastle.pkcs12.max_it_count raised above 10,000,000 now needs org.bouncycastle.pbe.max_iteration_count raised with it.
 - The light-weight CryptoProWrapEngine (RFC 4357 sec. 6.3) diversified the key encryption key in the caller's own array, so after init the KeyParameter it was given held the diversified key, and initialising again with the same parameters - to unwrap what had just been wrapped, say - diversified it a second time and used a different key. It also failed with a NullPointerException when given no S-box, although init has a branch for that case. It now diversifies a copy, and given no S-box uses the GOST 28147 engine's default S-box for the diversification, the one the wrap itself then uses. The provider's GOST 28147 key wrap ciphers were unaffected, as they always supply an S-box and build a new KeyParameter on every init.
+- Closing an org.bouncycastle.crypto.io.CipherOutputStream or org.bouncycastle.jcajce.io.CipherOutputStream a second time finalised the cipher again, although java.io.Closeable specifies that closing a stream that is already closed has no effect, and javax.crypto.CipherOutputStream does nothing on a repeated close(). The first close() leaves the cipher reset, so the second finalised it with no input and wrote the result after the ciphertext: one more padding block for a padded block cipher, one more tag for EAX, CCM, OCB and GCM-SIV. The output then failed to decrypt, or in ECB mode decrypted without error to different data. GCM, which may not be reused for encryption, made the second close() throw an IOException instead, as did writing the extra block or tag to a sink that refuses writes once closed, such as a FileOutputStream. In decrypt mode a padded or AEAD stream threw on the second close() after writing out the whole plaintext, an AEAD mode reporting invalid ciphertext for a message whose tag had already been verified. A try-with-resources block that also closed the stream itself, or that declared a wrapping stream such as a BufferedOutputStream as well, was enough to cause this. Both streams now record the first close() and return at once from any later call.
 
 ### 2.1.3 Additional Features and Functionality
 

prov/src/main/java/org/bouncycastle/jcajce/io/CipherOutputStream.java

@@ -31,6 +31,7 @@ public class CipherOutputStream
 {
     private final Cipher cipher;
     private final byte[] oneByte = new byte[1];
+    private boolean closed;
 
     /**
      * Constructs a CipherOutputStream from an OutputStream and a Cipher.
@@ -108,6 +109,12 @@ public void flush()
     public void close()
         throws IOException
     {
+        if (closed)
+        {
+            return;
+        }
+        closed = true;
+
         IOException error = null;
         try
         {

prov/src/test/java/org/bouncycastle/jce/provider/test/CipherStreamTest2.java

@@ -463,6 +463,44 @@ private static Key generateKey(String name)
         return kGen.generateKey();
     }
 
+    private void testDoubleClose(String name)
+            throws Exception
+    {
+        Key key = generateKey(name);
+        Cipher encrypt = Cipher.getInstance(name, "BC");
+        Cipher decrypt = Cipher.getInstance(name, "BC");
+        encrypt.init(Cipher.ENCRYPT_MODE, key);
+        decrypt.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(encrypt.getIV()));
+
+        byte[] data = new byte[33];
+        ByteArrayOutputStream bOut = new ByteArrayOutputStream();
+        OutputStream cOut = new CipherOutputStream(bOut, encrypt);
+
+        cOut.write(data);
+        cOut.close();
+
+        byte[] expected = bOut.toByteArray();
+
+        cOut.close();
+
+        if (!Arrays.areEqual(expected, bOut.toByteArray()))
+        {
+            fail("second close changed the output: " + name);
+        }
+
+        ByteArrayOutputStream pOut = new ByteArrayOutputStream();
+        OutputStream dOut = new CipherOutputStream(pOut, decrypt);
+
+        dOut.write(expected);
+        dOut.close();
+        dOut.close();
+
+        if (!Arrays.areEqual(data, pOut.toByteArray()))
+        {
+            fail("double closed decryption failed: " + name);
+        }
+    }
+
     public void performTest()
             throws Exception
     {
@@ -472,6 +510,14 @@ public void performTest()
             this.streamSize = testSizes[i];
             performTests();
         }
+
+        testDoubleClose("AES/CBC/PKCS5Padding");
+        testDoubleClose("AES/EAX/NoPadding");
+        String jvm = System.getProperty("java.version");
+        if (!(jvm.length() > 2 && (jvm.charAt(2) == '5' || jvm.charAt(2) == '6')))
+        {
+            testDoubleClose("AES/GCM/NoPadding");
+        }
     }
 
     private void performTests()