Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8fd5c52488 |
@@ -16,6 +16,7 @@ import (
|
||||
"github.com/sagernet/sing/common"
|
||||
"github.com/sagernet/sing/common/buf"
|
||||
"github.com/sagernet/sing/common/control"
|
||||
E "github.com/sagernet/sing/common/exceptions" // lx:tproxy_writeback_connect
|
||||
M "github.com/sagernet/sing/common/metadata"
|
||||
N "github.com/sagernet/sing/common/network"
|
||||
"github.com/sagernet/sing/common/udpnat2"
|
||||
@@ -68,7 +69,26 @@ func (t *TProxy) Start(stage adapter.StartStage) error {
|
||||
}
|
||||
|
||||
func (t *TProxy) Close() error {
|
||||
return t.listener.Close()
|
||||
err := t.listener.Close()
|
||||
// lx:begin tproxy_writeback_connect
|
||||
// Closing the listener stops INGRESS but leaves every live UDP NAT session in
|
||||
// the cache, and each session holds a write-back socket bound to its original
|
||||
// destination. Nothing else ever wakes those sessions: the cache evicts
|
||||
// lazily (on the next Get/Add), and after this inbound is gone there is no
|
||||
// next Get. For a DNS session answered in-engine by hijack-dns there is not
|
||||
// even an outbound connection whose failure could unwind it, so its socket
|
||||
// survives until a GC finalizer happens to reach it.
|
||||
//
|
||||
// That is invisible upstream, where an inbound is closed once at shutdown. On
|
||||
// this fork the engine is REBUILT on every config apply, so each apply would
|
||||
// strand another generation of sockets on the router's own LAN :53. Purge
|
||||
// evicts every session; the cache's OnEvict closes the conn, which unblocks
|
||||
// the session's routing goroutine and runs its onClose — the one place the
|
||||
// write-back socket is actually closed. Purge AFTER the listener so a packet
|
||||
// arriving mid-teardown cannot re-create a session behind us.
|
||||
t.udpNat.Purge()
|
||||
// lx:end tproxy_writeback_connect
|
||||
return err
|
||||
}
|
||||
|
||||
func (t *TProxy) NewConnection(ctx context.Context, conn net.Conn, metadata adapter.InboundContext, onClose N.CloseHandlerFunc) {
|
||||
@@ -121,18 +141,92 @@ type tproxyPacketWriter struct {
|
||||
conn *net.UDPConn
|
||||
}
|
||||
|
||||
// lx:begin tproxy_writeback_connect
|
||||
//
|
||||
// The TPROXY UDP write-back socket is bound to the ORIGINAL DESTINATION, so the
|
||||
// client sees the reply coming from the address it addressed. Upstream leaves
|
||||
// that socket UNCONNECTED (net.ListenPacket + WriteToUDPAddrPort), and that is
|
||||
// the bug this block exists for.
|
||||
//
|
||||
// An unconnected socket bound to <addr>:<port> is, as far as the kernel is
|
||||
// concerned, a RECEIVER for that address:port — and because the socket also
|
||||
// carries SO_REUSEADDR it silently joins the UDP demultiplex set of whatever
|
||||
// else is bound there. Nothing ever reads from it: this writer only sends. So
|
||||
// every datagram the kernel happens to hand it is lost.
|
||||
//
|
||||
// On a router that transparently intercepts LAN DNS towards its OWN address
|
||||
// (`nft ... udp dport 53 tproxy ...`) the original destination IS the router's
|
||||
// LAN address, so each intercepted DNS session parks another silent receiver on
|
||||
// <router-lan-ip>:53 right next to dnsmasq's socket — observed on the stand:
|
||||
// 33 such sockets against dnsmasq's one, several with a growing Recv-Q. The
|
||||
// host's own queries to that address take the loopback path, are never diverted,
|
||||
// and are therefore demultiplexed among all of them: they land in one of the
|
||||
// silent sockets at random and time out, permanently and unpredictably, while
|
||||
// every other DNS path on the box keeps working.
|
||||
//
|
||||
// Connecting the socket fixes it at the root. compute_score() in the kernel's
|
||||
// UDP lookup REJECTS a connected socket for any peer other than the connected
|
||||
// one, so a write-back socket can no longer be handed a datagram it will not
|
||||
// read. Nothing about the reply changes — same spoofed source address, same
|
||||
// single peer, one Write instead of one WriteTo.
|
||||
//
|
||||
// The unconnected path is kept verbatim for a destination that cannot be bound
|
||||
// (a domain socksaddr), so no existing case regresses.
|
||||
|
||||
// newTProxyWriteBack creates the connected write-back socket: local address =
|
||||
// the original destination (transparent bind), peer = the client. A package
|
||||
// variable so the behaviour can be tested without CAP_NET_ADMIN.
|
||||
var newTProxyWriteBack = func(w *tproxyPacketWriter, destination M.Socksaddr) (*net.UDPConn, error) {
|
||||
var dialer net.Dialer
|
||||
dialer.LocalAddr = destination.UDPAddr()
|
||||
dialer.Control = control.Append(dialer.Control, control.ReuseAddr())
|
||||
dialer.Control = control.Append(dialer.Control, redir.TProxyWriteBack())
|
||||
conn, err := w.listener.DialContext(dialer, w.ctx, "udp", w.source.String())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
udpConn, loaded := conn.(*net.UDPConn)
|
||||
if !loaded {
|
||||
conn.Close()
|
||||
return nil, E.New("tproxy write back: unexpected connection type ", conn)
|
||||
}
|
||||
return udpConn, nil
|
||||
}
|
||||
|
||||
// lx:end tproxy_writeback_connect
|
||||
|
||||
func (w *tproxyPacketWriter) WritePacket(buffer *buf.Buffer, destination M.Socksaddr) error {
|
||||
defer buffer.Release()
|
||||
if w.listener.ListenOptions().NetNs == "" {
|
||||
conn := w.conn
|
||||
if w.destination == destination && conn != nil {
|
||||
_, err := conn.WriteToUDPAddrPort(buffer.Bytes(), w.source)
|
||||
// lx:begin tproxy_writeback_connect
|
||||
// The cached socket is CONNECTED to the client, so this is a plain
|
||||
// Write. A failed write also closes it: upstream only dropped the
|
||||
// reference, leaving the fd to the GC finalizer.
|
||||
_, err := conn.Write(buffer.Bytes())
|
||||
if err != nil {
|
||||
conn.Close()
|
||||
w.conn = nil
|
||||
}
|
||||
return err
|
||||
// lx:end tproxy_writeback_connect
|
||||
}
|
||||
}
|
||||
// lx:begin tproxy_writeback_connect
|
||||
if destination.IsIP() {
|
||||
udpConn, err := newTProxyWriteBack(w, destination)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if w.listener.ListenOptions().NetNs == "" && w.destination == destination {
|
||||
w.conn = udpConn
|
||||
} else {
|
||||
defer udpConn.Close()
|
||||
}
|
||||
return common.Error(udpConn.Write(buffer.Bytes()))
|
||||
}
|
||||
// lx:end tproxy_writeback_connect
|
||||
var listenConfig net.ListenConfig
|
||||
listenConfig.Control = control.Append(listenConfig.Control, control.ReuseAddr())
|
||||
listenConfig.Control = control.Append(listenConfig.Control, redir.TProxyWriteBack())
|
||||
|
||||
@@ -0,0 +1,234 @@
|
||||
package redirect
|
||||
|
||||
// Regression cover for the lx:tproxy_writeback_connect block in tproxy.go.
|
||||
//
|
||||
// The failure it guards against, seen on a live BPi-R3 Mini: `shaterd` held 33
|
||||
// UNCONNECTED UDP sockets on the router's own LAN address :53 — the write-back
|
||||
// sockets of intercepted DNS sessions — alongside dnsmasq's single socket on the
|
||||
// same address:port. Nothing reads a write-back socket, so every host-originated
|
||||
// query that the kernel demultiplexed into one of them was silently dropped
|
||||
// (Recv-Q climbing, sender timing out). Connecting the socket to the one peer it
|
||||
// ever talks to removes it from the demultiplex set for everybody else.
|
||||
//
|
||||
// The real socket needs CAP_NET_ADMIN (IP_TRANSPARENT) and a foreign bind, so
|
||||
// these tests drive the seam (newTProxyWriteBack) with an ordinary connected
|
||||
// loopback socket. That is enough to pin both properties that actually broke:
|
||||
//
|
||||
// - the write path uses the CONNECTED form. If WritePacket ever goes back to
|
||||
// WriteToUDPAddrPort, Go returns ErrWriteToConnected on a connected socket
|
||||
// and these tests fail — i.e. the test cannot pass with an unconnected
|
||||
// write-back socket, which is exactly the regression.
|
||||
// - repeated writes to the same destination REUSE one socket instead of
|
||||
// accumulating a new one per packet.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"net/netip"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/sagernet/sing-box/common/listener"
|
||||
"github.com/sagernet/sing-box/option"
|
||||
|
||||
"github.com/sagernet/sing/common/buf"
|
||||
M "github.com/sagernet/sing/common/metadata"
|
||||
N "github.com/sagernet/sing/common/network"
|
||||
"github.com/sagernet/sing/common/udpnat2"
|
||||
)
|
||||
|
||||
// writeBackHarness stands up a loopback "client" socket and a tproxyPacketWriter
|
||||
// whose socket factory returns a plain connected UDP socket aimed at it. It
|
||||
// returns the writer, the client socket and a pointer to the factory call count.
|
||||
func writeBackHarness(t *testing.T) (*tproxyPacketWriter, *net.UDPConn, *int) {
|
||||
t.Helper()
|
||||
|
||||
client, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)})
|
||||
if err != nil {
|
||||
t.Fatalf("client socket: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { client.Close() })
|
||||
|
||||
source := netip.MustParseAddrPort(client.LocalAddr().String())
|
||||
w := &tproxyPacketWriter{
|
||||
ctx: context.Background(),
|
||||
source: source,
|
||||
// A listener with empty options is enough: WritePacket only reads
|
||||
// ListenOptions().NetNs, and the factory is stubbed below.
|
||||
listener: listener.New(listener.Options{
|
||||
Context: context.Background(),
|
||||
Listen: option.ListenOptions{},
|
||||
}),
|
||||
destination: M.SocksaddrFrom(netip.MustParseAddr("127.0.0.1"), 53),
|
||||
}
|
||||
|
||||
calls := 0
|
||||
orig := newTProxyWriteBack
|
||||
t.Cleanup(func() { newTProxyWriteBack = orig })
|
||||
newTProxyWriteBack = func(w *tproxyPacketWriter, destination M.Socksaddr) (*net.UDPConn, error) {
|
||||
calls++
|
||||
// The real implementation binds the ORIGINAL DESTINATION transparently
|
||||
// and connects to w.source; here we only reproduce the connect half,
|
||||
// which is the property under test.
|
||||
conn, err := net.DialUDP("udp", nil, net.UDPAddrFromAddrPort(w.source))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return conn, nil
|
||||
}
|
||||
return w, client, &calls
|
||||
}
|
||||
|
||||
// readOne reads one datagram from the client socket with a short deadline.
|
||||
func readOne(t *testing.T, client *net.UDPConn) string {
|
||||
t.Helper()
|
||||
_ = client.SetReadDeadline(time.Now().Add(2 * time.Second))
|
||||
b := make([]byte, 512)
|
||||
n, _, err := client.ReadFrom(b)
|
||||
if err != nil {
|
||||
t.Fatalf("client read: %v", err)
|
||||
}
|
||||
return string(b[:n])
|
||||
}
|
||||
|
||||
// TestWriteBackUsesConnectedSocket: the reply must go out over a CONNECTED
|
||||
// socket. On the pre-fix code the write is WriteToUDPAddrPort, which Go refuses
|
||||
// on a connected socket ("use of WriteTo with pre-connected connection"), so
|
||||
// this test is red for exactly the shape that caused the outage.
|
||||
func TestWriteBackUsesConnectedSocket(t *testing.T) {
|
||||
w, client, calls := writeBackHarness(t)
|
||||
|
||||
if err := w.WritePacket(buf.As([]byte("first")).ToOwned(), w.destination); err != nil {
|
||||
t.Fatalf("WritePacket: %v", err)
|
||||
}
|
||||
if got := readOne(t, client); got != "first" {
|
||||
t.Fatalf("client got %q, want %q", got, "first")
|
||||
}
|
||||
if *calls != 1 {
|
||||
t.Fatalf("expected one write-back socket, got %d", *calls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteBackReusesOneSocket: a session that keeps answering the same client
|
||||
// must keep ONE socket, not open a fresh one per datagram. The live box carried
|
||||
// one socket per intercepted DNS session already; one per PACKET would turn a
|
||||
// nuisance into an fd exhaustion.
|
||||
func TestWriteBackReusesOneSocket(t *testing.T) {
|
||||
w, client, calls := writeBackHarness(t)
|
||||
|
||||
for i, payload := range []string{"a", "b", "c", "d"} {
|
||||
if err := w.WritePacket(buf.As([]byte(payload)).ToOwned(), w.destination); err != nil {
|
||||
t.Fatalf("WritePacket %d: %v", i, err)
|
||||
}
|
||||
if got := readOne(t, client); got != payload {
|
||||
t.Fatalf("packet %d: client got %q, want %q", i, got, payload)
|
||||
}
|
||||
}
|
||||
if *calls != 1 {
|
||||
t.Fatalf("four packets to one destination must share one socket, got %d sockets", *calls)
|
||||
}
|
||||
if w.conn == nil {
|
||||
t.Fatalf("the write-back socket must be cached on the writer for reuse")
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteBackClosesSocketOnWriteFailure: upstream dropped the reference to a
|
||||
// failed socket without closing it, leaving the fd to the GC finalizer. On a box
|
||||
// that already parks one socket per DNS session that is the wrong direction.
|
||||
func TestWriteBackClosesSocketOnWriteFailure(t *testing.T) {
|
||||
w, client, _ := writeBackHarness(t)
|
||||
|
||||
if err := w.WritePacket(buf.As([]byte("warm")).ToOwned(), w.destination); err != nil {
|
||||
t.Fatalf("WritePacket: %v", err)
|
||||
}
|
||||
readOne(t, client)
|
||||
|
||||
cached := w.conn
|
||||
if cached == nil {
|
||||
t.Fatalf("expected a cached socket after the first write")
|
||||
}
|
||||
// Close it behind WritePacket's back so the next write fails, exactly as a
|
||||
// dead peer or a torn-down plane would make it fail.
|
||||
cached.Close()
|
||||
|
||||
if err := w.WritePacket(buf.As([]byte("boom")).ToOwned(), w.destination); err == nil {
|
||||
t.Fatalf("a write on a closed socket must report the failure")
|
||||
}
|
||||
if w.conn != nil {
|
||||
t.Fatalf("a failed write must drop the cached socket")
|
||||
}
|
||||
// A second Close on an already-closed conn is an error, which is how we know
|
||||
// WritePacket closed it rather than merely forgetting it.
|
||||
if err := cached.Close(); err == nil {
|
||||
t.Fatalf("WritePacket must CLOSE the failed socket, not just nil the field")
|
||||
}
|
||||
}
|
||||
|
||||
// --- Close() must release the UDP NAT sessions --------------------------------
|
||||
|
||||
// natSessionHandler stands in for the router: it drains the session conn until
|
||||
// it errors (which is what Close does to it) and then reports onClose, exactly
|
||||
// as the real routing goroutine does. onClose is where the write-back socket is
|
||||
// closed, so "onClose fired" is the observable proof the socket was released.
|
||||
type natSessionHandler struct {
|
||||
released chan error
|
||||
}
|
||||
|
||||
func (h *natSessionHandler) NewPacketConnectionEx(ctx context.Context, conn N.PacketConn, source M.Socksaddr, destination M.Socksaddr, onClose N.CloseHandlerFunc) {
|
||||
go func() {
|
||||
for {
|
||||
buffer := buf.NewSize(1024)
|
||||
_, err := conn.ReadPacket(buffer)
|
||||
buffer.Release()
|
||||
if err != nil {
|
||||
if onClose != nil {
|
||||
onClose(err)
|
||||
}
|
||||
h.released <- err
|
||||
return
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
// TestTProxyCloseReleasesNatSessions pins the apply-level half of the leak.
|
||||
//
|
||||
// Closing the inbound used to close the listener only. The NAT cache evicts
|
||||
// lazily, so after the inbound is gone nothing ever touches it again and every
|
||||
// live session — with the write-back socket it holds on the router's own
|
||||
// LAN :53 — was left to a GC finalizer. On this fork the engine is rebuilt on
|
||||
// every config apply, so that is one stranded generation of sockets per apply.
|
||||
func TestTProxyCloseReleasesNatSessions(t *testing.T) {
|
||||
handler := &natSessionHandler{released: make(chan error, 1)}
|
||||
tp := &TProxy{
|
||||
ctx: context.Background(),
|
||||
listener: listener.New(listener.Options{
|
||||
Context: context.Background(),
|
||||
Listen: option.ListenOptions{},
|
||||
}),
|
||||
}
|
||||
prepared := 0
|
||||
tp.udpNat = udpnat.New(handler, func(source M.Socksaddr, destination M.Socksaddr, userData any) (bool, context.Context, N.PacketWriter, N.CloseHandlerFunc) {
|
||||
prepared++
|
||||
return true, context.Background(), nil, func(error) {}
|
||||
}, time.Minute, false)
|
||||
|
||||
tp.udpNat.NewPacket(
|
||||
[][]byte{{0x00}},
|
||||
M.SocksaddrFrom(netip.MustParseAddr("10.67.0.2"), 40000),
|
||||
M.SocksaddrFrom(netip.MustParseAddr("10.67.0.1"), 53),
|
||||
nil,
|
||||
)
|
||||
if prepared != 1 {
|
||||
t.Fatalf("expected one NAT session, got %d", prepared)
|
||||
}
|
||||
|
||||
if err := tp.Close(); err != nil {
|
||||
t.Fatalf("Close: %v", err)
|
||||
}
|
||||
select {
|
||||
case <-handler.released:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("Close must release every live NAT session (and with it the write-back socket it holds)")
|
||||
}
|
||||
}
|
||||
@@ -21,11 +21,25 @@ package netplane
|
||||
// flow that crosses one of those windows keeps a conntrack entry that was formed
|
||||
// against a plane that no longer exists, and — for UDP, which has no handshake to
|
||||
// resynchronise on — every retry merely refreshes that entry instead of
|
||||
// re-deriving the path. The flow stays wedged for as long as the client keeps
|
||||
// asking, which is exactly the shape of the "DNS to the router's own LAN address
|
||||
// never comes back after `/etc/init.d/shater restart`" report: one flow family
|
||||
// dead, everything else healthy, `plane=full`, and a reconcile that fixes nothing
|
||||
// because there is nothing in the RULESET left to fix.
|
||||
// re-deriving the path.
|
||||
//
|
||||
// # What this is NOT
|
||||
//
|
||||
// This flush is HYGIENE, not the cure for the "DNS to the router's own LAN
|
||||
// address never comes back after a restart" report (B3). That turned out to be a
|
||||
// socket-level collision: the engine's TPROXY UDP write-back sockets were bound
|
||||
// UNCONNECTED to the original destination — the router's own LAN address :53 —
|
||||
// and so joined the kernel's demultiplex set next to dnsmasq's socket, silently
|
||||
// swallowing the host's own queries. The fix for that lives in the engine
|
||||
// (protocol/redirect/tproxy.go, lx:tproxy_writeback_connect); the stale
|
||||
// `[UNREPLIED]` conntrack entry seen on the live box was a CONSEQUENCE of the
|
||||
// unanswered query, not its cause.
|
||||
//
|
||||
// It is kept because it is independently correct and costs one netlink
|
||||
// round-trip per plane change: a DNS flow that was mid-flight across a plane
|
||||
// rebuild has a conntrack entry describing a delivery path that no longer
|
||||
// exists, and dropping it makes the first query after a restart re-derive its
|
||||
// path immediately instead of waiting out a retry.
|
||||
//
|
||||
// # Scope: :53/UDP only, deliberately
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user