JDK-8064546 : CipherInputStream throws BadPaddingException if stream is not fully read
  • Type: Bug
  • Component: security-libs
  • Sub-Component: javax.crypto
  • Affected Version: 7u72,8u25
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2014-11-11
  • Updated: 2016-08-26
  • Resolved: 2015-04-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7 JDK 8 JDK 9 Other
6u101Fixed 7u85Fixed 8u51Fixed 9 b60Fixed openjdk7uFixed
Related Reports
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_25"
Java(TM) SE Runtime Environment (build 1.8.0_25-b18)
Java HotSpot(TM) 64-Bit Server VM (build 25.25-b02, mixed mode)

Same error for:

java version "1.7.0_71"
Java(TM) SE Runtime Environment (build 1.7.0_71-b14)
Java HotSpot(TM) 64-Bit Server VM (build 24.71-b01, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows [Version 6.1.7601]

A DESCRIPTION OF THE PROBLEM :
When reading data from a CipherInputStream a BadPaddingException is thrown when the stream is closed before everything is read.

REGRESSION.  Last worked in version 8u20

ADDITIONAL REGRESSION INFORMATION: 
java version "1.8.0_20"
Java(TM) SE Runtime Environment (build 1.8.0_20-b26)
Java HotSpot(TM) 64-Bit Server VM (build 25.20-b23, mixed mode)

Also worked with:

java version "1.7.0_67"
Java(TM) SE Runtime Environment (build 1.7.0_67-b01)
Java HotSpot(TM) 64-Bit Server VM (build 24.65-b04, mixed mode)

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
- Decrypt an InputStream via CipherInputStream 
- Do not read all bytes from the stream 
- Close the CipherInputStream 

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Close without exception
ACTUAL -
javax.crypto.BadPaddingException thrown on close

ERROR MESSAGES/STACK TRACES THAT OCCUR :
Exception in thread "main" java.io.IOException: javax.crypto.BadPaddingException: Given final block not properly padded
	at javax.crypto.CipherInputStream.close(CipherInputStream.java:321)
	at temp.EncryptionTest.readData(EncryptionTest.java:88)
	at temp.EncryptionTest.main(EncryptionTest.java:23)
Caused by: javax.crypto.BadPaddingException: Given final block not properly padded
	at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:966)
	at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:824)
	at com.sun.crypto.provider.BlowfishCipher.engineDoFinal(BlowfishCipher.java:319)
	at javax.crypto.Cipher.doFinal(Cipher.java:2004)
	at javax.crypto.CipherInputStream.close(CipherInputStream.java:314)
	... 2 more

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
package temp;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.io.OutputStream;
import java.security.GeneralSecurityException;
import java.security.SecureRandom;

import javax.crypto.Cipher;
import javax.crypto.CipherInputStream;
import javax.crypto.CipherOutputStream;
import javax.crypto.KeyGenerator;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

public class EncryptionTest
{
    public static void main(String[] args) throws Throwable
    {
        EncryptionTest t = new EncryptionTest();
        byte[] d = t.wirteData();
        t.readData(d);
    }
    
    final static int     COUNT_BYTE_TO_WRITE = 999;
    
    final static int     COUNT_BYTE_TO_READ  = 99;
    
    final static boolean ENABLE_WORKAROUND   = false;
    
    Cipher               cipherOut, cipherIn;
    
    SecretKeySpec        key;
    
    byte[]               iv                  = new byte[8]; // Blowfish has 8 IV bytes
                                                            
