Symmetric encryption issue in Android 4.3

About a year ago, a bit after 4.3 release, I had to fix an error in one of my applications. I patched it then, but wished to return and understand better the underlying reason. The other day I came back to this code and dug a little deeper. This post is about what I found.

In one of my applications I use symmetric-key algorithm for encryption and decryption of data. Nothing fancy here, just a standard Java APIs everyone uses for such things.

Consider this sample code:



As I mentioned, it is a regular code that you're probably using with some adjustments in your application.
Until Jelly Beans 4.3, it worked as is without any issues. With the introduce of version 4.3, the above code failed to work reliably.
The cause for this is the ShortBufferException that was thrown on Cipher.update(..) function call (line 54):

javax.crypto.ShortBufferException: output buffer too small during update: 4096 < 4112
at org.apache.harmony.xnet.provider.jsse.OpenSSLCipher.updateInternal(OpenSSLCipher.java:317)
at org.apache.harmony.xnet.provider.jsse.OpenSSLCipher.engineUpdate(OpenSSLCipher.java:362)
at javax.crypto.Cipher.update(Cipher.java:989)
at javax.crypto.Cipher.update(Cipher.java:938)
at info.osom.sandbox.Crypto.execute(Crypto.java:54)
at info.osom.sandbox.Crypto.encrypt(Crypto.java:25)

Too small output buffer? The buffer was large enough for previous Android versions, so why it suddenly too short? Let's try to understand what happened here.

First of all, using the below code I will print the installed security providers:


The output lists same security providers on both 4.2.2 and 4.3 versions (in order):

org.apache.harmony.xnet.provider.jsse.OpenSSLProvider
org.apache.harmony.security.provider.cert.DRLCertFactory
com.android.org.bouncycastle.jce.provider.BouncyCastleProvider
org.apache.harmony.security.provider.crypto.CryptoProvider
org.apache.harmony.xnet.provider.jsse.JSSEProvider

But the actual provider that is being used for cipher instance is different between the two versions. On 4.2.2 version the BouncyCastle provider is being used, while on 4.3 the OpenSSLProvider takes its place. It's a time to understand how the security provider gets selected. From Cipher.getInstance() documentation:

Creates a new Cipher for the specified transformation. The installed providers are searched in order for an implementation of the specified transformation. The first found provider providing the transformation is used to create the cipher. If no provider is found an exception is thrown.

In my code I'm requesting for AES/CBC/PKCS5Padding transformation. The function asks each security provider in order whether it has implementation for such transformation. On 4.2.2, the first provider that contains the above transformation is BouncyCastleProvider (which is a third provider), while on 4.3 the first provider (OpenSSLProvider) told it has the implementation.

As you probably already concluded, on 4.3 the OpenSSLProvider was upgraded with new implementations, and one of the additions was the AES/CBC/PKCS5Padding implementation. And indeed, this commit (dated September 24, 2012) introduced "Cipher support for AES through OpenSSL". The author, Kenny Root, added some timings in comment that show a distinct advantage to favor OpenSSL implementation over BouncyCastle (at least for AES/CTR/PKCS5Padding):

Timings using encrypt with 256-bit key in CTR mode and PKCS5Padding:
    implementation inputSize     us linear runtime
           OpenSSL        16   11.4 =
           OpenSSL        32   12.1 =
           OpenSSL        64   13.2 =
           OpenSSL       128   15.1 =
           OpenSSL      1024   44.0 =
           OpenSSL      8192  275.0 ===
      BouncyCastle        16   11.5 =
      BouncyCastle        32   15.9 =
      BouncyCastle        64   24.6 =
      BouncyCastle       128   41.5 =
      BouncyCastle      1024  277.2 ===
      BouncyCastle      8192 2196.9 ==============================

The intent of this change is very much welcomed - after all, this makes our applications run faster. But the impact was harmful, at least for my application. Ok, so after understanding the change that caused the error, let's see why OpenSSL's implementation requires a larger buffer.

The Cipher.update(..) function call eventually calls OpenSSLCipher's engineUpdate(..) method. This is how it looks like:


Note that first of all getOutputSize(..) function is being called, which calculates the output buffer size given the buffer input size. These sizes will never be equal, which means that this implementation ALWAYS requires a larger output buffer. If the input buffer's size is an exact multiple of the block size, then additional block will be required. This is why the exception said that the implementation requires 4112 bytes length output buffer for 4096 bytes input buffer. The difference is 16 bytes, which is the block size. This is actually what the comment of getOutputSize function says:

The size of output if {@code doFinal()} is called with this {@code inputLen}. If padding is enabled and the size of the input puts it right at the block size, it will add another block for the padding.

The additional 16 bytes is a reasonable requirement... for Cipher.doFinal(..) function call (as comment suggests). But not for Cipher.update(..), since on update we don't have to add any padding. And indeed, even though the Cipher.getOutputSize(..) returns 4112 as a size, the Cipher.update(..) writes only 4096 bytes - which means that Cipher.getOutputSize(..) returns false results. I find this implementation very awkward, but hopefully it was done for a reason.

So how to overcome that?
You can of course keep using BouncyCastle implementation. For this, pass a "BC" string as a second argument for a Cipher.getInstance() function. But recall the timings - OpenSSL is much faster than BouncyCastle.

So what need to adjust in order to use the OpenSSL implementation?
You can decrease the input buffer by the block size or increase the output buffer by the same size. For example, if the block size is 16 bytes and the output buffer is 4096 bytes, then use input buffer with the length of 4080. This is how TextSecure fixed the very same issue.