BadPaddingException in CryptoManager

[Nucifera8472]

When using this CryptoManager I am getting a BadPaddingException.
I was able to fix it, and I have a theory what might be the cause of this:
In the CryptoManager code you also write the size of the encrypted data. When you read the size information back, you call read(), which will read 1 byte, but an Integer has a size of 4 bytes. So the amount of Bytes read for the encrypted data is wrong, which can result in BadPaddingException. Maybe this works for very small data sizes, since it worked for your sample, I don't have an explanation for it.
My fix was simply to not write the size of the encrypted data to the stream, and when reading the encrypted data part from the stream I am using readBytes() kotlin extension function to just read the rest of the stream.

[Dolanpls]

Thanks for the update @Nucifera8472 👍
So you mean to remove it.write(encryptedBytes.size) in the encrypt method and instead of calling

val encryptedBytesSize = it.read()
val encryptedBytes = ByteArray(encryptedBytesSize)
it.read(encryptedBytes)

when decrypt simply use

val encryptedBytes = it.readBytes()
it.read(encryptedBytes)

Is that correct?
Anyway, now I'm trying to encode a ~430kb file and the encrypted result is ~100kb
That is strange 😵‍💫 and when I try to decrypt the new file (~100kb) the result is a file with same size but is not the original photo (Neither of the two 100kb files can be opened)

Code:

cryptoManager.encrypt(sample.readBytes(), FileOutputStream(encrypted))
val decryptedByteArray = cryptoManager.decrypt(encrypted.inputStream())
decrypted.writeBytes(decryptedByteArray)

[Nucifera8472]

@Dolanpls Here are my encrypt and decrypt functions.

I've also made the following changes from the original code:

• I only call encryptCipher once and put it in a variable, otherwise it will create a new Cipher instance every time unnecessarily
• For the iv I also don't write the size to the stream, instead, I instantiate with a fixed key size and then use this key size (== iv size) to extract the correct bytes for the iv from the stream.

    fun encrypt(bytes: ByteArray, outputStream: OutputStream): ByteArray {
        val cipher = encryptCipher
        val encryptedBytes = cipher.doFinal(bytes)
        val iv = cipher.iv

        outputStream.use {
            it.write(iv)
            it.write(encryptedBytes)
        }
        return encryptedBytes
    }

    fun decrypt(inputStream: InputStream): ByteArray {
        return inputStream.use {
            val iv = ByteArray(KEY_SIZE)
            it.read(iv)
            val encryptedBytes = it.readBytes()
            getDecryptCipherForIv(iv).doFinal(encryptedBytes)
        }
    }

[marksheekey]

I believe the error can easily be solved by changing the parameter name here...

private fun getDecryptCipherForIv(iv: ByteArray): Cipher {
        return Cipher.getInstance(TRANSFORMATION).apply {
            init(Cipher.DECRYPT_MODE, getKey(), IvParameterSpec(iv))
        }
    }

If you change iv - to be storedIv and pass that that to ivParameter it should work (it worked for me).
Why?
Because in the scope of Cipher there is a method getIV(), this actually regens a new value. Android studio thinks it's begn smart by reducing getIv() to a property access, so that iv in the original code is actually doing a getIV(), not passing the value of iv.

[nilsi]

@Nucifera8472 Could you share the whole file? I'm not quite following how you're using the KEY_SIZE constant.

Still having BadPaddingException when trying to save an id access token in the datastore.

[nilsi]

It's definitely crashing if what you are trying to encrypt is too long/big. As my token seems to be.

[nilsi]

@Nucifera8472 I read your initial post again and see that you had a solution there.

So for me, it was enough to just read the rest of the stream instead of the length of the encrypted message since that does not work for big messages:

    fun encrypt(bytes: ByteArray, outputStream: OutputStream): ByteArray {
        val encryptedBytes = encryptCipher.doFinal(bytes)
        outputStream.use {
            it.write(encryptCipher.iv.size)
            it.write(encryptCipher.iv)
            it.write(encryptedBytes)
        }
        return encryptedBytes
    }

    fun decrypt(inputStream: InputStream): ByteArray {
        return inputStream.use {
            val ivSize = it.read()
            val iv = ByteArray(ivSize)
            it.read(iv)
            getDecryptCipherForIv(iv).doFinal(it.readBytes())
        }
    }

Are there any other known issues with this?

[Nucifera8472]

@nilsi Yes I can confirm what you are saying, this does not work with larger data. When testing my changed code with a larger json string, the middle part of it gets corrupted. As stated in this stackoverflow post, the internal crypto library code seems to fail silently on large data. Changing my code to a chunked encryption/decryption approached fixed this problem as well.

Here is the full class for your convenience if you wanna try it out for yourself.

class CryptoManager @Inject constructor() {

    private val keyStore = KeyStore.getInstance("AndroidKeyStore").apply {
        load(null)
    }

    private val encryptCipher
        get() = Cipher.getInstance(TRANSFORMATION).apply {
            init(Cipher.ENCRYPT_MODE, getKey())
        }

    private fun getDecryptCipherForIv(iv: ByteArray): Cipher {
        return Cipher.getInstance(TRANSFORMATION).apply {
            init(Cipher.DECRYPT_MODE, getKey(), IvParameterSpec(iv))
        }
    }

    private fun getKey(): SecretKey {
        val existingKey = keyStore.getEntry(ALIAS, null) as? KeyStore.SecretKeyEntry
        return existingKey?.secretKey ?: createKey()
    }

    private fun createKey(): SecretKey {
        return KeyGenerator.getInstance(ALGORITHM).apply {
            init(
                KeyGenParameterSpec.Builder(
                    ALIAS,
                    KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT
                )
                    .setKeySize(KEY_SIZE * 8) // key size in bits
                    .setBlockModes(BLOCK_MODE)
                    .setEncryptionPaddings(PADDING)
                    .setUserAuthenticationRequired(false)
                    .setRandomizedEncryptionRequired(true)
                    .build()
            )
        }.generateKey()
    }

