From e4d67c504d1a69c6f66a33b17106e1185bba9a67 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Sat, 24 Dec 2022 14:20:55 +0100 Subject: [PATCH] fix(identify): Don't fail on unknown multiaddr protocol (#3279) With this commit `libp2p-identify` no longer discards the whole identify payload in case a listen addr of the remote node is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining identify payload. This is especially relevant when rolling out a new protocol to a live network. Say that most nodes of a network run on an implementation version v1. Say that the `multiaddr` implementation is not aware of the `webrtc/` protocol. Say that a new version (v2) is rolled out to the network with support for the `webrtc/` protocol, listening via `webrtc/` by default. In such case all v1 nodes would discard all identify payloads of v2 nodes, given that the v2 identify payloads would contain the `webrtc/` protocol in their `listen_addr` addresses. See https://github.com/libp2p/rust-libp2p/issues/3244 for details. --- protocols/identify/CHANGELOG.md | 6 ++++ protocols/identify/src/protocol.rs | 46 ++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/protocols/identify/CHANGELOG.md b/protocols/identify/CHANGELOG.md index 5478793f44f..e4ce7498f32 100644 --- a/protocols/identify/CHANGELOG.md +++ b/protocols/identify/CHANGELOG.md @@ -9,6 +9,12 @@ [PR 3208]: https://github.com/libp2p/rust-libp2p/pull/3208 +# 0.41.1 + +- Skip invalid multiaddr in `listen_addrs`. See [PR 3246]. + +[PR 3246]: https://github.com/libp2p/rust-libp2p/pull/3246 + # 0.41.0 - Change default `cache_size` of `Config` to 100. See [PR 2995]. diff --git a/protocols/identify/src/protocol.rs b/protocols/identify/src/protocol.rs index 12e6de3f302..6da88c2ec6f 100644 --- a/protocols/identify/src/protocol.rs +++ b/protocols/identify/src/protocol.rs @@ -27,7 +27,7 @@ use libp2p_core::{ upgrade::{InboundUpgrade, OutboundUpgrade, UpgradeInfo}, Multiaddr, PublicKey, }; -use log::trace; +use log::{debug, trace}; use std::convert::TryFrom; use std::{io, iter, pin::Pin}; use thiserror::Error; @@ -220,14 +220,25 @@ impl TryFrom for Info { let listen_addrs = { let mut addrs = Vec::new(); for addr in msg.listen_addrs.into_iter() { - addrs.push(parse_multiaddr(addr)?); + match parse_multiaddr(addr) { + Ok(a) => addrs.push(a), + Err(e) => { + debug!("Unable to parse multiaddr: {e:?}"); + } + } } addrs }; let public_key = PublicKey::from_protobuf_encoding(&msg.public_key.unwrap_or_default())?; - let observed_addr = parse_multiaddr(msg.observed_addr.unwrap_or_default())?; + let observed_addr = match parse_multiaddr(msg.observed_addr.unwrap_or_default()) { + Ok(a) => a, + Err(e) => { + debug!("Unable to parse multiaddr: {e:?}"); + Multiaddr::empty() + } + }; let info = Info { public_key, protocol_version: msg.protocol_version.unwrap_or_default(), @@ -349,4 +360,33 @@ mod tests { bg_task.await; }); } + + #[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 = structs_proto::Identify { + agent_version: None, + listen_addrs: vec![valid_multiaddr_bytes, invalid_multiaddr], + observed_addr: None, + protocol_version: None, + protocols: vec![], + public_key: Some( + identity::Keypair::generate_ed25519() + .public() + .to_protobuf_encoding(), + ), + }; + + let info = Info::try_from(payload).expect("not to fail"); + + assert_eq!(info.listen_addrs, vec![valid_multiaddr]) + } }