Skip to content

Commit

Permalink
fix(identify): Don't fail on unknown multiaddr protocol (#3279)
Browse files Browse the repository at this point in the history
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 #3244 for details.
  • Loading branch information
mxinden committed Dec 24, 2022
1 parent 7dae650 commit e4d67c5
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
6 changes: 6 additions & 0 deletions protocols/identify/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
46 changes: 43 additions & 3 deletions protocols/identify/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -220,14 +220,25 @@ impl TryFrom<structs_proto::Identify> 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(),
Expand Down Expand Up @@ -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])
}
}

0 comments on commit e4d67c5

Please sign in to comment.