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.

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

Add versioning to PBKDF applications #5151

Closed
AaronFeickert opened this issue Jan 30, 2023 · 4 comments
Closed

Add versioning to PBKDF applications #5151

AaronFeickert opened this issue Jan 30, 2023 · 4 comments

Comments

@AaronFeickert
Copy link
Collaborator

AaronFeickert commented Jan 30, 2023

The codebase uses Argon2 as a password-based key derivation function (PBKDF). Specifically, it uses OWASP recommendations for its parameters, which are updated from time to time in order to reflect evolving threats from more advanced hardware. In fact, these parameters were recently updated.

Based on the current implementation, some future codebase updates to reflect parameter changes will be brittle, as there is currently no straightforward way to detect which parameters were used to generate any particular keys in applications not using the PHC string format. There should always be backwards compatibility for keys generated using older parameters, but users should (to the extent possible) always receive the safety and security benefits of the most recent parameters in use.

The best and most standard approach is to use versioning wherever a PBKDF is used, where a hardcoded version is stored or otherwise encoded such that the client uses the correct parameters for key derivation. It may important to bind this version to authenticated data in order to mitigate risks associated with version mismatches or downgrade attacks.

@AaronFeickert
Copy link
Collaborator Author

AaronFeickert commented Jan 30, 2023

The codebase uses Argon2 in the following places:

@AaronFeickert
Copy link
Collaborator Author

AaronFeickert commented Jan 30, 2023

Relating to this issue, we can simplify database encryption while supporting straightforward passphrase changes and upgrades.

Right now, database entries are encrypted using a key derived directly from the user's passphrase using the Argon2 PBKDF. This doesn't pose any particular security issues (aside from the ever-present risk of a weak passphrase, which we try to mitigate), but it makes it a pain to change the passphrase or upgrade the hashing parameters.

A way to make life a bit easier is to instead use a high-entropy random main key for encrypting database entries, and encrypt this key using a secondary key derived from the user's passphrase. When the user wishes to change their passphrase or upgrade to a newer version, we simply re-encrypt the main key using a newly-derived secondary key.

This has a second advantage, in that the use of authenticated encryption for this process allows for an easy check for the correct passphrase without needing to store a passphrase hash or test other database fields. It is still possible to store such a hash for versioning purposes, if this is more straightforward than storing a separate version identifier.

@CjS77
Copy link
Collaborator

CjS77 commented Jan 31, 2023

Great suggestion

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.
@AaronFeickert
Copy link
Collaborator Author

Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants