-
Notifications
You must be signed in to change notification settings - Fork 219
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
Changes from 10 commits
960ba69
0d8c05c
55c8834
e62428e
77024b8
1296b77
4285b6a
806b048
f7ec9bb
1c5df3e
dbbffcf
a0f7e91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
|
@@ -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 { | ||
|
@@ -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)) | ||
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can simplify this by using 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in PR 4994. |
||
|
||
#[cfg(test)] | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in PR 4994.