    public EncryptionTest() throws GeneralSecurityException
    {
        KeyGenerator keygenerator = KeyGenerator.getInstance("Blowfish");
        keygenerator.init(128);
        key = new SecretKeySpec(keygenerator.generateKey().getEncoded(),
                        "Blowfish");
        
        new SecureRandom().nextBytes(iv);
        
        cipherOut = Cipher.getInstance("Blowfish/CBC/PKCS5Padding");
        cipherIn = Cipher.getInstance("Blowfish/CBC/PKCS5Padding");
        
        cipherOut.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(iv));
        cipherIn.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(iv));
        
    }
    
    public byte[] wirteData() throws Throwable
    {
        ByteArrayOutputStream baos = new ByteArrayOutputStream();
        try (OutputStream oute = new CipherOutputStream(baos, cipherOut))
        {
            for (int i = 0; i < COUNT_BYTE_TO_WRITE; i++)
                oute.write(0);
            
            System.out.println("Write OK");
        }
        return baos.toByteArray();
    }
    
    public void readData(byte[] buf) throws Throwable
    {
        try (InputStream in = new ByteArrayInputStream(buf);
             InputStream ine = new CipherInputStream(in, cipherIn))
        {
            for (int i = 0; i < COUNT_BYTE_TO_READ; i++)
            {
                int x = ine.read();
                if (x != 0)
                    throw new RuntimeException("0 !=" + x);
            }
            
            if (ENABLE_WORKAROUND)
            {// WORKAROUND: ignore all other bytes
                while (ine.read() >= 0)
                {
                    while (ine.available() > 0)
                        ine.skip(ine.available());
                }
            }
        }
        
        System.out.println("Read OK");
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Add the following code before closing:

               while (ine.read() >= 0)
                {
                    while (ine.available() > 0)
                        ine.skip(ine.available());
                }


Comments
SQE Ok for all CPU15_03 releases.
26-05-2015

We have this fix in 7u-CPU. Nightly results are ok for 7u-CPU. So, expect low risk for JDK6u101 as well. SQE OK to take it to CPU15_03.
26-05-2015

After offline discussion with someone encountering this problem, their use-case seems reasonable and it's clear that close() should not be throwing exceptions. All useful exceptions happen during read(). AEAD exceptions are thrown by read(), which fully reads, decrypts, and verifies the message no matter how many bytes read() is asked to return. Other ciphers need to fully read the stream before a possible padding or blocksize exception can be thrown, which is reasonable given doFinal() should be called at the end of an encrypted message not the middle.
26-03-2015

The exceptions occur when the 512 byte buffer (ibuffer) does not have the whole encrypted message in it. When doFinal() is called it does not see the end of the encrypted message and throws an execption. However, if the whole encrypted message is in ibuffer, doFinal() completes and no exception is thrown even the user did not read the whole message out of ibuffer. This can be shown by using the provided test and changing the COUNT_BYTE_TO_WRITE between 511 and 513. Throwing an exception should not be based on a buffer size, so it seems reasonable to not throw a padding or blocksize exception when the stream is terminated prematurely in all cases.
26-03-2015

Received following response from the submitter: ------------------------------------------------------------------------------------------------------------------------- On 2/9/2015 3:23 PM, Heckel, Marcel wrote: > > Hi Pardeep, > > I could not find the link to create an account on openjdk.java.net. So I send to the response by email: > > I see several reasons why this behavior in not ok: > > 1) my special case: I have an application that uses ObjectInputStream and ObjectOutputStream to communicate. The encryption of the streams is done one application layer below. My protocol defines that only on object is in the object stream. So I closed the ObjectInputStream after I have read the object. But (I don't know why) ObjectOutputStream sometimes writes 1 byte more than ObjectInputStream reads. > > 2) The interface description of InputStream says nothing about that a stream must be fully read before it is closed. > > 3) The closed() method throws an IOException - If an I/O error has occurred. But a not fully read input stream is not an I/O error in my understanding. > > 4) Regarding the comment: "I would expect an exception to be thrown when partially encrypting or decrypting data otherwise the app may never know there was a failure ": Checking for bit errors is not the primary responsibility of an stream. This should be done in the lower levels (e.g. Layer 2 in OSI in case of network, or the operating system or hard discs in case of files. If I want additional error detection I would add explicitly a check sum to the stream. > > 5) I see also other cases were one don't want to read the full stream: e.g. searching for an encrypted file with some special content. If I know after reading some bytes, that this is not the file I'm looking for, I want to close the stream without reading if completely! > > Regards, > Marcel -------------------------------------------------------------------------------------------------------------------------------
09-02-2015

Sent an email to the submitter: -------------------------------------------------------------------------------------------------------------------------------- On 2/2/2015 3:54 PM, Pardeep Kumar Sharma wrote: > Hi Marcel, > > Can you please go through the evaluation of the bug submitted by you and respond back with the required information? > "Please explain why you would want to not fully read an encrypted stream? > > I would expect an exception to be thrown when partially encrypting or decrypting data otherwise the app may never know there was a failure. It is my opinion the previous methodology was incorrect. I would like input from the submitter in case I am not understand their logic." > > https://bugs.openjdk.java.net/browse/JDK-8064546 > > Thank You, > Pardeep --------------------------------------------------------------------------------------------------------------------------------------
02-02-2015

Please explain why you would want to not fully read an encrypted stream? I would expect an exception to be thrown when partially encrypting or decrypting data otherwise the app may never know there was a failure. It is my opinion the previous methodology was incorrect. I would like input from the submitter in case I am not understand their logic
11-11-2014

Checked for following: JDK 7u67 - Ok 7u72 - Exception encountered 8u20 - OK 8u25 - Exception encountered 8u40 (b12) - Ok
11-11-2014