Harden the 1.12 release changes (#614)

2026-10-12 02:16:57 +00:00 · 2026-08-09 00:24:53 +10:00
parent 016edaae00
commit c408126df1
18 changed files with 693 additions and 102 deletions
@@ -50,10 +50,21 @@ archives:
        formats: [zip]
    files:
      - none*
+checksum:
+  # The publishing workflow stages preflight-verified archives outside dist,
+  # then sets this glob so the real release checksums those exact files.
+  # Empty by default so normal local/snapshot releases keep standard behavior.
+  extra_files:
+    - glob: '{{ envOrDefault "RELEASE_ARCHIVES_GLOB" "" }}'
 release:
  draft: true
  replace_existing_draft: true
  prerelease: auto
+  # In CI, these are the archives produced and checked by the non-publishing
+  # preflight. The publishing run uses --skip=archive, so it cannot replace
+  # them with rebuilt, unvalidated archives.
+  extra_files:
+    - glob: '{{ envOrDefault "RELEASE_ARCHIVES_GLOB" "" }}'
 changelog:
  sort: asc
  filters:
@@ -44,10 +44,12 @@ jobs:
        run: go test -v ./...
  # ================
  # 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.
+  # Builds and verifies the versioned archives without credentials, then
+  # creates a DRAFT GitHub release containing those exact archives and the
+  # packages built by the publishing run. It also pushes version-tagged
+  # 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:
    name: Release
@@ -62,11 +64,49 @@ jobs:
        uses: actions/checkout@v7
        with:
          fetch-depth: 0
+          # Do not leave the write-capable job token in Git configuration;
+          # the preflight must run without publication credentials.
+          persist-credentials: false
      - name: Set up Go
        uses: actions/setup-go@v6
        with:
          go-version: stable
          cache: true
+      - name: Build release archives without publishing
+        uses: goreleaser/goreleaser-action@v6
+        with:
+          distribution: goreleaser
+          # v2.12.7 silently produced corrupt .gz archives (#610); v2.17.0
+          # verified clean — reproduce/verify with 3x snapshot A/B runs
+          # before changing this pin
+          version: v2.17.0
+          args: release --clean --config .github/goreleaser.yml --skip=publish,docker,nfpm
+      - name: Verify and stage release archives
+        # Fail closed before credentials are used. The publishing run skips
+        # its archive pipe and uploads these exact files from outside dist.
+        run: |
+          set -euo pipefail
+          shopt -s nullglob
+          archives=(dist/*.gz dist/*.zip)
+          if (( ${#archives[@]} == 0 )); then
+            echo "::error::GoReleaser produced no .gz or .zip archives"
+            exit 1
+          fi
+
+          bad=0
+          for archive in "${archives[@]}"; do
+            case "$archive" in
+              *.gz)  gzip -t "$archive" || { echo "CORRUPT: $archive"; bad=1; } ;;
+              *.zip) unzip -t "$archive" >/dev/null || { echo "CORRUPT: $archive"; bad=1; } ;;
+            esac
+          done
+          if (( bad != 0 )); then
+            exit "$bad"
+          fi
+
+          mkdir -p release/archives
+          mv -- "${archives[@]}" release/archives/
+          echo "verified and staged ${#archives[@]} release archives"
      - name: Set up QEMU
        uses: docker/setup-qemu-action@v4
      - name: Set up Docker Buildx
@@ -82,7 +122,7 @@ jobs:
        with:
          username: jpillora
          password: ${{ secrets.DOCKERHUB_TOKEN }}
-      - name: Run GoReleaser
+      - name: Publish with GoReleaser
        uses: goreleaser/goreleaser-action@v6
        with:
          distribution: goreleaser
@@ -90,22 +130,10 @@ jobs:
          # verified clean — reproduce/verify with 3x snapshot A/B runs
          # before changing this pin
          version: v2.17.0
-          args: release --clean --config .github/goreleaser.yml
+          # GoReleaser OSS v2.17.0 cannot publish a previous dist directory.
+          # Rebuild binaries for packages/images, but preserve the verified
+          # archives by skipping this run's archive pipe.
+          args: release --clean --config .github/goreleaser.yml --skip=archive
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-      - name: Verify archive integrity
-        # goreleaser can exit 0 while emitting corrupt archives (#610) —
-        # fail the job so a broken draft is never published (stage 2 only
-        # promotes what a human publishes, and a red job blocks that)
-        run: |
-          set -euo pipefail
-          bad=0
-          for f in dist/*.gz dist/*.zip; do
-            [ -e "$f" ] || continue
-            case "$f" in
-              *.gz)  gzip -t "$f" || { echo "CORRUPT: $f"; bad=1; } ;;
-              *.zip) unzip -t "$f" >/dev/null || { echo "CORRUPT: $f"; bad=1; } ;;
-            esac
-          done
-          [ "$bad" = 0 ] && echo "all archives verified"
-          exit "$bad"
+          RELEASE_ARCHIVES_GLOB: release/archives/*
@@ -42,10 +42,10 @@ On master, `--auth nocolon` silently degraded (server: user never registered →
 - **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).
+- **New info-level logs:** session `Open (user=… addr=… remotes=…)` / `Close (… duration=…)` and `Login failed for user "X" (ip)` — failed logins are finally fail2ban-able. The username is quoted and escaped, so an attacker-supplied name cannot forge a second log line. 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.
+- **WS read cap 512 KiB pre-auth** (`CHISEL_WS_READ_LIMIT`, only 0 disables; negative values use the default) on both sides. The pinned `x/crypto/ssh` accepts transport packets up to 256 KiB; the doubled ceiling clears that with room to spare, so no valid SSH packet is rejected, while retaining a finite pre-auth bound. Custom positive limits remain available through the env knob.
 - **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.

@@ -0,0 +1,143 @@
+# Chisel 1.12 Release Review Tasks
+
+This checklist tracks the findings from the `v1.11.8` to
+`v1.12.0-rc2` release review. Tasks are implemented and reviewed one at a
+time; each completed task is committed before work starts on the next one.
+
+## Task 1: Wait for graceful HTTP shutdown to finish
+
+- [x] Complete
+- Severity: high
+- Problem: `HTTPServer.Wait` waits for `http.Server.Serve`, which returns as
+  soon as `Shutdown` closes the listener. The CLI can therefore exit while
+  the shutdown goroutine is still draining active handlers.
+- Plan:
+  1. Make the shutdown/drain operation part of the lifecycle observed by
+     `HTTPServer.Wait`.
+  2. Preserve immediate refusal of new connections after cancellation.
+  3. Preserve the configurable `CHISEL_SHUTDOWN_GRACE` deadline and force
+     close any requests which outlive it.
+  4. Extend the HTTP server tests so they call `Wait` immediately after
+     cancellation and prove that it does not return before an in-flight
+     handler finishes.
+  5. Add timeout-path coverage proving `Wait` returns after the configured
+     grace period when a handler does not drain.
+- Acceptance criteria:
+  - A normal in-flight request completes before `Wait` returns.
+  - New requests are refused once shutdown begins.
+  - A stuck request cannot block shutdown past the configured grace period.
+  - Existing unit, race, and end-to-end tests pass.
+- Implementation review: approved by an independent Sol reviewer; focused,
+  full, race, vet, and diff-check validation passed.
+- Commit: `fix: wait for graceful HTTP shutdown`
+
+## Task 2: Let cancellation win over retry exhaustion
+
+- [x] Complete
+- Severity: medium
+- Problem: after `connectionOnce` returns, `connectionLoop` checks
+  `MaxRetryCount` before checking whether its context was cancelled. A
+  SIGINT/SIGTERM can consequently return `connection attempts exhausted`
+  and exit non-zero.
+- Plan:
+  1. Detect cancellation immediately after `connectionOnce` returns and
+     return the normal cancellation result before logging or applying the
+     retry budget.
+  2. Preserve non-zero failure when a live context genuinely exhausts the
+     configured retry count.
+  3. Add regression coverage for an already-cancelled context with
+     `MaxRetryCount == 0`.
+  4. Add coverage for cancellation after a connection has been established
+     if it can be done deterministically without timing-sensitive sleeps.
+- Acceptance criteria:
+  - Context cancellation and CLI signal shutdown return success for every
+    retry-count setting.
+  - Genuine retry exhaustion still returns `connection attempts exhausted`.
+  - Existing retry timing and attempt-count behavior is unchanged.
+- Implementation review: approved by an independent Sol reviewer; focused,
+  full, and race validation passed. A test-only ephemeral-port TOCTOU was
+  noted as non-blocking.
+- Commit: `fix: prefer cancellation over retry exhaustion`
+
+## Task 3: Allow every valid SSH packet through the WebSocket limit
+
+- [x] Complete
+- Severity: medium
+- Problem: the 64 KiB WebSocket message limit is based on an incorrect
+  approximately-35-KiB SSH maximum. The pinned `x/crypto/ssh` permits
+  packets up to 256 KiB, so large valid configuration requests can be
+  disconnected.
+- Plan:
+  1. Raise the default WebSocket read limit above the SSH transport maximum,
+     including framing/headroom, while retaining a finite pre-auth bound.
+  2. Keep `CHISEL_WS_READ_LIMIT=0` as the explicit opt-out and preserve
+     custom-limit behavior.
+  3. Add tests proving a message above 64 KiB but within the supported SSH
+     envelope succeeds, while a message above the new default fails.
+  4. Update help/release documentation and comments with the corrected
+     default and rationale.
+- Acceptance criteria:
+  - Previously valid SSH/configuration packets up to the SSH maximum are not
+    rejected by the default.
+  - Oversized pre-auth WebSocket messages remain bounded and rejected.
+  - Documentation and tests agree with the implementation default.
+- Implementation review: approved after revision by an independent Sol
+  reviewer; protocol-boundary, real SSH configuration, environment-isolation,
+  focused, and race tests passed.
+- Commit: `fix: accommodate valid SSH websocket messages`
+
+## Task 4: Validate release archives before publication starts
+
+- [x] Complete
+- Severity: low
+- Problem: the CI job currently runs GoReleaser's publishing operation
+  before `gzip -t`/`unzip -t`. A failed post-publication check leaves a
+  broken draft and pushed version images, and a red job does not
+  mechanically prevent a maintainer from publishing that draft.
+- Plan:
+  1. Add a non-publishing GoReleaser preflight which produces the release
+     archives before any registry login or release upload is performed.
+  2. Run integrity checks against every preflight `.gz` and `.zip` and fail
+     closed if no matching archives were produced.
+  3. Execute the real GoReleaser publication only after the preflight passes.
+  4. Keep the pinned GoReleaser version and validate the resulting workflow
+     and GoReleaser configuration locally.
+  5. Update workflow comments so they describe the actual enforcement
+     boundary without claiming that a red job blocks manual publication.
+- Acceptance criteria:
+  - No GitHub release or registry publication step runs before archive
+    integrity succeeds.
+  - Missing archives make the preflight fail rather than silently pass.
+  - The final publication still produces versioned archives, packages, and
+    multi-architecture Docker manifests.
+- Implementation review: approved after adding `persist-credentials: false`
+  to the release checkout; the reviewer confirmed the archive gate, staged
+  extra-file checksums/uploads, package and image pipelines, and fail-closed
+  shell behavior under GoReleaser v2.17.0.
+- Commit: `ci: validate archives before release publication`
+
+## Task 5: Quote untrusted usernames in failed-login logs
+
+- [x] Complete
+- Severity: low
+- Problem: failed SSH usernames are unauthenticated input and are logged with
+  `%s` at info level. Embedded newlines can forge log entries and interfere
+  with fail2ban-style consumers.
+- Plan:
+  1. Render failed-login usernames in a quoted/escaped representation that
+     keeps each attempt on one physical log line.
+  2. Preserve the remote address and operator-friendly normal username
+     output.
+  3. Add a regression test using a username containing newline/control
+     characters and assert that it cannot create an extra log line.
+  4. Confirm successful authentication and returned authentication errors
+     retain their existing semantics.
+- Acceptance criteria:
+  - Untrusted usernames cannot inject physical log lines.
+  - Failed attempts remain visible and useful to log parsers.
+  - Authentication behavior is unchanged.
+- Implementation review: approved after also quoting the aggregated SSH
+  handshake error at its debug-log sink; focused tests confirm that both
+  failed-login paths remain single-line while authentication errors and
+  remote-address reporting retain their behavior.
+- Commit: `fix: quote usernames in failed-login logs`
@@ -387,7 +387,7 @@ $ chisel client --help

 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.
+The server also caps inbound websocket message sizes before authentication (`CHISEL_WS_READ_LIMIT`, default 512 KiB), so unauthenticated peers cannot exhaust memory with oversized messages. The default sits comfortably above `x/crypto/ssh`'s 256 KiB maximum transport packet, so no valid SSH packet is ever rejected. Only `0` disables the limit; negative values fall back to the safe default.

 ### Authentication

@@ -487,7 +487,7 @@ Less common knobs are environment variables, all read with a `CHISEL_` prefix (e
 | `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_READ_LIMIT` | both        | `524288`           | max inbound websocket message bytes (0 = no limit; negative = default) |
 | `WS_BUFF_SIZE`  | both        | go default         | websocket read/write buffer sizes                  |
 | `UDP_MAX_SIZE`  | both        | `9012`             | max udp packet bytes                               |
 | `UDP_DEADLINE`  | exit node   | `15s`              | udp flow read deadline and idle-sweep age          |
@@ -286,7 +286,7 @@ Return errors; `main.go` already log.Fatals on the returned error.

 ### 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.
+Call `SetReadLimit` 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 pinned `x/crypto/ssh` supports transport packets up to 256 KiB, so the limit must sit above that plus framing; 512 KiB clears it with room to spare while retaining a finite pre-auth bound. The `WS_BUFF_SIZE` env already tunes buffer sizes.

 ## 18. Graceful shutdown: handle SIGTERM and use http.Server.Shutdown

@@ -25,6 +25,12 @@ func (c *Client) connectionLoop(ctx context.Context) error {
 	}
 	for {
 		connected, err := c.connectionOnce(ctx)
+		// Cancellation is a normal shutdown, even when the final connection
+		// attempt also consumes the configured retry budget.
+		if ctx.Err() != nil {
+			c.Infof("Cancelled")
+			return nil
+		}
 		//reset backoff after successful connections
 		if connected {
 			b.Reset()
@@ -2,9 +2,15 @@ package chclient

 import (
 	"context"
+	"errors"
+	"net"
+	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
+
+	chserver "github.com/jpillora/chisel/server"
 )

 func TestRetryIntervalDefaults(t *testing.T) {
@@ -53,9 +59,12 @@ func TestRetryIntervalExplicit(t *testing.T) {
 // 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
+	dialErr := errors.New("test dial failure")
 	c, err := NewClient(&Config{
-		Server:        "http://127.0.0.1:1",
+		Server: "http://example.invalid",
+		DialContext: func(context.Context, string, string) (net.Conn, error) {
+			return nil, dialErr
+		},
 		MaxRetryCount: 0,
 	})
 	if err != nil {
@@ -74,3 +83,81 @@ func TestGiveUpReturnsError(t *testing.T) {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
+
+func TestCancelledContextWinsOverRetryExhaustion(t *testing.T) {
+	var dials atomic.Int32
+	c, err := NewClient(&Config{
+		Server: "http://example.invalid",
+		DialContext: func(context.Context, string, string) (net.Conn, error) {
+			dials.Add(1)
+			return nil, errors.New("unexpected dial")
+		},
+		MaxRetryCount: 0,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	if err := c.connectionLoop(ctx); err != nil {
+		t.Fatalf("cancelled connection loop returned an error: %v", err)
+	}
+	if got := dials.Load(); got != 0 {
+		t.Fatalf("cancelled connection loop made %d dials, want 0", got)
+	}
+}
+
+func TestCancellationAfterConnectionWinsOverRetryExhaustion(t *testing.T) {
+	serverCtx, stopServer := context.WithCancel(context.Background())
+	server, err := chserver.NewServer(&chserver.Config{KeySeed: "retry-cancel-test"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	port := availableTCPPort(t)
+	if err := server.StartContext(serverCtx, "127.0.0.1", port); err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		stopServer()
+		if err := server.Wait(); err != nil {
+			t.Errorf("server shutdown: %v", err)
+		}
+	}()
+
+	clientCtx, stopClient := context.WithCancel(context.Background())
+	c, err := NewClient(&Config{
+		Fingerprint:   server.GetFingerprint(),
+		Server:        "http://127.0.0.1:" + port,
+		MaxRetryCount: 0,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := c.Start(clientCtx); err != nil {
+		t.Fatal(err)
+	}
+	defer stopClient()
+
+	readyCtx, stopReady := context.WithTimeout(context.Background(), 10*time.Second)
+	defer stopReady()
+	if !c.Ready(readyCtx) {
+		t.Fatal("client did not establish a connection")
+	}
+	stopClient()
+	if err := c.Wait(); err != nil {
+		t.Fatalf("cancelled connected client returned an error: %v", err)
+	}
+}
+
+func availableTCPPort(t *testing.T) string {
+	t.Helper()
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	port := l.Addr().(*net.TCPAddr).Port
+	if err := l.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return strconv.Itoa(port)
+}
@@ -206,7 +206,7 @@ func (s *Server) authUser(c ssh.ConnMetadata, password []byte) (*ssh.Permissions
 	user, found := s.users.Get(n)
 	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())
+		s.Infof("Login failed for user %q (%s)", n, c.RemoteAddr())
 		return nil, fmt.Errorf("invalid authentication for username: %s", n)
 	}
 	// pass the username through to the handshake handler, which
@@ -1,19 +1,30 @@
 package chserver

 import (
+	"bytes"
 	"net"
 	"strings"
 	"testing"
+
+	"github.com/jpillora/chisel/share/cio"
 )

-type testConnMeta struct{ user string }
+type testConnMeta struct {
+	user       string
+	remoteAddr net.Addr
+}

 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 (m testConnMeta) RemoteAddr() net.Addr {
+	if m.remoteAddr != nil {
+		return m.remoteAddr
+	}
+	return &net.TCPAddr{}
+}
+func (m testConnMeta) LocalAddr() net.Addr { return &net.TCPAddr{} }

 func TestAuthUser(t *testing.T) {
 	s, err := NewServer(&Config{KeySeed: "auth-test"})
@@ -24,7 +35,7 @@ func TestAuthUser(t *testing.T) {
 		t.Fatal(err)
 	}
 	//valid credentials carry the username through Permissions
-	perms, err := s.authUser(testConnMeta{"alice"}, []byte("secret"))
+	perms, err := s.authUser(testConnMeta{user: "alice"}, []byte("secret"))
 	if err != nil {
 		t.Fatalf("valid login failed: %v", err)
 	}
@@ -32,24 +43,60 @@ func TestAuthUser(t *testing.T) {
 		t.Fatalf("expected user extension, got %+v", perms)
 	}
 	//wrong password
-	if _, err := s.authUser(testConnMeta{"alice"}, []byte("wrong")); err == nil {
+	if _, err := s.authUser(testConnMeta{user: "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 {
+	if _, err := s.authUser(testConnMeta{user: "bob"}, []byte("secret")); err == nil {
 		t.Fatal("unknown user accepted")
 	}
 }

+func TestAuthUserFailedLoginQuotesUsername(t *testing.T) {
+	s, err := NewServer(&Config{KeySeed: "auth-log-test"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := s.AddUser("alice", "secret", ""); err != nil {
+		t.Fatal(err)
+	}
+
+	var logs bytes.Buffer
+	s.Logger = cio.NewLoggerFlag("server", 0)
+	s.Logger.Info = true
+	s.Logger.SetOutput(&logs)
+
+	username := "mallory\nserver: forged\r\t\x00"
+	remoteAddr := &net.TCPAddr{IP: net.ParseIP("192.0.2.10"), Port: 2222}
+	_, err = s.authUser(testConnMeta{
+		user:       username,
+		remoteAddr: remoteAddr,
+	}, []byte("wrong"))
+	if err == nil {
+		t.Fatal("malicious username was accepted")
+	}
+	if got, want := err.Error(), "invalid authentication for username: "+username; got != want {
+		t.Fatalf("authentication error changed: got %q, want %q", got, want)
+	}
+
+	const wantLog = "server: Login failed for user \"mallory\\nserver: forged\\r\\t\\x00\" (192.0.2.10:2222)\n"
+	if got := logs.String(); got != wantLog {
+		t.Fatalf("unexpected failed-login log:\n got: %q\nwant: %q", got, wantLog)
+	}
+	if got := strings.Count(logs.String(), "\n"); got != 1 {
+		t.Fatalf("attacker input created extra log lines: got %d newlines in %q", got, logs.String())
+	}
+}
+
 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"))
+	perms, err := s.authUser(testConnMeta{user: "anyone"}, []byte("anything"))
 	if err != nil {
 		t.Fatalf("allow-all rejected: %v", err)
 	}
@@ -62,7 +62,7 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) {
 	l.Debugf("Handshaking with %s...", req.RemoteAddr)
 	sshConn, chans, reqs, err := ssh.NewServerConn(conn, s.sshConfig)
 	if err != nil {
-		s.Debugf("Failed to handshake (%s)", err)
+		s.logHandshakeFailure(err)
 		return
 	}
 	// resolve the user from the authenticated username set by
@@ -197,3 +197,7 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) {
 	l.Infof("Close (user=%s addr=%s duration=%s)%s",
 		username, req.RemoteAddr, time.Since(opened), errmsg)
 }
+
+func (s *Server) logHandshakeFailure(err error) {
+	s.Debugf("Failed to handshake (%q)", err.Error())
+}
@@ -0,0 +1,32 @@
+package chserver
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/jpillora/chisel/share/cio"
+	"golang.org/x/crypto/ssh"
+)
+
+func TestLogHandshakeFailureQuotesAuthenticationError(t *testing.T) {
+	var logs bytes.Buffer
+	logger := cio.NewLoggerFlag("server", 0)
+	logger.Debug = true
+	logger.SetOutput(&logs)
+	s := &Server{Logger: logger}
+
+	username := "mallory\nserver: forged\r\t\x00"
+	authErr := fmt.Errorf("invalid authentication for username: %s", username)
+	handshakeErr := &ssh.ServerAuthError{Errors: []error{authErr}}
+	s.logHandshakeFailure(handshakeErr)
+
+	const wantLog = "server: Failed to handshake (\"[invalid authentication for username: mallory\\nserver: forged\\r\\t\\x00]\")\n"
+	if got := logs.String(); got != wantLog {
+		t.Fatalf("unexpected handshake failure log:\n got: %q\nwant: %q", got, wantLog)
+	}
+	if got := strings.Count(logs.String(), "\n"); got != 1 {
+		t.Fatalf("authentication error created extra log lines: got %d newlines in %q", got, logs.String())
+	}
+}
@@ -2,11 +2,12 @@ package cio

 import (
 	"fmt"
+	"io"
 	"log"
 	"os"
 )

-//Logger is pkg/log Logger with prefixing and 2 log levels
+// Logger is pkg/log Logger with prefixing and 2 log levels
 type Logger struct {
 	Info, Debug bool
 	//internal
@@ -45,6 +46,11 @@ func (l *Logger) Errorf(f string, args ...interface{}) error {
 	return fmt.Errorf(l.prefix+": "+f, args...)
 }

+// SetOutput sets the destination for logger output.
+func (l *Logger) SetOutput(w io.Writer) {
+	l.logger.SetOutput(w)
+}
+
 func (l *Logger) Fork(prefix string, args ...interface{}) *Logger {
 	//slip the parent prefix at the front
 	args = append([]interface{}{l.prefix}, args...)
@@ -13,20 +13,37 @@ type wsConn struct {
 	buff []byte
 }

-//NewWebSocketConn converts a websocket.Conn into a net.Conn
+const (
+	// x/crypto/ssh rejects transport packets whose length field exceeds this,
+	// so a full wire packet tops out around 256 KiB plus the 4-byte length and
+	// a MAC. Doubling leaves the exact framing overhead unspecified rather than
+	// tracking a dependency's internals, while retaining a finite bound on
+	// messages received before SSH authentication.
+	sshMaxTransportPacket     = 256 * 1024
+	defaultWebSocketReadLimit = 2 * sshMaxTransportPacket
+)
+
+// 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)))
+	websocketConn.SetReadLimit(configuredWebSocketReadLimit())
 	c := wsConn{
 		Conn: websocketConn,
 	}
 	return &c
 }

-//Read is not threadsafe though that's okay since there
-//should never be more than one reader
+func configuredWebSocketReadLimit() int64 {
+	// Only 0 disables the limit. Treat negative values as invalid rather than
+	// letting gorilla interpret them as another unlimited setting.
+	limit := settings.EnvInt("WS_READ_LIMIT", defaultWebSocketReadLimit)
+	if limit < 0 {
+		limit = defaultWebSocketReadLimit
+	}
+	return int64(limit)
+}
+
+// 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)
 	//use buffer or read new message
@@ -2,6 +2,7 @@ package cnet

 import (
 	"bytes"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -11,9 +12,9 @@ import (
 	"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) {
+// wsPair returns reads from the server side of a websocket connection wrapped
+// in NewWebSocketConn, plus the raw client side for sending test messages.
+func wsPair(t *testing.T, readSize int) (serverSide chan interface{}, client *websocket.Conn) {
 	t.Helper()
 	upgrader := websocket.Upgrader{}
 	out := make(chan interface{}, 1)
@@ -24,8 +25,9 @@ func wsPair(t *testing.T) (serverSide chan interface{}, client *websocket.Conn)
 			return
 		}
 		conn := NewWebSocketConn(ws)
+		defer conn.Close()
 		conn.SetDeadline(time.Now().Add(5 * time.Second))
-		buf := make([]byte, 1024)
+		buf := make([]byte, readSize)
 		n, err := conn.Read(buf)
 		if err != nil {
 			out <- err
@@ -43,37 +45,90 @@ func wsPair(t *testing.T) (serverSide chan interface{}, client *websocket.Conn)
 	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 {
+// TestWSConnReadDefaultAllowsSSHPacketEnvelope verifies that the default has
+// room for x/crypto/ssh's 256 KiB packet ceiling plus transport overhead.
+func TestWSConnReadDefaultAllowsSSHPacketEnvelope(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "")
+	const sshPacketWithHeadroom = sshMaxTransportPacket + 4*1024
+	out, client := wsPair(t, sshPacketWithHeadroom)
+	message := bytes.Repeat([]byte("x"), sshPacketWithHeadroom)
+	if err := client.WriteMessage(websocket.BinaryMessage, message); err != nil {
 		t.Fatal(err)
 	}
 	switch v := (<-out).(type) {
 	case []byte:
-		if string(v) != "hello" {
-			t.Fatalf("got %q", v)
+		if !bytes.Equal(v, message) {
+			t.Fatalf("read %d bytes, want %d", len(v), len(message))
 		}
 	case error:
 		t.Fatalf("read failed: %v", v)
 	}
 }
+
+// TestWSConnReadDefaultRejectsOversizedMessage verifies that the finite default
+// still rejects messages above its pre-auth bound.
+func TestWSConnReadDefaultRejectsOversizedMessage(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "")
+	out, client := wsPair(t, defaultWebSocketReadLimit+1)
+	message := bytes.Repeat([]byte("x"), defaultWebSocketReadLimit+1)
+	if err := client.WriteMessage(websocket.BinaryMessage, message); err != nil {
+		t.Fatal(err)
+	}
+	switch v := (<-out).(type) {
+	case error:
+		if !errors.Is(v, websocket.ErrReadLimit) {
+			t.Fatalf("read failed with %v, want %v", v, websocket.ErrReadLimit)
+		}
+	case []byte:
+		t.Fatalf("oversized message was read (%d bytes leaked through)", len(v))
+	}
+}
+
+func TestWSConnReadCustomLimit(t *testing.T) {
+	const limit = 32 * 1024
+	t.Setenv("CHISEL_WS_READ_LIMIT", "32768")
+	out, client := wsPair(t, limit+1)
+	if err := client.WriteMessage(websocket.BinaryMessage, bytes.Repeat([]byte("x"), limit+1)); err != nil {
+		t.Fatal(err)
+	}
+	v, ok := (<-out).(error)
+	if !ok {
+		t.Fatal("message above custom limit was read")
+	}
+	if !errors.Is(v, websocket.ErrReadLimit) {
+		t.Fatalf("read failed with %v, want %v", v, websocket.ErrReadLimit)
+	}
+}
+
+func TestWSConnReadNegativeLimitUsesDefault(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "-1")
+	out, client := wsPair(t, defaultWebSocketReadLimit+1)
+	message := bytes.Repeat([]byte("x"), defaultWebSocketReadLimit+1)
+	if err := client.WriteMessage(websocket.BinaryMessage, message); err != nil {
+		t.Fatal(err)
+	}
+	v, ok := (<-out).(error)
+	if !ok {
+		t.Fatal("negative limit disabled the read cap")
+	}
+	if !errors.Is(v, websocket.ErrReadLimit) {
+		t.Fatalf("read failed with %v, want %v", v, websocket.ErrReadLimit)
+	}
+}
+
+func TestWSConnReadLimitDisabled(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "0")
+	message := bytes.Repeat([]byte("x"), defaultWebSocketReadLimit+1)
+	out, client := wsPair(t, len(message))
+	if err := client.WriteMessage(websocket.BinaryMessage, message); err != nil {
+		t.Fatal(err)
+	}
+	switch v := (<-out).(type) {
+	case []byte:
+		if !bytes.Equal(v, message) {
+			t.Fatalf("read %d bytes, want %d", len(v), len(message))
+		}
+	case error:
+		t.Fatalf("read failed with limit disabled: %v", v)
+	}
+}
@@ -12,15 +12,15 @@ import (
 	"golang.org/x/sync/errgroup"
 )

-//HTTPServer extends net/http Server and
-//adds graceful shutdowns
+// HTTPServer extends net/http Server and
+// adds graceful shutdowns
 type HTTPServer struct {
 	*http.Server
 	waiterMux sync.Mutex
 	waiter    *errgroup.Group
 }

-//NewHTTPServer creates a new HTTPServer
+// NewHTTPServer creates a new HTTPServer
 func NewHTTPServer() *HTTPServer {
 	return &HTTPServer{
 		Server: &http.Server{},
@@ -54,7 +54,11 @@ func (h *HTTPServer) GoServe(ctx context.Context, l net.Listener, handler http.H
 	h.waiter.Go(func() error {
 		return h.Serve(l)
 	})
-	go func() {
+	//run the drain inside the group so Wait observes it. ctx here is the
+	//group's derived context, so this also unblocks when Serve returns —
+	//that is what lets a bare Close() (no cancellation) still finish Wait.
+	//Serve never returns nil: it reports ErrServerClosed on Close/Shutdown.
+	h.waiter.Go(func() error {
 		<-ctx.Done()
 		//graceful shutdown: stop accepting, drain in-flight requests
 		//for a grace period, then force-close the remainder. hijacked
@@ -64,7 +68,8 @@ func (h *HTTPServer) GoServe(ctx context.Context, l net.Listener, handler http.H
 		defer cancel()
 		h.Server.Shutdown(c)
 		h.Close()
-	}()
+		return nil
+	})
 	return nil
 }

@@ -9,9 +9,8 @@ import (
 	"time"
 )

-// TestGoServeGracefulDrain verifies that cancelling the GoServe context
-// drains in-flight requests instead of resetting them, while refusing
-// new connections.
+// TestGoServeGracefulDrain verifies that Wait observes the graceful drain,
+// while the server refuses new connections.
 func TestGoServeGracefulDrain(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -20,9 +19,12 @@ func TestGoServeGracefulDrain(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	started := make(chan struct{})
+	release := make(chan struct{})
 	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		time.Sleep(500 * time.Millisecond)
-		w.Write([]byte("done"))
+		close(started)
+		<-release
+		_, _ = w.Write([]byte("done"))
 	})
 	if err := h.GoServe(ctx, l, handler); err != nil {
 		t.Fatal(err)
@@ -41,8 +43,38 @@ func TestGoServeGracefulDrain(t *testing.T) {
 		resp.Body.Close()
 		bodyc <- string(b)
 	}()
-	time.Sleep(100 * time.Millisecond) //let the request reach the handler
+	select {
+	case <-started:
+	case <-time.After(3 * time.Second):
+		t.Fatal("timed out waiting for request to reach handler")
+	}
 	cancel()
+	waitc := make(chan error, 1)
+	go func() {
+		waitc <- h.Wait()
+	}()
+
+	// Shutdown closes the listener before waiting for active handlers. Retry
+	// until that close is observable, because cancellation and Shutdown run in
+	// separate goroutines.
+	addr := l.Addr().String()
+	deadline := time.Now().Add(3 * time.Second)
+	for {
+		conn, err := net.DialTimeout("tcp", addr, 100*time.Millisecond)
+		if err != nil {
+			break
+		}
+		conn.Close()
+		if time.Now().After(deadline) {
+			t.Fatal("new requests continued to succeed during shutdown")
+		}
+	}
+	select {
+	case err := <-waitc:
+		t.Fatalf("Wait returned before in-flight handler finished: %v", err)
+	default:
+	}
+	close(release)
 	select {
 	case b := <-bodyc:
 		if b != "done" {
@@ -53,11 +85,66 @@ func TestGoServeGracefulDrain(t *testing.T) {
 	case <-time.After(3 * time.Second):
 		t.Fatal("timed out waiting for drained response")
 	}
-	//new connections must be refused after shutdown
+	// New connections remain refused after the drain finishes.
 	if _, err := http.Get(url); err == nil {
 		t.Fatal("new request succeeded after shutdown")
 	}
-	if err := h.Wait(); err != nil {
+	if err := <-waitc; err != nil {
 		t.Fatalf("wait: %v", err)
 	}
 }
+
+func TestGoServeShutdownGraceTimeout(t *testing.T) {
+	const grace = 100 * time.Millisecond
+	t.Setenv("CHISEL_SHUTDOWN_GRACE", grace.String())
+
+	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)
+	}
+	started := make(chan struct{})
+	release := make(chan struct{})
+	defer close(release)
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		close(started)
+		<-release
+	})
+	if err := h.GoServe(ctx, l, handler); err != nil {
+		t.Fatal(err)
+	}
+
+	requestDone := make(chan struct{})
+	go func() {
+		defer close(requestDone)
+		resp, err := http.Get("http://" + l.Addr().String())
+		if err == nil {
+			resp.Body.Close()
+		}
+	}()
+	select {
+	case <-started:
+	case <-time.After(3 * time.Second):
+		t.Fatal("timed out waiting for request to reach handler")
+	}
+
+	start := time.Now()
+	cancel()
+	if err := h.Wait(); err != nil {
+		t.Fatalf("wait: %v", err)
+	}
+	elapsed := time.Since(start)
+	if elapsed < grace {
+		t.Fatalf("Wait returned before shutdown grace expired: %v", elapsed)
+	}
+	if elapsed > 3*time.Second {
+		t.Fatalf("Wait exceeded shutdown grace by too much: %v", elapsed)
+	}
+	select {
+	case <-requestDone:
+	case <-time.After(3 * time.Second):
+		t.Fatal("force-close did not unblock the active request")
+	}
+}
@@ -2,20 +2,24 @@ package e2e_test

 import (
 	"bytes"
+	"encoding/json"
 	"net"
 	"net/http"
+	"strings"
 	"testing"
 	"time"

 	chserver "github.com/jpillora/chisel/server"
+	"github.com/jpillora/chisel/share/settings"

 	"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) {
+// TestPreAuthOversizedMessage verifies that an unauthenticated peer
+// sending a huge websocket message gets disconnected by the read limit
+// instead of the server buffering the message into memory.
+func TestPreAuthOversizedMessage(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "")
 	s, err := chserver.NewServer(&chserver.Config{KeySeed: "ws-limit-test"})
 	if err != nil {
 		t.Fatal(err)
@@ -35,20 +39,79 @@ func TestPreAuthOversizedFrame(t *testing.T) {
 		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
+
+	// Drain the server's SSH identification. Without this, a read after the
+	// oversized write can consume the already-buffered banner and falsely look
+	// like the connection survived.
+	if err := ws.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		t.Fatal(err)
+	}
+	messageType, identification, err := ws.ReadMessage()
+	if err != nil {
+		t.Fatalf("read SSH identification: %v", err)
+	}
+	if messageType != websocket.BinaryMessage || !bytes.HasPrefix(identification, []byte("SSH-")) {
+		t.Fatalf("unexpected SSH identification message: type=%d payload=%q", messageType, identification)
+	}
+
+	// Send one byte above the default before authenticating. The server should
+	// close or reset the connection as soon as the read limit is exceeded.
+	oversized := bytes.Repeat([]byte("A"), 512*1024+1)
+	if err := ws.SetWriteDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		t.Fatal(err)
+	}
+	if err := ws.WriteMessage(websocket.BinaryMessage, oversized); err != nil {
+		if ne, ok := err.(net.Error); ok && ne.Timeout() {
+			t.Fatalf("timed out sending oversized pre-auth message: %v", err)
+		}
+		return // close/reset while writing: limit enforced
+	}
+	// Otherwise the next read must fail with a close/reset, not a timeout.
+	if err := ws.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		t.Fatal(err)
 	}
-	//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")
+		t.Fatal("received a message after oversized pre-auth message")
 	}
 	if ne, ok := err.(net.Error); ok && ne.Timeout() {
-		t.Fatal("connection still alive 5s after oversized pre-auth frame")
+		t.Fatal("connection still alive 5s after oversized pre-auth message")
+	}
+}
+
+// TestLargeConfigMessage verifies that a real SSH config request above the old
+// 64 KiB limit traverses the default WebSocket envelope successfully.
+func TestLargeConfigMessage(t *testing.T) {
+	t.Setenv("CHISEL_WS_READ_LIMIT", "")
+	s, err := chserver.NewServer(&chserver.Config{KeySeed: "large-config-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()
+
+	sc, _, _ := dialChiselSSH(t, "127.0.0.1:"+port, "user", "pass")
+	defer sc.Close()
+	cfg, err := json.Marshal(settings.Config{
+		// The development prefix suppresses version-mismatch logging while the
+		// remainder makes the actual SSH request larger than the former cap.
+		Version: "0.0.0-" + strings.Repeat("x", 96*1024),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(cfg) <= 64*1024 || len(cfg) >= 256*1024 {
+		t.Fatalf("config payload size %d is outside the intended SSH envelope", len(cfg))
+	}
+	ok, reply, err := sc.SendRequest("config", true, cfg)
+	if err != nil {
+		t.Fatalf("send large config request: %v", err)
+	}
+	if !ok {
+		t.Fatalf("large config rejected: %s", reply)
 	}
 }