Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hide sensitive data on tari repo (see issue #4846) #4967

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};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might consider renaming to something that describes the intent more clearly, like ValueEncryptionKey. But that's just a nit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in PR 4994.

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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May wish to rename the label to be consistent with the reverse-domain notation used elsewhere.

.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
}
Comment on lines -118 to 132
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this by using finalize_into on the hasher to populate aead_key in place. Otherwise output sticks around in memory.

use digest::FixedOutput;

let mut aead_key = CoreTransactionAEADKey::from(SafeArray::default());
DomainSeparatedHasher::<Blake256, TransactionKdfDomain>::new_with_label("encrypted_value")
        .chain(shared_secret.as_bytes())
        .chain(commitment.as_bytes())
        .finalize_into(GenericArray::from_mut_slice(aead_key.reveal_mut()));

aead_key

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in PR 4994.


#[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