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

feat!: add key commitment to database main key AEAD #5188

Merged

Conversation

AaronFeickert
Copy link
Collaborator

@AaronFeickert AaronFeickert commented Feb 16, 2023

Description

Updates database encryption by adding key commitment to the main key authenticated encryption.

Motivation and Context

Most authenticated encryption with associated data (AEAD) constructions, including the XChaCha20-Poly1305 construction used in the codebase, do not commit to keys. This is often not problematic, but in the context of password-based encryption, it can remove certain formal guarantees of authenticity.

Recent work suggests that the use of a key-committing AEAD as part of a password-based encryption design can provide improved security against particular attacks. While these are likely entirely theoretical for Tari database encryption, it makes sense to consider feasible mitigations.

While key-committing AEADs are not standardized or widely available in libraries, one paper suggests a particularly simple design that augments an arbitrary AEAD by using a key derivation process that includes a particular hash commitment with the ciphertext.

This PR adds the hash-of-key design to the AEAD used for encryption of the database main key.

Here is a diagram showing the complete database encryption design. Rectangular nodes are secret data, rounded nodes are functions/algorithms, and cylindrical nodes are data stored in the database. Double-ended arrows indicate encryption/decryption functionality.

flowchart LR
    pbkdf([PBKDF]) --> sdk[Secondary derivation key]
    pass[Passphrase] --> pbkdf
    salt[(Salt)] --> pbkdf
    version[(Version)] --> pbkdf

    sdk --> kdfenc([KDF]) --> sk[Secondary key]
    sdk --> kdfcom([KDF]) --> kc[(Key commitment)]

    aeadmk([AEAD])
    mk[Main key] <--> aeadmk
    sk --> aeadmk
    aeadmk <--> encmk[(Encrypted main key)]

    aeadfield([AEAD])
    field[Field data] <--> aeadfield
    mk --> aeadfield
    aeadfield <--> encfield[(Encrypted field data)]
Loading

How Has This Been Tested?

Existing unit tests pass. A new unit test passes. Manually tested creating a new wallet, loading it successfully (with the correct passphrase) and unsuccessfully (with an incorrect passphrase), and performing a successful and unsuccessful (with incorrect existing passphrase, and with a mistyped new passphrase) passphrase change.

BREAKING CHANGE: This adds an encryption-related field to the database and modifies key derivation, so attempts to access existing databases will fail.

@AaronFeickert AaronFeickert force-pushed the aead-key-commitment branch 3 times, most recently from 701dc14 to a6424c5 Compare February 17, 2023 21:09
@AaronFeickert AaronFeickert marked this pull request as ready for review February 17, 2023 21:11
@AaronFeickert AaronFeickert force-pushed the aead-key-commitment branch 2 times, most recently from b8a7744 to 533ed33 Compare February 20, 2023 15:31
@stringhandler stringhandler merged commit 95bc795 into tari-project:development Feb 21, 2023
stringhandler pushed a commit that referenced this pull request Feb 21, 2023
⚠ BREAKING CHANGES

* add key commitment to database main key AEAD  (5188)

Features

* add key commitment to database main key AEAD  ([5188](#5188)) ([95bc795](95bc795))
* add more burn details to burn command ([5169](#5169)) ([e417e57](e417e57))
* print out warning if wallet grpc connections fails ([5195](#5195)) ([4e1cb38](4e1cb38))


Bug Fixes

* add missing consensus constants to get_constants grpc ([5183](#5183)) ([9900d5d](9900d5d))
@AaronFeickert AaronFeickert deleted the aead-key-commitment branch February 21, 2023 14:45
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

Successfully merging this pull request may close these issues.

2 participants