laantungir/fips
1
0
Fork 0
mirror of https://github.com/jmcorgan/fips.git synced 2026-08-12 09:33:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

nostr: ignore stale traversal for active peers

Browse Source 
Skip BootstrapEvent::Established and BootstrapEvent::Failed dispatch
in poll_nostr_discovery for peers that are already connected or
actively handshaking. Without these guards, stale traversal events
arriving after a peer connected through a different path would
either attempt to adopt a redundant socket against the live
connection (Established) or poison the per-peer failure-state
cooldown and trigger redundant retraversal via schedule_retry /
try_peer_addresses (Failed).

The four guard sites use a new is_connecting_to_peer helper extracted
from the existing closure inside initiate_peer_connection; the helper
checks for an in-flight outbound handshake state. adopt_established_traversal
gains a defense-in-depth check returning PeerAlreadyExists when called
against an already-promoted peer, so the invariant holds if a future
caller bypasses the outer dispatch guard.

Side benefit: narrows a cooldown-poisoning vector previously available
to an attacker injecting stale failure events for an active peer.

Test coverage for the new behavior:

- test_try_peer_addresses_skips_connected_peer
- test_try_peer_addresses_skips_connecting_peer
- test_nostr_traversal_failure_skips_connected_peer (Failed-arm event
  injection)
- test_nostr_traversal_established_skips_connected_peer (Established-arm
  event injection, mirror of the Failed test)
- test_adopted_traversal_skips_already_connected_peer
  (adopt_established_traversal defense-in-depth)

CHANGELOG entry under [Unreleased] / Fixed.

Closes #87
This commit is contained in:
Martti Malmi
2026-05-15 15:06:54 +00:00
committed by Johnathan Corgan
parent ab1e248ff4
commit 87d1af0269
5 changed files with 274 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/node/lifecycle.rs
+78 -12
View File
@@ -122,12 +122,7 @@ impl Node {
}
// Check if connection already in progress to this peer
let already_connecting = self.connections.values().any(|conn| {
conn.expected_identity()
.map(|id| id.node_addr() == &peer_node_addr)
.unwrap_or(false)
});
if already_connecting {
if self.is_connecting_to_peer(&peer_node_addr) {
debug!(
npub = %peer_config.npub,
"Connection already in progress, skipping"
@@ -139,6 +134,14 @@ impl Node {
.await
}
fn is_connecting_to_peer(&self, peer_node_addr: &NodeAddr) -> bool {
self.connections.values().any(|conn| {
conn.expected_identity()
.map(|id| id.node_addr() == peer_node_addr)
.unwrap_or(false)
})
}
/// Initiate a connection to a peer on a specific transport and address.
///
/// For connectionless transports (UDP, Ethernet): allocates a link, starts
@@ -410,6 +413,23 @@ impl Node {
match event {
BootstrapEvent::Established { traversal } => {
let peer_npub = traversal.peer_npub.clone();
if let Ok(peer_identity) = PeerIdentity::from_npub(&peer_npub) {
let peer_addr = *peer_identity.node_addr();
if self.peers.contains_key(&peer_addr) {
debug!(
peer_npub = %peer_npub,
"Ignoring established NAT traversal for already-connected peer"
);
continue;
}
if self.is_connecting_to_peer(&peer_addr) {
debug!(
peer_npub = %peer_npub,
"Ignoring established NAT traversal while peer handshake is already in progress"
);
continue;
}
}
match self.adopt_established_traversal(traversal).await {
Ok(_) => {
info!(peer_npub = %peer_npub, "Adopted NAT traversal socket");
@@ -426,6 +446,28 @@ impl Node {
peer_config,
reason,
} => {
let peer_identity = match PeerIdentity::from_npub(&peer_config.npub) {
Ok(identity) => identity,
Err(_) => continue,
};
let node_addr = *peer_identity.node_addr();
if self.peers.contains_key(&node_addr) {
debug!(
npub = %peer_config.npub,
error = %reason,
"Ignoring failed NAT traversal for already-connected peer"
);
continue;
}
if self.is_connecting_to_peer(&node_addr) {
debug!(
npub = %peer_config.npub,
error = %reason,
"Ignoring failed NAT traversal while peer handshake is already in progress"
);
continue;
}
let now_ms = Self::now_ms();
let decision = bootstrap.record_traversal_failure(&peer_config.npub, now_ms);
if decision.should_warn {
@@ -476,11 +518,6 @@ impl Node {
});
}
let peer_identity = match PeerIdentity::from_npub(&peer_config.npub) {
Ok(identity) => identity,
Err(_) => continue,
};
if self
.try_peer_addresses(&peer_config, peer_identity, false)
.await
@@ -489,7 +526,6 @@ impl Node {
continue;
}
let node_addr = *peer_identity.node_addr();
self.schedule_retry(node_addr, now_ms);
if let Some(cooldown_until_ms) = decision.cooldown_until_ms
&& let Some(state) = self.retry_pending.get_mut(&node_addr)
@@ -1694,6 +1730,22 @@ impl Node {
peer_identity: PeerIdentity,
allow_bootstrap_nat: bool,
) -> Result<(), NodeError> {
let peer_node_addr = *peer_identity.node_addr();
if self.peers.contains_key(&peer_node_addr) {
debug!(
npub = %peer_config.npub,
"Peer already exists, skipping address attempts"
);
return Ok(());
}
if self.is_connecting_to_peer(&peer_node_addr) {
debug!(
npub = %peer_config.npub,
"Connection already in progress, skipping address attempts"
);
return Ok(());
}
// Static-first dialing: avoid delaying configured address attempts on
// advert fetch/network latency.
let static_addresses = self.static_peer_addresses(peer_config);
@@ -1837,6 +1889,20 @@ impl Node {
}
})?;
let peer_node_addr = *peer_identity.node_addr();
if self.peers.contains_key(&peer_node_addr) {
debug!(
peer_npub = %traversal.peer_npub,
"Ignoring NAT traversal handoff for already-connected peer"
);
return Err(NodeError::PeerAlreadyExists(peer_node_addr));
}
if self.is_connecting_to_peer(&peer_node_addr) {
debug!(
peer_npub = %traversal.peer_npub,
"Ignoring NAT traversal handoff while peer handshake is already in progress"
);
return Err(NodeError::PeerAlreadyExists(peer_node_addr));
}
self.peer_aliases
.insert(peer_node_addr, peer_identity.short_npub());