diff --git a/.codespellrc b/.codespellrc new file mode 100644 index 00000000..b934d53c --- /dev/null +++ b/.codespellrc @@ -0,0 +1,3 @@ +[codespell] +skip = go.sum,go.mod,.git,dist,TASKS.md +ignore-words-list = unparseable diff --git a/.github/Dockerfile b/.github/Dockerfile index 872f7e00..b3819af7 100644 --- a/.github/Dockerfile +++ b/.github/Dockerfile @@ -1,16 +1,10 @@ -# build stage -FROM golang:alpine AS build -RUN apk update && apk add git -ADD . /src -WORKDIR /src -ENV CGO_ENABLED=0 -RUN go build \ - -ldflags "-X github.com/jpillora/chisel/share.BuildVersion=$(git describe --abbrev=0 --tags)" \ - -o /tmp/bin -# run stage +# goreleaser builds the binary and provides it in the build context +FROM alpine:3 AS certs +RUN apk add --no-cache ca-certificates + FROM scratch LABEL maintainer="dev@jpillora.com" -COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ +COPY --from=certs /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ WORKDIR /app -COPY --from=build /tmp/bin /app/bin -ENTRYPOINT ["/app/bin"] \ No newline at end of file +COPY chisel /app/bin +ENTRYPOINT ["/app/bin"] diff --git a/.github/goreleaser.yml b/.github/goreleaser.yml index 8ddc1375..f47f74ab 100644 --- a/.github/goreleaser.yml +++ b/.github/goreleaser.yml @@ -52,6 +52,7 @@ archives: - none* release: draft: true + replace_existing_draft: true prerelease: auto changelog: sort: asc @@ -59,3 +60,82 @@ changelog: exclude: - "^docs:" - "^test:" + +dockers: + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-amd64" + - "docker.io/jpillora/chisel:{{ .Version }}-amd64" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/amd64" + goarch: amd64 + + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-arm64" + - "docker.io/jpillora/chisel:{{ .Version }}-arm64" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/arm64" + goarch: arm64 + + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-armv7" + - "docker.io/jpillora/chisel:{{ .Version }}-armv7" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/arm/v7" + goarch: arm + goarm: "7" + + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-armv6" + - "docker.io/jpillora/chisel:{{ .Version }}-armv6" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/arm/v6" + goarch: arm + goarm: "6" + + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-386" + - "docker.io/jpillora/chisel:{{ .Version }}-386" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/386" + goarch: "386" + + - image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-ppc64le" + - "docker.io/jpillora/chisel:{{ .Version }}-ppc64le" + use: buildx + dockerfile: .github/Dockerfile + build_flag_templates: + - "--platform=linux/ppc64le" + goarch: ppc64le + +# version manifests only — floating tags (latest / X / X.Y) are promoted +# by .github/workflows/promote.yml when the draft GitHub release is +# published (release stage 2) +docker_manifests: + - name_template: "ghcr.io/jpillora/chisel:{{ .Version }}" + image_templates: + - "ghcr.io/jpillora/chisel:{{ .Version }}-amd64" + - "ghcr.io/jpillora/chisel:{{ .Version }}-arm64" + - "ghcr.io/jpillora/chisel:{{ .Version }}-armv7" + - "ghcr.io/jpillora/chisel:{{ .Version }}-armv6" + - "ghcr.io/jpillora/chisel:{{ .Version }}-386" + - "ghcr.io/jpillora/chisel:{{ .Version }}-ppc64le" + + - name_template: "docker.io/jpillora/chisel:{{ .Version }}" + image_templates: + - "docker.io/jpillora/chisel:{{ .Version }}-amd64" + - "docker.io/jpillora/chisel:{{ .Version }}-arm64" + - "docker.io/jpillora/chisel:{{ .Version }}-armv7" + - "docker.io/jpillora/chisel:{{ .Version }}-armv6" + - "docker.io/jpillora/chisel:{{ .Version }}-386" + - "docker.io/jpillora/chisel:{{ .Version }}-ppc64le" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0036cc89..9b309af6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,24 @@ name: CI on: pull_request: {} - push: {} -permissions: write-all + push: + branches: [master] + tags: ["v*"] +permissions: + contents: read jobs: # ================ + # SPELLING CHECK + # ================ + codespell: + name: Spelling + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v7 + - name: codespell + uses: codespell-project/actions-codespell@v2 + # ================ # BUILD AND TEST JOB # ================ test: @@ -16,7 +30,7 @@ jobs: runs-on: ${{ matrix.platform }} steps: - name: Checkout - uses: actions/checkout@v5 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Set up Go @@ -29,64 +43,50 @@ jobs: - name: Test run: go test -v ./... # ================ - # RELEASE BINARIES (on push "v*" tag) + # RELEASE STAGE 1 (on push "v*" tag, after merging to master) + # Creates a DRAFT GitHub release and pushes version-tagged binaries, + # packages, and multi-arch Docker images via GoReleaser. Floating + # Docker tags (latest / X / X.Y) are not touched here — publishing + # the draft release triggers promote.yml (stage 2) which retags them. # ================ - release_binaries: - name: Release Binaries + release: + name: Release needs: test if: startsWith(github.ref, 'refs/tags/v') runs-on: ubuntu-latest + permissions: + contents: write + packages: write steps: - - name: Check out code - uses: actions/checkout@v5 + - name: Checkout + uses: actions/checkout@v7 with: fetch-depth: 0 - - name: goreleaser - if: success() - uses: docker://goreleaser/goreleaser:latest - env: - GITHUB_USER: ${{ github.repository_owner }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GOTOOLCHAIN: auto + - name: Set up Go + uses: actions/setup-go@v6 with: - args: release --config .github/goreleaser.yml - # ================ - # RELEASE DOCKER IMAGES (on push "v*" tag) - # ================ - release_docker: - name: Release Docker Images - needs: test - if: startsWith(github.ref, 'refs/tags/v') - runs-on: ubuntu-latest - steps: - - name: Check out code - uses: actions/checkout@v5 + go-version: stable + cache: true - name: Set up QEMU - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@v4 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - name: Login to DockerHub - uses: docker/login-action@v3 + uses: docker/setup-buildx-action@v4 + - name: Login to GHCR + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Login to Docker Hub + uses: docker/login-action@v4 with: username: jpillora password: ${{ secrets.DOCKERHUB_TOKEN }} - - name: Docker meta - id: meta - uses: docker/metadata-action@v5 - with: - images: jpillora/chisel - tags: | - type=semver,pattern={{version}} - type=semver,pattern={{major}}.{{minor}} - type=semver,pattern={{major}} - - name: Build and push - uses: docker/build-push-action@v6 + - name: Run GoReleaser + uses: goreleaser/goreleaser-action@v6 with: - context: . - file: .github/Dockerfile - platforms: linux/amd64,linux/arm64,linux/ppc64le,linux/386,linux/arm/v7,linux/arm/v6 - push: true - tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} - cache-from: type=gha - cache-to: type=gha,mode=max + distribution: goreleaser + version: v2.12.7 + args: release --clean --config .github/goreleaser.yml + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/promote.yml b/.github/workflows/promote.yml new file mode 100644 index 00000000..72b904c6 --- /dev/null +++ b/.github/workflows/promote.yml @@ -0,0 +1,56 @@ +name: Promote +# ================ +# RELEASE STAGE 2 (on publishing the draft GitHub release) +# Promotes the version-tagged multi-arch Docker manifests pushed by +# stage 1 (ci.yml release job) to latest / X / X.Y on GHCR and +# Docker Hub. Pure manifest copy — nothing is rebuilt. +# ================ +on: + release: + types: [published] + workflow_dispatch: + inputs: + tag: + description: 'Release tag to promote (e.g. v1.12.0)' + required: true +permissions: + contents: read +jobs: + docker-tags: + name: Promote Docker tags + # prereleases keep only their version tag; manual dispatch is an + # explicit operator override + if: ${{ github.event_name == 'workflow_dispatch' || !github.event.release.prerelease }} + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - name: Login to GHCR + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.repository_owner }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Login to Docker Hub + uses: docker/login-action@v4 + with: + username: jpillora + password: ${{ secrets.DOCKERHUB_TOKEN }} + - name: Retag version manifests to latest / major / major.minor + env: + TAG: ${{ github.event.release.tag_name || inputs.tag }} + run: | + set -euo pipefail + VERSION="${TAG#v}" + MAJOR="${VERSION%%.*}" + REST="${VERSION#*.}" + MINOR="${REST%%.*}" + echo "promoting ${VERSION} -> latest, ${MAJOR}, ${MAJOR}.${MINOR}" + for repo in ghcr.io/jpillora/chisel docker.io/jpillora/chisel; do + docker buildx imagetools create \ + --tag "${repo}:latest" \ + --tag "${repo}:${MAJOR}" \ + --tag "${repo}:${MAJOR}.${MINOR}" \ + "${repo}:${VERSION}" + done diff --git a/1.12-changes.md b/1.12-changes.md new file mode 100644 index 00000000..a987f94d --- /dev/null +++ b/1.12-changes.md @@ -0,0 +1,101 @@ +# Chisel 1.12 — UX & Compatibility Review + +**Scope:** user-facing changes on branch `ai-incoming` (the unreleased 1.12 line, HEAD `e0b639d`) compared to `master` (`310eec3` — released v1.11.7 `927abde` plus one dependency bump, no UX impact). +**Date:** 2026-07-02, revised 2026-07-17. +**Method:** three parallel code reviews (client surface, server/operator surface, tunnel/wire surface) plus manual verification of every load-bearing claim against the code. This revision re-verified each changed claim against current code. +**Status:** all four recommendations below were implemented on `ai-incoming` (commits `3c66f9b` and `c4038c3`) — the §5 gaps are closed. The branch has since been rebased onto current `master`, with its Go-module and CI-action dependencies brought fully up to date (§7). + +--- + +## Verdict + +The branch is a large but well-behaved surface change. Measured against master, there are only **three genuinely breaking changes for existing users, and the branch documents all of them** — plus it retroactively documents the SOCKS ACL break that master *already shipped* in v1.11.7 with zero README coverage. The one remaining rough edge is a silent automation-facing change (exit codes), which the changelog calls out. The diagnosability gap flagged in the original review — ACL denials logging only at debug level — has since been fixed (they now log at info). + +--- + +## 1. Breaking changes for existing users + +### a. SOCKS + `--authfile` — *already on master, branch adds the missing docs* + +Enforcement (`UserAddr()` → `"socks"`, channel-level ACL gate) shipped in v1.11.7 (`927abde`), so branch-vs-master this is **not a new break** — but today's released chisel has an *undocumented* breaking change: its README/`--help` say nothing about the `socks` token. The branch fixes that in five places (server help, client help, README auth section, SOCKS guide, 1.12 changelog) including the migration line "existing authfiles which should allow SOCKS5 must add an entry matching `socks`". This documentation is arguably the most valuable UX content on the branch. + +The original review flagged a diagnosability gap here: a denied user's server-side trace was `Debugf("Denied connection to socks (ACL)")`, invisible without `-v`, making "socks stopped working after upgrade" the #1 anticipated support ticket. **This is now fixed** — commit `3c66f9b` raised it to `Infof("Denied connection to %s (ACL)", hostPort)` (`share/tunnel/tunnel_out_ssh.go:57`), so operators see socks ACL denials at the default log level. + +### b. Truncated legacy MD5 fingerprints rejected — *branch-new, loud* + +Master's `verifyLegacyFingerprint` used `strings.HasPrefix`, so `--fingerprint a5:b3` matched any key with that MD5 prefix (~1-in-65k spoof risk). The branch requires the full 16-octet colon form (`client/client.go:248`). Affected users fail **loudly** at connect with `Invalid fingerprint (...)`, and the preceding info line helpfully prints the correct SHA256 fingerprint to migrate to. SHA256 fingerprints were always exact-match — only MD5 stragglers with shorthand configs are affected. Documented in README Security, and the 1.12 changelog now lists it as breaking (added in `c4038c3`). + +### c. `--auth` without a colon is now a fatal startup error — *branch-new, loud* + +On master, `--auth nocolon` silently degraded (server: user never registered → effectively no auth; client: empty credentials). That's a security footgun, and the branch turns it into `invalid auth string, expected :` at startup on both sides. Anyone hit by this was already running something other than what they believed. Documented in README, and the 1.12 changelog now lists it as breaking (added in `c4038c3`). + +### d. Exit code on `--max-retry-count` exhaustion: 0 → non-zero — *branch-new, silent for automation* + +`client_connect.go` now returns `connection attempts exhausted` on give-up (ctx-cancel/Ctrl-C still exits 0). Correct behavior, but it's the one change scripts and `Restart=on-failure` units experience with **no error message to notice** — the semantics of `$?` just flip. It *is* in the 1.12 changelog, which is the right mitigation. + +--- + +## 2. Quiet behavior changes (non-breaking, but observable) + +- **Reconnect pacing:** backoff floor moves 100ms → 1s (new `--min-retry-interval`). Softer thundering-herd on server restarts; individual reconnects marginally slower. +- **Dead connections actually die:** pings now time out (`CHISEL_PING_TIMEOUT`, default = keepalive interval, so ~50s at defaults vs 15–60 min of kernel retransmit limbo on master). Sleep/wake and NAT-timeout hangs become visible reconnects in logs. Old peers reply to pings, so mixed versions are fine. +- **Honest connect failures:** exit-side dial now happens *before* channel accept (`CHISEL_DIAL_TIMEOUT` 30s). Apps see "connection refused/timeout" instead of master's instant-success-then-EOF. Strictly better UX, but tools that measured "connect success" will notice. +- **Half-close propagation:** `shutdown(SHUT_WR)` traverses the tunnel (`share/cio/pipe.go`), fixing netcat-style pipelines and rsync. Falls back to full-close against old peers — no hangs, just old behavior. +- **SIGTERM is graceful:** first signal drains (HTTP drain `CHISEL_SHUTDOWN_GRACE` 5s, under docker's 10s default), second forces exit. Master died instantly on SIGTERM. +- **New info-level logs:** session `Open (user=… addr=… remotes=…)` / `Close (… duration=…)` and `Login failed for user X (ip)` — failed logins are finally fail2ban-able. Two side effects: log parsers keyed on the old debug `Closed connection` need updating, and tunnel endpoints now appear in default-level logs (mild privacy consideration for shipped logs). +- **Authfile reloads actually work:** the watcher survives vim renames, truncation, and k8s ConfigMap symlink swaps, with 100ms debounce; ACLs re-resolve per new channel; the `--auth` user is pinned across reloads and wins name clashes; removed users lose *new* tunnels but established ones aren't cut (documented). Operators who habitually restart after edits will find edits now apply live. +- **Unanchored ACL patterns warn** at every load, per pattern, unsuppressible. Common `.*`-style files keep working but get noisy — the warning is doing its job, since unanchored patterns really do over-match. +- **WS read cap 64KB pre-auth** (`CHISEL_WS_READ_LIMIT`, 0 disables) on both sides. Max legit SSH packet is ~35KB, so ~2× headroom; the only tail risk is a pathological config payload (thousands of remotes on one client), and the env knob is the escape hatch. +- **UDP at the flow cap:** master permanently blackholed flows past 100; branch sweeps idle over-cap flows (`CHISEL_UDP_DEADLINE` 15s), so DNS-heavy exit nodes recover instead of wedging. +- **Nicer failure edges:** partial `BindRemotes` failure now unbinds earlier listeners (no zombie ports); bad `--keyfile` errors no longer echo raw key material into logs; `go install` builds report real versions; `3000/UDP` uppercase now parses. + +--- + +## 3. Library consumers (Go API) + +No compile-breaking signature changes in `client`, `server`, or `share/...`. Additive: `client.Config.MinRetryInterval`, `Client.Ready(ctx)`, `Tunnel.Ready`, `UserIndex.PinUser`. Behavioral: `NewServer` returns errors where master called `log.Fatal` inside (a win for embedders), `Remote.UserAddr()` returns `"socks"` for forward-socks, `L4Proto` lowercases — only code depending on those exact outputs would notice. + +--- + +## 4. Mixed-version deployments + +Protocol string is unchanged (`chisel-v3`), and no handshake changes were found. + +- **v1.11.x client ↔ 1.12 server:** works. Socks ACL is server-side and already live since v1.11.7; WS cap and ping timeout are old-peer-safe. +- **1.12 client ↔ v1.11.x server:** works, degrading gracefully — no half-close benefit, no dial propagation, no server-side changes; `socks5://` proxy scheme is client-local. + +--- + +## 5. README review + +**Strengths:** breaking changes are stated *as* breaking, in the places users actually look (flag help, auth section, changelog); the env-var table surfaces a dozen previously undiscoverable knobs with sides and defaults (all verified against the code — `PING_TIMEOUT`, `WS_READ_LIMIT`, `UDP_*`, `SHUTDOWN_GRACE`, `DIAL_TIMEOUT`); the dead Heroku demo is replaced with a working fly.io recipe (linked `example/fly.toml` and `example/reverse-tunneling-authenticated.md` both exist); new TLS, CDN, and reverse-SOCKS-with-authfile guides fill real gaps; version-history typo (`akp`→`apk`) fixed. Several master help-text errors are corrected without behavior changes — verified that `remote-host` already defaulted to `127.0.0.1` in code (master's "0.0.0.0" doc was wrong) and `--backend`/`--proxy` were already aliases on master. + +**Gaps found at review time** (all four have since been fixed — see §6): + +1. The 1.12 changelog listed only the socks break; the MD5-fingerprint and auth-colon breaks were absent (only covered in prose sections). Changelogs are what people skim before upgrading. +2. Server-side ACL denials log at debug only — invisible to operators diagnosing post-upgrade socks failures without `-v`. +3. Client `--fingerprint` help said fingerprints "must be 44 characters containing a trailing equals" — contradicting the still-supported legacy MD5 colon form described in the Security section. +4. `PING_TIMEOUT` "default: keepalive interval" didn't state what happens with `--keepalive 0`. + +--- + +## 6. Recommendations — all implemented ✅ + +1. ✅ **Raise ACL denials to info level**: converts the one undocumented-feeling break into a self-diagnosing one. Implemented in `3c66f9b` (`tunnel_out_ssh.go` — denials now log `Denied connection to (ACL)` without `-v`). +2. ✅ **Add the two missing breaking bullets to the 1.12 changelog** (fingerprint exact-match, auth-colon fatal). Implemented in `c4038c3`. +3. ✅ **Reconcile the `--fingerprint` help text** with the legacy-MD5 reality — help now states legacy MD5 fingerprints are accepted only in full 16-octet form. Implemented in `c4038c3` (main.go + README help copy). +4. ✅ **"Upgrading to 1.12"** README subsection consolidating the four migration items (socks grant, full fingerprints, auth colon, exit codes), placed under the changelog. Implemented in `c4038c3`. The `PING_TIMEOUT` env-table row also now notes it is inert with `--keepalive 0` (verified: the keepalive loop is skipped entirely at `tunnel.go:93`). + +--- + +## 7. Dependency & supply-chain posture + +Since the original review the branch was rebased onto current `master` (`310eec3`) and its dependencies brought fully current. These carry **no user-facing behavior change** — they matter for the security and compatibility posture: + +- **Go modules:** `golang.org/x/crypto 0.54.0`, `golang.org/x/net 0.57.0`, `golang.org/x/sync 0.22.0`, `github.com/fsnotify/fsnotify 1.10.1` (indirect `x/sys 0.47.0`, `x/text 0.40.0`). `go build ./...`, `go vet ./...`, and `go mod verify` all pass. +- **CI actions:** `actions/checkout v7`, `docker/setup-qemu-action v4`, `docker/setup-buildx-action v4`, `docker/login-action v4`; `docker/build-push-action` was dropped by the goreleaser rework. +- **Dependabot coverage:** every open Dependabot PR (#597–#607) is now covered or superseded on the branch, and master's own crypto bump (#606) was absorbed by the rebase. Bringing master onto these versions should resolve the moderate Dependabot alert currently open on its default branch, if it stems from one of these modules. + +--- + +*Comparison basis: `ai-incoming@e0b639d` vs `master@310eec3` (released v1.11.7). Produced 2026-07-02, revised 2026-07-17.* diff --git a/Makefile b/Makefile index 7e0c7596..3f3d5b53 100644 --- a/Makefile +++ b/Makefile @@ -8,39 +8,37 @@ LDFLAGS=-ldflags "-s -w ${XBUILD} -buildid=${BUILD} -X github.com/jpillora/chise GOFILES=`go list ./...` GOFILESNOTEST=`go list ./... | grep -v test` -# Make Directory to store executables -$(shell mkdir -p ${DIR}) - all: @goreleaser build --skip-validate --single-target --config .github/goreleaser.yml -freebsd: lint +freebsd: lint ${DIR} env CGO_ENABLED=0 GOOS=freebsd GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-freebsd_amd64 . -linux: lint - env CGO_ENABLED=1 GOOS=linux GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-linux_amd64 . +linux: lint ${DIR} + env CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-linux_amd64 . -windows: lint - env CGO_ENABLED=1 GOOS=windows GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-windows_amd64 . +windows: lint ${DIR} + env CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-windows_amd64 . -darwin: +darwin: ${DIR} env CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build -trimpath ${LDFLAGS} ${GCFLAGS} ${ASMFLAGS} -o ${DIR}/chisel-darwin_amd64 . docker: @docker build . dep: ## Get the dependencies - @go get -u github.com/goreleaser/goreleaser - @go get -u github.com/boumenot/gocover-cobertura - @go get -v -d ./... - @go get -u all + @go install github.com/goreleaser/goreleaser/v2@latest + @go install github.com/boumenot/gocover-cobertura@latest @go mod tidy lint: ## Lint the files @go fmt ${GOFILES} @go vet ${GOFILESNOTEST} -test: ## Run unit tests +${DIR}: + mkdir -p ${DIR} + +test: ${DIR} ## Run unit tests @go test -coverprofile=${DIR}/coverage.out -race -short ${GOFILESNOTEST} @go tool cover -html=${DIR}/coverage.out -o ${DIR}/coverage.html @gocover-cobertura < ${DIR}/coverage.out > ${DIR}/coverage.xml diff --git a/README.md b/README.md index 867aff8d..8bc6f32c 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Chisel is a fast TCP/UDP tunnel, transported over HTTP, secured via SSH. Single - [Performant](./test/bench/perf.md)\* - [Encrypted connections](#security) using the SSH protocol (via `crypto/ssh`) - [Authenticated connections](#authentication); authenticated client connections with a users config file, authenticated server connections with fingerprint matching. -- Client auto-reconnects with [exponential backoff](https://github.com/jpillora/backoff) +- Client auto-reconnects with [exponential backoff](https://github.com/jpillora/backoff) (tunable via `--min/max-retry-interval`); keepalive pings time out, so silently dead connections (sleep/wake, NAT timeouts, server restarts) are detected and re-established - Clients can create multiple tunnel endpoints over one TCP connection - Clients can optionally pass through SOCKS or HTTP CONNECT proxies - Reverse port forwarding (Connections go through the server and out the client) @@ -39,14 +39,18 @@ Chisel is a fast TCP/UDP tunnel, transported over HTTP, secured via SSH. Single See [the latest release](https://github.com/jpillora/chisel/releases/latest) or download and install it now with `curl https://i.jpillora.com/chisel! | bash` +Binaries are built with the latest Go release, which sets the minimum OS versions: Windows 10 / Server 2016, macOS 12, Linux kernel 3.2, FreeBSD 12.2. For older systems (e.g. Windows 7), use [release v1.8.1](https://github.com/jpillora/chisel/releases/tag/v1.8.1) or earlier. + ### Docker -[![Docker Pulls](https://img.shields.io/docker/pulls/jpillora/chisel.svg)](https://hub.docker.com/r/jpillora/chisel/) [![Image Size](https://img.shields.io/docker/image-size/jpillora/chisel/latest)](https://microbadger.com/images/jpillora/chisel) +[![Docker Pulls](https://img.shields.io/docker/pulls/jpillora/chisel.svg)](https://hub.docker.com/r/jpillora/chisel/) [![Image Size](https://img.shields.io/docker/image-size/jpillora/chisel/latest)](https://hub.docker.com/r/jpillora/chisel/tags) ```sh docker run --rm -it jpillora/chisel --help ``` +Images are multi-arch and published to both Docker Hub (`jpillora/chisel`) and GitHub Container Registry (`ghcr.io/jpillora/chisel`). + ### Fedora The package is maintained by the Fedora community. If you encounter issues related to the usage of the RPM, please use this [issue tracker](https://bugzilla.redhat.com/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&classification=Fedora&component=chisel&list_id=11614537&product=Fedora&product=Fedora%20EPEL). @@ -63,22 +67,22 @@ $ go install github.com/jpillora/chisel@latest ## Demo -A [demo app](https://chisel-demo.herokuapp.com) on Heroku is running this `chisel server`: +You can run your own demo server in minutes (the old Heroku demo went away with Heroku's free tier). [`example/fly.toml`](example/fly.toml) deploys this `chisel server` to [fly.io](https://fly.io)'s free allowance: ```sh -$ chisel server --port $PORT --proxy http://example.com -# listens on $PORT, proxy web requests to http://example.com +$ chisel server --port $PORT --backend http://example.com +# listens on $PORT, proxies normal web requests to http://example.com ``` -This demo app is also running a [simple file server](https://www.npmjs.com/package/serve) on `:3000`, which is normally inaccessible due to Heroku's firewall. However, if we tunnel in with: +Deploy it with `fly launch --copy-config` from the `example/` directory, then tunnel to any service running beside the server, e.g.: ```sh -$ chisel client https://chisel-demo.herokuapp.com 3000 -# connects to chisel server at https://chisel-demo.herokuapp.com, +$ chisel client https://.fly.dev 3000 +# connects to your chisel server, # tunnels your localhost:3000 to the server's localhost:3000 ``` -and then visit [localhost:3000](http://localhost:3000/), we should see a directory listing. Also, if we visit the [demo app](https://chisel-demo.herokuapp.com) in the browser we should hit the server's default proxy and see a copy of [example.com](http://example.com). +Visiting your app's URL in a browser hits the server's default backend proxy and shows a copy of [example.com](http://example.com). ## Usage @@ -117,7 +121,7 @@ $ chisel server --help (defaults the environment variable HOST and falls back to 0.0.0.0). --port, -p, Defines the HTTP listening port (defaults to the environment - variable PORT and fallsback to port 8080). + variable PORT and falls back to port 8080). --key, (deprecated use --keygen and --keyfile instead) An optional string to seed the generation of a ECDSA public @@ -134,7 +138,8 @@ $ chisel server --help this flag is set, the --key option is ignored, and the provided private key is used to secure all communications. (defaults to the CHISEL_KEY_FILE environment variable). Since ECDSA keys are short, you may also set keyfile - to an inline base64 private key (e.g. chisel server --keygen - | base64). + to the inline key string itself, exactly as printed by --keygen (a base64 + string with a "ck-" prefix); no extra base64 encoding is needed. --authfile, An optional path to a users.json file. This file should be an object with users defined like: @@ -143,10 +148,20 @@ $ chisel server --help } when connects, their will be verified and then each of the remote addresses will be compared against the list - of address regular expressions for a match. Addresses will - always come in the form ":" for normal remotes - and "R::" for reverse port forwarding - remotes. This file will be automatically reloaded on change. + of address regular expressions for a match. Patterns are NOT + anchored by default: "10.0.0.1:80" also matches + "210.0.0.1:8080", and "." matches any character. Anchor your + patterns, e.g. "^10\.0\.0\.1:80$". The empty string "" + matches every address. Addresses will + always come in the form ":" for normal remotes, + "R::" for reverse port forwarding + remotes, and "socks" for SOCKS5 proxy access. Note that SOCKS5 + access previously bypassed this list; existing authfiles which + should allow SOCKS5 must add an entry matching "socks" (the + empty wildcard "" matches everything, including "socks"). This + file will be automatically reloaded on change. Reloads apply + to new connections and to new tunnels of connected clients; + established tunnels are not interrupted. --auth, An optional string representing a single user with full access, in the form of . It is equivalent to creating an @@ -161,7 +176,7 @@ $ chisel server --help --backend, Specifies another HTTP server to proxy requests to when chisel receives a normal HTTP request. Useful for hiding chisel in - plain sight. + plain sight. --proxy is accepted as an alias for this flag. --socks5, Allow clients to access the internal SOCKS5 proxy. See chisel client --help for more information. @@ -198,6 +213,8 @@ $ chisel server --help Signals: The chisel process is listening for: + a SIGINT or SIGTERM to begin a graceful shutdown + (a second signal forces an immediate exit), a SIGUSR2 to print process stats, and a SIGHUP to short-circuit the client reconnect timer @@ -227,7 +244,7 @@ $ chisel client --help ■ local-host defaults to 0.0.0.0 (all interfaces). ■ local-port defaults to remote-port. ■ remote-port is required*. - ■ remote-host defaults to 0.0.0.0 (server localhost). + ■ remote-host defaults to 127.0.0.1 (server localhost). ■ protocol defaults to tcp. which shares : from the server to the client @@ -256,7 +273,9 @@ $ chisel client --help specify "socks" in place of remote-host and remote-port. The default local host and port for a "socks" remote is 127.0.0.1:1080. Connections to this remote will terminate - at the server's internal SOCKS5 proxy. + at the server's internal SOCKS5 proxy. When the server also + has --authfile set, SOCKS5 access requires an entry matching + the token "socks" in the user's address list. When the chisel server has --reverse enabled, remotes can be prefixed with R to denote that they are reversed. That @@ -277,10 +296,13 @@ $ chisel client --help --fingerprint, A *strongly recommended* fingerprint string to perform host-key validation against the server's public key. - Fingerprint mismatches will close the connection. - Fingerprints are generated by hashing the ECDSA public key using - SHA256 and encoding the result in base64. - Fingerprints must be 44 characters containing a trailing equals (=). + Fingerprint mismatches will close the connection. + Fingerprints are generated by hashing the ECDSA public key using + SHA256 and encoding the result in base64. + Fingerprints must be 44 characters containing a trailing equals (=). + Legacy MD5 colon fingerprints (deprecated) are still accepted, + but only in their full 16-octet form; truncated prefixes are + rejected. --auth, An optional username and password (client authentication) in the form: ":". These credentials are compared to @@ -296,14 +318,20 @@ $ chisel client --help --max-retry-count, Maximum number of times to retry before exiting. Defaults to unlimited. + --min-retry-interval, Minimum wait time before retrying after a + disconnection. Defaults to 1 second. + --max-retry-interval, Maximum wait time before retrying after a disconnection. Defaults to 5 minutes. --proxy, An optional HTTP CONNECT or SOCKS5 proxy which will be used to reach the chisel server. Authentication can be specified - inside the URL. + inside the URL. Credentials must be URL-encoded; for example a + "#" in the password must be written as "%23". For example, http://admin:password@my-server.com:8081 or: socks://admin:password@my-server.com:1080 + The socks://, socks5:// and socks5h:// schemes are equivalent: + DNS is always resolved by the proxy. --header, Set a custom header in the form "HeaderName: HeaderContent". Can be used multiple times. (e.g --header "Foo: Bar" --header "Hello: World") @@ -341,6 +369,8 @@ $ chisel client --help Signals: The chisel process is listening for: + a SIGINT or SIGTERM to begin a graceful shutdown + (a second signal forces an immediate exit), a SIGUSR2 to print process stats, and a SIGHUP to short-circuit the client reconnect timer @@ -355,13 +385,41 @@ $ chisel client --help ### Security -Encryption is always enabled. When you start up a chisel server, it will generate an in-memory ECDSA public/private key pair. The public key fingerprint (base64 encoded SHA256) will be displayed as the server starts. Instead of generating a random key, the server may optionally specify a key file, using the `--keyfile` option. When clients connect, they will also display the server's public key fingerprint. The client can force a particular fingerprint using the `--fingerprint` option. See the `--help` above for more information. +Encryption is always enabled. When you start up a chisel server, it will generate an in-memory ECDSA public/private key pair. The public key fingerprint (base64 encoded SHA256) will be displayed as the server starts. Instead of generating a random key, the server may optionally specify a key file, using the `--keyfile` option. When clients connect, they will also display the server's public key fingerprint. The client can force a particular fingerprint using the `--fingerprint` option. Legacy MD5 fingerprints are still accepted but must be the full 16-octet colon form — truncated prefixes are rejected. See the `--help` above for more information. + +The server also caps inbound websocket message sizes before authentication (`CHISEL_WS_READ_LIMIT`, default 64KB), so unauthenticated peers cannot exhaust memory with oversized frames. ### Authentication Using the `--authfile` option, the server may optionally provide a `user.json` configuration file to create a list of accepted users. The client then authenticates using the `--auth` option. See [users.json](example/users.json) for an example authentication configuration file. See the `--help` above for more information. -Internally, this is done using the _Password_ authentication method provided by SSH. Learn more about `crypto/ssh` here http://blog.gopheracademy.com/go-and-ssh/. +Notes on authfile behavior: + +- The file is watched and **reloaded live** — including editor saves via rename (vim) and kubernetes configmap updates. Reloads apply to new connections and to new tunnels of already-connected clients; removed users lose access to new tunnels immediately, though established tunnels are not interrupted. +- Address patterns are regular expressions and are **not anchored** — anchor them with `^` and `$` (the server warns about unanchored patterns at load). The empty string `""` matches everything. +- SOCKS5 access is controlled by an entry matching the token `socks`. **Breaking**: SOCKS5 previously bypassed the authfile entirely; servers running `--socks5` with `--authfile` must grant `socks` to users who should keep proxy access (wildcard `""` entries keep working). +- Auth strings without a colon (`user:pass`) are now a **fatal startup error** on both server and client — previously they silently disabled authentication. +- The `--auth` user survives authfile reloads and wins name clashes with file users. + +Internally, this is done using the _Password_ authentication method provided by SSH. Learn more about `crypto/ssh` here http://blog.gopheracademy.com/go-and-ssh/. Session opens/closes (with user, source address and remotes) and failed login attempts are logged at info level. + +### TLS Guide + +The simplest secure setup is `--tls-domain`, which provisions a LetsEncrypt certificate automatically (requires port 443 and a DNS record pointing at the server): + +```sh +chisel server --port 443 --tls-domain chisel.example.com --auth user:pass +chisel client --auth user:pass https://chisel.example.com R:2222:localhost:22 +``` + +To use your own certificate (self-signed or internal CA), generate a key/cert pair and point both sides at the right files: + +```sh +chisel server --port 443 --tls-key key.pem --tls-cert cert.pem +chisel client --tls-ca ca.pem https://chisel.example.com 3000 +``` + +For mutual TLS, also pass `--tls-ca` to the server and `--tls-cert`/`--tls-key` to each client. Note that TLS wraps chisel's transport from the outside; the inner SSH layer still encrypts and authenticates, so `--fingerprint` validation works with or without TLS. ### SOCKS5 Guide with Docker @@ -392,6 +450,51 @@ Internally, this is done using the _Password_ authentication method provided by 1. Now you have an encrypted, authenticated SOCKS5 connection over HTTP +Note: if the server also uses `--authfile`, users need an entry matching the token `socks` to use the proxy (see [Authentication](#authentication)). + +### Reverse SOCKS with an Authfile + +To let a specific client act as a SOCKS exit node, grant it the reverse-socks listener address (`R:socks` listens on the server's `127.0.0.1:1080`): + +```json +{ + "exituser:password": ["^R:127\\.0\\.0\\.1:1080$"] +} +``` + +```sh +chisel server --reverse --authfile users.json +chisel client --auth exituser:password R:socks +# server-side consumers point SOCKS5 clients at 127.0.0.1:1080, +# and their traffic exits via the chisel client's network +``` + +See also the step-by-step [reverse tunneling example](example/reverse-tunneling-authenticated.md). + +### Running behind a CDN (Cloudflare) + +chisel works through CDNs that support WebSockets. For Cloudflare: enable WebSockets, proxy (orange-cloud) the DNS record, and connect clients with `https://`. The CDN terminates TLS, but the inner SSH layer means `--fingerprint` validation still authenticates your chisel server end-to-end — the CDN cannot read or modify tunneled traffic. Keep `--keepalive` at its `25s` default to stay under CDN idle timeouts, and note that proxies which strip `Upgrade` headers cannot carry chisel at all. + +### Tuning with environment variables + +Less common knobs are environment variables, all read with a `CHISEL_` prefix (e.g. `CHISEL_WS_TIMEOUT=10s`): + +| Variable | Side | Default | Purpose | +| --------------- | ----------- | ------------------ | -------------------------------------------------- | +| `WS_TIMEOUT` | client | `45s` | websocket handshake timeout | +| `SSH_TIMEOUT` | client | `30s` | ssh handshake timeout | +| `CONFIG_TIMEOUT`| server | `10s` | wait for the client's config request | +| `SSH_WAIT` | both | `35s` | how long new tunnels wait for an active connection | +| `PING_TIMEOUT` | both | keepalive interval | keepalive ping reply timeout (no pings if `--keepalive 0`) | +| `DIAL_TIMEOUT` | exit node | `30s` | tcp dial timeout for tunnel targets | +| `WS_READ_LIMIT` | both | `65536` | max inbound websocket message bytes (0 = no limit) | +| `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 | +| `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. #### Caveats @@ -401,7 +504,7 @@ Since WebSockets support is required: - PaaS providers vary in their support for WebSockets - Heroku has full support - Openshift has full support though connections are only accepted on ports 8443 and 8080 - - Google App Engine has **no** support (Track this on [their repo](https://code.google.com/p/googleappengine/issues/detail?id=2535)) + - Google App Engine standard has **no** support (the flexible environment does) ## Contributing @@ -423,8 +526,31 @@ Since WebSockets support is required: - `1.7` - Added UDP support - `1.8` - Move to a `scratch`Docker image - `1.9` - Bump to Go 1.21. Switch from `--key` seed to P256 key strings with `--key{gen,file}` (by @cmenginnz) -- `1.10` - Bump to Go 1.22. Add `.rpm` `.deb` and `.akp` to releases. Fix bad version comparison. +- `1.10` - Bump to Go 1.22. Add `.rpm` `.deb` and `.apk` to releases. Fix bad version comparison. - `1.11` - Bump to Go 1.25.1. Update all dependencies. +- `1.12` - (unreleased) Reliability and security pass: + - keepalive pings now time out (`CHISEL_PING_TIMEOUT`), so dead connections reconnect promptly after sleep/wake, NAT timeouts and server restarts + - authfile reloads survive editor renames and kubernetes configmap swaps, and apply live to connected clients (new tunnels; established tunnels are not interrupted) + - **breaking**: with `--socks5` + `--authfile`, SOCKS5 access now requires an authfile entry matching `socks` (wildcard `""` entries keep working) + - **breaking**: truncated legacy MD5 fingerprints are rejected — `--fingerprint` must be the full SHA256 form (or the full 16-octet MD5 colon form) + - **breaking**: auth strings without a colon (e.g. `--auth user`) are a fatal startup error instead of silently disabling authentication + - TCP half-close is propagated through tunnels, and unreachable targets reject the tunnel instead of presenting a dead connection (`CHISEL_DIAL_TIMEOUT`, default 30s) + - graceful shutdown on SIGTERM with HTTP request draining (`CHISEL_SHUTDOWN_GRACE`); a second signal force-exits + - UDP exit nodes no longer break or leak past 100 concurrent flows (`CHISEL_UDP_MAX_CONNS`) + - inbound websocket messages are size-capped pre-auth (`CHISEL_WS_READ_LIMIT`) + - server no longer panics when a client disconnects between the SSH handshake and its config request (#608) + - client exits non-zero when `--max-retry-count` is exhausted; new `--min-retry-interval` (default 1s); `socks5://` accepted for `--proxy` + - `go install` builds report their real version; sessions and failed logins are logged at info level + - releases now ship goreleaser-built multi-arch Docker images to GHCR and Docker Hub with correctly stamped versions; releasing is two-stage — tagging builds a draft GitHub release plus version-tagged images, and publishing the draft promotes the Docker `latest` / `X` / `X.Y` tags + +### Upgrading to 1.12 + +Four changes may require action when upgrading from 1.11.x or earlier: + +1. **SOCKS5 + `--authfile`** (enforced since v1.11.7): users who should keep proxy access need an authfile entry matching the token `socks` (the wildcard `""` keeps working). See [Authentication](#authentication). Denied requests are logged server-side as `Denied connection to socks (ACL)`. +2. **`--fingerprint`**: truncated legacy MD5 fingerprints are rejected. Use the full SHA256 fingerprint printed by the server and client (the full 16-octet MD5 colon form is still accepted, but deprecated). +3. **`--auth`** values must be `:` — strings without a colon now fail at startup instead of silently disabling authentication. +4. **Exit codes**: `chisel client` with `--max-retry-count` now exits non-zero when connection attempts are exhausted; scripts checking `$?` and systemd `Restart=on-failure` units will notice. ## License diff --git a/TASKS.md b/TASKS.md new file mode 100644 index 00000000..4ffb0720 --- /dev/null +++ b/TASKS.md @@ -0,0 +1,684 @@ +# TASKS + +a [meads](https://github.com/jpillora/meads) (`md`) managed task log + +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T11:52:45Z + +## 1. Keepalive ping has no timeout - dead connections are never detected + +* status: closed +* priority: P1 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T10:22:12Z + +### Problem + +`share/tunnel/tunnel.go:178` — keepAliveLoop calls `sshConn.SendRequest("ping", true, nil)` which blocks until the peer replies. On a dead TCP path (OS sleep/wake, NAT timeout, server hard reboot) no RST arrives, so the request blocks for the kernel retransmit timeout (15+ min). The tunnel looks connected but every OpenChannel hangs; the client never reconnects. + +This is the most-reported problem in the tracker. + +### Fix + +- Race SendRequest against a timer (keepalive interval or a `CHISEL_PING_TIMEOUT` env), close the ssh conn on timeout so the reconnect loop kicks in +- Replace the `time.Sleep` loop with a stoppable ticker +- Review/merge PR #581 (current attempt, has tests) + +### Refs + +- [#445](https://github.com/jpillora/chisel/issues/445) — Keepalive timers for disconnected chisel client +- [#560](https://github.com/jpillora/chisel/issues/560) — Client keeps disconnecting every 3-5 minutes +- [#579](https://github.com/jpillora/chisel/issues/579) — Cant reconnect after server reboot +- [PR #581](https://github.com/jpillora/chisel/pull/581) — add timeout to keepAliveLoop (preferred fix) +- [PR #583](https://github.com/jpillora/chisel/pull/583), [PR #488](https://github.com/jpillora/chisel/pull/488), [PR #442](https://github.com/jpillora/chisel/pull/442), [PR #481](https://github.com/jpillora/chisel/pull/481) — earlier attempts + +## 2. Authfile fsnotify watcher misses rename/truncate updates; no debounce + +* status: closed +* priority: P1 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T10:26:30Z + +### Problem + +`share/settings/users.go:100-121` — addWatchEvents watches the file path and only reacts to `fsnotify.Write`. Editors that write via tmp+rename (vim), truncate-then-write scripts, and Kubernetes configmap symlink swaps either kill the watch (inode replaced) or emit Create/Rename events that are ignored. A truncate+write can also race the reader into loading a half-written file: reload fails and stale users stay active until restart. + +### Fix + +- Watch the parent directory filtered by filename +- Re-add the watch after Remove/Rename events +- Debounce (~100ms) and only swap users when the JSON parses +- Review/merge PR #587 (adds debounce + path normalization) + +### Refs + +- [#493](https://github.com/jpillora/chisel/issues/493) — Intermittent authentication failure after updating users.json (k8s, restart required) +- [#485](https://github.com/jpillora/chisel/issues/485) — Version mismatch when using auth file (related reports) +- [PR #587](https://github.com/jpillora/chisel/pull/587) — debounce and improve config file watcher + +## 3. UDP exit node: flows beyond 100 are permanently broken and leak + +* status: closed +* priority: P1 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T10:37:22Z + +### Problem + +`share/tunnel/tunnel_out_ssh_udp.go:65-72` — handleWrite: when `udpConns.len() > maxConns` (100), the new conn is still dialed and stored in the map but no handleRead goroutine is spawned. Consequences: + +1. Responses for that flow are never read — silent blackhole +2. The entry is never removed (removal only happens in the handleRead defer), so the map never drops below the cap and **all** subsequent new flows stay broken until the SSH channel closes +3. Memory grows unbounded + +Affects busy DNS/QUIC tunnels. + +### Fix + +Review/merge PR #515 (fixes removal, makes the cap configurable). Longer term, implement the TODO at line 62: replace goroutine-per-flow with a periodic idle sweep. + +### Refs + +- [PR #515](https://github.com/jpillora/chisel/pull/515) — fix the udpConns map does not release new conns when its length is over 100 +- [#406](https://github.com/jpillora/chisel/issues/406) — Improve udp tunnel stability +- [#456](https://github.com/jpillora/chisel/issues/456) — Exposing a Minecraft Bedrock Server (UDP) + +## 4. Security: SOCKS channels bypass per-user ACL + +* status: closed +* priority: P1 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T10:31:28Z + +### Problem + +`share/tunnel/tunnel_out_ssh.go:50` skips the ACL for socks channels: + +```go +if t.Config.ACL != nil && !socks && !t.Config.ACL(hostPort) { +``` + +With `--socks5` and `--authfile` both enabled, a modified client can open a `socks` channel directly — without declaring a socks remote in its config — and get unrestricted egress through the SOCKS server, regardless of its authfile address list. The config-time check (`server/server_handler.go:113-122`) only validates remotes the client chooses to declare; channels are not required to match declared remotes. + +Note: `settings.Remote.UserAddr()` (`share/settings/remote.go:220-225`) returns `:` for socks remotes, so authfiles cannot even express "allow socks" cleanly — users allow it by accident via unanchored regexes. + +Channel ACLs were added in commit 44310b6 but left this hole. + +### Fix + +- Subject socks channels to the ACL with a well-known token (e.g. require `HasAccess("socks")`) +- Make `UserAddr()` return `socks` for socks remotes +- Document the migration for existing authfiles + +### Refs + +- [#563](https://github.com/jpillora/chisel/issues/563) — Are Unauthorized clients allowed to connect and use Chisel server? +- [#518](https://github.com/jpillora/chisel/issues/518) — How could configure authfile when using remote socks + +## 5. BindRemotes leaks bound listeners on partial failure + +* status: closed +* priority: P2 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T11:04:40Z + +### Problem + +`share/tunnel/tunnel.go:148-163` — NewProxy binds each remote eagerly (tcp/udp listen in `tunnel_in_proxy.go:45-70`). If remote N fails to bind after remotes 1..N-1 succeeded, BindRemotes returns the error without closing the earlier proxies, so their sockets stay bound for the process lifetime. On the server (reverse mode) a failed client config can orphan ports forever. + +The `CanListen()` precheck in server_handler.go shrinks the window but is TOCTOU (another bind can win between precheck and the real bind) and does not cover resolve errors. + +### Fix + +Add `Proxy.Close()` and close `proxies[0..i-1]` on error before returning. + +### Refs + +- [#492](https://github.com/jpillora/chisel/issues/492) — multiple sockets bind to the same address (related) + +## 6. Server auth: session map leak, panic race, literal %s error, timing-unsafe compare + +* status: closed +* priority: P2 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T11:07:30Z + +### Problem + +All in the server auth path (`server/server.go` authUser + `server/server_handler.go` handleWebsocket): + +**Session map leaks** — authUser stores the user in `s.sessions` keyed by SSH SessionID (`server.go:199-215`); `server_handler.go:67-77` deletes it only after a successful NewServerConn: + +- Client drops after PasswordCallback but before the handshake completes — entry never deleted +- Config request timeout path (`server_handler.go:83-89`) returns without Del + +Unbounded (if slow) growth on a public server. + +**Panic race** — if `users.Len()==0` during auth (allow-all, nothing stored) and an authfile reload makes `Len()>0` before the check at `server_handler.go:69-74`, the handler panics ("bug in ssh auth handler"). net/http recovers it, but the connection dies with a stack trace in the log. + +**authUser nits** (`server.go:207-209`): + +- `errors.New("Invalid authentication for username: %s")` — the `%s` is literal, never formatted +- `user.Pass != string(password)` is not constant time — a (mild) timing oracle on a public endpoint + +### Fix + +- Delete the session entry via defer in handleWebsocket; replace the panic with a logged close +- Better: return the user via `ssh.Permissions.Extensions` from PasswordCallback instead of a side map — removes the map, the race, and the TODO at `server.go:212` entirely +- Fix the error string; use `crypto/subtle.ConstantTimeCompare`; add tests + +## 8. Auth user store: --auth validation foot-guns + reload semantics (#549) + +* status: closed +* priority: P2 +* type: bug +* created: 2026-06-10T10:22:47Z +* updated: 2026-06-12T11:17:02Z + +### Problem + +Three related defects in how the user store is built and reloaded: + +**(a) Silently disabled** — `settings.ParseAuth` (`share/settings/user.go:10-16`) returns empty strings when the value has no colon. `server/server.go:71-77` only adds the user when `Name != ""`, so `chisel server --auth secretword` starts with **no authentication** and no warning. The client likewise silently sends empty creds. + +**(b) --auth user silently dropped** — when `--auth` is combined with `--authfile`, the --auth user is added to the same UserIndex that authfile reloads `Reset()` (`share/settings/users.go:61-69` and `:157`), so the first file reload deletes the --auth user and that client can no longer connect. + +**(c) Reload does not affect connected clients** — the per-connection ACL closure captures the `*settings.User` object at handshake time (`server/server_handler.go:146-148`: `tunnelConfig.ACL = user.HasAccess`). After a reload, removed users keep tunneling until they disconnect, and updated Addrs lists are not applied to existing connections. + +### Fix + +- Make auth strings without a colon a fatal config error on both sides +- Keep the --auth user separate from file-sourced users, or re-add it after each reload +- Resolve the user by name from `s.users` at channel-open time so current Addrs always apply; optionally track active `ssh.Conn` per user and `Close()` the ones removed/changed on reload (#549 explicitly expects disconnection). At minimum document the behavior + +Pairs with the external-auth (`--authurl`) idea (task 28). + +### Refs + +- [#549](https://github.com/jpillora/chisel/issues/549) — clients are not disconnected when auth file is replaced + +## 10. Client connect loop: exit 0 on give-up, 100ms backoff Min, robustness nits + +* status: closed +* priority: P2 +* type: bug +* created: 2026-06-10T10:23:15Z +* updated: 2026-06-12T11:11:58Z + +### Problem + +All in `client/client_connect.go` connectionLoop: + +**Exit code** (`:49-52`) — after exhausting `--max-retry-count` the loop breaks, calls `c.Close()` and returns nil, so the process exits 0. Scripts and systemd units cannot distinguish "tunnel worked and was interrupted" from "never connected". + +**Backoff Min** (`:22`) — `backoff.Backoff{Max: MaxRetryInterval}` leaves Min at the library default of **100ms**, so a down server gets hit at 100ms/200ms/400ms... by every client — a reconnect stampede when a popular server restarts. Related wart: `client/client.go:78` silently raises any `MaxRetryInterval < 1s` to 5 minutes. + +**Robustness nits**: + +- `:33` calls `err.Error()` before the nil check at `:37` — connectionOnce currently never returns `(true, nil)` because `ssh.Conn.Wait()` returns io.EOF on clean close, but any future change makes this a nil-pointer panic +- `:46` — `c.Infof(msg)` passes pre-built text as the format string; any literal `%` garbles output + +### Fix + +- Return an error when attempts are exhausted so `main.go` log.Fatal exits non-zero; keep exit 0 for ctx-cancelled shutdown +- Set a saner backoff Min (e.g. 1s) and/or add `--min-retry-interval`; warn or document the 5-minute floor +- Guard `err != nil` first; use `c.Infof("%s", msg)` (grep for the pattern elsewhere) + +### Refs + +- [PR #537](https://github.com/jpillora/chisel/pull/537) — Fix client backoff +- [#579](https://github.com/jpillora/chisel/issues/579) — Cant reconnect after server reboot (likely related) + +## 12. Uppercase /UDP remote suffix parses but later fails as unknown proto + +* status: closed +* priority: P3 +* type: bug +* created: 2026-06-10T10:23:15Z +* updated: 2026-06-12T11:23:56Z + +### Problem + +`share/settings/remote.go:154-163` — the l4Proto regex is case-insensitive (`(?i)/(tcp|udp)$`) but the returned proto keeps the original case, while every comparison expects lowercase (`remote.go:114-127`, `tunnel_in_proxy.go:48-67`, `tunnel_out_ssh.go:42`). So `chisel client SERVER 1.1.1.1:53/UDP` decodes successfully, then dies at listen time with "unknown local proto". The head is lowercased but the proto is not — inconsistent. + +### Fix + +`strings.ToLower` the proto in L4Proto; add a remote_test case. + +## 14. NewServer kills the host process with log.Fatal + +* status: closed +* priority: P3 +* type: bug +* created: 2026-06-10T10:23:15Z +* updated: 2026-06-12T11:25:35Z + +### Problem + +`server/server.go:79-111` — the key-loading paths call `log.Fatalf`/`log.Fatal` (failed to read key file, invalid key, failed to generate key, failed to parse key) inside NewServer, which otherwise returns `(*Server, error)`. Chisel is also consumed as a library, and log.Fatal kills the embedding process and skips deferred cleanup. + +### Fix + +Return errors; `main.go` already log.Fatals on the returned error. + +### Refs + +- [#542](https://github.com/jpillora/chisel/issues/542) — Usage in go code? +- [#497](https://github.com/jpillora/chisel/issues/497) — there is no stop function? + +## 17. Set websocket read limits (pre-auth memory DoS hardening) + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T10:59:53Z + +### Problem + +`share/cnet/conn_ws.go:25-53` reads entire websocket messages into memory (`ReadMessage`) and buffers the remainder, with no size cap; gorilla/websocket defaults to unlimited message size. On the server this happens **before SSH authentication**, so an unauthenticated peer can send a multi-GB frame and OOM the process. + +### Fix + +SSH packets are <= ~35KB, so call `SetReadLimit` (e.g. 64KB) on both server (`server/server_handler.go` after Upgrade) and client (`client/client_connect.go` after Dial), or switch wsConn.Read to NextReader-based streaming. The `WS_BUFF_SIZE` env already tunes buffer sizes; a read limit closes the abuse case. + +## 18. Graceful shutdown: handle SIGTERM and use http.Server.Shutdown + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T10:56:34Z + +### Problem + +- `share/cos/common.go:12-22` InterruptContext only registers `os.Interrupt`; SIGTERM (docker stop, kubernetes) takes the default kill path so context-based cleanup never runs (the code comment even wonders "windows compatible?") +- `share/cnet/http_server.go` is documented as "adds graceful shutdowns" but GoServe calls `Close()` (immediate) when the ctx ends; the `listenErr` field is dead code + +### Fix + +Add `syscall.SIGTERM` (unix) and make a second signal force-exit. Use `http.Server.Shutdown` with a grace period, then Close. + +### Refs + +- [PR #564](https://github.com/jpillora/chisel/pull/564) — Reuse existing function to implement graceful shutdown logic (review alongside) + +## 19. Half-close support + dial-failure propagation through tunnels + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T10:52:18Z + +### Problem + +`cio.Pipe` (`share/cio/pipe.go:9-30`) closes **both** directions as soon as either io.Copy finishes, so TCP half-close semantics (send, shutdown(WR), read reply) break across the tunnel, and a FIN from one end becomes a full teardown. + +Related: `share/tunnel/tunnel_out_ssh.go:87-95` accepts the SSH channel **before** dialing the target, so inbound TCP clients see a successful connection even when the target is down/refusing. Also `net.Dial` there has no timeout/context (hangs the channel for the OS dial timeout). + +### Fix + +Design pass: + +- CloseWrite-aware Pipe (`ssh.Channel` and `*net.TCPConn` both have CloseWrite) +- Reject channel on dial failure (or dial-before-accept) +- Context-aware dial with timeout +- e2e coverage in test/e2e + +### Refs + +- [#535](https://github.com/jpillora/chisel/issues/535) — Non graceful closing of remote connection +- [#447](https://github.com/jpillora/chisel/issues/447) — when channel is closed, outbound always by use +- [PR #536](https://github.com/jpillora/chisel/pull/536) — Half closing of connections when CloseWrite() is available +- [PR #548](https://github.com/jpillora/chisel/pull/548) — Add CloseWrite() to rwcConn (SOCKS path) +- [PR #538](https://github.com/jpillora/chisel/pull/538) — TCP reset on remote connection failure + +## 20. CI/release hygiene: scope permissions, fix docker version stamping + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T10:45:45Z + +### Problem + +- `.github/workflows/ci.yml` sets `permissions: write-all` for all jobs on every push AND pull_request (also causes duplicate runs on PR branches) +- `.github/Dockerfile` stamps the version via `git describe --abbrev=0 --tags` during docker build; the release_docker checkout has no tags fetched (no `fetch-depth: 0`), so docker images get wrong/empty versions + +### Fix + +Scope to least privilege (`contents: read` for test, `contents: write` only on release jobs). Review/merge PR #584: goreleaser-built docker images (multi-arch, GHCR + Docker Hub), single release job, scoped permissions. + +### Refs + +- [#417](https://github.com/jpillora/chisel/issues/417) — Docker/github version string mismatch +- [PR #584](https://github.com/jpillora/chisel/pull/584) — Improve Docker: use GoReleaser for images + +## 21. Tracker triage: close stale dependabot PRs and resolved issues + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T10:40:41Z + +### Stale dependabot PRs + +Superseded by the dependency updates in [PR #568](https://github.com/jpillora/chisel/pull/568) (2025-09) and [PR #578](https://github.com/jpillora/chisel/pull/578) (2026-02) — close: +[#448](https://github.com/jpillora/chisel/pull/448), [#449](https://github.com/jpillora/chisel/pull/449), [#450](https://github.com/jpillora/chisel/pull/450), [#451](https://github.com/jpillora/chisel/pull/451), [#467](https://github.com/jpillora/chisel/pull/467), [#470](https://github.com/jpillora/chisel/pull/470), [#478](https://github.com/jpillora/chisel/pull/478), [#496](https://github.com/jpillora/chisel/pull/496), [#513](https://github.com/jpillora/chisel/pull/513), [#516](https://github.com/jpillora/chisel/pull/516), [#517](https://github.com/jpillora/chisel/pull/517). + +The dependabot config evidently rots — consider grouped monthly updates or renovate ([#559](https://github.com/jpillora/chisel/issues/559); [#452](https://github.com/jpillora/chisel/issues/452) reported confusing "fake pushes"). + +### Issues closeable now + +- [#561](https://github.com/jpillora/chisel/issues/561) — Heroku demo dead (close via docs task 23) +- [#585](https://github.com/jpillora/chisel/issues/585) — fixed by [PR #586](https://github.com/jpillora/chisel/pull/586) +- [#445](https://github.com/jpillora/chisel/issues/445), [#560](https://github.com/jpillora/chisel/issues/560) — fold into keepalive fix (task 1) +- [#541](https://github.com/jpillora/chisel/issues/541) — unsubstantiated buffer-overflow claim; no evidence provided; close with explanation +- [#504](https://github.com/jpillora/chisel/issues/504), [#550](https://github.com/jpillora/chisel/issues/550), [#485](https://github.com/jpillora/chisel/issues/485) — close via version fallback (task 22) +- [#498](https://github.com/jpillora/chisel/issues/498) — close via docs task 23 + +### Duplicate PRs + +Keepalive fixes [PR #581](https://github.com/jpillora/chisel/pull/581) / [PR #488](https://github.com/jpillora/chisel/pull/488) / [PR #442](https://github.com/jpillora/chisel/pull/442) — pick #581, close the rest with thanks. + +## 22. Version: fall back to debug.ReadBuildInfo when ldflags absent + +* status: closed +* priority: P2 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T11:02:07Z + +### Problem + +`go install github.com/jpillora/chisel@latest` produces BuildVersion `0.0.0-src`, which then logs "Client version (0.0.0-src) differs from server version (...)" on every connect — recurring user confusion. + +### Fix + +In `share/version.go`, when BuildVersion is the default, fall back to `runtime/debug.ReadBuildInfo().Main.Version` (gives v1.x.y for module installs). Skip the mismatch warning in `server/server_handler.go:104-111` when either side reports a dev/unknown version. + +### Refs + +- [#504](https://github.com/jpillora/chisel/issues/504) — Client version (0.0.0-src) differs from server version (v1.9.1) +- [#550](https://github.com/jpillora/chisel/issues/550) — Client version difference on Kali Linux +- [#485](https://github.com/jpillora/chisel/issues/485) — Version mismatch when using auth file + +## 23. Docs refresh: CLI help + README (wrong defaults, dead demo, install cmd, proxy creds) + +* status: closed +* priority: P3 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T11:30:50Z +* Demo section: chisel-demo.herokuapp.com is dead (Heroku free tier removed; issue #561). Replace with a fly.io demo (example/fly.toml already exists) or drop the section. + +### CLI help text (main.go, rendered into README via md-tmpl) + +- `main.go:317` claims "remote-host defaults to 0.0.0.0 (server localhost)" — the code defaults RemoteHost to `127.0.0.1` (`share/settings/remote.go:110-112`), which is what "server localhost" actually means. Fix and re-render +- `--keyfile` help is misleading: the "inline base64" example (`chisel server --keygen - | base64`) is wrong because keygen already outputs a `ck-...` base64 string; no extra base64 step is needed — [#498](https://github.com/jpillora/chisel/issues/498), [PR #461](https://github.com/jpillora/chisel/pull/461) +- Sweep for accuracy: "fallsback" (`main.go:105`), ragged tab indentation in the --fingerprint paragraph + +### README + +- Demo section: chisel-demo.herokuapp.com is dead (Heroku free tier removed). Replace with a fly.io demo (`example/fly.toml` exists) or drop the section — [#561](https://github.com/jpillora/chisel/issues/561) +- Verify install one-liner `curl https://i.jpillora.com/chisel! | bash` — [PR #562](https://github.com/jpillora/chisel/pull/562) claims broken +- Demo text says `--proxy` where the flag docs say `--backend`; `main.go:189-190` registers both names for the same field — document the alias — [PR #556](https://github.com/jpillora/chisel/pull/556) +- Document that `--proxy` credentials must be URL-encoded (a `#` in the password truncates the URL) — [#396](https://github.com/jpillora/chisel/issues/396) +- Dead links/badges: microbadger badge, Google App Engine tracker (code.google.com) +- Requested examples: TLS setup walkthrough ([#533](https://github.com/jpillora/chisel/issues/533)), reverse socks + authfile ([#518](https://github.com/jpillora/chisel/issues/518)), cloudflare/CDN fronting notes ([#490](https://github.com/jpillora/chisel/issues/490)) +- Document the AUTH env var for both sides and the CHISEL_* env knobs (WS_TIMEOUT, SSH_TIMEOUT, UDP_MAX_SIZE, UDP_DEADLINE, CONFIG_TIMEOUT, SSH_WAIT) — currently undocumented + +## 24. Accept socks5:// scheme in client --proxy + +* status: closed +* priority: P3 +* type: task +* created: 2026-06-10T10:23:53Z +* updated: 2026-06-12T11:33:12Z + +### Problem + +`client/client.go:265-294` setProxy accepts only `socks://` and `socks5h://` and rejects `socks5://` — the most common spelling. + +### Fix + +Add `socks5` to the allowed schemes (same SOCKS5 dialer). Note in help/docs: all variants resolve DNS via the proxy (`golang.org/x/net/proxy.SOCKS5`), so socks5 vs socks5h semantics are identical here. One-liner plus a test and help-text update. + +### Refs + +- [#474](https://github.com/jpillora/chisel/issues/474) — Support socks5:// protocol as client proxy protocol + +## 25. Trivial sweep: typos, tiny community PRs, server_listen.go dead code + +* status: closed +* priority: P3 +* type: task +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:36:27Z + +### server_listen.go dead code + +- `server/server_listen.go:41-43`: the "LetsEncrypt will attempt to connect to your domain on port 443" warning is computed inside the hasKeyCert branch but guarded by hasDomains — impossible, since `hasDomains && hasKeyCert` already returned an error at line 27. Move the warning into the hasDomains path (port != 443) where it was clearly intended +- Line 56 `if err == nil` is always true (err checked at line 47) — [PR #546](https://github.com/jpillora/chisel/pull/546) removes it; merge/credit + +### Pending trivial PRs to merge or replicate + +- [PR #588](https://github.com/jpillora/chisel/pull/588) / [PR #575](https://github.com/jpillora/chisel/pull/575) — "forwaring" -> "forwarding" (`server/server_handler.go:126`, user-facing error message) +- [PR #528](https://github.com/jpillora/chisel/pull/528) — typo fix +- [PR #430](https://github.com/jpillora/chisel/pull/430) — add codespell to CI (prevents recurrence) + +### Typos found in review + +- "respresent" — `server/server.go:37` +- "recieved" — `share/cos/signal.go:27` (user-facing log line) +- "aquired" — `share/tunnel/tunnel_in_proxy_udp.go:181` +- "extacts" — `share/settings/remote.go:156` +- "faily" — `share/settings/remote.go:122` +- "successfuly" — `server/server_handler.go:135` +- "fallsback" — `main.go:105` (help text; overlaps docs task 23) + +## 26. Auth/fingerprint hardening: legacy prefix match, unanchored regexes + +* status: closed +* priority: P3 +* type: task +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:39:13Z + +### Items + +**(a) Legacy MD5 fingerprint prefix match:** `client/client.go:233` uses `strings.HasPrefix(got, expect)` — a truncated legacy fingerprint like `a5:32` still "verifies" (matches 1 in 65k keys). Require the full 16-octet colon form before accepting. + +**(b) Unanchored authfile regexes:** `share/settings/user.go:24-33` uses MatchString — an entry `10.0.0.1:80` also authorizes `210.0.0.1:8080` and `10.0.0.1:8000`; dots match any char. Anchoring by default is breaking, so at minimum document loudly with anchored examples — including that an empty-string entry (UserAllowAll) matches everything — and consider an opt-in flag or a startup warning for unanchored patterns. + +(The socks `UserAddr() == ":"` quirk is covered by the SOCKS ACL bug, task 4.) + +### Refs + +- [#383](https://github.com/jpillora/chisel/issues/383) — Can chisel server be configured to allow only certain ports? +- [#543](https://github.com/jpillora/chisel/issues/543) — Using auth.json file with target verification + +## 27. Build hygiene: Makefile flags, build tags, deprecated APIs, test nits + +* status: closed +* priority: P4 +* type: task +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:44:17Z + +### Items + +- Makefile linux/windows targets set `CGO_ENABLED=1` while goreleaser releases use 0 (static binaries are the point) — align to 0 +- Makefile `dep` target uses deprecated `go get -u` for tools; use `go install tool@version`. `mkdir -p` runs on every make parse +- Old `//+build` tags (`share/cos/signal.go`, `signal_windows.go`, `pprof.go`) — switch to `//go:build` +- Deprecated `net.Error.Temporary()` at `share/tunnel/tunnel_in_proxy_udp.go:91` +- `client/client_test.go:39` uses log.Fatal instead of t.Fatal, and calls t.Fatal from the HTTP handler goroutine (illegal cross-goroutine use; hangs instead of failing) +- Dead code: `share/cio/pipe.go` pipeVis/vis const, `share/cnet/http_server.go` listenErr field, commented MeterRWC call (`tunnel_out_ssh.go:61`) +- e2e setup sleeps 50ms for client readiness (`test/e2e/setup_test.go:116`, acknowledged TODO) — add a readiness signal API to Client instead + +## 28. External auth provider: --authurl webhook + +* status: inprogress +* priority: P3 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:21:39Z + +### Idea + +[PR #582](https://github.com/jpillora/chisel/pull/582) implements `--authurl`: the server POSTs `{"username", "password"}` to an HTTP service; a 200 response with a JSON array of address regexes grants access, anything else denies. + +### Decisions needed + +- Response caching/TTL +- Request timeout + fail-closed semantics +- Secret/header for authenticating chisel to the auth service +- Interaction with live-ACL semantics (see task 8 — resolving the user per channel-open would make webhook auth consistent too) + +Review the PR rather than reimplementing. + +### Refs + +- [PR #582](https://github.com/jpillora/chisel/pull/582) — Add support for server --authurl parameter +- [#476](https://github.com/jpillora/chisel/issues/476) — Feature request: support external authentication system +- [#574](https://github.com/jpillora/chisel/issues/574) — Integration for OTP / Std Logging + +## 29. Observability: connection logs, /metrics, session introspection + +* status: closed +* priority: P3 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:21:11Z + +### Recurring asks + +- See connected clients and their IPs — [#530](https://github.com/jpillora/chisel/issues/530) +- Log all connection attempts incl. failed auth — [#521](https://github.com/jpillora/chisel/issues/521) +- Show authenticated user on connect — [#501](https://github.com/jpillora/chisel/issues/501) +- Active-connection count endpoint for autoscaling — [#522](https://github.com/jpillora/chisel/issues/522), [PR #408](https://github.com/jpillora/chisel/pull/408) +- Prometheus /metrics — [PR #407](https://github.com/jpillora/chisel/pull/407) +- Connect/disconnect hooks — [#410](https://github.com/jpillora/chisel/issues/410), [PR #487](https://github.com/jpillora/chisel/pull/487) +- Client identifier — [#468](https://github.com/jpillora/chisel/issues/468) + +### Minimal valuable step + +Structured Info-level log on session open/close with user, remote IP, declared remotes (the server currently logs sessions only at Debug level — server_handler.go); failed auth at Info (currently Debugf, `server.go:208`). + +Optional next: a /metrics endpoint (sessions, tunnels, bytes — ConnCount and Meter plumbing already exist) gated behind a flag, reusing the health/version switch in handleClientHandler. + +## 30. HTTP fallback transport (SSE/long-poll) for websocket-hostile proxies + +* status: inprogress +* priority: P3 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:17:32Z + +### Idea + +[#589](https://github.com/jpillora/chisel/issues/589) proposes an opt-in SSE+POST transport: long-lived GET for server->client, POST per frame for client->server, reducing to a net.Conn behind the `share/cnet` seam (like conn_ws.go / conn_rwc.go), keeping the single multiplexed SSH session, stdlib only, websocket stays the default. + +### Why + +Long-standing demand from environments that strip Upgrade headers ([#24](https://github.com/jpillora/chisel/issues/24), [#375](https://github.com/jpillora/chisel/issues/375)) and IDS-blocked deployments ([#507](https://github.com/jpillora/chisel/issues/507), [#432](https://github.com/jpillora/chisel/issues/432)). + +### Considerations + +Evaluate building on webdial vs the standalone implementation offered in the issue; buffering-proxy pathologies; auth/path configurability ([#566](https://github.com/jpillora/chisel/issues/566) custom URI pairs well). + +### Refs + +- [#589](https://github.com/jpillora/chisel/issues/589) — SSE transport for environments that block WebSocket upgrades + +## 31. PROXY protocol support (server ingress + reverse-remote egress) + +* status: inprogress +* priority: P4 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:47:23Z + +### Idea + +1. Accept PROXY protocol on the chisel server listener so the real client IP survives an L4 load balancer — affects the logging/ACL-by-IP asks +2. Optionally emit PROXY headers on reverse-remote connections so backend services behind the client see the original source address + +Interacts with requestlog TrustProxy (`server/server.go:176-178`). Parsing must be optional and default-off (header injection risk on public listeners). + +### Refs + +- [#540](https://github.com/jpillora/chisel/issues/540) — [FEAT] Add Proxy Protocol support +- [PR #552](https://github.com/jpillora/chisel/pull/552) — PROXY v2 support (evaluate) + +## 32. Unix domain socket remotes + +* status: inprogress +* priority: P4 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:48:47Z + +### Idea + +Support unix sockets as local listeners and/or remote dial targets (e.g. tunneling a docker.sock or postgres socket). + +Main blocker is remote syntax — `settings.Remote` parsing is colon-delimited (`share/settings/remote.go:43-133`); something like `unix:/path/to.sock` on either side needs explicit escaping rules. The dial/listen split in tunnel_in_proxy.go / tunnel_out_ssh.go is already proto-keyed (tcp/udp), so adding `unix` as an L4Proto is mechanically straightforward. + +### Refs + +- [#399](https://github.com/jpillora/chisel/issues/399) — [UDS] add unix domain support to chisel + +## 33. Key/TLS management UX bundle + +* status: inprogress +* priority: P4 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:49:43Z +* --keygen-json (PR #460): emit {key, fingerprint} JSON for automation; pairs with #499 (key automation ask). + +### Bundle + +- `--keygen-json`: emit `{key, fingerprint}` JSON for automation — [PR #460](https://github.com/jpillora/chisel/pull/460); pairs with [#499](https://github.com/jpillora/chisel/issues/499) (key automation) +- Encrypted PKCS#8 private keys + `--tls-keypass` for mTLS keys — [PR #565](https://github.com/jpillora/chisel/pull/565) +- Avoid plaintext proxy password: read from terminal/stdin — [PR #532](https://github.com/jpillora/chisel/pull/532) +- TLS fingerprint pinning for the transport layer, complementing the SSH `--fingerprint` — [#577](https://github.com/jpillora/chisel/issues/577) + +Keep `test/e2e/env_key_test.go` green throughout — regression test for [#570](https://github.com/jpillora/chisel/issues/570) / [PR #571](https://github.com/jpillora/chisel/pull/571) (CHISEL_KEY env fix). + +## 34. Assorted feature asks worth triaging + +* status: inprogress +* priority: P4 +* type: idea +* created: 2026-06-10T10:24:37Z +* updated: 2026-06-12T11:50:36Z + +### Grab-bag from the last 100 issues + +- Random/ephemeral local port (allow port 0 + report allocation) — [#434](https://github.com/jpillora/chisel/issues/434), [#410](https://github.com/jpillora/chisel/issues/410); isPort currently rejects 0 (`share/settings/remote.go:135-144`) +- Config-file driven client/server — [#393](https://github.com/jpillora/chisel/issues/393); CHISEL_MODE-style env selection for docker — [#436](https://github.com/jpillora/chisel/issues/436) +- WebSocket compression / zstd — [#529](https://github.com/jpillora/chisel/issues/529); gorilla supports permessage-deflate via EnableCompression +- IPv4/IPv6 dial preference flag — localhost resolving to ::1 surprises users when the service binds 127.0.0.1 only — [#544](https://github.com/jpillora/chisel/issues/544), [#479](https://github.com/jpillora/chisel/issues/479), [#520](https://github.com/jpillora/chisel/issues/520) +- Custom websocket endpoint path (--uri) — [#566](https://github.com/jpillora/chisel/issues/566); pairs with the SSE transport idea (task 30) +- Win7 support — [#576](https://github.com/jpillora/chisel/issues/576); infeasible on modern Go, document minimum OS in README instead + +## 35. Dependabot rot: switch to grouped monthly updates or renovate + +* status: inprogress +* priority: P4 +* type: task +* created: 2026-06-12T10:40:33Z +* updated: 2026-06-12T11:52:45Z + +The dependabot config produced 11 stale PRs that sat unmerged for years (closed 2026-06-12 in task 21). Decide between dependabot grouped monthly updates or renovate, then implement. Refs: [#559](https://github.com/jpillora/chisel/issues/559) (renovate suggestion), [#452](https://github.com/jpillora/chisel/issues/452) (confusing \"fake pushes\" from dependabot). diff --git a/client/client.go b/client/client.go index 49b62e65..3891f86a 100644 --- a/client/client.go +++ b/client/client.go @@ -35,6 +35,7 @@ type Config struct { Auth string KeepAlive time.Duration MaxRetryCount int + MinRetryInterval time.Duration MaxRetryInterval time.Duration Server string Proxy string @@ -75,9 +76,6 @@ func NewClient(c *Config) (*Client, error) { if !strings.HasPrefix(c.Server, "http") { c.Server = "http://" + c.Server } - if c.MaxRetryInterval < time.Second { - c.MaxRetryInterval = 5 * time.Minute - } u, err := url.Parse(c.Server) if err != nil { return nil, err @@ -106,6 +104,18 @@ func NewClient(c *Config) (*Client, error) { } //set default log level client.Logger.Info = true + //retry interval defaults; explicit values are honored + if c.MinRetryInterval <= 0 { + c.MinRetryInterval = time.Second + } + if c.MaxRetryInterval <= 0 { + c.MaxRetryInterval = 5 * time.Minute + } + if c.MaxRetryInterval < c.MinRetryInterval { + client.Infof("max-retry-interval (%s) raised to min-retry-interval (%s)", + c.MaxRetryInterval, c.MinRetryInterval) + c.MaxRetryInterval = c.MinRetryInterval + } //configure tls if u.Scheme == "wss" { tc := &tls.Config{} @@ -172,6 +182,9 @@ func NewClient(c *Config) (*Client, error) { } //ssh auth and config user, pass := settings.ParseAuth(c.Auth) + if c.Auth != "" && user == "" { + return nil, fmt.Errorf("invalid auth string, expected :") + } client.sshConfig = &ssh.ClientConfig{ User: user, Auth: []ssh.AuthMethod{ssh.Password(pass)}, @@ -221,7 +234,9 @@ func (c *Client) verifyServer(hostname string, remote net.Addr, key ssh.PublicKe return nil } -// verifyLegacyFingerprint calculates and compares legacy MD5 fingerprints +// verifyLegacyFingerprint calculates and compares legacy MD5 fingerprints, +// requiring the full 16-octet colon form (a prefix match would let a +// truncated fingerprint "verify" against ~1 in 65k keys) func (c *Client) verifyLegacyFingerprint(key ssh.PublicKey) error { bytes := md5.Sum(key.Marshal()) strbytes := make([]string, len(bytes)) @@ -230,7 +245,7 @@ func (c *Client) verifyLegacyFingerprint(key ssh.PublicKey) error { } got := strings.Join(strbytes, ":") expect := c.config.Fingerprint - if !strings.HasPrefix(got, expect) { + if got != expect { return fmt.Errorf("Invalid fingerprint (%s)", got) } return nil @@ -270,10 +285,11 @@ func (c *Client) setProxy(u *url.URL, d *websocket.Dialer) error { } return nil } - // SOCKS5 proxy - if u.Scheme != "socks" && u.Scheme != "socks5h" { + // SOCKS5 proxy. all variants behave identically: DNS is + // resolved by the proxy (golang.org/x/net/proxy.SOCKS5) + if u.Scheme != "socks" && u.Scheme != "socks5" && u.Scheme != "socks5h" { return fmt.Errorf( - "unsupported socks proxy type: %s:// (only socks5h:// or socks:// is supported)", + "unsupported socks proxy type: %s:// (only socks://, socks5:// or socks5h:// are supported)", u.Scheme, ) } @@ -293,6 +309,13 @@ func (c *Client) setProxy(u *url.URL, d *websocket.Dialer) error { return nil } +// Ready blocks until the client has an active connection to the +// server, returning false if the context is cancelled or the +// connection wait times out first +func (c *Client) Ready(ctx context.Context) bool { + return c.tunnel.Ready(ctx) +} + // Wait blocks while the client is running. func (c *Client) Wait() error { return c.eg.Wait() diff --git a/client/client_auth_test.go b/client/client_auth_test.go new file mode 100644 index 00000000..9f35f582 --- /dev/null +++ b/client/client_auth_test.go @@ -0,0 +1,13 @@ +package chclient + +import "testing" + +func TestInvalidAuthString(t *testing.T) { + //auth strings without a colon used to silently send empty creds + if _, err := NewClient(&Config{ + Server: "http://localhost:0", + Auth: "nocolon", + }); err == nil { + t.Fatal("client accepted --auth without a colon") + } +} diff --git a/client/client_connect.go b/client/client_connect.go index 884c7647..1cfdf5b9 100644 --- a/client/client_connect.go +++ b/client/client_connect.go @@ -19,7 +19,10 @@ import ( func (c *Client) connectionLoop(ctx context.Context) error { //connection loop! - b := &backoff.Backoff{Max: c.config.MaxRetryInterval} + b := &backoff.Backoff{ + Min: c.config.MinRetryInterval, + Max: c.config.MaxRetryInterval, + } for { connected, err := c.connectionOnce(ctx) //reset backoff after successful connections @@ -30,7 +33,7 @@ func (c *Client) connectionLoop(ctx context.Context) error { attempt := int(b.Attempt()) maxAttempt := c.config.MaxRetryCount //dont print closed-connection errors - if strings.HasSuffix(err.Error(), "use of closed network connection") { + if err != nil && strings.HasSuffix(err.Error(), "use of closed network connection") { err = io.EOF } //show error message and attempt counts (excluding disconnects) @@ -43,12 +46,15 @@ func (c *Client) connectionLoop(ctx context.Context) error { } msg += fmt.Sprintf(" (Attempt: %d/%s)", attempt, maxAttemptVal) } - c.Infof(msg) + c.Infof("%s", msg) } //give up? if maxAttempt >= 0 && attempt >= maxAttempt { c.Infof("Give up") - break + c.Close() + //unlike a ctx-cancelled shutdown, exhausting the + //connection attempts is an error (non-zero exit) + return errors.New("connection attempts exhausted") } d := b.Duration() c.Infof("Retrying in %s...", d) @@ -60,8 +66,6 @@ func (c *Client) connectionLoop(ctx context.Context) error { return nil } } - c.Close() - return nil } // connectionOnce connects to the chisel server and blocks @@ -102,9 +106,9 @@ func (c *Client) connectionOnce(ctx context.Context) (connected bool, err error) e := err.Error() if strings.Contains(e, "unable to authenticate") { c.Infof("Authentication failed") - c.Debugf(e) + c.Debugf("%s", e) } else { - c.Infof(e) + c.Infof("%s", e) } return false, err } diff --git a/client/client_fingerprint_test.go b/client/client_fingerprint_test.go new file mode 100644 index 00000000..cb8af957 --- /dev/null +++ b/client/client_fingerprint_test.go @@ -0,0 +1,43 @@ +package chclient + +import ( + "crypto/md5" + "fmt" + "strings" + "testing" + + "github.com/jpillora/chisel/share/ccrypto" + "golang.org/x/crypto/ssh" +) + +// TestLegacyFingerprintRequiresFullMatch verifies truncated legacy MD5 +// fingerprints are rejected; a prefix match would "verify" against +// roughly 1 in 65k keys for a two-octet prefix. +func TestLegacyFingerprintRequiresFullMatch(t *testing.T) { + pem, err := ccrypto.Seed2PEM("legacy-fp-test") + if err != nil { + t.Fatal(err) + } + priv, err := ssh.ParsePrivateKey(pem) + if err != nil { + t.Fatal(err) + } + pub := priv.PublicKey() + sum := md5.Sum(pub.Marshal()) + parts := make([]string, len(sum)) + for i, b := range sum { + parts[i] = fmt.Sprintf("%02x", b) + } + full := strings.Join(parts, ":") + mk := func(fp string) *Client { + return &Client{config: &Config{Fingerprint: fp}} + } + if err := mk(full).verifyLegacyFingerprint(pub); err != nil { + t.Fatalf("full legacy fingerprint rejected: %v", err) + } + for _, truncated := range []string{full[:5], full[:23], "a5:32"} { + if err := mk(truncated).verifyLegacyFingerprint(pub); err == nil { + t.Fatalf("truncated legacy fingerprint %q accepted", truncated) + } + } +} diff --git a/client/client_proxy_test.go b/client/client_proxy_test.go new file mode 100644 index 00000000..0d026174 --- /dev/null +++ b/client/client_proxy_test.go @@ -0,0 +1,39 @@ +package chclient + +import ( + "net/url" + "testing" + + "github.com/gorilla/websocket" +) + +func TestSetProxySchemes(t *testing.T) { + //all socks variants use the same SOCKS5 dialer + for _, scheme := range []string{"socks", "socks5", "socks5h"} { + u, err := url.Parse(scheme + "://127.0.0.1:1080") + if err != nil { + t.Fatal(err) + } + d := &websocket.Dialer{} + if err := (&Client{}).setProxy(u, d); err != nil { + t.Fatalf("%s:// rejected: %v", scheme, err) + } + if d.NetDial == nil { + t.Fatalf("%s:// did not install the socks dialer", scheme) + } + } + //http CONNECT proxies pass through + u, _ := url.Parse("http://127.0.0.1:8080") + d := &websocket.Dialer{} + if err := (&Client{}).setProxy(u, d); err != nil { + t.Fatal(err) + } + if d.Proxy == nil { + t.Fatal("http:// did not set the CONNECT proxy") + } + //unknown socks variants are rejected + u, _ = url.Parse("socks4://127.0.0.1:1080") + if err := (&Client{}).setProxy(u, &websocket.Dialer{}); err == nil { + t.Fatal("socks4:// unexpectedly accepted") + } +} diff --git a/client/client_retry_test.go b/client/client_retry_test.go new file mode 100644 index 00000000..a315fac5 --- /dev/null +++ b/client/client_retry_test.go @@ -0,0 +1,76 @@ +package chclient + +import ( + "context" + "strings" + "testing" + "time" +) + +func TestRetryIntervalDefaults(t *testing.T) { + c, err := NewClient(&Config{Server: "http://localhost:0"}) + if err != nil { + t.Fatal(err) + } + if got := c.config.MinRetryInterval; got != time.Second { + t.Fatalf("default MinRetryInterval = %s, want 1s", got) + } + if got := c.config.MaxRetryInterval; got != 5*time.Minute { + t.Fatalf("default MaxRetryInterval = %s, want 5m", got) + } +} + +func TestRetryIntervalExplicit(t *testing.T) { + //explicit values are honored, including sub-second ones + c, err := NewClient(&Config{ + Server: "http://localhost:0", + MinRetryInterval: 200 * time.Millisecond, + MaxRetryInterval: 2 * time.Second, + }) + if err != nil { + t.Fatal(err) + } + if got := c.config.MinRetryInterval; got != 200*time.Millisecond { + t.Fatalf("MinRetryInterval = %s, want 200ms", got) + } + if got := c.config.MaxRetryInterval; got != 2*time.Second { + t.Fatalf("MaxRetryInterval = %s, want 2s", got) + } + //max below min is raised to min + c2, err := NewClient(&Config{ + Server: "http://localhost:0", + MinRetryInterval: 5 * time.Second, + MaxRetryInterval: 2 * time.Second, + }) + if err != nil { + t.Fatal(err) + } + if got := c2.config.MaxRetryInterval; got != 5*time.Second { + t.Fatalf("inverted MaxRetryInterval = %s, want raised to 5s", got) + } +} + +// TestGiveUpReturnsError verifies that exhausting --max-retry-count +// surfaces an error (non-zero process exit) instead of nil. +func TestGiveUpReturnsError(t *testing.T) { + //port 1 is reserved/unbound: connection refused immediately + c, err := NewClient(&Config{ + Server: "http://127.0.0.1:1", + MaxRetryCount: 0, + }) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := c.Start(ctx); err != nil { + t.Fatal(err) + } + err = c.Wait() + if err == nil { + t.Fatal("expected an error after exhausting connection attempts") + } + if !strings.Contains(err.Error(), "exhausted") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/client/client_test.go b/client/client_test.go index f947171a..44a68252 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2,7 +2,6 @@ package chclient import ( "crypto/elliptic" - "log" "net/http" "net/http/httptest" "sync" @@ -14,13 +13,13 @@ import ( ) func TestCustomHeaders(t *testing.T) { - //fake server + //fake server, records the header for the main goroutine to + //assert (t.Fatal must not be called from the handler goroutine) wg := sync.WaitGroup{} wg.Add(1) + var gotFoo string server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - if req.Header.Get("Foo") != "Bar" { - t.Fatal("expected header Foo to be 'Bar'") - } + gotFoo = req.Header.Get("Foo") wg.Done() })) defer server.Close() @@ -36,12 +35,15 @@ func TestCustomHeaders(t *testing.T) { } c, err := NewClient(&config) if err != nil { - log.Fatal(err) + t.Fatal(err) } go c.Run() - //wait for test to complete + //wait for the fake server to receive the request wg.Wait() c.Close() + if gotFoo != "Bar" { + t.Fatalf("expected header Foo to be 'Bar', got %q", gotFoo) + } } func TestFallbackLegacyFingerprint(t *testing.T) { diff --git a/example/users.json b/example/users.json index 23039cc1..1ac8252c 100644 --- a/example/users.json +++ b/example/users.json @@ -8,6 +8,7 @@ "ping:pong": [ "^0.0.0.0:[45]000$", "^example.com:80$", - "^R:0.0.0.0:7000$" + "^R:0.0.0.0:7000$", + "^socks$" ] } diff --git a/go.mod b/go.mod index 916a2d85..fdf11af0 100644 --- a/go.mod +++ b/go.mod @@ -6,20 +6,20 @@ toolchain go1.25.7 require ( github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 - github.com/fsnotify/fsnotify v1.9.0 + github.com/fsnotify/fsnotify v1.10.1 github.com/gorilla/websocket v1.5.3 github.com/jpillora/backoff v1.0.0 github.com/jpillora/requestlog v1.0.0 github.com/jpillora/sizestr v1.0.0 - golang.org/x/crypto v0.52.0 - golang.org/x/net v0.54.0 - golang.org/x/sync v0.20.0 + golang.org/x/crypto v0.54.0 + golang.org/x/net v0.57.0 + golang.org/x/sync v0.22.0 ) require ( github.com/andrew-d/go-termutil v0.0.0-20150726205930-009166a695a2 // indirect github.com/jpillora/ansi v1.0.3 // indirect github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce // indirect - golang.org/x/sys v0.45.0 // indirect - golang.org/x/text v0.37.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/text v0.40.0 // indirect ) diff --git a/go.sum b/go.sum index 0ba339e8..c731732a 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/andrew-d/go-termutil v0.0.0-20150726205930-009166a695a2 h1:axBiC50cNZ github.com/andrew-d/go-termutil v0.0.0-20150726205930-009166a695a2/go.mod h1:jnzFpU88PccN/tPPhCpnNU8mZphvKxYM9lLNkd8e+os= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= -github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= -github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= +github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= +github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/jpillora/ansi v1.0.3 h1:nn4Jzti0EmRfDxm7JtEs5LzCbNwd5sv+0aE+LdS9/ZQ= @@ -16,15 +16,15 @@ github.com/jpillora/sizestr v1.0.0 h1:4tr0FLxs1Mtq3TnsLDV+GYUWG7Q26a6s+tV5Zfw2yg github.com/jpillora/sizestr v1.0.0/go.mod h1:bUhLv4ctkknatr6gR42qPxirmd5+ds1u7mzD+MZ33f0= github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce h1:fb190+cK2Xz/dvi9Hv8eCYJYvIGUTN2/KLq1pT6CjEc= github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce/go.mod h1:o8v6yHRoik09Xen7gje4m9ERNah1d1PPsVq1VEx9vE4= -golang.org/x/crypto v0.52.0 h1:RMs7fP2rXdep0CftQlK8Uf+kibLm7qkCcradZWYz988= -golang.org/x/crypto v0.52.0/go.mod h1:1QgfPxDqh0T2M/elOJtp9RvuR95kVjir0e6/BvEmGbc= -golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= -golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= -golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= -golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= -golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= -golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= +golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= diff --git a/main.go b/main.go index 7af1f45e..2b3fe35d 100644 --- a/main.go +++ b/main.go @@ -75,6 +75,8 @@ var commonHelp = ` Signals: The chisel process is listening for: + a SIGINT or SIGTERM to begin a graceful shutdown + (a second signal forces an immediate exit), a SIGUSR2 to print process stats, and a SIGHUP to short-circuit the client reconnect timer @@ -102,7 +104,7 @@ var serverHelp = ` (defaults the environment variable HOST and falls back to 0.0.0.0). --port, -p, Defines the HTTP listening port (defaults to the environment - variable PORT and fallsback to port 8080). + variable PORT and falls back to port 8080). --key, (deprecated use --keygen and --keyfile instead) An optional string to seed the generation of a ECDSA public @@ -119,7 +121,8 @@ var serverHelp = ` this flag is set, the --key option is ignored, and the provided private key is used to secure all communications. (defaults to the CHISEL_KEY_FILE environment variable). Since ECDSA keys are short, you may also set keyfile - to an inline base64 private key (e.g. chisel server --keygen - | base64). + to the inline key string itself, exactly as printed by --keygen (a base64 + string with a "ck-" prefix); no extra base64 encoding is needed. --authfile, An optional path to a users.json file. This file should be an object with users defined like: @@ -128,10 +131,20 @@ var serverHelp = ` } when connects, their will be verified and then each of the remote addresses will be compared against the list - of address regular expressions for a match. Addresses will - always come in the form ":" for normal remotes - and "R::" for reverse port forwarding - remotes. This file will be automatically reloaded on change. + of address regular expressions for a match. Patterns are NOT + anchored by default: "10.0.0.1:80" also matches + "210.0.0.1:8080", and "." matches any character. Anchor your + patterns, e.g. "^10\.0\.0\.1:80$". The empty string "" + matches every address. Addresses will + always come in the form ":" for normal remotes, + "R::" for reverse port forwarding + remotes, and "socks" for SOCKS5 proxy access. Note that SOCKS5 + access previously bypassed this list; existing authfiles which + should allow SOCKS5 must add an entry matching "socks" (the + empty wildcard "" matches everything, including "socks"). This + file will be automatically reloaded on change. Reloads apply + to new connections and to new tunnels of connected clients; + established tunnels are not interrupted. --auth, An optional string representing a single user with full access, in the form of . It is equivalent to creating an @@ -146,7 +159,7 @@ var serverHelp = ` --backend, Specifies another HTTP server to proxy requests to when chisel receives a normal HTTP request. Useful for hiding chisel in - plain sight. + plain sight. --proxy is accepted as an alias for this flag. --socks5, Allow clients to access the internal SOCKS5 proxy. See chisel client --help for more information. @@ -314,7 +327,7 @@ var clientHelp = ` ■ local-host defaults to 0.0.0.0 (all interfaces). ■ local-port defaults to remote-port. ■ remote-port is required*. - ■ remote-host defaults to 0.0.0.0 (server localhost). + ■ remote-host defaults to 127.0.0.1 (server localhost). ■ protocol defaults to tcp. which shares : from the server to the client @@ -343,7 +356,9 @@ var clientHelp = ` specify "socks" in place of remote-host and remote-port. The default local host and port for a "socks" remote is 127.0.0.1:1080. Connections to this remote will terminate - at the server's internal SOCKS5 proxy. + at the server's internal SOCKS5 proxy. When the server also + has --authfile set, SOCKS5 access requires an entry matching + the token "socks" in the user's address list. When the chisel server has --reverse enabled, remotes can be prefixed with R to denote that they are reversed. That @@ -364,10 +379,13 @@ var clientHelp = ` --fingerprint, A *strongly recommended* fingerprint string to perform host-key validation against the server's public key. - Fingerprint mismatches will close the connection. - Fingerprints are generated by hashing the ECDSA public key using - SHA256 and encoding the result in base64. - Fingerprints must be 44 characters containing a trailing equals (=). + Fingerprint mismatches will close the connection. + Fingerprints are generated by hashing the ECDSA public key using + SHA256 and encoding the result in base64. + Fingerprints must be 44 characters containing a trailing equals (=). + Legacy MD5 colon fingerprints (deprecated) are still accepted, + but only in their full 16-octet form; truncated prefixes are + rejected. --auth, An optional username and password (client authentication) in the form: ":". These credentials are compared to @@ -383,14 +401,20 @@ var clientHelp = ` --max-retry-count, Maximum number of times to retry before exiting. Defaults to unlimited. + --min-retry-interval, Minimum wait time before retrying after a + disconnection. Defaults to 1 second. + --max-retry-interval, Maximum wait time before retrying after a disconnection. Defaults to 5 minutes. --proxy, An optional HTTP CONNECT or SOCKS5 proxy which will be used to reach the chisel server. Authentication can be specified - inside the URL. + inside the URL. Credentials must be URL-encoded; for example a + "#" in the password must be written as "%23". For example, http://admin:password@my-server.com:8081 or: socks://admin:password@my-server.com:1080 + The socks://, socks5:// and socks5h:// schemes are equivalent: + DNS is always resolved by the proxy. --header, Set a custom header in the form "HeaderName: HeaderContent". Can be used multiple times. (e.g --header "Foo: Bar" --header "Hello: World") @@ -428,6 +452,7 @@ func client(args []string) { flags.StringVar(&config.Auth, "auth", "", "") flags.DurationVar(&config.KeepAlive, "keepalive", 25*time.Second, "") flags.IntVar(&config.MaxRetryCount, "max-retry-count", -1, "") + flags.DurationVar(&config.MinRetryInterval, "min-retry-interval", 0, "") flags.DurationVar(&config.MaxRetryInterval, "max-retry-interval", 0, "") flags.StringVar(&config.Proxy, "proxy", "", "") flags.StringVar(&config.TLS.CA, "tls-ca", "", "") diff --git a/server/server.go b/server/server.go index 8a702fce..281994a7 100644 --- a/server/server.go +++ b/server/server.go @@ -2,8 +2,8 @@ package chserver import ( "context" - "errors" - "log" + "crypto/subtle" + "fmt" "net/http" "net/http/httputil" "net/url" @@ -34,7 +34,7 @@ type Config struct { TLS TLSConfig } -// Server respresent a chisel service +// Server represent a chisel service type Server struct { *cio.Logger config *Config @@ -42,7 +42,6 @@ type Server struct { httpServer *cnet.HTTPServer reverseProxy *httputil.ReverseProxy sessCount int32 - sessions *settings.Users sshConfig *ssh.ServerConfig users *settings.UserIndex } @@ -59,20 +58,21 @@ func NewServer(c *Config) (*Server, error) { config: c, httpServer: cnet.NewHTTPServer(), Logger: cio.NewLogger("server"), - sessions: settings.NewUsers(), } server.Info = true server.users = settings.NewUserIndex(server.Logger) - if c.AuthFile != "" { - if err := server.users.LoadUsers(c.AuthFile); err != nil { - return nil, err - } - } + //pin the --auth user first so authfile reloads cannot drop it if c.Auth != "" { u := &settings.User{Addrs: []*regexp.Regexp{settings.UserAllowAll}} u.Name, u.Pass = settings.ParseAuth(c.Auth) - if u.Name != "" { - server.users.AddUser(u) + if u.Name == "" { + return nil, server.Errorf("invalid auth string, expected :") + } + server.users.PinUser(u) + } + if c.AuthFile != "" { + if err := server.users.LoadUsers(c.AuthFile); err != nil { + return nil, err } } @@ -86,7 +86,7 @@ func NewServer(c *Config) (*Server, error) { } else { key, err = os.ReadFile(c.KeyFile) if err != nil { - log.Fatalf("Failed to read key file %s", c.KeyFile) + return nil, server.Errorf("Failed to read key file %s: %s", c.KeyFile, err) } } @@ -94,21 +94,21 @@ func NewServer(c *Config) (*Server, error) { if ccrypto.IsChiselKey(key) { pemBytes, err = ccrypto.ChiselKey2PEM(key) if err != nil { - log.Fatalf("Invalid key %s", string(key)) + return nil, server.Errorf("Invalid chisel key: %s", err) } } } else { //generate private key (optionally using seed) pemBytes, err = ccrypto.Seed2PEM(c.KeySeed) if err != nil { - log.Fatal("Failed to generate key") + return nil, server.Errorf("Failed to generate key: %s", err) } } //convert into ssh.PrivateKey private, err := ssh.ParsePrivateKey(pemBytes) if err != nil { - log.Fatal("Failed to parse key") + return nil, server.Errorf("Failed to parse key: %s", err) } //fingerprint this key server.fingerprint = ccrypto.FingerprintKey(private.PublicKey()) @@ -204,14 +204,16 @@ func (s *Server) authUser(c ssh.ConnMetadata, password []byte) (*ssh.Permissions // check the user exists and has matching password n := c.User() user, found := s.users.Get(n) - if !found || user.Pass != string(password) { - s.Debugf("Login failed for user: %s", n) - return nil, errors.New("Invalid authentication for username: %s") - } - // insert the user session map - // TODO this should probably have a lock on it given the map isn't thread-safe - s.sessions.Set(string(c.SessionID()), user) - return nil, nil + if !found || subtle.ConstantTimeCompare([]byte(user.Pass), password) != 1 { + //info level: operators should see failed attempts (#521) + s.Infof("Login failed for user %s (%s)", n, c.RemoteAddr()) + return nil, fmt.Errorf("invalid authentication for username: %s", n) + } + // pass the username through to the handshake handler, which + // re-resolves the user (no session state to leak or race) + return &ssh.Permissions{ + Extensions: map[string]string{"user": n}, + }, nil } // AddUser adds a new user into the server user index diff --git a/server/server_auth_test.go b/server/server_auth_test.go new file mode 100644 index 00000000..a789db95 --- /dev/null +++ b/server/server_auth_test.go @@ -0,0 +1,66 @@ +package chserver + +import ( + "net" + "strings" + "testing" +) + +type testConnMeta struct{ user string } + +func (m testConnMeta) User() string { return m.user } +func (m testConnMeta) SessionID() []byte { return []byte("test-session") } +func (m testConnMeta) ClientVersion() []byte { return nil } +func (m testConnMeta) ServerVersion() []byte { return nil } +func (m testConnMeta) RemoteAddr() net.Addr { return &net.TCPAddr{} } +func (m testConnMeta) LocalAddr() net.Addr { return &net.TCPAddr{} } + +func TestAuthUser(t *testing.T) { + s, err := NewServer(&Config{KeySeed: "auth-test"}) + if err != nil { + t.Fatal(err) + } + if err := s.AddUser("alice", "secret", ""); err != nil { + t.Fatal(err) + } + //valid credentials carry the username through Permissions + perms, err := s.authUser(testConnMeta{"alice"}, []byte("secret")) + if err != nil { + t.Fatalf("valid login failed: %v", err) + } + if perms == nil || perms.Extensions["user"] != "alice" { + t.Fatalf("expected user extension, got %+v", perms) + } + //wrong password + if _, err := s.authUser(testConnMeta{"alice"}, []byte("wrong")); err == nil { + t.Fatal("wrong password accepted") + } else if strings.Contains(err.Error(), "%s") || !strings.Contains(err.Error(), "alice") { + t.Fatalf("malformed error message: %q", err) + } + //unknown user + if _, err := s.authUser(testConnMeta{"bob"}, []byte("secret")); err == nil { + t.Fatal("unknown user accepted") + } +} + +func TestAuthUserAllowAll(t *testing.T) { + //no users configured: authentication is disabled + s, err := NewServer(&Config{KeySeed: "auth-allow-all"}) + if err != nil { + t.Fatal(err) + } + perms, err := s.authUser(testConnMeta{"anyone"}, []byte("anything")) + if err != nil { + t.Fatalf("allow-all rejected: %v", err) + } + if perms != nil { + t.Fatalf("expected nil permissions for allow-all, got %+v", perms) + } +} + +func TestInvalidAuthString(t *testing.T) { + //auth strings without a colon used to silently disable auth + if _, err := NewServer(&Config{KeySeed: "x", Auth: "nocolon"}); err == nil { + t.Fatal("server accepted --auth without a colon") + } +} diff --git a/server/server_handler.go b/server/server_handler.go index 8b5a68fd..5c993f95 100644 --- a/server/server_handler.go +++ b/server/server_handler.go @@ -1,6 +1,7 @@ package chserver import ( + "fmt" "net/http" "strings" "sync/atomic" @@ -64,16 +65,19 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { s.Debugf("Failed to handshake (%s)", err) return } - // pull the users from the session map + // resolve the user from the authenticated username set by + // authUser (nil permissions means auth is disabled: allow-all) var user *settings.User - if s.users.Len() > 0 { - sid := string(sshConn.SessionID()) - u, ok := s.sessions.Get(sid) - if !ok { - panic("bug in ssh auth handler") + if sshConn.Permissions != nil { + n := sshConn.Permissions.Extensions["user"] + u, found := s.users.Get(n) + if !found { + //user was removed by an authfile reload mid-handshake + l.Infof("User %s no longer exists", n) + sshConn.Close() + return } user = u - s.sessions.Del(sid) } // chisel server handshake (reverse of client handshake) // verify configuration @@ -87,6 +91,13 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { sshConn.Close() return } + if r == nil { + //connection closed before the config request arrived; the + //closed request channel yields nil + l.Debugf("Connection closed before configuration") + sshConn.Close() + return + } failed := func(err error) { l.Debugf("Failed: %s", err) r.Reply(false, []byte(err.Error())) @@ -100,13 +111,14 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { failed(s.Errorf("invalid config")) return } - //print if client and server versions dont match + //print if client and server versions dont match, + //skipping dev/unknown builds where the mismatch is meaningless cv := strings.TrimPrefix(c.Version, "v") - if cv == "" { - cv = "" - } sv := strings.TrimPrefix(chshare.BuildVersion, "v") - if cv != sv { + devVersion := func(v string) bool { + return v == "" || strings.HasPrefix(v, "0.0.0") + } + if cv != sv && !devVersion(cv) && !devVersion(sv) { l.Infof("Client version (%s) differs from server version (%s)", cv, sv) } //validate remotes @@ -123,7 +135,7 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { //confirm reverse tunnels are allowed if r.Reverse && !s.config.Reverse { l.Debugf("Denied reverse port forwarding request, please enable --reverse") - failed(s.Errorf("Reverse port forwaring not enabled on server")) + failed(s.Errorf("Reverse port forwarding not enabled on server")) return } //confirm reverse tunnel is available @@ -132,8 +144,17 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { return } } - //successfuly validated config! + //successfully validated config! r.Reply(true, nil) + //log session opens at info level so operators can see connected + //clients, their IPs and declared remotes without debug mode + username := "-" + if user != nil { + username = user.Name + } + opened := time.Now() + l.Infof("Open (user=%s addr=%s remotes=%s)", + username, req.RemoteAddr, strings.Join(c.Remotes.Encode(), ",")) //tunnel per ssh connection tunnelConfig := tunnel.Config{ Logger: l, @@ -142,9 +163,15 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { Socks: s.config.Socks5, KeepAlive: s.config.KeepAlive, } - //enforce ACL on every channel, not just the initial config + //enforce ACL on every channel, not just the initial config. + //the user is re-resolved from the live index per channel, so + //authfile reloads apply to connected clients' new tunnels if user != nil { - tunnelConfig.ACL = user.HasAccess + name := user.Name + tunnelConfig.ACL = func(addr string) bool { + u, found := s.users.Get(name) + return found && u.HasAccess(addr) + } } tunnel := tunnel.New(tunnelConfig) //bind @@ -163,9 +190,10 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) { return tunnel.BindRemotes(ctx, serverInbound) }) err = eg.Wait() + errmsg := "" if err != nil && !strings.HasSuffix(err.Error(), "EOF") { - l.Debugf("Closed connection (%s)", err) - } else { - l.Debugf("Closed connection") + errmsg = fmt.Sprintf(" (error %s)", err) } + l.Infof("Close (user=%s addr=%s duration=%s)%s", + username, req.RemoteAddr, time.Since(opened), errmsg) } diff --git a/server/server_listen.go b/server/server_listen.go index f6eb1ffa..25773ffc 100644 --- a/server/server_listen.go +++ b/server/server_listen.go @@ -28,19 +28,19 @@ func (s *Server) listener(host, port string) (net.Listener, error) { return nil, errors.New("cannot use key/cert and domains") } var tlsConf *tls.Config + extra := "" if hasDomains { tlsConf = s.tlsLetsEncrypt(s.config.TLS.Domains) + if port != "443" { + extra = " (WARNING: LetsEncrypt will attempt to connect to your domain on port 443)" + } } - extra := "" if hasKeyCert { c, err := s.tlsKeyCert(s.config.TLS.Key, s.config.TLS.Cert, s.config.TLS.CA) if err != nil { return nil, err } tlsConf = c - if port != "443" && hasDomains { - extra = " (WARNING: LetsEncrypt will attempt to connect to your domain on port 443)" - } } //tcp listen l, err := net.Listen("tcp", host+":"+port) @@ -53,9 +53,7 @@ func (s *Server) listener(host, port string) (net.Listener, error) { proto += "s" l = tls.NewListener(l, tlsConf) } - if err == nil { - s.Infof("Listening on %s://%s:%s%s", proto, host, port, extra) - } + s.Infof("Listening on %s://%s:%s%s", proto, host, port, extra) return l, nil } diff --git a/server/server_test.go b/server/server_test.go new file mode 100644 index 00000000..2475adb2 --- /dev/null +++ b/server/server_test.go @@ -0,0 +1,25 @@ +package chserver + +import ( + "os" + "path/filepath" + "testing" +) + +// TestNewServerKeyErrors verifies key-loading failures are returned as +// errors rather than calling log.Fatal, which would kill processes +// embedding chisel as a library. +func TestNewServerKeyErrors(t *testing.T) { + //missing key file + if _, err := NewServer(&Config{KeyFile: "/nonexistent/key"}); err == nil { + t.Fatal("expected error for missing key file") + } + //unparseable key material + bad := filepath.Join(t.TempDir(), "bad.key") + if err := os.WriteFile(bad, []byte("not a pem key"), 0600); err != nil { + t.Fatal(err) + } + if _, err := NewServer(&Config{KeyFile: bad}); err == nil { + t.Fatal("expected error for unparseable key file") + } +} diff --git a/share/cio/pipe.go b/share/cio/pipe.go index e32fc44e..61d00d3d 100644 --- a/share/cio/pipe.go +++ b/share/cio/pipe.go @@ -2,47 +2,59 @@ package cio import ( "io" - "log" "sync" ) +//closeWriter is implemented by connections which support +//half-close (*net.TCPConn, ssh.Channel, cnet rwcConn) +type closeWriter interface { + CloseWrite() error +} + +//Pipe copies between src and dst in both directions, propagating +//half-closes: when one direction reaches a clean EOF, the write side +//of its destination is closed (CloseWrite) while the other direction +//keeps flowing. Copy errors tear down both ends. Both connections +//are fully closed before Pipe returns. func Pipe(src io.ReadWriteCloser, dst io.ReadWriteCloser) (int64, int64) { var sent, received int64 var wg sync.WaitGroup - var o sync.Once - close := func() { + teardown := func() { src.Close() dst.Close() } wg.Add(2) go func() { - received, _ = io.Copy(src, dst) - o.Do(close) - wg.Done() + defer wg.Done() + var err error + sent, err = io.Copy(dst, src) + if err != nil { + teardown() + return + } + closeWrite(dst) }() go func() { - sent, _ = io.Copy(dst, src) - o.Do(close) - wg.Done() + defer wg.Done() + var err error + received, err = io.Copy(src, dst) + if err != nil { + teardown() + return + } + closeWrite(src) }() wg.Wait() + teardown() return sent, received } -const vis = false - -type pipeVisPrinter struct { - name string -} - -func (p pipeVisPrinter) Write(b []byte) (int, error) { - log.Printf(">>> %s: %x", p.name, b) - return len(b), nil -} - -func pipeVis(name string, r io.Reader) io.Reader { - if vis { - return io.TeeReader(r, pipeVisPrinter{name}) +//closeWrite half-closes the connection when supported, +//falling back to a full close +func closeWrite(c io.ReadWriteCloser) { + if cw, ok := c.(closeWriter); ok { + cw.CloseWrite() + return } - return r + c.Close() } diff --git a/share/cnet/conn_rwc.go b/share/cnet/conn_rwc.go index 55749d11..2a0dfd7d 100644 --- a/share/cnet/conn_rwc.go +++ b/share/cnet/conn_rwc.go @@ -11,6 +11,10 @@ type rwcConn struct { buff []byte } +type closeWriter interface { + CloseWrite() error +} + //NewRWCConn converts a RWC into a net.Conn func NewRWCConn(rwc io.ReadWriteCloser) net.Conn { c := rwcConn{ @@ -46,3 +50,18 @@ func (c *rwcConn) SetReadDeadline(t time.Time) error { func (c *rwcConn) SetWriteDeadline(t time.Time) error { return nil //no-op } + +//CloseWrite propagates half-closes to the underlying +//connection when supported (e.g. ssh.Channel). When the underlying +//stream cannot half-close, it falls back to a full Close — sacrificing +//any in-flight reverse traffic — so the peer observes EOF rather than +//cio.Pipe hanging on a half-close that never happened +func (c *rwcConn) CloseWrite() error { + if cw, ok := c.ReadWriteCloser.(closeWriter); ok { + return cw.CloseWrite() + } + //underlying stream can't half-close; fall back to a full close so the + //peer still observes EOF (otherwise cio.Pipe treats the no-op as a + //successful half-close and the opposite copy can block forever) + return c.Close() +} diff --git a/share/cnet/conn_rwc_test.go b/share/cnet/conn_rwc_test.go new file mode 100644 index 00000000..dd146daa --- /dev/null +++ b/share/cnet/conn_rwc_test.go @@ -0,0 +1,64 @@ +package cnet + +import ( + "io" + "testing" +) + +type fakeRWC struct { + io.ReadWriteCloser + closedWrite bool +} + +func (f *fakeRWC) Read([]byte) (int, error) { return 0, io.EOF } +func (f *fakeRWC) Write([]byte) (int, error) { return 0, io.ErrClosedPipe } +func (f *fakeRWC) Close() error { return nil } +func (f *fakeRWC) CloseWrite() error { + f.closedWrite = true + return nil +} + +// TestRWCConnCloseWrite verifies half-closes are delegated to the +// underlying stream when supported (e.g. ssh.Channel for socks conns). +func TestRWCConnCloseWrite(t *testing.T) { + f := &fakeRWC{} + c := NewRWCConn(f) + cw, ok := c.(interface{ CloseWrite() error }) + if !ok { + t.Fatal("rwcConn does not expose CloseWrite") + } + if err := cw.CloseWrite(); err != nil { + t.Fatal(err) + } + if !f.closedWrite { + t.Fatal("CloseWrite was not delegated to the underlying stream") + } +} + +// plainRWC is a ReadWriteCloser without half-close support. +type plainRWC struct { + closed bool +} + +func (p *plainRWC) Read([]byte) (int, error) { return 0, io.EOF } +func (p *plainRWC) Write([]byte) (int, error) { return 0, io.ErrClosedPipe } +func (p *plainRWC) Close() error { p.closed = true; return nil } + +// TestRWCConnCloseWriteFallback verifies that when the underlying stream +// cannot half-close, CloseWrite falls back to a full Close so the peer +// still observes EOF instead of a silent no-op (which would let cio.Pipe +// treat the half-close as successful and hang the opposite copy). +func TestRWCConnCloseWriteFallback(t *testing.T) { + p := &plainRWC{} + c := NewRWCConn(p) + cw, ok := c.(interface{ CloseWrite() error }) + if !ok { + t.Fatal("rwcConn does not expose CloseWrite") + } + if err := cw.CloseWrite(); err != nil { + t.Fatal(err) + } + if !p.closed { + t.Fatal("CloseWrite did not fall back to Close for a non-half-close stream") + } +} diff --git a/share/cnet/conn_ws.go b/share/cnet/conn_ws.go index 9639e991..79ab80ba 100644 --- a/share/cnet/conn_ws.go +++ b/share/cnet/conn_ws.go @@ -5,6 +5,7 @@ import ( "time" "github.com/gorilla/websocket" + "github.com/jpillora/chisel/share/settings" ) type wsConn struct { @@ -14,13 +15,17 @@ type wsConn struct { //NewWebSocketConn converts a websocket.Conn into a net.Conn func NewWebSocketConn(websocketConn *websocket.Conn) net.Conn { + //ssh packets are at most ~35KB, so cap inbound messages to + //prevent pre-auth memory exhaustion from oversized frames. + //tune with WS_READ_LIMIT (0 disables the limit) + websocketConn.SetReadLimit(int64(settings.EnvInt("WS_READ_LIMIT", 64*1024))) c := wsConn{ Conn: websocketConn, } return &c } -//Read is not threadsafe though thats okay since there +//Read is not threadsafe though that's okay since there //should never be more than one reader func (c *wsConn) Read(dst []byte) (int, error) { ldst := len(dst) diff --git a/share/cnet/conn_ws_test.go b/share/cnet/conn_ws_test.go new file mode 100644 index 00000000..96644be0 --- /dev/null +++ b/share/cnet/conn_ws_test.go @@ -0,0 +1,79 @@ +package cnet + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" +) + +//wsPair returns the server side of a websocket connection wrapped in +//NewWebSocketConn, plus the raw client side for sending test frames. +func wsPair(t *testing.T) (serverSide chan interface{}, client *websocket.Conn) { + t.Helper() + upgrader := websocket.Upgrader{} + out := make(chan interface{}, 1) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ws, err := upgrader.Upgrade(w, r, nil) + if err != nil { + t.Errorf("upgrade: %v", err) + return + } + conn := NewWebSocketConn(ws) + conn.SetDeadline(time.Now().Add(5 * time.Second)) + buf := make([]byte, 1024) + n, err := conn.Read(buf) + if err != nil { + out <- err + return + } + out <- append([]byte(nil), buf[:n]...) + })) + t.Cleanup(ts.Close) + url := "ws" + strings.TrimPrefix(ts.URL, "http") + ws, _, err := websocket.DefaultDialer.Dial(url, nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { ws.Close() }) + return out, ws +} + +//TestWSConnReadLimit verifies that an oversized inbound message errors +//the read instead of being buffered into memory. +func TestWSConnReadLimit(t *testing.T) { + out, client := wsPair(t) + //bigger than the 64KB default limit + big := bytes.Repeat([]byte("x"), 128*1024) + if err := client.WriteMessage(websocket.BinaryMessage, big); err != nil { + t.Fatal(err) + } + switch v := (<-out).(type) { + case error: + //read correctly failed + case []byte: + t.Fatalf("oversized message was read (%d bytes leaked through)", len(v)) + default: + _ = v + } +} + +//TestWSConnReadNormal verifies normal-sized messages still flow. +func TestWSConnReadNormal(t *testing.T) { + out, client := wsPair(t) + if err := client.WriteMessage(websocket.BinaryMessage, []byte("hello")); err != nil { + t.Fatal(err) + } + switch v := (<-out).(type) { + case []byte: + if string(v) != "hello" { + t.Fatalf("got %q", v) + } + case error: + t.Fatalf("read failed: %v", v) + } +} diff --git a/share/cnet/http_server.go b/share/cnet/http_server.go index 6f46a712..30d5e0b7 100644 --- a/share/cnet/http_server.go +++ b/share/cnet/http_server.go @@ -6,7 +6,9 @@ import ( "net" "net/http" "sync" + "time" + "github.com/jpillora/chisel/share/settings" "golang.org/x/sync/errgroup" ) @@ -16,7 +18,6 @@ type HTTPServer struct { *http.Server waiterMux sync.Mutex waiter *errgroup.Group - listenErr error } //NewHTTPServer creates a new HTTPServer @@ -55,6 +56,13 @@ func (h *HTTPServer) GoServe(ctx context.Context, l net.Listener, handler http.H }) go func() { <-ctx.Done() + //graceful shutdown: stop accepting, drain in-flight requests + //for a grace period, then force-close the remainder. hijacked + //connections (websocket tunnels) are closed by their owners. + grace := settings.EnvDuration("SHUTDOWN_GRACE", 5*time.Second) + c, cancel := context.WithTimeout(context.Background(), grace) + defer cancel() + h.Server.Shutdown(c) h.Close() }() return nil diff --git a/share/cnet/http_server_test.go b/share/cnet/http_server_test.go new file mode 100644 index 00000000..ac7e2226 --- /dev/null +++ b/share/cnet/http_server_test.go @@ -0,0 +1,63 @@ +package cnet + +import ( + "context" + "io" + "net" + "net/http" + "testing" + "time" +) + +// TestGoServeGracefulDrain verifies that cancelling the GoServe context +// drains in-flight requests instead of resetting them, while refusing +// new connections. +func TestGoServeGracefulDrain(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + h := NewHTTPServer() + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(500 * time.Millisecond) + w.Write([]byte("done")) + }) + if err := h.GoServe(ctx, l, handler); err != nil { + t.Fatal(err) + } + url := "http://" + l.Addr().String() + //fire an in-flight request, then cancel the server mid-request + bodyc := make(chan string, 1) + errc := make(chan error, 1) + go func() { + resp, err := http.Get(url) + if err != nil { + errc <- err + return + } + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + bodyc <- string(b) + }() + time.Sleep(100 * time.Millisecond) //let the request reach the handler + cancel() + select { + case b := <-bodyc: + if b != "done" { + t.Fatalf("in-flight response corrupted: %q", b) + } + case err := <-errc: + t.Fatalf("in-flight request failed during shutdown: %v", err) + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for drained response") + } + //new connections must be refused after shutdown + if _, err := http.Get(url); err == nil { + t.Fatal("new request succeeded after shutdown") + } + if err := h.Wait(); err != nil { + t.Fatalf("wait: %v", err) + } +} diff --git a/share/cos/common.go b/share/cos/common.go index 9098044d..575c534e 100644 --- a/share/cos/common.go +++ b/share/cos/common.go @@ -2,21 +2,29 @@ package cos import ( "context" + "log" "os" "os/signal" + "syscall" "time" ) -//InterruptContext returns a context which is -//cancelled on OS Interrupt +//InterruptContext returns a context which is cancelled on OS +//interrupt or SIGTERM (docker stop, kubernetes pod termination). +//A second signal forces an immediate exit. func InterruptContext() context.Context { ctx, cancel := context.WithCancel(context.Background()) + sig := make(chan os.Signal, 1) + //register before returning so there is no window where a + //SIGTERM takes the default kill path + //(SIGTERM never fires on windows, os.Interrupt covers ctrl-c) + signal.Notify(sig, os.Interrupt, syscall.SIGTERM) go func() { - sig := make(chan os.Signal, 1) - signal.Notify(sig, os.Interrupt) //windows compatible? <-sig - signal.Stop(sig) - cancel() + cancel() //begin graceful shutdown + <-sig + log.Print("second interrupt signal, forcing exit") + os.Exit(1) }() return ctx } diff --git a/share/cos/common_test.go b/share/cos/common_test.go new file mode 100644 index 00000000..3f842dd3 --- /dev/null +++ b/share/cos/common_test.go @@ -0,0 +1,25 @@ +//go:build !windows + +package cos + +import ( + "os" + "syscall" + "testing" + "time" +) + +// TestInterruptContextSIGTERM verifies that SIGTERM (docker stop, +// kubernetes) cancels the context rather than killing the process. +func TestInterruptContextSIGTERM(t *testing.T) { + ctx := InterruptContext() + if err := syscall.Kill(os.Getpid(), syscall.SIGTERM); err != nil { + t.Fatal(err) + } + select { + case <-ctx.Done(): + //graceful path taken + case <-time.After(2 * time.Second): + t.Fatal("SIGTERM did not cancel the context") + } +} diff --git a/share/cos/pprof.go b/share/cos/pprof.go index 6c95b7e7..dae65bb1 100644 --- a/share/cos/pprof.go +++ b/share/cos/pprof.go @@ -1,4 +1,4 @@ -// +build pprof +//go:build pprof package cos diff --git a/share/cos/signal.go b/share/cos/signal.go index f44e4487..cf32b0c1 100644 --- a/share/cos/signal.go +++ b/share/cos/signal.go @@ -1,4 +1,4 @@ -//+build !windows +//go:build !windows package cos @@ -24,7 +24,7 @@ func GoStats() { for range c { memStats := runtime.MemStats{} runtime.ReadMemStats(&memStats) - log.Printf("recieved SIGUSR2, go-routines: %d, go-memory-usage: %s", + log.Printf("received SIGUSR2, go-routines: %d, go-memory-usage: %s", runtime.NumGoroutine(), sizestr.ToString(int64(memStats.Alloc))) } diff --git a/share/cos/signal_windows.go b/share/cos/signal_windows.go index 747b3d47..f4540712 100644 --- a/share/cos/signal_windows.go +++ b/share/cos/signal_windows.go @@ -1,4 +1,4 @@ -//+build windows +//go:build windows package cos diff --git a/share/settings/remote.go b/share/settings/remote.go index 520b2fcd..4f9e7fe6 100644 --- a/share/settings/remote.go +++ b/share/settings/remote.go @@ -119,7 +119,7 @@ func DecodeRemote(s string) (*Remote, error) { } if r.LocalProto != r.RemoteProto { //TODO support cross protocol - //tcp <-> udp, is faily straight forward + //tcp <-> udp, is fairly straightforward //udp <-> tcp, is trickier since udp is stateless and tcp is not return nil, errors.New("cross-protocol remotes are not supported yet") } @@ -153,11 +153,13 @@ func isHost(s string) bool { var l4Proto = regexp.MustCompile(`(?i)\/(tcp|udp)$`) -//L4Proto extacts the layer-4 protocol from the given string +//L4Proto extracts the layer-4 protocol from the given string func L4Proto(s string) (head, proto string) { if l4Proto.MatchString(s) { l := len(s) - return strings.ToLower(s[:l-4]), s[l-3:] + //lowercase the proto to match the case-insensitive regex, + //all later comparisons expect "tcp"/"udp" + return strings.ToLower(s[:l-4]), strings.ToLower(s[l-3:]) } return s, "" } @@ -221,10 +223,6 @@ 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" } diff --git a/share/settings/remote_test.go b/share/settings/remote_test.go index b2ba39bb..fa71956f 100644 --- a/share/settings/remote_test.go +++ b/share/settings/remote_test.go @@ -78,6 +78,18 @@ func TestRemoteDecode(t *testing.T) { }, "0.0.0.0:53:1.1.1.1:53/udp", }, + { + //uppercase suffix parses to lowercase proto + "1.1.1.1:53/UDP", + Remote{ + LocalPort: "53", + LocalProto: "udp", + RemoteHost: "1.1.1.1", + RemotePort: "53", + RemoteProto: "udp", + }, + "0.0.0.0:53:1.1.1.1:53/udp", + }, { "localhost:5353:1.1.1.1:53/udp", Remote{ diff --git a/share/settings/users.go b/share/settings/users.go index a6f0a093..f0e1153c 100644 --- a/share/settings/users.go +++ b/share/settings/users.go @@ -5,8 +5,12 @@ import ( "errors" "fmt" "os" + "path/filepath" "regexp" + "runtime" + "strings" "sync" + "time" "github.com/fsnotify/fsnotify" "github.com/jpillora/chisel/share/cio" @@ -73,6 +77,14 @@ type UserIndex struct { *cio.Logger *Users configFile string + pinned []*User +} + +// PinUser adds a user which survives configuration file +// reloads (e.g. the --auth user). Pin before LoadUsers. +func (u *UserIndex) PinUser(user *User) { + u.pinned = append(u.pinned, user) + u.AddUser(user) } // NewUserIndex creates a source for users @@ -96,30 +108,96 @@ func (u *UserIndex) LoadUsers(configFile string) error { return nil } -// watchEvents is responsible for watching for updates to the file and reloading +// addWatchEvents is responsible for watching for updates to the file and reloading func (u *UserIndex) addWatchEvents() error { watcher, err := fsnotify.NewWatcher() if err != nil { return err } - if err := watcher.Add(u.configFile); err != nil { + configPath, err := filepath.Abs(u.configFile) + if err != nil { + watcher.Close() + return err + } + //watch the parent directory instead of the file itself, so the watch + //survives editors and orchestrators which replace the file rather + //than write to it (vim tmp+rename, truncate+write, kubernetes + //configmap symlink swaps) + if err := watcher.Add(filepath.Dir(configPath)); err != nil { + watcher.Close() return err } + //track the resolved path to catch symlink retargets + realPath, _ := filepath.EvalSymlinks(configPath) go func() { - for e := range watcher.Events { - if e.Op&fsnotify.Write != fsnotify.Write { - continue - } - if err := u.loadUserIndex(); err != nil { - u.Infof("Failed to reload the users configuration: %s", err) - } else { - u.Debugf("Users configuration successfully reloaded from: %s", u.configFile) + //collapse event bursts (truncate+write, remove+create) into a + //single reload once events settle, so half-written files are + //not loaded + const debounce = 100 * time.Millisecond + timer := time.NewTimer(debounce) + timer.Stop() + //fsnotify does not reliably deliver directory events for + //kubelet-style symlink swaps on every platform (notably + //macOS/kqueue), so reconcile the resolved path periodically + //as a fallback + reconcile := time.NewTicker(time.Second) + defer reconcile.Stop() + for { + select { + case e, ok := <-watcher.Events: + if !ok { + return + } + if u.watchHit(e, configPath, &realPath) { + timer.Reset(debounce) + } + case <-timer.C: + if err := u.loadUserIndex(); err != nil { + u.Infof("Failed to reload the users configuration: %s", err) + } else { + u.Debugf("Users configuration successfully reloaded from: %s", u.configFile) + } + case <-reconcile.C: + //catch symlink retargets that arrived without an event + if current, err := filepath.EvalSymlinks(configPath); err == nil && current != realPath { + realPath = current + timer.Reset(debounce) + } + case err, ok := <-watcher.Errors: + if !ok { + return + } + u.Infof("Error watching the users configuration: %s", err) } } }() return nil } +// watchHit reports whether the event affects the configured file, either +// directly or via a symlink retarget (kubernetes configmaps swap a +// symlinked directory rather than writing to the watched file). +func (u *UserIndex) watchHit(e fsnotify.Event, configPath string, realPath *string) bool { + if e.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename|fsnotify.Remove) == 0 { + return false //ignore chmod + } + if eventPath, err := filepath.Abs(e.Name); err == nil && samePath(eventPath, configPath) { + return true + } + if current, err := filepath.EvalSymlinks(configPath); err == nil && current != *realPath { + *realPath = current + return true + } + return false +} + +func samePath(a, b string) bool { + if runtime.GOOS == "windows" { + return strings.EqualFold(a, b) + } + return a == b +} + // loadUserIndex is responsible for loading the users configuration func (u *UserIndex) loadUserIndex() error { if u.configFile == "" { @@ -148,12 +226,16 @@ func (u *UserIndex) loadUserIndex() error { if err != nil { return errors.New("Invalid address regex") } + if !strings.HasPrefix(r, "^") || !strings.HasSuffix(r, "$") { + u.Infof("authfile: pattern %q (user %s) is unanchored and "+ + "may match unintended addresses; anchor it with ^ and $", r, user.Name) + } user.Addrs = append(user.Addrs, re) } } users = append(users, user) } - //swap - u.Reset(users) + //swap, keeping pinned users (pinned last: they win name clashes) + u.Reset(append(users, u.pinned...)) return nil } diff --git a/share/settings/users_watch_test.go b/share/settings/users_watch_test.go new file mode 100644 index 00000000..6f8a48b1 --- /dev/null +++ b/share/settings/users_watch_test.go @@ -0,0 +1,163 @@ +package settings + +import ( + "os" + "path/filepath" + "regexp" + "runtime" + "testing" + "time" + + "github.com/jpillora/chisel/share/cio" +) + +func writeUser(t *testing.T, path, name string) { + t.Helper() + if err := os.WriteFile(path, []byte(`{"`+name+`:pass": ["*"]}`), 0600); err != nil { + t.Fatal(err) + } +} + +func loadIndex(t *testing.T, cfg, name string) *UserIndex { + t.Helper() + index := NewUserIndex(cio.NewLogger("test")) + if err := index.LoadUsers(cfg); err != nil { + t.Fatal(err) + } + if _, found := index.Get(name); !found { + t.Fatalf("expected initial user %q", name) + } + return index +} + +func waitUser(index *UserIndex, name string) bool { + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + if _, found := index.Get(name); found { + return true + } + time.Sleep(20 * time.Millisecond) + } + return false +} + +// TestWatchWriteInPlace covers plain writes to the watched file. +func TestWatchWriteInPlace(t *testing.T) { + cfg := filepath.Join(t.TempDir(), "users.json") + writeUser(t, cfg, "alice") + index := loadIndex(t, cfg, "alice") + writeUser(t, cfg, "bob") + if !waitUser(index, "bob") { + t.Fatal("write-in-place update not detected") + } +} + +// TestWatchRenameOver covers editors which write a temp file and rename +// it over the original (vim), replacing the inode. +func TestWatchRenameOver(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "users.json") + writeUser(t, cfg, "alice") + index := loadIndex(t, cfg, "alice") + tmp := filepath.Join(dir, "users.json.tmp") + writeUser(t, tmp, "bob") + if err := os.Rename(tmp, cfg); err != nil { + t.Fatal(err) + } + if !waitUser(index, "bob") { + t.Fatal("rename-over update not detected") + } +} + +// TestWatchInvalidJSONKeepsUsers covers half-written or corrupt files: +// the existing users must remain active, and a later valid write must +// still be picked up. +func TestWatchInvalidJSONKeepsUsers(t *testing.T) { + cfg := filepath.Join(t.TempDir(), "users.json") + writeUser(t, cfg, "alice") + index := loadIndex(t, cfg, "alice") + if err := os.WriteFile(cfg, []byte(`{"truncated`), 0600); err != nil { + t.Fatal(err) + } + //wait for the debounced reload attempt to fail + time.Sleep(500 * time.Millisecond) + if _, found := index.Get("alice"); !found { + t.Fatal("users lost after invalid JSON") + } + writeUser(t, cfg, "bob") + if !waitUser(index, "bob") { + t.Fatal("update after invalid JSON not detected") + } +} + +// TestWatchSymlinkSwap covers kubernetes configmap updates: the file is a +// symlink into a versioned directory, and updates atomically replace the +// directory symlink — no event ever fires for the file path itself. +func TestWatchSymlinkSwap(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlinks not supported") + } + base := t.TempDir() + v1 := filepath.Join(base, "v1") + v2 := filepath.Join(base, "v2") + for _, dir := range []string{v1, v2} { + if err := os.Mkdir(dir, 0700); err != nil { + t.Fatal(err) + } + } + writeUser(t, filepath.Join(v1, "users.json"), "alice") + writeUser(t, filepath.Join(v2, "users.json"), "bob") + data := filepath.Join(base, "..data") + if err := os.Symlink("v1", data); err != nil { + t.Fatal(err) + } + cfg := filepath.Join(base, "users.json") + if err := os.Symlink(filepath.Join("..data", "users.json"), cfg); err != nil { + t.Fatal(err) + } + index := loadIndex(t, cfg, "alice") + //atomic swap, kubelet-style: new symlink renamed over the old one + tmp := filepath.Join(base, "..data_tmp") + if err := os.Symlink("v2", tmp); err != nil { + t.Fatal(err) + } + if err := os.Rename(tmp, data); err != nil { + t.Fatal(err) + } + if !waitUser(index, "bob") { + t.Fatal("symlink swap update not detected") + } +} + +// TestWatchPinnedUserSurvivesReload verifies that pinned users (--auth) +// are not dropped when the authfile reloads; previously the first +// reload Reset() deleted them. +func TestWatchPinnedUserSurvivesReload(t *testing.T) { + cfg := filepath.Join(t.TempDir(), "users.json") + writeUser(t, cfg, "alice") + index := NewUserIndex(cio.NewLogger("test")) + index.PinUser(&User{ + Name: "pinned", + Pass: "pw", + Addrs: []*regexp.Regexp{UserAllowAll}, + }) + if err := index.LoadUsers(cfg); err != nil { + t.Fatal(err) + } + for _, n := range []string{"alice", "pinned"} { + if _, found := index.Get(n); !found { + t.Fatalf("expected user %q after load", n) + } + } + //reload: replace alice with bob + writeUser(t, cfg, "bob") + if !waitUser(index, "bob") { + t.Fatal("reload not detected") + } + if _, found := index.Get("pinned"); !found { + t.Fatal("pinned user dropped by authfile reload") + } + if _, found := index.Get("alice"); found { + t.Fatal("alice should have been replaced by the reload") + } +} diff --git a/share/tunnel/tunnel.go b/share/tunnel/tunnel.go index f2da628c..39af78de 100644 --- a/share/tunnel/tunnel.go +++ b/share/tunnel/tunnel.go @@ -95,7 +95,7 @@ func (t *Tunnel) BindSSH(ctx context.Context, c ssh.Conn, reqs <-chan *ssh.Reque } //block until closed go t.handleSSHRequests(reqs) - go t.handleSSHChannels(chans) + go t.handleSSHChannels(ctx, chans) t.Debugf("SSH connected") err := c.Wait() t.Debugf("SSH disconnected") @@ -134,6 +134,13 @@ func (t *Tunnel) getSSH(ctx context.Context) ssh.Conn { } } +//Ready waits for an active ssh connection, returning false +//if none arrives before the context is cancelled or the +//connection wait times out +func (t *Tunnel) Ready(ctx context.Context) bool { + return t.getSSH(ctx) != nil +} + func (t *Tunnel) activatingConnWait() <-chan struct{} { ch := make(chan struct{}) go func() { @@ -156,6 +163,10 @@ func (t *Tunnel) BindRemotes(ctx context.Context, remotes []*settings.Remote) er for i, remote := range remotes { p, err := NewProxy(t.Logger, t, t.proxyCount, remote) if err != nil { + //unbind the proxies which already bound + for _, p := range proxies[:i] { + p.Close() + } return err } proxies[i] = p @@ -177,17 +188,47 @@ func (t *Tunnel) BindRemotes(ctx context.Context, remotes []*settings.Remote) er func (t *Tunnel) keepAliveLoop(sshConn ssh.Conn) { //ping forever + pingTimeout := settings.EnvDuration("PING_TIMEOUT", t.Config.KeepAlive) + ticker := time.NewTicker(t.Config.KeepAlive) + defer ticker.Stop() + //stop the loop when the connection closes + closed := make(chan struct{}) + go func() { + sshConn.Wait() + close(closed) + }() for { - time.Sleep(t.Config.KeepAlive) - _, b, err := sshConn.SendRequest("ping", true, nil) - if err != nil { - break + select { + case <-closed: + return + case <-ticker.C: } - if len(b) > 0 && !bytes.Equal(b, []byte("pong")) { - t.Debugf("strange ping response") + if err := t.keepAlivePing(sshConn, pingTimeout); err != nil { + t.Debugf("ping failed: %s", err) break } } //close ssh connection on abnormal ping sshConn.Close() } + +//keepAlivePing sends an ssh ping request and waits for the reply. +//SendRequest blocks indefinitely on a dead TCP connection (OS sleep/wake, +//NAT timeout, server hard reboot — no RST arrives), so race it against +//a timer and report failure when no reply arrives in time. +func (t *Tunnel) keepAlivePing(sshConn ssh.Conn, timeout time.Duration) error { + errc := make(chan error, 1) + go func() { + _, b, err := sshConn.SendRequest("ping", true, nil) + if err == nil && len(b) > 0 && !bytes.Equal(b, []byte("pong")) { + err = errors.New("strange ping response") + } + errc <- err + }() + select { + case err := <-errc: + return err + case <-time.After(timeout): + return errors.New("ping timeout") + } +} diff --git a/share/tunnel/tunnel_bind_test.go b/share/tunnel/tunnel_bind_test.go new file mode 100644 index 00000000..db561610 --- /dev/null +++ b/share/tunnel/tunnel_bind_test.go @@ -0,0 +1,52 @@ +package tunnel + +import ( + "context" + "net" + "testing" + + "github.com/jpillora/chisel/share/cio" + "github.com/jpillora/chisel/share/settings" +) + +// TestBindRemotesUnbindsOnPartialFailure verifies that when one remote +// fails to bind, the previously bound listeners are released instead of +// staying bound for the process lifetime. +func TestBindRemotesUnbindsOnPartialFailure(t *testing.T) { + //occupy a port so the second remote fails to bind + blocker, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer blocker.Close() + _, occupiedPort, _ := net.SplitHostPort(blocker.Addr().String()) + //pick a free port for the first remote + probe, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + _, freePort, _ := net.SplitHostPort(probe.Addr().String()) + probe.Close() + //bind: first remote succeeds, second fails + tun := New(Config{Logger: cio.NewLogger("test"), Inbound: true}) + remotes := []*settings.Remote{} + for _, s := range []string{ + "127.0.0.1:" + freePort + ":127.0.0.1:1", + "127.0.0.1:" + occupiedPort + ":127.0.0.1:1", + } { + r, err := settings.DecodeRemote(s) + if err != nil { + t.Fatal(err) + } + remotes = append(remotes, r) + } + if err := tun.BindRemotes(context.Background(), remotes); err == nil { + t.Fatal("expected bind error for occupied port") + } + //the first remote's port must be free again + l, err := net.Listen("tcp", "127.0.0.1:"+freePort) + if err != nil { + t.Fatalf("port %s still bound after failed BindRemotes: %v", freePort, err) + } + l.Close() +} diff --git a/share/tunnel/tunnel_in_proxy.go b/share/tunnel/tunnel_in_proxy.go index 007fb0c7..49888b3d 100644 --- a/share/tunnel/tunnel_in_proxy.go +++ b/share/tunnel/tunnel_in_proxy.go @@ -69,6 +69,18 @@ func (p *Proxy) listen() error { return nil } +//Close unbinds the proxy's listeners. proxies which have +//been Run are instead closed by cancelling their context. +func (p *Proxy) Close() error { + if p.tcp != nil { + return p.tcp.Close() + } + if p.udp != nil { + return p.udp.inbound.Close() + } + return nil +} + //Run enables the proxy and blocks while its active, //close the proxy by cancelling the context. func (p *Proxy) Run(ctx context.Context) error { diff --git a/share/tunnel/tunnel_in_proxy_udp.go b/share/tunnel/tunnel_in_proxy_udp.go index 3f3fa8be..c09e4d3e 100644 --- a/share/tunnel/tunnel_in_proxy_udp.go +++ b/share/tunnel/tunnel_in_proxy_udp.go @@ -64,7 +64,7 @@ type udpListener struct { func (u *udpListener) run(ctx context.Context) error { defer u.inbound.Close() - //udp doesnt accept connections, + //udp doesn't accept connections, //udp simply forwards packets //and therefore only needs to listen eg, ctx := errgroup.WithContext(ctx) @@ -88,7 +88,7 @@ func (u *udpListener) runInbound(ctx context.Context) error { //read from inbound udp u.inbound.SetReadDeadline(time.Now().Add(time.Second)) n, addr, err := u.inbound.ReadFromUDP(buff) - if e, ok := err.(net.Error); ok && (e.Timeout() || e.Temporary()) { + if e, ok := err.(net.Error); ok && e.Timeout() { continue } if err != nil { @@ -178,7 +178,7 @@ func (u *udpListener) getUDPChan(ctx context.Context) (*udpChannel, error) { c: rwc, } u.outbound = o - u.Debugf("aquired channel") + u.Debugf("acquired channel") return o, nil } diff --git a/share/tunnel/tunnel_keepalive_test.go b/share/tunnel/tunnel_keepalive_test.go new file mode 100644 index 00000000..47d2909a --- /dev/null +++ b/share/tunnel/tunnel_keepalive_test.go @@ -0,0 +1,99 @@ +package tunnel + +import ( + "net" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/jpillora/chisel/share/cio" + "golang.org/x/crypto/ssh" +) + +//mockSSHConn implements ssh.Conn for keepalive tests. +//When dead, SendRequest blocks until the connection is closed, simulating +//a dead TCP connection where no RST ever arrives (e.g. after OS sleep/wake). +//When healthy, SendRequest replies "pong" immediately. +type mockSSHConn struct { + dead bool + closeOnce sync.Once + closed chan struct{} + pings atomic.Int32 +} + +var _ ssh.Conn = (*mockSSHConn)(nil) + +func newMockSSHConn(dead bool) *mockSSHConn { + return &mockSSHConn{dead: dead, closed: make(chan struct{})} +} + +func (m *mockSSHConn) SendRequest(string, bool, []byte) (bool, []byte, error) { + if m.dead { + <-m.closed + return false, nil, net.ErrClosed + } + select { + case <-m.closed: + return false, nil, net.ErrClosed + default: + m.pings.Add(1) + return true, []byte("pong"), nil + } +} + +func (m *mockSSHConn) OpenChannel(string, []byte) (ssh.Channel, <-chan *ssh.Request, error) { + return nil, nil, net.ErrClosed +} + +func (m *mockSSHConn) Close() error { + m.closeOnce.Do(func() { close(m.closed) }) + return nil +} + +func (m *mockSSHConn) Wait() error { <-m.closed; return nil } +func (m *mockSSHConn) User() string { return "" } +func (m *mockSSHConn) SessionID() []byte { return nil } +func (m *mockSSHConn) ClientVersion() []byte { return nil } +func (m *mockSSHConn) ServerVersion() []byte { return nil } +func (m *mockSSHConn) RemoteAddr() net.Addr { return &net.TCPAddr{} } +func (m *mockSSHConn) LocalAddr() net.Addr { return &net.TCPAddr{} } + +//TestKeepAliveLoopTimeout verifies that keepAliveLoop closes the +//connection when a ping receives no reply within the ping timeout +//(the silently-dead connection scenario). +func TestKeepAliveLoopTimeout(t *testing.T) { + conn := newMockSSHConn(true) + tun := New(Config{ + Logger: cio.NewLogger("test"), + KeepAlive: 50 * time.Millisecond, + }) + go tun.keepAliveLoop(conn) + select { + case <-conn.closed: + //dead connection detected and closed + case <-time.After(2 * time.Second): + t.Fatal("keepAliveLoop did not close dead connection") + } +} + +//TestKeepAliveLoopHealthy verifies that keepAliveLoop keeps pinging a +//responsive connection without closing it. +func TestKeepAliveLoopHealthy(t *testing.T) { + conn := newMockSSHConn(false) + tun := New(Config{ + Logger: cio.NewLogger("test"), + KeepAlive: 20 * time.Millisecond, + }) + go tun.keepAliveLoop(conn) + time.Sleep(150 * time.Millisecond) + select { + case <-conn.closed: + t.Fatal("keepAliveLoop closed a healthy connection") + default: + } + if n := conn.pings.Load(); n < 2 { + t.Fatalf("expected at least 2 pings, got %d", n) + } + conn.Close() +} diff --git a/share/tunnel/tunnel_out_ssh.go b/share/tunnel/tunnel_out_ssh.go index 3b380fa8..a54f3e10 100644 --- a/share/tunnel/tunnel_out_ssh.go +++ b/share/tunnel/tunnel_out_ssh.go @@ -1,10 +1,12 @@ package tunnel import ( + "context" "fmt" "io" "net" "strings" + "time" "github.com/jpillora/chisel/share/cio" "github.com/jpillora/chisel/share/cnet" @@ -24,13 +26,13 @@ func (t *Tunnel) handleSSHRequests(reqs <-chan *ssh.Request) { } } -func (t *Tunnel) handleSSHChannels(chans <-chan ssh.NewChannel) { +func (t *Tunnel) handleSSHChannels(ctx context.Context, chans <-chan ssh.NewChannel) { for ch := range chans { - go t.handleSSHChannel(ch) + go t.handleSSHChannel(ctx, ch) } } -func (t *Tunnel) handleSSHChannel(ch ssh.NewChannel) { +func (t *Tunnel) handleSSHChannel(ctx context.Context, ch ssh.NewChannel) { if !t.Config.Outbound { t.Debugf("Denied outbound connection") ch.Reject(ssh.Prohibited, "Denied outbound connection") @@ -47,20 +49,38 @@ func (t *Tunnel) handleSSHChannel(ch ssh.NewChannel) { return } //check ACL against the actual requested destination - //(hostPort == "socks" for socks channels, so socks is gated too, - //symmetric with the config-time UserAddr() check in server_handler.go) + //(socks channels are checked against the well-known token "socks") if t.Config.ACL != nil && !t.Config.ACL(hostPort) { - t.Debugf("Denied connection to %s (ACL)", hostPort) + //info-level: post-1.12 the most likely cause is an authfile + //missing a "socks" grant, and operators need to see that + //without -v + t.Infof("Denied connection to %s (ACL)", hostPort) ch.Reject(ssh.Prohibited, "access denied") return } + //tcp: dial the target before accepting the channel, so dial + //failures propagate to the inbound side instead of presenting + //a successful connection to a dead target + var dst net.Conn + if !socks && !udp { + d := net.Dialer{Timeout: settings.EnvDuration("DIAL_TIMEOUT", 30*time.Second)} + c, err := d.DialContext(ctx, "tcp", hostPort) + if err != nil { + t.Debugf("Failed to dial %s: %s", hostPort, err) + ch.Reject(ssh.ConnectionFailed, err.Error()) + return + } + dst = c + } sshChan, reqs, err := ch.Accept() if err != nil { t.Debugf("Failed to accept stream: %s", err) + if dst != nil { + dst.Close() + } return } stream := io.ReadWriteCloser(sshChan) - //cnet.MeterRWC(t.Logger.Fork("sshchan"), sshChan) defer stream.Close() go ssh.DiscardRequests(reqs) l := t.Logger.Fork("conn#%d", t.connStats.New()) @@ -72,7 +92,7 @@ func (t *Tunnel) handleSSHChannel(ch ssh.NewChannel) { } else if udp { err = t.handleUDP(l, stream, hostPort) } else { - err = t.handleTCP(l, stream, hostPort) + err = t.handleTCP(l, stream, dst) } t.connStats.Close() errmsg := "" @@ -86,11 +106,7 @@ func (t *Tunnel) handleSocks(src io.ReadWriteCloser) error { return t.socksServer.ServeConn(cnet.NewRWCConn(src)) } -func (t *Tunnel) handleTCP(l *cio.Logger, src io.ReadWriteCloser, hostPort string) error { - dst, err := net.Dial("tcp", hostPort) - if err != nil { - return err - } +func (t *Tunnel) handleTCP(l *cio.Logger, src io.ReadWriteCloser, dst net.Conn) error { s, r := cio.Pipe(src, dst) l.Debugf("sent %s received %s", sizestr.ToString(s), sizestr.ToString(r)) return nil diff --git a/share/tunnel/tunnel_out_ssh_udp.go b/share/tunnel/tunnel_out_ssh_udp.go index d3c4c62f..de8a4600 100644 --- a/share/tunnel/tunnel_out_ssh_udp.go +++ b/share/tunnel/tunnel_out_ssh_udp.go @@ -28,6 +28,8 @@ func (t *Tunnel) handleUDP(l *cio.Logger, rwc io.ReadWriteCloser, hostPort strin }, udpConns: conns, maxMTU: settings.EnvInt("UDP_MAX_SIZE", 9012), + maxConns: settings.EnvInt("UDP_MAX_CONNS", 100), + deadline: settings.EnvDuration("UDP_DEADLINE", 15*time.Second), } h.Debugf("UDP max size: %d bytes", h.maxMTU) for { @@ -43,48 +45,72 @@ type udpHandler struct { hostPort string *udpChannel *udpConns - maxMTU int + maxMTU int + maxConns int + deadline time.Duration + lastSweep time.Time } func (h *udpHandler) handleWrite(p *udpPacket) error { if err := h.r.Decode(&p); err != nil { return err } + //remove idle write-only connections, + //making room for new flows to become readable again + h.sweepWriteOnly() //dial now, we know we must write conn, exists, err := h.udpConns.dial(p.Src, h.hostPort) if err != nil { return err } //however, we dont know if we must read... - //spawn up to go-routines to wait - //for a reply. - //TODO configurable - //TODO++ dont use go-routines, switch to pollable - // array of listeners where all listeners are - // sweeped periodically, removing the idle ones - const maxConns = 100 + //spawn up to (UDP_MAX_CONNS) go-routines + //to wait for replies; flows beyond the cap are write-only + //(replies are dropped) and are swept once idle if !exists { - if h.udpConns.len() <= maxConns { + if h.udpConns.len() <= h.maxConns { go h.handleRead(p, conn) } else { - h.Debugf("exceeded max udp connections (%d)", maxConns) + conn.writeOnly = true + h.Debugf("exceeded max udp connections (%d), flow %s is write-only", h.maxConns, p.Src) } } - _, err = conn.Write(p.Payload) - if err != nil { - return err + if _, err := conn.Write(p.Payload); err != nil { + //write failures (broken flow, ICMP unreachable, conn + //closed by its reader) drop the flow, not the channel + h.Debugf("write error (%s): %s", p.Src, err) + h.udpConns.remove(conn.id) + conn.Close() + return nil + } + if conn.writeOnly { + conn.lastWrite = time.Now() } return nil } +//sweepWriteOnly closes and removes write-only connections which have +//been idle for longer than the read deadline. it runs in the write +//goroutine (at most once per second) so it cannot race in-flight writes. +func (h *udpHandler) sweepWriteOnly() { + now := time.Now() + if now.Sub(h.lastSweep) < time.Second { + return + } + h.lastSweep = now + h.udpConns.sweepWriteOnly(now.Add(-h.deadline)) +} + func (h *udpHandler) handleRead(p *udpPacket, conn *udpConn) { - //ensure connection is cleaned up - defer h.udpConns.remove(conn.id) + //ensure connection is cleaned up and closed + defer func() { + h.udpConns.remove(conn.id) + conn.Close() + }() buff := make([]byte, h.maxMTU) for { - //response must arrive within 15 seconds - deadline := settings.EnvDuration("UDP_DEADLINE", 15*time.Second) - conn.SetReadDeadline(time.Now().Add(deadline)) + //response must arrive within the deadline + conn.SetReadDeadline(time.Now().Add(h.deadline)) //read response n, err := conn.Read(buff) if err != nil { @@ -149,7 +175,24 @@ func (cs *udpConns) closeAll() { cs.Unlock() } +//sweepWriteOnly closes and removes write-only connections +//whose last write is older than the given time +func (cs *udpConns) sweepWriteOnly(olderThan time.Time) { + cs.Lock() + defer cs.Unlock() + for id, conn := range cs.m { + if conn.writeOnly && conn.lastWrite.Before(olderThan) { + conn.Close() + delete(cs.m, id) + } + } +} + type udpConn struct { id string net.Conn + //write-only flows (over the conn cap) have no read goroutine; + //these fields are only touched by the single write goroutine + writeOnly bool + lastWrite time.Time } diff --git a/share/tunnel/tunnel_out_ssh_udp_test.go b/share/tunnel/tunnel_out_ssh_udp_test.go new file mode 100644 index 00000000..8e5895a2 --- /dev/null +++ b/share/tunnel/tunnel_out_ssh_udp_test.go @@ -0,0 +1,37 @@ +package tunnel + +import ( + "net" + "testing" + "time" + + "github.com/jpillora/chisel/share/cio" +) + +// TestUDPSweepWriteOnly verifies that only stale write-only connections +// are swept; fresh write-only conns and reader-owned conns must remain. +func TestUDPSweepWriteOnly(t *testing.T) { + pipe := func() net.Conn { + a, _ := net.Pipe() + return a + } + now := time.Now() + cs := &udpConns{ + Logger: cio.NewLogger("test"), + m: map[string]*udpConn{ + "stale": {id: "stale", Conn: pipe(), writeOnly: true, lastWrite: now.Add(-time.Minute)}, + "fresh": {id: "fresh", Conn: pipe(), writeOnly: true, lastWrite: now}, + "reader": {id: "reader", Conn: pipe()}, + }, + } + cs.sweepWriteOnly(now.Add(-30 * time.Second)) + if _, ok := cs.m["stale"]; ok { + t.Fatal("stale write-only conn was not swept") + } + if _, ok := cs.m["fresh"]; !ok { + t.Fatal("fresh write-only conn was wrongly swept") + } + if _, ok := cs.m["reader"]; !ok { + t.Fatal("reader conn was wrongly swept") + } +} diff --git a/share/version.go b/share/version.go index f503f85c..9383c9ee 100644 --- a/share/version.go +++ b/share/version.go @@ -1,9 +1,36 @@ package chshare +import ( + "runtime/debug" + "strings" +) + //ProtocolVersion of chisel. When backwards //incompatible changes are made, this will //be incremented to signify a protocol //mismatch. var ProtocolVersion = "chisel-v3" +//BuildVersion is set at build time via ldflags, +//and otherwise falls back to the go module version +//embedded by `go install pkg@version` (see init) var BuildVersion = "0.0.0-src" + +func init() { + if info, ok := debug.ReadBuildInfo(); ok { + BuildVersion = fallbackVersion(BuildVersion, info) + } +} + +//fallbackVersion returns the go module version when the build +//version was not stamped via ldflags +func fallbackVersion(current string, info *debug.BuildInfo) string { + if current != "0.0.0-src" { + return current //stamped via ldflags + } + v := info.Main.Version + if v == "" || v == "(devel)" { + return current //source build, no module version + } + return strings.TrimPrefix(v, "v") +} diff --git a/share/version_test.go b/share/version_test.go new file mode 100644 index 00000000..f50f87e0 --- /dev/null +++ b/share/version_test.go @@ -0,0 +1,29 @@ +package chshare + +import ( + "runtime/debug" + "testing" +) + +func TestFallbackVersion(t *testing.T) { + infoWith := func(v string) *debug.BuildInfo { + i := &debug.BuildInfo{} + i.Main.Version = v + return i + } + for _, tc := range []struct { + current, module, want string + }{ + //module install: use the embedded module version + {"0.0.0-src", "v1.11.6", "1.11.6"}, + //source build: no module version available + {"0.0.0-src", "(devel)", "0.0.0-src"}, + {"0.0.0-src", "", "0.0.0-src"}, + //ldflags-stamped builds always win + {"1.12.0", "v1.11.6", "1.12.0"}, + } { + if got := fallbackVersion(tc.current, infoWith(tc.module)); got != tc.want { + t.Errorf("fallbackVersion(%q, %q) = %q, want %q", tc.current, tc.module, got, tc.want) + } + } +} diff --git a/test/e2e/acl_reload_test.go b/test/e2e/acl_reload_test.go new file mode 100644 index 00000000..f1615e7d --- /dev/null +++ b/test/e2e/acl_reload_test.go @@ -0,0 +1,76 @@ +package e2e_test + +import ( + "fmt" + "io" + "net" + "testing" + + chserver "github.com/jpillora/chisel/server" + "github.com/jpillora/chisel/share/settings" + "golang.org/x/crypto/ssh" +) + +// TestACLAppliesAfterUserRemoval verifies the user is re-resolved per +// channel: once removed (e.g. by an authfile reload), a still-connected +// client is denied new tunnels. Established semantics are documented: +// the session itself is not killed. +func TestACLAppliesAfterUserRemoval(t *testing.T) { + targetPort := availablePort() + tl, err := net.Listen("tcp", "127.0.0.1:"+targetPort) + if err != nil { + t.Fatal(err) + } + defer tl.Close() + go func() { + for { + c, err := tl.Accept() + if err != nil { + return + } + c.Write([]byte("OK")) + c.Close() + } + }() + s, err := chserver.NewServer(&chserver.Config{KeySeed: "acl-reload-test"}) + if err != nil { + t.Fatal(err) + } + s.Debug = debug + if err := s.AddUser("user", "pass", fmt.Sprintf(`^127\.0\.0\.1:%s$`, targetPort)); 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() + r, err := settings.DecodeRemote(fmt.Sprintf("0.0.0.0:%s:127.0.0.1:%s", targetPort, targetPort)) + if err != nil { + t.Fatal(err) + } + sendConfig(t, sc, []*settings.Remote{r}) + target := net.JoinHostPort("127.0.0.1", targetPort) + //while the user exists, channels open fine + ch, reqs, err := sc.OpenChannel("chisel", []byte(target)) + if err != nil { + t.Fatalf("channel rejected while user exists: %v", err) + } + go ssh.DiscardRequests(reqs) + b := make([]byte, 2) + if _, err := io.ReadFull(ch, b); err != nil || string(b) != "OK" { + t.Fatalf("read through tunnel: %q %v", b, err) + } + ch.Close() + //remove the user (as an authfile reload would) + s.DeleteUser("user") + //the connected client's next channel must be denied + ch2, _, err := sc.OpenChannel("chisel", []byte(target)) + if err == nil { + ch2.Close() + t.Fatal("channel accepted after user removal") + } + t.Logf("channel correctly rejected after removal: %v", err) +} diff --git a/test/e2e/acl_socks_test.go b/test/e2e/acl_socks_test.go new file mode 100644 index 00000000..403155ce --- /dev/null +++ b/test/e2e/acl_socks_test.go @@ -0,0 +1,107 @@ +package e2e_test + +import ( + "encoding/json" + "io" + "testing" + + chserver "github.com/jpillora/chisel/server" + "github.com/jpillora/chisel/share/settings" + "golang.org/x/crypto/ssh" +) + +// startSocksACLServer starts a socks5-enabled chisel server with a +// single user limited to the given address whitelist. +func startSocksACLServer(t *testing.T, seed string, addrs ...string) string { + t.Helper() + s, err := chserver.NewServer(&chserver.Config{ + KeySeed: seed, + Socks5: true, + }) + if err != nil { + t.Fatal(err) + } + s.Debug = debug + if err := s.AddUser("user", "pass", addrs...); err != nil { + t.Fatal(err) + } + port := availablePort() + if err := s.Start("127.0.0.1", port); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { s.Close() }) + return "127.0.0.1:" + port +} + +// TestSocksChannelDenied verifies that a modified client cannot open a +// "socks" channel directly (without declaring a socks remote) when the +// user's ACL has no entry matching "socks". +func TestSocksChannelDenied(t *testing.T) { + addr := startSocksACLServer(t, "socks-acl-denied", `^127\.0\.0\.1:80$`) + sc, _, _ := dialChiselSSH(t, addr, "user", "pass") + defer sc.Close() + sendConfig(t, sc, nil) //declare no remotes + ch, _, err := sc.OpenChannel("chisel", []byte("socks")) + if err == nil { + ch.Close() + t.Fatal("socks channel accepted despite ACL without a socks entry") + } + t.Logf("socks channel correctly rejected: %v", err) +} + +// TestSocksChannelAllowed verifies that a user with a "socks" ACL entry +// can use the SOCKS5 proxy, via a real SOCKS5 handshake. +func TestSocksChannelAllowed(t *testing.T) { + addr := startSocksACLServer(t, "socks-acl-allowed", `^socks$`) + sc, _, _ := dialChiselSSH(t, addr, "user", "pass") + defer sc.Close() + sendConfig(t, sc, nil) + ch, reqs, err := sc.OpenChannel("chisel", []byte("socks")) + if err != nil { + t.Fatalf("socks channel rejected for authorized user: %v", err) + } + go ssh.DiscardRequests(reqs) + defer ch.Close() + //SOCKS5 greeting: version 5, 1 method, no-auth + if _, err := ch.Write([]byte{0x05, 0x01, 0x00}); err != nil { + t.Fatal(err) + } + reply := make([]byte, 2) + if _, err := io.ReadFull(ch, reply); err != nil { + t.Fatal(err) + } + if reply[0] != 0x05 || reply[1] != 0x00 { + t.Fatalf("unexpected SOCKS5 reply: %v", reply) + } + t.Log("SOCKS5 handshake succeeded for authorized user") +} + +// TestSocksRemoteConfigACL verifies that declared socks remotes are +// checked against the "socks" token at config time (Remote.UserAddr). +func TestSocksRemoteConfigACL(t *testing.T) { + remote, err := settings.DecodeRemote("5000:socks") + if err != nil { + t.Fatal(err) + } + cfg, err := json.Marshal(settings.Config{Version: "0", Remotes: []*settings.Remote{remote}}) + if err != nil { + t.Fatal(err) + } + //denied without a socks entry + addr := startSocksACLServer(t, "socks-acl-config-denied", `^127\.0\.0\.1:80$`) + sc, _, _ := dialChiselSSH(t, addr, "user", "pass") + defer sc.Close() + ok, reply, err := sc.SendRequest("config", true, cfg) + if err != nil { + t.Fatal(err) + } + if ok { + t.Fatal("socks remote accepted at config time without a socks ACL entry") + } + t.Logf("socks remote correctly rejected at config time: %s", reply) + //allowed with a socks entry + addr2 := startSocksACLServer(t, "socks-acl-config-allowed", `^socks$`) + sc2, _, _ := dialChiselSSH(t, addr2, "user", "pass") + defer sc2.Close() + sendConfig(t, sc2, []*settings.Remote{remote}) +} diff --git a/test/e2e/auth_test.go b/test/e2e/auth_test.go index cd758c5d..540d9bac 100644 --- a/test/e2e/auth_test.go +++ b/test/e2e/auth_test.go @@ -19,7 +19,10 @@ func TestAuth(t *testing.T) { teardown := simpleSetup(t, &chserver.Config{ KeySeed: "foobar", - Auth: "../bench/userfile", + //note: this was the path "../bench/userfile", which + //silently disabled auth entirely (no colon in the + //string); invalid auth strings are now a fatal error + Auth: "foo:bar", }, &chclient.Config{ Remotes: []string{ diff --git a/test/e2e/halfclose_test.go b/test/e2e/halfclose_test.go new file mode 100644 index 00000000..c2980292 --- /dev/null +++ b/test/e2e/halfclose_test.go @@ -0,0 +1,129 @@ +package e2e_test + +import ( + "fmt" + "io" + "net" + "strings" + "testing" + "time" + + chclient "github.com/jpillora/chisel/client" + chserver "github.com/jpillora/chisel/server" + "github.com/jpillora/chisel/share/settings" +) + +// TestHalfCloseThroughTunnel verifies TCP half-close semantics across +// the tunnel: the client sends a request, closes its write side, and +// the server only replies after reading EOF. With the old Pipe, the +// CloseWrite became a full teardown and the reply was lost. +func TestHalfCloseThroughTunnel(t *testing.T) { + //target which reads until EOF, then replies, then closes + targetPort := availablePort() + tl, err := net.Listen("tcp", "127.0.0.1:"+targetPort) + if err != nil { + t.Fatal(err) + } + defer tl.Close() + go func() { + for { + c, err := tl.Accept() + if err != nil { + return + } + go func(c net.Conn) { + defer c.Close() + b, _ := io.ReadAll(c) + c.Write(append(b, '!')) + }(c) + } + }() + //chisel client+server + tunnelPort := availablePort() + teardown := simpleSetup(t, + &chserver.Config{}, + &chclient.Config{ + Remotes: []string{tunnelPort + ":127.0.0.1:" + targetPort}, + }, + ) + defer teardown() + //send, half-close, then read the reply until EOF + conn, err := net.Dial("tcp", "127.0.0.1:"+tunnelPort) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + conn.SetDeadline(time.Now().Add(5 * time.Second)) + if _, err := conn.Write([]byte("hello")); err != nil { + t.Fatal(err) + } + if err := conn.(*net.TCPConn).CloseWrite(); err != nil { + t.Fatal(err) + } + b, err := io.ReadAll(conn) + if err != nil { + t.Fatalf("read reply: %v", err) + } + if got := string(b); got != "hello!" { + t.Fatalf("got %q, want %q", got, "hello!") + } +} + +// TestDialFailureRejectsChannel verifies that when the exit side cannot +// reach the target, the ssh channel is rejected (instead of accepted +// and silently torn down) so the failure propagates to the client. +func TestDialFailureRejectsChannel(t *testing.T) { + //allocate a port and leave it closed + deadPort := availablePort() + s, err := chserver.NewServer(&chserver.Config{KeySeed: "dial-fail-test"}) + if err != nil { + t.Fatal(err) + } + s.Debug = debug + 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, "", "") + defer sc.Close() + r, err := settings.DecodeRemote(fmt.Sprintf("0.0.0.0:%s:127.0.0.1:%s", deadPort, deadPort)) + if err != nil { + t.Fatal(err) + } + sendConfig(t, sc, []*settings.Remote{r}) + ch, _, err := sc.OpenChannel("chisel", []byte("127.0.0.1:"+deadPort)) + if err == nil { + ch.Close() + t.Fatal("channel to unreachable target was accepted") + } + if !strings.Contains(err.Error(), "refused") { + t.Logf("rejection reason: %v", err) + } +} + +// TestDialFailureClosesLocalConn verifies the inbound side: a client +// connecting to the local proxy port for a dead target must see the +// connection close promptly with no data, not hang. +func TestDialFailureClosesLocalConn(t *testing.T) { + deadPort := availablePort() + tunnelPort := availablePort() + teardown := simpleSetup(t, + &chserver.Config{}, + &chclient.Config{ + Remotes: []string{tunnelPort + ":127.0.0.1:" + deadPort}, + }, + ) + defer teardown() + conn, err := net.Dial("tcp", "127.0.0.1:"+tunnelPort) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + b := make([]byte, 8) + n, err := conn.Read(b) + if err != io.EOF { + t.Fatalf("expected prompt EOF, got n=%d err=%v", n, err) + } +} diff --git a/test/e2e/keepalive_test.go b/test/e2e/keepalive_test.go new file mode 100644 index 00000000..adc67bdf --- /dev/null +++ b/test/e2e/keepalive_test.go @@ -0,0 +1,195 @@ +package e2e_test + +import ( + "context" + "fmt" + "io" + "net" + "net/http" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + chclient "github.com/jpillora/chisel/client" + chserver "github.com/jpillora/chisel/server" +) + +//freezableProxy is a TCP proxy that can silently blackhole its current +//connections — data is discarded without closing them, so neither end +//receives a RST or FIN. This simulates a dead path after OS sleep/wake +//or a NAT timeout. Connections made after FreezeExisting are unaffected, +//so reconnect attempts always succeed. +type freezableProxy struct { + listener net.Listener + target string + mut sync.Mutex + frozen []*atomic.Bool //one flag per active connection pair +} + +func newFreezableProxy(target string) (*freezableProxy, error) { + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + return nil, err + } + p := &freezableProxy{listener: l, target: target} + go p.serve() + return p, nil +} + +func (p *freezableProxy) Addr() string { return p.listener.Addr().String() } + +func (p *freezableProxy) Close() { p.listener.Close() } + +//FreezeExisting blackholes all current connections. +func (p *freezableProxy) FreezeExisting() { + p.mut.Lock() + defer p.mut.Unlock() + for _, f := range p.frozen { + f.Store(true) + } + p.frozen = nil +} + +func (p *freezableProxy) serve() { + for { + src, err := p.listener.Accept() + if err != nil { + return + } + dst, err := net.Dial("tcp", p.target) + if err != nil { + src.Close() + continue + } + frozen := &atomic.Bool{} + p.mut.Lock() + p.frozen = append(p.frozen, frozen) + p.mut.Unlock() + go p.pipe(src, dst, frozen) + go p.pipe(dst, src, frozen) + } +} + +func (p *freezableProxy) pipe(dst, src net.Conn, frozen *atomic.Bool) { + defer src.Close() + defer dst.Close() + buf := make([]byte, 32*1024) + for { + n, err := src.Read(buf) + if err != nil { + return + } + if frozen.Load() { + continue //discard: the connection is a black hole + } + if _, err := dst.Write(buf[:n]); err != nil { + return + } + } +} + +//TestKeepAliveReconnect verifies that when the network path goes silent +//(packets dropped without RST/FIN, as after OS sleep/wake) the keepalive +//ping timeout closes the dead connection and the client reconnects, +//restoring port forwarding. Without the ping timeout, SendRequest blocks +//until the kernel retransmit timeout (15+ minutes) and the tunnel hangs. +func TestKeepAliveReconnect(t *testing.T) { + const keepalive = 200 * time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + //fileserver (the tunnelled endpoint) + filePort := availablePort() + fileAddr := "127.0.0.1:" + filePort + f := http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + w.Write(append(b, '!')) + }), + } + fl, err := net.Listen("tcp", fileAddr) + if err != nil { + t.Fatal(err) + } + go f.Serve(fl) + defer f.Close() + //chisel server + server, err := chserver.NewServer(&chserver.Config{}) + if err != nil { + t.Fatal(err) + } + server.Debug = debug + serverPort := availablePort() + if err := server.StartContext(ctx, "127.0.0.1", serverPort); err != nil { + t.Fatal(err) + } + defer func() { + cancel() + server.Wait() + }() + //freezable proxy between client and server + proxy, err := newFreezableProxy("127.0.0.1:" + serverPort) + if err != nil { + t.Fatal(err) + } + defer proxy.Close() + //chisel client connects via the proxy so the link can be killed + tunnelPort := availablePort() + client, err := chclient.NewClient(&chclient.Config{ + Fingerprint: server.GetFingerprint(), + Server: "http://" + proxy.Addr(), + Remotes: []string{tunnelPort + ":" + fileAddr}, + KeepAlive: keepalive, + MaxRetryCount: -1, + MaxRetryInterval: time.Second, + }) + if err != nil { + t.Fatal(err) + } + client.Debug = debug + if err := client.Start(ctx); err != nil { + t.Fatal(err) + } + defer func() { + cancel() + client.Wait() + }() + //tunnel must work before the link dies + url := "http://localhost:" + tunnelPort + if err := waitPost(url, "hello", "hello!", 5*time.Second); err != nil { + t.Fatalf("pre-freeze: %v", err) + } + //silently kill the established connection — the in-flight ping can + //never be answered, so only the ping timeout can detect the failure + proxy.FreezeExisting() + //keepalive should close the dead connection and reconnect + if err := waitPost(url, "world", "world!", 10*time.Second); err != nil { + t.Fatalf("tunnel did not recover after dead link: %v", err) + } +} + +//waitPost polls the tunnel until it returns the expected response. +func waitPost(url, body, want string, timeout time.Duration) error { + c := http.Client{Timeout: 2 * time.Second} + deadline := time.Now().Add(timeout) + for { + var got string + resp, err := c.Post(url, "text/plain", strings.NewReader(body)) + if err == nil { + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + got = string(b) + if got == want { + return nil + } + } + if time.Now().After(deadline) { + if err != nil { + return fmt.Errorf("timed out: %w", err) + } + return fmt.Errorf("timed out: got %q, want %q", got, want) + } + time.Sleep(100 * time.Millisecond) + } +} diff --git a/test/e2e/preconfig_test.go b/test/e2e/preconfig_test.go new file mode 100644 index 00000000..6c702279 --- /dev/null +++ b/test/e2e/preconfig_test.go @@ -0,0 +1,81 @@ +package e2e_test + +import ( + "bytes" + "log" + "os" + "strings" + "sync" + "testing" + "time" + + chserver "github.com/jpillora/chisel/server" + "github.com/jpillora/chisel/share/settings" +) + +// syncBuffer is a goroutine-safe writer for capturing log output. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// TestConnCloseBeforeConfig verifies the server survives a client which +// completes the SSH handshake but drops the connection before sending the +// "config" request (issue #608 bug 2). The closed request channel yields a +// nil *ssh.Request; without a nil guard the handler panics (recovered by +// net/http, which logs "http: panic serving" via the global log package). +func TestConnCloseBeforeConfig(t *testing.T) { + // capture the global logger, where net/http reports recovered handler panics + buf := &syncBuffer{} + log.SetOutput(buf) + defer log.SetOutput(os.Stderr) + + s, err := chserver.NewServer(&chserver.Config{ + KeySeed: "preconfig-test", + }) + if err != nil { + t.Fatal(err) + } + s.Debug = debug + port := availablePort() + if err := s.Start("127.0.0.1", port); err != nil { + t.Fatal(err) + } + defer s.Close() + serverAddr := "127.0.0.1:" + port + + // handshake then hang up without sending the config request + sc, _, _ := dialChiselSSH(t, serverAddr, "user", "pass") + sc.Close() + + // give the handler time to observe the closed request channel, + // failing fast if the panic appears + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if strings.Contains(buf.String(), "panic") { + t.Fatalf("server panicked on pre-config close:\n%s", buf.String()) + } + time.Sleep(50 * time.Millisecond) + } + + // server must still serve a well-behaved client afterwards + sc2, _, _ := dialChiselSSH(t, serverAddr, "user", "pass") + defer sc2.Close() + r, err := settings.DecodeRemote("0.0.0.0:" + availablePort() + ":127.0.0.1:80") + if err != nil { + t.Fatal(err) + } + sendConfig(t, sc2, []*settings.Remote{r}) +} diff --git a/test/e2e/setup_test.go b/test/e2e/setup_test.go index c1611bfb..b6c3e5a6 100644 --- a/test/e2e/setup_test.go +++ b/test/e2e/setup_test.go @@ -12,6 +12,7 @@ import ( chclient "github.com/jpillora/chisel/client" chserver "github.com/jpillora/chisel/server" + "github.com/jpillora/chisel/share/settings" ) const debug = true @@ -110,11 +111,31 @@ func (tl *testLayout) setup(t *testing.T) (server *chserver.Server, client *chcl // t.Fatalf("goroutines left %d", d) // } } - //wait a bit... - //TODO: client signal API, similar to os.Notify(signal) - // wait for client setup - time.Sleep(50 * time.Millisecond) - //ready + //wait for the client to establish its server connection. + //negative tests may never connect (e.g. rejected client certs): + //their own assertions decide the outcome + if !client.Ready(ctx) { + t.Log("setup: client did not become ready") + return server, client, teardown + } + //wait for tunnel listeners to accept: forward remotes bind on + //the client at Start, but reverse remotes are bound on the + //server after the handshake completes + for _, s := range tl.client.Remotes { + r, err := settings.DecodeRemote(s) + if err != nil || r.Stdio || r.LocalProto != "tcp" { + continue + } + addr := "127.0.0.1:" + r.LocalPort + for i := 0; i < 100; i++ { + conn, err := net.DialTimeout("tcp", addr, 100*time.Millisecond) + if err == nil { + conn.Close() + break + } + time.Sleep(20 * time.Millisecond) + } + } return server, client, teardown } diff --git a/test/e2e/udp_cap_test.go b/test/e2e/udp_cap_test.go new file mode 100644 index 00000000..4cd3350c --- /dev/null +++ b/test/e2e/udp_cap_test.go @@ -0,0 +1,84 @@ +package e2e_test + +import ( + "net" + "testing" + "time" + + chclient "github.com/jpillora/chisel/client" + chserver "github.com/jpillora/chisel/server" +) + +// TestUDPConnCapRecovery verifies the UDP conn cap behavior: flows over +// the cap (UDP_MAX_CONNS) are write-only, and — unlike the old behavior +// where over-cap flows were never removed and permanently broke all new +// flows — they are swept once idle, so later flows get read slots again. +func TestUDPConnCapRecovery(t *testing.T) { + t.Setenv("CHISEL_UDP_MAX_CONNS", "1") + t.Setenv("CHISEL_UDP_DEADLINE", "300ms") + //udp echo server + echoPort := availableUDPPort() + a, _ := net.ResolveUDPAddr("udp", ":"+echoPort) + l, err := net.ListenUDP("udp", a) + if err != nil { + t.Fatal(err) + } + defer l.Close() + go func() { + b := make([]byte, 128) + for { + n, addr, err := l.ReadFrom(b) + if err != nil { + return + } + l.WriteTo(b[:n], addr) + } + }() + //chisel client+server + inboundPort := availableUDPPort() + teardown := simpleSetup(t, + &chserver.Config{}, + &chclient.Config{ + Remotes: []string{ + inboundPort + ":" + echoPort + "/udp", + }, + }, + ) + defer teardown() + //echo sends one packet from a fresh source port (= a new flow on + //the exit node) and waits for the reply + echo := func(msg string) (string, error) { + conn, err := net.Dial("udp4", "localhost:"+inboundPort) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if _, err := conn.Write([]byte(msg)); err != nil { + t.Fatal(err) + } + b := make([]byte, 128) + conn.SetReadDeadline(time.Now().Add(time.Second)) + n, err := conn.Read(b) + if err != nil { + return "", err + } + return string(b[:n]), nil + } + //flow 1 takes the only read slot + if got, err := echo("one"); err != nil || got != "one" { + t.Fatalf("flow 1 failed: %q %v", got, err) + } + //flow 2 is over the cap: write-only, so its reply must be dropped + if got, err := echo("two"); err == nil { + t.Fatalf("flow 2 got a reply (%q) despite the conn cap", got) + } + //wait past the read deadline (frees flow 1) and the sweep interval + //(allows flow 2 to be swept on the next packet) + time.Sleep(1200 * time.Millisecond) + //flow 3's packet sweeps the idle write-only flow 2 and gets a + //read slot — before the sweep existed, every flow from here on + //was permanently write-only + if got, err := echo("three"); err != nil || got != "three" { + t.Fatalf("flow 3 did not recover after sweep: %q %v", got, err) + } +} diff --git a/test/e2e/ws_limit_test.go b/test/e2e/ws_limit_test.go new file mode 100644 index 00000000..13b5f123 --- /dev/null +++ b/test/e2e/ws_limit_test.go @@ -0,0 +1,54 @@ +package e2e_test + +import ( + "bytes" + "net" + "net/http" + "testing" + "time" + + chserver "github.com/jpillora/chisel/server" + + "github.com/gorilla/websocket" +) + +// TestPreAuthOversizedFrame verifies that an unauthenticated peer +// sending a huge websocket frame gets disconnected by the read limit +// instead of the server buffering the frame into memory. +func TestPreAuthOversizedFrame(t *testing.T) { + s, err := chserver.NewServer(&chserver.Config{KeySeed: "ws-limit-test"}) + if err != nil { + t.Fatal(err) + } + s.Debug = debug + port := availablePort() + if err := s.Start("127.0.0.1", port); err != nil { + t.Fatal(err) + } + defer s.Close() + //raw websocket connection with the chisel subprotocol, no ssh + ws, _, err := (&websocket.Dialer{ + HandshakeTimeout: 5 * time.Second, + Subprotocols: []string{"chisel-v3"}, + }).Dial("ws://127.0.0.1:"+port, http.Header{}) + if err != nil { + t.Fatal(err) + } + defer ws.Close() + //send a 5MB frame before authenticating; the server should kill + //the connection part-way through the frame + big := bytes.Repeat([]byte("A"), 5*1024*1024) + if err := ws.WriteMessage(websocket.BinaryMessage, big); err != nil { + return //reset mid-write: limit enforced + } + //otherwise the next read must fail with a close/reset, not + //a timeout (a timeout means the connection survived) + ws.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, _, err = ws.ReadMessage() + if err == nil { + t.Fatal("received a message after oversized pre-auth frame") + } + if ne, ok := err.(net.Error); ok && ne.Timeout() { + t.Fatal("connection still alive 5s after oversized pre-auth frame") + } +}