Cipher doFinal pad block corrupted while decrypting file with AES

Question

I am creating a program that lets users upload and download encrypted files and decrypt them if they have the correct permissions to do so. Encrypting and uploading is fine, and so is downloading, but when I try to decrypt I get the "pad block corrupted" error.

What I am trying to do is take the encrypted file and then make a copy that is unencrypted

I shortened what I could so please don't comment that it looks incomplete.

KeyGen:

public static SecretKey genGroupSecretKey() {
    try {
        KeyGenerator keyGen = KeyGenerator.getInstance("AES", "BC");
        keyGen.init(128);
        return keyGen.generateKey();
    } catch (NoSuchAlgorithmException | NoSuchProviderException e) {
        System.err.println("Error: " + e.getMessage());
        e.printStackTrace(System.err);
    }
    return null;
}

Encrypt:

try (FileInputStream fis = new FileInputStream(sourceFile)) {
    response = Utils.decryptEnv((byte[]) tempResponse.getObjContents().get(0), fsSecretKey, ivSpec);

    Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", "BC");
    cipher.init(Cipher.ENCRYPT_MODE, secretKey, ivSpec);

    do {
        byte[] buf = new byte[4096];

        int n = fis.read(buf); // can throw an IOException
        else if (n < 0) {
            System.out.println("Read error");
            fis.close();
            return false;
        }

        byte[] cipherBuf = cipher.doFinal(buf);

        // send through socket blah blah blah

    } while (fis.available() > 0);

Decrypt:

   ...     

   Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", "BC");
   cipher.init(Cipher.DECRYPT_MODE, secKey, ivSpec);

    File file = new File("D_" + filename); // D_ for decrypted
    FileOutputStream fos = null;
    if (!file.exists()) {
        file.createNewFile();
        fos = new FileOutputStream(file);
    }

    try (FileInputStream fis = new FileInputStream(filename)) {
        do {
            byte[] buf = new byte[4096];
            int n = fis.read(buf);
            if (n < 0) {
                System.out.println("Read error");
                fis.close();
                return false;
            }

            byte[] cipherBuf = cipher.doFinal(buf);  // ERROR HERE
            System.out.println("IS THIS WORKING WTF: " + new String(cipherBuf));
            fos.write(cipherBuf, 0, n);

        } while (fis.available() > 0);
        fis.close();
        fos.close();
        return true;

Answer 1

You should you Cipher.doFinal() only for the very last block of data. You should modify both your encryption and decryption code to use Cipher.update() for all intermediate data and use a single call to Cipher.doFinal() at the end. It is permitted to use Cipher.update() for all intermediate data block and just call the Cipher.doFinal() with the empty array at the end.

Also, you are ignoring the return value of fis.read(buf) when you pass the data read from the stream to the cipher.

Also, you are using InputStream.available() as a condition to end the cycle. Even if the code inside the cycle would be correct, you would be getting unpredictable results due to spurious triggering of cycle condition.

Use the following pattern to work with the ciphers:

Cipher cipher = ...
InputStream in = ...
OutputStream out = ...

byte[] inputBuffer = new byte[ BUFFER_SIZE ];
int r = in.read(inputBuffer);
while ( r >= 0 ) {
    byte[] outputUpdate = cipher.update( inputBuffer, 0, r );
    out.write( outputUpdate );
    r = in.read(inputBuffer);
}
byte[] outputFinalUpdate = cipher.doFinal();
out.write( outputFinalUpdate );

Check other variants of Cipher.update() - it is possible to use two pre-allocated buffers and minimize additional allocations.

Also check if you can reuse javax.crypto.CipherInputStream and javax.crypto.CipherOutputStream classes so that you do not have to work with the cipher directly.