A0-1526: Sign addressing information (#798)

* Sign addressing information

* Better name for verification

* Elaborate why zero signatures

Co-authored-by: timorl <[email protected]>

finality-aleph/src/crypto.rs

@@ -2,7 +2,7 @@ use std::{convert::TryInto, sync::Arc};
 
 use aleph_primitives::{AuthorityId, AuthoritySignature, KEY_TYPE};
 use codec::{Decode, Encode};
-use sp_core::crypto::KeyTypeId;
+use sp_core::{crypto::KeyTypeId, ed25519::Signature as RawSignature};
 use sp_keystore::{CryptoStore, Error as KeystoreError};
 use sp_runtime::RuntimeAppPublic;
 
@@ -24,6 +24,13 @@ impl From<AuthoritySignature> for Signature {
     }
 }
 
+// This is here just for a compatibility hack, remove when removing legacy/v1 authentications.
+impl From<[u8; 64]> for Signature {
+    fn from(bytes: [u8; 64]) -> Signature {
+        Signature(RawSignature::from_raw(bytes).into())
+    }
+}
+
 /// Ties an authority identification and a cryptography keystore together for use in
 /// signing that requires an authority.
 #[derive(Clone)]

finality-aleph/src/network/manager/compatibility.rs

@@ -259,13 +259,15 @@ mod test {
             NetworkIdentity,
         },
         nodes::testing::new_pen,
-        tcp_network::{testing::new_identity, LegacyTcpMultiaddress, TcpAddressingInformation},
+        tcp_network::{
+            testing::new_identity, LegacyTcpMultiaddress, SignedTcpAddressingInformation,
+        },
         testing::mocks::validator_network::MockAddressingInformation,
         NodeIndex, SessionId, Version,
     };
 
     /// Session Handler used for generating versioned authentication in `raw_authentication_v1`
