From 224d219af146aaf7dfffa1e99143c679938a44cb Mon Sep 17 00:00:00 2001 From: Max Inden Date: Fri, 23 Dec 2022 15:53:54 +0100 Subject: [PATCH 1/5] fix(kad): Skip invalid multiaddr With this commit `libp2p-kad` no longer discards the whole peer payload in case an addr is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining payload. See https://github.com/libp2p/rust-libp2p/issues/3244 for details. --- protocols/kad/CHANGELOG.md | 6 ++++++ protocols/kad/Cargo.toml | 4 ++-- protocols/kad/src/protocol.rs | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index 842550618c1..a3b2ffe51bc 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -1,3 +1,9 @@ +# 0.42.1 + +- Skip invalid multiaddr in `Peer` `addrs`. See [PR XXX]. + +[PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX + # 0.42.0 - Update to `libp2p-core` `v0.38.0`. diff --git a/protocols/kad/Cargo.toml b/protocols/kad/Cargo.toml index 195cd763190..313b999b970 100644 --- a/protocols/kad/Cargo.toml +++ b/protocols/kad/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-kad" edition = "2021" rust-version = "1.62.0" description = "Kademlia protocol for libp2p" -version = "0.42.0" +version = "0.42.1" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" @@ -44,7 +44,7 @@ prost-build = "0.11" [features] serde = ["dep:serde", "bytes/serde"] -# Passing arguments to the docsrs builder in order to properly document cfg's. +# Passing arguments to the docsrs builder in order to properly document cfg's. # More information: https://docs.rs/about/builds#cross-compiling [package.metadata.docs.rs] all-features = true diff --git a/protocols/kad/src/protocol.rs b/protocols/kad/src/protocol.rs index 707edd8fe02..6f67853afc8 100644 --- a/protocols/kad/src/protocol.rs +++ b/protocols/kad/src/protocol.rs @@ -35,6 +35,7 @@ use futures::prelude::*; use instant::Instant; use libp2p_core::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo}; use libp2p_core::{Multiaddr, PeerId}; +use log::debug; use prost::Message; use std::{borrow::Cow, convert::TryFrom, time::Duration}; use std::{io, iter}; @@ -105,10 +106,13 @@ impl TryFrom for KadPeer { let mut addrs = Vec::with_capacity(peer.addrs.len()); for addr in peer.addrs.into_iter() { - let as_ma = Multiaddr::try_from(addr).map_err(invalid_data)?; - addrs.push(as_ma); + match Multiaddr::try_from(addr) { + Ok(a) => addrs.push(a), + Err(e) => { + debug!("Unable to parse multiaddr: {e:?}"); + } + }; } - debug_assert_eq!(addrs.len(), addrs.capacity()); let connection_ty = proto::message::ConnectionType::from_i32(peer.connection) .ok_or_else(|| invalid_data("unknown connection type"))? @@ -601,6 +605,29 @@ where #[cfg(test)] mod tests { + use super::*; + + #[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 invalid_multiaddr = { + let a = vec![255; 8]; + assert!(Multiaddr::try_from(a.clone()).is_err()); + a + }; + + let payload = proto::message::Peer { + id: PeerId::random().to_bytes(), + addrs: vec![valid_multiaddr_bytes, invalid_multiaddr], + connection: proto::message::ConnectionType::CanConnect.into(), + }; + + let peer = KadPeer::try_from(payload).expect("not to fail"); + + assert_eq!(peer.multiaddrs, vec![valid_multiaddr]) + } /*// TODO: restore use self::libp2p_tcp::TcpTransport; From f14933785d70e7c01a80c2ad5c53822714dda131 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 24 Dec 2022 09:53:01 +0100 Subject: [PATCH 2/5] Update protocols/kad/CHANGELOG.md Co-authored-by: Thomas Eizinger --- protocols/kad/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index a3b2ffe51bc..13a1de82c30 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -1,6 +1,6 @@ # 0.42.1 -- Skip invalid multiaddr in `Peer` `addrs`. See [PR XXX]. +- Skip unparsable multiaddr in `Peer::addrs`. See [PR XXX]. [PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX From 98e6eeb758f91d898ecda7f9f0bbb81621ecba0c Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 24 Dec 2022 09:53:32 +0100 Subject: [PATCH 3/5] Update protocols/kad/CHANGELOG.md --- protocols/kad/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index 13a1de82c30..9eb24500ae6 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -1,8 +1,8 @@ # 0.42.1 -- Skip unparsable multiaddr in `Peer::addrs`. See [PR XXX]. +- Skip unparsable multiaddr in `Peer::addrs`. See [PR 3280]. -[PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX +[PR 3280]: https://github.com/libp2p/rust-libp2p/pull/3280 # 0.42.0 From 5a13b090497e98a7cfa6afc0d1b8911d7e4ff85e Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 24 Dec 2022 09:53:59 +0100 Subject: [PATCH 4/5] Update protocols/kad/src/protocol.rs --- protocols/kad/src/protocol.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/protocols/kad/src/protocol.rs b/protocols/kad/src/protocol.rs index 6f67853afc8..fb28356471e 100644 --- a/protocols/kad/src/protocol.rs +++ b/protocols/kad/src/protocol.rs @@ -35,7 +35,6 @@ use futures::prelude::*; use instant::Instant; use libp2p_core::upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo}; use libp2p_core::{Multiaddr, PeerId}; -use log::debug; use prost::Message; use std::{borrow::Cow, convert::TryFrom, time::Duration}; use std::{io, iter}; From 537c396aa32d61e96eb4ff6c4d9ef983c024d2d3 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 24 Dec 2022 09:54:20 +0100 Subject: [PATCH 5/5] Update protocols/kad/src/protocol.rs Co-authored-by: Thomas Eizinger --- protocols/kad/src/protocol.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/kad/src/protocol.rs b/protocols/kad/src/protocol.rs index fb28356471e..3988d08f24d 100644 --- a/protocols/kad/src/protocol.rs +++ b/protocols/kad/src/protocol.rs @@ -108,7 +108,7 @@ impl TryFrom for KadPeer { match Multiaddr::try_from(addr) { Ok(a) => addrs.push(a), Err(e) => { - debug!("Unable to parse multiaddr: {e:?}"); + log::debug!("Unable to parse multiaddr: {e}"); } }; }