fix(android): make decrypt side-effect-free and return false for stale/missing keys #773
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix annoying backup/restore behavior on Android
The problem that drove us crazy
You know that moment when your app gets reinstalled and suddenly the keychain acts weird? Here's what was happening:
allowBackup="true"→RN_KEYCHAINpreferences get restored, but the actual encryption keys are gonegetAllGenericPasswordServices()suddenly shows that entry existsBasically, just trying to read an entry would change what the listing API returned. That's not supposed to happen.
What we tried first
Setting
allowBackup="false"works around this by not restoring the preferences in the first place. But lots of apps already ship with backups enabled and can't easily change that.The real fix
We made two key changes:
1. Stop creating keys during reads
2. Handle the "stale data" case gracefully
false(not found) instead of throwingfalseinstead of throwingWhy this helps everyone
falseinstead of erroring outCryptoFailedExceptionerrors, often caused by the same underlying backup/restore mismatchThe trade-off
We're now treating some decryption failures (like authentication tag mismatches) as "not found" instead of explicit errors. In 99% of cases this is what you want after a backup restore. In rare cases where you really need to know if data was corrupted vs just stale, you might miss that signal.
We added clear warning logs when this happens, and we could add a "strict mode" later if needed.
How to test
Quick test:
allowBackup="true")getAllGenericPasswordServices()→ should be empty and stay emptyfalsecleanlygetAllGenericPasswordServices()again → should still be empty (this is the key improvement!)Before this fix, step 5 would suddenly show the entry you tried to read in step 4, even though it couldn't actually be decrypted.
Manual backup simulation (for deeper testing):
You can manually simulate the backup/restore scenario using Android Studio's Device Explorer:
/data/data/com.yourapp/files/datastore/RN_KEYCHAIN.preferences_pbto your computerRN_KEYCHAIN.preferences_pbfile back to the same location (simulates backup restore - prefs restored, Keystore still empty)falsewithout mutating the KeystoreBottom line
Your keychain will now behave predictably after backup/restore, even if you can't change your backup settings. Reading entries won't have weird side effects anymore, and you'll see fewer random crypto exceptions in production.
Other inconsistency spotted (to address separately)
Note: There's still an inconsistency where
hasGenericPasswordchecks preferences (stored) whilegetAllGenericPasswordServiceschecks Keystore (decryptable). This PR reduces the confusion by preventing reads from mutating Keystore state, but we should consider fixing that too.