From f275fb1f8240309e78fe1667b1f109cfe7632254 Mon Sep 17 00:00:00 2001 From: Jaime Pillora Date: Fri, 1 May 2026 08:14:30 +1000 Subject: [PATCH 1/6] Fix race in tunnel waitGroup causing negative counter panic (#586) Co-authored-by: Claude Opus 4.7 (1M context) --- share/tunnel/wg.go | 24 ++++++++++------ share/tunnel/wg_test.go | 63 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 share/tunnel/wg_test.go diff --git a/share/tunnel/wg.go b/share/tunnel/wg.go index e915ad0a..e3a3d8eb 100644 --- a/share/tunnel/wg.go +++ b/share/tunnel/wg.go @@ -1,30 +1,36 @@ package tunnel -import ( - "sync" - "sync/atomic" -) +import "sync" type waitGroup struct { + mu sync.Mutex inner sync.WaitGroup - n int32 + n int } func (w *waitGroup) Add(n int) { - atomic.AddInt32(&w.n, int32(n)) + w.mu.Lock() + w.n += n w.inner.Add(n) + w.mu.Unlock() } func (w *waitGroup) Done() { - if n := atomic.LoadInt32(&w.n); n > 0 && atomic.CompareAndSwapInt32(&w.n, n, n-1) { + w.mu.Lock() + if w.n > 0 { + w.n-- w.inner.Done() } + w.mu.Unlock() } func (w *waitGroup) DoneAll() { - for atomic.LoadInt32(&w.n) > 0 { - w.Done() + w.mu.Lock() + for w.n > 0 { + w.n-- + w.inner.Done() } + w.mu.Unlock() } func (w *waitGroup) Wait() { diff --git a/share/tunnel/wg_test.go b/share/tunnel/wg_test.go new file mode 100644 index 00000000..01e0b844 --- /dev/null +++ b/share/tunnel/wg_test.go @@ -0,0 +1,63 @@ +package tunnel + +import ( + "sync" + "testing" + "time" +) + +// TestWaitGroupAddDoneAllRace stresses concurrent Add/DoneAll to reproduce the +// race fixed in #585. Without the mutex, Add() is non-atomic between bumping +// the counter and the inner sync.WaitGroup, so DoneAll()'s loop can call +// inner.Done() before inner.Add() runs and panic with "negative WaitGroup +// counter". +func TestWaitGroupAddDoneAllRace(t *testing.T) { + var wg waitGroup + stop := make(chan struct{}) + var workers sync.WaitGroup + for i := 0; i < 8; i++ { + workers.Add(2) + go func() { + defer workers.Done() + for { + select { + case <-stop: + return + default: + wg.Add(1) + } + } + }() + go func() { + defer workers.Done() + for { + select { + case <-stop: + return + default: + wg.DoneAll() + } + } + }() + } + time.Sleep(500 * time.Millisecond) + close(stop) + workers.Wait() + wg.DoneAll() + wg.Wait() +} + +func TestWaitGroupDoneAllDrains(t *testing.T) { + var wg waitGroup + wg.Add(3) + wg.DoneAll() + wg.Wait() +} + +func TestWaitGroupDoneIsIdempotent(t *testing.T) { + var wg waitGroup + wg.Add(1) + wg.Done() + wg.Done() // extra Done must be a no-op, not a panic + wg.Wait() +} From a2e422127f62027ca7426c586c7a2ed514227c6f Mon Sep 17 00:00:00 2001 From: Jaime Pillora Date: Sat, 27 Jun 2026 23:02:05 +1000 Subject: [PATCH 2/6] Improve SOCKS auth: enforce per-user ACL on socks channels (#591) Co-authored-by: Claude Opus 4.8 --- share/settings/remote.go | 7 ++ share/tunnel/tunnel_out_ssh.go | 4 +- test/e2e/acl_channel_test.go | 137 +++++++++++++++++++++++++++++++++ test/e2e/socks_test.go | 114 ++++++++++++++++++++++++++- 4 files changed, 258 insertions(+), 4 deletions(-) diff --git a/share/settings/remote.go b/share/settings/remote.go index bfe6cbae..520b2fcd 100644 --- a/share/settings/remote.go +++ b/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 } diff --git a/share/tunnel/tunnel_out_ssh.go b/share/tunnel/tunnel_out_ssh.go index 0f2fb5d2..3b380fa8 100644 --- a/share/tunnel/tunnel_out_ssh.go +++ b/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 diff --git a/test/e2e/acl_channel_test.go b/test/e2e/acl_channel_test.go index 65e18388..0b5ce147 100644 --- a/test/e2e/acl_channel_test.go +++ b/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") +} diff --git a/test/e2e/socks_test.go b/test/e2e/socks_test.go index 420f4337..cc3b155a 100644 --- a/test/e2e/socks_test.go +++ b/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") +} From 82dfd48e158b73fe3eee5f652ea67b05a7a5525a Mon Sep 17 00:00:00 2001 From: arshiya-99 Date: Fri, 10 Jul 2026 13:07:23 +0530 Subject: [PATCH 3/6] clean workspace dependencies for release --- go.mod | 4 ++-- go.sum | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index d665ad8a..73cf3c04 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/jpillora/chisel -go 1.26.4 +go 1.26.5 require ( github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 @@ -10,7 +10,7 @@ require ( github.com/jpillora/requestlog v1.0.0 github.com/jpillora/sizestr v1.0.0 golang.org/x/crypto v0.53.0 - golang.org/x/net v0.55.0 + golang.org/x/net v0.56.0 golang.org/x/sync v0.21.0 ) diff --git a/go.sum b/go.sum index 359b1cc4..a2e8edb8 100644 --- a/go.sum +++ b/go.sum @@ -18,8 +18,8 @@ github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce h1:fb190+cK2Xz/dvi9 github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce/go.mod h1:o8v6yHRoik09Xen7gje4m9ERNah1d1PPsVq1VEx9vE4= golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= -golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= -golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= From a1c05033e2ba14908208416b4513d050ca220ca3 Mon Sep 17 00:00:00 2001 From: arshiya-99 Date: Mon, 13 Jul 2026 12:07:18 +0530 Subject: [PATCH 4/6] Add .trivyignore to suppress CVE-2026-48113 false positive Trivy flags CVE-2026-48113 on our build because the OutSystems fork tag `1.11.5-os.N` is a SemVer pre-release of `1.11.5`, sorting below the fixed version. The fix is already present via PR #51. Ref: RDODCP-917 --- .trivyignore | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .trivyignore diff --git a/.trivyignore b/.trivyignore new file mode 100644 index 00000000..ca9dd300 --- /dev/null +++ b/.trivyignore @@ -0,0 +1,11 @@ +# Trivy ignore file — Cloud Connector / Chisel publish pipeline +# +# CVE-2026-48113 — chisel "ACL Bypass via Post-Handshake SSH Channel ExtraData Injection". +# FALSE POSITIVE for our build: the fix is already present in chisel (PR #51, +# "Enforce auth ACL on tunnel channels", contained in tags v1.11.5-os.1/2/3). +# Trivy flags it only because the OutSystems fork tag `1.11.5-os.N` is a SemVer +# *pre-release* of `1.11.5`, so it sorts *below* the fixed `1.11.5`. +# Re-evaluate (and remove this entry) when chisel rebases onto an upstream +# version > 1.11.5, at which point the tag sorts above the fix. +# Ref: RDODCP-917 +CVE-2026-48113 From 8f87f8dc9763bbffcd7e2517f1c50191da1edb79 Mon Sep 17 00:00:00 2001 From: arshiya-99 Date: Mon, 13 Jul 2026 14:58:57 +0530 Subject: [PATCH 5/6] Remove .trivyignore --- .trivyignore | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 .trivyignore diff --git a/.trivyignore b/.trivyignore deleted file mode 100644 index ca9dd300..00000000 --- a/.trivyignore +++ /dev/null @@ -1,11 +0,0 @@ -# Trivy ignore file — Cloud Connector / Chisel publish pipeline -# -# CVE-2026-48113 — chisel "ACL Bypass via Post-Handshake SSH Channel ExtraData Injection". -# FALSE POSITIVE for our build: the fix is already present in chisel (PR #51, -# "Enforce auth ACL on tunnel channels", contained in tags v1.11.5-os.1/2/3). -# Trivy flags it only because the OutSystems fork tag `1.11.5-os.N` is a SemVer -# *pre-release* of `1.11.5`, so it sorts *below* the fixed `1.11.5`. -# Re-evaluate (and remove this entry) when chisel rebases onto an upstream -# version > 1.11.5, at which point the tag sorts above the fix. -# Ref: RDODCP-917 -CVE-2026-48113 From b7c61b050aebb5a0f519aeda009f188a3b23fe92 Mon Sep 17 00:00:00 2001 From: arshiya-99 Date: Mon, 13 Jul 2026 15:47:45 +0530 Subject: [PATCH 6/6] Wire Verbose flag to client logger level (upstream #281) The Verbose field was present in Config but never used. Apply the missing half of upstream commit 200a8e2: set Logger.Info = c.Verbose so callers can control log verbosity programmatically. --- client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/client.go b/client/client.go index 49b62e65..59698be6 100644 --- a/client/client.go +++ b/client/client.go @@ -105,7 +105,7 @@ func NewClient(c *Config) (*Client, error) { tlsConfig: nil, } //set default log level - client.Logger.Info = true + client.Logger.Info = c.Verbose //configure tls if u.Scheme == "wss" { tc := &tls.Config{}