socks5: add an idle timeout to UDP associations

app/cmd/client.go

@@ -195,6 +195,11 @@ type socks5Config struct {
 	Username   string `mapstructure:"username"`
 	Password   string `mapstructure:"password"`
 	DisableUDP bool   `mapstructure:"disableUDP"`
+	// Timeout is the idle timeout of a UDP association. Zero uses the default.
+	// Named `timeout` to match udpTProxyConfig, udpRedirectConfig and tunConfig,
+	// which spell the same knob that way — tunConfig included, which likewise
+	// carries both TCP and UDP and still calls it `timeout`.
+	Timeout time.Duration `mapstructure:"timeout"`
 }
 
 type httpConfig struct {
@@ -1011,6 +1016,7 @@ func clientSOCKS5(config socks5Config, c client.Client) error {
 		HyClient:    c,
 		AuthFunc:    authFunc,
 		DisableUDP:  config.DisableUDP,
+		UDPTimeout:  config.Timeout,
 		EventLogger: &socks5Logger{},
 	}
 	logger.Info("SOCKS5 server listening", zap.String("addr", config.Listen))

app/cmd/client_test.go

@@ -102,6 +102,7 @@ func TestClientConfig(t *testing.T) {
 			Username:   "anon",
 			Password:   "bro",
 			DisableUDP: true,
+			Timeout:    30 * time.Second,
 		},
 		HTTP: &httpConfig{
 			Listen:   "127.0.0.1:8080",

app/cmd/client_test.yaml

@@ -81,6 +81,7 @@ socks5:
   username: anon
   password: bro
   disableUDP: true
+  timeout: 30s
 
 http:
   listen: 127.0.0.1:8080

app/internal/http/server.go

@@ -36,6 +36,21 @@ type EventLogger interface {
 	HTTPError(addr net.Addr, reqURL string, err error)
 }
 
+const (
+	// handshakeTimeout bounds how long a client may take to send its FIRST
+	// request header. A connection that is opened and then never speaks is not
+	// a client waiting to be served.
+	handshakeTimeout = 10 * time.Second
+
+	// keepAliveIdleTimeout bounds the wait BETWEEN keep-alive requests, where a
+	// silent client is ordinary rather than suspect. Deliberately much longer
+	// than handshakeTimeout: browsers hold idle keep-alive connections for tens
+	// of seconds, and closing them sooner would trade one leak for a lot of
+	// needless reconnects. Matches the 60s already used by the other entry
+	// points.
+	keepAliveIdleTimeout = 60 * time.Second
+)
+
 func (s *Server) Serve(listener net.Listener) error {
 	for {
 		conn, err := listener.Accept()
@@ -48,13 +63,25 @@ func (s *Server) Serve(listener net.Listener) error {
 
 func (s *Server) dispatch(conn net.Conn) {
 	bufReader := bufio.NewReader(conn)
+	first := true
 	for {
+		// Bound the wait for a request header; without it a client that connects
+		// and stays silent pins a goroutine and a descriptor for the lifetime of
+		// the process. Cleared below once the request is in, so it never applies
+		// to the body or to proxied traffic.
+		timeout := keepAliveIdleTimeout
+		if first {
+			timeout = handshakeTimeout
+		}
+		_ = conn.SetReadDeadline(time.Now().Add(timeout))
 		req, err := http.ReadRequest(bufReader)
 		if err != nil {
 			// Connection error or invalid request
 			_ = conn.Close()
 			return
 		}
+		_ = conn.SetReadDeadline(time.Time{})
+		first = false
 		if s.AuthFunc != nil {
 			authOK := false
 			// Check the Proxy-Authorization header

app/internal/proxymux/mux.go

@@ -6,8 +6,13 @@ import (
 	"io"
 	"net"
 	"sync"
+	"time"
 )
 
+// sniffTimeout bounds how long we wait for the first byte that tells HTTP from
+// SOCKS5 apart.
+const sniffTimeout = 10 * time.Second
+
 func newMuxListener(listener net.Listener, deleteFunc func()) *muxListener {
 	l := &muxListener{
 		base:       listener,
@@ -116,11 +121,15 @@ func (l *muxListener) mainLoop() {
 }
 
 func (l *muxListener) dispatch(conn net.Conn) {
+	// Bound the protocol sniff. A client that connects and sends nothing would
+	// otherwise hold this goroutine and its file descriptor forever.
+	_ = conn.SetReadDeadline(time.Now().Add(sniffTimeout))
 	var b [1]byte
 	if _, err := io.ReadFull(conn, b[:]); err != nil {
 		conn.Close()
 		return
 	}
+	_ = conn.SetReadDeadline(time.Time{})
 
 	l.lock.Lock()
 	var target *subListener

app/internal/proxymux/mux_test.go

@@ -54,6 +54,10 @@ func testMockConn(t *testing.T, b []byte) net.Conn {
 		isClosed = true
 		return nil
 	})
+	// dispatch() bounds the protocol sniff with a read deadline and clears it
+	// afterwards. Maybe() because not every caller of this helper reaches
+	// dispatch.
+	mockConn.EXPECT().SetReadDeadline(mock.Anything).Return(nil).Maybe()
 	return mockConn
 }
 

app/internal/socks5/server.go

@@ -2,21 +2,39 @@ package socks5
 
 import (
 	"encoding/binary"
+	"errors"
 	"io"
 	"net"
+	"time"
 
 	"github.com/txthinking/socks5"
 
 	"github.com/apernet/hysteria/core/v2/client"
 )
 
-const udpBufferSize = 4096
+const (
+	udpBufferSize = 4096
+
+	// defaultUDPTimeout is the idle timeout of a UDP association, matching the
+	// value already used by the tproxy and forwarding entry points.
+	defaultUDPTimeout = 60 * time.Second
+
+	// handshakeTimeout bounds how long a client may take to complete the SOCKS5
+	// handshake. Without it a client that connects and then stays silent pins a
+	// goroutine and a file descriptor for the lifetime of the process.
+	handshakeTimeout = 10 * time.Second
+)
 
 // Server is a SOCKS5 server using a Hysteria client as outbound.
 type Server struct {
-	HyClient    client.Client
-	AuthFunc    func(username, password string) bool // nil = no authentication
-	DisableUDP  bool
+	HyClient   client.Client
+	AuthFunc   func(username, password string) bool // nil = no authentication
+	DisableUDP bool
+	// UDPTimeout is the idle timeout of a UDP association. Zero means
+	// defaultUDPTimeout. A UDP association owns an ephemeral port for as long as
+	// it lives, so without a timeout a client that never closes its control
+	// connection pins that port indefinitely.
+	UDPTimeout  time.Duration
 	EventLogger EventLogger
 }
 
@@ -38,6 +56,9 @@ func (s *Server) Serve(listener net.Listener) error {
 }
 
 func (s *Server) dispatch(conn net.Conn) {
+	// Bound the handshake. Cleared below so it never applies to the proxied
+	// traffic itself, which is legitimately long-lived and idle.
+	_ = conn.SetReadDeadline(time.Now().Add(handshakeTimeout))
 	ok, _ := s.negotiate(conn)
 	if !ok {
 		_ = conn.Close()
@@ -49,6 +70,7 @@ func (s *Server) dispatch(conn net.Conn) {
 		_ = conn.Close()
 		return
 	}
+	_ = conn.SetReadDeadline(time.Time{})
 	switch req.Cmd {
 	case socks5.CmdConnect: // TCP
 		s.handleTCP(conn, req)
@@ -218,13 +240,40 @@ func (s *Server) handleUDP(conn net.Conn, req *socks5.Request) {
 		errChan <- err
 	}()
 	closeErr = <-errChan
+	if isUDPTimeout(closeErr) {
+		// Going idle is how a UDP association normally ends: there is no FIN in
+		// UDP, so silence is the only signal we get. Reporting it as an error
+		// would fill the log with routine events.
+		closeErr = nil
+	}
+}
+
+// updateUDPDeadline slides the association's idle deadline forward. Mirrors
+// UDPTProxy.updateConnDeadline in app/internal/tproxy/udp_linux.go.
+func (s *Server) updateUDPDeadline(conn *net.UDPConn) error {
+	timeout := s.UDPTimeout
+	if timeout == 0 {
+		timeout = defaultUDPTimeout
+	}
+	return conn.SetReadDeadline(time.Now().Add(timeout))
+}
+
+// isUDPTimeout reports whether err is this association going idle, which is an
+// ordinary end of life rather than a failure.
+func isUDPTimeout(err error) bool {
+	var netErr net.Error
+	return errors.As(err, &netErr) && netErr.Timeout()
 }
 
 func (s *Server) udpServer(udpConn *net.UDPConn, hyUDP client.HyUDPConn) error {
 	var clientAddr *net.UDPAddr
 	buf := make([]byte, udpBufferSize)
 	// local -> remote
 	for {
+		// Refresh the idle deadline before every read. An association that stops
+		// carrying traffic must release its ephemeral port instead of waiting for
+		// the control connection, which the client may never close.
+		_ = s.updateUDPDeadline(udpConn)
 		n, cAddr, err := udpConn.ReadFromUDP(buf)
 		if err != nil {
 			return err
@@ -250,6 +299,8 @@ func (s *Server) udpServer(udpConn *net.UDPConn, hyUDP client.HyUDPConn) error {
 						_ = udpConn.Close()
 						return
 					}
+					// Traffic in this direction keeps the association alive too.
+					_ = s.updateUDPDeadline(udpConn)
 					atyp, addr, port, err := socks5.ParseAddress(from)
 					if err != nil {
 						continue

app/internal/socks5/udp_exhaustion_test.go

@@ -0,0 +1,73 @@
+package socks5
+
+import (
+	"net"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apernet/hysteria/app/v2/internal/utils_test"
+)
+
+// countHeldPorts reports how many of the given relay ports are still bound.
+func countHeldPorts(ports []int) int {
+	held := 0
+	for _, p := range ports {
+		c, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: p})
+		if err != nil {
+			held++
+			continue
+		}
+		_ = c.Close()
+	}
+	return held
+}
+
+// TestUDPAssociationsDoNotAccumulate measures the defect directly: a client that
+// opens many UDP associations and keeps every control connection open — the
+// shape produced by a tun-to-SOCKS bridge forwarding DNS — must not pin one
+// ephemeral port per association indefinitely.
+//
+// Run against the unpatched code this reports "held after idle: 50 of 50".
+func TestUDPAssociationsDoNotAccumulate(t *testing.T) {
+	const associations = 50
+	const idle = 300 * time.Millisecond
+
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	assert.NoError(t, err)
+	defer l.Close()
+
+	s := &Server{
+		HyClient:   &utils_test.MockEchoHyClient{},
+		UDPTimeout: idle,
+	}
+	go func() { _ = s.Serve(l) }()
+
+	conns := make([]net.Conn, 0, associations)
+	ports := make([]int, 0, associations)
+	for i := 0; i < associations; i++ {
+		c, p := associate(t, l.Addr().String())
+		conns = append(conns, c)
+		ports = append(ports, p)
+	}
+	defer func() {
+		for _, c := range conns {
+			_ = c.Close()
+		}
+	}()
+
+	heldWhileLive := countHeldPorts(ports)
+	t.Logf("held while live:  %d of %d", heldWhileLive, associations)
+	assert.Equal(t, associations, heldWhileLive,
+		"control: every live association must hold its port")
+
+	// Every control connection stays open. Only the idle timeout can release
+	// these ports.
+	assert.Eventually(t, func() bool { return countHeldPorts(ports) == 0 },
+		5*time.Second, 100*time.Millisecond,
+		"all idle associations must release their ports")
+
+	heldAfterIdle := countHeldPorts(ports)
+	t.Logf("held after idle:  %d of %d  (control connections still open)", heldAfterIdle, associations)
+}