From c93182c8fe7db8f557a80edbc83a59e7d42e4e9f Mon Sep 17 00:00:00 2001
From: Stan Bondi <sdbondi@users.noreply.github.com>
Date: Wed, 10 Aug 2022 17:53:01 +0400
Subject: [PATCH] fix(comms)!: use domain hasher for noise DH key derivation
 (#4432)

Description
---
Hashes the noise DH shared secret

Motivation and Context
---
All DH shared secrets should be hashed to remove any possible slight bias in the resulting key.

Ref #4393

How Has This Been Tested?
---
Unit test updated. Existing tests pass
---
 comms/core/src/noise/crypto_resolver.rs | 18 +++++++++++++-----
 comms/core/src/types.rs                 |  3 +++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/comms/core/src/noise/crypto_resolver.rs b/comms/core/src/noise/crypto_resolver.rs
index f281412410..51a5a6d227 100644
--- a/comms/core/src/noise/crypto_resolver.rs
+++ b/comms/core/src/noise/crypto_resolver.rs
@@ -20,6 +20,7 @@
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+use digest::Digest;
 use rand::rngs::OsRng;
 use snow::{
     params::{CipherChoice, DHChoice, HashChoice},
@@ -27,11 +28,13 @@ use snow::{
     types::{Cipher, Dh, Hash, Random},
 };
 use tari_crypto::{
+    hash::blake2::Blake256,
+    hashing::DomainSeparatedHasher,
     keys::{DiffieHellmanSharedSecret, PublicKey, SecretKey},
     tari_utilities::ByteArray,
 };
 
-use crate::types::{CommsPublicKey, CommsSecretKey};
+use crate::types::{CommsCoreHashDomain, CommsPublicKey, CommsSecretKey};
 
 macro_rules! copy_slice {
     ($inslice:expr, $outslice:expr) => {
@@ -63,6 +66,11 @@ impl CryptoResolver for TariCryptoResolver {
     }
 }
 
+fn noise_kdf(shared_key: &CommsPublicKey) -> [u8; 32] {
+    let hasher = DomainSeparatedHasher::<Blake256, CommsCoreHashDomain>::new_with_label("noise.dh");
+    Digest::finalize(hasher.chain(shared_key.as_bytes())).into()
+}
+
 #[derive(Default)]
 struct CommsDiffieHellman {
     secret_key: CommsSecretKey,
@@ -107,8 +115,8 @@ impl Dh for CommsDiffieHellman {
     fn dh(&self, public_key: &[u8], out: &mut [u8]) -> Result<(), ()> {
         let pk = CommsPublicKey::from_bytes(&public_key[..self.pub_len()]).map_err(|_| ())?;
         let shared = CommsPublicKey::shared_secret(&self.secret_key, &pk);
-        let shared_bytes = shared.as_bytes();
-        copy_slice!(shared_bytes, out);
+        let hash = noise_kdf(&shared);
+        copy_slice!(hash, out);
         Ok(())
     }
 }
@@ -149,11 +157,11 @@ mod test {
 
         let (secret_key2, public_key2) = CommsPublicKey::random_keypair(&mut OsRng);
         let expected_shared = CommsPublicKey::shared_secret(&secret_key2, &public_key);
+        let expected_shared = noise_kdf(&expected_shared);
 
         let mut out = [0; 32];
         dh.dh(public_key2.as_bytes(), &mut out).unwrap();
-        let shared = CommsPublicKey::from_bytes(&out).unwrap();
 
-        assert_eq!(shared, expected_shared);
+        assert_eq!(out, expected_shared);
     }
 }
diff --git a/comms/core/src/types.rs b/comms/core/src/types.rs
index 9efad846cc..23005e680c 100644
--- a/comms/core/src/types.rs
+++ b/comms/core/src/types.rs
@@ -24,6 +24,7 @@
 
 use tari_crypto::{
     hash::blake2::Blake256,
+    hash_domain,
     keys::PublicKey,
     ristretto::RistrettoPublicKey,
     signatures::SchnorrSignature,
@@ -55,3 +56,5 @@ pub type CommsDataStore = LMDBStore;
 pub type CommsDatabase = LMDBWrapper<PeerId, Peer>;
 #[cfg(test)]
 pub type CommsDatabase = HashmapDatabase<PeerId, Peer>;
+
+hash_domain!(CommsCoreHashDomain, "com.tari.comms.core", 0);