Migration: Issue with reading StreamingAeadWithFallback input stream after fallback

[jgilfelt]

Refs #14

I am trying to update an app that currently uses the encrypted Preferences DataStore implementation from encrypted-datastore:1.0.0-alpha02. The DataStore was setup as follows:

val aead = AndroidKeysetManager.Builder()
    .withSharedPref(context, "master_keyset", "master_key_preference")
    .withKeyTemplate(KeyTemplates.get("AES256_GCM"))
    .withMasterKeyUri("android-keystore://master_key")
    .build()
    .keysetHandle.getPrimitive(Aead::class.java)

PreferenceDataStoreFactory.createEncrypted(aead) {
    context.preferencesDataStoreFile(DATASTORE_NAME)
}

I have updated to security-crypto-datastore-preferences:1.0.0-alpha04 and changed the setup as follows to enable Aead fallback for existing users as follows:

val aead = AndroidKeysetManager.Builder()
    .withSharedPref(context, "master_keyset", "master_key_preference")
    .withKeyTemplate(KeyTemplates.get("AES256_GCM"))
    .withMasterKeyUri("android-keystore://master_key")
    .build()
    .keysetHandle.getPrimitive(Aead::class.java)
    
PreferenceDataStoreFactory.createEncrypted(
    encryptionOptions = { fallbackAead = aead }
) {
    EncryptedFile.Builder(
        context.preferencesDataStoreFile(DATASTORE_NAME), // Keep the same file
        context,
        MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC),
        EncryptedFile.FileEncryptionScheme.AES256_GCM_HKDF_4KB
    ).build()
}

When the app runs, no errors are raised, but any previously saved datastore key/values are not decrypting properly.

Debugging revealed that when the fallback lambda is executed to get the Aead decrypted FileInputStream, this method is always just returning the original encrypted FileInputStream, because its available() method returns 0 (suggesting that it has already been read).

encrypted-datastore/encrypted-datastore/src/main/kotlin/Aead.kt

Lines 6 to 12 in 22f8a5d

	internal fun Aead.newDecryptedStream(inputStream: InputStream): InputStream {
	return if (inputStream.available() > 0) {
	decrypt(inputStream.readBytes(), null).inputStream()
	} else {
	inputStream
	}
	}

I discovered I could fix the issue by adding a call to stream.close() in the fallback catch block just before stream is reassigned, so that the aborted StreamingAead input stream is closed first before reading from the fallback Aead stream.

encrypted-datastore/encrypted-datastore/src/main/kotlin/migration/StreamingAeadWithFallback.kt

Lines 33 to 42 in 22f8a5d

	override fun read(b: ByteArray, offset: Int, length: Int): Int {
	return try {
	stream.read(b, offset, length)
	} catch (e: IOException) {
	if (!e.isProbablyEncryptedWithAeadException()) throw e
	stream = fallbackStream()
	// Try to read again from the new delegate
	stream.read(b, offset, length)
	}
	}

override fun read(b: ByteArray, offset: Int, length: Int): Int {
    return try {
        stream.read(b, offset, length)
    } catch (e: IOException) {
        if (!e.isProbablyEncryptedWithAeadException()) throw e
        stream.close()                                                       // <-- FIX
        stream = fallbackStream()
        // Try to read again from the new delegate
        stream.read(b, offset, length)
    }
}

I am honestly not sure why it is behaving this way. Perhaps there are some subtle differences between FileInputStream and the ByteArrayInputStream used in the tests?

Hopefully there is enough information here for you to reproduce the issue. Let me know if not.

[osipxd]

Thank you, reproduced!
FileInputStream can not be reused after it was already used by Tink, so available() always returns 0. Fixed it by wrapping InputStream with BufferedInputStream for all streams not supporting marking.

[osipxd]

Please, check if the issue fixed in v1.0.0-beta01. Fell free to reopen this issue in case it is not

[jgilfelt]

Brilliant, thank you! v1.0.0-beta01 is working perfectly now for both new and migrated data store instances.