Skip to content

Commit 6a90c7c

Browse files
ethicnologyethicnology
committed
refactor(storage): skip FSS hybrid on non-Android + harden -25308 check
Addresses i5hi's review on PR #2112: 1. Gate the FSS9/FSS10 hybrid behind Platform.isAndroid. The fallback chain (case null: probe fss10 → readAll() → catch → try fss9) exists solely to recover 6.5.2 Android users whose wallet data sits in Jetpack Security's EncryptedSharedPreferences (ESP, Tink- backed). ESP is androidx.security.crypto — Android-only. iOS, macOS, Linux, Windows, and web have no ESP cohort. On every non-Android platform, fss9 and fss10 hit the same OS-native secure store with byte-identical wire semantics (e.g. iOS: both call SecItemAdd / SecItemCopyMatching with the same service id + key + accessibility class). Items written by either version are mutually readable, so the fallback chain is dead code there. That dead code was the only eager keychain read left in DI bootstrap. Removing it on non-Android eliminates -25308 from pre-warmed iOS launches structurally — not via after-the-fact exception mapping. Non-Android users with a stale `fss9` cohort flag (practically unreachable since both plugins query the same backing store and would have either both succeeded or both failed) are silently transitioned to `fss10` so the cohort flag stops surfacing the Android-specific "legacy storage" UI warning (wallet_bloc.dart / home_errors.dart) on what is actually a current iOS install. 2. Harden -25308 detection. flutter_secure_storage has historically shifted the OSStatus between `details` / `code` / `message` across versions. Pinning to a single field means a future fork bump could silently regress every keychain mapping. Added `_isLocked(e)` in both impls to match all three.
1 parent 1241a6a commit 6a90c7c

3 files changed

Lines changed: 199 additions & 134 deletions

File tree