    fun encrypt(bytes: ByteArray, outputStream: OutputStream) {
        val cipher = encryptCipher
        val iv = cipher.iv
        outputStream.use {
            it.write(iv)
            // write the payload in chunks to make sure to support larger data amounts (this would otherwise fail silently and result in corrupted data being read back)
            ////////////////////////////////////
            val inputStream = ByteArrayInputStream(bytes)
            val buffer = ByteArray(CHUNK_SIZE)
            while (inputStream.available() > CHUNK_SIZE) {
                inputStream.read(buffer)
                val ciphertextChunk = cipher.update(buffer)
                it.write(ciphertextChunk)
            }
            // the last chunk must be written using doFinal() because this takes the padding into account
            val remainingBytes = inputStream.readBytes()
            val lastChunk = cipher.doFinal(remainingBytes)
            it.write(lastChunk)
            //////////////////////////////////
        }    
    }

    fun decrypt(inputStream: InputStream): ByteArray {
        return inputStream.use {
            val iv = ByteArray(KEY_SIZE)
            it.read(iv)
            val cipher = getDecryptCipherForIv(iv)
            val outputStream = ByteArrayOutputStream()

            // read the payload in chunks to make sure to support larger data amounts (this would otherwise fail silently and result in corrupted data being read back)
            ////////////////////////////////////
            val buffer = ByteArray(CHUNK_SIZE)
            while (inputStream.available() > CHUNK_SIZE) {
                inputStream.read(buffer)
                val ciphertextChunk = cipher.update(buffer)
                outputStream.write(ciphertextChunk)
            }
            // the last chunk must be read using doFinal() because this takes the padding into account
            val remainingBytes = inputStream.readBytes()
            val lastChunk = cipher.doFinal(remainingBytes)
            outputStream.write(lastChunk)
            //////////////////////////////////

            outputStream.toByteArray()
        }
    }

    companion object {
        private const val CHUNK_SIZE = 1024 * 4 // bytes
        private const val KEY_SIZE = 16 // bytes
        private const val ALIAS = "my_alias"
        private const val ALGORITHM = KeyProperties.KEY_ALGORITHM_AES
        private const val BLOCK_MODE = KeyProperties.BLOCK_MODE_CBC
        private const val PADDING = KeyProperties.ENCRYPTION_PADDING_PKCS7
        private const val TRANSFORMATION = "$ALGORITHM/$BLOCK_MODE/$PADDING"
    }

}

[nilsi]

Ah okay. Thanks a lot! I gave up on this approach yesterday and went for EncryptedSharedPreferences instead. I also had issues after killing and restarting the app even with smaller texts so I thought it seemed too fragile to use in production. Such a shame since I spent quite a bit of time trying to make it work.

[IvanWHATS]

@Nucifera8472, thank you very match on your solution. I couldn't figure out what was causing the problem. Your code solved a problem I was struggling with for a few days.
But, could you please explain why CHUNK_SIZE is exactly 1024 * 4 and why KEY_SIZE is 16 and then * 8? And why these constants should be such values?

[Nucifera8472]

@IvanWHATS I'm glad this helped you. Here is some background information:

• For AES, the legal key sizes are 128, 192, and 256 bits. I chose 128 bits (16 Bytes)
• IV size is equal to the block size, which is 16 Bytes. It's actually bad code here because I reused the KEY_SIZE constant, since in this setup it's the same value, but for other key sizes it's not.
• I mostly use Bytes instead of bits in the code because these sizes are often used to instantiate byte arrays, so it's more convenient, but the setKeySize() method requires the parameter to be in bits, so multiplication by 8 was needed to convert Bytes back to bits.
• CHUNK_SIZE you can choose how you like. It shouldn't be so big that encryption doesn't work anymore, but that size varies between devices. I assume that when this size is quite small there might be a little bit of an overhead, which you could notice for large overall sizes, but I haven't tested this. I copied the constants from Stackoverflow, and didn't change the 1024*4 afterward. Could have written 4096 of course ^^, but I get that it's sometimes easier to write 4KB this way.

[IvanWHATS]

@Nucifera8472, thanks or your reply. Now it's much clearer.
I want to add that this solution works for relatively small data too. In my case, I was having issues with decrypted data corruption where it's total size was 42 bytes. And this method helped me.

[yogeshpaliyal]

Here is How I modified the CryptoManager file to support for larger data to encrypt and decrypt.
https://github.com/yogeshpaliyal/KeyPass/pull/668/files

[abalta]

Here is How I modified the CryptoManager file to support for larger data to encrypt and decrypt. https://github.com/yogeshpaliyal/KeyPass/pull/668/files

I use your entire CryptoManager class, but I still has IllegalBlockSizeException.

[Ev357]

i don't care about the file, i just used it for simple passwords so i modified the functions like this:

fun encrypt(clearText: String): String {
    val cipherText =
        Base64.encodeToString(encryptCipher.doFinal(clearText.toByteArray()), Base64.DEFAULT)
    val iv = Base64.encodeToString(encryptCipher.iv, Base64.DEFAULT)

    return "${cipherText}.$iv"
}
fun decrypt(cipherText: String): String {
    val array = cipherText.split(".")
    val cipherData = Base64.decode(array.first(), Base64.DEFAULT)
    val iv = Base64.decode(array[1], Base64.DEFAULT)

    val clearText = getDecryptCipherForIv(iv).doFinal(cipherData)

    return String(clearText, 0, clearText.size, Charsets.UTF_8)
}

and solved the BadPaddingException by reverting commit