okio
okio copied to clipboard
AES/GCM encryption with CipherSink fails on Android
Basically, I have a piece of code that grabs a file, splits it into chunks and encrypts each one individually before sending everything over the network.
For creating the encryption sink, cipherSink is called and then a bunch of segments are written; the unit tests are running just fine and working as expected but when I tested this on Android, I got the following exception:
java.lang.IllegalStateException: Unexpected output size 8208 for input size 16
at okio.CipherSink.update(CipherSink.kt:53)
at okio.CipherSink.write(CipherSink.kt:40)
...
After some digging, I managed to narrow it down to differences in behavior between the default JVM crypto provider and the one used on Android, when calls are made to cipher.update(...) for AEAD ciphers. Normally, calling update returns encrypted data but on Android the encrypted data is returned all at once, after a call to cipher.doFinal(...); all previous calls to update buffer the data but return nothing. This is intentional - see google/conscript#657, google/conscript#697 so it's unlikely to change.
Now we get to the trouble with CipherSink.update(...) (and, I assume CipherSource since the code is similar):
var outputSize = cipher.getOutputSize(size)
while (outputSize > Segment.SIZE) {
check(size > blockSize) { "Unexpected output size $outputSize for input size $size" }
size -= blockSize
outputSize = cipher.getOutputSize(size)
}
What happens here (on Android) is that for each call to CipherSink.update, nothing is actually encrypted or passed down the stream until cipher.doFinal(...) is called; cipher.getOutputSize(size), however, will return the amount that's currently buffered plus however much is needed to encrypt size amount of bytes. After "encrypting" some data this way, outputSize > Segment.SIZE will evaluate to true and we go in the while block. The code then tries to reduce the size of the data we are trying to encrypt so that it can fit in a single segment but it doesn't matter because the cipher is already buffering everything we gave it up to that point; getOutputSize will always return too much data. Eventually, the check fails and the above exception shows up.
Unless I got things wrong, the gist is that on Android we can encrypt/decrypt only about 8kb at a time and that's not great.
Since I'm not familiar at all with okio's internals, I don't really have a fix for this.
Here's a short test (at the end) and its output that, hopefully, illustrates things a bit better:
Output on Android
[plain/0] written=2048, outputSize=2064, encrypted=0
[plain/1] written=4096, outputSize=4112, encrypted=0
[plain/2] written=6144, outputSize=6160, encrypted=0
[plain/3] written=8192, outputSize=8208, encrypted=0
[plain/4] written=10240, outputSize=10256, encrypted=0
[plain/final] written=10240, outputSize=12304, encrypted=10256
[okio/0] written=2048, outputSize=?, encrypted=?
[okio/1] written=4096, outputSize=?, encrypted=?
[okio/2] written=6144, outputSize=?, encrypted=?
----- begin exception -----
java.lang.IllegalStateException: Unexpected output size 8208 for input size 16
Output on JVM
[plain/0] written=2048, outputSize=2064, encrypted=2048
[plain/1] written=4096, outputSize=2064, encrypted=2048
[plain/2] written=6144, outputSize=2064, encrypted=2048
[plain/3] written=8192, outputSize=2064, encrypted=2048
[plain/4] written=10240, outputSize=2064, encrypted=2048
[plain/final] written=10240, outputSize=2064, encrypted=16
[okio/0] written=2048, outputSize=?, encrypted=?
[okio/1] written=4096, outputSize=?, encrypted=?
[okio/2] written=6144, outputSize=?, encrypted=?
[okio/3] written=8192, outputSize=?, encrypted=?
[okio/4] written=10240, outputSize=?, encrypted=?
Note the difference between outputSize and encrypted; on Android we get nothing back until doFinal is called even though outputSize keeps increasing.
val ivSize = 12 // bytes
val keySize = 16 // bytes
val tagSize = 16 // bytes
val iv = Random.nextBytes(ivSize)
val key = Random.nextBytes(keySize)
fun createCipher(): Cipher {
val cipher = Cipher.getInstance("AES/GCM/NoPadding")
val spec = GCMParameterSpec(tagSize * 8, iv)
cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(key, "AES"), spec)
return cipher
}
val basePlaintext = "abcdefgh".toByteArray()
val sourceSize = 20 * 1024
val source = Buffer().apply {
(0 until (sourceSize / basePlaintext.size)).map {
write(basePlaintext)
}
}
val writeSize = 10 * 1024L
val stepSize = (2 * 1024L)
val a = encryptPlain(
cipher = createCipher(),
writeSize = writeSize,
stepSize = stepSize,
source = source.copy().readByteArray()
)
val b = encryptWithOkio(
cipher = createCipher(),
writeSize = writeSize,
stepSize = stepSize,
source = source
)
assertThat(a, equalTo(b))
Encrypting using okio:
fun encryptWithOkio(cipher: Cipher, writeSize: Long, stepSize: Long, source: Buffer): ByteString {
val buffer = Buffer()
val sink = buffer.cipherSink(cipher)
var written = 0L
(0 until (writeSize / stepSize)).map {
sink.write(source, byteCount = stepSize)
written += stepSize
println("[okio/$it] written=$written, outputSize=?, encrypted=?")
}
sink.close()
return buffer.readByteArray().toByteString()
}
Encrypting using the plain Java classes:
fun encryptPlain(cipher: Cipher, writeSize: Long, stepSize: Long, source: ByteArray): ByteString {
val sink = ByteArrayOutputStream()
var written = 0L
(0 until (writeSize / stepSize)).map {
val outputSize = cipher.getOutputSize(stepSize.toInt())
val encrypted = cipher.update(source, written.toInt(), stepSize.toInt())
sink.write(encrypted)
written += stepSize
println("[plain/$it] written=$written, outputSize=$outputSize, encrypted=${encrypted.size}")
}
val outputSize = cipher.getOutputSize(stepSize.toInt())
val finalEncrypted = cipher.doFinal()
sink.write(finalEncrypted)
sink.close()
println("[plain/final] written=$written, outputSize=$outputSize, encrypted=${finalEncrypted.size}")
return sink.toByteArray().toByteString()
}
If needed:
import okio.*
import okio.ByteString.Companion.toByteString
import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.MatcherAssert.assertThat
import java.io.ByteArrayOutputStream
import java.nio.file.Files
import java.nio.file.Path
import javax.crypto.Cipher
import javax.crypto.spec.GCMParameterSpec
import javax.crypto.spec.SecretKeySpec
import kotlin.io.use
import kotlin.random.Random
Thanks for reporting this and for all the details. I was able to reproduce as well.
Unfortunately I'm not sure that there's much that can be done, it seems that the default Android implementation of AES-GCM simply doesn't support streaming. Given that, there's probably not even much point of using okio at all with it, there's no gain compared to simply using byte arrays.
However, poking around a bit, I found that BouncyCastle is available as an alternative security provider on Android API 31 and works fine.
val cipher = Cipher.getInstance("AES/GCM/NoPadding", "BC")
I haven't looked deeper, so I can't guarantee whether this provider is always available on all devices and OS versions, and whether it always will be in the future. You can also look into Tink which should be portable, but of course that'll add weight to an Android app.
In the meantime, there's no "generic" way to block this issue in okio, it would have to be something at initialization looking at the Cipher's provider to forbid AndroidOpenSSL version 1.0, and/or document this limitation.
Okio is already integrated with everything else in my project so it made sense to try to use the cipher sink/source. As for using BouncyCastle, yeah, it may or may not continue to be supported; I wouldn't rely on it.
The solution I eventually came to is implementing my own Source and Sink that try to get data from the cipher until something is available. On the JVM that should work as it does now - every call to cipher.update returns something so we pass it along the stream; on Android it will keep trying to get data from cipher.update until the underlying stream is fully consumed, then doFinal will produce the final result.
class CipherSource(
private val source: BufferedSource,
private val cipher: Cipher
) : Source {
private var finalized: Boolean = false
private val processed = Buffer()
override fun read(sink: Buffer, byteCount: Long): Long {
require(byteCount >= 0L) { "Invalid byteCount requested: [$byteCount]" }
if (byteCount == 0L) return 0L
if (!finalized) {
val buffer = Buffer()
var read = 0L
while (!source.exhausted() && read < byteCount) {
source.read(buffer, byteCount)
val currentProcessed = cipher.update(buffer.readByteArray())
read += currentProcessed.size
processed.write(currentProcessed)
}
if (source.exhausted()) {
finalized = true
processed.write(cipher.doFinal())
}
}
return processed.read(sink, processed.size)
}
override fun close() = source.close()
override fun timeout(): Timeout = source.timeout()
}
There's probably a more elegant or performant way of doing this but for now it seems to be working.
Thanks for the explanation. Yeah we can fix this in Okio!
Unfortunately in this code we lose the streaming capabilities of the "normal" implementation of Cipher 😢. If I attempt to read a very large amount of bytes at once, then they will all be written to the Cipher instance, which means it'll have to allocate all at once the space for the encrypted result, rather than using segments.
The general problem is that with this broken AES-GCM implementation of Cipher that Android provides, the call to doFinal inevitably returns the full encrypted data. I therefore don't see any way to make that compatible with a streamed implementation using Okio, since there's no way to guarantee the output could fit within a segment, so it's always necessary to allocate the full byte array to store it, which kind of defeats the purpose of streaming.
I guess the solution for Okio would be some sort of exceptional fallback which would stop streaming when it detects that the implementation isn't working. At that point, I suppose there's nothing better to do than to just update into the cipher all the data from the underlying source, then decrypt it to to a byte array which would be broken down when writing it to the buffer from which the final data is read. Essentially this means replacing the check(size > blockSize) { "Unexpected output size $outputSize for input size $size" } at CipherSource.kt#L66 with:
if (size <= blockSize) {
final = true
buffer.write(cipher.doFinal(source.readByteArray()))
return
}