AES-128 Encryption not working on Java < 1.7

Question

I've been chipping away at a school assignment for 3 days, and finally finished it today, error-free and working fine! Except, I was testing it on Java 1.7, and the school servers (where the professor will compile it) run 1.6. So, I tested my code on 1.6, wanting to cover all my bases, and I get a BadPaddingException upon decryption.

[EDIT] Warning: this code does not follow common security practices and should not be used in production code.

Originally, I had this, which works fine on 1.7 (sorry, lots of code.. all relevant..):

public static String aes128(String key, String data, final int direction) {
    SecureRandom rand = new SecureRandom(key.getBytes());
    byte[] randBytes = new byte[16];
    rand.nextBytes(randBytes);
    SecretKey encKey = new SecretKeySpec(randBytes, "AES");

    Cipher cipher = null;
    try {
        cipher = Cipher.getInstance("AES");
        cipher.init((direction == ENCRYPT ? Cipher.ENCRYPT_MODE : Cipher.DECRYPT_MODE), encKey);
    } catch (InvalidKeyException e) {
        return null;
    } catch (NoSuchPaddingException e) {
        return null;
    } catch (NoSuchAlgorithmException e) {
        return null;
    }

    try {
        if (direction == ENCRYPT) {
            byte[] encVal = cipher.doFinal(data.getBytes());
            String encryptedValue = Base64.encode(encVal);
            return encryptedValue;
        } else {
            byte[] dataBytes = Base64.decode(data);
            byte[] encVal = cipher.doFinal(dataBytes);
            return new String(encVal);
        }
    } catch (NullPointerException e) {
        return null;
    } catch (BadPaddingException e) {
        return null;
    } catch (IllegalBlockSizeException e) {
        return null;
    }
}

However, my BadPaddingException catch block executes upon decryption:

javax.crypto.BadPaddingException: Given final block not properly padded
        at com.sun.crypto.provider.SunJCE_f.b(DashoA13*..)
        at com.sun.crypto.provider.SunJCE_f.b(DashoA13*..)
        at com.sun.crypto.provider.AESCipher.engineDoFinal(DashoA13*..)
        at javax.crypto.Cipher.doFinal(DashoA13*..)
        at CipherUtils.aes128(CipherUtils.java:112)
        at CipherUtils.decryptFile(CipherUtils.java:44)
        at decryptFile.main(decryptFile.java:21)

This is what I tried to fix it (basically, I added all the padding/unpadding myself, and used NoPadding):

public static String aes128(String key, String data, final int direction) {
    // PADCHAR = (char)0x10 as String
    while (key.length() % 16 > 0)
        key = key + PADCHAR; // Added this loop

    SecureRandom rand = new SecureRandom(key.getBytes());
    byte[] randBytes = new byte[16];
    rand.nextBytes(randBytes);
    SecretKey encKey = new SecretKeySpec(randBytes, "AES");
    AlgorithmParameterSpec paramSpec = new IvParameterSpec(key.getBytes()); // Created this

    Cipher cipher = null;
    try {
        cipher = Cipher.getInstance("AES/CBC/NoPadding"); // Added CBC/NoPadding
        cipher.init((direction == ENCRYPT ? Cipher.ENCRYPT_MODE : Cipher.DECRYPT_MODE), encKey, paramSpec); // Added paramSpec
    } catch (InvalidKeyException e) {
        return null;
    } catch (NoSuchPaddingException e) {
        return null;
    } catch (NoSuchAlgorithmException e) {
        return null;
    } catch (InvalidAlgorithmParameterException e) {
        return null; // Added this catch{}
    }

    try {
        if (direction == ENCRYPT) {
            while (data.length() % 16 > 0)
                data = data + PADCHAR; // Added this loop

            byte[] encVal = cipher.doFinal(data.getBytes());
            String encryptedValue = Base64.encode(encVal);
            return encryptedValue;
        } else {
            byte[] dataBytes = Base64.decode(data);
            byte[] encVal = cipher.doFinal(dataBytes);
            return new String(encVal);
        }
    } catch (NullPointerException e) {
        return null;
    } catch (BadPaddingException e) {
        return null;
    } catch (IllegalBlockSizeException e) {
        return null;
    }
}

When using this, I just get gibberish in and out:

Out: u¢;èÉ÷JRLòB±J°N°[9cRÐ{ªv=]I¯¿©:
´RLA©êí;R([¶Ü9¸ßv&%®µ^#û|Bá (80)
Unpadded: u¢;èÉ÷JRLòB±J°N°[9cRÐ{ªv=]I¯¿©:
´RLA©êí;R([¶Ü9¸ßv&%®µ^#û|Bá (79)

It is also worth noting that 1.6 and 1.7 produce different encrypted strings.

For example, on 1.7, encrypting xy (including a SHA-1 hash) with key hi produces:

XLUVZBIJv1n/FV2MzaBK3FLPQRCQF2FY+ghyajdqCGsggAN4aac8bfwscrLaQT7BMHJgfnjJLn+/rwGv0UEW+dbRIMQkNAwkGeSjda3aEpk=

On 1.6, the same thing produces:

nqeahRnA0IuRn7HXUD1JnkhWB5uq/Ng+srUBYE3ycGHDC1QB6Xo7cPU6aEJxH7NKqe3kRN3rT/Ctl/OrhqVkyDDThbkY8LLP39ocC3oP/JE=

I didn't expect the assignment to take so long, so my time has run out and it does need to be done tonight. If there is no answer by then, however, I'll just leave a note to my teacher regarding this. It appears to be some issue that was fixed in 1.7... though hopefully can be remedied through the correct addition/fix in my code.

Thanks a ton for everyone's time!

Answer 1

First off:

For almost all systems, encrypting the same plaintext twice should always (i.e. with very very high probability) produce different ciphertext.

The traditional example is that it allows a CPA adversary to distinguish E("attack at dawn") from E("attack at dusk") with only two queries. (There are a handful of systems where you want deterministic encryption, but the right way to do this is "synthetic IV" or cipher modes like CMC and EME.)

Ultimately, the problem is that SecureRandom() is not intended for key derivation.

• If the input "key" is a passphrase, you should be using something like PBKDF2 (or scrypt() or bcrypt()).
  • Additionally, you should be using an explicit charset, e.g. String.getBytes("UTF-8").
• If the input "key" is a key, the most common string representation is a hexdump. Java doesn't include an unhexing function, but there are several here.
  • If the input is a "master key" and you want to derive a subkey, then you should be hashing it with other data. There's not much point if the subkey is always the same.

Additional nitpicks:

• Your code is vulnerable to a padding oracle attack; you really should be verifying a MAC before doing anything with the data (or better, using an authenticated encryption mode).
• In your second listing, you explicitly reuse the IV. Bad! Assuming CBC mode, the IV used should be unpredictable; SecureRandom is useful here.