From 041d74f01b864c6990edcfb0607aab8ff236b920 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 10:24:49 +0000 Subject: [PATCH 1/7] first commit --- .../storage/sqlite_db/mod.rs | 18 +- .../storage/sqlite_db/new_output_sql.rs | 13 +- .../storage/sqlite_db/output_sql.rs | 155 +++++++++++++++++- 3 files changed, 177 insertions(+), 9 deletions(-) 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 83bb110a5e..998a5a5b45 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 @@ -45,6 +45,7 @@ use tari_core::transactions::transaction_components::{OutputType, TransactionOut use tari_crypto::tari_utilities::{hex::Hex, ByteArray}; use tari_script::{ExecutionStack, TariScript}; use tokio::time::Instant; +use zeroize::Zeroize; use crate::{ output_manager_service::{ @@ -108,21 +109,22 @@ impl OutputManagerSqliteDatabase { } fn insert(&self, key_value_pair: DbKeyValuePair, conn: &SqliteConnection) -> Result<(), OutputManagerStorageError> { + let cipher = acquire_read_lock!(self.cipher).as_ref(); + match key_value_pair { DbKeyValuePair::UnspentOutput(c, o) => { if OutputSql::find_by_commitment_and_cancelled(&c.to_vec(), false, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, None, None)?; - self.encrypt_if_necessary(&mut new_output)?; + let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, None, None, None)?; + self.encrypt_if_necessary(new_output)?; new_output.commit(conn)? }, DbKeyValuePair::UnspentOutputWithTxId(c, (tx_id, o)) => { if OutputSql::find_by_commitment_and_cancelled(&c.to_vec(), false, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, Some(tx_id), None)?; - self.encrypt_if_necessary(&mut new_output)?; + let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, Some(tx_id), None, cipher)?; new_output.commit(conn)? }, DbKeyValuePair::OutputToBeReceived(c, (tx_id, o, coinbase_block_height)) => { @@ -134,8 +136,8 @@ impl OutputManagerSqliteDatabase { OutputStatus::EncumberedToBeReceived, Some(tx_id), coinbase_block_height, + cipher, )?; - self.encrypt_if_necessary(&mut new_output)?; new_output.commit(conn)? }, @@ -1256,10 +1258,14 @@ fn update_outputs_with_tx_id_and_status_to_new_status( } /// These are the fields that can be updated for an Output -#[derive(Default)] +#[derive(Default, Zeroize)] +#[zeroize(drop)] pub struct UpdateOutput { + #[zeroize(skip)] status: Option, + #[zeroize(skip)] received_in_tx_id: Option>, + #[zeroize(skip)] spent_in_tx_id: Option>, spending_key: Option>, script_private_key: Option>, 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 2878e54a6c..f15336cedd 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 @@ -73,8 +73,9 @@ impl NewOutputSql { status: OutputStatus, received_in_tx_id: Option, coinbase_block_height: Option, + cipher: Option<&XChaCha20Poly1305>, ) -> Result { - Ok(Self { + let mut output = Self { commitment: Some(output.commitment.to_vec()), spending_key: output.unblinded_output.spending_key.to_vec(), value: output.unblinded_output.value.as_u64() as i64, @@ -101,7 +102,15 @@ impl NewOutputSql { encrypted_value: output.unblinded_output.encrypted_value.to_vec(), minimum_value_promise: output.unblinded_output.minimum_value_promise.as_u64() as i64, source: output.source as i32, - }) + }; + + if let Some(cipher) = cipher { + output + .encrypt(cipher) + .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; + } + + Ok(output) } /// Write this struct to the database 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 b7cd3f6216..20c13dfbb8 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 chacha20poly1305::XChaCha20Poly1305; +use chacha20poly1305::{ChaCha20Poly1305, XChaCha20Poly1305}; use chrono::NaiveDateTime; use derivative::Derivative; use diesel::{prelude::*, sql_query, SqliteConnection}; @@ -621,6 +621,159 @@ impl OutputSql { } } +impl OutputSql { + pub fn to_db_unblinded_output( + mut self, + cipher: Option, + ) -> Result { + if let Some(cipher) = cipher { + self.decrypt(&cipher); + } + + let features: OutputFeatures = + serde_json::from_str(&self.features_json).map_err(|s| OutputManagerStorageError::ConversionError { + reason: format!("Could not convert json into OutputFeatures:{}", s), + })?; + + let encrypted_value = EncryptedValue::from_bytes(&self.encrypted_value)?; + let unblinded_output = UnblindedOutput::new_current_version( + MicroTari::from(self.value as u64), + PrivateKey::from_vec(&self.spending_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PrivateKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + features, + TariScript::from_bytes(self.script.as_slice())?, + ExecutionStack::from_bytes(self.input_data.as_slice())?, + PrivateKey::from_vec(&self.script_private_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PrivateKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + PublicKey::from_vec(&self.sender_offset_public_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PublicKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + ComSignature::new( + Commitment::from_vec(&self.metadata_signature_nonce).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PublicKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + PrivateKey::from_vec(&self.metadata_signature_u_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PrivateKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + PrivateKey::from_vec(&self.metadata_signature_v_key).map_err(|_| { + error!( + target: LOG_TARGET, + "Could not create PrivateKey from stored bytes, They might be encrypted" + ); + OutputManagerStorageError::ConversionError { + reason: "PrivateKey could not be converted from bytes".to_string(), + } + })?, + ), + self.script_lock_height as u64, + Covenant::from_bytes(&self.covenant).map_err(|e| { + error!( + target: LOG_TARGET, + "Could not create Covenant from stored bytes ({}), They might be encrypted", e + ); + OutputManagerStorageError::ConversionError { + reason: "Covenant could not be converted from bytes".to_string(), + } + })?, + encrypted_value, + MicroTari::from(self.minimum_value_promise as u64), + ); + + // we manually zeroize the sensitive data associated with OutputSql, in order to remove potential leaks + use zeroize::{Zeroize, Zeroizing}; + Zeroizing::new(self.spending_key).zeroize(); + Zeroizing::new(self.script_private_key).zeroize(); + self.spending_key = self.spending_key.iter().map(|x| 0u8).collect::>(); + self.script_private_key = self.script_private_key.iter().map(|x| 0u8).collect::>(); + + let hash = match self.hash { + None => { + let factories = CryptoFactories::default(); + unblinded_output.as_transaction_output(&factories)?.hash() + }, + Some(v) => match v.try_into() { + Ok(v) => v, + Err(e) => { + error!(target: LOG_TARGET, "Malformed transaction hash: {}", e); + return Err(OutputManagerStorageError::ConversionError { + reason: "Malformed transaction hash".to_string(), + }); + }, + }, + }; + let commitment = match self.commitment { + None => { + let factories = CryptoFactories::default(); + factories + .commitment + .commit(&unblinded_output.spending_key, &unblinded_output.value.into()) + }, + Some(c) => Commitment::from_vec(&c)?, + }; + let spending_priority = (self.spending_priority as u32).into(); + let mined_in_block = match self.mined_in_block { + Some(v) => match v.try_into() { + Ok(v) => Some(v), + Err(_) => None, + }, + None => None, + }; + let marked_deleted_in_block = match self.marked_deleted_in_block { + Some(v) => match v.try_into() { + Ok(v) => Some(v), + Err(_) => None, + }, + None => None, + }; + Ok(DbUnblindedOutput { + commitment, + unblinded_output, + hash, + status: self.status.try_into()?, + mined_height: self.mined_height.map(|mh| mh as u64), + mined_in_block, + mined_mmr_position: self.mined_mmr_position.map(|mp| mp as u64), + mined_timestamp: self.mined_timestamp, + marked_deleted_at_height: self.marked_deleted_at_height.map(|d| d as u64), + marked_deleted_in_block, + spending_priority, + source: self.source.try_into()?, + }) + } +} + /// Conversion from an DbUnblindedOutput to the Sql datatype form impl TryFrom for DbUnblindedOutput { type Error = OutputManagerStorageError; From 9e1385f8852887f4170f53f55845b3d0601ee7fa Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 11:30:41 +0000 Subject: [PATCH 2/7] remove encryption --- .../storage/sqlite_db/mod.rs | 207 ++++------------ .../storage/sqlite_db/output_sql.rs | 221 +++--------------- 2 files changed, 73 insertions(+), 355 deletions(-) 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 5976c0e5e4..28a3ab34a5 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 @@ -21,7 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use std::{ - convert::{TryFrom, TryInto}, + convert::TryFrom, sync::{Arc, RwLock}, }; @@ -45,7 +45,6 @@ use tari_core::transactions::transaction_components::{OutputType, TransactionOut use tari_crypto::tari_utilities::{hex::Hex, ByteArray}; use tari_script::{ExecutionStack, TariScript}; use tokio::time::Instant; -use zeroize::Zeroize; use crate::{ output_manager_service::{ @@ -117,7 +116,6 @@ impl OutputManagerSqliteDatabase { return Err(OutputManagerStorageError::DuplicateOutput); } let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, None, None, None)?; - self.encrypt_if_necessary(new_output)?; new_output.commit(conn)? }, DbKeyValuePair::UnspentOutputWithTxId(c, (tx_id, o)) => { @@ -161,12 +159,13 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let result = match key { DbKey::SpentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Spent, &conn) { Ok(mut o) => { self.decrypt_if_necessary(&mut o)?; - Some(DbValue::SpentOutput(Box::new(DbUnblindedOutput::try_from(o)?))) + Some(DbValue::SpentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) }, Err(e) => { match e { @@ -179,7 +178,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { DbKey::UnspentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Unspent, &conn) { Ok(mut o) => { self.decrypt_if_necessary(&mut o)?; - Some(DbValue::UnspentOutput(Box::new(DbUnblindedOutput::try_from(o)?))) + Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) }, Err(e) => { match e { @@ -193,7 +192,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { match OutputSql::find_by_hash(hash.as_slice(), OutputStatus::Unspent, &(*conn)) { Ok(mut o) => { self.decrypt_if_necessary(&mut o)?; - Some(DbValue::UnspentOutput(Box::new(DbUnblindedOutput::try_from(o)?))) + Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) }, Err(e) => { match e { @@ -208,7 +207,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { match OutputSql::find_by_commitment(&commitment.to_vec(), &conn) { Ok(mut o) => { self.decrypt_if_necessary(&mut o)?; - Some(DbValue::AnyOutput(Box::new(DbUnblindedOutput::try_from(o)?))) + Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?))) }, Err(e) => { match e { @@ -227,7 +226,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Some(DbValue::AnyOutputs( outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>()?, )) }, @@ -240,7 +239,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Some(DbValue::UnspentOutputs( outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>()?, )) }, @@ -253,7 +252,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Some(DbValue::SpentOutputs( outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>()?, )) }, @@ -266,7 +265,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Some(DbValue::UnspentOutputs( outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>()?, )) }, @@ -279,7 +278,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Some(DbValue::InvalidOutputs( outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>()?, )) }, @@ -317,26 +316,22 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; let mut outputs = OutputSql::index_by_output_type(output_type, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } + let cipher = acquire_read_lock!(self.cipher).as_ref(); outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } fn fetch_sorted_unspent_outputs(&self) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; let mut outputs = OutputSql::index_unspent(&conn)?; - for output in &mut outputs { - self.decrypt_if_necessary(output)?; - } + let cipher = acquire_read_lock!(self.cipher).as_ref(); outputs .into_iter() - .map(DbUnblindedOutput::try_from) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } @@ -345,9 +340,8 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); let mut outputs = OutputSql::index_marked_deleted_in_block_is_null(&conn)?; - for output in &mut outputs { - self.decrypt_if_necessary(output)?; - } + let cipher = acquire_read_lock!(self.cipher).as_ref(); + if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -360,7 +354,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { outputs .into_iter() - .map(DbUnblindedOutput::try_from) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } @@ -369,9 +363,8 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); let mut outputs = OutputSql::index_unconfirmed(&conn)?; - for output in &mut outputs { - self.decrypt_if_necessary(output)?; - } + let cipher = acquire_read_lock!(self.cipher).as_ref(); + if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -384,7 +377,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { outputs .into_iter() - .map(DbUnblindedOutput::try_from) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } @@ -392,6 +385,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let mut msg = "".to_string(); let result = match op { @@ -409,7 +403,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(mut o) => { o.delete(&conn)?; self.decrypt_if_necessary(&mut o)?; - Ok(Some(DbValue::AnyOutput(Box::new(DbUnblindedOutput::try_from(o)?)))) + Ok(Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?)))) }, Err(e) => match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => Ok(None), @@ -447,6 +441,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let mut outputs = OutputSql::index_status(OutputStatus::EncumberedToBeReceived, &conn)?; outputs.extend(OutputSql::index_status( @@ -454,9 +449,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { &conn, )?); outputs.extend(OutputSql::index_status(OutputStatus::UnspentMinedUnconfirmed, &conn)?); - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } + if start.elapsed().as_millis() > 0 { trace!( target: LOG_TARGET, @@ -468,7 +461,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { } outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } @@ -659,6 +652,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let mut commitments = Vec::with_capacity(outputs_to_send.len()); for output in outputs_to_send { @@ -700,8 +694,8 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { OutputStatus::ShortTermEncumberedToBeReceived, Some(tx_id), None, + cipher, )?; - self.encrypt_if_necessary(&mut new_output)?; new_output.commit(&conn)?; } if start.elapsed().as_millis() > 0 { @@ -787,6 +781,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let output = OutputSql::first_by_mined_height_desc(&conn)?; if start.elapsed().as_millis() > 0 { @@ -799,10 +794,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); } match output { - Some(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Ok(Some(o.try_into()?)) - }, + Some(mut o) => Ok(Some(o.to_db_unblinded_output(cipher)?)), None => Ok(None), } } @@ -811,6 +803,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); let output = OutputSql::first_by_marked_deleted_height_desc(&conn)?; if start.elapsed().as_millis() > 0 { @@ -823,10 +816,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); } match output { - Some(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Ok(Some(o.try_into()?)) - }, + Some(mut o) => Ok(Some(o.to_db_unblinded_output(cipher)?)), None => Ok(None), } } @@ -987,104 +977,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(()) } - fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), OutputManagerStorageError> { - let mut current_cipher = acquire_write_lock!(self.cipher); - - if (*current_cipher).is_some() { - return Err(OutputManagerStorageError::AlreadyEncrypted); - } - - let start = Instant::now(); - let conn = self.database_connection.get_pooled_connection()?; - let acquire_lock = start.elapsed(); - let mut outputs = OutputSql::index(&conn)?; - - // If the db is already encrypted then the very first output we try to encrypt will fail. - for o in &mut outputs { - // Test if this output is encrypted or not to avoid a double encryption. - let _secret_key = PrivateKey::from_vec(&o.spending_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might already be encrypted" - ); - OutputManagerStorageError::AlreadyEncrypted - })?; - o.encrypt(&cipher) - .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; - o.update_encryption(&conn)?; - } - - let mut known_one_sided_payment_scripts = KnownOneSidedPaymentScriptSql::index(&conn)?; - - for script in &mut known_one_sided_payment_scripts { - let _secret_key = PrivateKey::from_vec(&script.private_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might already be encrypted" - ); - OutputManagerStorageError::AlreadyEncrypted - })?; - script - .encrypt(&cipher) - .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; - script.update_encryption(&conn)?; - } - - (*current_cipher) = Some(cipher); - if start.elapsed().as_millis() > 0 { - trace!( - target: LOG_TARGET, - "sqlite profile - apply_encryption: lock {} + db_op {} = {} ms", - acquire_lock.as_millis(), - (start.elapsed() - acquire_lock).as_millis(), - start.elapsed().as_millis() - ); - } - - Ok(()) - } - - fn remove_encryption(&self) -> Result<(), OutputManagerStorageError> { - let mut current_cipher = acquire_write_lock!(self.cipher); - let cipher = if let Some(cipher) = (*current_cipher).clone().take() { - cipher - } else { - return Ok(()); - }; - let start = Instant::now(); - let conn = self.database_connection.get_pooled_connection()?; - let acquire_lock = start.elapsed(); - let mut outputs = OutputSql::index(&conn)?; - - for o in &mut outputs { - o.decrypt(&cipher) - .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; - o.update_encryption(&conn)?; - } - - let mut known_one_sided_payment_scripts = KnownOneSidedPaymentScriptSql::index(&conn)?; - - for script in &mut known_one_sided_payment_scripts { - script - .decrypt(&cipher) - .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; - script.update_encryption(&conn)?; - } - - // Now that all the decryption has been completed we can safely remove the cipher fully - std::mem::drop((*current_cipher).take()); - if start.elapsed().as_millis() > 0 { - trace!( - target: LOG_TARGET, - "sqlite profile - remove_encryption: lock {} + db_op {} = {} ms", - acquire_lock.as_millis(), - (start.elapsed() - acquire_lock).as_millis(), - start.elapsed().as_millis() - ); - } - Ok(()) - } - fn set_coinbase_abandoned(&self, tx_id: TxId, abandoned: bool) -> Result<(), OutputManagerStorageError> { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; @@ -1154,12 +1046,13 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); if OutputSql::find_by_commitment_and_cancelled(&output.commitment.to_vec(), false, &conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new(output, OutputStatus::EncumberedToBeReceived, Some(tx_id), None)?; - self.encrypt_if_necessary(&mut new_output)?; + let mut new_output = + NewOutputSql::new(output, OutputStatus::EncumberedToBeReceived, Some(tx_id), None, cipher)?; new_output.commit(&conn)?; if start.elapsed().as_millis() > 0 { @@ -1184,10 +1077,10 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); + let cipher = acquire_read_lock!(self.cipher).as_ref(); + let mut outputs = OutputSql::fetch_unspent_outputs_for_spending(selection_criteria, amount, tip_height, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } + trace!( target: LOG_TARGET, "sqlite profile - fetch_unspent_outputs_for_spending: lock {} + db_op {} = {} ms", @@ -1197,24 +1090,24 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } fn fetch_outputs_by_tx_id(&self, tx_id: TxId) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; let mut outputs = OutputSql::find_by_tx_id(tx_id, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } + let cipher = acquire_read_lock!(self.cipher).as_ref(); + outputs .iter() - .map(|o| DbUnblindedOutput::try_from(o.clone())) + .map(|o| o.to_db_unblinded_output(cipher)) .collect::, _>>() } fn fetch_outputs_by(&self, q: OutputBackendQuery) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; + let cipher = acquire_read_lock!(self.cipher).as_ref(); Ok(OutputSql::fetch_outputs_by(q, &conn)? .into_iter() .filter_map(|mut x| { @@ -1223,7 +1116,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { return None; } - DbUnblindedOutput::try_from(x) + x.to_db_unblinded_output(cipher) .map_err(|e| { error!( target: LOG_TARGET, @@ -1258,17 +1151,11 @@ fn update_outputs_with_tx_id_and_status_to_new_status( } /// These are the fields that can be updated for an Output -#[derive(Default, Zeroize)] -#[zeroize(drop)] +#[derive(Clone, Default)] pub struct UpdateOutput { - #[zeroize(skip)] status: Option, - #[zeroize(skip)] received_in_tx_id: Option>, - #[zeroize(skip)] spent_in_tx_id: Option>, - spending_key: Option>, - script_private_key: Option>, metadata_signature_ephemeral_commitment: Option>, metadata_signature_ephemeral_pubkey: Option>, metadata_signature_u_a: Option>, @@ -1284,8 +1171,6 @@ pub struct UpdateOutputSql { status: Option, received_in_tx_id: Option>, spent_in_tx_id: Option>, - spending_key: Option>, - script_private_key: Option>, metadata_signature_ephemeral_commitment: Option>, metadata_signature_ephemeral_pubkey: Option>, metadata_signature_u_a: Option>, @@ -1300,8 +1185,6 @@ impl From for UpdateOutputSql { fn from(u: UpdateOutput) -> Self { Self { status: u.status.map(|t| t as i32), - spending_key: u.spending_key, - script_private_key: u.script_private_key, metadata_signature_ephemeral_commitment: u.metadata_signature_ephemeral_commitment, metadata_signature_ephemeral_pubkey: u.metadata_signature_ephemeral_pubkey, metadata_signature_u_a: u.metadata_signature_u_a, 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 6c314bd631..ca7f6bf069 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 chacha20poly1305::{ChaCha20Poly1305, XChaCha20Poly1305}; +use chacha20poly1305::XChaCha20Poly1305; use chrono::NaiveDateTime; use derivative::Derivative; use diesel::{prelude::*, sql_query, SqliteConnection}; @@ -41,6 +41,7 @@ use tari_core::{ }; use tari_crypto::{commitment::HomomorphicCommitmentFactory, tari_utilities::ByteArray}; use tari_script::{ExecutionStack, TariScript}; +use zeroize::Zeroize; use crate::{ output_manager_service::{ @@ -609,24 +610,10 @@ impl OutputSql { OutputSql::find(&self.spending_key, conn) } - /// Update the changed fields of this record after encryption/decryption is performed - pub fn update_encryption(&self, conn: &SqliteConnection) -> Result<(), OutputManagerStorageError> { - let _output_sql = self.update( - UpdateOutput { - spending_key: Some(self.spending_key.clone()), - script_private_key: Some(self.script_private_key.clone()), - ..Default::default() - }, - conn, - )?; - Ok(()) - } -} - -impl OutputSql { + #[allow(clippy::too_many_lines)] pub fn to_db_unblinded_output( mut self, - cipher: Option, + cipher: Option<&XChaCha20Poly1305>, ) -> Result { if let Some(cipher) = cipher { self.decrypt(&cipher); @@ -670,158 +657,8 @@ impl OutputSql { reason: "PrivateKey could not be converted from bytes".to_string(), } })?, - ComSignature::new( - Commitment::from_vec(&self.metadata_signature_nonce).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PublicKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, - PrivateKey::from_vec(&self.metadata_signature_u_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, - PrivateKey::from_vec(&self.metadata_signature_v_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, - ), - self.script_lock_height as u64, - Covenant::from_bytes(&self.covenant).map_err(|e| { - error!( - target: LOG_TARGET, - "Could not create Covenant from stored bytes ({}), They might be encrypted", e - ); - OutputManagerStorageError::ConversionError { - reason: "Covenant could not be converted from bytes".to_string(), - } - })?, - encrypted_value, - MicroTari::from(self.minimum_value_promise as u64), - ); - - // we manually zeroize the sensitive data associated with OutputSql, in order to remove potential leaks - use zeroize::{Zeroize, Zeroizing}; - Zeroizing::new(self.spending_key).zeroize(); - Zeroizing::new(self.script_private_key).zeroize(); - self.spending_key = self.spending_key.iter().map(|x| 0u8).collect::>(); - self.script_private_key = self.script_private_key.iter().map(|x| 0u8).collect::>(); - - let hash = match self.hash { - None => { - let factories = CryptoFactories::default(); - unblinded_output.as_transaction_output(&factories)?.hash() - }, - Some(v) => match v.try_into() { - Ok(v) => v, - Err(e) => { - error!(target: LOG_TARGET, "Malformed transaction hash: {}", e); - return Err(OutputManagerStorageError::ConversionError { - reason: "Malformed transaction hash".to_string(), - }); - }, - }, - }; - let commitment = match self.commitment { - None => { - let factories = CryptoFactories::default(); - factories - .commitment - .commit(&unblinded_output.spending_key, &unblinded_output.value.into()) - }, - Some(c) => Commitment::from_vec(&c)?, - }; - let spending_priority = (self.spending_priority as u32).into(); - let mined_in_block = match self.mined_in_block { - Some(v) => match v.try_into() { - Ok(v) => Some(v), - Err(_) => None, - }, - None => None, - }; - let marked_deleted_in_block = match self.marked_deleted_in_block { - Some(v) => match v.try_into() { - Ok(v) => Some(v), - Err(_) => None, - }, - None => None, - }; - Ok(DbUnblindedOutput { - commitment, - unblinded_output, - hash, - status: self.status.try_into()?, - mined_height: self.mined_height.map(|mh| mh as u64), - mined_in_block, - mined_mmr_position: self.mined_mmr_position.map(|mp| mp as u64), - mined_timestamp: self.mined_timestamp, - marked_deleted_at_height: self.marked_deleted_at_height.map(|d| d as u64), - marked_deleted_in_block, - spending_priority, - source: self.source.try_into()?, - }) - } -} - -/// Conversion from an DbUnblindedOutput to the Sql datatype form -impl TryFrom for DbUnblindedOutput { - type Error = OutputManagerStorageError; - - #[allow(clippy::too_many_lines)] - fn try_from(o: OutputSql) -> Result { - let features: OutputFeatures = - serde_json::from_str(&o.features_json).map_err(|s| OutputManagerStorageError::ConversionError { - reason: format!("Could not convert json into OutputFeatures:{}", s), - })?; - - let encrypted_value = EncryptedValue::from_bytes(&o.encrypted_value)?; - let unblinded_output = UnblindedOutput::new_current_version( - MicroTari::from(o.value as u64), - PrivateKey::from_vec(&o.spending_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, - features, - TariScript::from_bytes(o.script.as_slice())?, - ExecutionStack::from_bytes(o.input_data.as_slice())?, - PrivateKey::from_vec(&o.script_private_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PrivateKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, - PublicKey::from_vec(&o.sender_offset_public_key).map_err(|_| { - error!( - target: LOG_TARGET, - "Could not create PublicKey from stored bytes, They might be encrypted" - ); - OutputManagerStorageError::ConversionError { - reason: "PrivateKey could not be converted from bytes".to_string(), - } - })?, ComAndPubSignature::new( - Commitment::from_vec(&o.metadata_signature_ephemeral_commitment).map_err(|_| { + Commitment::from_vec(&self.metadata_signature_ephemeral_commitment).map_err(|_| { error!( target: LOG_TARGET, "Could not create Commitment from stored bytes, They might be encrypted" @@ -830,7 +667,7 @@ impl TryFrom for DbUnblindedOutput { reason: "Commitment could not be converted from bytes".to_string(), } })?, - PublicKey::from_vec(&o.metadata_signature_ephemeral_pubkey).map_err(|_| { + PublicKey::from_vec(&self.metadata_signature_ephemeral_pubkey).map_err(|_| { error!( target: LOG_TARGET, "Could not create PublicKey from stored bytes, They might be encrypted" @@ -839,7 +676,7 @@ impl TryFrom for DbUnblindedOutput { reason: "PublicKey could not be converted from bytes".to_string(), } })?, - PrivateKey::from_vec(&o.metadata_signature_u_a).map_err(|_| { + PrivateKey::from_vec(&self.metadata_signature_u_a).map_err(|_| { error!( target: LOG_TARGET, "Could not create PrivateKey from stored bytes, They might be encrypted" @@ -848,7 +685,7 @@ impl TryFrom for DbUnblindedOutput { reason: "PrivateKey could not be converted from bytes".to_string(), } })?, - PrivateKey::from_vec(&o.metadata_signature_u_x).map_err(|_| { + PrivateKey::from_vec(&self.metadata_signature_u_x).map_err(|_| { error!( target: LOG_TARGET, "Could not create PrivateKey from stored bytes, They might be encrypted" @@ -857,7 +694,7 @@ impl TryFrom for DbUnblindedOutput { reason: "PrivateKey could not be converted from bytes".to_string(), } })?, - PrivateKey::from_vec(&o.metadata_signature_u_y).map_err(|_| { + PrivateKey::from_vec(&self.metadata_signature_u_y).map_err(|_| { error!( target: LOG_TARGET, "Could not create PrivateKey from stored bytes, They might be encrypted" @@ -867,8 +704,8 @@ impl TryFrom for DbUnblindedOutput { } })?, ), - o.script_lock_height as u64, - Covenant::from_bytes(&o.covenant).map_err(|e| { + self.script_lock_height as u64, + Covenant::from_bytes(&self.covenant).map_err(|e| { error!( target: LOG_TARGET, "Could not create Covenant from stored bytes ({}), They might be encrypted", e @@ -878,10 +715,14 @@ impl TryFrom for DbUnblindedOutput { } })?, encrypted_value, - MicroTari::from(o.minimum_value_promise as u64), + MicroTari::from(self.minimum_value_promise as u64), ); - let hash = match o.hash { + // we manually zeroize the sensitive data associated with OuptputSql, to avoid any leaks + self.spending_key.zeroize(); + self.script_private_key.zeroize(); + + let hash = match self.hash { None => { let factories = CryptoFactories::default(); unblinded_output.as_transaction_output(&factories)?.hash() @@ -896,7 +737,7 @@ impl TryFrom for DbUnblindedOutput { }, }, }; - let commitment = match o.commitment { + let commitment = match self.commitment { None => { let factories = CryptoFactories::default(); factories @@ -905,34 +746,34 @@ impl TryFrom for DbUnblindedOutput { }, Some(c) => Commitment::from_vec(&c)?, }; - let spending_priority = (o.spending_priority as u32).into(); - let mined_in_block = match o.mined_in_block { + let spending_priority = (self.spending_priority as u32).into(); + let mined_in_block = match self.mined_in_block { Some(v) => match v.try_into() { Ok(v) => Some(v), Err(_) => None, }, None => None, }; - let marked_deleted_in_block = match o.marked_deleted_in_block { + let marked_deleted_in_block = match self.marked_deleted_in_block { Some(v) => match v.try_into() { Ok(v) => Some(v), Err(_) => None, }, None => None, }; - Ok(Self { + Ok(DbUnblindedOutput { commitment, unblinded_output, hash, - status: o.status.try_into()?, - mined_height: o.mined_height.map(|mh| mh as u64), + status: self.status.try_into()?, + mined_height: self.mined_height.map(|mh| mh as u64), mined_in_block, - mined_mmr_position: o.mined_mmr_position.map(|mp| mp as u64), - mined_timestamp: o.mined_timestamp, - marked_deleted_at_height: o.marked_deleted_at_height.map(|d| d as u64), + mined_mmr_position: self.mined_mmr_position.map(|mp| mp as u64), + mined_timestamp: self.mined_timestamp, + marked_deleted_at_height: self.marked_deleted_at_height.map(|d| d as u64), marked_deleted_in_block, spending_priority, - source: o.source.try_into()?, + source: self.source.try_into()?, }) } } @@ -971,9 +812,3 @@ impl Encryptable for OutputSql { Ok(()) } } - -// impl PartialEq for OutputSql { -// fn eq(&self, other: &NewOutputSql) -> bool { -// &NewOutputSql::from(self.clone()) == other -// } -// } From 416225daca594331fe5a4fadca738000c8915379 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 11:43:01 +0000 Subject: [PATCH 3/7] remove all instances of decryption/encryption of output sql --- .../storage/sqlite_db/mod.rs | 53 +++++-------------- 1 file changed, 13 insertions(+), 40 deletions(-) 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 28a3ab34a5..a1c4954031 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 @@ -153,6 +153,14 @@ impl OutputManagerSqliteDatabase { } impl OutputManagerBackend for OutputManagerSqliteDatabase { + fn apply_encryption(&self, _cipher: XChaCha20Poly1305) -> Result<(), OutputManagerStorageError> { + Ok(()) + } + + fn remove_encryption(&self) -> Result<(), OutputManagerStorageError> { + Ok(()) + } + #[allow(clippy::cognitive_complexity)] #[allow(clippy::too_many_lines)] fn fetch(&self, key: &DbKey) -> Result, OutputManagerStorageError> { @@ -163,10 +171,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let result = match key { DbKey::SpentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Spent, &conn) { - Ok(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Some(DbValue::SpentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) - }, + Ok(mut o) => Some(DbValue::SpentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -176,10 +181,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, }, DbKey::UnspentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Unspent, &conn) { - Ok(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) - }, + Ok(mut o) => Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -190,10 +192,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::UnspentOutputHash(hash) => { match OutputSql::find_by_hash(hash.as_slice(), OutputStatus::Unspent, &(*conn)) { - Ok(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))) - }, + Ok(mut o) => Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -205,10 +204,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::AnyOutputByCommitment(commitment) => { match OutputSql::find_by_commitment(&commitment.to_vec(), &conn) { - Ok(mut o) => { - self.decrypt_if_necessary(&mut o)?; - Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?))) - }, + Ok(mut o) => Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -220,9 +216,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::OutputsByTxIdAndStatus(tx_id, status) => { let mut outputs = OutputSql::find_by_tx_id_and_status(*tx_id, *status, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } + Some(DbValue::AnyOutputs( outputs .iter() @@ -232,9 +226,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::UnspentOutputs => { let mut outputs = OutputSql::index_status(OutputStatus::Unspent, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } Some(DbValue::UnspentOutputs( outputs @@ -245,9 +236,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::SpentOutputs => { let mut outputs = OutputSql::index_status(OutputStatus::Spent, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } Some(DbValue::SpentOutputs( outputs @@ -258,9 +246,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::TimeLockedUnspentOutputs(tip) => { let mut outputs = OutputSql::index_time_locked(*tip, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } Some(DbValue::UnspentOutputs( outputs @@ -271,9 +256,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::InvalidOutputs => { let mut outputs = OutputSql::index_status(OutputStatus::Invalid, &conn)?; - for o in &mut outputs { - self.decrypt_if_necessary(o)?; - } Some(DbValue::InvalidOutputs( outputs @@ -284,9 +266,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::KnownOneSidedPaymentScripts => { let mut known_one_sided_payment_scripts = KnownOneSidedPaymentScriptSql::index(&conn)?; - for script in &mut known_one_sided_payment_scripts { - self.decrypt_if_necessary(script)?; - } Some(DbValue::KnownOneSidedPaymentScripts( known_one_sided_payment_scripts @@ -402,7 +381,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { match OutputSql::find_by_commitment(&commitment.to_vec(), &conn) { Ok(mut o) => { o.delete(&conn)?; - self.decrypt_if_necessary(&mut o)?; Ok(Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?)))) }, Err(e) => match e { @@ -1111,11 +1089,6 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { Ok(OutputSql::fetch_outputs_by(q, &conn)? .into_iter() .filter_map(|mut x| { - if let Err(e) = self.decrypt_if_necessary(&mut x) { - error!(target: LOG_TARGET, "failed to `decrypt_if_necessary`: {:#?}", e); - return None; - } - x.to_db_unblinded_output(cipher) .map_err(|e| { error!( From d60aa6e11e0bad61c556669f7499ca791fe4b774 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 17:11:21 +0000 Subject: [PATCH 4/7] compiler checks --- .../storage/sqlite_db/mod.rs | 334 +++++++----------- .../storage/sqlite_db/new_output_sql.rs | 1 - .../storage/sqlite_db/output_sql.rs | 3 +- 3 files changed, 136 insertions(+), 202 deletions(-) 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 a1c4954031..4a9ae2620c 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 @@ -45,6 +45,7 @@ use tari_core::transactions::transaction_components::{OutputType, TransactionOut use tari_crypto::tari_utilities::{hex::Hex, ByteArray}; use tari_script::{ExecutionStack, TariScript}; use tokio::time::Instant; +use zeroize::Zeroize; use crate::{ output_manager_service::{ @@ -83,68 +84,44 @@ impl OutputManagerSqliteDatabase { } } - 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) - .map_err(|_| OutputManagerStorageError::AeadError("Decryption Error".to_string()))?; - } - Ok(()) - } - - 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) - .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; - } - Ok(()) - } - fn insert(&self, key_value_pair: DbKeyValuePair, conn: &SqliteConnection) -> Result<(), OutputManagerStorageError> { - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); match key_value_pair { DbKeyValuePair::UnspentOutput(c, o) => { if OutputSql::find_by_commitment_and_cancelled(&c.to_vec(), false, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, None, None, None)?; + let new_output = NewOutputSql::new(*o, OutputStatus::Unspent, None, None, cipher.as_ref())?; new_output.commit(conn)? }, DbKeyValuePair::UnspentOutputWithTxId(c, (tx_id, o)) => { if OutputSql::find_by_commitment_and_cancelled(&c.to_vec(), false, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new(*o, OutputStatus::Unspent, Some(tx_id), None, cipher)?; + let new_output = NewOutputSql::new(*o, OutputStatus::Unspent, Some(tx_id), None, cipher.as_ref())?; new_output.commit(conn)? }, DbKeyValuePair::OutputToBeReceived(c, (tx_id, o, coinbase_block_height)) => { if OutputSql::find_by_commitment_and_cancelled(&c.to_vec(), false, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = NewOutputSql::new( + let new_output = NewOutputSql::new( *o, OutputStatus::EncumberedToBeReceived, Some(tx_id), coinbase_block_height, - cipher, + cipher.as_ref(), )?; new_output.commit(conn)? }, DbKeyValuePair::KnownOneSidedPaymentScripts(script) => { - let mut script_sql = KnownOneSidedPaymentScriptSql::from(script); + let script_sql = + KnownOneSidedPaymentScriptSql::from_known_one_sided_payment_script(script, cipher.as_ref())?; if KnownOneSidedPaymentScriptSql::find(&script_sql.script_hash, conn).is_ok() { return Err(OutputManagerStorageError::DuplicateScript); } - self.encrypt_if_necessary(&mut script_sql)?; script_sql.commit(conn)? }, } @@ -167,11 +144,13 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let result = match key { DbKey::SpentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Spent, &conn) { - Ok(mut o) => Some(DbValue::SpentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), + Ok(o) => Some(DbValue::SpentOutput(Box::new( + o.to_db_unblinded_output(cipher.as_ref())?, + ))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -181,7 +160,9 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, }, DbKey::UnspentOutput(k) => match OutputSql::find_status(&k.to_vec(), OutputStatus::Unspent, &conn) { - Ok(mut o) => Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), + Ok(o) => Some(DbValue::UnspentOutput(Box::new( + o.to_db_unblinded_output(cipher.as_ref())?, + ))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -192,7 +173,9 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::UnspentOutputHash(hash) => { match OutputSql::find_by_hash(hash.as_slice(), OutputStatus::Unspent, &(*conn)) { - Ok(mut o) => Some(DbValue::UnspentOutput(Box::new(o.to_db_unblinded_output(cipher)?))), + Ok(o) => Some(DbValue::UnspentOutput(Box::new( + o.to_db_unblinded_output(cipher.as_ref())?, + ))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -204,7 +187,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { }, DbKey::AnyOutputByCommitment(commitment) => { match OutputSql::find_by_commitment(&commitment.to_vec(), &conn) { - Ok(mut o) => Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?))), + Ok(o) => Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher.as_ref())?))), Err(e) => { match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => (), @@ -215,62 +198,62 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { } }, DbKey::OutputsByTxIdAndStatus(tx_id, status) => { - let mut outputs = OutputSql::find_by_tx_id_and_status(*tx_id, *status, &conn)?; + let outputs = OutputSql::find_by_tx_id_and_status(*tx_id, *status, &conn)?; Some(DbValue::AnyOutputs( outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>()?, )) }, DbKey::UnspentOutputs => { - let mut outputs = OutputSql::index_status(OutputStatus::Unspent, &conn)?; + let outputs = OutputSql::index_status(OutputStatus::Unspent, &conn)?; Some(DbValue::UnspentOutputs( outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>()?, )) }, DbKey::SpentOutputs => { - let mut outputs = OutputSql::index_status(OutputStatus::Spent, &conn)?; + let outputs = OutputSql::index_status(OutputStatus::Spent, &conn)?; Some(DbValue::SpentOutputs( outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>()?, )) }, DbKey::TimeLockedUnspentOutputs(tip) => { - let mut outputs = OutputSql::index_time_locked(*tip, &conn)?; + let outputs = OutputSql::index_time_locked(*tip, &conn)?; Some(DbValue::UnspentOutputs( outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>()?, )) }, DbKey::InvalidOutputs => { - let mut outputs = OutputSql::index_status(OutputStatus::Invalid, &conn)?; + let outputs = OutputSql::index_status(OutputStatus::Invalid, &conn)?; Some(DbValue::InvalidOutputs( outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>()?, )) }, DbKey::KnownOneSidedPaymentScripts => { - let mut known_one_sided_payment_scripts = KnownOneSidedPaymentScriptSql::index(&conn)?; + let known_one_sided_payment_scripts = KnownOneSidedPaymentScriptSql::index(&conn)?; Some(DbValue::KnownOneSidedPaymentScripts( known_one_sided_payment_scripts .iter() - .map(|script| KnownOneSidedPaymentScript::try_from(script.clone())) + .map(|script| script.clone().to_known_one_sided_payment_script(cipher.as_ref())) .collect::, _>>()?, )) }, @@ -294,23 +277,23 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { output_type: OutputType, ) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; - let mut outputs = OutputSql::index_by_output_type(output_type, &conn)?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let outputs = OutputSql::index_by_output_type(output_type, &conn)?; + let cipher = acquire_read_lock!(self.cipher); outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } fn fetch_sorted_unspent_outputs(&self) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; - let mut outputs = OutputSql::index_unspent(&conn)?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let outputs = OutputSql::index_unspent(&conn)?; + let cipher = acquire_read_lock!(self.cipher); outputs .into_iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } @@ -318,8 +301,8 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let mut outputs = OutputSql::index_marked_deleted_in_block_is_null(&conn)?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let outputs = OutputSql::index_marked_deleted_in_block_is_null(&conn)?; + let cipher = acquire_read_lock!(self.cipher); if start.elapsed().as_millis() > 0 { trace!( @@ -333,7 +316,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { outputs .into_iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } @@ -341,8 +324,8 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let mut outputs = OutputSql::index_unconfirmed(&conn)?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let outputs = OutputSql::index_unconfirmed(&conn)?; + let cipher = acquire_read_lock!(self.cipher); if start.elapsed().as_millis() > 0 { trace!( @@ -356,7 +339,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { outputs .into_iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } @@ -364,7 +347,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let mut msg = "".to_string(); let result = match op { @@ -379,9 +362,11 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { msg.push_str("Remove"); // Used by coinbase when mining. match OutputSql::find_by_commitment(&commitment.to_vec(), &conn) { - Ok(mut o) => { + Ok(o) => { o.delete(&conn)?; - Ok(Some(DbValue::AnyOutput(Box::new(o.to_db_unblinded_output(cipher)?)))) + Ok(Some(DbValue::AnyOutput(Box::new( + o.to_db_unblinded_output(cipher.as_ref())?, + )))) }, Err(e) => match e { OutputManagerStorageError::DieselError(DieselError::NotFound) => Ok(None), @@ -419,7 +404,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let mut outputs = OutputSql::index_status(OutputStatus::EncumberedToBeReceived, &conn)?; outputs.extend(OutputSql::index_status( @@ -439,7 +424,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { } outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } @@ -630,7 +615,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let mut commitments = Vec::with_capacity(outputs_to_send.len()); for output in outputs_to_send { @@ -667,12 +652,12 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { })?; for co in outputs_to_receive { - let mut new_output = NewOutputSql::new( + let new_output = NewOutputSql::new( co.clone(), OutputStatus::ShortTermEncumberedToBeReceived, Some(tx_id), None, - cipher, + cipher.as_ref(), )?; new_output.commit(&conn)?; } @@ -759,7 +744,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let output = OutputSql::first_by_mined_height_desc(&conn)?; if start.elapsed().as_millis() > 0 { @@ -772,7 +757,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); } match output { - Some(mut o) => Ok(Some(o.to_db_unblinded_output(cipher)?)), + Some(o) => Ok(Some(o.to_db_unblinded_output(cipher.as_ref())?)), None => Ok(None), } } @@ -781,7 +766,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); let output = OutputSql::first_by_marked_deleted_height_desc(&conn)?; if start.elapsed().as_millis() > 0 { @@ -794,7 +779,7 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); } match output { - Some(mut o) => Ok(Some(o.to_db_unblinded_output(cipher)?)), + Some(o) => Ok(Some(o.to_db_unblinded_output(cipher.as_ref())?)), None => Ok(None), } } @@ -1024,13 +1009,18 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); if OutputSql::find_by_commitment_and_cancelled(&output.commitment.to_vec(), false, &conn).is_ok() { return Err(OutputManagerStorageError::DuplicateOutput); } - let mut new_output = - NewOutputSql::new(output, OutputStatus::EncumberedToBeReceived, Some(tx_id), None, cipher)?; + let new_output = NewOutputSql::new( + output, + OutputStatus::EncumberedToBeReceived, + Some(tx_id), + None, + cipher.as_ref(), + )?; new_output.commit(&conn)?; if start.elapsed().as_millis() > 0 { @@ -1055,9 +1045,9 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { let start = Instant::now(); let conn = self.database_connection.get_pooled_connection()?; let acquire_lock = start.elapsed(); - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); - let mut outputs = OutputSql::fetch_unspent_outputs_for_spending(selection_criteria, amount, tip_height, &conn)?; + let outputs = OutputSql::fetch_unspent_outputs_for_spending(selection_criteria, amount, tip_height, &conn)?; trace!( target: LOG_TARGET, @@ -1068,28 +1058,28 @@ impl OutputManagerBackend for OutputManagerSqliteDatabase { ); outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } fn fetch_outputs_by_tx_id(&self, tx_id: TxId) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; - let mut outputs = OutputSql::find_by_tx_id(tx_id, &conn)?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let outputs = OutputSql::find_by_tx_id(tx_id, &conn)?; + let cipher = acquire_read_lock!(self.cipher); outputs .iter() - .map(|o| o.to_db_unblinded_output(cipher)) + .map(|o| o.clone().to_db_unblinded_output(cipher.as_ref())) .collect::, _>>() } fn fetch_outputs_by(&self, q: OutputBackendQuery) -> Result, OutputManagerStorageError> { let conn = self.database_connection.get_pooled_connection()?; - let cipher = acquire_read_lock!(self.cipher).as_ref(); + let cipher = acquire_read_lock!(self.cipher); Ok(OutputSql::fetch_outputs_by(q, &conn)? .into_iter() - .filter_map(|mut x| { - x.to_db_unblinded_output(cipher) + .filter_map(|x| { + x.to_db_unblinded_output(cipher.as_ref()) .map_err(|e| { error!( target: LOG_TARGET, @@ -1189,7 +1179,6 @@ pub struct KnownOneSidedPaymentScriptSql { #[derive(AsChangeset)] #[table_name = "known_one_sided_payment_scripts"] pub struct UpdateKnownOneSidedPaymentScript { - private_key: Option>, script: Option>, input: Option>, } @@ -1248,27 +1237,18 @@ impl KnownOneSidedPaymentScriptSql { KnownOneSidedPaymentScriptSql::find(&self.script_hash, conn) } - /// Update the changed fields of this record after encryption/decryption is performed - pub fn update_encryption(&self, conn: &SqliteConnection) -> Result<(), OutputManagerStorageError> { - let _known_one_sided_payment_script_sql = self.update( - UpdateKnownOneSidedPaymentScript { - private_key: Some(self.private_key.clone()), - script: None, - input: None, - }, - conn, - )?; - Ok(()) - } -} - -/// Conversion from an KnownOneSidedPaymentScript to the Sql datatype form -impl TryFrom for KnownOneSidedPaymentScript { - type Error = OutputManagerStorageError; + /// Conversion from an KnownOneSidedPaymentScriptSQL to the datatype form + pub fn to_known_one_sided_payment_script( + mut self, + cipher: Option<&XChaCha20Poly1305>, + ) -> Result { + if let Some(cipher) = cipher { + self.decrypt(cipher) + .map_err(|e| OutputManagerStorageError::AeadError(e))?; + } - fn try_from(o: KnownOneSidedPaymentScriptSql) -> Result { - let script_hash = o.script_hash; - let private_key = PrivateKey::from_bytes(&o.private_key).map_err(|_| { + let script_hash = self.script_hash; + let private_key = PrivateKey::from_bytes(&self.private_key).map_err(|_| { error!( target: LOG_TARGET, "Could not create PrivateKey from stored bytes, They might be encrypted" @@ -1277,19 +1257,24 @@ impl TryFrom for KnownOneSidedPaymentScript { reason: "PrivateKey could not be converted from bytes".to_string(), } })?; - let script = TariScript::from_bytes(&o.script).map_err(|_| { + + // in order to avoid memory leaks of sensitive data, we zeroize the current private key buffer + self.private_key.zeroize(); + + let script = TariScript::from_bytes(&self.script).map_err(|_| { error!(target: LOG_TARGET, "Could not create tari script from stored bytes"); OutputManagerStorageError::ConversionError { reason: "Tari Script could not be converted from bytes".to_string(), } })?; - let input = ExecutionStack::from_bytes(&o.input).map_err(|_| { + let input = ExecutionStack::from_bytes(&self.input).map_err(|_| { error!(target: LOG_TARGET, "Could not create execution stack from stored bytes"); OutputManagerStorageError::ConversionError { reason: "ExecutionStack could not be converted from bytes".to_string(), } })?; - let script_lock_height = o.script_lock_height as u64; + let script_lock_height = self.script_lock_height as u64; + Ok(KnownOneSidedPaymentScript { script_hash, private_key, @@ -1298,23 +1283,33 @@ impl TryFrom for KnownOneSidedPaymentScript { script_lock_height, }) } -} -/// Conversion from an KnownOneSidedPaymentScriptSQL to the datatype form -impl From for KnownOneSidedPaymentScriptSql { - fn from(known_script: KnownOneSidedPaymentScript) -> Self { + /// Conversion from an KnownOneSidedPaymentScriptSQL to the datatype form + pub fn from_known_one_sided_payment_script( + known_script: KnownOneSidedPaymentScript, + cipher: Option<&XChaCha20Poly1305>, + ) -> Result { let script_lock_height = known_script.script_lock_height as i64; let script_hash = known_script.script_hash; let private_key = known_script.private_key.as_bytes().to_vec(); let script = known_script.script.to_bytes().to_vec(); let input = known_script.input.to_bytes().to_vec(); - KnownOneSidedPaymentScriptSql { + + let mut output = KnownOneSidedPaymentScriptSql { script_hash, private_key, script, input, script_lock_height, + }; + + // encrypt in place the output, so no private_key memory leaks remain + if let Some(cipher) = cipher { + output + .encrypt(cipher) + .map_err(|e| OutputManagerStorageError::AeadError(e))?; } + Ok(output) } } @@ -1342,12 +1337,11 @@ impl Encryptable for KnownOneSidedPaymentScriptSql { #[cfg(test)] mod test { - use std::{mem::size_of, time::Duration}; + use std::mem::size_of; use chacha20poly1305::{aead::NewAead, Key, XChaCha20Poly1305}; use diesel::{Connection, SqliteConnection}; use rand::{rngs::OsRng, RngCore}; - use tari_common_sqlite::sqlite_connection_pool::SqliteConnectionPool; use tari_common_types::types::CommitmentFactory; use tari_core::transactions::{ tari_amount::MicroTari, @@ -1357,22 +1351,15 @@ mod test { }; use tari_script::script; use tari_test_utils::random; + use tari_utilities::ByteArray; use tempfile::tempdir; use crate::{ output_manager_service::storage::{ - database::{DbKey, OutputManagerBackend}, models::DbUnblindedOutput, - sqlite_db::{ - new_output_sql::NewOutputSql, - output_sql::OutputSql, - OutputManagerSqliteDatabase, - OutputStatus, - UpdateOutput, - }, + sqlite_db::{new_output_sql::NewOutputSql, output_sql::OutputSql, OutputStatus, UpdateOutput}, OutputSource, }, - storage::sqlite_utilities::wallet_db_connection::WalletDbConnection, util::encryption::Encryptable, }; @@ -1409,7 +1396,7 @@ mod test { for _i in 0..2 { let (_, uo) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None, OutputSource::Unknown).unwrap(); - let o = NewOutputSql::new(uo, OutputStatus::Unspent, None, None).unwrap(); + let o = NewOutputSql::new(uo, OutputStatus::Unspent, None, None, None).unwrap(); outputs.push(o.clone()); outputs_unspent.push(o.clone()); o.commit(&conn).unwrap(); @@ -1418,7 +1405,7 @@ mod test { for _i in 0..3 { let (_, uo) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None, OutputSource::Unknown).unwrap(); - let o = NewOutputSql::new(uo, OutputStatus::Spent, None, None).unwrap(); + let o = NewOutputSql::new(uo, OutputStatus::Spent, None, None, None).unwrap(); outputs.push(o.clone()); outputs_spent.push(o.clone()); o.commit(&conn).unwrap(); @@ -1517,35 +1504,39 @@ mod test { conn.execute("PRAGMA foreign_keys = ON").unwrap(); let factories = CryptoFactories::default(); - let (_, uo) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); - let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None, OutputSource::Unknown).unwrap(); - let output = NewOutputSql::new(uo, OutputStatus::Unspent, None, None).unwrap(); - 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 (_, uo) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); + let decrypted_spending_key = uo.spending_key.clone().to_vec(); + + let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None, OutputSource::Unknown).unwrap(); + + let mut output = NewOutputSql::new(uo, OutputStatus::Unspent, None, None, Some(&cipher)).unwrap(); + output.commit(&conn).unwrap(); - let unencrypted_output = OutputSql::find(output.spending_key.as_slice(), &conn).unwrap(); + let mut encrypted_output = OutputSql::find(output.spending_key.as_slice(), &conn).unwrap(); - assert!(unencrypted_output.clone().decrypt(&cipher).is_err()); - unencrypted_output.delete(&conn).unwrap(); + // Aead encryption of spending key contains 24 bytes nonce + 16 bytes tag + 32 bytes encrypted spneding key + assert_eq!(encrypted_output.spending_key.len(), 32 + 24 + 16); + assert_eq!(encrypted_output.spending_key, output.spending_key); - let mut encrypted_output = output.clone(); - encrypted_output.encrypt(&cipher).unwrap(); - encrypted_output.commit(&conn).unwrap(); + let mut decrypted_output = encrypted_output.clone(); - let outputs = OutputSql::index(&conn).unwrap(); - let mut decrypted_output = outputs[0].clone(); decrypted_output.decrypt(&cipher).unwrap(); - assert_eq!(decrypted_output.spending_key, output.spending_key); + assert_eq!(decrypted_output.spending_key.len(), 32); + assert_eq!(decrypted_output.spending_key, decrypted_spending_key); + + let mut output_2 = output.clone(); + output_2.decrypt(&cipher).unwrap(); + assert_eq!(decrypted_output.spending_key, output_2.spending_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(); + assert!(encrypted_output.decrypt(&wrong_cipher).is_err()); + assert!(output.decrypt(&wrong_cipher).is_err()); assert_eq!( OutputSql::find(output.spending_key.as_slice(), &conn) @@ -1554,66 +1545,9 @@ mod test { output.spending_key ); - decrypted_output.encrypt(&cipher).unwrap(); - decrypted_output.update_encryption(&conn).unwrap(); - let outputs = OutputSql::index(&conn).unwrap(); let mut decrypted_output2 = outputs[0].clone(); decrypted_output2.decrypt(&cipher).unwrap(); - assert_eq!(decrypted_output2.spending_key, output.spending_key); - } - - #[test] - fn test_apply_remove_encryption() { - let db_name = format!("{}.sqlite3", random::string(8).as_str()); - let temp_dir = tempdir().unwrap(); - let db_folder = temp_dir.path().to_str().unwrap().to_string(); - let db_path = format!("{}{}", db_folder, db_name); - - embed_migrations!("./migrations"); - let mut pool = SqliteConnectionPool::new(db_path.clone(), 1, true, true, Duration::from_secs(60)); - pool.create_pool() - .unwrap_or_else(|_| panic!("Error connecting to {}", db_path)); - // Note: For this test the connection pool is setup with a pool size of one; the pooled connection must go out - // of scope to be released once obtained otherwise subsequent calls to obtain a pooled connection will fail . - { - let conn = pool - .get_pooled_connection() - .unwrap_or_else(|_| panic!("Error connecting to {}", db_path)); - - embedded_migrations::run_with_output(&conn, &mut std::io::stdout()).expect("Migration failed"); - let factories = CryptoFactories::default(); - - let (_, uo) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); - let uo = DbUnblindedOutput::from_unblinded_output(uo, &factories, None, OutputSource::Unknown).unwrap(); - let output = NewOutputSql::new(uo, OutputStatus::Unspent, None, None).unwrap(); - output.commit(&conn).unwrap(); - - let (_, uo2) = make_input(MicroTari::from(100 + OsRng.next_u64() % 1000)); - let uo2 = DbUnblindedOutput::from_unblinded_output(uo2, &factories, None, OutputSource::Unknown).unwrap(); - let output2 = NewOutputSql::new(uo2, OutputStatus::Unspent, None, None).unwrap(); - output2.commit(&conn).unwrap(); - } - - 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); - - let db1 = OutputManagerSqliteDatabase::new(connection.clone(), Some(cipher.clone())); - assert!(db1.apply_encryption(cipher.clone()).is_err()); - - let db2 = OutputManagerSqliteDatabase::new(connection.clone(), None); - assert!(db2.remove_encryption().is_ok()); - db2.apply_encryption(cipher).unwrap(); - - let db3 = OutputManagerSqliteDatabase::new(connection, None); - assert!(db3.fetch(&DbKey::UnspentOutputs).is_err()); - - db2.remove_encryption().unwrap(); - - assert!(db3.fetch(&DbKey::UnspentOutputs).is_ok()); + assert_eq!(decrypted_output2.spending_key, decrypted_output.spending_key); } } 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 f8530f8d28..a104da2ac5 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 @@ -117,7 +117,6 @@ impl NewOutputSql { .encrypt(cipher) .map_err(|_| OutputManagerStorageError::AeadError("Encryption Error".to_string()))?; } - Ok(output) } 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 ca7f6bf069..1dd2eb707e 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 @@ -616,7 +616,8 @@ impl OutputSql { cipher: Option<&XChaCha20Poly1305>, ) -> Result { if let Some(cipher) = cipher { - self.decrypt(&cipher); + self.decrypt(&cipher) + .map_err(|e| OutputManagerStorageError::AeadError(e))?; } let features: OutputFeatures = From 61a0a5a704aa62ebf4260cd27552cdcf156b3530 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 18:26:29 +0000 Subject: [PATCH 5/7] add zeroize to plaintext in encrypt_bytes_integral_nonce --- base_layer/wallet/src/util/encryption.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/base_layer/wallet/src/util/encryption.rs b/base_layer/wallet/src/util/encryption.rs index 915fa163e1..697b42ab2a 100644 --- a/base_layer/wallet/src/util/encryption.rs +++ b/base_layer/wallet/src/util/encryption.rs @@ -30,6 +30,7 @@ use chacha20poly1305::{ }; use rand::{rngs::OsRng, RngCore}; use tari_utilities::ByteArray; +use zeroize::Zeroize; pub trait Encryptable { const KEY_MANAGER: &'static [u8] = b"KEY_MANAGER"; @@ -77,7 +78,7 @@ pub fn decrypt_bytes_integral_nonce( pub fn encrypt_bytes_integral_nonce( cipher: &XChaCha20Poly1305, domain: Vec, - plaintext: Vec, + mut plaintext: Vec, ) -> Result, String> { // Produce a secure random nonce let mut nonce = [0u8; size_of::()]; @@ -86,13 +87,16 @@ pub fn encrypt_bytes_integral_nonce( // Bind the domain as additional data let payload = Payload { - msg: plaintext.as_bytes(), - aad: domain.as_bytes(), + msg: plaintext.as_slice(), + aad: domain.as_slice(), }; // Attempt authenticated encryption let mut ciphertext = cipher.encrypt(nonce_ga, payload).map_err(|e| e.to_string())?; + // zeroize plaintext to avoid leaking sensitive data + plaintext.zeroize(); + // Concatenate the nonce and ciphertext (which already include the tag) let mut ciphertext_integral_nonce = nonce.to_vec(); ciphertext_integral_nonce.append(&mut ciphertext); From 37bd39a776f8f3d41ede1e3400674379ab053fa8 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 23:20:12 +0000 Subject: [PATCH 6/7] encrypt/decrypt with hidden/zeroize data guarantees --- .../storage/sqlite_db/key_manager_state.rs | 15 ++- .../storage/sqlite_db/mod.rs | 7 +- .../storage/sqlite_db/new_output_sql.rs | 11 +- .../storage/sqlite_db/output_sql.rs | 10 +- .../wallet/src/storage/sqlite_db/wallet.rs | 105 +++++++++++------- .../transaction_service/storage/sqlite_db.rs | 23 +++- base_layer/wallet/src/util/encryption.rs | 11 +- 7 files changed, 120 insertions(+), 62 deletions(-) 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 f5ee46da49..e4f55ae04f 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 @@ -25,6 +25,7 @@ use std::convert::TryFrom; use chacha20poly1305::XChaCha20Poly1305; use chrono::{NaiveDateTime, Utc}; use diesel::{prelude::*, SqliteConnection}; +use tari_utilities::Hidden; use crate::{ key_manager_service::{error::KeyManagerStorageError, storage::database::KeyManagerState}, @@ -158,8 +159,11 @@ impl Encryptable for KeyManagerStateSql { } 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())?; + self.primary_key_index = encrypt_bytes_integral_nonce( + cipher, + self.domain("primary_key_index"), + Hidden::hide(self.primary_key_index.clone()), + )?; Ok(()) } @@ -180,8 +184,11 @@ impl Encryptable for NewKeyManagerStateSql { } 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())?; + self.primary_key_index = encrypt_bytes_integral_nonce( + cipher, + self.domain("primary_key_index"), + Hidden::hide(self.primary_key_index.clone()), + )?; Ok(()) } 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 4a9ae2620c..dd3131ed59 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 @@ -44,6 +44,7 @@ use tari_common_types::{ use tari_core::transactions::transaction_components::{OutputType, TransactionOutput}; use tari_crypto::tari_utilities::{hex::Hex, ByteArray}; use tari_script::{ExecutionStack, TariScript}; +use tari_utilities::Hidden; use tokio::time::Instant; use zeroize::Zeroize; @@ -1325,7 +1326,11 @@ impl Encryptable for KnownOneSidedPaymentScriptSql { } fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - self.private_key = encrypt_bytes_integral_nonce(cipher, self.domain("private_key"), self.private_key.clone())?; + self.private_key = encrypt_bytes_integral_nonce( + cipher, + self.domain("private_key"), + Hidden::hide(self.private_key.clone()), + )?; Ok(()) } 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 a104da2ac5..38e264dc0b 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 @@ -23,7 +23,7 @@ use chacha20poly1305::XChaCha20Poly1305; use derivative::Derivative; use diesel::{prelude::*, SqliteConnection}; use tari_common_types::transaction::TxId; -use tari_utilities::ByteArray; +use tari_utilities::{ByteArray, Hidden}; use crate::{ output_manager_service::{ @@ -136,13 +136,16 @@ impl Encryptable for NewOutputSql { } fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - self.spending_key = - encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; + self.spending_key = encrypt_bytes_integral_nonce( + cipher, + self.domain("spending_key"), + Hidden::hide(self.spending_key.clone()), + )?; self.script_private_key = encrypt_bytes_integral_nonce( cipher, self.domain("script_private_key"), - self.script_private_key.clone(), + Hidden::hide(self.script_private_key.clone()), )?; Ok(()) 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 1dd2eb707e..251e835b1e 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 @@ -41,6 +41,7 @@ use tari_core::{ }; use tari_crypto::{commitment::HomomorphicCommitmentFactory, tari_utilities::ByteArray}; use tari_script::{ExecutionStack, TariScript}; +use tari_utilities::Hidden; use zeroize::Zeroize; use crate::{ @@ -788,13 +789,16 @@ impl Encryptable for OutputSql { } fn encrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - self.spending_key = - encrypt_bytes_integral_nonce(cipher, self.domain("spending_key"), self.spending_key.clone())?; + self.spending_key = encrypt_bytes_integral_nonce( + cipher, + self.domain("spending_key"), + Hidden::hide(self.spending_key.clone()), + )?; self.script_private_key = encrypt_bytes_integral_nonce( cipher, self.domain("script_private_key"), - self.script_private_key.clone(), + Hidden::hide(self.script_private_key.clone()), )?; Ok(()) diff --git a/base_layer/wallet/src/storage/sqlite_db/wallet.rs b/base_layer/wallet/src/storage/sqlite_db/wallet.rs index f0fcfb36a3..1b6ebdf4d7 100644 --- a/base_layer/wallet/src/storage/sqlite_db/wallet.rs +++ b/base_layer/wallet/src/storage/sqlite_db/wallet.rs @@ -44,10 +44,11 @@ use tari_key_manager::cipher_seed::CipherSeed; use tari_utilities::{ hex::{from_hex, Hex}, message_format::MessageFormat, + Hidden, SafePassword, }; use tokio::time::Instant; -use zeroize::Zeroizing; +use zeroize::{Zeroize, Zeroizing}; use crate::{ error::WalletStorageError, @@ -93,7 +94,7 @@ impl WalletSqliteDatabase { WalletSettingSql::new(DbKey::WalletBirthday.to_string(), birthday.to_string()).set(conn)?; }, Some(cipher) => { - let seed_bytes = seed.encipher(None)?; + let seed_bytes = Hidden::hide(seed.encipher(None)?); let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(cipher, b"wallet_setting_master_seed".to_vec(), seed_bytes) .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?; @@ -110,13 +111,17 @@ impl WalletSqliteDatabase { let seed = match cipher.as_ref() { None => CipherSeed::from_enciphered_bytes(&from_hex(seed_str.as_str())?, None)?, Some(cipher) => { - let decrypted_key_bytes = decrypt_bytes_integral_nonce( - cipher, - b"wallet_setting_master_seed".to_vec(), - from_hex(seed_str.as_str())?, - ) - .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?; - CipherSeed::from_enciphered_bytes(&decrypted_key_bytes, None)? + // Decrypted_key_bytes contains sensitive data regarding decrypted + // seed words. For this reason, we should zeroize the underlying data buffer + let decrypted_key_bytes = Hidden::hide( + decrypt_bytes_integral_nonce( + cipher, + b"wallet_setting_master_seed".to_vec(), + from_hex(seed_str.as_str())?, + ) + .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?, + ); + CipherSeed::from_enciphered_bytes(decrypted_key_bytes.reveal(), None)? }, }; @@ -175,7 +180,9 @@ impl WalletSqliteDatabase { WalletSettingSql::new(DbKey::TorId.to_string(), tor_string).set(conn)?; }, Some(cipher) => { - let bytes = bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?; + let bytes = Hidden::hide( + bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?, + ); let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(cipher, b"wallet_setting_tor_id".to_vec(), bytes) .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?; @@ -195,11 +202,14 @@ impl WalletSqliteDatabase { TorIdentity::from_json(&key_str).map_err(|e| WalletStorageError::ConversionError(e.to_string()))? }, Some(cipher) => { - let decrypted_key_bytes = + // we must zeroize decrypted_key_bytes, as this contains sensitive data, + // including private key informations + let decrypted_key_bytes = Hidden::hide( decrypt_bytes_integral_nonce(cipher, b"wallet_setting_tor_id".to_vec(), from_hex(&key_str)?) - .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?; + .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?, + ); - bincode::deserialize(&decrypted_key_bytes) + bincode::deserialize(decrypted_key_bytes.reveal()) .map_err(|e| WalletStorageError::ConversionError(e.to_string()))? }, }; @@ -465,10 +475,10 @@ impl WalletBackend for WalletSqliteDatabase { Some(sk) => sk, }; - let master_seed_bytes = from_hex(master_seed_str.as_str())?; + let master_seed_bytes = Hidden::hide(from_hex(master_seed_str.as_str())?); // Sanity check that the decrypted bytes are a valid CipherSeed - let _master_seed = CipherSeed::from_enciphered_bytes(&master_seed_bytes, None)?; + let _master_seed = CipherSeed::from_enciphered_bytes(master_seed_bytes.reveal(), None)?; let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(&cipher, b"wallet_setting_master_seed".to_vec(), master_seed_bytes) .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?; @@ -486,7 +496,8 @@ impl WalletBackend for WalletSqliteDatabase { let tor_id = WalletSettingSql::get(DbKey::TorId.to_string(), &conn)?; if let Some(v) = tor_id { let tor = TorIdentity::from_json(&v).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?; - let bytes = bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?; + let bytes = + Hidden::hide(bincode::serialize(&tor).map_err(|e| WalletStorageError::ConversionError(e.to_string()))?); let ciphertext_integral_nonce = encrypt_bytes_integral_nonce(&cipher, b"wallet_setting_tor_id".to_vec(), bytes) .map_err(|e| WalletStorageError::AeadError(format!("Encryption Error:{}", e)))?; @@ -522,16 +533,18 @@ impl WalletBackend for WalletSqliteDatabase { Some(sk) => sk, }; - let master_seed_bytes = decrypt_bytes_integral_nonce( - &cipher, - b"wallet_setting_master_seed".to_vec(), - from_hex(master_seed_str.as_str())?, - ) - .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?; + let master_seed_bytes = Hidden::hide( + decrypt_bytes_integral_nonce( + &cipher, + b"wallet_setting_master_seed".to_vec(), + from_hex(master_seed_str.as_str())?, + ) + .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?, + ); // Sanity check that the decrypted bytes are a valid CipherSeed - let _master_seed = CipherSeed::from_enciphered_bytes(&master_seed_bytes, None)?; - WalletSettingSql::new(DbKey::MasterSeed.to_string(), master_seed_bytes.to_hex()).set(&conn)?; + let _master_seed = CipherSeed::from_enciphered_bytes(master_seed_bytes.reveal(), None)?; + WalletSettingSql::new(DbKey::MasterSeed.to_string(), master_seed_bytes.reveal().to_hex()).set(&conn)?; let _ = WalletSettingSql::clear(DbKey::PassphraseHash.to_string(), &conn)?; let _ = WalletSettingSql::clear(DbKey::EncryptionSalt.to_string(), &conn)?; @@ -547,11 +560,14 @@ impl WalletBackend for WalletSqliteDatabase { // remove tor id encryption if present let key_str = WalletSettingSql::get(DbKey::TorId.to_string(), &conn)?; if let Some(v) = key_str { - let decrypted_key_bytes = + // decrypted_key_bytes contains sensitive information regarding private key, we thus + // make sure the data is appropriately zeroized when we leave the current scope + let decrypted_key_bytes = Hidden::hide( decrypt_bytes_integral_nonce(&cipher, b"wallet_setting_tor_id".to_vec(), from_hex(v.as_str())?) - .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?; + .map_err(|e| WalletStorageError::AeadError(format!("Decryption Error:{}", e)))?, + ); - let tor_id: TorIdentity = bincode::deserialize(&decrypted_key_bytes) + let tor_id: TorIdentity = bincode::deserialize(decrypted_key_bytes.reveal()) .map_err(|e| WalletStorageError::ConversionError(e.to_string()))?; let tor_string = tor_id @@ -703,20 +719,24 @@ fn check_db_encryption_status( return Err(WalletStorageError::MissingNonce); } - let decrypted_key = + // decrypted key contains sensitive data, we make sure we appropriately zeroize + // the corresponding data buffer, when leaving the current scope + let decrypted_key = Hidden::hide( decrypt_bytes_integral_nonce(&cipher_inner, b"wallet_setting_master_seed".to_vec(), sk_bytes) .map_err(|e| { error!(target: LOG_TARGET, "Incorrect passphrase ({})", e); WalletStorageError::InvalidPassphrase - })?; + })?, + ); - let _cipher_seed = CipherSeed::from_enciphered_bytes(&decrypted_key, None).map_err(|_| { - error!( - target: LOG_TARGET, - "Decrypted Master Secret Key cannot be parsed into a Cipher Seed" - ); - WalletStorageError::InvalidEncryptionCipher - })?; + let _cipher_seed = + CipherSeed::from_enciphered_bytes(decrypted_key.reveal(), None).map_err(|_| { + error!( + target: LOG_TARGET, + "Decrypted Master Secret Key cannot be parsed into a Cipher Seed" + ); + WalletStorageError::InvalidEncryptionCipher + })?; } else { error!( target: LOG_TARGET, @@ -832,15 +852,19 @@ impl Encryptable for ClientKeyValueSql { #[allow(unused_assignments)] 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(); + self.value = encrypt_bytes_integral_nonce( + cipher, + self.domain("value"), + Hidden::hide(self.value.as_bytes().to_vec()), + )? + .to_hex(); Ok(()) } #[allow(unused_assignments)] fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - let decrypted_value = decrypt_bytes_integral_nonce( + let mut decrypted_value = decrypt_bytes_integral_nonce( cipher, self.domain("value"), from_hex(self.value.as_str()).map_err(|e| e.to_string())?, @@ -850,6 +874,9 @@ impl Encryptable for ClientKeyValueSql { .map_err(|e| e.to_string())? .to_string(); + // we zeroize the decrypted value + decrypted_value.zeroize(); + Ok(()) } } 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 1efb172da0..5366e8bcc6 100644 --- a/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs +++ b/base_layer/wallet/src/transaction_service/storage/sqlite_db.rs @@ -46,9 +46,11 @@ use tari_core::transactions::tari_amount::MicroTari; use tari_utilities::{ hex::{from_hex, Hex}, ByteArray, + Hidden, }; use thiserror::Error; use tokio::time::Instant; +use zeroize::Zeroize; use crate::{ schema::{completed_transactions, inbound_transactions, outbound_transactions}, @@ -1537,7 +1539,7 @@ impl Encryptable for InboundTransactionSql { self.receiver_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("receiver_protocol"), - self.receiver_protocol.as_bytes().to_vec(), + Hidden::hide(self.receiver_protocol.as_bytes().to_vec()), )? .to_hex(); @@ -1545,7 +1547,7 @@ impl Encryptable for InboundTransactionSql { } fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - let decrypted_protocol = decrypt_bytes_integral_nonce( + let mut decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("receiver_protocol"), from_hex(self.receiver_protocol.as_str()).map_err(|e| e.to_string())?, @@ -1555,6 +1557,9 @@ impl Encryptable for InboundTransactionSql { .map_err(|e| e.to_string())? .to_string(); + // zeroize the decrypted protocol data buffer + decrypted_protocol.zeroize(); + Ok(()) } } @@ -1788,7 +1793,7 @@ impl Encryptable for OutboundTransactionSql { self.sender_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("sender_protocol"), - self.sender_protocol.as_bytes().to_vec(), + Hidden::hide(self.sender_protocol.as_bytes().to_vec()), )? .to_hex(); @@ -1796,7 +1801,7 @@ impl Encryptable for OutboundTransactionSql { } fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - let decrypted_protocol = decrypt_bytes_integral_nonce( + let mut decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("sender_protocol"), from_hex(self.sender_protocol.as_str()).map_err(|e| e.to_string())?, @@ -1806,6 +1811,9 @@ impl Encryptable for OutboundTransactionSql { .map_err(|e| e.to_string())? .to_string(); + // zeroize the decrypted protocol data buffer + decrypted_protocol.zeroize(); + Ok(()) } } @@ -2194,7 +2202,7 @@ impl Encryptable for CompletedTransactionSql { self.transaction_protocol = encrypt_bytes_integral_nonce( cipher, self.domain("transaction_protocol"), - self.transaction_protocol.as_bytes().to_vec(), + Hidden::hide(self.transaction_protocol.as_bytes().to_vec()), )? .to_hex(); @@ -2202,7 +2210,7 @@ impl Encryptable for CompletedTransactionSql { } fn decrypt(&mut self, cipher: &XChaCha20Poly1305) -> Result<(), String> { - let decrypted_protocol = decrypt_bytes_integral_nonce( + let mut decrypted_protocol = decrypt_bytes_integral_nonce( cipher, self.domain("transaction_protocol"), from_hex(self.transaction_protocol.as_str()).map_err(|e| e.to_string())?, @@ -2212,6 +2220,9 @@ impl Encryptable for CompletedTransactionSql { .map_err(|e| e.to_string())? .to_string(); + // zeroize the decrypted protocol data buffer + decrypted_protocol.zeroize(); + Ok(()) } } diff --git a/base_layer/wallet/src/util/encryption.rs b/base_layer/wallet/src/util/encryption.rs index 697b42ab2a..48cfa2c64b 100644 --- a/base_layer/wallet/src/util/encryption.rs +++ b/base_layer/wallet/src/util/encryption.rs @@ -29,7 +29,7 @@ use chacha20poly1305::{ XNonce, }; use rand::{rngs::OsRng, RngCore}; -use tari_utilities::ByteArray; +use tari_utilities::{ByteArray, Hidden}; use zeroize::Zeroize; pub trait Encryptable { @@ -78,7 +78,7 @@ pub fn decrypt_bytes_integral_nonce( pub fn encrypt_bytes_integral_nonce( cipher: &XChaCha20Poly1305, domain: Vec, - mut plaintext: Vec, + mut plaintext: Hidden>, ) -> Result, String> { // Produce a secure random nonce let mut nonce = [0u8; size_of::()]; @@ -87,7 +87,7 @@ pub fn encrypt_bytes_integral_nonce( // Bind the domain as additional data let payload = Payload { - msg: plaintext.as_slice(), + msg: plaintext.reveal(), aad: domain.as_slice(), }; @@ -110,7 +110,7 @@ mod test { use chacha20poly1305::{aead::NewAead, Key, Tag, XChaCha20Poly1305, XNonce}; use rand::{rngs::OsRng, RngCore}; - use tari_utilities::ByteArray; + use tari_utilities::{ByteArray, Hidden}; use crate::util::encryption::{decrypt_bytes_integral_nonce, encrypt_bytes_integral_nonce}; @@ -123,7 +123,8 @@ mod test { 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(); + let ciphertext = + encrypt_bytes_integral_nonce(&cipher, b"correct_domain".to_vec(), Hidden::hide(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 From e8436a14964d5d00d21948962dabae7a03e54885 Mon Sep 17 00:00:00 2001 From: jorgeantonio21 Date: Thu, 24 Nov 2022 23:23:03 +0000 Subject: [PATCH 7/7] remove unnecessary zeroize --- base_layer/wallet/src/util/encryption.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/base_layer/wallet/src/util/encryption.rs b/base_layer/wallet/src/util/encryption.rs index 48cfa2c64b..9d84535aa9 100644 --- a/base_layer/wallet/src/util/encryption.rs +++ b/base_layer/wallet/src/util/encryption.rs @@ -78,7 +78,7 @@ pub fn decrypt_bytes_integral_nonce( pub fn encrypt_bytes_integral_nonce( cipher: &XChaCha20Poly1305, domain: Vec, - mut plaintext: Hidden>, + plaintext: Hidden>, ) -> Result, String> { // Produce a secure random nonce let mut nonce = [0u8; size_of::()]; @@ -94,9 +94,6 @@ pub fn encrypt_bytes_integral_nonce( // Attempt authenticated encryption let mut ciphertext = cipher.encrypt(nonce_ga, payload).map_err(|e| e.to_string())?; - // zeroize plaintext to avoid leaking sensitive data - plaintext.zeroize(); - // Concatenate the nonce and ciphertext (which already include the tag) let mut ciphertext_integral_nonce = nonce.to_vec(); ciphertext_integral_nonce.append(&mut ciphertext);