diff --git a/Cargo.lock b/Cargo.lock index 0172af429ee1..d8c479976860 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3548,8 +3548,7 @@ dependencies = [ [[package]] name = "multiaddr" version = "0.18.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92a651988b3ed3ad1bc8c87d016bb92f6f395b84ed1db9b926b32b1fc5a2c8b5" +source = "git+https://github.com/multiformats/rust-multiaddr?rev=d0e9ec219645ce8bd8ae7926e4b7316a67c35f84#d0e9ec219645ce8bd8ae7926e4b7316a67c35f84" dependencies = [ "arrayref", "byteorder", diff --git a/Cargo.toml b/Cargo.toml index 2c823756bbee..4859b07e3f89 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,7 +112,8 @@ libp2p-websocket = { version = "0.43.0", path = "transports/websocket" } libp2p-websocket-websys = { version = "0.3.0", path = "transports/websocket-websys" } libp2p-webtransport-websys = { version = "0.2.0", path = "transports/webtransport-websys" } libp2p-yamux = { version = "0.45.0", path = "muxers/yamux" } -multiaddr = "0.18.0" +# multiaddr = "0.18.0" +multiaddr = { git = "https://github.com/multiformats/rust-multiaddr", rev = "d0e9ec219645ce8bd8ae7926e4b7316a67c35f84" } multihash = "0.19.1" multistream-select = { version = "0.13.0", path = "misc/multistream-select" } prometheus-client = "0.22.0" diff --git a/protocols/identify/CHANGELOG.md b/protocols/identify/CHANGELOG.md index 0b47163b51a7..dd8af46e9c43 100644 --- a/protocols/identify/CHANGELOG.md +++ b/protocols/identify/CHANGELOG.md @@ -2,6 +2,8 @@ - Add `Info` to the `libp2p-identify::Event::Pushed` to report pushed info. See [PR 4527](https://github.com/libp2p/rust-libp2p/pull/4527) +- Ensure `Multiaddr` handled and returned by `Behaviour` are `/p2p` terminated. + See [PR 4596](https://github.com/libp2p/rust-libp2p/pull/4596). ## 0.43.1 diff --git a/protocols/identify/src/behaviour.rs b/protocols/identify/src/behaviour.rs index d58bcb4f5eb7..93dc30f76a07 100644 --- a/protocols/identify/src/behaviour.rs +++ b/protocols/identify/src/behaviour.rs @@ -471,6 +471,7 @@ impl PeerCache { Some(cache) => cache, }; + let addresses = addresses.filter_map(|a| a.with_p2p(peer).ok()); cache.put(peer, HashSet::from_iter(addresses)); } diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index d2b92195ab38..cf4a768f4bdf 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -11,6 +11,8 @@ - Remove previously deprecated type-aliases. Users should follow the convention of importing the `libp2p::kad` module and referring to symbols as `kad::Behaviour` etc. See [PR 4733](https://github.com/libp2p/rust-libp2p/pull/4733). +- Ensure `Multiaddr` handled and returned by `Behaviour` are `/p2p` terminated. + See [PR 4596](https://github.com/libp2p/rust-libp2p/pull/4596). ## 0.44.6 diff --git a/protocols/kad/src/addresses.rs b/protocols/kad/src/addresses.rs index 3c2af4173fde..c82e0d82ab93 100644 --- a/protocols/kad/src/addresses.rs +++ b/protocols/kad/src/addresses.rs @@ -23,6 +23,7 @@ use smallvec::SmallVec; use std::fmt; /// A non-empty list of (unique) addresses of a peer in the routing table. +/// Every address must be a fully-qualified /p2p address. #[derive(Clone)] pub struct Addresses { addrs: SmallVec<[Multiaddr; 6]>, diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 0b187955e39d..e0aa5ce31a47 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -513,6 +513,10 @@ where /// If the routing table has been updated as a result of this operation, /// a [`Event::RoutingUpdated`] event is emitted. pub fn add_address(&mut self, peer: &PeerId, address: Multiaddr) -> RoutingUpdate { + // ensuring address is a fully-qualified /p2p multiaddr + let Ok(address) = address.with_p2p(*peer) else { + return RoutingUpdate::Failed; + }; let key = kbucket::Key::from(*peer); match self.kbuckets.entry(&key) { kbucket::Entry::Present(mut entry, _) => { @@ -593,6 +597,7 @@ where peer: &PeerId, address: &Multiaddr, ) -> Option, Addresses>> { + let address = &address.to_owned().with_p2p(*peer).ok()?; let key = kbucket::Key::from(*peer); match self.kbuckets.entry(&key) { kbucket::Entry::Present(mut entry, _) => { diff --git a/protocols/kad/src/protocol.rs b/protocols/kad/src/protocol.rs index 1cf147456756..359e9e67c50b 100644 --- a/protocols/kad/src/protocol.rs +++ b/protocols/kad/src/protocol.rs @@ -103,8 +103,11 @@ impl TryFrom for KadPeer { let mut addrs = Vec::with_capacity(peer.addrs.len()); for addr in peer.addrs.into_iter() { - match Multiaddr::try_from(addr) { - Ok(a) => addrs.push(a), + match Multiaddr::try_from(addr).map(|addr| addr.with_p2p(node_id)) { + Ok(Ok(a)) => addrs.push(a), + Ok(Err(a)) => { + log::debug!("Unable to parse multiaddr: {a} is not compatible with {node_id}"); + } Err(e) => { log::debug!("Unable to parse multiaddr: {e}"); } @@ -596,10 +599,34 @@ where mod tests { use super::*; + #[test] + fn append_p2p() { + let peer_id = PeerId::random(); + let multiaddr = "/ip6/2001:db8::/tcp/1234".parse::().unwrap(); + + let payload = proto::Peer { + id: peer_id.to_bytes(), + addrs: vec![multiaddr.to_vec()], + connection: proto::ConnectionType::CAN_CONNECT, + }; + + let peer = KadPeer::try_from(payload).unwrap(); + + assert_eq!(peer.multiaddrs, vec![multiaddr.with_p2p(peer_id).unwrap()]) + } + #[test] fn skip_invalid_multiaddr() { - let valid_multiaddr: Multiaddr = "/ip6/2001:db8::/tcp/1234".parse().unwrap(); - let valid_multiaddr_bytes = valid_multiaddr.to_vec(); + let peer_id = PeerId::random(); + let multiaddr = "/ip6/2001:db8::/tcp/1234".parse::().unwrap(); + + let valid_multiaddr = multiaddr.clone().with_p2p(peer_id).unwrap(); + + let multiaddr_with_incorrect_peer_id = { + let other_peer_id = PeerId::random(); + assert_ne!(peer_id, other_peer_id); + multiaddr.with_p2p(other_peer_id).unwrap() + }; let invalid_multiaddr = { let a = vec![255; 8]; @@ -608,12 +635,16 @@ mod tests { }; let payload = proto::Peer { - id: PeerId::random().to_bytes(), - addrs: vec![valid_multiaddr_bytes, invalid_multiaddr], + id: peer_id.to_bytes(), + addrs: vec![ + valid_multiaddr.to_vec(), + multiaddr_with_incorrect_peer_id.to_vec(), + invalid_multiaddr, + ], connection: proto::ConnectionType::CAN_CONNECT, }; - let peer = KadPeer::try_from(payload).expect("not to fail"); + let peer = KadPeer::try_from(payload).unwrap(); assert_eq!(peer.multiaddrs, vec![valid_multiaddr]) } diff --git a/protocols/mdns/CHANGELOG.md b/protocols/mdns/CHANGELOG.md index 060fac8c51c7..e84457f83843 100644 --- a/protocols/mdns/CHANGELOG.md +++ b/protocols/mdns/CHANGELOG.md @@ -2,6 +2,8 @@ - Don't perform IO in `Behaviour::poll`. See [PR 4623](https://github.com/libp2p/rust-libp2p/pull/4623). +- Ensure `Multiaddr` handled and returned by `Behaviour` are `/p2p` terminated. + See [PR 4596](https://github.com/libp2p/rust-libp2p/pull/4596). ## 0.44.0 diff --git a/protocols/mdns/src/behaviour/iface/query.rs b/protocols/mdns/src/behaviour/iface/query.rs index 0185028f6ff3..0d2a486762ed 100644 --- a/protocols/mdns/src/behaviour/iface/query.rs +++ b/protocols/mdns/src/behaviour/iface/query.rs @@ -181,6 +181,7 @@ impl MdnsResponse { peer.addresses().iter().filter_map(move |address| { let new_addr = address_translation(address, &observed)?; + let new_addr = new_addr.with_p2p(*peer.id()).ok()?; Some((*peer.id(), new_addr, new_expiration)) }) diff --git a/swarm/src/lib.rs b/swarm/src/lib.rs index 908936069e0b..ea988d8b7900 100644 --- a/swarm/src/lib.rs +++ b/swarm/src/lib.rs @@ -135,7 +135,6 @@ use dial_opts::{DialOpts, PeerCondition}; use futures::{prelude::*, stream::FusedStream}; use libp2p_core::{ connection::ConnectedPoint, - multiaddr, muxing::StreamMuxerBox, transport::{self, ListenerId, TransportError, TransportEvent}, Endpoint, Multiaddr, Transport, @@ -513,24 +512,30 @@ where let dials = addresses .into_iter() - .map(|a| match p2p_addr(peer_id, a) { - Ok(address) => { - let dial = match dial_opts.role_override() { - Endpoint::Dialer => self.transport.dial(address.clone()), - Endpoint::Listener => self.transport.dial_as_listener(address.clone()), - }; - match dial { - Ok(fut) => fut - .map(|r| (address, r.map_err(TransportError::Other))) - .boxed(), - Err(err) => futures::future::ready((address, Err(err))).boxed(), + .map(|address| { + let address_result = match peer_id { + Some(peer_id) => address.with_p2p(peer_id), + None => Ok(address), + }; + match address_result { + Ok(address) => { + let dial = match dial_opts.role_override() { + Endpoint::Dialer => self.transport.dial(address.clone()), + Endpoint::Listener => self.transport.dial_as_listener(address.clone()), + }; + match dial { + Ok(fut) => fut + .map(|r| (address, r.map_err(TransportError::Other))) + .boxed(), + Err(err) => futures::future::ready((address, Err(err))).boxed(), + } } + Err(address) => futures::future::ready(( + address.clone(), + Err(TransportError::MultiaddrNotSupported(address)), + )) + .boxed(), } - Err(address) => futures::future::ready(( - address.clone(), - Err(TransportError::MultiaddrNotSupported(address)), - )) - .boxed(), }) .collect(); @@ -1703,33 +1708,6 @@ impl NetworkInfo { } } -/// Ensures a given `Multiaddr` is a `/p2p/...` address for the given peer. -/// -/// If the given address is already a `p2p` address for the given peer, -/// i.e. the last encapsulated protocol is `/p2p/`, this is a no-op. -/// -/// If the given address is already a `p2p` address for a different peer -/// than the one given, the given `Multiaddr` is returned as an `Err`. -/// -/// If the given address is not yet a `p2p` address for the given peer, -/// the `/p2p/` protocol is appended to the returned address. -fn p2p_addr(peer: Option, addr: Multiaddr) -> Result { - let peer = match peer { - Some(p) => p, - None => return Ok(addr), - }; - - if let Some(multiaddr::Protocol::P2p(peer_id)) = addr.iter().last() { - if peer_id != peer { - return Err(addr); - } - - return Ok(addr); - } - - Ok(addr.with(multiaddr::Protocol::P2p(peer))) -} - #[cfg(test)] mod tests { use super::*;