1
\$\begingroup\$

One operation of my Android project is to decrypt one file (chosen in a set of ~30 files for pagination1) of ~10 Mb each and I do this with the following:

fun InputStream.cipherDecryptWithSessionPwd(password: String): String {
    var payloadArray: List<Byte>? = this.readBytes().toList()
    var salt = payloadArray?.subList(0,32)
    var spec: Rfc2898DeriveBytes? = Rfc2898DeriveBytes(password, salt?.toByteArray(), 50000)
    var realData = payloadArray?.subList(32, payloadArray.size)

    val keyBytes = spec?.getBytes(256 / 8)
    val ivBytes = spec?.getBytes(128 / 8)

    var cipher: Cipher? = Cipher.getInstance("AES/CFB/PKCS7Padding")
    var secretKey: SecretKeySpec? = SecretKeySpec(keyBytes, "AES")
    var ivSpec: IvParameterSpec? = IvParameterSpec(ivBytes)
    cipher?.init(Cipher.DECRYPT_MODE, secretKey, ivSpec)
    var byteArrayInputStream: ByteArrayInputStream? = ByteArrayInputStream(realData?.toByteArray())
    var decipherStream: CipherInputStream? = CipherInputStream(byteArrayInputStream, cipher)

    val d = BufferedReader(InputStreamReader(decipherStream, Charsets.UTF_8), 0xFFFF)
    return d.readLine()
}

The problem with this is that at around the 72nd time2 I repeat the operation Android kills my application because it runs out of memory3.

This is the second version of the decryption function, the first one caused the OOM error around the 25th time the operation was made. The problem here clearly is the number of conversions done (InputStream -> ByteArray -> List<Byte> and others).

I don't know how to optimize more, the first version of the method was not mine and I never did encryption (and optimization) before.

Scenario (and what have been tried):
1 - Files are stored encrypted on the device, can't have them decrypted but at runtime
2 - With the decrypted result (JSON) I fill a model using [Gson] (https://github.com/google/gson), but the OOME seem to be thrown when my function is running (the exception points at the first line of the function)
3 - Calculations are done and the model is disposed of when the next page is shown, then the whole operation has to be repeated # - We tried reducing the number of JSON "records" inside each file from 5000 per file to 1000 per file reducing the file size to 2 Mb but the problem persists

Notes:
1 - We event tried reducing the pagination reaching files size to 2 Mb but the problem persists
2 - This has to be done at least 130 times per session
3 - With, Knox my application is the only one allowed to run on the device

\$\endgroup\$
6
  • \$\begingroup\$ Well, if it kills it the 72nd time it runs, doesn't that mean that the function you posted has enough memory to run, but there's something else going on outside of the snippet? Are you storing all the results in memory for each file? In normal applications I might say just your results to disk, but since the data seems to be sensitive, maybe not. You might have to limit the amount of results you keep in memory and re-calculate them when you want to view them again. \$\endgroup\$ – Slothario Nov 22 '19 at 15:36
  • \$\begingroup\$ @Slothario - I've edited the question with more details \$\endgroup\$ – Cliff Burton Nov 22 '19 at 15:52
  • \$\begingroup\$ Are you sure the results are fully disposed of, though? Have you tried profiling it for memory leaks, or data that's still hanging around that isn't needed? \$\endgroup\$ – Slothario Nov 22 '19 at 15:58
  • \$\begingroup\$ Yep, I profiled the whole operation (it has been a looong week), I set to null each object when no more needed and nothing worked, I think that asking here is the last resort. One option would drastically change the whole project causing a seatback of months \$\endgroup\$ – Cliff Burton Nov 22 '19 at 16:05
  • \$\begingroup\$ @Slothario - are you asking these questions because the function is good for you? \$\endgroup\$ – Cliff Burton Nov 22 '19 at 16:06
1
\$\begingroup\$

Let's take a look line per line.

var payloadArray: List<Byte>? = this.readBytes().toList()

That's a really terrible idea. Generally you stream files using FileInputStream, or you can map files using the NIO (new I/O) interface of Java. Then you can read the bytes piecemeal.

val salt = payloadArray.sliceArray(0 until 32)

* Salts only have to be 128 bits maximum, 256 bits is complete overkill. You can slice or cut bytes from arrays just as well, or from a stream of course. No need for the salt and realData as List.

val cipher = Cipher.getInstance("AES/CFB/PKCS7Padding")

* CFB mode with PKCS#7 doesn't make any sense; CFB is a streaming mode of operation, so padding the plaintext makes no sense whatsoever. Try NoPadding for future encryptions.

val secretKey: SecretKeySpec? = SecretKeySpec(keyBytes, "AES")

You expect secretKey to become null?

return d.readLine()

Wait, what? Single line string of 10 MB? Somebody needs to learn how to end lines I think. Generally such large strings are not useful, think of a way to split them up. Are you sure you need it all in one go? If not, keep it on disk.

Note that it is perfectly possible to decrypt CFB ciphertext from on any position, as long as the starting position is a multiple of the block size (16 for AES) and the previous ciphertext block is known.


The ones with the * are issues with the protocol, and those cannot be changed without re-encrypting the plaintext.

\$\endgroup\$
0
1
\$\begingroup\$

I can't help too much as I don't have running code, but I can at least get rid of all List and Array conversions, that will make it run faster and reduce lot of memory garbage. I also removed all the optionals as immutability is our best friend :)

Is it ran once per stream and then it is closed or more than once per stream? There is no information about how stream is being handled and if it is closed properly. That can be issue.

fun InputStream.cipherDecryptWithSessionPwd(password: String): String {
    val payloadArray: ByteArray = this.readBytes()
    val salt = payloadArray.sliceArray(0 until 32)
    val spec = Rfc2898DeriveBytes(password, salt, 50000)
    val realData = payloadArray.sliceArray(32 until payloadArray.size)

    val keyBytes = spec.getBytes(256 / 8)
    val ivBytes = spec.getBytes(128 / 8)

    val cipher = Cipher.getInstance("AES/CFB/PKCS7Padding")
    val secretKey: SecretKeySpec? = SecretKeySpec(keyBytes, "AES")
    val ivSpec: IvParameterSpec? = IvParameterSpec(ivBytes)
    cipher.init(Cipher.DECRYPT_MODE, secretKey, ivSpec)
    val byteArrayInputStream = ByteArrayInputStream(realData)
    val decipherStream = CipherInputStream(byteArrayInputStream, cipher)

    val d = BufferedReader(InputStreamReader(decipherStream, Charsets.UTF_8), 0xFFFF)
    return d.readLine()
}

Edit:

I was thinking what you could cache and only possible variable is I guess cipher:

val cipher = Cipher.getInstance("AES/CFB/PKCS7Padding")

It probably won't speed up performance as I expect it to be cached factory method, but you can try putting variable outside of function anyway :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.