Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Issue 372 keep pass phrase in memory #1249

Merged
merged 38 commits into from
May 27, 2021
Merged

Issue 372 keep pass phrase in memory #1249

merged 38 commits into from
May 27, 2021

Conversation

DenBond7
Copy link
Collaborator

@DenBond7 DenBond7 commented May 25, 2021 •
edited
Loading

This PR adds a base realization of "keep pass phrase in memory" support
close #372 //

Tests (delete all except exactly one):

  • Tests will be added later. As it is a base realization issues for tests will be added later

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities

DenBond7 added 30 commits May 6, 2021 21:37
@DenBond7
Added PassPhrasesInRAMService. Refactored code.| #372
fbe8635
@DenBond7
Dropped using 'longId'. Migrated to use only 'fingerprint'. Updated t…
015b392 
…he database version to 25. Refactored code.| #372
@DenBond7
Fixed bugs in tests.| #372
01663ed
@DenBond7
Modified KeysStorage. Moved to use Passphrase instead of String where…
ce1da5d 
… it is possible. Refactored code.| #372
@DenBond7
Moved NodeKeyDetails to 'security.model'.| #372
6d9e714
@DenBond7
Renamed KeyDetails.Type to KeyDetails.SourceType.| #372
efe23bf
@DenBond7
Renamed KeyDetails to KeyImportDetails.| #372
cb0de1f
@DenBond7
Renamed NodeKeyDetails to PgpKeyDetails.| #372
a3dc863
@DenBond7
Refactored code.| #372
cc38ae8
@DenBond7
Fixed refactoring errors.| #372
c0ea860
@DenBond7
Refactored code.| #372
464a966
@DenBond7
Refactored code.| #372
62ae248
@DenBond7
Fixed PrivateKeysManager.| #372
24ce45d
@DenBond7
Refactored code.| #372
35a917f
@DenBond7
Replaced KeyEntity with PgpKeyDetails where it is possible.| #372
9755c83
@DenBond7
Modified the database schema to store different passphrase types.| #372
138f242
@DenBond7
Modified KeysDao to prevent saving passphrase if passphraseType == Ke…
abcfc6c 
…yEntity.PassphraseType.RAM.| #372
@DenBond7
Modified UI in CheckKeysActivity. Added some logic to CheckPrivateKey…
7cfd151 
…sViewModel. Refactored code.| #372
@DenBond7
Merge branch 'master' into issue_372_keep_pass_phrase_in_memory
3cc081b 
# Conflicts:
#	FlowCrypt/src/main/java/com/flowcrypt/email/jetpack/viewmodel/CheckPrivateKeysViewModel.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/jetpack/viewmodel/PrivateKeysViewModel.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/security/KeysStorageImpl.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/security/SecurityUtils.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpKey.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/service/actionqueue/actions/BackupPrivateKeyToInboxAction.kt
#	FlowCrypt/src/main/java/com/flowcrypt/email/service/attachment/AttachmentDownloadManagerService.kt
@DenBond7
Fixed conflicts after merge.| #372
8c8fcdc
@DenBond7
Modified PrivateKeysViewModel to be able to store passphrase in RAM. …
91d9820 
…Refactored code in KeysStorageImpl.| #372
@DenBond7
Added auto manage of PassPhrasesInRAMService.| #372
0ea427a
@DenBond7
Improved KeysStorageImpl.updatePassPhrasesCache().| #372
795e3a0
@DenBond7
Modified PrivateKeyDetailsFragment to be able to update(set or erase)…
0ed46ee 
… a pass phrase for passphraseType == KeyEntity.PassphraseType.RAM.| #372
@DenBond7
Updated a notification in PassPhrasesInRAMService.| #372
ab88dd9
@DenBond7
Fixed a bug in CheckKeysActivity. Refactored code.| #372
f07a3a0
@DenBond7
Fixed the database migration.| #372
ec23742
@DenBond7
Fixed a bug in KeysStorageImpl.getPGPSecretKeyRingByFingerprint().| #372
804ab80
@DenBond7
Added some logic to KeysStorageImpl.| #372
4dedb27
@DenBond7
Fixed a bug in AttachmentDownloadManagerService.| #372
2401ad6
@tomholub
Copy link
Collaborator

Instead of Pass phrase was erased please say Pass phrase purged from memory

@tomholub
Copy link
Collaborator

The node key caching is causing confusion indeed - just purged pass phrase from memory, but can still decrypt. Since it will take some time before we can completely drop node (while Ivan works on re-implementing parsing messages), we should consider keeping this feature enabled only on debug builds for now.

@DenBond7
Copy link
Collaborator Author

The node key caching is causing confusion indeed - just purged pass phrase from memory, but can still decrypt. Since it will take some time before we can completely drop node (while Ivan works on re-implementing parsing messages), we should consider keeping this feature enabled only on debug builds for now.

Yup. I totally agree. It confuses me too.

@tomholub
Copy link
Collaborator

Is it possible to restart the node process? That would clear the cache. Otherwise I'd need to add a call for purging cache. Maybe actually there already is one.

@tomholub
Copy link
Collaborator

Unfortunately, I didn't expose this through an endpoint earlier.

@tomholub
Copy link
Collaborator

Otherwise it works as expected 👍

@tomholub
Copy link
Collaborator

(except the missing UI functionality to re-enter pass phrase in various places)

@DenBond7
Copy link
Collaborator Author

Is it possible to restart the node process? That would clear the cache. Otherwise I'd need to add a call for purging cache. Maybe actually there already is one.

I'm not sure that restart it's a good idea. That process is not fast. On slow devices it can take up to 5-7 seconds

@tomholub
Copy link
Collaborator

That may be ok if it only happens when I click "forget pass phrase" - a rare thing to do

@tomholub
Copy link
Collaborator

It would allow us to start testing this new functionality with users already now before node is dropped

@DenBond7
Copy link
Collaborator Author

(except the missing UI functionality to re-enter pass phrase in various places)

Yup. I've created a few issues where we can discuss that. Too many changes need to make for that.

@DenBond7
Copy link
Collaborator Author

It would allow us to start testing this new functionality with users already now before node is dropped

Got it. Please let me think about that. I'll create a separate issue.

@DenBond7 DenBond7 mentioned this pull request May 26, 2021
Closed
@DenBond7 DenBond7 marked this pull request as ready for review May 27, 2021 07:34
@DenBond7 DenBond7 requested a review from tomholub May 27, 2021 07:34
@DenBond7
Copy link
Collaborator Author

it's ready for a review

@DenBond7 DenBond7 merged commit 4980e51 into master May 27, 2021
@DenBond7 DenBond7 deleted the issue_372_keep_pass_phrase_in_memory branch May 27, 2021 08:08
@tomholub tomholub mentioned this pull request May 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomholub tomholub tomholub approved these changes

@IvanPizhenko IvanPizhenko Awaiting requested review from IvanPizhenko

@seisvelas seisvelas Awaiting requested review from seisvelas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

option to only keep pass phrase in memory
2 participants
@DenBond7 @tomholub