fix(tunnel): restrict reverse UDP return peers

- record only listener peers observed from inbound UDP - expire and cap peer state, rejecting untrusted destinations - cover reverse UDP flow and destination validation Co-Authored-By: Codex GPT-5 <[email protected]>
2026-10-12 02:16:57 +00:00 · 2026-09-01 20:44:13 +10:00
parent fe4f4fe7e6
commit 3c00f04ca1
4 changed files with 245 additions and 29 deletions
@@ -490,8 +490,8 @@ Less common knobs are environment variables, all read with a `CHISEL_` prefix (e
 | `WS_READ_LIMIT` | both        | `524288`           | max inbound websocket message bytes (0 = no limit; negative = default) |
 | `WS_BUFF_SIZE`  | both        | go default         | websocket read/write buffer sizes                  |
 | `UDP_MAX_SIZE`  | both        | `9012`             | max udp packet bytes                               |
-| `UDP_DEADLINE`  | exit node   | `15s`              | udp flow read deadline and idle-sweep age          |
-| `UDP_MAX_CONNS` | exit node   | `100`              | max concurrent udp flows per tunnel                |
+| `UDP_DEADLINE`  | both        | `15s`              | udp flow read deadline and return-peer expiry      |
+| `UDP_MAX_CONNS` | both        | `100`              | max udp flows or return peers per tunnel           |
 | `SHUTDOWN_GRACE`| server      | `5s`               | http request drain time on shutdown                |

 `HOST`, `PORT`, `AUTH`, and `CHISEL_KEY`/`CHISEL_KEY_FILE` are documented in the `--help` texts above.
@@ -18,18 +18,19 @@ import (
 	"golang.org/x/sync/errgroup"
 )

-//listenUDP is a special listener which forwards packets via
-//the bound ssh connection. tricky part is multiplexing lots of
-//udp clients through the entry node. each will listen on its
-//own source-port for a response:
-//                                                (random)
-//    src-1 1111->...                         dst-1 6345->7777
-//    src-2 2222->... <---> udp <---> udp <-> dst-1 7543->7777
-//    src-3 3333->...    listener    handler  dst-1 1444->7777
+// listenUDP is a special listener which forwards packets via
+// the bound ssh connection. tricky part is multiplexing lots of
+// udp clients through the entry node. each will listen on its
+// own source-port for a response:
 //
-//we must store these mappings (1111-6345, etc) in memory for a length
-//of time, so that when the exit node receives a response on 6345, it
-//knows to return it to 1111.
+//	                                            (random)
+//	src-1 1111->...                         dst-1 6345->7777
+//	src-2 2222->... <---> udp <---> udp <-> dst-1 7543->7777
+//	src-3 3333->...    listener    handler  dst-1 1444->7777
+//
+// we must store these mappings (1111-6345, etc) in memory for a length
+// of time, so that when the exit node receives a response on 6345, it
+// knows to return it to 1111.
 func listenUDP(l *cio.Logger, sshTun sshTunnel, remote *settings.Remote) (*udpListener, error) {
 	a, err := net.ResolveUDPAddr("udp", remote.Local())
 	if err != nil {
@@ -41,11 +42,14 @@ func listenUDP(l *cio.Logger, sshTun sshTunnel, remote *settings.Remote) (*udpLi
 	}
 	//ready
 	u := &udpListener{
-		Logger:  l,
-		sshTun:  sshTun,
-		remote:  remote,
-		inbound: conn,
-		maxMTU:  settings.EnvInt("UDP_MAX_SIZE", 9012),
+		Logger:       l,
+		sshTun:       sshTun,
+		remote:       remote,
+		inbound:      conn,
+		maxMTU:       settings.EnvInt("UDP_MAX_SIZE", 9012),
+		peers:        map[string]udpPeer{},
+		peerDeadline: settings.EnvDuration("UDP_DEADLINE", 15*time.Second),
+		maxPeers:     settings.EnvInt("UDP_MAX_CONNS", 100),
 	}
 	u.Debugf("UDP max size: %d bytes", u.maxMTU)
 	return u, nil
@@ -53,13 +57,24 @@ func listenUDP(l *cio.Logger, sshTun sshTunnel, remote *settings.Remote) (*udpLi

 type udpListener struct {
 	*cio.Logger
-	sshTun      sshTunnel
-	remote      *settings.Remote
-	inbound     *net.UDPConn
-	outboundMut sync.Mutex
-	outbound    *udpChannel
-	sent, recv  int64
-	maxMTU      int
+	sshTun       sshTunnel
+	remote       *settings.Remote
+	inbound      *net.UDPConn
+	outboundMut  sync.Mutex
+	outbound     *udpChannel
+	peerMut      sync.Mutex
+	peers        map[string]udpPeer
+	peerDeadline time.Duration
+	maxPeers     int
+	sent, recv   int64
+	maxMTU       int
+}
+
+// udpPeer is a listener peer that actually sent a datagram to the bound UDP
+// socket. Only these addresses may receive a return packet from the client.
+type udpPeer struct {
+	addr     *net.UDPAddr
+	lastSeen time.Time
 }

 func (u *udpListener) run(ctx context.Context) error {
@@ -94,6 +109,9 @@ func (u *udpListener) runInbound(ctx context.Context) error {
 		if err != nil {
 			return u.Errorf("read error: %w", err)
 		}
+		// Only accept return packets for peers which have actually sent to
+		// this listener. The peer address is otherwise client-controlled.
+		u.rememberPeer(addr)
 		//upsert ssh channel
 		uc, err := u.getUDPChan(ctx)
 		if err != nil {
@@ -134,10 +152,12 @@ func (u *udpListener) runOutbound(ctx context.Context) error {
 		} else if err != nil {
 			return u.Errorf("decode error: %w", err)
 		}
-		//write back to inbound udp
-		addr, err := net.ResolveUDPAddr("udp", p.Src)
-		if err != nil {
-			return u.Errorf("resolve error: %w", err)
+		//write back to an observed UDP peer only. p.Src comes from the SSH
+		//client, so it must not be resolved or used as a new destination.
+		addr, ok := u.returnPeer(p.Src)
+		if !ok {
+			u.Debugf("Dropping UDP return packet for unobserved peer %q", p.Src)
+			continue
 		}
 		n, err := u.inbound.WriteToUDP(p.Payload, addr)
 		if err != nil {
@@ -149,6 +169,68 @@ func (u *udpListener) runOutbound(ctx context.Context) error {
 	return nil
 }

+// rememberPeer records an address obtained directly from ReadFromUDP. Entries
+// expire after UDP_DEADLINE and the map is capped at UDP_MAX_CONNS so an
+// Internet-facing listener cannot retain unbounded peer state.
+func (u *udpListener) rememberPeer(addr *net.UDPAddr) {
+	now := time.Now()
+	key := addr.String()
+	u.peerMut.Lock()
+	defer u.peerMut.Unlock()
+	if u.peers == nil {
+		u.peers = map[string]udpPeer{}
+	}
+	u.expirePeers(now)
+	if _, found := u.peers[key]; !found && len(u.peers) >= u.maxPeers {
+		u.evictOldestPeer()
+	}
+	// ReadFromUDP allocates the address today, but retain our own copy so the
+	// peer table cannot depend on that implementation detail.
+	peerAddr := *addr
+	peerAddr.IP = append(net.IP(nil), addr.IP...)
+	u.peers[key] = udpPeer{addr: &peerAddr, lastSeen: now}
+}
+
+// returnPeer obtains a non-expired peer recorded by rememberPeer. Matching the
+// exact string sent by runInbound also rejects hostname and alternate-address
+// encodings supplied by a client.
+func (u *udpListener) returnPeer(src string) (*net.UDPAddr, bool) {
+	now := time.Now()
+	u.peerMut.Lock()
+	defer u.peerMut.Unlock()
+	peer, found := u.peers[src]
+	if !found {
+		return nil, false
+	}
+	if peer.lastSeen.Before(now.Add(-u.peerDeadline)) {
+		delete(u.peers, src)
+		return nil, false
+	}
+	return peer.addr, true
+}
+
+func (u *udpListener) expirePeers(now time.Time) {
+	for key, peer := range u.peers {
+		if peer.lastSeen.Before(now.Add(-u.peerDeadline)) {
+			delete(u.peers, key)
+		}
+	}
+}
+
+func (u *udpListener) evictOldestPeer() {
+	var oldestKey string
+	var oldest time.Time
+	for key, peer := range u.peers {
+		if oldestKey == "" || peer.lastSeen.Before(oldest) {
+			oldestKey = key
+			oldest = peer.lastSeen
+		}
+	}
+	if oldestKey != "" {
+		delete(u.peers, oldestKey)
+	}
+}
+
 func (u *udpListener) getUDPChan(ctx context.Context) (*udpChannel, error) {
 	u.outboundMut.Lock()
 	defer u.outboundMut.Unlock()
@@ -0,0 +1,81 @@
+package tunnel
+
+import (
+	"context"
+	"encoding/gob"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/jpillora/chisel/share/cio"
+)
+
+func TestUDPListenerReturnsOnlyToObservedPeers(t *testing.T) {
+	listener, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer listener.Close()
+	allowed, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer allowed.Close()
+	blocked, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer blocked.Close()
+
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	u := &udpListener{
+		Logger:       cio.NewLogger("test"),
+		inbound:      listener,
+		outbound:     &udpChannel{r: gob.NewDecoder(server), w: gob.NewEncoder(server), c: server},
+		peers:        map[string]udpPeer{},
+		peerDeadline: time.Second,
+		maxPeers:     2,
+	}
+	u.rememberPeer(allowed.LocalAddr().(*net.UDPAddr))
+	runErr := make(chan error, 1)
+	go func() { runErr <- u.runOutbound(ctx) }()
+
+	encode := gob.NewEncoder(client)
+	if err := encode.Encode(udpPacket{Src: blocked.LocalAddr().String(), Payload: []byte("blocked")}); err != nil {
+		t.Fatal(err)
+	}
+	blocked.SetReadDeadline(time.Now().Add(200 * time.Millisecond))
+	if _, _, err := blocked.ReadFromUDP(make([]byte, 64)); err == nil {
+		t.Fatal("unobserved peer received a return packet")
+	} else if e, ok := err.(net.Error); !ok || !e.Timeout() {
+		t.Fatalf("read from unobserved peer: %v", err)
+	}
+
+	if err := encode.Encode(udpPacket{Src: allowed.LocalAddr().String(), Payload: []byte("allowed")}); err != nil {
+		t.Fatal(err)
+	}
+	allowed.SetReadDeadline(time.Now().Add(time.Second))
+	buf := make([]byte, 64)
+	n, _, err := allowed.ReadFromUDP(buf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := string(buf[:n]); got != "allowed" {
+		t.Fatalf("got %q, want allowed", got)
+	}
+
+	cancel()
+	client.Close()
+	select {
+	case err := <-runErr:
+		if err != nil {
+			t.Fatal(err)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("runOutbound did not stop")
+	}
+}
@@ -73,6 +73,59 @@ func TestUDP(t *testing.T) {
 	}
 }

+func TestReverseUDP(t *testing.T) {
+	echoPort := availableUDPPort()
+	echoAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:"+echoPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+	echo, err := net.ListenUDP("udp", echoAddr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer echo.Close()
+	go func() {
+		buf := make([]byte, 128)
+		for {
+			n, addr, err := echo.ReadFromUDP(buf)
+			if err != nil {
+				return
+			}
+			echo.WriteToUDP(buf[:n], addr)
+		}
+	}()
+
+	inboundPort := availableUDPPort()
+	teardown := simpleSetup(t,
+		&chserver.Config{Reverse: true},
+		&chclient.Config{Remotes: []string{
+			"R:" + inboundPort + ":127.0.0.1:" + echoPort + "/udp",
+		}},
+	)
+	defer teardown()
+
+	deadline := time.Now().Add(2 * time.Second)
+	for time.Now().Before(deadline) {
+		conn, err := net.Dial("udp", "127.0.0.1:"+inboundPort)
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, err = conn.Write([]byte("reverse"))
+		if err == nil {
+			buf := make([]byte, 128)
+			conn.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
+			n, readErr := conn.Read(buf)
+			if readErr == nil && string(buf[:n]) == "reverse" {
+				conn.Close()
+				return
+			}
+		}
+		conn.Close()
+		time.Sleep(20 * time.Millisecond)
+	}
+	t.Fatal("reverse UDP packet did not round-trip")
+}
+
 func availableUDPPort() string {
 	a, _ := net.ResolveUDPAddr("udp", ":0")
 	l, err := net.ListenUDP("udp", a)