Why doesn't this simple AES encryption work?

Question

Why does not this AES encryption work? I've written it in Java to test, but I am not able to decrypt. I get garbage upon decryption. Why? Its so simple - In the main method, print plain text, encrypt, print cipher text, decrypt, print plain text again. Am I doing something wrong? Please help me figure out the problem.

import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

public class AESTest {
    public static void main(String [] args) {
        try {
            String plainText = "Hello World!!!!!";
            String encryptionKey = "E072EDF9534053A0B6C581C58FBF25CC";

            System.out.println("Before encryption - " + plainText);

            String cipherText = encrypt(plainText, encryptionKey);

            System.out.println("After encryption - " + cipherText);

            String decrypted = decrypt(cipherText, encryptionKey);

            System.out.println("After decryption - " + decrypted);
        } catch (Exception e) {
            e.printStackTrace();
        } 
    }

    public static String encrypt(String plainText, String passkey) throws Exception {
        Cipher cipher = Cipher.getInstance("AES/CBC/NoPadding", "SunJCE");
        SecretKeySpec key = new SecretKeySpec(hexStringToByteArray(passkey), "AES");
        cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(new byte[cipher.getBlockSize()]));
        String cipherText = new String(cipher.doFinal(plainText.getBytes()));
        return cipherText;
    }

    public static String decrypt(String cipherText, String passkey) throws Exception{
        Cipher cipher = Cipher.getInstance("AES/CBC/NoPadding", "SunJCE");
        SecretKeySpec key = new SecretKeySpec(hexStringToByteArray(passkey), "AES");
        cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(new byte[cipher.getBlockSize()]));
        String plainText = new String(cipher.doFinal(cipherText.getBytes()));
        return plainText;
    }

    public static byte[] hexStringToByteArray(String s) {
        int len = s.length();
        byte[] data = new byte[len / 2];
        for (int i = 0; i < len; i += 2) {
            data[i / 2] = (byte) ((Character.digit(s.charAt(i), 16) << 4)
                                 + Character.digit(s.charAt(i+1), 16));
        }
        return data;
    }
}

Answer 1

The output of the cipher is a sequence of random-looking bytes. You have no guarantee that these bytes will be a valid encoding for a character string in whatever is your system's default encoding. So this line:

 String cipherText = new String(cipher.doFinal(.....));

is likely to lose information that you'll need for decryption.

Therefore you will not get the right bytes reconstructed in your decrypt operation. For example, if your default encoding is UTF-8, it is overwhelmingly unlikely that the correct ciphertext is something that String.getBytes() is even able to produce.

Answer 2

Two things:

No padding can only work if you use input that is an exact mulitple of your key size, which is 128 bit or 16 bytes. So in your particular case "Hello World!!!!!".getBytes() is actually a multiple of 16, but this is of course not true for arbitrary Strings.

Use "AES/CBC/PKCS5Padding" instead to solve this issue.

Do not turn your encrypted data into a String - this will and change the encrypted output. There's no guarantee that new String(byte[]).getBytes() returns the exact same byte array! So you should leave the encrypted data as what it is - a stream of bytes. Thus encrypt should return byte[] instead and decrypt should take byte[] as input - this is a working example:

public class NewClass {
    public static void main(String [] args) {
        try {
            String plainText = "Hello World!!!!";
            String encryptionKey = "E072EDF9534053A0B6C581C58FBF25CC";

            System.out.println("Before encryption - " + plainText);

            byte[] cipherText = encrypt(plainText, encryptionKey);

            System.out.println("After encryption - " + cipherText);

            String decrypted = decrypt(cipherText, encryptionKey);

            // -> Hello World!!!!
            System.out.println("After decryption - " + decrypted);
        } catch (Exception e) {
            e.printStackTrace();
        } 
    }

    public static byte[] encrypt(String plainText, String passkey) throws Exception {
        Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", "SunJCE");
        SecretKeySpec key = new SecretKeySpec(hexStringToByteArray(passkey), "AES");
        cipher.init(Cipher.ENCRYPT_MODE, key, new IvParameterSpec(new byte[cipher.getBlockSize()]));
        return cipher.doFinal(plainText.getBytes());
    }

    public static String decrypt(byte[] cipherText, String passkey) throws Exception{
        Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", "SunJCE");
        SecretKeySpec key = new SecretKeySpec(hexStringToByteArray(passkey), "AES");
        cipher.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(new    byte[cipher.getBlockSize()]));
        return new String(cipher.doFinal(cipherText));
} 

Answer 3

You need to create the SecretKeySpec object once and use it for both encrypt and decrypt. Currently the code is creating two different keys for each operation and this will definitely lead to incorrect results.