-
Notifications
You must be signed in to change notification settings - Fork 1
fix: keychain persistence #319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…:synonymdev/bitkit-ios into fix/clean-keychain-persistence-uninstall
…edKeychainScenario by creating a documment marker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances keychain security by implementing AES-GCM encryption for sensitive data and adding orphaned keychain detection to prevent data recovery after app uninstall.
Changes:
- Implements AES-GCM encryption for all keychain entries using a key stored in App Group storage
- Adds Documents directory marker file for reliable detection of app reinstallation
- Enhances migration logic to handle RN-to-native transitions and orphaned keychain scenarios
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Bitkit/Utilities/KeychainCrypto.swift | New encryption/decryption utilities with AES-GCM and Documents marker management |
| Bitkit/Utilities/Keychain.swift | Updated to encrypt data before storage and decrypt after retrieval with migration support |
| Bitkit/Utilities/Errors.swift | Added failedToDecrypt error case |
| Bitkit/Utilities/AppReset.swift | Enhanced to delete encryption key and App Group UserDefaults |
| Bitkit/AppScene.swift | Added orphaned keychain detection logic with Documents marker validation |
| Bitkit/Services/MigrationsService.swift | Added RN keychain cleanup after successful migration |
| Bitkit/Services/VssBackupClient.swift | Fixed duplicate setup prevention |
| Bitkit/Constants/Env.swift | Centralized appGroupIdentifier constant |
| Bitkit/Models/ReceivedTxSheetDetails.swift | Updated to use centralized appGroupIdentifier |
| BitkitTests/KeychainCryptoTests.swift | Comprehensive tests for encryption, decryption, and key management |
| BitkitTests/KeychainTests.swift | Updated with encryption integration and migration tests |
| BitkitTests/KeychainiCloudSyncTests.swift | New tests verifying keychain items don't sync to iCloud |
| Bitkit.xcodeproj/project.pbxproj | Added KeychainCrypto.swift to project |
|
Draft to implement comments |
|
OBS:
|
Fix #293
Description
This PR prevents sensitive keychain data (mnemonics, passphrases, PINs) from being recovered after app uninstall by:
This only affects the upgrade path. Normal usage after installation and migration from RN works correctly.
Why Documents Marker?
The encryption key is stored in App Group storage, which persists after uninstall on iOS. The Documents directory is reliably deleted on uninstall, so the marker file serves as the reliable detection mechanism.
Testing Scenarios
reset-and-restore.mp4
unninstall-and-reinstall.mp4
fresh-install.mp4
migrate-from-RN-with-passphrase-and-pin.mp4