diff --git a/handshake.go b/handshake.go index 5be47a7..73883dd 100644 --- a/handshake.go +++ b/handshake.go @@ -891,26 +891,12 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis "peer_node_id": peerNodeID, "justification": justification, }) - // Fast pre-checks: an already-trusted peer (no new trust needed) and - // over-cap spam are handled here. The authoritative trust/cap decisions - // are still made under the lock below, so this is a guard, not the - // enforcement. - hm.mu.Lock() - if _, ok := hm.trusted[peerNodeID]; ok && hm.reconcileTrustBindingLocked(peerNodeID, msg.PublicKey) { - hm.sendAcceptLocked(peerNodeID) - hm.mu.Unlock() - slog.Debug("node already trusted (pre-hook fast path)", "peer_node_id", peerNodeID) - return - } - if _, exists := hm.pending[peerNodeID]; !exists { - if len(hm.pending) >= maxPendingHandshakes || - (source != 0 && hm.pendingBySource[source] >= maxPendingPerSource) { - hm.mu.Unlock() - slog.Warn("handshake rejected before control hook: pending queue full", "peer_node_id", peerNodeID, "source", source) - return - } - } - hm.mu.Unlock() + // The pending-queue caps are enforced where a request is actually queued, + // at the end of this function. They used to be checked here as well, to + // keep over-cap spam away from a control hook that no longer exists — + // which also dropped a peer the rules below would have accepted without + // queueing it (mutual request, shared network, trusted agent, + // trust-auto-approve) whenever the queue happened to be full. hm.mu.Lock() defer hm.mu.Unlock() @@ -1207,20 +1193,9 @@ func (hm *Manager) ProcessRelayedRequest(fromNodeID uint32, justification string func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string) { justification = sanitizeJustification(justification) - // Relayed handshake spam: an unknown peer whose request can neither be - // queued (pending cap full) nor short-circuited (not already trusted) is - // rejected here. Already-trusted or already-pending peers are never dropped - // here — they fall through to the existing handling below, where the caps - // are authoritatively enforced. - hm.mu.Lock() - _, alreadyTrusted := hm.trusted[fromNodeID] - _, alreadyPending := hm.pending[fromNodeID] - overCap := !alreadyPending && len(hm.pending) >= maxPendingHandshakes - hm.mu.Unlock() - if !alreadyTrusted && overCap { - slog.Warn("relayed handshake rejected before control hook: pending queue full", "peer_node_id", fromNodeID) - return - } + // As in handleRequest: the pending caps are enforced where the request is + // queued, below, so a full queue does not stop a peer the auto-accept + // rules would take without queueing it. hm.mu.Lock() defer hm.mu.Unlock() diff --git a/zz_pending_full_auto_accept_test.go b/zz_pending_full_auto_accept_test.go new file mode 100644 index 0000000..76962e7 --- /dev/null +++ b/zz_pending_full_auto_accept_test.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package handshake + +import ( + "testing" + "time" +) + +func fillPending(m *Manager) { + m.mu.Lock() + for i := uint32(1000); i < 1000+uint32(maxPendingHandshakes); i++ { + m.pending[i] = &PendingHandshake{NodeID: i} + } + m.mu.Unlock() +} + +// A full pending queue used to drop every new peer before the auto-accept +// rules ran, so a node with trust-auto-approve on (or a mutual request +// outstanding) refused peers it would have trusted without queueing them. +// The caps only apply to a request that actually has to be queued. +func TestFullPendingQueueDoesNotBlockAutoAccept(t *testing.T) { + t.Run("trust-auto-approve, direct", func(t *testing.T) { + m, _ := hsTestManager(t, true) + fillPending(m) + m.handleRequest(nil, &HandshakeMsg{NodeID: 5000, PublicKey: "k"}, false) + m.mu.RLock() + defer m.mu.RUnlock() + if _, ok := m.trusted[5000]; !ok { + t.Fatal("auto-approve node dropped a new peer because its pending queue was full") + } + if n := len(m.pending); n != maxPendingHandshakes { + t.Fatalf("pending len = %d, want it unchanged at %d", n, maxPendingHandshakes) + } + }) + + t.Run("trust-auto-approve, relayed", func(t *testing.T) { + m, _ := hsTestManager(t, true) + fillPending(m) + m.processRelayedRequest(6000, "join") + m.mu.RLock() + defer m.mu.RUnlock() + if _, ok := m.trusted[6000]; !ok { + t.Fatal("auto-approve node dropped a relayed request because its pending queue was full") + } + }) + + t.Run("mutual request, relayed", func(t *testing.T) { + m, _ := hsTestManager(t, false) + fillPending(m) + m.mu.Lock() + m.outgoing[7000] = time.Now() + m.mu.Unlock() + m.processRelayedRequest(7000, "join") + m.mu.RLock() + defer m.mu.RUnlock() + if _, ok := m.trusted[7000]; !ok { + t.Fatal("a mutual request was dropped because the pending queue was full") + } + }) + + t.Run("nothing to auto-accept: still not queued", func(t *testing.T) { + m, _ := hsTestManager(t, false) + fillPending(m) + m.handleRequest(nil, &HandshakeMsg{NodeID: 8000, PublicKey: "k"}, false) + m.processRelayedRequest(8001, "join") + m.mu.RLock() + defer m.mu.RUnlock() + for _, id := range []uint32{8000, 8001} { + if _, ok := m.pending[id]; ok { + t.Fatalf("over-cap peer %d was queued", id) + } + if _, ok := m.trusted[id]; ok { + t.Fatalf("over-cap peer %d was trusted", id) + } + } + if n := len(m.pending); n != maxPendingHandshakes { + t.Fatalf("pending len = %d, want %d", n, maxPendingHandshakes) + } + }) +}