fix: address points in issue #4138 and companions

comms/dht/Cargo.toml

@@ -23,6 +23,7 @@ anyhow = "1.0.53"
 bitflags = "1.2.0"
 bytes = "0.5"
 chacha20 = "0.7.1"
+chacha20poly1305 = "0.9.1"
 chrono = { version = "0.4.19", default-features = false }
 diesel = { version = "1.4.7", features = ["sqlite", "serde_json", "chrono", "numeric"] }
 diesel_migrations = "1.4.0"

comms/dht/src/crypt.rs

@@ -28,10 +28,16 @@ use chacha20::{
     Key,
     Nonce,
 };
-use digest::{Digest, FixedOutput};
+use chacha20poly1305::{
+    self,
+    aead::{Aead, NewAead},
+    ChaCha20Poly1305,
+};
 use rand::{rngs::OsRng, RngCore};
 use tari_comms::types::{Challenge, CommsPublicKey};
 use tari_crypto::{
+    hash::blake2::Blake256,
+    hashing::{DomainSeparatedHasher, GenericHashDomain},
     keys::{DiffieHellmanSharedSecret, PublicKey},
     tari_utilities::{epoch_time::EpochTime, ByteArray},
 };
@@ -43,17 +49,53 @@ use crate::{
     version::DhtProtocolVersion,
 };
 
+const DOMAIN_SEPARATION_CHALLENGE_LABEL: &str = "com.tari.comms.dht.crypt.challenge";
+const DOMAIN_SEPARATION_KEY_MESSAGE_LABEL: &str = "com.tari.comms.dht.crypt.challenge";
+const DOMAIN_SEPARATION_KEY_SIGNATURE_LABEL: &str = "com.tari.comms.dht.crypt.key_signature";
+
 #[derive(Debug, Clone, Zeroize)]
 #[zeroize(drop)]
 pub struct CipherKey(chacha20::Key);
+pub struct AuthenticatedCipherKey(chacha20poly1305::Key);
+
+// TODO:
+//  1. rename mac_challenge function
+//  2. check if output of generate_ecdh_secret has correct output size
 
 /// Generates a Diffie-Hellman secret `kx.G` as a `chacha20::Key` given secret scalar `k` and public key `P = x.G`.
-pub fn generate_ecdh_secret<PK>(secret_key: &PK::K, public_key: &PK) -> CipherKey
+pub fn generate_ecdh_secret<PK>(secret_key: &PK::K, public_key: &PK) -> [u8; 32]
 where PK: PublicKey + DiffieHellmanSharedSecret<PK = PK> {
     // TODO: PK will still leave the secret in released memory. Implementing Zerioze on RistrettoPublicKey is not
     //       currently possible because (Compressed)RistrettoPoint does not implement it.
     let k = PK::shared_secret(secret_key, public_key);
-    CipherKey(*Key::from_slice(k.as_bytes()))
+    let mut output = [0u8; 32];
+
+    output.copy_from_slice(k.as_bytes());
+    output
+}
+
+pub fn generate_key_message(data: &[u8]) -> CipherKey {
+    // domain separated hash of data (e.g. ecdh shared secret) using hashing API
+    let domain_separated_hash =
+        DomainSeparatedHasher::<Challenge, GenericHashDomain>::new(DOMAIN_SEPARATION_KEY_MESSAGE_LABEL)
+            .chain(data)
+            .finalize()
+            .into_vec();
+
+    // Domain separation uses Challenge = Blake256, thus its output has 32-byte length
+    CipherKey(*Key::from_slice(domain_separated_hash.as_bytes()))
+}
+
+pub fn generate_key_signature_for_authenticated_encryption(data: &[u8]) -> AuthenticatedCipherKey {
+    // domain separated of data (e.g. ecdh shared secret) using hashing API
+    let domain_separated_hash =
+        DomainSeparatedHasher::<Blake256, GenericHashDomain>::new(DOMAIN_SEPARATION_KEY_SIGNATURE_LABEL)
+            .chain(data)
+            .finalize()
+            .into_vec();
+
+    // Domain separation uses Challenge = Blake256, thus its output has 32-byte length
+    AuthenticatedCipherKey(*chacha20poly1305::Key::from_slice(domain_separated_hash.as_bytes()))
 }
 
 /// Decrypts cipher text using ChaCha20 stream cipher given the cipher key and cipher text with integral nonce.
