Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(connection-limit): set bypass rules for connections #5720

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
+35 −1
Draft

feat(connection-limit): set bypass rules for connections #5720

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d78a671
set bypass rules
drHuangMHT Dec 6, 2024
05c757f
Merge branch 'master' into connection-limit-whitelist
dariusc93 Dec 16, 2024
cbf009c
Merge branch 'master' into connection-limit-whitelist
dariusc93 Dec 28, 2024
399fd34
Merge branch 'master' into connection-limit-whitelist
drHuangMHT Jan 3, 2025
639c3d2
changelog and documentation
drHuangMHT Jan 3, 2025
227b062
replace multiaddr bypass with filter
drHuangMHT Jan 10, 2025
d9901ee
Merge branch 'master' into connection-limit-whitelist
drHuangMHT Jan 23, 2025
efa83b1
reduce diff
drHuangMHT Jan 23, 2025
47b81c3
fix incorrect condition
drHuangMHT Jan 23, 2025
aa01b26
bypass by PeerId only
drHuangMHT Jan 31, 2025
092f313
move bypass list into Behaviour
drHuangMHT Feb 1, 2025
2836212
minor version bump
drHuangMHT Feb 1, 2025
368313c
Merge branch 'master' into connection-limit-whitelist
dariusc93 Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 118 additions & 61 deletions misc/connection-limits/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ use libp2p_swarm::{
/// ```
pub struct Behaviour {
limits: ConnectionLimits,
bypass_rules: BypassRules,

pending_inbound_connections: HashSet<ConnectionId>,
pending_outbound_connections: HashSet<ConnectionId>,
Expand All @@ -76,9 +77,10 @@ pub struct Behaviour {
}

impl Behaviour {
pub fn new(limits: ConnectionLimits) -> Self {
pub fn new(limits: ConnectionLimits, bypass_rules: BypassRules) -> Self {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should remove bypass_rules from here and have it be empty internally by default, or have a separate function to supply it when constructing the behaviour. This would then allow us to keep this as a patch release instead of bumping the minor version of the crate due to it being a breaking change. Thoughts?

Self {
limits,
bypass_rules,
pending_inbound_connections: Default::default(),
pending_outbound_connections: Default::default(),
established_inbound_connections: Default::default(),
Expand All @@ -92,6 +94,10 @@ impl Behaviour {
pub fn limits_mut(&mut self) -> &mut ConnectionLimits {
&mut self.limits
}

pub fn bypass_rules_mut(&mut self) -> &mut BypassRules {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the advantage of having an external structure (BypassRules) instead of having these methods in the main Behaviour?

&mut self.bypass_rules
}
}

fn check_limit(limit: Option<u32>, current: usize, kind: Kind) -> Result<(), ConnectionDenied> {
Expand Down Expand Up @@ -208,22 +214,57 @@ impl ConnectionLimits {
}
}

#[derive(Debug, Clone, Default)]
pub struct BypassRules {
by_peer_id: HashSet<PeerId>,
by_multiaddr: HashSet<Multiaddr>,
}
impl BypassRules {
pub fn new(peer_ids: HashSet<PeerId>, remote_multiaddrs: HashSet<Multiaddr>) -> Self {
Self {
by_peer_id: peer_ids,
by_multiaddr: remote_multiaddrs,
}
}
pub fn bypass_peer_id(&mut self, peer_id: &PeerId) {
self.by_peer_id.insert(*peer_id);
}
pub fn remove_peer_id(&mut self, peer_id: &PeerId) {
self.by_peer_id.remove(peer_id);
}
pub fn bypass_multiaddr(&mut self, multiaddr: Multiaddr) {
self.by_multiaddr.insert(multiaddr);
}
pub fn remove_multiaddr(&mut self, multiaddr: &Multiaddr) {
self.by_multiaddr.remove(multiaddr);
}
pub fn is_peer_bypassed(&self, peer: &PeerId) -> bool {
self.by_peer_id.contains(peer)
}
pub fn is_addr_bypassed(&self, addr: &Multiaddr) -> bool {
self.by_multiaddr.contains(addr)
}
}

impl NetworkBehaviour for Behaviour {
type ConnectionHandler = dummy::ConnectionHandler;
type ToSwarm = Infallible;

fn handle_pending_inbound_connection(
&mut self,
connection_id: ConnectionId,
_: &Multiaddr,
_: &Multiaddr,
local_addr: &Multiaddr,
remote_addr: &Multiaddr,
) -> Result<(), ConnectionDenied> {
check_limit(
self.limits.max_pending_incoming,
self.pending_inbound_connections.len(),
Kind::PendingIncoming,
)?;

if !(self.bypass_rules.is_addr_bypassed(local_addr)
|| self.bypass_rules.is_addr_bypassed(remote_addr))
{
check_limit(
self.limits.max_pending_incoming,
self.pending_inbound_connections.len(),
Kind::PendingIncoming,
)?;
}
self.pending_inbound_connections.insert(connection_id);

Ok(())
Expand All @@ -233,46 +274,60 @@ impl NetworkBehaviour for Behaviour {
&mut self,
connection_id: ConnectionId,
peer: PeerId,
_: &Multiaddr,
_: &Multiaddr,
local_addr: &Multiaddr,
remote_addr: &Multiaddr,
) -> Result<THandler<Self>, ConnectionDenied> {
self.pending_inbound_connections.remove(&connection_id);

check_limit(
self.limits.max_established_incoming,
self.established_inbound_connections.len(),
Kind::EstablishedIncoming,
)?;
check_limit(
self.limits.max_established_per_peer,
self.established_per_peer
.get(&peer)
.map(|connections| connections.len())
.unwrap_or(0),
Kind::EstablishedPerPeer,
)?;
check_limit(
self.limits.max_established_total,
self.established_inbound_connections.len()
+ self.established_outbound_connections.len(),
Kind::EstablishedTotal,
)?;

if !(self.bypass_rules.is_addr_bypassed(local_addr)
drHuangMHT marked this conversation as resolved.
Show resolved Hide resolved
|| self.bypass_rules.is_addr_bypassed(remote_addr)
|| self.bypass_rules.is_peer_bypassed(&peer))
{
check_limit(
self.limits.max_established_incoming,
self.established_inbound_connections.len(),
Kind::EstablishedIncoming,
)?;
check_limit(
self.limits.max_established_per_peer,
self.established_per_peer
.get(&peer)
.map(|connections| connections.len())
.unwrap_or(0),
Kind::EstablishedPerPeer,
)?;
check_limit(
self.limits.max_established_total,
self.established_inbound_connections.len()
+ self.established_outbound_connections.len(),
Kind::EstablishedTotal,
)?;
}
Ok(dummy::ConnectionHandler)
}

fn handle_pending_outbound_connection(
&mut self,
connection_id: ConnectionId,
_: Option<PeerId>,
_: &[Multiaddr],
maybe_peer: Option<PeerId>,
addresses: &[Multiaddr],
_: Endpoint,
) -> Result<Vec<Multiaddr>, ConnectionDenied> {
check_limit(
self.limits.max_pending_outgoing,
self.pending_outbound_connections.len(),
Kind::PendingOutgoing,
)?;
let mut is_bypassed = false;
if let Some(peer) = maybe_peer {
is_bypassed = self.bypass_rules.is_peer_bypassed(&peer)
}
is_bypassed = is_bypassed
|| addresses
.iter()
.any(|addr| self.bypass_rules.is_addr_bypassed(addr));
drHuangMHT marked this conversation as resolved.
Show resolved Hide resolved
if !is_bypassed {
check_limit(
self.limits.max_pending_outgoing,
self.pending_outbound_connections.len(),
Kind::PendingOutgoing,
)?;
}

self.pending_outbound_connections.insert(connection_id);

Expand All @@ -283,31 +338,33 @@ impl NetworkBehaviour for Behaviour {
&mut self,
connection_id: ConnectionId,
peer: PeerId,
_: &Multiaddr,
addr: &Multiaddr,
_: Endpoint,
_: PortUse,
) -> Result<THandler<Self>, ConnectionDenied> {
self.pending_outbound_connections.remove(&connection_id);

check_limit(
self.limits.max_established_outgoing,
self.established_outbound_connections.len(),
Kind::EstablishedOutgoing,
)?;
check_limit(
self.limits.max_established_per_peer,
self.established_per_peer
.get(&peer)
.map(|connections| connections.len())
.unwrap_or(0),
Kind::EstablishedPerPeer,
)?;
check_limit(
self.limits.max_established_total,
self.established_inbound_connections.len()
+ self.established_outbound_connections.len(),
Kind::EstablishedTotal,
)?;
if !(self.bypass_rules.is_peer_bypassed(&peer) || self.bypass_rules.is_addr_bypassed(addr))
{
check_limit(
self.limits.max_established_outgoing,
self.established_outbound_connections.len(),
Kind::EstablishedOutgoing,
)?;
check_limit(
self.limits.max_established_per_peer,
self.established_per_peer
.get(&peer)
.map(|connections| connections.len())
.unwrap_or(0),
Kind::EstablishedPerPeer,
)?;
check_limit(
self.limits.max_established_total,
self.established_inbound_connections.len()
+ self.established_outbound_connections.len(),
Kind::EstablishedTotal,
)?;
}

Ok(dummy::ConnectionHandler)
}
Expand Down Expand Up @@ -544,13 +601,13 @@ mod tests {
impl Behaviour {
fn new(limits: ConnectionLimits) -> Self {
Self {
limits: super::Behaviour::new(limits),
limits: super::Behaviour::new(limits, Default::default()),
connection_denier: None.into(),
}
}
fn new_with_connection_denier(limits: ConnectionLimits) -> Self {
Self {
limits: super::Behaviour::new(limits),
limits: super::Behaviour::new(limits, Default::default()),
connection_denier: Some(ConnectionDenier {}).into(),
}
}
Expand Down
Loading