From 66d480dbdcc23f31e95fe11e030769f9488f3f96 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 18 Oct 2026 04:09:16 +0700 Subject: [PATCH] fix(storage): retain legacy Android cipher migration support --- .../spotiflac/SecureStorageMigrationTest.kt | 184 ++++++++++++++++++ lib/services/secure_storage_options.dart | 3 + pubspec.lock | 6 +- pubspec.yaml | 7 +- test/extension_storage_service_test.dart | 36 ++++ 5 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 android/app/src/androidTest/kotlin/com/zarz/spotiflac/SecureStorageMigrationTest.kt diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SecureStorageMigrationTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SecureStorageMigrationTest.kt new file mode 100644 index 00000000..d14470a5 --- /dev/null +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/SecureStorageMigrationTest.kt @@ -0,0 +1,184 @@ +package com.zarz.spotiflac + +import android.content.Context +import android.content.ContextWrapper +import android.content.SharedPreferences +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import android.util.Base64 +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.it_nomads.fluttersecurestorage.FlutterSecureStorage +import com.it_nomads.fluttersecurestorage.FlutterSecureStorageConfig +import com.it_nomads.fluttersecurestorage.SecurePreferencesCallback +import java.security.KeyPairGenerator +import java.security.KeyStore +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import javax.crypto.Cipher +import javax.crypto.KeyGenerator +import org.junit.After +import org.junit.Assert.* +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class SecureStorageMigrationTest { + private val base = InstrumentationRegistry.getInstrumentation().targetContext + private val fixtureId = "secure_storage_migration_${System.nanoTime()}" + private val preferenceNames = mutableSetOf() + private val context = object : ContextWrapper(base) { + override fun getApplicationContext(): Context = this + override fun getPackageName(): String = "${base.packageName}.$fixtureId" + override fun getSharedPreferences(name: String, mode: Int): SharedPreferences { + val isolatedName = "$fixtureId.$name" + preferenceNames.add(isolatedName) + return base.getSharedPreferences(isolatedName, mode) + } + } + private val config = FlutterSecureStorageConfig(mapOf( + "resetOnError" to "false", + "migrateOnAlgorithmChange" to "true", + "migrateWithBackup" to "true", + "keyCipherAlgorithm" to "RSA_ECB_OAEPwithSHA_256andMGF1Padding", + "storageCipherAlgorithm" to "AES_GCM_NoPadding", + )) + private val values = mapOf( + "extension_storage_master_key_v2" to Base64.encodeToString(ByteArray(32) { 7 }, Base64.NO_WRAP), + "example_network_password" to "example-password", + "example_session_token" to "example-token", + ) + + @After + fun cleanup() { + preferenceNames.forEach { base.deleteSharedPreferences(it) } + val keys = KeyStore.getInstance("AndroidKeyStore").apply { load(null) } + keys.aliases().toList().filter { it.startsWith("${context.packageName}.") } + .forEach { keys.deleteEntry(it) } + } + + @Test + fun migratesUnmarkedLegacyCbcDataWithoutRotatingTheMasterKey() { + seedLegacy(gcm = false, marked = false) + assertReadableAfterRestart() + } + + @Test + fun migratesLegacyAlgorithmMarkersFromTheDataPreferences() { + seedLegacy(gcm = false, marked = true) + assertReadableAfterRestart() + } + + @Test + fun migratesPkcs1WrappedGcmDataInsteadOfReportingInvalidKey() { + seedLegacy(gcm = true, marked = true) + assertReadableAfterRestart() + } + + @Test + fun readsTheLegacyV9GcmWrappedKeyName() { + seedLegacy(gcm = true, marked = true, legacyGcmKey = true) + assertReadableAfterRestart() + } + + @Test + fun currentCipherDataRemainsReadableAfterReinitialization() { + val storage = openStorage() + values.forEach { (key, value) -> callStorage(storage, "write", storage.addPrefixToKey(key), value) } + assertReadableAfterRestart() + } + + @Test + fun unrecoverableWrappedKeyDoesNotEraseEncryptedValues() { + seedLegacy(gcm = false, marked = false) + val data = context.getSharedPreferences("FlutterSecureStorage", Context.MODE_PRIVATE) + val before = data.all.toMap() + context.getSharedPreferences("FlutterSecureKeyStorage", Context.MODE_PRIVATE).edit() + .putString(CBC_KEY, Base64.encodeToString(byteArrayOf(1, 2, 3), Base64.NO_WRAP)) + .commit() + assertNotNull(initialize(FlutterSecureStorage(context))) + before.forEach { (key, value) -> + assertEquals(value, data.getString(key, null)) + assertEquals(value, data.getString("${key}_BACKUP", null)) + } + } + + private fun assertReadableAfterRestart() { + repeat(2) { + val storage = openStorage() + values.forEach { (key, value) -> assertEquals(value, callStorage(storage, "read", storage.addPrefixToKey(key))) } + } + } + + private fun openStorage(): FlutterSecureStorage = FlutterSecureStorage(context).also { + initialize(it)?.let { error -> throw AssertionError("Secure storage initialization failed", error) } + } + + private fun initialize(storage: FlutterSecureStorage): Exception? { + val complete = CountDownLatch(1) + val failure = AtomicReference() + storage.initialize(config, object : SecurePreferencesCallback { + override fun onSuccess(value: Void?) { complete.countDown() } + override fun onError(error: Exception) { failure.set(error); complete.countDown() } + }) + assertTrue("Secure storage callback timed out", complete.await(10, TimeUnit.SECONDS)) + return failure.get() + } + + // v11 added callbacks to read/write. Exercise either API so the same test + // can demonstrate the regression before changing the dependency version. + @Suppress("UNCHECKED_CAST") + private fun callStorage(storage: FlutterSecureStorage, name: String, vararg args: String): T? { + val method = storage.javaClass.methods.single { it.name == name } + if (method.parameterCount == args.size) return method.invoke(storage, *args) as T? + val complete = CountDownLatch(1) + val result = AtomicReference() + val failure = AtomicReference() + method.invoke(storage, *args, object : SecurePreferencesCallback { + override fun onSuccess(value: T?) { result.set(value); complete.countDown() } + override fun onError(error: Exception) { failure.set(error); complete.countDown() } + }) + assertTrue("Secure storage operation timed out", complete.await(10, TimeUnit.SECONDS)) + failure.get()?.let { throw it } + return result.get() + } + + // Build old ciphertext independently of the plugin so removing its legacy + // readers makes this regression fail instead of changing the fixture too. + private fun seedLegacy(gcm: Boolean, marked: Boolean, legacyGcmKey: Boolean = false) { + val generator = KeyPairGenerator.getInstance("RSA", "AndroidKeyStore") + generator.initialize(KeyGenParameterSpec.Builder( + "${context.packageName}.FlutterSecureStoragePluginKey", + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, + ).setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1) + .setDigests(KeyProperties.DIGEST_SHA256).build()) + val pair = generator.generateKeyPair() + val aes = KeyGenerator.getInstance("AES").apply { init(128) }.generateKey() + val wrapper = Cipher.getInstance("RSA/ECB/PKCS1Padding") + wrapper.init(Cipher.WRAP_MODE, pair.public) + val keyName = when { + legacyGcmKey -> "VGhpcyBpcyB0aGUga2V5IGZvcihBIHNlY3XyZZBzdG9yYWdlIEFFUyBLZXkK" + gcm -> "AES$CBC_KEY" + else -> CBC_KEY + } + context.getSharedPreferences("FlutterSecureKeyStorage", Context.MODE_PRIVATE).edit() + .putString(keyName, Base64.encodeToString(wrapper.wrap(aes), Base64.DEFAULT)).commit() + val data = context.getSharedPreferences("FlutterSecureStorage", Context.MODE_PRIVATE).edit() + values.forEach { (key, value) -> + val cipher = Cipher.getInstance(if (gcm) "AES/GCM/NoPadding" else "AES/CBC/PKCS7Padding") + cipher.init(Cipher.ENCRYPT_MODE, aes) + val encrypted = cipher.iv + cipher.doFinal(value.toByteArray(Charsets.UTF_8)) + data.putString("${config.sharedPreferencesKeyPrefix}_$key", Base64.encodeToString(encrypted, Base64.DEFAULT)) + } + if (marked) { + data.putString("FlutterSecureSAlgorithmKey", "RSA_ECB_PKCS1Padding") + data.putString("FlutterSecureSAlgorithmStorage", if (gcm) "AES_GCM_NoPadding" else "AES_CBC_PKCS7Padding") + } + assertTrue(data.commit()) + } + + companion object { + private const val CBC_KEY = "VGhpcyBpcyB0aGUga2V5IGZvciBhIHNlY3VyZSBzdG9yYWdlIEFFUyBLZXkK" + } +} diff --git a/lib/services/secure_storage_options.dart b/lib/services/secure_storage_options.dart index dc05276d..78d7836a 100644 --- a/lib/services/secure_storage_options.dart +++ b/lib/services/secure_storage_options.dart @@ -4,5 +4,8 @@ import 'package:flutter_secure_storage/flutter_secure_storage.dart'; // migration in any client must not erase the extension master key stored there. const secureStorageAndroidOptions = AndroidOptions( resetOnError: false, + migrateOnAlgorithmChange: true, migrateWithBackup: true, + keyCipherAlgorithm: KeyCipherAlgorithm.RSA_ECB_OAEPwithSHA_256andMGF1Padding, + storageCipherAlgorithm: StorageCipherAlgorithm.AES_GCM_NoPadding, ); diff --git a/pubspec.lock b/pubspec.lock index 35df9c65..d3c1ab6b 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -593,12 +593,12 @@ packages: dependency: "direct main" description: name: flutter_secure_storage - sha256: d4e1fb6b2cb524868929e78dc0282fa000554b22060fb53789dc481c9fc95bb8 + sha256: fe638107c5f69119156ada2db5a57734385fac3f64430bd7252a00d3ead2ca4b url: "https://pub.dev" source: hosted - version: "11.2.0" + version: "10.3.4" flutter_secure_storage_darwin: - dependency: transitive + dependency: "direct overridden" description: name: flutter_secure_storage_darwin sha256: a031ceac9b070e62ef183cd7f7d1a8798ebde402d5049f0fe6eb3a51bd797a21 diff --git a/pubspec.yaml b/pubspec.yaml index ed42648c..521f4406 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -24,7 +24,9 @@ dependencies: # Storage & Persistence shared_preferences: ^2.5.6 - flutter_secure_storage: ^11.0.0 + # v11 removes legacy Android readers, breaking users who skip the v10 migration. + # Keep the compatibility release until old encrypted installs can be migrated. + flutter_secure_storage: 10.3.4 spotiflac_discord: path: third_party/spotiflac_discord path_provider: ^2.1.6 @@ -89,6 +91,9 @@ dependencies: audio_session: ^0.2.4 dependency_overrides: + # Retain the existing Apple keychain fixes while Android uses legacy migration. + # The v10 and v11 Apple option maps use the same platform interface contract. + flutter_secure_storage_darwin: 0.4.3 # Per-player audio processing tap; upstream playback and session ownership stay intact. audioplayers_darwin: path: third_party/audioplayers_darwin diff --git a/test/extension_storage_service_test.dart b/test/extension_storage_service_test.dart index 5fc1b424..d9144f2d 100644 --- a/test/extension_storage_service_test.dart +++ b/test/extension_storage_service_test.dart @@ -34,7 +34,13 @@ void main() { final args = Map.from(call.arguments as Map); final options = Map.from(args['options'] as Map); expect(options['resetOnError'], 'false'); + expect(options['migrateOnAlgorithmChange'], 'true'); expect(options['migrateWithBackup'], 'true'); + expect( + options['keyCipherAlgorithm'], + 'RSA_ECB_OAEPwithSHA_256andMGF1Padding', + ); + expect(options['storageCipherAlgorithm'], 'AES_GCM_NoPadding'); if (call.method == 'read') { if (readError != null) throw readError!; return values[args['key']]; @@ -143,6 +149,36 @@ void main() { }, ); + test( + 'failed cipher migration preserves encrypted files and the key', + () async { + final key = base64Encode(List.filled(32, 4)); + values[keyName] = key; + final file = await seed( + 'support/extension_data/example.provider/settings.enc', + [8, 9, 10], + ); + readError = PlatformException( + code: 'Exception encountered', + message: + 'Migration failed after algorithm change ' + '(Invalid key, key type incompatible with cipher). ' + 'Enable resetOnError=true or call deleteAll().', + ); + await expectLater( + ExtensionStorageService.prepare(), + throwsA(isA()), + ); + expect(mutations, isEmpty); + expect(values[keyName], key); + expect(await file.readAsBytes(), [8, 9, 10]); + + readError = null; + expect((await ExtensionStorageService.prepare()).masterKey, key); + expect(mutations, isEmpty); + }, + ); + for (final legacy in [false, true]) { test( 'fresh installs and plaintext legacy settings can create a key (legacy=$legacy)',