@@ -73,6 +115,22 @@ pub fn decrypt(cipher_key: &CipherKey, cipher_text: &[u8]) -> Result<Vec<u8>, Dh
     Ok(cipher_text)
 }
 
+pub fn decrypt_with_chacha20_poly1305(
+    cipher_key: &AuthenticatedCipherKey,
+    cipher_signature: &[u8],
+) -> Result<Vec<u8>, DhtOutboundError> {
+    let nonce = [0u8; size_of::<chacha20poly1305::Nonce>()];
+
+    let nonce_ga = chacha20poly1305::Nonce::from_slice(&nonce);
+
+    let cipher = ChaCha20Poly1305::new(&cipher_key.0);
+    let decrypted_signature = cipher
+        .decrypt(nonce_ga, cipher_signature)
+        .map_err(|_| DhtOutboundError::CipherError(String::from("Authenticated decryption failed")))?;
+
+    Ok(decrypted_signature)
+}
+
 /// Encrypt the plain text using the ChaCha20 stream cipher
 pub fn encrypt(cipher_key: &CipherKey, plain_text: &[u8]) -> Vec<u8> {
     let mut nonce = [0u8; size_of::<Nonce>()];
@@ -88,6 +146,28 @@ pub fn encrypt(cipher_key: &CipherKey, plain_text: &[u8]) -> Vec<u8> {
     buf
 }
 
+/// Produces authenticated encryption of the signature using the ChaCha20-Poly1305 stream cipher,
+/// refer to https://docs.rs/chacha20poly1305/latest/chacha20poly1305/# for more details.
+/// Attention: as pointed in https://github.com/tari-project/tari/issues/4138, it is possible
+/// to use a fixed length Nonce, with homogeneous zero data, as this does not incur any security
+/// vulnerabilities. However, such function is not intented to be used outside of dht scope
+pub fn encrypt_with_chacha20_poly1305(
+    cipher_key: &AuthenticatedCipherKey,
+    signature: &[u8],
+) -> Result<Vec<u8>, DhtOutboundError> {
+    let nonce = [0u8; size_of::<chacha20poly1305::Nonce>()];
+
+    let nonce_ga = chacha20poly1305::Nonce::from_slice(&nonce);
+    let cipher = ChaCha20Poly1305::new(&cipher_key.0);
+
+    // length of encrypted equals signature.len() + 16 (the latter being the tag size for ChaCha20-poly1305)
+    let encrypted = cipher
+        .encrypt(nonce_ga, signature)
+        .map_err(|_| DhtOutboundError::CipherError(String::from("Authenticated encryption failed")))?;
+
+    Ok(encrypted)
+}
+
 /// Generates a 32-byte hashed challenge that commits to the message header and body
 pub fn create_message_challenge(header: &DhtMessageHeader, body: &[u8]) -> [u8; 32] {
     create_message_challenge_parts(
@@ -111,14 +191,10 @@ pub fn create_message_challenge_parts(
     ephemeral_public_key: Option<&CommsPublicKey>,
     body: &[u8],
 ) -> [u8; 32] {
-    let mut mac_challenge = Challenge::new();
-    mac_challenge.update(&protocol_version.as_bytes());
-    mac_challenge.update(destination.to_inner_bytes());
-    mac_challenge.update(&(message_type as i32).to_le_bytes());
-    mac_challenge.update(&flags.bits().to_le_bytes());
+    // get byte representation of `expires` input
     let expires = expires.map(|t| t.as_u64().to_le_bytes()).unwrap_or_default();
-    mac_challenge.update(&expires);
 
+    // get byte representation of `ephemeral_public_key`
     let e_pk = ephemeral_public_key
         .map(|e_pk| {
             let mut buf = [0u8; 32];
@@ -127,10 +203,25 @@ pub fn create_message_challenge_parts(
             buf
         })
         .unwrap_or_default();
-    mac_challenge.update(&e_pk);
 
-    mac_challenge.update(&body);
-    mac_challenge.finalize_fixed().into()
+    // we digest the given data into a domain independent hash function to produce a signature
+    // use of the hashing API for domain separation and deal with variable length input
+    let domain_separated_hash =
+        DomainSeparatedHasher::<Challenge, GenericHashDomain>::new(DOMAIN_SEPARATION_CHALLENGE_LABEL)
+            .chain(&protocol_version.as_bytes())
+            .chain(destination.to_inner_bytes())
+            .chain(&(message_type as i32).to_le_bytes())
+            .chain(&flags.bits().to_le_bytes())
+            .chain(&expires)
+            .chain(&e_pk)
+            .chain(&body)
+            .finalize()
+            .into_vec();
+
+    let mut output = [0u8; 32];
+    // the use of Challenge bind to Blake256, should always produce a 32-byte length output data
+    output.copy_from_slice(domain_separated_hash.as_slice());
+    output
 }
 
 #[cfg(test)]
@@ -160,4 +251,96 @@ mod test {
         let secret_msg = "Last enemy position 0830h AJ 9863".as_bytes().to_vec();
         assert_eq!(plain_text, secret_msg);
     }
+
+    #[test]
+    fn sanity_check() {
+        let domain_separated_hash =
+            DomainSeparatedHasher::<Blake256, GenericHashDomain>::new(DOMAIN_SEPARATION_KEY_SIGNATURE_LABEL)
+                .chain(&[10, 12, 13, 82, 93, 101, 87, 28, 27, 17, 11, 35, 43])
+                .finalize()
+                .into_vec();
+
+        // Domain separation uses Challenge = Blake256, thus its output has 32-byte length
+        let key = AuthenticatedCipherKey(*chacha20poly1305::Key::from_slice(domain_separated_hash.as_bytes()));
+
+        let signature = b"Top secret message, handle with care".as_slice();
+        let n = signature.len();
+        let nonce = [0u8; size_of::<chacha20poly1305::Nonce>()];
+
+        let nonce_ga = chacha20poly1305::Nonce::from_slice(&nonce);
+        let cipher = ChaCha20Poly1305::new(&key.0);
+
+        let encrypted = cipher
+            .encrypt(nonce_ga, signature)
+            .map_err(|_| DhtOutboundError::CipherError(String::from("Authenticated encryption failed")))
+            .unwrap();
+
+        assert_eq!(encrypted.len(), n + 16);
+    }
+
+    #[test]
+    fn decryption_fails_in_case_tag_is_manipulated() {
+        let (sk, pk) = CommsPublicKey::random_keypair(&mut OsRng);
+        let key_data = generate_ecdh_secret(&sk, &pk);
+        let key = generate_key_signature_for_authenticated_encryption(&key_data);
+
+        let signature = b"Top secret message, handle with care".as_slice();
+
+        let mut encrypted = encrypt_with_chacha20_poly1305(&key, signature).unwrap();
+
+        // sanity check to validate that encrypted.len() = signature.len() + 16
+        assert_eq!(encrypted.len(), signature.len() + 16);
+
+        // manipulate tag and check that decryption fails
+        let n = encrypted.len();
+        encrypted[n - 1] += 1;
+
+        // decryption should fail
+        assert!(decrypt_with_chacha20_poly1305(&key, encrypted.as_slice())
+            .unwrap_err()
+            .to_string()
+            .contains("Authenticated decryption failed"));
+    }
+
+    #[test]
+    fn decryption_fails_in_case_body_message_is_manipulated() {
+        let (sk, pk) = CommsPublicKey::random_keypair(&mut OsRng);
+        let key_data = generate_ecdh_secret(&sk, &pk);
+        let key = generate_key_signature_for_authenticated_encryption(&key_data);
+
+        let signature = b"Top secret message, handle with care".as_slice();
+
+        let mut encrypted = encrypt_with_chacha20_poly1305(&key, signature).unwrap();
+
+        // manipulate encrypted message body and check that decryption fails
+        encrypted[0] += 1;
+
+        // decryption should fail
+        assert!(decrypt_with_chacha20_poly1305(&key, encrypted.as_slice())
+            .unwrap_err()
+            .to_string()
+            .contains("Authenticated decryption failed"));
+    }
+
+    #[test]
+    fn decryption_fails_if_message_sned_to_incorrect_node() {
+        let (sk, pk) = CommsPublicKey::random_keypair(&mut OsRng);
+        let (other_sk, other_pk) = CommsPublicKey::random_keypair(&mut OsRng);
+
+        let key_data = generate_ecdh_secret(&sk, &pk);
+        let other_key_data = generate_ecdh_secret(&other_sk, &other_pk);
+
+        let key = generate_key_signature_for_authenticated_encryption(&key_data);
+        let other_key = generate_key_signature_for_authenticated_encryption(&other_key_data);
+
+        let signature = b"Top secret message, handle with care".as_slice();
+
+        let encrypted = encrypt_with_chacha20_poly1305(&key, signature).unwrap();
+
+        // decryption should fail
+        assert!(decrypt_with_chacha20_poly1305(&other_key, encrypted.as_slice())
+            .unwrap_err()
+            .to_string()
+            .contains("Authenticated decryption failed"));
+    }
 }

comms/dht/src/dedup/mod.rs

@@ -176,7 +176,8 @@ mod test {
 
         assert!(dedup.poll_ready(&mut cx).is_ready());
         let node_identity = make_node_identity();
-        let inbound_message = make_dht_inbound_message(&node_identity, vec![], DhtMessageFlags::empty(), false, false);
+        let inbound_message =
+            make_dht_inbound_message(&node_identity, vec![], DhtMessageFlags::empty(), false, false).unwrap();
         let decrypted_msg = DecryptedDhtMessage::succeeded(wrap_in_envelope_body!(vec![]), None, inbound_message);
 
         rt.block_on(dedup.call(decrypted_msg.clone())).unwrap();
@@ -201,7 +202,8 @@ mod test {
             DhtMessageFlags::empty(),
             false,
             false,
-        );
+        )
+        .unwrap();
         let decrypted1 = DecryptedDhtMessage::succeeded(wrap_in_envelope_body!(vec![]), None, dht_message);
 
         let node_identity = make_node_identity();
@@ -211,7 +213,8 @@ mod test {
             DhtMessageFlags::empty(),
             false,
             false,
-        );
+        )
+        .unwrap();
         let decrypted2 = DecryptedDhtMessage::succeeded(wrap_in_envelope_body!(vec![]), None, dht_message);
 
         assert_eq!(decrypted1.dedup_hash, decrypted2.dedup_hash);

comms/dht/src/dht.rs

@@ -496,7 +496,8 @@ mod test {
             false,
             MessageTag::new(),
             false,
-        );
+        )
+        .unwrap();
         let inbound_message = make_comms_inbound_message(&node_identity, dht_envelope.to_encoded_bytes().into());
 
         let msg = {
@@ -547,7 +548,8 @@ mod test {
             true,
             MessageTag::new(),
             true,
-        );
+        )
+        .unwrap();
 
         let inbound_message = make_comms_inbound_message(&node_identity, dht_envelope.to_encoded_bytes().into());
 
@@ -596,15 +598,17 @@ mod test {
         // Encrypt for someone else
         let node_identity2 = make_node_identity();
         let ecdh_key = crypt::generate_ecdh_secret(node_identity2.secret_key(), node_identity2.public_key());
-        let encrypted_bytes = crypt::encrypt(&ecdh_key, &msg.to_encoded_bytes());
+        let key_message = crypt::generate_key_message(&ecdh_key);
+        let encrypted_bytes = crypt::encrypt(&key_message, &msg.to_encoded_bytes());
         let dht_envelope = make_dht_envelope(
             &node_identity2,
             encrypted_bytes,
             DhtMessageFlags::ENCRYPTED,
             true,
             MessageTag::new(),
             true,
-        );
+        )
+        .unwrap();
 
         let origin_mac = dht_envelope.header.as_ref().unwrap().origin_mac.clone();
         assert!(!origin_mac.is_empty());
@@ -662,7 +666,8 @@ mod test {
             false,
             MessageTag::new(),
             false,
-        );
+        )
+        .unwrap();
         dht_envelope.header.as_mut().unwrap().message_type = DhtMessageType::SafStoredMessages as i32;
         let inbound_message = make_comms_inbound_message(&node_identity, dht_envelope.to_encoded_bytes().into());