fix(identify): validate public key from remote peer

Related #5706 Discard `Info` messages received from a remote peer that contain a public key that doesn't match their peer ID, and log a warning. Don't emit `identify::Received` events to the swarm containing whatever public key they sent. Pull-Request: #5707.
libp2p · Dec 13, 2024 · c8c1b80 · c8c1b80
1 parent 9f197c2
commit c8c1b80
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 41 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -81,7 +81,7 @@ libp2p-dcutr = { version = "0.12.1", path = "protocols/dcutr" }
 libp2p-dns = { version = "0.42.1", path = "transports/dns" }
 libp2p-floodsub = { version = "0.45.0", path = "protocols/floodsub" }
 libp2p-gossipsub = { version = "0.48.0", path = "protocols/gossipsub" }
-libp2p-identify = { version = "0.46.0", path = "protocols/identify" }
+libp2p-identify = { version = "0.46.1", path = "protocols/identify" }
 libp2p-identity = { version = "0.2.10" }
 libp2p-kad = { version = "0.47.1", path = "protocols/kad" }
 libp2p-mdns = { version = "0.46.1", path = "protocols/mdns" }

diff --git a/protocols/identify/CHANGELOG.md b/protocols/identify/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.46.1
+- Discard `Info`s received from remote peers that contain a public key that doesn't match their peer ID.
+  See [PR 5707](https://github.com/libp2p/rust-libp2p/pull/5707).
+
 ## 0.46.0
 
 - Make `identify::Config` fields private and add getter functions.

diff --git a/protocols/identify/Cargo.toml b/protocols/identify/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-identify"
 edition = "2021"
 rust-version = { workspace = true }
 description = "Nodes identification protocol for libp2p"
-version = "0.46.0"
+version = "0.46.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"

diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs
@@ -242,10 +242,17 @@ impl Handler {
         }
     }
 
-    fn handle_incoming_info(&mut self, info: &Info) {
+    /// If the public key matches the remote peer, handles the given `info` and returns `true`.
+    fn handle_incoming_info(&mut self, info: &Info) -> bool {
+        let derived_peer_id = info.public_key.to_peer_id();
+        if self.remote_peer_id != derived_peer_id {
+            return false;
+        }
+
         self.remote_info.replace(info.clone());
 
         self.update_supported_protocols_for_remote(info);
+        true
     }
 
     fn update_supported_protocols_for_remote(&mut self, remote_info: &Info) {
@@ -344,45 +351,61 @@ impl ConnectionHandler for Handler {
             return Poll::Ready(event);
         }
 
-        match self.active_streams.poll_unpin(cx) {
-            Poll::Ready(Ok(Ok(Success::ReceivedIdentify(remote_info)))) => {
-                self.handle_incoming_info(&remote_info);
-
-                return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(Event::Identified(
-                    remote_info,
-                )));
-            }
-            Poll::Ready(Ok(Ok(Success::SentIdentifyPush(info)))) => {
-                return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
-                    Event::IdentificationPushed(info),
-                ));
-            }
-            Poll::Ready(Ok(Ok(Success::SentIdentify))) => {
-                return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
-                    Event::Identification,
-                ));
-            }
-            Poll::Ready(Ok(Ok(Success::ReceivedIdentifyPush(remote_push_info)))) => {
-                if let Some(mut info) = self.remote_info.clone() {
-                    info.merge(remote_push_info);
-                    self.handle_incoming_info(&info);
-
+        while let Poll::Ready(ready) = self.active_streams.poll_unpin(cx) {
+            match ready {
+                Ok(Ok(Success::ReceivedIdentify(remote_info))) => {
+                    if self.handle_incoming_info(&remote_info) {
+                        return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
+                            Event::Identified(remote_info),
+                        ));
+                    } else {
+                        tracing::warn!(
+                            %self.remote_peer_id,
+                            ?remote_info.public_key,
+                            derived_peer_id=%remote_info.public_key.to_peer_id(),
+                            "Discarding received identify message as public key does not match remote peer ID",
+                        );
+                    }
+                }
+                Ok(Ok(Success::SentIdentifyPush(info))) => {
                     return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
-                        Event::Identified(info),
+                        Event::IdentificationPushed(info),
                     ));
-                };
-            }
-            Poll::Ready(Ok(Err(e))) => {
-                return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
-                    Event::IdentificationError(StreamUpgradeError::Apply(e)),
-                ));
-            }
-            Poll::Ready(Err(Timeout { .. })) => {
-                return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
-                    Event::IdentificationError(StreamUpgradeError::Timeout),
-                ));
+                }
+                Ok(Ok(Success::SentIdentify)) => {
+                    return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
+                        Event::Identification,
+                    ));
+                }
+                Ok(Ok(Success::ReceivedIdentifyPush(remote_push_info))) => {
+                    if let Some(mut info) = self.remote_info.clone() {
+                        info.merge(remote_push_info);
+
+                        if self.handle_incoming_info(&info) {
+                            return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
+                                Event::Identified(info),
+                            ));
+                        } else {
+                            tracing::warn!(
+                                %self.remote_peer_id,
+                                ?info.public_key,
+                                derived_peer_id=%info.public_key.to_peer_id(),
+                                "Discarding received identify message as public key does not match remote peer ID",
+                            );
+                        }
+                    }
+                }
+                Ok(Err(e)) => {
+                    return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
+                        Event::IdentificationError(StreamUpgradeError::Apply(e)),
+                    ));
+                }
+                Err(Timeout { .. }) => {
+                    return Poll::Ready(ConnectionHandlerEvent::NotifyBehaviour(
+                        Event::IdentificationError(StreamUpgradeError::Timeout),
+                    ));
+                }
             }
