Skip to content

Commit

Permalink
fix: hide sensitive data on tari repo (see issue #4846) (#4967)
Browse files Browse the repository at this point in the history
Description
---
The goal of this PR is to zeroize sensitive data content across the Tari repo.  We pay special attention to kdf key data and other occurrences of sensitive data. For more details, we refer to issue #4846. Moreover, this PR is a companion to #4953 and #4925

Motivation and Context
---
Tackle issue #4846.

How Has This Been Tested?
---
  • Loading branch information
jorgeantonio21 committed Nov 30, 2022
1 parent 143f305 commit bcc47e1
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 57 deletions.
56 changes: 29 additions & 27 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use tari_core::transactions::{
};
use tari_script::{ExecutionStack, TariScript};
use tari_utilities::ByteArray;
use zeroize::Zeroize;

use crate::tari_rpc as grpc;

Expand Down Expand Up @@ -63,7 +64,7 @@ impl From<UnblindedOutput> for grpc::UnblindedOutput {
impl TryFrom<grpc::UnblindedOutput> for UnblindedOutput {
type Error = String;

fn try_from(output: grpc::UnblindedOutput) -> Result<Self, Self::Error> {
fn try_from(mut output: grpc::UnblindedOutput) -> Result<Self, Self::Error> {
let spending_key =
PrivateKey::from_bytes(output.spending_key.as_bytes()).map_err(|e| format!("spending_key: {:?}", e))?;

Expand Down Expand Up @@ -96,6 +97,10 @@ impl TryFrom<grpc::UnblindedOutput> for UnblindedOutput {

let minimum_value_promise = MicroTari::from(output.minimum_value_promise);

// zeroize output sensitive data
output.spending_key.zeroize();
output.script_private_key.zeroize();

Ok(Self::new(
TransactionOutputVersion::try_from(0u8)?,
MicroTari::from(output.value),
Expand Down
3 changes: 2 additions & 1 deletion applications/tari_console_wallet/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use tari_common::configuration::{ConfigOverrideProvider, Network};
use tari_common_types::tari_address::TariAddress;
use tari_comms::multiaddr::Multiaddr;
use tari_core::transactions::{tari_amount, tari_amount::MicroTari};
use tari_key_manager::SeedWords;
use tari_utilities::{
hex::{Hex, HexError},
SafePassword,
Expand All @@ -59,7 +60,7 @@ pub struct Cli {
/// Supply the optional wallet seed words for recovery on the command line. They should be in one string space
/// separated. e.g. --seed-words "seed1 seed2 ..."
#[clap(long, alias = "seed-words")]
pub seed_words: Option<String>,
pub seed_words: Option<SeedWords>,
/// Supply the optional file name to save the wallet seed words into
#[clap(long, aliases = &["seed_words_file_name", "seed-words-file"], parse(from_os_str))]
pub seed_words_file_name: Option<PathBuf>,
Expand Down
14 changes: 2 additions & 12 deletions applications/tari_console_wallet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ use tari_common::{
configuration::bootstrap::ApplicationType,
exit_codes::{ExitCode, ExitError},
};
use tari_crypto::tari_utilities::Hidden;
use tari_key_manager::{cipher_seed::CipherSeed, SeedWords};
use tari_key_manager::cipher_seed::CipherSeed;
#[cfg(all(unix, feature = "libtor"))]
use tari_libtor::tor::Tor;
use tari_shutdown::Shutdown;
Expand Down Expand Up @@ -213,16 +212,7 @@ fn get_password(config: &ApplicationConfig, cli: &Cli) -> Option<SafePassword> {

fn get_recovery_seed(boot_mode: WalletBoot, cli: &Cli) -> Result<Option<CipherSeed>, ExitError> {
if matches!(boot_mode, WalletBoot::Recovery) {
let seed = if cli.seed_words.is_some() {
// need to zeroize first, to clean up memory of cli.seed_words clone
let seed_words: SeedWords = SeedWords::new(
cli.seed_words
.as_ref()
.unwrap()
.split_whitespace()
.map(|s| Hidden::hide(s.to_string()))
.collect(),
);
let seed = if let Some(ref seed_words) = cli.seed_words {
get_seed_from_seed_words(seed_words)?
} else {
prompt_private_key_from_seed_words()?
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_console_wallet/src/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ pub fn prompt_private_key_from_seed_words() -> Result<CipherSeed, ExitError> {
}

/// Return seed matching the seed words.
pub fn get_seed_from_seed_words(seed_words: SeedWords) -> Result<CipherSeed, ExitError> {
pub fn get_seed_from_seed_words(seed_words: &SeedWords) -> Result<CipherSeed, ExitError> {
debug!(target: LOG_TARGET, "Return seed derived from the provided seed words");
match CipherSeed::from_mnemonic(&seed_words, None) {
match CipherSeed::from_mnemonic(seed_words, None) {
Ok(seed) => Ok(seed),
Err(e) => {
let err_msg = format!("MnemonicError parsing seed words: {}", e);
Expand Down
1 change: 1 addition & 0 deletions base_layer/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ tokio = { version = "1.20", features = ["time", "sync", "macros"] }
tracing = "0.1.26"
tracing-attributes = "*"
uint = { version = "0.9", default-features = false }
zeroize = "1"

[dev-dependencies]
tari_p2p = { version = "^0.41", path = "../../base_layer/p2p", features = ["test-mocks"] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,24 @@ use borsh::{BorshDeserialize, BorshSerialize};
use chacha20poly1305::{
aead::{Aead, Error, Payload},
ChaCha20Poly1305,
Key,
KeyInit,
Nonce,
};
use digest::generic_array::GenericArray;
use serde::{Deserialize, Serialize};
use tari_common_types::types::{Commitment, PrivateKey};
use tari_crypto::{hash::blake2::Blake256, hashing::DomainSeparatedHasher};
use tari_utilities::{ByteArray, ByteArrayError};
use tari_utilities::{safe_array::SafeArray, ByteArray, ByteArrayError};
use thiserror::Error;
use zeroize::Zeroize;

use super::{CoreTransactionAEADKey, AEAD_KEY_LEN};
use crate::transactions::{tari_amount::MicroTari, TransactionKdfDomain};

const SIZE: usize = 24;

/// value: u64 + tag: [u8; 16]
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, Hash, BorshSerialize, BorshDeserialize)]
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, Hash, BorshSerialize, BorshDeserialize, Zeroize)]
pub struct EncryptedValue(#[serde(with = "tari_utilities::serde::hex")] pub [u8; SIZE]);

impl Default for EncryptedValue {
Expand Down Expand Up @@ -89,7 +91,8 @@ impl EncryptedValue {
aad: Self::TAG,
};
// Included in the public transaction
let buffer = ChaCha20Poly1305::new(&aead_key).encrypt(&Nonce::default(), aead_payload)?;
let buffer = ChaCha20Poly1305::new(GenericArray::from_slice(aead_key.reveal()))
.encrypt(&Nonce::default(), aead_payload)?;
let mut data: [u8; SIZE] = [0; SIZE];
data.copy_from_slice(&buffer);
Ok(EncryptedValue(data))
Expand All @@ -107,21 +110,25 @@ impl EncryptedValue {
aad: Self::TAG,
};
let mut value_bytes = [0u8; 8];
let decrypted_bytes = ChaCha20Poly1305::new(&aead_key).decrypt(&Nonce::default(), aead_payload)?;
let decrypted_bytes = ChaCha20Poly1305::new(GenericArray::from_slice(aead_key.reveal()))
.decrypt(&Nonce::default(), aead_payload)?;
value_bytes.clone_from_slice(&decrypted_bytes[..8]);
Ok(u64::from_le_bytes(value_bytes).into())
}
}

// Generate a ChaCha20-Poly1305 key from an ECDH shared secret and commitment using Blake2b
fn kdf_aead(shared_secret: &PrivateKey, commitment: &Commitment) -> Key {
const AEAD_KEY_LENGTH: usize = 32; // The length in bytes of a ChaCha20-Poly1305 AEAD key
fn kdf_aead(shared_secret: &PrivateKey, commitment: &Commitment) -> CoreTransactionAEADKey {
let output = DomainSeparatedHasher::<Blake256, TransactionKdfDomain>::new_with_label("encrypted_value")
.chain(shared_secret.as_bytes())
.chain(commitment.as_bytes())
.finalize();

*Key::from_slice(&output.as_ref()[..AEAD_KEY_LENGTH])
let default_array = SafeArray::<u8, AEAD_KEY_LEN>::default();
let mut aead_key = CoreTransactionAEADKey::from(default_array);
aead_key.reveal_mut().copy_from_slice(&output.as_ref()[..AEAD_KEY_LEN]);

aead_key
}

#[cfg(test)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
// Portions of this file were originally copyrighted (c) 2018 The Grin Developers, issued under the Apache License,
// Version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0.

use chacha20poly1305::Key;
pub use encrypted_value::{EncryptedValue, EncryptionError};
pub use error::TransactionError;
pub use kernel_builder::KernelBuilder;
Expand All @@ -34,6 +35,7 @@ pub use output_type::OutputType;
pub use side_chain::*;
use tari_common_types::types::{Commitment, FixedHash, PublicKey};
use tari_script::TariScript;
use tari_utilities::{hidden_type, safe_array::SafeArray, Hidden};
pub use transaction::Transaction;
pub use transaction_builder::TransactionBuilder;
pub use transaction_input::{SpentOutput, TransactionInput};
Expand All @@ -44,6 +46,7 @@ pub use transaction_output::TransactionOutput;
pub use transaction_output_version::TransactionOutputVersion;
pub use unblinded_output::UnblindedOutput;
pub use unblinded_output_builder::UnblindedOutputBuilder;
use zeroize::Zeroize;

mod encrypted_value;
mod error;
Expand Down Expand Up @@ -73,6 +76,10 @@ mod test;
pub const MAX_TRANSACTION_INPUTS: usize = 12_500;
pub const MAX_TRANSACTION_OUTPUTS: usize = 500;
pub const MAX_TRANSACTION_RECIPIENTS: usize = 15;
pub(crate) const AEAD_KEY_LEN: usize = std::mem::size_of::<Key>();

// Type for hiding aead key encryption
hidden_type!(CoreTransactionAEADKey, SafeArray<u8, AEAD_KEY_LEN>);

//---------------------------------------- Crate functions ----------------------------------------------------//

Expand Down
Loading

0 comments on commit bcc47e1

Please sign in to comment.