From 85acc2f1a06afa4e7b184e4577c2b081691783da Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 29 Aug 2022 11:40:49 +0200 Subject: [PATCH] fix!: replace AES-GCM with XChaCha20-Poly1305 (#4550) Description --- This replaces AES-GCM, which is used for encrypted storage, with XChaCha20-Poly1305. It gets rid of a superfluous MAC. It updates tests to account for more failure modes. This is an updated and cleaner version of [PR 4529](https://github.com/tari-project/tari/pull/4529) (which used ChaCha20-Poly1305) that should be easier to review. Motivation and Context --- The work in [PR 4340](https://github.com/tari-project/tari/pull/4340) binds field data into a MAC that is included with AES-GCM ciphertext. This binding is important and useful, but is done in the wrong place; AES-GCM already includes an authentication tag that is built in to the ciphertext and parsed automatically on decryption. This work adds the field binding directly into the authenticated encryption as associated data, which is the standard way to include public context. It means the existing additional MAC is no longer needed, since malleated field data will fail the authentication phase of decryption. Separately, the use of AES-GCM for authenticated encryption is suboptimal. Its nonce length is short enough that the use of random nonces can be risky if enough data is encrypted under a common key; if a nonce is ever reused, an attacker can use the resulting ciphertext to forge messages on arbitrary (even unused) nonces and the same key. While this seems unlikely to occur in the use cases of interest, there really isn't any good reason to use AES-GCM unless you have hardware support for it and speed is critical. The XChaCha20-Poly1305 authenticated encryption construction uses longer nonces, and it is considered safe to generate them randomly. While this construction has not yet been standardized (an existing draft standard has expired), it is common. Its cryptographic sibling ChaCha20-Poly1305 uses the same nonce length as AES-GCM, and while nonce reuse is "somewhat less worse" than in AES-GCM, it would still be unsafe in this context if it occurred (forgeries are limited to existing nonces). How Has This Been Tested? --- Existing tests pass. New tests are included. --- Cargo.lock | 102 ++++++------- base_layer/wallet/Cargo.toml | 2 +- .../wallet/src/key_manager_service/handle.rs | 4 +- .../src/key_manager_service/interface.rs | 4 +- .../wallet/src/key_manager_service/mock.rs | 4 +- .../wallet/src/key_manager_service/service.rs | 4 +- .../storage/database/backend.rs | 4 +- .../storage/database/mod.rs | 4 +- .../storage/sqlite_db/key_manager_state.rs | 14 +- .../storage/sqlite_db/mod.rs | 14 +- .../src/output_manager_service/handle.rs | 6 +- .../storage/database/backend.rs | 4 +- .../storage/database/mod.rs | 4 +- .../storage/sqlite_db/mod.rs | 46 +++--- .../storage/sqlite_db/new_output_sql.rs | 8 +- .../storage/sqlite_db/output_sql.rs | 8 +- base_layer/wallet/src/storage/database.rs | 6 +- .../wallet/src/storage/sqlite_db/wallet.rs | 41 +++--- .../wallet/src/transaction_service/handle.rs | 6 +- .../transaction_service/storage/database.rs | 6 +- .../transaction_service/storage/sqlite_db.rs | 56 +++++--- base_layer/wallet/src/types.rs | 8 -- base_layer/wallet/src/util/encryption.rs | 136 +++++++++++------- .../key_manager_service_tests/service.rs | 11 +- .../output_manager_service_tests/storage.rs | 10 +- .../transaction_service_tests/storage.rs | 18 ++- 26 files changed, 285 insertions(+), 245 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8dafbaf4e..2598b0f3c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -39,17 +39,6 @@ dependencies = [ "opaque-debug 0.3.0", ] -[[package]] -name = "aes" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bfe0133578c0986e1fe3dfcd4af1cc5b2dd6c3dbf534d69916ce16a2701d40ba" -dependencies = [ - "cfg-if 1.0.0", - "cipher 0.4.3", - "cpufeatures 0.2.2", -] - [[package]] name = "aes-gcm" version = "0.9.4" @@ -57,24 +46,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df5f85a83a7d8b0442b6aa7b504b8212c1733da07b98aae43d4bc21b2cb3cdf6" dependencies = [ "aead 0.4.3", - "aes 0.7.5", + "aes", "cipher 0.3.0", - "ctr 0.8.0", - "ghash 0.4.4", - "subtle", -] - -[[package]] -name = "aes-gcm" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82e1366e0c69c9f927b1fa5ce2c7bf9eafc8f9268c0b9800729e8b267612447c" -dependencies = [ - "aead 0.5.1", - "aes 0.8.1", - "cipher 0.4.3", - "ctr 0.9.1", - "ghash 0.5.0", + "ctr", + "ghash", "subtle", ] @@ -614,6 +589,17 @@ dependencies = [ "zeroize", ] +[[package]] +name = "chacha20" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7fc89c7c5b9e7a02dfe45cd2367bae382f9ed31c61ca8debe5f827c420a2f08" +dependencies = [ + "cfg-if 1.0.0", + "cipher 0.4.3", + "cpufeatures 0.2.2", +] + [[package]] name = "chacha20poly1305" version = "0.8.0" @@ -623,7 +609,7 @@ dependencies = [ "aead 0.4.3", "chacha20 0.7.1", "cipher 0.3.0", - "poly1305", + "poly1305 0.7.2", "zeroize", ] @@ -636,7 +622,20 @@ dependencies = [ "aead 0.4.3", "chacha20 0.8.2", "cipher 0.3.0", - "poly1305", + "poly1305 0.7.2", + "zeroize", +] + +[[package]] +name = "chacha20poly1305" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10cd79432192d1c0f4e1a0fef9527696cc039165d729fb41b3f4f4f354c2dc35" +dependencies = [ + "aead 0.5.1", + "chacha20 0.9.0", + "cipher 0.4.3", + "poly1305 0.8.0", "zeroize", ] @@ -687,6 +686,7 @@ checksum = "d1873270f8f7942c191139cb8a40fd228da6c3fd2fc376d7e92d47aa14aeb59e" dependencies = [ "crypto-common", "inout", + "zeroize", ] [[package]] @@ -1203,15 +1203,6 @@ dependencies = [ "cipher 0.3.0", ] -[[package]] -name = "ctr" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d14f329cfbaf5d0e06b5e87fff7e265d2673c5ea7d2c27691a2c107db1442a0" -dependencies = [ - "cipher 0.4.3", -] - [[package]] name = "curl-sys" version = "0.4.55+curl-7.83.1" @@ -1880,17 +1871,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1583cc1656d7839fd3732b80cf4f38850336cdb9b8ded1cd399ca62958de3c99" dependencies = [ "opaque-debug 0.3.0", - "polyval 0.5.3", -] - -[[package]] -name = "ghash" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d930750de5717d2dd0b8c0d42c076c0e884c81a73e6cab859bbd2339c71e3e40" -dependencies = [ - "opaque-debug 0.3.0", - "polyval 0.6.0", + "polyval", ] [[package]] @@ -3331,7 +3312,7 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0c63db779c3f090b540dfa0484f8adc2d380e3aa60cdb0f3a7a97454e22edc5" dependencies = [ - "aes 0.7.5", + "aes", "base64 0.13.0", "bitfield", "block-modes", @@ -3494,27 +3475,26 @@ dependencies = [ ] [[package]] -name = "polyval" -version = "0.5.3" +name = "poly1305" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8419d2b623c7c0896ff2d5d96e2cb4ede590fed28fcc34934f4c33c036e620a1" +checksum = "8159bd90725d2df49889a078b54f4f79e87f1f8a8444194cdca81d38f5393abf" dependencies = [ - "cfg-if 1.0.0", "cpufeatures 0.2.2", "opaque-debug 0.3.0", - "universal-hash 0.4.1", + "universal-hash 0.5.0", ] [[package]] name = "polyval" -version = "0.6.0" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ef234e08c11dfcb2e56f79fd70f6f2eb7f025c0ce2333e82f4f0518ecad30c6" +checksum = "8419d2b623c7c0896ff2d5d96e2cb4ede590fed28fcc34934f4c33c036e620a1" dependencies = [ "cfg-if 1.0.0", "cpufeatures 0.2.2", "opaque-debug 0.3.0", - "universal-hash 0.5.0", + "universal-hash 0.4.1", ] [[package]] @@ -4477,7 +4457,7 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6142f7c25e94f6fd25a32c3348ec230df9109b463f59c8c7acc4bd34936babb7" dependencies = [ - "aes-gcm 0.9.4", + "aes-gcm", "blake2 0.9.2", "chacha20poly1305 0.8.0", "rand 0.8.5", @@ -5380,11 +5360,11 @@ dependencies = [ name = "tari_wallet" version = "0.37.0" dependencies = [ - "aes-gcm 0.10.1", "argon2 0.2.4", "async-trait", "bincode", "blake2 0.9.2", + "chacha20poly1305 0.10.1", "chrono", "clear_on_drop", "crossbeam-channel", diff --git a/base_layer/wallet/Cargo.toml b/base_layer/wallet/Cargo.toml index 69302e1f94..7f347d8979 100644 --- a/base_layer/wallet/Cargo.toml +++ b/base_layer/wallet/Cargo.toml @@ -27,7 +27,6 @@ tari_utilities = { git = "https://github.com/tari-project/tari_utilities.git", t # Uncomment for normal use (non tokio-console tracing) tokio = { version = "1.14", features = ["sync", "macros"] } -aes-gcm = "0.10.1" async-trait = "0.1.50" argon2 = "0.2" bincode = "1.3.1" @@ -56,6 +55,7 @@ thiserror = "1.0.26" tower = "0.4" prost = "0.9" itertools = "0.10.3" +chacha20poly1305 = "0.10.1" [dependencies.tari_core] path = "../../base_layer/core" diff --git a/base_layer/wallet/src/key_manager_service/handle.rs b/base_layer/wallet/src/key_manager_service/handle.rs index 7d5407489b..b3c8213238 100644 --- a/base_layer/wallet/src/key_manager_service/handle.rs +++ b/base_layer/wallet/src/key_manager_service/handle.rs @@ -22,7 +22,7 @@ use std::sync::Arc; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use tari_common_types::types::PrivateKey; use tari_key_manager::cipher_seed::CipherSeed; use tokio::sync::RwLock; @@ -70,7 +70,7 @@ where TBackend: KeyManagerBackend + 'static .await } - async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> { + async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> { (*self.key_manager_inner).write().await.apply_encryption(cipher).await } diff --git a/base_layer/wallet/src/key_manager_service/interface.rs b/base_layer/wallet/src/key_manager_service/interface.rs index 5557ecb3bc..a03266e03c 100644 --- a/base_layer/wallet/src/key_manager_service/interface.rs +++ b/base_layer/wallet/src/key_manager_service/interface.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use tari_common_types::types::{PrivateKey, PublicKey}; use tari_crypto::keys::PublicKey as PublicKeyTrait; @@ -57,7 +57,7 @@ pub trait KeyManagerInterface: Clone + Send + Sync + 'static { /// Encrypts the key manager state using the provided cipher. An error is returned if the state is already /// encrypted. - async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError>; + async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError>; /// Decrypts the key manager state using the provided cipher. An error is returned if the state is not encrypted. async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError>; diff --git a/base_layer/wallet/src/key_manager_service/mock.rs b/base_layer/wallet/src/key_manager_service/mock.rs index abc8903aff..3a9ba2a3da 100644 --- a/base_layer/wallet/src/key_manager_service/mock.rs +++ b/base_layer/wallet/src/key_manager_service/mock.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use log::*; use tari_common_types::types::PrivateKey; use tari_key_manager::{cipher_seed::CipherSeed, key_manager::KeyManager}; @@ -154,7 +154,7 @@ impl KeyManagerInterface for KeyManagerMock { self.get_key_at_index_mock(branch.into(), index).await } - async fn apply_encryption(&self, _cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> { + async fn apply_encryption(&self, _cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> { unimplemented!("Not supported"); } diff --git a/base_layer/wallet/src/key_manager_service/service.rs b/base_layer/wallet/src/key_manager_service/service.rs index 091f8e372a..75cea72c21 100644 --- a/base_layer/wallet/src/key_manager_service/service.rs +++ b/base_layer/wallet/src/key_manager_service/service.rs @@ -19,7 +19,7 @@ // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use futures::lock::Mutex; use log::*; use tari_common_types::types::PrivateKey; @@ -110,7 +110,7 @@ where TBackend: KeyManagerBackend + 'static Ok(key.k) } - pub async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerServiceError> { + pub async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> { self.db.apply_encryption(cipher).await?; Ok(()) } diff --git a/base_layer/wallet/src/key_manager_service/storage/database/backend.rs b/base_layer/wallet/src/key_manager_service/storage/database/backend.rs index 5fa8af54f0..4ed19b968b 100644 --- a/base_layer/wallet/src/key_manager_service/storage/database/backend.rs +++ b/base_layer/wallet/src/key_manager_service/storage/database/backend.rs @@ -19,7 +19,7 @@ // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use crate::key_manager_service::{error::KeyManagerStorageError, storage::database::KeyManagerState}; @@ -35,7 +35,7 @@ pub trait KeyManagerBackend: Send + Sync + Clone { /// This method will set the currently stored key index for the key manager. fn set_key_index(&self, branch: String, index: u64) -> Result<(), KeyManagerStorageError>; /// Apply encryption to the backend. - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerStorageError>; + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerStorageError>; /// Remove encryption from the backend. fn remove_encryption(&self) -> Result<(), KeyManagerStorageError>; } diff --git a/base_layer/wallet/src/key_manager_service/storage/database/mod.rs b/base_layer/wallet/src/key_manager_service/storage/database/mod.rs index d294d06727..4c390f5010 100644 --- a/base_layer/wallet/src/key_manager_service/storage/database/mod.rs +++ b/base_layer/wallet/src/key_manager_service/storage/database/mod.rs @@ -23,8 +23,8 @@ mod backend; use std::sync::Arc; -use aes_gcm::Aes256Gcm; pub use backend::KeyManagerBackend; +use chacha20poly1305::XChaCha20Poly1305; use crate::key_manager_service::error::KeyManagerStorageError; @@ -95,7 +95,7 @@ where T: KeyManagerBackend + 'static /// Encrypts the entire key manager with all branches. /// This will only encrypt the index used, as the master seed phrase is not directly stored with the key manager. - pub async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerStorageError> { + pub async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerStorageError> { let db_clone = self.db.clone(); tokio::task::spawn_blocking(move || db_clone.apply_encryption(cipher)) .await diff --git a/base_layer/wallet/src/key_manager_service/storage/sqlite_db/key_manager_state.rs b/base_layer/wallet/src/key_manager_service/storage/sqlite_db/key_manager_state.rs index 8ee80df77c..f5ee46da49 100644 --- a/base_layer/wallet/src/key_manager_service/storage/sqlite_db/key_manager_state.rs +++ b/base_layer/wallet/src/key_manager_service/storage/sqlite_db/key_manager_state.rs @@ -22,7 +22,7 @@ use std::convert::TryFrom; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use chrono::{NaiveDateTime, Utc}; use diesel::{prelude::*, SqliteConnection}; @@ -150,21 +150,21 @@ pub struct KeyManagerStateUpdateSql { primary_key_index: Option>, } -impl Encryptable for KeyManagerStateSql { +impl Encryptable for KeyManagerStateSql { fn domain(&self, field_name: &'static str) -> Vec { [Self::KEY_MANAGER, self.branch_seed.as_bytes(), field_name.as_bytes()] .concat() .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.primary_key_index = encrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?; Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.primary_key_index = decrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?; @@ -172,21 +172,21 @@ impl Encryptable for KeyManagerStateSql { } } -impl Encryptable for NewKeyManagerStateSql { +impl Encryptable for NewKeyManagerStateSql { fn domain(&self, field_name: &'static str) -> Vec { [Self::KEY_MANAGER, self.branch_seed.as_bytes(), field_name.as_bytes()] .concat() .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.primary_key_index = encrypt_bytes_integral_nonce(cipher, self.domain("primary_key_index"), self.primary_key_index.clone())?; Ok(()) } - fn decrypt(&mut self, _cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, _cipher: &XChaCha20Poly1305) -> Result<(), String> { unimplemented!("Not supported") } } diff --git a/base_layer/wallet/src/key_manager_service/storage/sqlite_db/mod.rs b/base_layer/wallet/src/key_manager_service/storage/sqlite_db/mod.rs index ed34222817..8fae15894e 100644 --- a/base_layer/wallet/src/key_manager_service/storage/sqlite_db/mod.rs +++ b/base_layer/wallet/src/key_manager_service/storage/sqlite_db/mod.rs @@ -25,7 +25,7 @@ use std::{ sync::{Arc, RwLock}, }; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; pub use key_manager_state::{KeyManagerStateSql, NewKeyManagerStateSql}; use log::*; use tokio::time::Instant; @@ -47,7 +47,7 @@ const LOG_TARGET: &str = "wallet::key_manager_service::database::wallet"; #[derive(Clone)] pub struct KeyManagerSqliteDatabase { database_connection: WalletDbConnection, - cipher: Arc>>, + cipher: Arc>>, } impl KeyManagerSqliteDatabase { @@ -56,7 +56,7 @@ impl KeyManagerSqliteDatabase { /// not encrypt sensitive fields pub fn new( database_connection: WalletDbConnection, - cipher: Option, + cipher: Option, ) -> Result { let db = Self { database_connection, @@ -65,7 +65,7 @@ impl KeyManagerSqliteDatabase { Ok(db) } - fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), KeyManagerStorageError> { + fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), KeyManagerStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.decrypt(cipher) @@ -74,7 +74,7 @@ impl KeyManagerSqliteDatabase { Ok(()) } - fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), KeyManagerStorageError> { + fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), KeyManagerStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.encrypt(cipher) @@ -178,7 +178,7 @@ impl KeyManagerBackend for KeyManagerSqliteDatabase { Ok(()) } - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), KeyManagerStorageError> { + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerStorageError> { let mut current_cipher = acquire_write_lock!(self.cipher); if (*current_cipher).is_some() { @@ -230,7 +230,7 @@ impl KeyManagerBackend for KeyManagerSqliteDatabase { key_manager_state.set_state(&conn)?; } // Now that all the decryption has been completed we can safely remove the cipher fully - let _ = (*current_cipher).take(); + std::mem::drop((*current_cipher).take()); if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, diff --git a/base_layer/wallet/src/output_manager_service/handle.rs b/base_layer/wallet/src/output_manager_service/handle.rs index eff4735e1d..3403678213 100644 --- a/base_layer/wallet/src/output_manager_service/handle.rs +++ b/base_layer/wallet/src/output_manager_service/handle.rs @@ -22,7 +22,7 @@ use std::{fmt, fmt::Formatter, sync::Arc}; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use tari_common_types::{ transaction::TxId, types::{Commitment, HashOutput, PublicKey}, @@ -116,7 +116,7 @@ pub enum OutputManagerRequest { commitments: Vec, fee_per_gram: MicroTari, }, - ApplyEncryption(Box), + ApplyEncryption(Box), RemoveEncryption, FeeEstimate { amount: MicroTari, @@ -769,7 +769,7 @@ impl OutputManagerHandle { } } - pub async fn apply_encryption(&mut self, cipher: Aes256Gcm) -> Result<(), OutputManagerError> { + pub async fn apply_encryption(&mut self, cipher: XChaCha20Poly1305) -> Result<(), OutputManagerError> { match self .handle .call(OutputManagerRequest::ApplyEncryption(Box::new(cipher))) diff --git a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs index 878497345a..f1f72772d4 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/backend.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/backend.rs @@ -1,7 +1,7 @@ // Copyright 2022 The Tari Project // SPDX-License-Identifier: BSD-3-Clause -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use tari_common_types::{ transaction::TxId, types::{Commitment, FixedHash}, @@ -83,7 +83,7 @@ pub trait OutputManagerBackend: Send + Sync + Clone { /// If an invalid output is found to be valid this function will turn it back into an unspent output fn revalidate_unspent_output(&self, spending_key: &Commitment) -> Result<(), OutputManagerStorageError>; /// Apply encryption to the backend. - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), OutputManagerStorageError>; + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), OutputManagerStorageError>; /// Remove encryption from the backend. fn remove_encryption(&self) -> Result<(), OutputManagerStorageError>; diff --git a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs index 7cf2794e36..934c6e0161 100644 --- a/base_layer/wallet/src/output_manager_service/storage/database/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/database/mod.rs @@ -26,8 +26,8 @@ use std::{ sync::Arc, }; -use aes_gcm::Aes256Gcm; pub use backend::OutputManagerBackend; +use chacha20poly1305::XChaCha20Poly1305; use log::*; use tari_common_types::{ transaction::TxId, @@ -327,7 +327,7 @@ where T: OutputManagerBackend + 'static self.db.reinstate_cancelled_inbound_output(tx_id) } - pub fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), OutputManagerStorageError> { + pub fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), OutputManagerStorageError> { self.db.apply_encryption(cipher) } diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs index fac47b8f04..b6a5c8dc4e 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/mod.rs @@ -25,7 +25,7 @@ use std::{ sync::{Arc, RwLock}, }; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use chrono::NaiveDateTime; use derivative::Derivative; use diesel::{prelude::*, result::Error as DieselError, SqliteConnection}; @@ -67,18 +67,21 @@ const LOG_TARGET: &str = "wallet::output_manager_service::database::wallet"; #[derive(Clone)] pub struct OutputManagerSqliteDatabase { database_connection: WalletDbConnection, - cipher: Arc>>, + cipher: Arc>>, } impl OutputManagerSqliteDatabase { - pub fn new(database_connection: WalletDbConnection, cipher: Option) -> Self { + pub fn new(database_connection: WalletDbConnection, cipher: Option) -> Self { Self { database_connection, cipher: Arc::new(RwLock::new(cipher)), } } - fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), OutputManagerStorageError> { + fn decrypt_if_necessary>( + &self, + o: &mut T, + ) -> Result<(), OutputManagerStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.decrypt(cipher) @@ -87,7 +90,10 @@ impl OutputManagerSqliteDatabase { Ok(()) } - fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), OutputManagerStorageError> { + fn encrypt_if_necessary>( + &self, + o: &mut T, + ) -> Result<(), OutputManagerStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.encrypt(cipher) @@ -967,7 +973,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), OutputManagerStorageError> { + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), OutputManagerStorageError> { let mut current_cipher = acquire_write_lock!(self.cipher); if (*current_cipher).is_some() { @@ -1052,7 +1058,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { } // Now that all the decryption has been completed we can safely remove the cipher fully - let _ = (*current_cipher).take(); + std::mem::drop((*current_cipher).take()); if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -1422,7 +1428,7 @@ impl From for KnownOneSidedPaymentScriptSql { } } -impl Encryptable for KnownOneSidedPaymentScriptSql { +impl Encryptable for KnownOneSidedPaymentScriptSql { fn domain(&self, field_name: &'static str) -> Vec { [ Self::KNOWN_ONESIDED_PAYMENT_SCRIPT, @@ -1433,12 +1439,12 @@ impl Encryptable for KnownOneSidedPaymentScriptSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.private_key = encrypt_bytes_integral_nonce(cipher, self.domain("private_key"), self.private_key.clone())?; Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.private_key = decrypt_bytes_integral_nonce(cipher, self.domain("private_key"), self.private_key.clone())?; Ok(()) } @@ -1446,9 +1452,9 @@ impl Encryptable for KnownOneSidedPaymentScriptSql { #[cfg(test)] mod test { - use std::time::Duration; + use std::{mem::size_of, time::Duration}; - use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; + use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; use diesel::{Connection, SqliteConnection}; use rand::{rngs::OsRng, RngCore}; use tari_common_sqlite::sqlite_connection_pool::SqliteConnectionPool; @@ -1624,8 +1630,10 @@ mod test { let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None).unwrap(); let output = NewOutputSql::new(uo, OutputStatus::Unspent, None, None).unwrap(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); output.commit(&conn).unwrap(); let unencrypted_output = OutputSql::find(output.spending_key.as_slice(), &conn).unwrap(); @@ -1642,8 +1650,8 @@ mod test { decrypted_output.decrypt(&cipher).unwrap(); assert_eq!(decrypted_output.spending_key, output.spending_key); - let wrong_key = GenericArray::from_slice(b"an example very very wrong key!!"); - let wrong_cipher = Aes256Gcm::new(wrong_key); + let wrong_key = Key::from_slice(b"an example very very wrong key!!"); + let wrong_cipher = XChaCha20Poly1305::new(wrong_key); assert!(outputs[0].clone().decrypt(&wrong_cipher).is_err()); decrypted_output.update_encryption(&conn).unwrap(); @@ -1696,8 +1704,10 @@ mod test { output2.commit(&conn).unwrap(); } - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); let connection = WalletDbConnection::new(pool, None); diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs index e2a92a3a2f..502b13c202 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/new_output_sql.rs @@ -19,7 +19,7 @@ // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use derivative::Derivative; use diesel::{prelude::*, SqliteConnection}; use tari_common_types::transaction::TxId; @@ -109,7 +109,7 @@ impl NewOutputSql { } } -impl Encryptable for NewOutputSql { +impl Encryptable for NewOutputSql { fn domain(&self, field_name: &'static str) -> Vec { // WARNING: using `OUTPUT` for both NewOutputSql and OutputSql due to later transition without re-encryption [Self::OUTPUT, self.script.as_slice(), field_name.as_bytes()] @@ -117,7 +117,7 @@ impl Encryptable for NewOutputSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.spending_key = encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; @@ -130,7 +130,7 @@ impl Encryptable for NewOutputSql { Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.spending_key = decrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; diff --git a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs index 8542487192..140b8a11fd 100644 --- a/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs +++ b/base_layer/wallet/src/output_manager_service/storage/sqlite_db/output_sql.rs @@ -22,7 +22,7 @@ use std::convert::{TryFrom, TryInto}; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use chrono::NaiveDateTime; use derivative::Derivative; use diesel::{prelude::*, sql_query, SqliteConnection}; @@ -743,7 +743,7 @@ impl TryFrom for DbUnblindedOutput { } } -impl Encryptable for OutputSql { +impl Encryptable for OutputSql { fn domain(&self, field_name: &'static str) -> Vec { // WARNING: using `OUTPUT` for both NewOutputSql and OutputSql due to later transition without re-encryption [Self::OUTPUT, self.script.as_slice(), field_name.as_bytes()] @@ -751,7 +751,7 @@ impl Encryptable for OutputSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.spending_key = encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; @@ -764,7 +764,7 @@ impl Encryptable for OutputSql { Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.spending_key = decrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; diff --git a/base_layer/wallet/src/storage/database.rs b/base_layer/wallet/src/storage/database.rs index 9050ef9a34..9c870e7f0e 100644 --- a/base_layer/wallet/src/storage/database.rs +++ b/base_layer/wallet/src/storage/database.rs @@ -25,7 +25,7 @@ use std::{ sync::Arc, }; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use log::*; use tari_common_types::chain_metadata::ChainMetadata; use tari_comms::{ @@ -47,7 +47,7 @@ pub trait WalletBackend: Send + Sync + Clone { /// Modify the state the of the backend with a write operation fn write(&self, op: WriteOperation) -> Result, WalletStorageError>; /// Apply encryption to the backend. - fn apply_encryption(&self, passphrase: SafePassword) -> Result; + fn apply_encryption(&self, passphrase: SafePassword) -> Result; /// Remove encryption from the backend. fn remove_encryption(&self) -> Result<(), WalletStorageError>; @@ -277,7 +277,7 @@ where T: WalletBackend + 'static Ok(()) } - pub async fn apply_encryption(&self, passphrase: SafePassword) -> Result { + pub async fn apply_encryption(&self, passphrase: SafePassword) -> Result { let db_clone = self.db.clone(); tokio::task::spawn_blocking(move || db_clone.apply_encryption(passphrase)) .await diff --git a/base_layer/wallet/src/storage/sqlite_db/wallet.rs b/base_layer/wallet/src/storage/sqlite_db/wallet.rs index e180c0a43d..8c50660a19 100644 --- a/base_layer/wallet/src/storage/sqlite_db/wallet.rs +++ b/base_layer/wallet/src/storage/sqlite_db/wallet.rs @@ -22,15 +22,16 @@ use std::{ convert::TryFrom, + mem::size_of, str::{from_utf8, FromStr}, sync::{Arc, RwLock}, }; -use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; use argon2::{ password_hash::{rand_core::OsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString}, Argon2, }; +use chacha20poly1305::{Key, KeyInit, Tag, XChaCha20Poly1305, XNonce}; use diesel::{prelude::*, SqliteConnection}; use log::*; use tari_common_types::chain_metadata::ChainMetadata; @@ -55,13 +56,7 @@ use crate::{ sqlite_db::scanned_blocks::ScannedBlockSql, sqlite_utilities::wallet_db_connection::WalletDbConnection, }, - util::encryption::{ - decrypt_bytes_integral_nonce, - encrypt_bytes_integral_nonce, - Encryptable, - AES_MAC_BYTES, - AES_NONCE_BYTES, - }, + util::encryption::{decrypt_bytes_integral_nonce, encrypt_bytes_integral_nonce, Encryptable}, utxo_scanner_service::service::ScannedBlock, }; @@ -71,7 +66,7 @@ const LOG_TARGET: &str = "wallet::storage::wallet"; #[derive(Clone)] pub struct WalletSqliteDatabase { database_connection: WalletDbConnection, - cipher: Arc>>, + cipher: Arc>>, } impl WalletSqliteDatabase { pub fn new( @@ -130,7 +125,7 @@ impl WalletSqliteDatabase { } } - fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), WalletStorageError> { + fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), WalletStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.decrypt(cipher) @@ -139,7 +134,7 @@ impl WalletSqliteDatabase { Ok(()) } - fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), WalletStorageError> { + fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), WalletStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.encrypt(cipher) @@ -341,7 +336,7 @@ impl WalletSqliteDatabase { Ok(None) } - pub fn cipher(&self) -> Option { + pub fn cipher(&self) -> Option { let cipher = acquire_read_lock!(self.cipher); (*cipher).clone() } @@ -396,7 +391,7 @@ impl WalletBackend for WalletSqliteDatabase { } } - fn apply_encryption(&self, passphrase: SafePassword) -> Result { + fn apply_encryption(&self, passphrase: SafePassword) -> Result { let mut current_cipher = acquire_write_lock!(self.cipher); if current_cipher.is_some() { return Err(WalletStorageError::AlreadyEncrypted); @@ -428,8 +423,8 @@ impl WalletBackend for WalletSqliteDatabase { .hash .ok_or_else(|| WalletStorageError::AeadError("Problem generating encryption key hash".to_string()))?; - let key = GenericArray::from_slice(derived_encryption_key.as_bytes()); - let cipher = Aes256Gcm::new(key); + let key = Key::from_slice(derived_encryption_key.as_bytes()); + let cipher = XChaCha20Poly1305::new(key); WalletSettingSql::new(DbKey::PassphraseHash.to_string(), passphrase_hash).set(&conn)?; WalletSettingSql::new(DbKey::EncryptionSalt.to_string(), encryption_salt.as_str().to_string()).set(&conn)?; @@ -535,7 +530,7 @@ impl WalletBackend for WalletSqliteDatabase { } // Now that all the decryption has been completed we can safely remove the cipher fully - let _ = (*current_cipher).take(); + std::mem::drop((*current_cipher).take()); if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -592,7 +587,7 @@ impl WalletBackend for WalletSqliteDatabase { fn check_db_encryption_status( database_connection: &WalletDbConnection, passphrase: Option, -) -> Result, WalletStorageError> { +) -> Result, WalletStorageError> { let start = Instant::now(); let conn = database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); @@ -623,8 +618,8 @@ fn check_db_encryption_status( .map_err(|e| WalletStorageError::AeadError(e.to_string()))? .hash .ok_or_else(|| WalletStorageError::AeadError("Problem generating encryption key hash".to_string()))?; - let key = GenericArray::from_slice(derived_encryption_key.as_bytes()); - Some(Aes256Gcm::new(key)) + let key = Key::from_slice(derived_encryption_key.as_bytes()); + Some(XChaCha20Poly1305::new(key)) }, _ => None, }; @@ -656,7 +651,7 @@ fn check_db_encryption_status( if let Some(cipher_inner) = cipher.clone() { let sk_bytes: Vec = from_hex(sk.as_str())?; - if sk_bytes.len() < AES_NONCE_BYTES + AES_MAC_BYTES { + if sk_bytes.len() < size_of::() + size_of::() { return Err(WalletStorageError::MissingNonce); } @@ -780,7 +775,7 @@ impl ClientKeyValueSql { } } -impl Encryptable for ClientKeyValueSql { +impl Encryptable for ClientKeyValueSql { fn domain(&self, field_name: &'static str) -> Vec { [Self::CLIENT_KEY_VALUE, self.key.as_bytes(), field_name.as_bytes()] .concat() @@ -788,7 +783,7 @@ impl Encryptable for ClientKeyValueSql { } #[allow(unused_assignments)] - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.value = encrypt_bytes_integral_nonce(cipher, self.domain("value"), self.value.as_bytes().to_vec())?.to_hex(); @@ -796,7 +791,7 @@ impl Encryptable for ClientKeyValueSql { } #[allow(unused_assignments)] - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { let decrypted_value = decrypt_bytes_integral_nonce( cipher, self.domain("value"), diff --git a/base_layer/wallet/src/transaction_service/handle.rs b/base_layer/wallet/src/transaction_service/handle.rs index efa38339ac..9a68c97690 100644 --- a/base_layer/wallet/src/transaction_service/handle.rs +++ b/base_layer/wallet/src/transaction_service/handle.rs @@ -27,7 +27,7 @@ use std::{ sync::Arc, }; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use chrono::NaiveDateTime; use tari_common_types::{ transaction::{ImportStatus, TxId}, @@ -114,7 +114,7 @@ pub enum TransactionServiceRequest { SubmitTransactionToSelf(TxId, Transaction, MicroTari, MicroTari, String), SetLowPowerMode, SetNormalPowerMode, - ApplyEncryption(Box), + ApplyEncryption(Box), RemoveEncryption, GenerateCoinbaseTransaction(MicroTari, MicroTari, u64), RestartTransactionProtocols, @@ -713,7 +713,7 @@ impl TransactionServiceHandle { } } - pub async fn apply_encryption(&mut self, cipher: Aes256Gcm) -> Result<(), TransactionServiceError> { + pub async fn apply_encryption(&mut self, cipher: XChaCha20Poly1305) -> Result<(), TransactionServiceError> { match self .handle .call(TransactionServiceRequest::ApplyEncryption(Box::new(cipher))) diff --git a/base_layer/wallet/src/transaction_service/storage/database.rs b/base_layer/wallet/src/transaction_service/storage/database.rs index 5b157a2e79..6fc21c2354 100644 --- a/base_layer/wallet/src/transaction_service/storage/database.rs +++ b/base_layer/wallet/src/transaction_service/storage/database.rs @@ -28,7 +28,7 @@ use std::{ sync::Arc, }; -use aes_gcm::Aes256Gcm; +use chacha20poly1305::XChaCha20Poly1305; use chrono::{NaiveDateTime, Utc}; use log::*; use tari_common_types::{ @@ -125,7 +125,7 @@ pub trait TransactionBackend: Send + Sync + Clone { amount: MicroTari, ) -> Result, TransactionStorageError>; /// Apply encryption to the backend. - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), TransactionStorageError>; + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), TransactionStorageError>; /// Remove encryption from the backend. fn remove_encryption(&self) -> Result<(), TransactionStorageError>; /// Increment the send counter and timestamp of a transaction @@ -827,7 +827,7 @@ where T: TransactionBackend + 'static .and_then(|inner_result| inner_result) } - pub async fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), TransactionStorageError> { + pub async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), TransactionStorageError> { let db_clone = self.db.clone(); tokio::task::spawn_blocking(move || db_clone.apply_encryption(cipher)) .await diff --git a/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs b/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs index 719038f74d..46c96aca8c 100644 --- a/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs +++ b/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs @@ -27,7 +27,7 @@ use std::{ sync::{Arc, RwLock}, }; -use aes_gcm::{self, Aes256Gcm}; +use chacha20poly1305::XChaCha20Poly1305; use chrono::{NaiveDateTime, Utc}; use diesel::{prelude::*, result::Error as DieselError, SqliteConnection}; use log::*; @@ -78,11 +78,11 @@ const LOG_TARGET: &str = "wallet::transaction_service::database::wallet"; #[derive(Clone)] pub struct TransactionServiceSqliteDatabase { database_connection: WalletDbConnection, - cipher: Arc>>, + cipher: Arc>>, } impl TransactionServiceSqliteDatabase { - pub fn new(database_connection: WalletDbConnection, cipher: Option) -> Self { + pub fn new(database_connection: WalletDbConnection, cipher: Option) -> Self { Self { database_connection, cipher: Arc::new(RwLock::new(cipher)), @@ -202,7 +202,10 @@ impl TransactionServiceSqliteDatabase { } } - fn decrypt_if_necessary>(&self, o: &mut T) -> Result<(), TransactionStorageError> { + fn decrypt_if_necessary>( + &self, + o: &mut T, + ) -> Result<(), TransactionStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.decrypt(cipher) @@ -211,7 +214,10 @@ impl TransactionServiceSqliteDatabase { Ok(()) } - fn encrypt_if_necessary>(&self, o: &mut T) -> Result<(), TransactionStorageError> { + fn encrypt_if_necessary>( + &self, + o: &mut T, + ) -> Result<(), TransactionStorageError> { let cipher = acquire_read_lock!(self.cipher); if let Some(cipher) = cipher.as_ref() { o.encrypt(cipher) @@ -791,7 +797,7 @@ impl TransactionBackend for TransactionServiceSqliteDatabase { Ok(()) } - fn apply_encryption(&self, cipher: Aes256Gcm) -> Result<(), TransactionStorageError> { + fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), TransactionStorageError> { let mut current_cipher = acquire_write_lock!(self.cipher); if (*current_cipher).is_some() { @@ -900,7 +906,7 @@ impl TransactionBackend for TransactionServiceSqliteDatabase { } // Now that all the decryption has been completed we can safely remove the cipher fully - let _ = (*current_cipher).take(); + std::mem::drop((*current_cipher).take()); if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -1442,7 +1448,7 @@ impl InboundTransactionSql { } } -impl Encryptable for InboundTransactionSql { +impl Encryptable for InboundTransactionSql { fn domain(&self, field_name: &'static str) -> Vec { [ Self::INBOUND_TRANSACTION, @@ -1453,7 +1459,7 @@ impl Encryptable for InboundTransactionSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.receiver_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("receiver_protocol"), @@ -1464,7 +1470,7 @@ impl Encryptable for InboundTransactionSql { Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { let decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("receiver_protocol"), @@ -1630,7 +1636,7 @@ impl OutboundTransactionSql { } } -impl Encryptable for OutboundTransactionSql { +impl Encryptable for OutboundTransactionSql { fn domain(&self, field_name: &'static str) -> Vec { [ Self::OUTBOUND_TRANSACTION, @@ -1641,7 +1647,7 @@ impl Encryptable for OutboundTransactionSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.sender_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("sender_protocol"), @@ -1652,7 +1658,7 @@ impl Encryptable for OutboundTransactionSql { Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { let decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("sender_protocol"), @@ -1976,7 +1982,7 @@ impl CompletedTransactionSql { } } -impl Encryptable for CompletedTransactionSql { +impl Encryptable for CompletedTransactionSql { fn domain(&self, field_name: &'static str) -> Vec { [ Self::COMPLETED_TRANSACTION, @@ -1987,7 +1993,7 @@ impl Encryptable for CompletedTransactionSql { .to_vec() } - fn encrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { self.transaction_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("transaction_protocol"), @@ -1998,7 +2004,7 @@ impl Encryptable for CompletedTransactionSql { Ok(()) } - fn decrypt(&mut self, cipher: &Aes256Gcm) -> Result<(), String> { + fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { let decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("transaction_protocol"), @@ -2194,12 +2200,12 @@ impl UnconfirmedTransactionInfoSql { #[cfg(test)] mod test { - use std::{convert::TryFrom, time::Duration}; + use std::{convert::TryFrom, mem::size_of, time::Duration}; - use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; + use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; use chrono::Utc; use diesel::{Connection, SqliteConnection}; - use rand::rngs::OsRng; + use rand::{rngs::OsRng, RngCore}; use tari_common_sqlite::sqlite_connection_pool::SqliteConnectionPool; use tari_common_types::{ transaction::{TransactionDirection, TransactionStatus, TxId}, @@ -2656,8 +2662,10 @@ mod test { conn.execute("PRAGMA foreign_keys = ON").unwrap(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); let inbound_tx = InboundTransaction { tx_id: 1u64.into(), @@ -2827,8 +2835,10 @@ mod test { completed_tx_sql.commit(&conn).unwrap(); } - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); let connection = WalletDbConnection::new(pool, None); diff --git a/base_layer/wallet/src/types.rs b/base_layer/wallet/src/types.rs index 4442fb8f61..2b3fc9dd31 100644 --- a/base_layer/wallet/src/types.rs +++ b/base_layer/wallet/src/types.rs @@ -32,12 +32,4 @@ pub(crate) trait PersistentKeyManager { fn create_and_store_new(&mut self) -> Result; } -hasher!( - Blake256, - WalletEncryptionHasher, - "com.tari.base_layer.wallet.encryption", - 1, - wallet_encryption_hasher -); - hasher!(Blake256, WalletHasher, "com.tari.base_layer.wallet", 1, wallet_hasher); diff --git a/base_layer/wallet/src/util/encryption.rs b/base_layer/wallet/src/util/encryption.rs index 3ec30df8a9..b826ad5839 100644 --- a/base_layer/wallet/src/util/encryption.rs +++ b/base_layer/wallet/src/util/encryption.rs @@ -20,19 +20,17 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::{ - aead::{generic_array::GenericArray, Aead, Error as AeadError}, - Aes256Gcm, +use std::mem::size_of; + +use chacha20poly1305::{ + aead::{Aead, Error as AeadError, Payload}, + Tag, + XChaCha20Poly1305, + XNonce, }; use rand::{rngs::OsRng, RngCore}; use tari_utilities::ByteArray; -use crate::types::WalletEncryptionHasher; - -pub const AES_NONCE_BYTES: usize = 12; -pub const AES_KEY_BYTES: usize = 32; -pub const AES_MAC_BYTES: usize = 32; - pub trait Encryptable { const KEY_MANAGER: &'static [u8] = b"KEY_MANAGER"; const OUTPUT: &'static [u8] = b"OUTPUT"; @@ -49,92 +47,132 @@ pub trait Encryptable { fn decrypt(&mut self, cipher: &C) -> Result<(), String>; } +// Decrypt data (with domain binding and authentication) using XChaCha20-Poly1305 pub fn decrypt_bytes_integral_nonce( - cipher: &Aes256Gcm, + cipher: &XChaCha20Poly1305, domain: Vec, ciphertext: Vec, ) -> Result, String> { - if ciphertext.len() < AES_NONCE_BYTES + AES_MAC_BYTES { + // We need at least a nonce and tag, or there's no point in attempting decryption + if ciphertext.len() < size_of::() + size_of::() { return Err(AeadError.to_string()); } - let (nonce, ciphertext) = ciphertext.split_at(AES_NONCE_BYTES); - let (ciphertext, appended_mac) = ciphertext.split_at(ciphertext.len().saturating_sub(AES_MAC_BYTES)); - let nonce = GenericArray::from_slice(nonce); + // Extract the nonce + let (nonce, ciphertext) = ciphertext.split_at(size_of::()); + let nonce_ga = XNonce::from_slice(nonce); - let expected_mac = WalletEncryptionHasher::new_with_label("storage_encryption_mac") - .chain(nonce.as_slice()) - .chain(ciphertext) - .chain(domain) - .finalize(); - - if appended_mac != expected_mac.as_ref() { - return Err(AeadError.to_string()); - } + let payload = Payload { + msg: ciphertext, + aad: domain.as_bytes(), + }; - let plaintext = cipher.decrypt(nonce, ciphertext.as_ref()).map_err(|e| e.to_string())?; + // Attempt authentication and decryption + let plaintext = cipher.decrypt(nonce_ga, payload).map_err(|e| e.to_string())?; Ok(plaintext) } +// Encrypt data (with domain binding and authentication) using XChaCha20-Poly1305 pub fn encrypt_bytes_integral_nonce( - cipher: &Aes256Gcm, + cipher: &XChaCha20Poly1305, domain: Vec, plaintext: Vec, ) -> Result, String> { - let mut nonce = [0u8; AES_NONCE_BYTES]; + // Produce a secure random nonce + let mut nonce = [0u8; size_of::()]; OsRng.fill_bytes(&mut nonce); - let nonce_ga = GenericArray::from_slice(&nonce); + let nonce_ga = XNonce::from_slice(&nonce); - let mut ciphertext = cipher - .encrypt(nonce_ga, plaintext.as_bytes()) - .map_err(|e| e.to_string())?; + // Bind the domain as additional data + let payload = Payload { + msg: plaintext.as_bytes(), + aad: domain.as_bytes(), + }; - let mut mac = WalletEncryptionHasher::new_with_label("storage_encryption_mac") - .chain(nonce.as_slice()) - .chain(ciphertext.clone()) - .chain(domain.as_slice()) - .finalize() - .as_ref() - .to_vec(); + // Attempt authenticated encryption + let mut ciphertext = cipher.encrypt(nonce_ga, payload).map_err(|e| e.to_string())?; + // Concatenate the nonce and ciphertext (which already include the tag) let mut ciphertext_integral_nonce = nonce.to_vec(); ciphertext_integral_nonce.append(&mut ciphertext); - ciphertext_integral_nonce.append(&mut mac); Ok(ciphertext_integral_nonce) } #[cfg(test)] mod test { - use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; + use std::mem::size_of; + + use chacha20poly1305::{Key, KeyInit, Tag, XChaCha20Poly1305, XNonce}; + use rand::{rngs::OsRng, RngCore}; + use tari_utilities::ByteArray; use crate::util::encryption::{decrypt_bytes_integral_nonce, encrypt_bytes_integral_nonce}; #[test] fn test_encrypt_decrypt() { + // Encrypt a message let plaintext = b"The quick brown fox was annoying".to_vec(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); let ciphertext = encrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), plaintext.clone()).unwrap(); + + // Check the ciphertext size, which we rely on for later tests + // It should extend the plaintext size by the nonce and tag sizes + assert_eq!( + ciphertext.len(), + size_of::() + plaintext.len() + size_of::() + ); + + // Valid decryption must succeed and yield correct plaintext let decrypted_text = decrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), ciphertext.clone()).unwrap(); - - // decrypted text must be equal to the original plaintext assert_eq!(decrypted_text, plaintext); - // must fail with a wrong domain + // Must fail on an incorrect domain assert!(decrypt_bytes_integral_nonce(&cipher, b"wrong_domain".to_vec(), ciphertext.clone()).is_err()); - // must fail without nonce - assert!(decrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), ciphertext[0..12].to_vec()).is_err()); - - // must fail without mac + // Must fail with an evil nonce + let ciphertext_with_evil_nonce = ciphertext + .clone() + .splice(0..size_of::(), [0u8; size_of::()]) + .collect(); + assert!(decrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), ciphertext_with_evil_nonce).is_err()); + + // Must fail with malleated ciphertext + let ciphertext_with_evil_ciphertext = ciphertext + .clone() + .splice( + size_of::()..(ciphertext.len() - size_of::()), + vec![0u8; plaintext.len()], + ) + .collect(); + assert!( + decrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), ciphertext_with_evil_ciphertext).is_err() + ); + + // Must fail with malleated authentication tag + let ciphertext_with_evil_tag = ciphertext + .clone() + .splice((ciphertext.len() - size_of::())..ciphertext.len(), vec![ + 0u8; + size_of::< + Tag, + >( + ) + ]) + .collect(); + assert!(decrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), ciphertext_with_evil_tag).is_err()); + + // Must fail if truncated too short (if shorter than a nonce and tag, decryption is not even attempted) assert!(decrypt_bytes_integral_nonce( &cipher, b"correct_domain".to_vec(), - ciphertext[0..ciphertext.len().saturating_sub(32)].to_vec() + ciphertext[0..(size_of::() + size_of::() - 1)].to_vec() ) .is_err()); } diff --git a/base_layer/wallet/tests/key_manager_service_tests/service.rs b/base_layer/wallet/tests/key_manager_service_tests/service.rs index e01303c3d8..0898c69459 100644 --- a/base_layer/wallet/tests/key_manager_service_tests/service.rs +++ b/base_layer/wallet/tests/key_manager_service_tests/service.rs @@ -20,7 +20,10 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; +use std::mem::size_of; + +use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; +use rand::{rngs::OsRng, RngCore}; use tari_key_manager::cipher_seed::CipherSeed; use tari_wallet::key_manager_service::{ storage::{database::KeyManagerDatabase, sqlite_db::KeyManagerSqliteDatabase}, @@ -73,8 +76,10 @@ async fn get_key_at_test_no_encryption() { async fn get_key_at_test_with_encryption() { let (connection, _tempdir) = get_temp_sqlite_database_connection(); let cipher = CipherSeed::new(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let db_cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let db_cipher = XChaCha20Poly1305::new(key_ga); let key_manager = KeyManagerHandle::new( cipher, KeyManagerDatabase::new(KeyManagerSqliteDatabase::new(connection, Some(db_cipher)).unwrap()), diff --git a/base_layer/wallet/tests/output_manager_service_tests/storage.rs b/base_layer/wallet/tests/output_manager_service_tests/storage.rs index f5b15010c3..a1dd6958b3 100644 --- a/base_layer/wallet/tests/output_manager_service_tests/storage.rs +++ b/base_layer/wallet/tests/output_manager_service_tests/storage.rs @@ -20,7 +20,9 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; +use std::mem::size_of; + +use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; use rand::{rngs::OsRng, RngCore}; use tari_common_types::{transaction::TxId, types::FixedHash}; use tari_core::transactions::{tari_amount::MicroTari, CryptoFactories}; @@ -322,8 +324,10 @@ pub fn test_output_manager_sqlite_db() { pub fn test_output_manager_sqlite_db_encrypted() { let (connection, _tempdir) = get_temp_sqlite_database_connection(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); test_db_backend(OutputManagerSqliteDatabase::new(connection, Some(cipher))); } diff --git a/base_layer/wallet/tests/transaction_service_tests/storage.rs b/base_layer/wallet/tests/transaction_service_tests/storage.rs index 680ea7352b..6b0da3b13b 100644 --- a/base_layer/wallet/tests/transaction_service_tests/storage.rs +++ b/base_layer/wallet/tests/transaction_service_tests/storage.rs @@ -20,9 +20,11 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use aes_gcm::{aead::generic_array::GenericArray, Aes256Gcm, KeyInit}; +use std::mem::size_of; + +use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; use chrono::{NaiveDateTime, Utc}; -use rand::rngs::OsRng; +use rand::{rngs::OsRng, RngCore}; use tari_common_types::{ transaction::{TransactionDirection, TransactionStatus, TxId}, types::{FixedHash, PrivateKey, PublicKey, Signature}, @@ -579,8 +581,10 @@ pub fn test_transaction_service_sqlite_db_encrypted() { let db_path = format!("{}/{}", db_folder, db_name); let connection = run_migration_and_create_sqlite_connection(&db_path, 16).unwrap(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); test_db_backend(TransactionServiceSqliteDatabase::new(connection, Some(cipher))); } @@ -593,8 +597,10 @@ async fn import_tx_and_read_it_from_db() { let db_path = format!("{}/{}", db_folder, db_name); let connection = run_migration_and_create_sqlite_connection(&db_path, 16).unwrap(); - let key = GenericArray::from_slice(b"an example very very secret key."); - let cipher = Aes256Gcm::new(key); + let mut key = [0u8; size_of::()]; + OsRng.fill_bytes(&mut key); + let key_ga = Key::from_slice(&key); + let cipher = XChaCha20Poly1305::new(key_ga); let sqlite_db = TransactionServiceSqliteDatabase::new(connection, Some(cipher)); let transaction = CompletedTransaction::new(