From 51ab40b02bcb72543bf1e1d2909c64748ace35bf Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 23 Apr 2025 19:48:42 +0300 Subject: [PATCH 1/7] identify: Verify provided public key Signed-off-by: Alexandru Vasile --- src/protocol/libp2p/identify.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 051b57fd..3710795e 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -329,6 +329,16 @@ impl Identify { tracing::trace!(target: LOG_TARGET, ?peer, ?info, "peer identified"); + // The check ensures the provided public key is the same one as the + // one exchanged during the connection establishment. + if let Some(public_key) = &info.public_key { + let public_key = PublicKey::from_protobuf_encoding(&public_key)?; + if public_key.to_peer_id() != peer { + tracing::debug!(target: LOG_TARGET, ?peer, "peer identified provided invalid public key"); + return Err(Error::InvalidData); + } + } + let listen_addresses = info .listen_addrs .iter() From c8752c29da0a8a8d6e4a7fcffac3216d9de65e18 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 23 Apr 2025 19:53:05 +0300 Subject: [PATCH 2/7] identify: Check the provided address ends with the peer ID Signed-off-by: Alexandru Vasile --- src/protocol/libp2p/identify.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 3710795e..a4816b40 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -342,8 +342,24 @@ impl Identify { let listen_addresses = info .listen_addrs .iter() - .filter_map(|address| Multiaddr::try_from(address.clone()).ok()) + .filter_map(|address| { + let address = Multiaddr::try_from(address.clone()).ok()?; + + // Ensure the address ends with the provided peer ID and is not empty. + if address.is_empty() { + return None; + } + if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { + if peer_id != peer.into() { + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid address"); + return None; + } + } + + Some(address) + }) .collect(); + let observed_address = info.observed_addr.and_then(|address| Multiaddr::try_from(address).ok()); let protocol_version = info.protocol_version; From 3d4c88645493d8b369a985551ae502ded9179b86 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 23 Apr 2025 20:00:56 +0300 Subject: [PATCH 3/7] identify: Ensure the observed address coresponds to us Signed-off-by: Alexandru Vasile --- src/protocol/libp2p/identify.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index a4816b40..42649a4d 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -172,6 +172,9 @@ pub(crate) struct Identify { // Public key of the local node, filled by `Litep2p`. public: PublicKey, + /// Local peer ID. + local_peer_id: PeerId, + /// Protocol version. protocol_version: String, @@ -191,11 +194,15 @@ pub(crate) struct Identify { impl Identify { /// Create new [`Identify`] protocol. pub(crate) fn new(service: TransportService, config: Config) -> Self { + let public = config.public.expect("public key to be supplied"); + let local_peer_id = public.to_peer_id(); + Self { service, tx: config.tx_event, peers: HashMap::new(), - public: config.public.expect("public key to be supplied"), + public, + local_peer_id, protocol_version: config.protocol_version, user_agent: config.user_agent.unwrap_or(DEFAULT_AGENT.to_string()), pending_inbound: FuturesStream::new(), @@ -313,6 +320,8 @@ impl Identify { "outbound substream opened" ); + let local_peer_id = self.local_peer_id.clone(); + self.pending_outbound.push(Box::pin(async move { let payload = match tokio::time::timeout(Duration::from_secs(10), substream.next()).await { @@ -361,7 +370,23 @@ impl Identify { .collect(); let observed_address = - info.observed_addr.and_then(|address| Multiaddr::try_from(address).ok()); + info.observed_addr.and_then(|address| { + let address = Multiaddr::try_from(address).ok()?; + + if address.is_empty() { + return None; + } + + if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { + if peer_id != local_peer_id.into() { + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid address"); + return None; + } + } + + Some(address) + }); + let protocol_version = info.protocol_version; let user_agent = info.agent_version; From 0199557e278eab73395d22ae1dc0562241d4ec8e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 24 Apr 2025 17:05:37 +0300 Subject: [PATCH 4/7] identify: Add more debug logs Signed-off-by: Alexandru Vasile --- src/protocol/libp2p/identify.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 42649a4d..05a9b19a 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -334,14 +334,22 @@ impl Identify { Ok(Some(Ok(payload))) => payload, }; - let info = identify_schema::Identify::decode(payload.to_vec().as_slice())?; + let info = identify_schema::Identify::decode(payload.to_vec().as_slice()).map_err( + |err| { + tracing::debug!(target: LOG_TARGET, ?peer, ?err, "peer identified provided undecodable identify response"); + err + })?; tracing::trace!(target: LOG_TARGET, ?peer, ?info, "peer identified"); // The check ensures the provided public key is the same one as the // one exchanged during the connection establishment. if let Some(public_key) = &info.public_key { - let public_key = PublicKey::from_protobuf_encoding(&public_key)?; + let public_key = PublicKey::from_protobuf_encoding(&public_key).map_err(|err| { + tracing::debug!(target: LOG_TARGET, ?peer, ?err, "peer identified provided undecodable public key"); + err + })?; + if public_key.to_peer_id() != peer { tracing::debug!(target: LOG_TARGET, ?peer, "peer identified provided invalid public key"); return Err(Error::InvalidData); @@ -356,11 +364,12 @@ impl Identify { // Ensure the address ends with the provided peer ID and is not empty. if address.is_empty() { + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided empty listen address"); return None; } if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { if peer_id != peer.into() { - tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid address"); + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid listen address"); return None; } } @@ -374,12 +383,13 @@ impl Identify { let address = Multiaddr::try_from(address).ok()?; if address.is_empty() { + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided empty observed address"); return None; } if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { if peer_id != local_peer_id.into() { - tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid address"); + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid observed address"); return None; } } From f4b0c6b2dfe4b2b42d0fe9e50517acfa04eaf7de Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 28 Apr 2025 13:01:01 +0300 Subject: [PATCH 5/7] identify: Update expect message Signed-off-by: Alexandru Vasile --- src/protocol/libp2p/identify.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 05a9b19a..9983df6f 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -194,7 +194,10 @@ pub(crate) struct Identify { impl Identify { /// Create new [`Identify`] protocol. pub(crate) fn new(service: TransportService, config: Config) -> Self { - let public = config.public.expect("public key to be supplied"); + // The public key is always supplied by litep2p and is the one + // used to identify the local peer. This is a similar story to the + // supported protocols. + let public = config.public.expect("public key to always be supplied by litep2p; qed"); let local_peer_id = public.to_peer_id(); Self { From 139107544863fbd2998cea809234d861660d0f94 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Mon, 28 Apr 2025 13:02:34 +0300 Subject: [PATCH 6/7] Update src/protocol/libp2p/identify.rs Co-authored-by: Dmitry Markin --- src/protocol/libp2p/identify.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 9983df6f..1b2f9e72 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -372,7 +372,7 @@ impl Identify { } if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { if peer_id != peer.into() { - tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid listen address"); + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided listen address with incorrect peer ID; discarding the address"); return None; } } From 27a6f3d4cb2278eddb7a5b06d546b65a42191ca7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Mon, 28 Apr 2025 13:02:52 +0300 Subject: [PATCH 7/7] Update src/protocol/libp2p/identify.rs Co-authored-by: Dmitry Markin --- src/protocol/libp2p/identify.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protocol/libp2p/identify.rs b/src/protocol/libp2p/identify.rs index 1b2f9e72..1dee1336 100644 --- a/src/protocol/libp2p/identify.rs +++ b/src/protocol/libp2p/identify.rs @@ -392,7 +392,7 @@ impl Identify { if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() { if peer_id != local_peer_id.into() { - tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided invalid observed address"); + tracing::debug!(target: LOG_TARGET, ?peer, ?address, "peer identified provided observed address with peer ID not matching our peer ID; discarding address"); return None; } }