Improve SOCKS auth: enforce per-user ACL on socks channels (#591)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: jpillora

share/settings/remote.go

@@ -221,6 +221,13 @@ func (r Remote) UserAddr() string {
 	if r.Reverse {
 		return "R:" + r.LocalHost + ":" + r.LocalPort
 	}
+	//forward socks is granted via the literal "socks" token, matching the
+	//per-channel ACL check in tunnel_out_ssh.go (ExtraData == "socks").
+	//Without this it would be ":" (empty host:port), which is opaque and
+	//inconsistent with the channel-level check.
+	if r.Socks {
+		return "socks"
+	}
 	return r.RemoteHost + ":" + r.RemotePort
 }
 

share/tunnel/tunnel_out_ssh.go

@@ -47,7 +47,9 @@ func (t *Tunnel) handleSSHChannel(ch ssh.NewChannel) {
 		return
 	}
 	//check ACL against the actual requested destination
-	if t.Config.ACL != nil && !socks && !t.Config.ACL(hostPort) {
+	//(hostPort == "socks" for socks channels, so socks is gated too,
+	//symmetric with the config-time UserAddr() check in server_handler.go)
+	if t.Config.ACL != nil && !t.Config.ACL(hostPort) {
 		t.Debugf("Denied connection to %s (ACL)", hostPort)
 		ch.Reject(ssh.Prohibited, "access denied")
 		return

test/e2e/acl_channel_test.go

@@ -6,6 +6,8 @@ import (
 	"io"
 	"net"
 	"net/http"
+	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -321,3 +323,138 @@ func TestAuthWildcardChannel(t *testing.T) {
 	}
 	t.Logf("wildcard user correctly allowed")
 }
+
+// TestAuthSocksChannelDenied verifies that a user who is NOT granted socks
+// cannot open a socks channel, even when the server runs with --socks5.
+// socks channels are authorized against the per-user ACL like any other
+// destination (the channel token is the literal "socks").
+func TestAuthSocksChannelDenied(t *testing.T) {
+	allowedPort := availablePort()
+
+	// SOCKS5 is enabled, so any rejection can only come from the ACL,
+	// not from the "SOCKS5 is not enabled" guard.
+	s, err := chserver.NewServer(&chserver.Config{
+		KeySeed: "acl-socks-denied",
+		Socks5:  true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Debug = debug
+	// user is pinned to a single TCP destination; socks is NOT granted
+	if err := s.AddUser("user", "pass", fmt.Sprintf(`^127\.0\.0\.1:%s$`, allowedPort)); err != nil {
+		t.Fatal(err)
+	}
+	serverPort := availablePort()
+	if err := s.Start("127.0.0.1", serverPort); err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+
+	// declare ONLY the allowed remote so the config handshake passes
+	sc, _, _ := dialChiselSSH(t, "127.0.0.1:"+serverPort, "user", "pass")
+	defer sc.Close()
+	r, err := settings.DecodeRemote(fmt.Sprintf("0.0.0.0:%s:127.0.0.1:%s", allowedPort, allowedPort))
+	if err != nil {
+		t.Fatal(err)
+	}
+	sendConfig(t, sc, []*settings.Remote{r})
+
+	// open a raw channel whose ExtraData is "socks" — must be denied by the ACL
+	ch, _, err := sc.OpenChannel("chisel", []byte("socks"))
+	if err == nil {
+		ch.Close()
+		t.Fatal("socks channel was accepted for a user without socks access")
+	}
+	// it must be the ACL talking, not "SOCKS5 is not enabled"
+	if !strings.Contains(err.Error(), "access denied") {
+		t.Fatalf("socks channel rejected for the wrong reason: %v", err)
+	}
+	t.Logf("socks channel correctly rejected by ACL: %v", err)
+}
+
+// TestAuthSocksChannelAllowed verifies the ACL does not over-block: a user
+// explicitly granted socks (a regex matching the "socks" channel token) can
+// open a socks channel and reach a destination through the SOCKS5 proxy.
+func TestAuthSocksChannelAllowed(t *testing.T) {
+	targetPort := availablePort()
+
+	// a destination to CONNECT to through socks
+	ln, err := net.Listen("tcp", "127.0.0.1:"+targetPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			conn.Write([]byte("VIA-SOCKS"))
+			conn.Close()
+		}
+	}()
+
+	s, err := chserver.NewServer(&chserver.Config{
+		KeySeed: "acl-socks-allowed",
+		Socks5:  true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Debug = debug
+	// grant socks at the channel level (the channel token is the literal "socks")
+	if err := s.AddUser("user", "pass", `^socks$`); err != nil {
+		t.Fatal(err)
+	}
+	serverPort := availablePort()
+	if err := s.Start("127.0.0.1", serverPort); err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+
+	sc, _, _ := dialChiselSSH(t, "127.0.0.1:"+serverPort, "user", "pass")
+	defer sc.Close()
+	// no remotes to declare; the socks channel is opened directly
+	sendConfig(t, sc, []*settings.Remote{})
+
+	ch, reqs, err := sc.OpenChannel("chisel", []byte("socks"))
+	if err != nil {
+		t.Fatalf("socks channel rejected for a user granted socks: %v", err)
+	}
+	go ssh.DiscardRequests(reqs)
+	defer ch.Close()
+
+	// drive a minimal SOCKS5 CONNECT to the allowed local listener
+	if _, err := ch.Write([]byte{0x05, 0x01, 0x00}); err != nil { // greeting: no-auth
+		t.Fatal(err)
+	}
+	if _, err := io.ReadFull(ch, make([]byte, 2)); err != nil { // method selection
+		t.Fatal(err)
+	}
+	port, err := strconv.Atoi(targetPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+	connect := []byte{0x05, 0x01, 0x00, 0x01, 127, 0, 0, 1, byte(port >> 8), byte(port)}
+	if _, err := ch.Write(connect); err != nil {
+		t.Fatal(err)
+	}
+	reply := make([]byte, 10)
+	if _, err := io.ReadFull(ch, reply); err != nil {
+		t.Fatal(err)
+	}
+	if reply[1] != 0x00 {
+		t.Fatalf("socks CONNECT failed, rep=%d", reply[1])
+	}
+	buf := make([]byte, 16)
+	n, err := ch.Read(buf)
+	if err != nil && err != io.EOF {
+		t.Fatalf("read via socks: %v", err)
+	}
+	if string(buf[:n]) != "VIA-SOCKS" {
+		t.Fatalf("expected 'VIA-SOCKS' via socks, got %q", buf[:n])
+	}
+	t.Logf("socks channel allowed and functional for a granted user")
+}

test/e2e/socks_test.go

@@ -1,5 +1,113 @@
 package e2e_test
 
-//TODO tests for:
-// - SOCKS-client -> [client -> server SOCKS] -> endpoint
-// - SOCKS-client -> [server -> client SOCKS] -> endpoint
+import (
+	"context"
+	"io"
+	"net"
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+
+	"golang.org/x/net/proxy"
+
+	chclient "github.com/jpillora/chisel/client"
+	chserver "github.com/jpillora/chisel/server"
+)
+
+//TODO test: SOCKS-client -> [server -> client SOCKS] -> endpoint (reverse socks)
+
+// TestSocksEndToEnd exercises the full forward-SOCKS5 path for a user granted
+// socks via "^socks$":
+//
+//	http client -> chisel client's local SOCKS5 -> tunnel ->
+//	chisel server's SOCKS5 proxy -> endpoint
+//
+// It covers both ACL checks: the config-time check (UserAddr() == "socks")
+// lets the client register its socks remote, and the per-channel check lets
+// each tunnelled connection through. A "^socks$"-only user only succeeds when
+// both checks agree on the "socks" token.
+func TestSocksEndToEnd(t *testing.T) {
+	// endpoint reached via the socks proxy: echoes the request body + '!'
+	endpoint := &http.Server{
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			b, _ := io.ReadAll(r.Body)
+			w.Write(append(b, '!'))
+		}),
+	}
+	endpointPort := availablePort()
+	el, err := net.Listen("tcp", "127.0.0.1:"+endpointPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+	go endpoint.Serve(el)
+	defer endpoint.Close()
+
+	// chisel server: SOCKS5 enabled, user granted ONLY socks
+	s, err := chserver.NewServer(&chserver.Config{
+		KeySeed: "socks-e2e",
+		Socks5:  true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Debug = debug
+	if err := s.AddUser("user", "pass", `^socks$`); err != nil {
+		t.Fatal(err)
+	}
+	serverPort := availablePort()
+	if err := s.Start("127.0.0.1", serverPort); err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+
+	// chisel client: local SOCKS5 listener, authed as the socks-only user
+	socksPort := availablePort()
+	c, err := chclient.NewClient(&chclient.Config{
+		Server:      "http://127.0.0.1:" + serverPort,
+		Auth:        "user:pass",
+		Fingerprint: s.GetFingerprint(),
+		Remotes:     []string{"127.0.0.1:" + socksPort + ":socks"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	c.Debug = debug
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	if err := c.Start(ctx); err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+
+	// HTTP request through the local SOCKS5 proxy to the endpoint, retrying
+	// until the tunnel is established (bounded ~5s).
+	dialer, err := proxy.SOCKS5("tcp", "127.0.0.1:"+socksPort, nil, proxy.Direct)
+	if err != nil {
+		t.Fatal(err)
+	}
+	httpClient := &http.Client{
+		Timeout: 2 * time.Second,
+		Transport: &http.Transport{
+			DialContext: func(_ context.Context, network, addr string) (net.Conn, error) {
+				return dialer.Dial(network, addr)
+			},
+		},
+	}
+	endpointURL := "http://" + net.JoinHostPort("127.0.0.1", endpointPort) + "/"
+	var body string
+	for i := 0; i < 50; i++ {
+		resp, err := httpClient.Post(endpointURL, "text/plain", strings.NewReader("foo"))
+		if err == nil {
+			b, _ := io.ReadAll(resp.Body)
+			resp.Body.Close()
+			body = string(b)
+			break
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+	if body != "foo!" {
+		t.Fatalf("expected \"foo!\" through socks, got %q", body)
+	}
+	t.Logf("forward socks end-to-end works for a ^socks$ user")
+}