Files
netbird/proxy/internal/tcp/accept_test.go
mlsmaycon 2d8b0310a4 [client, proxy] IPv6 in-place apply + accept-loop hardening on netstack listeners
Two related fixes for the embedded netbird client and the per-account
inbound listeners that ride on its gVisor netstack.

client/internal/engine.go — replace hasIPv6Changed with reconcileIPv6:

  - First v6 assignment (current had no v6, conf carries one) is applied
    in place via WGIface.UpdateAddr instead of returning ErrResetConnection.
    Pre-fix, every embedded client whose account had IPv6 enabled would
    reset on its first NetworkMap sync — boot config has no v6, the sync
    introduces one, the engine tore itself down to "apply" it. That
    teardown destroys the gVisor netstack and orphans every listener
    bound on it, which is what made the proxy's per-account :80/:443
    silently stop accepting traffic.
  - v6 removed clears in place.
  - v6 swapped to a different non-empty value still resets (gVisor
    netstack can't safely swap its address at runtime).
  - Mutates e.config.WgAddr to match the applied state so subsequent
    PeerConfig comparisons are stable.

proxy/internal/tcp/accept.go (new) + proxy/inbound.go +
proxy/internal/tcp/router.go — harden the two Accept() loops on
netstack-backed listeners:

  - IsClosedListenerErr recognises net.ErrClosed AND gVisor's
    "endpoint is in invalid state" — the latter survives gonet's
    *net.OpError wrapping in a way errors.Is(.., net.ErrClosed) does
    not. Without this the loop spins CPU-hot after the underlying
    netstack is destroyed (peer rekey, embedded-client reset, account
    churn), emitting one log line per iteration.
  - AcceptBackoff implements the exponential backoff that
    net/http.Server.Serve uses on transient Accept errors: 5ms doubling
    up to 1s. Defence-in-depth so an unknown sticky error cannot burn
    a CPU core even if IsClosedListenerErr misses its signature.

proxy/internal/roundtrip/netbird.go — emit a single structured INFO
line summarising every embed.Options flag (account_id, service_id,
public_key, management_url, wg_port, block_inbound, block_lan_access,
disable_ipv6, no_userspace, presence of credentials) when each
per-account embedded client is created. Secrets reduced to a "present"
boolean — never logged verbatim. Diagnostic-only; no behavior change,
but it makes the "why is this embedded peer misbehaving" loop a single
log read instead of a code dive.

Tests (real listeners, scripted errors, no mocks of production code):
  - engine_reconcileipv6_test.go: 8 cases for every transition (first
    assignment, no change, removed, prefix-length changed, value
    changed, invalid bytes, UpdateAddr error) plus a updateConfig
    integration check that the fix actually fires on a v6-added
    PeerConfig.
  - accept_test.go: IsClosedListenerErr matrix + AcceptBackoff
    progression / cap / reset / cancel-during-wait / cancel-before-call.
  - router_test.go, inbound_test.go: scriptedAcceptListener +
    TestRouter_Serve_ExitsOnGVisorInvalidEndpoint and
    TestFeedRouterFromListener_ExitsOnGVisorInvalidEndpoint —
    regression guards that fail in 2 s if the loop ever spins.
2026-06-18 10:37:51 +02:00

143 lines
4.9 KiB
Go

package tcp
import (
"context"
"errors"
"fmt"
"net"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestIsClosedListenerErr_NetErrClosed verifies the stdlib path: a
// closed *net.Listener returns net.ErrClosed wrapped in *net.OpError,
// and IsClosedListenerErr must unwrap it.
func TestIsClosedListenerErr_NetErrClosed(t *testing.T) {
wrapped := &net.OpError{Op: "accept", Net: "tcp", Err: net.ErrClosed}
assert.True(t, IsClosedListenerErr(wrapped),
"net.OpError wrapping net.ErrClosed must be recognised as closed")
}
// TestIsClosedListenerErr_GVisorInvalidEndpoint is the load-bearing
// regression guard. A gVisor netstack listener whose endpoint has been
// destroyed returns this exact text. Without recognising it the accept
// loop spins forever and burns a CPU core.
func TestIsClosedListenerErr_GVisorInvalidEndpoint(t *testing.T) {
err := fmt.Errorf("accept tcp 10.10.1.254:80: endpoint is in invalid state")
assert.True(t, IsClosedListenerErr(err),
"gVisor 'endpoint is in invalid state' must be recognised as closed")
}
// TestIsClosedListenerErr_OtherError confirms we don't over-match —
// transient errors must keep returning false so the backoff path runs.
func TestIsClosedListenerErr_OtherError(t *testing.T) {
cases := []error{
errors.New("temporary failure"),
errors.New("accept tcp 10.10.1.254:80: too many open files"),
nil,
}
for _, c := range cases {
assert.False(t, IsClosedListenerErr(c),
"unexpected match on %v — must not be treated as closed", c)
}
}
// TestAcceptBackoff_ProgressionAndCap asserts the doubling schedule:
// 5ms, 10ms, 20ms, 40ms, ... capped at 1s. The test runs against a
// real timer but uses tight bounds so a slow CI machine still passes.
func TestAcceptBackoff_ProgressionAndCap(t *testing.T) {
var b AcceptBackoff
expected := []time.Duration{
5 * time.Millisecond,
10 * time.Millisecond,
20 * time.Millisecond,
40 * time.Millisecond,
}
for i, want := range expected {
start := time.Now()
ok := b.Backoff(context.Background())
elapsed := time.Since(start)
require.True(t, ok, "Backoff %d must complete; ctx is alive", i)
assert.GreaterOrEqual(t, elapsed, want,
"backoff %d (%v) must wait at least the configured delay", i, want)
assert.Less(t, elapsed, want*4,
"backoff %d (%v) must not overshoot by more than 4x — caps misbehaving", i, want)
}
// Burn enough rounds to reach the cap, then assert subsequent
// rounds stay at exactly maxAcceptDelay (1s) — the timer should
// never exceed it.
for range 6 {
b.Backoff(context.Background())
}
assert.Equal(t, maxAcceptDelay, b.delay,
"after enough doublings the delay must clamp to maxAcceptDelay")
}
// TestAcceptBackoff_Reset confirms that a successful Accept resets the
// schedule — a busy-then-quiet listener mustn't stay on a 1s timer
// after recovery.
func TestAcceptBackoff_Reset(t *testing.T) {
var b AcceptBackoff
for range 5 {
b.Backoff(context.Background())
}
require.NotEqual(t, time.Duration(0), b.delay, "precondition: delay must have accumulated")
b.Reset()
assert.Equal(t, time.Duration(0), b.delay, "Reset must zero the delay")
start := time.Now()
ok := b.Backoff(context.Background())
elapsed := time.Since(start)
require.True(t, ok, "Backoff after Reset must complete")
assert.GreaterOrEqual(t, elapsed, minAcceptDelay,
"after Reset the next backoff must restart at minAcceptDelay")
assert.Less(t, elapsed, 50*time.Millisecond,
"after Reset the next backoff must NOT carry over the prior delay")
}
// TestAcceptBackoff_CancelDuringWait proves the loop exits promptly
// when ctx fires mid-wait. Without this, a tear-down would still take
// up to 1 second per orphaned listener.
func TestAcceptBackoff_CancelDuringWait(t *testing.T) {
var b AcceptBackoff
// Drive the backoff up so the next call will wait ~1s — long
// enough that we can detect early cancellation.
for range 10 {
b.Backoff(context.Background())
}
require.Equal(t, maxAcceptDelay, b.delay)
ctx, cancel := context.WithCancel(context.Background())
go func() {
time.Sleep(20 * time.Millisecond)
cancel()
}()
start := time.Now()
ok := b.Backoff(ctx)
elapsed := time.Since(start)
assert.False(t, ok, "Backoff must return false when ctx is cancelled mid-wait")
assert.Less(t, elapsed, 200*time.Millisecond,
"cancellation must short-circuit the timer; took %v", elapsed)
}
// TestAcceptBackoff_CancelBeforeCall — when ctx is already done the
// loop exits without sleeping at all.
func TestAcceptBackoff_CancelBeforeCall(t *testing.T) {
var b AcceptBackoff
ctx, cancel := context.WithCancel(context.Background())
cancel()
start := time.Now()
ok := b.Backoff(ctx)
elapsed := time.Since(start)
assert.False(t, ok, "Backoff must return false when ctx is already cancelled")
assert.Less(t, elapsed, 50*time.Millisecond,
"already-cancelled ctx must return immediately; took %v", elapsed)
}