Given final block not properly padded

Question

I am trying to implement password based encryption algorithm, but I get this exception:

javax.crypto.BadPaddingException: Given final block not properly padded

What might be the problem?

Here is my code:

public class PasswordCrypter {

    private Key key;

    public PasswordCrypter(String password)  {
        try{
            KeyGenerator generator;
            generator = KeyGenerator.getInstance("DES");
            SecureRandom sec = new SecureRandom(password.getBytes());
            generator.init(sec);
            key = generator.generateKey();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }


    public byte[] encrypt(byte[] array) throws CrypterException {
        try{
            Cipher cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
            cipher.init(Cipher.ENCRYPT_MODE, key);

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

    public byte[] decrypt(byte[] array) throws CrypterException{
        try{
            Cipher cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
            cipher.init(Cipher.DECRYPT_MODE, key);

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

(The JUnit Test)

public class PasswordCrypterTest {

    private static final byte[] MESSAGE = "Alpacas are awesome!".getBytes();
    private PasswordCrypter[] passwordCrypters;
    private byte[][] encryptedMessages;

    @Before
    public void setUp() {
        passwordCrypters = new PasswordCrypter[] {
            new PasswordCrypter("passwd"),
            new PasswordCrypter("passwd"),
            new PasswordCrypter("otherPasswd")
        };

        encryptedMessages = new byte[passwordCrypters.length][];
        for (int i = 0; i < passwordCrypters.length; i++) {
            encryptedMessages[i] = passwordCrypters[i].encrypt(MESSAGE);
        }
    }

    @Test
    public void testEncrypt() {
        for (byte[] encryptedMessage : encryptedMessages) {
            assertFalse(Arrays.equals(MESSAGE, encryptedMessage));
        }

        assertFalse(Arrays.equals(encryptedMessages[0], encryptedMessages[2]));
        assertFalse(Arrays.equals(encryptedMessages[1], encryptedMessages[2]));
    }

    @Test
    public void testDecrypt() {
        for (int i = 0; i < passwordCrypters.length; i++) {
            assertArrayEquals(MESSAGE, passwordCrypters[i].decrypt(encryptedMessages[i]));
        }

        assertArrayEquals(MESSAGE, passwordCrypters[0].decrypt(encryptedMessages[1]));
        assertArrayEquals(MESSAGE, passwordCrypters[1].decrypt(encryptedMessages[0]));

        try {
            assertFalse(Arrays.equals(MESSAGE, passwordCrypters[0].decrypt(encryptedMessages[2])));
        } catch (CrypterException e) {
            // Anything goes as long as the above statement is not true.
        }

        try {
            assertFalse(Arrays.equals(MESSAGE, passwordCrypters[2].decrypt(encryptedMessages[1])));
        } catch (CrypterException e) {
            // Anything goes as long as the above statement is not true.
        }
    }
}

Answer 1

If you try to decrypt PKCS5-padded data with the wrong key, and then unpad it (which is done by the Cipher class automatically), you most likely will get the BadPaddingException (with probably of slightly less than 255/256, around 99.61%), because the padding has a special structure which is validated during unpad and very few keys would produce a valid padding.

So, if you get this exception, catch it and treat it as "wrong key".

This also can happen when you provide a wrong password, which then is used to get the key from a keystore, or which is converted into a key using a key generation function.

Of course, bad padding can also happen if your data is corrupted in transport.

That said, there are some security remarks about your scheme:

• For password-based encryption, you should use a SecretKeyFactory and PBEKeySpec instead of using a SecureRandom with KeyGenerator. The reason is that the SecureRandom could be a different algorithm on each Java implementation, giving you a different key. The SecretKeyFactory does the key derivation in a defined manner (and a manner which is deemed secure, if you select the right algorithm).

• Don't use ECB-mode. It encrypts each block independently, which means that identical plain text blocks also give always identical ciphertext blocks.

  Preferably use a secure mode of operation, like CBC (Cipher block chaining) or CTR (Counter). Alternatively, use a mode which also includes authentication, like GCM (Galois-Counter mode) or CCM (Counter with CBC-MAC), see next point.

• You normally don't want only confidentiality, but also authentication, which makes sure the message is not tampered with. (This also prevents chosen-ciphertext attacks on your cipher, i.e. helps for confidentiality.) So, add a MAC (message authentication code) to your message, or use a cipher mode which includes authentication (see previous point).

• DES has an effective key size of only 56 bits. This key space is quite small, it can be brute-forced in some hours by a dedicated attacker. If you generate your key by a password, this will get even faster. Also, DES has a block size of only 64 bits, which adds some more weaknesses in chaining modes. Use a modern algorithm like AES instead, which has a block size of 128 bits, and a key size of 128 bits (for the standard variant).