Skip to content

Commit 9a63169

Browse files
MegaRedHandd-roak
authored andcommitted
fix(l1): ignore unknown protocols in capability exchange (lambdaclass#3543)
**Motivation** Failing due to a peer having extra capabilities can make us lose exceptional peers. Hence, we want to ignore any extra capabilities they have. **Description** This PR changes `Capability.protocol` to be an 8-byte array instead of a string, allowing us to store any string we receive.
1 parent 621d6ef commit 9a63169

File tree

3 files changed

+75
-25
lines changed

3 files changed

+75
-25
lines changed

crates/networking/p2p/rlpx/connection/server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ where
534534

535535
// Check if we have any capability in common and store the highest version
536536
for cap in &hello_message.capabilities {
537-
match cap.protocol {
537+
match cap.protocol() {
538538
"eth" => {
539539
if SUPPORTED_ETH_CAPABILITIES.contains(cap)
540540
&& cap.version > negotiated_eth_version

crates/networking/p2p/rlpx/p2p.rs

Lines changed: 72 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,54 +18,83 @@ pub const SUPPORTED_ETH_CAPABILITIES: [Capability; 1] = [Capability::eth(68)];
1818
pub const SUPPORTED_SNAP_CAPABILITIES: [Capability; 1] = [Capability::snap(1)];
1919
pub const SUPPORTED_P2P_CAPABILITIES: [Capability; 1] = [Capability::p2p(5)];
2020

21+
const CAPABILITY_NAME_MAX_LENGTH: usize = 8;
22+
23+
// Pads the input array to the right with zeros to ensure it is 8 bytes long.
24+
// Panics if the input is longer than 8 bytes.
25+
const fn pad_right<const N: usize>(input: &[u8; N]) -> [u8; 8] {
26+
assert!(
27+
N <= CAPABILITY_NAME_MAX_LENGTH,
28+
"Input array must be 8 bytes or less"
29+
);
30+
31+
let mut padded = [0_u8; CAPABILITY_NAME_MAX_LENGTH];
32+
let mut i = 0;
33+
while i < input.len() {
34+
padded[i] = input[i];
35+
i += 1;
36+
}
37+
padded
38+
}
39+
2140
#[derive(Debug, Clone, PartialEq)]
41+
/// A capability is identified by a short ASCII name (max eight characters) and version number
2242
pub struct Capability {
23-
pub protocol: &'static str,
43+
protocol: [u8; CAPABILITY_NAME_MAX_LENGTH],
2444
pub version: u8,
2545
}
2646

2747
impl Capability {
2848
pub const fn eth(version: u8) -> Self {
2949
Capability {
30-
protocol: "eth",
50+
protocol: pad_right(b"eth"),
3151
version,
3252
}
3353
}
3454

3555
pub const fn p2p(version: u8) -> Self {
3656
Capability {
37-
protocol: "p2p",
57+
protocol: pad_right(b"p2p"),
3858
version,
3959
}
4060
}
4161

4262
pub const fn snap(version: u8) -> Self {
4363
Capability {
44-
protocol: "snap",
64+
protocol: pad_right(b"snap"),
4565
version,
4666
}
4767
}
68+
69+
pub fn protocol(&self) -> &str {
70+
let len = self
71+
.protocol
72+
.iter()
73+
.position(|c| c == &b'\0')
74+
.unwrap_or(CAPABILITY_NAME_MAX_LENGTH);
75+
str::from_utf8(&self.protocol[..len]).expect("value parsed as utf8 in RLPDecode")
76+
}
4877
}
4978

5079
impl RLPEncode for Capability {
5180
fn encode(&self, buf: &mut dyn BufMut) {
5281
Encoder::new(buf)
53-
.encode_field(&self.protocol)
82+
.encode_field(&self.protocol())
5483
.encode_field(&self.version)
5584
.finish();
5685
}
5786
}
5887

5988
impl RLPDecode for Capability {
6089
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
61-
let (protocol, rest) = String::decode_unfinished(&rlp[1..])?;
62-
let (version, rest) = u8::decode_unfinished(rest)?;
63-
match protocol.as_str() {
64-
"eth" => Ok((Capability::eth(version), rest)),
65-
"p2p" => Ok((Capability::p2p(version), rest)),
66-
"snap" => Ok((Capability::snap(version), rest)),
67-
_ => Err(RLPDecodeError::MalformedData),
90+
let (protocol_name, rest) = String::decode_unfinished(&rlp[1..])?;
91+
if protocol_name.len() > CAPABILITY_NAME_MAX_LENGTH {
92+
return Err(RLPDecodeError::InvalidLength);
6893
}
94+
let (version, rest) = u8::decode_unfinished(rest)?;
95+
let mut protocol = [0; CAPABILITY_NAME_MAX_LENGTH];
96+
protocol[..protocol_name.len()].copy_from_slice(protocol_name.as_bytes());
97+
Ok((Capability { protocol, version }, rest))
6998
}
7099
}
71100

@@ -74,7 +103,7 @@ impl Serialize for Capability {
74103
where
75104
S: serde::Serializer,
76105
{
77-
serializer.serialize_str(&format!("{}/{}", self.protocol, self.version))
106+
serializer.serialize_str(&format!("{}/{}", self.protocol(), self.version))
78107
}
79108
}
80109

@@ -320,3 +349,33 @@ impl RLPxMessage for PongMessage {
320349
Ok(Self {})
321350
}
322351
}
352+
353+
#[cfg(test)]
354+
mod tests {
355+
use ethrex_rlp::{decode::RLPDecode, encode::RLPEncode};
356+
357+
use crate::rlpx::p2p::Capability;
358+
359+
#[test]
360+
fn test_encode_capability() {
361+
let capability = Capability::eth(8);
362+
let encoded = capability.encode_to_vec();
363+
364+
assert_eq!(&encoded, &[197_u8, 131, b'e', b't', b'h', 8]);
365+
}
366+
367+
#[test]
368+
fn test_decode_capability() {
369+
let encoded_bytes = &[197_u8, 131, b'e', b't', b'h', 8];
370+
let decoded = Capability::decode(encoded_bytes).unwrap();
371+
372+
assert_eq!(decoded, Capability::eth(8));
373+
}
374+
375+
#[test]
376+
fn test_protocol() {
377+
let capability = Capability::eth(68);
378+
379+
assert_eq!(capability.protocol(), "eth");
380+
}
381+
}

crates/networking/rpc/admin/peers.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl From<PeerData> for RpcPeer {
4949
let mut protocols = Protocols::default();
5050
// Fill protocol data
5151
for cap in &peer.supported_capabilities {
52-
match cap.protocol {
52+
match cap.protocol() {
5353
"p2p" => {
5454
protocols.p2p = Some(ProtocolData {
5555
version: cap.version,
@@ -111,16 +111,7 @@ mod tests {
111111
// Set node capabilities and other relevant data
112112
peer.is_connected = true;
113113
peer.is_connection_inbound = false;
114-
peer.supported_capabilities = vec![
115-
Capability {
116-
protocol: "eth",
117-
version: 68,
118-
},
119-
Capability {
120-
protocol: "snap",
121-
version: 1,
122-
},
123-
];
114+
peer.supported_capabilities = vec![Capability::eth(68), Capability::snap(1)];
124115
peer.node.version = Some("ethrex/test".to_string());
125116
// The first serialized peer shown in geth's documentation example: https://geth.ethereum.org/docs/interacting-with-geth/rpc/ns-admin#admin-peers
126117
// The fields "localAddress", "static", "trusted" and "name" were removed as we do not have the necessary information to show them

0 commit comments

Comments
 (0)