-            Poll::Pending => {}
         }
 
         Poll::Pending

diff --git a/protocols/identify/src/protocol.rs b/protocols/identify/src/protocol.rs
@@ -39,7 +39,7 @@ pub const PUSH_PROTOCOL_NAME: StreamProtocol = StreamProtocol::new("/ipfs/id/pus
 /// Identify information of a peer sent in protocol messages.
 #[derive(Debug, Clone)]
 pub struct Info {
-    /// The public key of the local peer.
+    /// The public key of the peer.
     pub public_key: PublicKey,
     /// Application-specific version of the protocol family used by the peer,
     /// e.g. `ipfs/1.0.0` or `polkadot/1.0.0`.

diff --git a/protocols/identify/tests/smoke.rs b/protocols/identify/tests/smoke.rs
@@ -6,6 +6,7 @@ use std::{
 
 use futures::StreamExt;
 use libp2p_identify as identify;
+use libp2p_identity::Keypair;
 use libp2p_swarm::{Swarm, SwarmEvent};
 use libp2p_swarm_test::SwarmExt;
 use tracing_subscriber::EnvFilter;
@@ -440,3 +441,45 @@ async fn configured_interval_starts_after_first_identify() {
 
     assert!(time_to_first_identify < identify_interval)
 }
+
+#[async_std::test]
+async fn reject_mismatched_public_key() {
+    let _ = tracing_subscriber::fmt()
+        .with_env_filter(EnvFilter::from_default_env())
+        .try_init();
+
+    let mut honest_swarm = Swarm::new_ephemeral(|identity| {
+        identify::Behaviour::new(
+            identify::Config::new("a".to_string(), identity.public())
+                .with_interval(Duration::from_secs(1)),
+        )
+    });
+    let mut spoofing_swarm = Swarm::new_ephemeral(|_unused_identity| {
+        let arbitrary_public_key = Keypair::generate_ed25519().public();
+        identify::Behaviour::new(
+            identify::Config::new("a".to_string(), arbitrary_public_key)
+                .with_interval(Duration::from_secs(1)),
+        )
+    });
+
+    honest_swarm.listen().with_memory_addr_external().await;
+    spoofing_swarm.connect(&mut honest_swarm).await;
+
+    spoofing_swarm
+        .wait(|event| {
+            matches!(event, SwarmEvent::Behaviour(identify::Event::Sent { .. })).then_some(())
+        })
+        .await;
+
+    let honest_swarm_events = futures::stream::poll_fn(|cx| honest_swarm.poll_next_unpin(cx))
+        .take(4)
+        .collect::<Vec<_>>()
+        .await;
+
+    assert!(
+        !honest_swarm_events
+            .iter()
+            .any(|e| matches!(e, SwarmEvent::Behaviour(identify::Event::Received { .. }))),
+        "should emit no received events as received public key won't match remote peer",
+    );
+}