Add functionality for update encryption on wallet, when passphrase gets updated by user #5003
Assignees
Comments
stringhandler
pushed a commit
that referenced
this issue
Jan 18, 2023
Description --- Adds wallet password complexity feedback. Allows empty passwords. Adds a warning indicating that password changing functionality is [not yet implemented](#5003). Adds tests. Closes [issue 5101](#5101). Motivation and Context --- The only check on a wallet password is that it not be empty. This introduces two issues: - The user has no feedback on the practical strength of their password. - The user may specifically wish not to set a password for fail-available backups. This PR uses the [zxcvbn](https://crates.io/crates/zxcvbn) password complexity library to score a password and provide actionable feedback to the user. When the user enters a new password or changes their password, feedback is displayed if applicable. This is informational; the user is free to ignore the feedback if they wish. Further, the user is now allowed to set an empty password, which may be desired for backups that must fail available. A warning is displayed if this happens. Finally, a warning message is displayed during the password changing process to indicate that this functionality is incomplete. How Has This Been Tested? --- Existing CI passes. New tests pass. Tested manually for new wallets.
|
Currently, the wallet does not have the ability to change its passphrase. |
This was referenced Jan 30, 2023
stringhandler
pushed a commit
that referenced
this issue
Feb 3, 2023
Description --- Refactors database encryption to enable easier [password changes](#5003) and [PBKDF parameter updates](#5151). BREAKING CHANGE: This significantly changes how database encryption is performed, so existing databases will be rendered permanently inaccessible. Motivation and Context --- Currently, the user's passphrase is used with a PBKDF (as of now, `Argon2` with [recommended parameters](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id)) to derive the `XChaCha20-Poly1305` key used for database encryption. While this is a safe way to handle database encryption (up to the complexity of the passphrase), it is brittle. Changing the passphrase or updating the PBKDF parameters would require that all database entries be re-encrypted with a new key, which is likely to be tedious and prone to corruption. This can be [improved](#5151 (comment)) by refactoring how encryption is done. When the user wishes to set up encryption, we generate a high-entropy random `XChaCha20-Poly1305` key (the _main key_) to do the actual authenticated encryption of database entries. We then run the user's passphrase and a random salt through the PBKDF to derive another `XChaCha20-Poly1305` key (the _secondary key_), and encrypt the main key with authentication. Finally, we store the secondary key salt, the encrypted main key, and a version identifier in the database. When we need to decrypt database entries, we derive the secondary key using the salt (and parameters from the version identifier), decrypt and authenticate the main key, and use the main key for database operations. When the user decides to change their passphrase, or if we wish to update the PBKDF parameters, we simply derive a new secondary key, re-encrypt the main key with it, and store the new secondary key salt, encrypted main key, and version identifier. This operation can be done quickly and with minimal risk. This PR includes the refactoring needed to support this two-key design. However, it does not directly integrate password change or PBKDF parameter update functionality, which is deferred to future work. How Has This Been Tested? --- Existing tests pass. Manual testing is still needed. BREAKING CHANGE: This significantly changes how database encryption is performed, so existing databases will be rendered permanently inaccessible.
|
Now that PR 5154 is merged, it's possible to support this. A few things to keep in mind:
|
stringhandler
pushed a commit
that referenced
this issue
Feb 13, 2023
Description --- Adds functionality to change wallet passphrase, updating to the latest encryption version parameters at the same time. Minor refactoring of database main and secondary key operations. Removes unneeded functionality that tried to assert any cipher seed was encrypted (and [associated tests](https://github.com/tari-project/tari/blob/b7747a29c7032278b3ed88e13823d6e4fe7de45e/base_layer/wallet/src/storage/sqlite_db/wallet.rs#L771-L823)). Closes [issue 5003](#5003). Motivation and Context --- Currently, wallet passphrases cannot be changed; there is a CLI flow, but it is non-functional. This PR continues the work started in [PR 5154](#5154), which refactors wallet database encryption. When the user wishes to change their passphrase, we do the following: - Attempt to use the existing passphrase to authenticate and decrypt the encrypted database main key. If this fails, the passphrase is incorrect, so we abort. - Get the latest encryption version parameters from the hardcoded list. - Generate a fresh secondary key salt. Use it and the new passphrase to derive a new secondary key, using the fetched encryption version parameters. - Encrypt the database main key using the new secondary key. - Store the new version identifier, secondary key salt, and encrypted main key in the database, overwriting the previous values. The update operations are done in a single transaction to mitigate the (unlikely) risk of database corruption. There isn't an included test for this, but you can simulate a failure by having the transaction closure return an error, after which the existing wallet passphrase should continue to work. How Has This Been Tested? --- Existing unit tests pass. A new unit test passes. Manually tested a successful passphrase change. Manually tested a failed passphrase change. Manually tested a simulated database transaction failure.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: