IllegalBlockSizeException when using aes/ecb/pkcs5padding to decrypt a byte array

386 Views Asked by At

I understand that aes encryption needs to be in blocks of 16, but I was under the impression that using Cipher.getInstance("AES/ECB/PKCS5PADDING"); padded the byte array to achieve this. My code is below:

CipherUtils.java

private static byte[] key = {
        0x74, 0x68, 0x69, 0x73, 0x49, 0x73, 0x41, 0x53, 0x65, 0x63, 0x72, 0x65, 0x74, 0x4b, 0x65, 0x79
};//"thisIsASecretKey";

public static byte[] EncryptByteArray(byte[] array)
{
    try
    {
        Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5PADDING");
        SecretKeySpec secretKey = new SecretKeySpec(key, "AES");
        cipher.init(Cipher.ENCRYPT_MODE, secretKey);

        return (cipher.doFinal(array));
    }
    catch (Exception e)
    {
      e.printStackTrace();

    }
    return null;
}

public static byte[] DecryptByteArray(byte[] array)
{
    try
    {
        Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5PADDING");
        SecretKeySpec secretKey = new SecretKeySpec(key, "AES");
        cipher.init(Cipher.DECRYPT_MODE, secretKey);

        return cipher.doFinal(array);
    }
    catch (Exception e)
    {
      e.printStackTrace();

    }
    return null;
}

Main Program

        fis = new FileInputStream(path);

        toDecrypt = new byte[fis.available()+1];

        int content;
        int i = 0;
        while ((content = fis.read()) != -1) {

            // convert to byte and display it
            toDecrypt[i] = (byte)content;
            i += 1;
        }

        byte[] decryptedStr = CipherUtils.DecryptByteArray(toDecrypt);

        FileOutputStream decryptedStream = new FileOutputStream(path);
        decryptedStream.write (decryptedStr);
        decryptedStream.close();

The file at path was encrypted using the function in cipherutils.java and written to the file using FileOutputStream.write

Update- I'm building for Android using Gradle.

2

There are 2 best solutions below

0
On BEST ANSWER

Managed to work something out using @JonSkeet's answer (thanks!) :

        File file = new File(path);
        fis = new FileInputStream(file);

        toDecrypt = new byte[(int)file.length()];
        fis.read(toDecrypt);

        byte[] decrypted = CipherUtils.DecryptByteArray(toDecrypt);
        FileOutputStream decryptedStream = new FileOutputStream(bookPath);
        decryptedStream.write (decrypted);
        decryptedStream.close();

As he pointed out, I shouldn't have been using the available() method. There was also a much better & faster way to write to the file than iterating through each byte! As per the comments, I have also changed the encryption to CBC mode with a random IV.

2
On

This is the problem:

toDecrypt = new byte[fis.available()+1];

Firstly, you're using the available() method, which is never a good idea. Next, even assuming it is returning the length of the file, you're adding 1 to it - why? You want just the bytes in the file, surely.

The simplest way to do that is just to use Files.readAllBytes:

byte[] toDecrypt = Files.readAllBytes(Paths.get(path));
// TODO: Change the method name to follow Java conventions
byte[] decrypted = CipherUtils.DecryptByteArray(toDecrypt);
Files.write(Paths.get(path), decrypted);

Now you don't need to worry about closing the file streams, either... (If you'd managed to decrypt, you probably wouldn't be able to write, because you've still got the file open for reading in your current code.)

I'd also strongly recommend revisiting your exception "handling":

  • Catching Exception is almost always a bad idea
  • Calling e.printStackTrace() and then continuing as if nothing happened is almost always a bad idea