option to only keep pass phrase in memory #372
Comments
|
part of #714 |
|
@tomholub Could you share more details? I mean how do you see that. Because due to the difficult lifecycle of the Android app we can have different thoughts about this issue. I need a little more info. |
|
When generate key pair in Android, there is an option to associate an authentication mecahism with it. The authentication mechanism could be device fingerprint. For this, fingerprint needs to be registered. If authentication mechanism is bound to the key, one cannot access the key without providing the fingerprint. If user deletes the fingerprint on Android device, then that particular key cannot be accessed anymore. When use Android key store provider, the key pair is generated and stored in TEE (Trusted execution environment)/StrongBox and cannot be exported. TEE is available I believe after Andoid 6. |
|
Sorry Bilal - this issue is not about user's fingerprint or TEE or pins or other similar mechanisms. While this is a related concern (how to ensure that user has acceptable UX on a phone, instead of having to always input long pass phrases), this particular issue is purely about not storing pass phrases in long-term storage. Above, Den is asking about how exactly to achieve that, which may involve some of what you mentioned, but that's not the core of the issue. For this particular issue, I'd like to see something like this: (consumers) During setup (of new OpenPGP key, or when importing key or when loading from backup - whenever I'm creating or entering pass phrase), the user should see two radio buttons:
Default is to store, just like now. If the user switches it to memory-only, then we do not store pass phrase with the key (or at all). We only keep the pass phrase in memory for up to 4 hours from the moment it was stored - then it needs to be forgotten. During those 4 hours, the key will be used for actions (eg decrypt messages). After those 4 hours, the user will be prompted for a pass phrase with a modal / alert to re-enter it, at which point it will be again remembered for 4 hours. If app gets killed, pass phrase gets forgotten. For now, there will be no way for user to switch this anywhere in settings. They would have to log out and re-setup. (enterprise users with appropriate OrgRule as described in #1168 - don't ask user, automatically only store in memory) @DenBond7 with this in mind - is this achievable purely using standard Java code? Just keep some information in background process in memory for up to 4 hours, then wipe it? |
|
@tomholub Thanks! That's what I meant.
It's ok in the Java world. But... It can be a little difficult on Android. Let me explain. We can have any data in the RAM while the app process is alive. But you can easy reproduce a situation that kills our process due to memory optimization. And it will a regular Android system behavior. For example, please run FlowCrypt then move it to the background. After that try to open 10-20 different apps. After that try to open FlowCrypt and you will that the app starts from the fresh state. It depends on different things: manufacturer, RAM and etc. I can't control that. So only the following can be true:
|
|
Can we do something like this?
When After 4 hours, clear the passpharse and store again securely after taking input from the user. |
|
@bilalashraf123 of course we can do that. We can have a task for Work manager that will check and clear temporary passphrases. @tomholub |
|
For example like Mi Fit, Google Translate
|
|
please correct me If I am wrong. The actual requirement could be not to ask for passphrase for 4 hours. It does not matter where to store the password temporarily. |
We didn't discuss this in detail on mobile apps, just a note that "eventually would be good to use fingerprint or pin to protect the pass phrase". We did discuss it on the browser extension, where we could use session storage (that browser may store to file system) that we manually empty after 4 hours, and this was rejected - saving it locally is a hazard that the customer does not want. On Android the situation is a bit different due to the abundance of options to address this further (pin/fingerprint/secure keychain and so on) but I don't want to get into it yet, because it's an implementation and design rabbit hole (plus so many types of devices, etc). |
We can store in memory as you say, and consider it expected behavior that the pass phrase may last less than 4 hours if the OS kills the process for memory management. |
This is an excellent option! That way, there could also be a button "forget now" on the notification. |
DenBond7
added
architecture
|
@tomholub By the way, what about operations like changing passphrase? As for me if a user chooses the option |
Initially, you can do that. |
|
@tomholub I'm thinking about the current issue. It isn't a simple and for now I've just added only a base functionality. And it produced a lot of changes in the code (architecture changes). I think we have to divide the current issue to a few smaller to be able to review it comfortable. What already works:
I propose too accept the current changes for this issue and open new issues for the following:
All of those cases is not handled properly now. App doesn't crash but a user will see some sort of errors. @tomholub It would be great if you test the current changes with described cases to see what exactly happens One more thing. We can't release the current changes while we don't cover all cases that I've described. For now I propose to make available use pass phrase in RAM only for the debug builds. The release builds should work as before. |
|
You can still render both ram and storage radio buttons during setup, but in production builds make the ram option grayed out. Then we can release even before we fix all of these. |
Agree. Good idea 👍 |
|
@tomholub Please share your opinion - would you like to have a sound when we show a notification or I have to show it silently?
|
|
silently please |
|
Even better if the notification didn't really "show up", and only be visible when we go to notifications |
* Added PassPhrasesInRAMService. Refactored code.| #372 * Dropped using 'longId'. Migrated to use only 'fingerprint'. Updated the database version to 25. Refactored code.| #372 * Fixed bugs in tests.| #372 * Modified KeysStorage. Moved to use Passphrase instead of String where it is possible. Refactored code.| #372 * Moved NodeKeyDetails to 'security.model'.| #372 * Renamed KeyDetails.Type to KeyDetails.SourceType.| #372 * Renamed KeyDetails to KeyImportDetails.| #372 * Renamed NodeKeyDetails to PgpKeyDetails.| #372 * Refactored code.| #372 * Fixed refactoring errors.| #372 * Refactored code.| #372 * Refactored code.| #372 * Fixed PrivateKeysManager.| #372 * Refactored code.| #372 * Replaced KeyEntity with PgpKeyDetails where it is possible.| #372 * Modified the database schema to store different passphrase types.| #372 * Modified KeysDao to prevent saving passphrase if passphraseType == KeyEntity.PassphraseType.RAM.| #372 * Modified UI in CheckKeysActivity. Added some logic to CheckPrivateKeysViewModel. Refactored code.| #372 * Fixed conflicts after merge.| #372 * Modified PrivateKeysViewModel to be able to store passphrase in RAM. Refactored code in KeysStorageImpl.| #372 * Added auto manage of PassPhrasesInRAMService.| #372 * Improved KeysStorageImpl.updatePassPhrasesCache().| #372 * Modified PrivateKeyDetailsFragment to be able to update(set or erase) a pass phrase for passphraseType == KeyEntity.PassphraseType.RAM.| #372 * Updated a notification in PassPhrasesInRAMService.| #372 * Fixed a bug in CheckKeysActivity. Refactored code.| #372 * Fixed the database migration.| #372 * Fixed a bug in KeysStorageImpl.getPGPSecretKeyRingByFingerprint().| #372 * Added some logic to KeysStorageImpl.| #372 * Fixed a bug in AttachmentDownloadManagerService.| #372 * Fixed some tests.| #372 * Fixed lint warnings.| #372 * Marked some code as deprecated.| #372 * Renamed some strings.| #372 * Disabled "Keep pass phrase in memory" for release builds.| #372 * Made notifications for PassPhrasesInRAMService silent.| #372 * Removed unused code.| #372 * Fixed method names.| #372
|
For the history. Here I've added only a base realization without error handling. |
part of #1298
The text was updated successfully, but these errors were encountered: