From 5ac6133a8ab0707dfd97cf1647d709256bb9c05b Mon Sep 17 00:00:00 2001
From: Stan Bondi <sdbondi@users.noreply.github.com>
Date: Tue, 24 May 2022 16:28:33 +0400
Subject: [PATCH] fix(comms)!: commit to public key and nonce in identity sig
 (#3928)

Description
---

- commit to public nonce and public key on identity signature

Motivation and Context
---
Schnorr signatures are not secure unless public nonce and public key are committed to

How Has This Been Tested?
---
Existing tests (internal to identity signature)
---
 .../src/peer_manager/identity_signature.rs    | 32 +++++++++++++++----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/comms/core/src/peer_manager/identity_signature.rs b/comms/core/src/peer_manager/identity_signature.rs
index 085731c80e..f1f2d3d39e 100644
--- a/comms/core/src/peer_manager/identity_signature.rs
+++ b/comms/core/src/peer_manager/identity_signature.rs
@@ -27,7 +27,7 @@ use digest::Digest;
 use prost::Message;
 use rand::rngs::OsRng;
 use serde::{Deserialize, Serialize};
-use tari_crypto::keys::SecretKey;
+use tari_crypto::keys::PublicKey as PublicKeyTrait;
 use tari_utilities::ByteArray;
 
 use crate::{
@@ -64,9 +64,17 @@ impl IdentitySignature {
         addresses: I,
         updated_at: DateTime<Utc>,
     ) -> Self {
-        let challenge = Self::construct_challenge(Self::LATEST_VERSION, features, addresses, updated_at);
-        let nonce = CommsSecretKey::random(&mut OsRng);
-        let signature = Signature::sign(secret_key.clone(), nonce, &challenge.finalize())
+        let public_key = CommsPublicKey::from_secret_key(secret_key);
+        let (secret_nonce, public_nonce) = CommsPublicKey::random_keypair(&mut OsRng);
+        let challenge = Self::construct_challenge(
+            &public_key,
+            &public_nonce,
+            Self::LATEST_VERSION,
+            features,
+            addresses,
+            updated_at,
+        );
+        let signature = Signature::sign(secret_key.clone(), secret_nonce, &challenge.finalize())
             .expect("unreachable panic: challenge hash digest is the correct length");
         Self {
             version: Self::LATEST_VERSION,
@@ -110,17 +118,29 @@ impl IdentitySignature {
             return false;
         }
 
-        let challenge = Self::construct_challenge(self.version, features, addresses, self.updated_at);
+        let challenge = Self::construct_challenge(
+            public_key,
+            self.signature.get_public_nonce(),
+            self.version,
+            features,
+            addresses,
+            self.updated_at,
+        );
         self.signature.verify_challenge(public_key, &challenge.finalize())
     }
 
     fn construct_challenge<'a, I: IntoIterator<Item = &'a Multiaddr>>(
+        public_key: &CommsPublicKey,
+        public_nonce: &CommsPublicKey,
         version: u8,
         features: PeerFeatures,
         addresses: I,
         updated_at: DateTime<Utc>,
     ) -> Challenge {
+        // e = H(P||R||m)
         let challenge = Challenge::new()
+            .chain(public_key.as_bytes())
+            .chain(public_nonce.as_bytes())
             .chain(version.to_le_bytes())
             .chain(u64::try_from(updated_at.timestamp()).unwrap().to_le_bytes())
             .chain(features.bits().to_le_bytes());
@@ -177,7 +197,7 @@ impl From<&IdentitySignature> for proto::identity::IdentitySignature {
 mod test {
     use std::str::FromStr;
 
-    use tari_crypto::keys::PublicKey;
+    use tari_crypto::keys::{PublicKey, SecretKey};
 
     use super::*;
     use crate::peer_manager::{NodeId, PeerFlags};