diff --git a/Cargo.lock b/Cargo.lock index 18b579f6d2c..271ab64b919 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2900,7 +2900,7 @@ dependencies = [ [[package]] name = "libp2p-identify" -version = "0.46.0" +version = "0.46.1" dependencies = [ "async-std", "asynchronous-codec", diff --git a/Cargo.toml b/Cargo.toml index e0feda0392a..c77768db311 100644 --- 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 index 9051c331bbc..2b136740156 100644 --- 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 index d7f6b6eca76..d2aeb74e626 100644 --- 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 "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/protocols/identify/src/handler.rs b/protocols/identify/src/handler.rs index cda49f992b8..6e5af290cd2 100644 --- 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 index 33aeedb7c4f..257ec1f88d2 100644 --- 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 index dd48b314173..0d2818df0a4 100644 --- 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::>() + .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", + ); +}