From ca2919a168751a38f9cf9956df121a6d1f2b8767 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 14 May 2025 13:12:17 +0000 Subject: [PATCH 1/7] crypto/noise: Show peerIDs that fail to decode Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index 2e4b6db0..5ed36632 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -350,6 +350,7 @@ pub struct NoiseSocket { read_buffer: Vec, canonical_max_read: usize, decrypt_buffer: Option>, + peer: PeerId, } impl NoiseSocket { @@ -358,6 +359,7 @@ impl NoiseSocket { noise: NoiseContext, max_read_ahead_factor: usize, max_write_buffer_size: usize, + peer: PeerId, ) -> Self { Self { io, @@ -380,6 +382,7 @@ impl NoiseSocket { max_read: max_read_ahead_factor * MAX_NOISE_MSG_LEN, }, canonical_max_read: max_read_ahead_factor * MAX_NOISE_MSG_LEN, + peer, } } @@ -764,6 +767,7 @@ pub async fn handshake( noise.into_transport()?, max_read_ahead_factor, max_write_buffer_size, + peer, ), peer, )) From 3bd9085b405ed2c109610ac402d0aed58b52cd92 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 14 May 2025 13:13:54 +0000 Subject: [PATCH 2/7] crypto/noise: Add extra debug messages Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index 5ed36632..80564bd2 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -545,7 +545,7 @@ impl AsyncRead for NoiseSocket { buf, ) { Err(error) => { - tracing::error!(target: LOG_TARGET, ?error, "failed to decrypt message"); + tracing::error!(target: LOG_TARGET, ?error, peer = ?this.peer, "failed to decrypt message for bigger buffers"); return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); } Ok(nread) => { @@ -563,7 +563,7 @@ impl AsyncRead for NoiseSocket { &mut buffer, ) { Err(error) => { - tracing::error!(target: LOG_TARGET, ?error, "failed to decrypt message"); + tracing::error!(target: LOG_TARGET, ?error, "failed to decrypt message for smaller buffers"); return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); } Ok(nread) => { From 43cb2db30efbaa7d928d6a56ba268e068fc250ca Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 14 May 2025 13:23:44 +0000 Subject: [PATCH 3/7] crypto/noise: Add peerID to both failures Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index 80564bd2..b28ce9c2 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -545,7 +545,13 @@ impl AsyncRead for NoiseSocket { buf, ) { Err(error) => { - tracing::error!(target: LOG_TARGET, ?error, peer = ?this.peer, "failed to decrypt message for bigger buffers"); + tracing::error!( + target: LOG_TARGET, + ?error, + peer = ?this.peer, + "failed to decrypt message for bigger buffers" + ); + return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); } Ok(nread) => { @@ -563,7 +569,13 @@ impl AsyncRead for NoiseSocket { &mut buffer, ) { Err(error) => { - tracing::error!(target: LOG_TARGET, ?error, "failed to decrypt message for smaller buffers"); + tracing::error!( + target: LOG_TARGET, + ?error, + peer = ?this.peer, + "failed to decrypt message for smaller buffers" + ); + return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); } Ok(nread) => { From 74ee6078dfa90098f468d0726438397749fa4f5b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 22 May 2025 15:54:05 +0000 Subject: [PATCH 4/7] crypto/noise: Add packet info to warnings Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index b28ce9c2..820e2c06 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -549,7 +549,9 @@ impl AsyncRead for NoiseSocket { target: LOG_TARGET, ?error, peer = ?this.peer, - "failed to decrypt message for bigger buffers" + buf_len = ?buf.len(), + frame_size = ?frame_size, + "failed to decrypt message" ); return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); @@ -573,7 +575,9 @@ impl AsyncRead for NoiseSocket { target: LOG_TARGET, ?error, peer = ?this.peer, - "failed to decrypt message for smaller buffers" + buf_len = ?buf.len(), + frame_size = ?frame_size, + "failed to decrypt message for smaller buffer" ); return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); From a1978f606db55d1cc6666a58037c1da34b90e518 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 22 May 2025 16:06:08 +0000 Subject: [PATCH 5/7] crypto/noise: Enhance all logs with connection type and peerID Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 75 +++++++++++++++++++++++---- src/transport/tcp/connection.rs | 5 ++ src/transport/websocket/connection.rs | 5 ++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index 820e2c06..b17c3340 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -351,6 +351,7 @@ pub struct NoiseSocket { canonical_max_read: usize, decrypt_buffer: Option>, peer: PeerId, + ty: HandshakeTransport, } impl NoiseSocket { @@ -360,6 +361,7 @@ impl NoiseSocket { max_read_ahead_factor: usize, max_write_buffer_size: usize, peer: PeerId, + ty: HandshakeTransport, ) -> Self { Self { io, @@ -383,6 +385,7 @@ impl NoiseSocket { }, canonical_max_read: max_read_ahead_factor * MAX_NOISE_MSG_LEN, peer, + ty, } } @@ -427,7 +430,13 @@ impl AsyncRead for NoiseSocket { }, }; - tracing::trace!(target: LOG_TARGET, ?nread, "read data from socket"); + tracing::trace!( + target: LOG_TARGET, + ?nread, + ty = ?this.ty, + peer = ?this.peer, + "read data from socket" + ); this.nread += nread; this.read_state = ReadState::ReadFrameLen; @@ -436,13 +445,25 @@ impl AsyncRead for NoiseSocket { let mut remaining = match this.nread.checked_sub(this.offset) { Some(remaining) => remaining, None => { - tracing::error!(target: LOG_TARGET, "offset is larger than the number of bytes read"); + tracing::error!( + target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, + nread = ?this.nread, + offset = ?this.offset, + "offset is larger than the number of bytes read" + ); return Poll::Ready(Err(io::ErrorKind::PermissionDenied.into())); } }; if remaining < 2 { - tracing::trace!(target: LOG_TARGET, "reset read buffer"); + tracing::trace!( + target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, + "reset read buffer" + ); this.reset_read_state(remaining); continue; } @@ -459,13 +480,20 @@ impl AsyncRead for NoiseSocket { } }; - tracing::trace!(target: LOG_TARGET, "current frame size = {frame_size}"); + tracing::trace!( + target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, + "current frame size = {frame_size}" + ); if remaining < frame_size { // `read_buffer` can fit the full frame size. if this.nread + frame_size < this.canonical_max_read { tracing::trace!( target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, max_size = ?this.canonical_max_read, next_frame_size = ?(this.nread + frame_size), "read buffer can fit the full frame", @@ -478,7 +506,12 @@ impl AsyncRead for NoiseSocket { continue; } - tracing::trace!(target: LOG_TARGET, "use auxiliary buffer extension"); + tracing::trace!( + target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, + "use auxiliary buffer extension" + ); // use the auxiliary memory at the end of the read buffer for reading the // frame @@ -492,6 +525,8 @@ impl AsyncRead for NoiseSocket { if frame_size <= NOISE_EXTRA_ENCRYPT_SPACE { tracing::error!( target: LOG_TARGET, + ty = ?this.ty, + peer = ?this.peer, ?frame_size, max_size = ?NOISE_EXTRA_ENCRYPT_SPACE, "invalid frame size", @@ -547,10 +582,11 @@ impl AsyncRead for NoiseSocket { Err(error) => { tracing::error!( target: LOG_TARGET, - ?error, + ty = ?this.ty, peer = ?this.peer, buf_len = ?buf.len(), frame_size = ?frame_size, + ?error, "failed to decrypt message" ); @@ -573,10 +609,11 @@ impl AsyncRead for NoiseSocket { Err(error) => { tracing::error!( target: LOG_TARGET, - ?error, + ty = ?this.ty, peer = ?this.peer, buf_len = ?buf.len(), frame_size = ?frame_size, + ?error, "failed to decrypt message for smaller buffer" ); @@ -624,7 +661,14 @@ impl AsyncWrite for NoiseSocket { match this.noise.write_message(chunk, &mut this.encrypt_buffer[offset + 2..]) { Err(error) => { - tracing::error!(target: LOG_TARGET, ?error, "failed to encrypt message"); + tracing::error!( + target: LOG_TARGET, + ?error, + ty = ?this.ty, + peer = ?this.peer, + "failed to encrypt message" + ); + return Poll::Ready(Err(io::ErrorKind::InvalidData.into())); } Ok(nwritten) => { @@ -720,6 +764,15 @@ fn parse_and_verify_peer_id( Ok(peer_id) } +/// The type of the transport used for the crypto/noise protocol. +/// +/// This is used for logging purposes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum HandshakeTransport { + Tcp, + WebSocket, +} + /// Perform Noise handshake. pub async fn handshake( mut io: S, @@ -728,9 +781,10 @@ pub async fn handshake( max_read_ahead_factor: usize, max_write_buffer_size: usize, timeout: std::time::Duration, + ty: HandshakeTransport, ) -> Result<(NoiseSocket, PeerId), NegotiationError> { let handle_handshake = async move { - tracing::debug!(target: LOG_TARGET, ?role, "start noise handshake"); + tracing::debug!(target: LOG_TARGET, ?role, ?ty, "start noise handshake"); let mut noise = NoiseContext::new(keypair, role)?; let payload = match role { @@ -746,7 +800,7 @@ pub async fn handshake( let payload = handshake_schema::NoiseHandshakePayload::decode(message) .map_err(ParseError::from) .map_err(|err| { - tracing::error!(target: LOG_TARGET, ?err, "failed to decode remote identity message"); + tracing::error!(target: LOG_TARGET, ?err, ?ty, "failed to decode remote identity message"); err })?; @@ -784,6 +838,7 @@ pub async fn handshake( max_read_ahead_factor, max_write_buffer_size, peer, + ty, ), peer, )) diff --git a/src/transport/tcp/connection.rs b/src/transport/tcp/connection.rs index 8a7806d5..c208af8d 100644 --- a/src/transport/tcp/connection.rs +++ b/src/transport/tcp/connection.rs @@ -438,6 +438,7 @@ impl TcpConnection { max_read_ahead_factor, max_write_buffer_size, substream_open_timeout, + noise::HandshakeTransport::Tcp, ) .await?; @@ -1154,6 +1155,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::Tcp, ) .await .unwrap(); @@ -1212,6 +1214,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::Tcp, ) .await .unwrap(); @@ -1286,6 +1289,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::Tcp, ) .await .unwrap(); @@ -1339,6 +1343,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::Tcp, ) .await .unwrap(); diff --git a/src/transport/websocket/connection.rs b/src/transport/websocket/connection.rs index 7c4dd56f..bd00f470 100644 --- a/src/transport/websocket/connection.rs +++ b/src/transport/websocket/connection.rs @@ -323,6 +323,7 @@ impl WebSocketConnection { max_read_ahead_factor, max_write_buffer_size, substream_open_timeout, + noise::HandshakeTransport::WebSocket, ) .await?; @@ -1094,6 +1095,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::WebSocket, ) .await .unwrap(); @@ -1162,6 +1164,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::WebSocket, ) .await .unwrap(); @@ -1261,6 +1264,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::WebSocket, ) .await .unwrap(); @@ -1320,6 +1324,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::WebSocket, ) .await .unwrap(); From e0f9040ad460c2b80d262cf849263a7747036f77 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 23 May 2025 08:50:21 +0000 Subject: [PATCH 6/7] crypto/noise: Add transport type websocket to tests Signed-off-by: Alexandru Vasile --- src/crypto/noise/mod.rs | 2 ++ src/transport/websocket/connection.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/src/crypto/noise/mod.rs b/src/crypto/noise/mod.rs index b17c3340..775363e7 100644 --- a/src/crypto/noise/mod.rs +++ b/src/crypto/noise/mod.rs @@ -893,6 +893,7 @@ mod tests { MAX_READ_AHEAD_FACTOR, MAX_WRITE_BUFFER_SIZE, std::time::Duration::from_secs(10), + HandshakeTransport::Tcp, ), handshake( io2, @@ -901,6 +902,7 @@ mod tests { MAX_READ_AHEAD_FACTOR, MAX_WRITE_BUFFER_SIZE, std::time::Duration::from_secs(10), + HandshakeTransport::Tcp, ) ); let (mut res1, mut res2) = (res1.unwrap(), res2.unwrap()); diff --git a/src/transport/websocket/connection.rs b/src/transport/websocket/connection.rs index bd00f470..d6dc9268 100644 --- a/src/transport/websocket/connection.rs +++ b/src/transport/websocket/connection.rs @@ -888,6 +888,7 @@ mod tests { 5, 2, std::time::Duration::from_secs(10), + noise::HandshakeTransport::WebSocket, ) .await .unwrap(); From a6a3521f25dc255d6b0dbe6b5209572afce4193f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 23 May 2025 08:55:11 +0000 Subject: [PATCH 7/7] tests: Fix build warnings Signed-off-by: Alexandru Vasile --- src/codec/identity.rs | 2 +- src/protocol/libp2p/kademlia/bucket.rs | 4 ++-- tests/conformance/rust/kademlia.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/codec/identity.rs b/src/codec/identity.rs index 266bff4b..f3e47716 100644 --- a/src/codec/identity.rs +++ b/src/codec/identity.rs @@ -102,7 +102,7 @@ mod tests { let bytes = [3u8; 64]; let mut bytes = BytesMut::from(&bytes[..]); - let decoded = codec.decode(&mut bytes); + assert!(codec.decode(&mut bytes).unwrap().is_none()); } #[test] diff --git a/src/protocol/libp2p/kademlia/bucket.rs b/src/protocol/libp2p/kademlia/bucket.rs index e1b115cf..4c999efc 100644 --- a/src/protocol/libp2p/kademlia/bucket.rs +++ b/src/protocol/libp2p/kademlia/bucket.rs @@ -128,7 +128,7 @@ mod tests { .collect::>(); let target = Key::from(PeerId::random()); - let mut iter = bucket.closest_iter(&target); + let iter = bucket.closest_iter(&target); let mut prev = None; for node in iter { @@ -173,7 +173,7 @@ mod tests { .collect::>(); let target = Key::from(PeerId::random()); - let mut iter = bucket.closest_iter(&target); + let iter = bucket.closest_iter(&target); let mut prev = None; let mut num_peers = 0usize; diff --git a/tests/conformance/rust/kademlia.rs b/tests/conformance/rust/kademlia.rs index 8c9afbfa..84761914 100644 --- a/tests/conformance/rust/kademlia.rs +++ b/tests/conformance/rust/kademlia.rs @@ -24,7 +24,7 @@ use libp2p::{ identify, identity, kad::{ self, store::RecordStore, AddProviderOk, GetProvidersOk, InboundRequest, - KademliaEvent as Libp2pKademliaEvent, QueryResult, RecordKey as Libp2pRecordKey, + KademliaEvent as Libp2pKademliaEvent, QueryResult, }, swarm::{keep_alive, AddressScore, NetworkBehaviour, SwarmBuilder, SwarmEvent}, PeerId, Swarm,