-    async fn handler() -> SessionHandler<LegacyTcpMultiaddress, TcpAddressingInformation> {
+    async fn handler() -> SessionHandler<LegacyTcpMultiaddress, SignedTcpAddressingInformation> {
         let mnemonic = "ring cool spatial rookie need wing opinion pond fork garbage more april";
         let external_addresses = vec![
             String::from("addr1"),
@@ -275,7 +277,7 @@ mod test {
 
         let keystore = Arc::new(KeyStore::new());
         let pen = new_pen(mnemonic, keystore).await;
-        let identity = new_identity(external_addresses, pen.authority_id());
+        let identity = new_identity(external_addresses, &pen).await;
 
         SessionHandler::new(
             Some((NodeIndex(21), pen)),
@@ -287,8 +289,8 @@ mod test {
     }
 
     fn authentication_v1(
-        handler: SessionHandler<LegacyTcpMultiaddress, TcpAddressingInformation>,
-    ) -> VersionedAuthentication<LegacyTcpMultiaddress, TcpAddressingInformation> {
+        handler: SessionHandler<LegacyTcpMultiaddress, SignedTcpAddressingInformation>,
+    ) -> VersionedAuthentication<LegacyTcpMultiaddress, SignedTcpAddressingInformation> {
         match handler
             .authentication()
             .expect("should have authentication")
@@ -301,8 +303,8 @@ mod test {
     }
 
     fn authentication_v2(
-        handler: SessionHandler<LegacyTcpMultiaddress, TcpAddressingInformation>,
-    ) -> VersionedAuthentication<LegacyTcpMultiaddress, TcpAddressingInformation> {
+        handler: SessionHandler<LegacyTcpMultiaddress, SignedTcpAddressingInformation>,
+    ) -> VersionedAuthentication<LegacyTcpMultiaddress, SignedTcpAddressingInformation> {
         match handler
             .authentication()
             .expect("should have authentication")
@@ -342,13 +344,16 @@ mod test {
     fn raw_authentication_v2() -> Vec<u8> {
         //TODO: this will fail, check what it should be
         vec![
-            2, 0, 127, 0, 50, 40, 192, 239, 72, 72, 119, 156, 76, 37, 212, 220, 76, 165, 39, 73,
+            2, 0, 191, 0, 50, 40, 192, 239, 72, 72, 119, 156, 76, 37, 212, 220, 76, 165, 39, 73,
             20, 89, 77, 66, 171, 174, 61, 31, 254, 137, 186, 1, 7, 141, 187, 219, 20, 97, 100, 100,
-            114, 49, 8, 20, 97, 100, 100, 114, 50, 20, 97, 100, 100, 114, 51, 21, 0, 0, 0, 0, 0, 0,
-            0, 37, 0, 0, 0, 62, 4, 215, 148, 82, 197, 128, 124, 68, 183, 132, 114, 101, 15, 49,
-            220, 175, 29, 128, 15, 163, 6, 147, 56, 103, 140, 125, 92, 92, 243, 194, 168, 63, 65,
-            101, 78, 165, 63, 169, 132, 73, 212, 6, 10, 231, 78, 48, 219, 70, 23, 180, 227, 95,
-            141, 111, 60, 245, 119, 27, 84, 187, 33, 77, 2,
+            114, 49, 8, 20, 97, 100, 100, 114, 50, 20, 97, 100, 100, 114, 51, 193, 134, 174, 215,
+            223, 67, 113, 105, 253, 217, 120, 59, 47, 176, 146, 72, 205, 114, 242, 242, 115, 214,
+            97, 112, 69, 56, 119, 168, 164, 170, 74, 7, 97, 149, 53, 122, 42, 209, 198, 146, 6,
+            169, 37, 242, 131, 152, 209, 10, 52, 78, 218, 52, 69, 81, 235, 254, 58, 44, 134, 201,
+            119, 132, 5, 8, 21, 0, 0, 0, 0, 0, 0, 0, 37, 0, 0, 0, 230, 134, 124, 175, 213, 131, 76,
+            99, 89, 247, 169, 129, 87, 134, 249, 172, 99, 77, 203, 254, 12, 171, 178, 163, 47, 145,
+            104, 166, 75, 174, 164, 119, 197, 78, 101, 221, 52, 51, 116, 221, 67, 45, 196, 65, 61,
+            5, 246, 111, 56, 215, 145, 48, 170, 241, 60, 68, 231, 187, 72, 201, 18, 82, 249, 11,
         ]
     }
 

finality-aleph/src/network/manager/session.rs

@@ -152,6 +152,9 @@ impl<M: Data, A: AddressingInformation + TryFrom<Vec<M>> + Into<Vec<M>>> Handler
         let (auth_data, signature) = &authentication;
 
         let address = auth_data.address();
+        if !address.verify() {
+            return None;
+        }
         let peer_id = address.peer_id();
         if peer_id == self.own_peer_id {
             return None;
@@ -274,7 +277,7 @@ mod tests {
             testing::{authentication, legacy_authentication},
             AddressingInformation,
         },
-        testing::mocks::validator_network::random_address,
+        testing::mocks::validator_network::{random_address, random_invalid_address},
         NodeIndex, SessionId,
     };
 
@@ -446,6 +449,31 @@ mod tests {
         assert_eq!(handler0.peer_id(&NodeIndex(1)), Some(peer_id1));
     }
 
+    #[tokio::test]
+    async fn ignores_invalid_authentication() {
+        let crypto_basics = crypto_basics(NUM_NODES).await;
+        let mut handler0 = Handler::new(
+            Some(crypto_basics.0[0].clone()),
+            crypto_basics.1.clone(),
+            SessionId(43),
+            random_address(),
+        )
+        .await;
+        let handler1 = Handler::new(
+            Some(crypto_basics.0[1].clone()),
+            crypto_basics.1.clone(),
+            SessionId(43),
+            random_invalid_address(),
+        )
+        .await;
+        assert!(handler0
+            .handle_authentication(authentication(&handler1))
+            .is_none());
+        let missing_nodes = handler0.missing_nodes();
+        let expected_missing: Vec<_> = (1..NUM_NODES).map(NodeIndex).collect();
+        assert_eq!(missing_nodes, expected_missing);
+    }
+
     #[tokio::test]
     async fn ignores_badly_signed_authentication() {
         let crypto_basics = crypto_basics(NUM_NODES).await;

finality-aleph/src/network/mod.rs

@@ -89,8 +89,11 @@ pub trait PeerId: PartialEq + Eq + Clone + Debug + Display + Hash + Codec + Send
 pub trait AddressingInformation: Debug + Hash + Codec + Clone + Eq + Send + Sync + 'static {
     type PeerId: PeerId;
 
-    /// Returns the peer id associated with this multiaddress if it exists and is unique.
+    /// Returns the peer id associated with this address.
     fn peer_id(&self) -> Self::PeerId;
+
+    /// Verify the information.
+    fn verify(&self) -> bool;
 }
 
 /// The Authentication protocol is used for validator discovery.

finality-aleph/src/nodes/validator_node.rs

@@ -76,7 +76,7 @@ where
     let (dialer, listener, network_identity) = new_tcp_network(
         ("0.0.0.0", validator_port),
         external_addresses,
-        network_authority_pen.authority_id(),
+        &network_authority_pen,
     )
     .await
     .expect("we should have working networking");

finality-aleph/src/tcp_network.rs

@@ -106,9 +106,8 @@ pub enum AddressingInformationError {
     NoAddress,
 }
 
-/// A representation of TCP addressing information with an associated peer ID.
 #[derive(Debug, Hash, Encode, Decode, Clone, PartialEq, Eq)]
-pub struct TcpAddressingInformation {
+struct TcpAddressingInformation {
     peer_id: AuthorityId,
     // Easiest way to ensure that the Vec below is nonempty...
     primary_address: String,
@@ -153,23 +152,6 @@ impl From<TcpAddressingInformation> for Vec<LegacyTcpMultiaddress> {
     }
 }
 
-impl AddressingInformation for TcpAddressingInformation {
-    type PeerId = AuthorityId;
-
-    fn peer_id(&self) -> Self::PeerId {
-        self.peer_id.clone()
-    }
-}
-
-impl NetworkIdentity for TcpAddressingInformation {
-    type PeerId = AuthorityId;
-    type AddressingInformation = TcpAddressingInformation;
-
-    fn identity(&self) -> Self::AddressingInformation {
-        self.clone()
-    }
-}
-
 impl TcpAddressingInformation {
     fn new(
         addresses: Vec<String>,
@@ -186,25 +168,100 @@ impl TcpAddressingInformation {
             peer_id,
         })
     }
+
+    fn peer_id(&self) -> AuthorityId {
+        self.peer_id.clone()
+    }
+}
+
+/// A representation of TCP addressing information with an associated peer ID, self-signed.
+#[derive(Debug, Hash, Encode, Decode, Clone, PartialEq, Eq)]
+pub struct SignedTcpAddressingInformation {
+    addressing_information: TcpAddressingInformation,
+    signature: Signature,
+}
+
+impl TryFrom<Vec<LegacyTcpMultiaddress>> for SignedTcpAddressingInformation {
+    type Error = AddressingInformationError;
+
+    fn try_from(legacy: Vec<LegacyTcpMultiaddress>) -> Result<Self, Self::Error> {
+        let addressing_information = legacy.try_into()?;
+        // This will never get validated, but that is alright and working as intended.
+        // We temporarily accept legacy messages and there is no way to verify them completely,
+        // since they were unsigned previously. In the next update we will remove this, and the
+        // problem will be completely gone.
+        let signature = [0; 64].into();
+        Ok(SignedTcpAddressingInformation {
+            addressing_information,
+            signature,
+        })
+    }
+}
+
+impl From<SignedTcpAddressingInformation> for Vec<LegacyTcpMultiaddress> {
+    fn from(address: SignedTcpAddressingInformation) -> Self {
+        address.addressing_information.into()
+    }
+}
+
+impl AddressingInformation for SignedTcpAddressingInformation {
+    type PeerId = AuthorityId;
+
+    fn peer_id(&self) -> Self::PeerId {
+        self.addressing_information.peer_id()
+    }
+
+    fn verify(&self) -> bool {
+        self.peer_id()
+            .verify(&self.addressing_information.encode(), &self.signature)
+    }
+}
+
+impl NetworkIdentity for SignedTcpAddressingInformation {
+    type PeerId = AuthorityId;
+    type AddressingInformation = SignedTcpAddressingInformation;
+
+    fn identity(&self) -> Self::AddressingInformation {
+        self.clone()
+    }
+}
+
+impl SignedTcpAddressingInformation {
+    async fn new(
+        addresses: Vec<String>,
+        authority_pen: &AuthorityPen,
+    ) -> Result<SignedTcpAddressingInformation, AddressingInformationError> {
+        let peer_id = authority_pen.authority_id();
+        let addressing_information = TcpAddressingInformation::new(addresses, peer_id)?;
+        let signature = authority_pen.sign(&addressing_information.encode()).await;
+        Ok(SignedTcpAddressingInformation {
+            addressing_information,
+            signature,
+        })
+    }
 }
 
 #[derive(Clone)]
 struct TcpDialer;
 
 #[async_trait::async_trait]
-impl Dialer<TcpAddressingInformation> for TcpDialer {
+impl Dialer<SignedTcpAddressingInformation> for TcpDialer {
     type Connection = TcpStream;
     type Error = std::io::Error;
 
     async fn connect(
         &mut self,
-        address: TcpAddressingInformation,
+        address: SignedTcpAddressingInformation,
     ) -> Result<Self::Connection, Self::Error> {
+        let SignedTcpAddressingInformation {
+            addressing_information,
+            ..
+        } = address;
         let TcpAddressingInformation {
             primary_address,
             other_addresses,
             ..
-        } = address;
+        } = addressing_information;
         let parsed_addresses: Vec<_> = iter::once(primary_address)
             .chain(other_addresses)
             .filter_map(|address| address.to_socket_addrs().ok())
@@ -242,34 +299,38 @@ impl From<AddressingInformationError> for Error {
 pub async fn new_tcp_network<A: ToSocketAddrs>(
     listening_addresses: A,
     external_addresses: Vec<String>,
-    peer_id: AuthorityId,
+    authority_pen: &AuthorityPen,
 ) -> Result<
     (
-        impl Dialer<TcpAddressingInformation>,
+        impl Dialer<SignedTcpAddressingInformation>,
         impl Listener,
-        impl NetworkIdentity<AddressingInformation = TcpAddressingInformation, PeerId = AuthorityId>,
+        impl NetworkIdentity<
+            AddressingInformation = SignedTcpAddressingInformation,
+            PeerId = AuthorityId,
+        >,
     ),
     Error,
 > {
     let listener = TcpListener::bind(listening_addresses).await?;
-    let identity = TcpAddressingInformation::new(external_addresses, peer_id)?;
+    let identity = SignedTcpAddressingInformation::new(external_addresses, authority_pen).await?;
     Ok((TcpDialer {}, listener, identity))
 }
 
 #[cfg(test)]
 pub mod testing {
     use aleph_primitives::AuthorityId;
 
-    use super::TcpAddressingInformation;
-    use crate::network::NetworkIdentity;
+    use super::SignedTcpAddressingInformation;
+    use crate::{crypto::AuthorityPen, network::NetworkIdentity};
 
     /// Creates a realistic identity.
-    pub fn new_identity(
+    pub async fn new_identity(
         external_addresses: Vec<String>,
-        peer_id: AuthorityId,
-    ) -> impl NetworkIdentity<AddressingInformation = TcpAddressingInformation, PeerId = AuthorityId>
+        authority_pen: &AuthorityPen,
+    ) -> impl NetworkIdentity<AddressingInformation = SignedTcpAddressingInformation, PeerId = AuthorityId>
     {
-        TcpAddressingInformation::new(external_addresses, peer_id)
+        SignedTcpAddressingInformation::new(external_addresses, authority_pen)
+            .await
             .expect("the provided addresses are fine")
     }
 }