‎lib/core/storage/data/datasources/key_value_storage/impl/secure_storage_data_source_impl.dart‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@ class SecureStorageDatasourceImpl implements KeyValueStorageDatasource<String> {
3535
try {
3636
return await body();
3737
} on PlatformException catch (e) {
38-
if (e.details == _errSecInteractionNotAllowed) {
38+
// Belt-and-suspenders: across `flutter_secure_storage` releases,
39+
// the OSStatus has historically appeared in `details` (current
40+
// fork), `code` (older versions, as a string), or embedded in
41+
// `message`. Match all three so a future fork bump that shifts
42+
// the field doesn't silently regress this whole class of
43+
// handling without a compile error.
44+
if (_isLocked(e)) {
3945
final target = key != null ? ' "$key"' : '';
4046
log.warning(
4147
'Device not unlocked since boot (${operation.name}$target)',
@@ -46,6 +52,11 @@ class SecureStorageDatasourceImpl implements KeyValueStorageDatasource<String> {
4652
}
4753
}
4854

55+
bool _isLocked(PlatformException e) =>
56+
e.details == _errSecInteractionNotAllowed ||
57+
e.code == '$_errSecInteractionNotAllowed' ||
58+
(e.message ?? '').contains('$_errSecInteractionNotAllowed');
59+
4960
@override
5061
Future<void> saveValue({required String key, required String value}) {
5162
return _wrap(

‎lib/core/storage/data/datasources/key_value_storage/impl/secure_storage_legacy_datasource_impl.dart‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ class SecureStorageLegacyDatasourceImpl
3131
try {
3232
return await body();
3333
} on PlatformException catch (e) {
34-
if (e.details == _errSecInteractionNotAllowed) {
34+
// See note in `secure_storage_data_source_impl.dart` `_isLocked`
35+
// — the OSStatus field has shifted across fss releases, so we
36+
// check `details` / `code` / `message` all three.
37+
if (_isLocked(e)) {
3538
final target = key != null ? ' "$key"' : '';
3639
log.warning(
3740
'Device not unlocked since boot '
@@ -43,6 +46,11 @@ class SecureStorageLegacyDatasourceImpl
4346
}
4447
}
4548

49+
bool _isLocked(PlatformException e) =>
50+
e.details == _errSecInteractionNotAllowed ||
51+
e.code == '$_errSecInteractionNotAllowed' ||
52+
(e.message ?? '').contains('$_errSecInteractionNotAllowed');
53+
4654
@override
4755
Future<void> saveValue({required String key, required String value}) {
4856
return _wrap(

‎lib/core/storage/storage_locator.dart‎

Lines changed: 178 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -41,155 +41,201 @@ class StorageLocator {
4141

4242
late final KeyValueStorageDatasource<String> secureStorageDatasource;
4343

44-
switch (existingLibrary) {
45-
case null:
46-
// No flag — fresh install, 6.5.2 user, or first run of 6.10.0.
47-
// Try FSS10 first with migrateOnAlgorithmChange:false. A 6.5.2 user
48-
// with ESP data will hit the explicit error branch in
49-
// FlutterSecureStorage.java that throws "EncryptedSharedPreferences
50-
// data found but migration is disabled" — caught here and routed to
51-
// FSS9 which can still read ESP data via encryptedSharedPreferences:true.
52-
log.fine('StorageLocator: no existing flag — attempting fss10 init');
53-
try {
54-
final storage = fss10.FlutterSecureStorage(
55-
aOptions: const fss10.AndroidOptions(
56-
// Never auto-delete data on errors. v10 default is true.
57-
resetOnError: false,
58-
// Never run any migration. With the fork's StorageCipherFactory
59-
// patch, this also makes fresh installs initialize the cipher
60-
// cleanly (saved=current when no markers exist). 6.5.2 ESP
61-
// users hit the line-195 error branch and get caught into the
62-
// FSS9 fallback below.
63-
migrateOnAlgorithmChange: false,
64-
),
65-
iOptions: const fss10.IOSOptions(
66-
accessibility:
67-
fss10.KeychainAccessibility.first_unlock_this_device,
68-
),
69-
);
70-
// Trigger native init by reading. The constructor alone never
71-
// throws — failures only surface on data access.
72-
final data = await storage.readAll();
73-
log.fine(
74-
'StorageLocator: fss10 readAll returned ${data.length} entries',
75-
);
76-
77-
// Belt-and-suspenders: if FSS10 returns empty but the SQLite DB
78-
// from a prior install exists, treat it as a silent failure and
79-
// route to FSS9.
80-
if (data.isEmpty) {
81-
final docsDir = await getApplicationDocumentsDirectory();
82-
final dbFile = File(
83-
p.join(docsDir.path, 'bullbitcoin_sqlite.sqlite'),
84-
);
85-
if (await dbFile.exists()) {
86-
log.warning(
87-
'StorageLocator: fss10 readAll returned empty but database '
88-
'exists — silent failure detected, falling back to fss9',
89-
);
90-
throw Exception(
91-
'FSS10 silent failure: prior install data exists '
92-
'but readAll returned 0 entries',
93-
);
94-
}
95-
log.fine(
96-
'StorageLocator: readAll empty + no database = fresh install',
97-
);
98-
}
99-
100-
secureStorageDatasource = SecureStorageDatasourceImpl(storage);
101-
// Only commit the flag AFTER readAll confirms FSS10 works.
102-
await seedStoreTypeDatasource.write(
103-
SeedStoreTypeModel.fromEntity(
104-
const SeedStoreType(storageLibrary: SeedStorageLibrary.fss10),
105-
),
106-
);
107-
log.fine('StorageLocator: fss10 verified and flag written');
108-
} catch (fss10Error) {
109-
log.warning(
110-
'StorageLocator: fss10 readAll failed — falling back to fss9. '
111-
'Error: ${fss10Error.runtimeType}: $fss10Error',
112-
error: fss10Error,
113-
);
114-
44+
if (Platform.isAndroid) {
45+
// The FSS9/FSS10 hybrid fallback chain below exists solely to
46+
// recover 6.5.2 Android users whose wallet data lives in Jetpack
47+
// Security's EncryptedSharedPreferences (ESP, Tink-backed). ESP
48+
// is `androidx.security.crypto` — Android-only. The whole probe
49+
// + fallback dance, including the eager `storage.readAll()` call
50+
// in the `case null:` branch, exists for that one cohort.
51+
//
52+
// On every other platform (the `else` branch below) we skip the
53+
// hybrid entirely: fss9 and fss10 route to the same OS-native
54+
// secure store with byte-identical wire semantics (e.g. iOS:
55+
// both hit Keychain via `SecItemAdd` / `SecItemCopyMatching`
56+
// with the same service id + key + accessibility class), so the
57+
// fallback serves no purpose there and the eager keychain read
58+
// is what causes the pre-warm `-25308` failure class.
59+
switch (existingLibrary) {
60+
case null:
61+
// No flag — fresh install, 6.5.2 user, or first run of 6.10.0.
62+
// Try FSS10 first with migrateOnAlgorithmChange:false. A 6.5.2 user
63+
// with ESP data will hit the explicit error branch in
64+
// FlutterSecureStorage.java that throws "EncryptedSharedPreferences
65+
// data found but migration is disabled" — caught here and routed to
66+
// FSS9 which can still read ESP data via encryptedSharedPreferences:true.
67+
log.fine('StorageLocator: no existing flag — attempting fss10 init');
11568
try {
116-
final storage = fss9.FlutterSecureStorage(
117-
aOptions: const fss9.AndroidOptions(
69+
final storage = fss10.FlutterSecureStorage(
70+
aOptions: const fss10.AndroidOptions(
71+
// Never auto-delete data on errors. v10 default is true.
11872
resetOnError: false,
119-
// ESP path used by 6.5.2 — same MasterKey alias and Tink
120-
// scheme, so existing data decrypts cleanly.
121-
encryptedSharedPreferences: true,
73+
// Never run any migration. With the fork's StorageCipherFactory
74+
// patch, this also makes fresh installs initialize the cipher
75+
// cleanly (saved=current when no markers exist). 6.5.2 ESP
76+
// users hit the line-195 error branch and get caught into the
77+
// FSS9 fallback below.
78+
migrateOnAlgorithmChange: false,
12279
),
123-
iOptions: const fss9.IOSOptions(
80+
iOptions: const fss10.IOSOptions(
12481
accessibility:
125-
fss9.KeychainAccessibility.first_unlock_this_device,
82+
fss10.KeychainAccessibility.first_unlock_this_device,
12683
),
12784
);
85+
// Trigger native init by reading. The constructor alone never
86+
// throws — failures only surface on data access.
12887
final data = await storage.readAll();
12988
log.fine(
130-
'StorageLocator: fss9 readAll returned ${data.length} entries',
131-
);
132-
secureStorageDatasource = SecureStorageLegacyDatasourceImpl(
133-
storage,
89+
'StorageLocator: fss10 readAll returned ${data.length} entries',
13490
);
91+
92+
// Belt-and-suspenders: if FSS10 returns empty but the SQLite DB
93+
// from a prior install exists, treat it as a silent failure and
94+
// route to FSS9.
95+
if (data.isEmpty) {
96+
final docsDir = await getApplicationDocumentsDirectory();
97+
final dbFile = File(
98+
p.join(docsDir.path, 'bullbitcoin_sqlite.sqlite'),
99+
);
100+
if (await dbFile.exists()) {
101+
log.warning(
102+
'StorageLocator: fss10 readAll returned empty but database '
103+
'exists — silent failure detected, falling back to fss9',
104+
);
105+
throw Exception(
106+
'FSS10 silent failure: prior install data exists '
107+
'but readAll returned 0 entries',
108+
);
109+
}
110+
log.fine(
111+
'StorageLocator: readAll empty + no database = fresh install',
112+
);
113+
}
114+
115+
secureStorageDatasource = SecureStorageDatasourceImpl(storage);
116+
// Only commit the flag AFTER readAll confirms FSS10 works.
135117
await seedStoreTypeDatasource.write(
136118
SeedStoreTypeModel.fromEntity(
137-
const SeedStoreType(storageLibrary: SeedStorageLibrary.fss9),
119+
const SeedStoreType(storageLibrary: SeedStorageLibrary.fss10),
138120
),
139121
);
140-
log.fine('StorageLocator: fss9 fallback verified and flag written');
141-
} catch (fss9Error) {
142-
log.severe(
143-
message:
144-
'StorageLocator: both fss10 and fss9 failed. '
145-
'fss10: ${fss10Error.runtimeType}, '
146-
'fss9: ${fss9Error.runtimeType}',
147-
error: fss9Error,
148-
trace: StackTrace.current,
149-
category: ReportCategory.migration,
122+
log.fine('StorageLocator: fss10 verified and flag written');
123+
} catch (fss10Error) {
124+
log.warning(
125+
'StorageLocator: fss10 readAll failed — falling back to fss9. '
126+
'Error: ${fss10Error.runtimeType}: $fss10Error',
127+
error: fss10Error,
150128
);
151-
rethrow;
129+
130+
try {
131+
final storage = fss9.FlutterSecureStorage(
132+
aOptions: const fss9.AndroidOptions(
133+
resetOnError: false,
134+
// ESP path used by 6.5.2 — same MasterKey alias and Tink
135+
// scheme, so existing data decrypts cleanly.
136+
encryptedSharedPreferences: true,
137+
),
138+
iOptions: const fss9.IOSOptions(
139+
accessibility:
140+
fss9.KeychainAccessibility.first_unlock_this_device,
141+
),
142+
);
143+
final data = await storage.readAll();
144+
log.fine(
145+
'StorageLocator: fss9 readAll returned ${data.length} entries',
146+
);
147+
secureStorageDatasource = SecureStorageLegacyDatasourceImpl(
148+
storage,
149+
);
150+
await seedStoreTypeDatasource.write(
151+
SeedStoreTypeModel.fromEntity(
152+
const SeedStoreType(storageLibrary: SeedStorageLibrary.fss9),
153+
),
154+
);
155+
log.fine(
156+
'StorageLocator: fss9 fallback verified and flag written',
157+
);
158+
} catch (fss9Error) {
159+
log.severe(
160+
message:
161+
'StorageLocator: both fss10 and fss9 failed. '
162+
'fss10: ${fss10Error.runtimeType}, '
163+
'fss9: ${fss9Error.runtimeType}',
164+
error: fss9Error,
165+
trace: StackTrace.current,
166+
category: ReportCategory.migration,
167+
);
168+
rethrow;
169+
}
152170
}
153-
}
154-
155-
case SeedStorageLibrary.fss9:
156-
// Previous session committed to FSS9 (6.5.2 ESP user). Stay on FSS9
157-
// with the same options that succeeded then. The app shows a
158-
// LegacyStorageWarningOverlay prompting backup + reinstall.
159-
log.fine(
160-
'StorageLocator: existing flag is fss9 — using legacy storage',
161-
);
162-
final storage = fss9.FlutterSecureStorage(
163-
aOptions: const fss9.AndroidOptions(
164-
resetOnError: false,
165-
encryptedSharedPreferences: true,
166-
),
167-
iOptions: const fss9.IOSOptions(
168-
accessibility: fss9.KeychainAccessibility.first_unlock_this_device,
169-
),
170-
);
171-
secureStorageDatasource = SecureStorageLegacyDatasourceImpl(storage);
172-
log.fine('StorageLocator: fss9 legacy storage initialized from flag');
173-
174-
case SeedStorageLibrary.fss10:
175-
// Previous session committed to FSS10 (cohort A, prior fresh
176-
// install, or a 6.5.2 user that fell through to FSS10 after the
177-
// dance — this last case shouldn't happen since fss9-flag is
178-
// written for that path).
179-
log.fine(
180-
'StorageLocator: existing flag is fss10 — using current storage',
181-
);
182-
final storage = fss10.FlutterSecureStorage(
183-
aOptions: const fss10.AndroidOptions(
184-
resetOnError: false,
185-
migrateOnAlgorithmChange: false,
186-
),
187-
iOptions: const fss10.IOSOptions(
188-
accessibility: fss10.KeychainAccessibility.first_unlock_this_device,
171+
172+
case SeedStorageLibrary.fss9:
173+
// Previous session committed to FSS9 (6.5.2 ESP user). Stay on FSS9
174+
// with the same options that succeeded then. The app shows a
175+
// LegacyStorageWarningOverlay prompting backup + reinstall.
176+
log.fine(
177+
'StorageLocator: existing flag is fss9 — using legacy storage',
178+
);
179+
final storage = fss9.FlutterSecureStorage(
180+
aOptions: const fss9.AndroidOptions(
181+
resetOnError: false,
182+
encryptedSharedPreferences: true,
183+
),
184+
iOptions: const fss9.IOSOptions(
185+
accessibility:
186+
fss9.KeychainAccessibility.first_unlock_this_device,
187+
),
188+
);
189+
secureStorageDatasource = SecureStorageLegacyDatasourceImpl(storage);
190+
log.fine('StorageLocator: fss9 legacy storage initialized from flag');
191+
192+
case SeedStorageLibrary.fss10:
193+
// Previous session committed to FSS10 (cohort A, prior fresh
194+
// install, or a 6.5.2 user that fell through to FSS10 after the
195+
// dance — this last case shouldn't happen since fss9-flag is
196+
// written for that path).
197+
log.fine(
198+
'StorageLocator: existing flag is fss10 — using current storage',
199+
);
200+
final storage = fss10.FlutterSecureStorage(
201+
aOptions: const fss10.AndroidOptions(
202+
resetOnError: false,
203+
migrateOnAlgorithmChange: false,
204+
),
205+
iOptions: const fss10.IOSOptions(
206+
accessibility:
207+
fss10.KeychainAccessibility.first_unlock_this_device,
208+
),
209+
);
210+
secureStorageDatasource = SecureStorageDatasourceImpl(storage);
211+
log.fine('StorageLocator: fss10 storage initialized from flag');
212+
}
213+
} else {
214+
// Non-Android: fss10 direct, no probe, no fallback. See the
215+
// `if (Platform.isAndroid)` comment above for the rationale.
216+
// Normalize any stray `fss9` flag (practically unreachable here
217+
// but cheap to enforce) so the cohort-based "legacy storage" UI
218+
// warning in `wallet_bloc.dart` / `home_errors.dart` doesn't
219+
// surface for current iOS installs.
220+
log.fine('StorageLocator: non-Android — fss10 direct, no probe');
221+
final storage = fss10.FlutterSecureStorage(
222+
aOptions: const fss10.AndroidOptions(
223+
resetOnError: false,
224+
migrateOnAlgorithmChange: false,
225+
),
226+
iOptions: const fss10.IOSOptions(
227+
accessibility: fss10.KeychainAccessibility.first_unlock_this_device,
228+
),
229+
);
230+
secureStorageDatasource = SecureStorageDatasourceImpl(storage);
231+
232+
if (existingLibrary != SeedStorageLibrary.fss10) {
233+
await seedStoreTypeDatasource.write(
234+
SeedStoreTypeModel.fromEntity(
235+
const SeedStoreType(storageLibrary: SeedStorageLibrary.fss10),
189236
),
190237
);
191-
secureStorageDatasource = SecureStorageDatasourceImpl(storage);
192-
log.fine('StorageLocator: fss10 storage initialized from flag');
238+
}
193239
}
194240

195241
locator.registerLazySingleton<KeyValueStorageDatasource<String>>(

0 commit comments

Comments
 (0)