Skip to content

Bug: Node can accept self-connection due to missing guard in connect protocol #2139

@sanity

Description

@sanity

Problem

When a joiner connects to a gateway that has no other peers, the connect request can be forwarded back to the joiner, causing the node to accept a connection from itself and attempt NAT traversal to its own public IP.

User Impact

  • "Subscribing to room..." hangs indefinitely in River
  • Node fails to properly join the network when the gateway has limited peers
  • Connection attempts timeout with "max connection attempts reached" trying to connect to self

Root Cause

Two missing guards in the connect protocol:

  1. should_accept() in connection_manager.rs:149 - doesn't check if peer_id == self.get_peer_key()
  2. Gateway forwarding logic - the gateway forwards the ConnectRequest back to the only peer it knows (the joiner)

Reproduction Log Evidence

# Joiner receives ConnectRequest with itself as joiner:
ConnectRequest { target: v6MWKgqHiBMNcGtG, ttl: 9, joiner: v6MWKgqHkAt4vEzk }

# Joiner accepts connection from itself:
acceptance issued, acceptor_peer: v6MWKgqHkAt4vEzk, joiner_peer: v6MWKgqHkAt4vEzk

# Then tries to connect to its own public IP:
Connecting to peer, tx: ..., remote_addr: 136.62.52.28:43375

# Which fails:
Outbound handshake failed: max connection attempts reached

Proposed Fix

Add a self-connection guard in should_accept():

pub fn should_accept(&self, location: Location, peer_id: &PeerId) -> bool {
    // Don't accept connections from ourselves
    if let Some(own_id) = self.get_peer_key() {
        if &own_id == peer_id {
            tracing::warn!(%peer_id, "should_accept: rejecting self-connection attempt");
            return false;
        }
    }
    // ... rest of method
}

This should be a simple fix.

Environment

  • Freenet v0.1.38
  • River (commit recent)

Additional Context

Note: there's already a debug_assert! at connection_manager.rs:400 checking this condition in add_connection(), suggesting this case was considered but not fully guarded against:

debug_assert!(self.get_peer_key().expect("should be set") != peer);

[AI-assisted - Claude]

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-networkingArea: Networking, ring protocol, peer discoveryE-easyExperience needed to fix/implement: Easy / not muchP-highHigh priorityT-bugType: Something is broken

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions