From 38464bf978ba01326cafa0061b7342d5db81ba4d Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Mon, 6 Apr 2026 22:18:57 +0200 Subject: [PATCH 1/3] docs(cli): add spz port-forward architecture --- ...026-04-06-spz-port-forward-architecture.md | 361 ++++++++++++++++++ 1 file changed, 361 insertions(+) create mode 100644 docs/2026-04-06-spz-port-forward-architecture.md diff --git a/docs/2026-04-06-spz-port-forward-architecture.md b/docs/2026-04-06-spz-port-forward-architecture.md new file mode 100644 index 0000000..ca3a09c --- /dev/null +++ b/docs/2026-04-06-spz-port-forward-architecture.md @@ -0,0 +1,361 @@ +--- +date: 2026-04-06 +author: Onur Solmaz +title: spz Port-Forward Architecture +tags: [spritz, spz, cli, ssh, port-forwarding, architecture] +--- + +## Overview + +This document defines the preferred Spritz CLI contract for local access to +private instance ports. + +The core design decision is that Spritz should expose an explicit, +deployment-agnostic low-level primitive: + +```bash +spz port-forward --local --remote +``` + +This should be the canonical Spritz CLI shape for forwarding a local loopback +port to a private port inside one Spritz instance. + +It should be implemented as an instance-scoped control-plane feature, not as a +Kubernetes workflow, not as a browser preview product, and not as a +deployment-specific convenience alias. + +## TL;DR + +- Spritz core should add `spz port-forward`. +- The command should be explicit and transport-agnostic in meaning, while the + first implementation should reuse the existing SSH credential minting path. +- The command should default to: + - local bind host `127.0.0.1` + - remote target host `127.0.0.1` +- Spritz should authenticate the tunnel, not the application protocol behind + the forwarded port. +- Application-specific wrappers may exist downstream, but the upstream Spritz + primitive should stay generic and explicit. + +## Problem + +Users and downstream deployments need a safe way to reach private ports inside +an instance from a local machine. + +Common examples: + +- open a local browser against a web app running inside an instance +- attach a local debugger to a process listening only inside the instance +- inspect an internal HTTP endpoint without exposing it on public ingress + +Today, the CLI has raw instance access primitives such as: + +- `spz ssh ` +- `spz terminal ` + +Those are useful, but they force callers to manually construct port-forwarding +behavior on top of raw SSH usage. + +That creates several problems: + +- the workflow is discoverable only for users who already know `ssh -L` +- the product contract is implicit instead of first-class +- downstreams are pushed toward app-specific commands too early +- users may reach for the wrong tool, such as `kubectl port-forward`, which + bypasses the Spritz instance access boundary + +Spritz should expose the intent directly. + +## Goals + +- Provide one clear CLI primitive for local access to private instance ports. +- Keep the contract generic across apps, runtimes, and deployments. +- Reuse the existing Spritz instance auth boundary. +- Avoid requiring Kubernetes credentials or pod-level knowledge. +- Keep the default security posture loopback-only on both sides. +- Make it easy for downstream deployments to build higher-level wrappers later. + +## Non-goals + +- Defining app-specific preview commands in upstream Spritz. +- Requiring public ingress for development servers. +- Turning Spritz into a remote desktop or VNC product. +- Solving application-layer login, cookies, or browser session reuse. +- Replacing `spz ssh` as the raw shell-access primitive. + +## Design Decision + +Spritz should standardize an explicit low-level forwarding command: + +```bash +spz port-forward --local --remote +``` + +This should be the core primitive. It should mean: + +- listen on one local loopback port +- forward to one private remote port inside one named instance +- keep the tunnel alive until interrupted + +The first implementation should be built on the existing SSH certificate mint +flow already used by `spz ssh`. + +The product contract, however, should be described as "instance port +forwarding", not as "raw SSH with custom flags". That distinction matters: + +- users reason about what they want to do, not the transport internals +- the control plane remains the owner of authorization and target resolution +- future transports may change without renaming the user-facing intent + +## Why `spz port-forward` Instead Of `spz preview` + +`preview` is the wrong upstream primitive because it encodes application +semantics into the core CLI. + +Problems with `preview` as the base command: + +- it sounds browser-specific +- it sounds frontend-specific +- it does not generalize cleanly to debuggers, admin endpoints, metrics, or + other private instance ports +- it encourages Spritz core to learn too much about downstream app types + +`port-forward` is better because it is: + +- explicit +- generic +- already familiar to operators and developers +- honest about what the command does + +Downstreams may still layer higher-level wrappers later, but those wrappers +should sit on top of the explicit primitive rather than replace it. + +## Why Not `kubectl port-forward` + +`kubectl port-forward` is useful for cluster operators, but it is not the +right Spritz product contract. + +Problems with making it the standard workflow: + +- it requires Kubernetes credentials +- it exposes pod and namespace details to the caller +- it targets infrastructure objects instead of instance identity +- it bypasses the Spritz access boundary and control-plane policy + +Spritz should own instance access through its own control plane. + +Cluster-level port-forwarding should remain an operator fallback, not the +standard user path. + +## Authentication And Security Boundary + +The forwarding contract should be explicit about what Spritz does and does not +authenticate. + +Spritz owns: + +- authenticating the caller to the Spritz control plane +- authorizing access to the target instance +- minting any short-lived forwarding credentials +- binding the local tunnel to the target instance and target remote port + +Spritz does not own: + +- application login inside the forwarded service +- application cookies +- bearer tokens for the app behind the forwarded port +- browser session reuse across origins + +This separation is desirable. + +Example: + +- a caller may use `spz port-forward` to reach `http://localhost:3000` +- the forwarded app may still ask the browser to log in normally +- that application auth flow is outside the Spritz forwarding contract + +Spritz should authenticate the tunnel, not the application protocol. + +## Security Model + +The default security posture should be strict. + +### Local bind defaults + +The command should bind locally to loopback only by default: + +- `127.0.0.1:` + +It should not expose the forwarded port on: + +- `0.0.0.0` +- public interfaces +- LAN interfaces + +If wider binding is ever supported, it should require an explicit flag and a +separate security review. It should not be part of the first implementation. + +### Remote target defaults + +The command should target loopback inside the instance by default: + +- `127.0.0.1:` + +This keeps the first implementation focused on the common safe case: + +- connecting to services that intentionally listen only inside the instance + +### Forwarding direction + +The first implementation should support local forwarding only. + +Allowed: + +- local machine -> instance + +Not in scope: + +- remote forwarding +- reverse tunnels +- listener exposure from the instance back toward the caller + +### Credential lifetime + +When implemented over SSH, the command should reuse the existing short-lived +certificate flow and host verification behavior used by `spz ssh`. + +The caller should not need to manage long-lived static keys. + +## Proposed Command Contract + +The initial CLI surface should stay simple and explicit. + +Preferred shape: + +```bash +spz port-forward --local 3000 --remote 3000 +``` + +Recommended flags for the first version: + +- positional instance name +- `--namespace ` when needed +- `--local ` +- `--remote ` +- `--print` + +Recommended defaults: + +- local bind host: `127.0.0.1` +- remote target host: `127.0.0.1` +- transport implementation: existing SSH path + +Recommended behavior: + +- print a clear one-line summary before attaching +- stay in the foreground +- exit cleanly on interrupt +- clean up temporary credentials on exit + +Example user-visible output: + +```text +Forwarding 127.0.0.1:3000 -> my-instance:127.0.0.1:3000 +Press Ctrl+C to stop. +``` + +## Relationship To `spz ssh` + +`spz ssh` should remain the raw shell-access command. + +`spz port-forward` should be a sibling command with a narrower purpose: + +- `spz ssh`: interactive shell access +- `spz port-forward`: local access to one private instance port + +The implementation may share most of the credential plumbing. + +That is good: + +- less duplicated auth logic +- one consistent trust and host-verification path +- one clear control-plane contract for instance access + +## Downstream Wrappers + +Spritz core should stop at the generic primitive. + +Downstream deployments may build convenience wrappers on top, for example: + +- frontend development helpers +- app-specific "open preview" flows +- wrapper commands that start a local browser automatically + +Those are valid downstream choices, but they should not define the core Spritz +contract. + +The holy grail shape is: + +- Spritz core provides `spz port-forward` +- downstream deployments compose deployment-specific UX on top of it + +That keeps Spritz portable while still enabling polished local workflows where +needed. + +## Implementation Plan + +### Phase 1: Core CLI Primitive + +- add `spz port-forward` +- reuse the existing SSH credential minting path +- support one local forward per command invocation +- keep both local and remote hosts pinned to loopback + +Acceptance criteria: + +- an authorized caller can reach a private instance port without manual + `ssh -L` composition +- the command requires no Kubernetes credentials +- the command targets one named instance, not a pod + +### Phase 2: CLI Help And Tests + +- document the new command in CLI help +- add help tests for the new usage line +- add command tests for printed SSH execution shape or equivalent command + plumbing + +Acceptance criteria: + +- the new command is discoverable through `spz --help` +- printed guidance stays generic and deployment-agnostic + +### Phase 3: Downstream Composition + +- allow downstreams to add wrappers without changing the core primitive +- document that application auth remains outside Spritz forwarding + +Acceptance criteria: + +- upstream Spritz stays generic +- downstream deployments can build local developer UX on top cleanly + +## Validation + +This architecture is successful when all of the following are true: + +- the standard path for instance port access is `spz port-forward`, not raw + `ssh -L` +- the caller does not need Kubernetes credentials +- the command works by instance identity rather than pod identity +- the default bind scope is local loopback only +- the application behind the forwarded port can keep its own auth model +- downstream wrappers can exist without forcing Spritz core to become + app-specific + +## References + +- `docs/2026-03-13-spz-audience-and-external-owner-guidance.md` +- `docs/2026-03-24-privileged-conversation-debug-and-test-architecture.md` +- `docs/2026-03-30-browser-websocket-auth-model.md` +- `cli/src/index.ts` From 0aaafe9f85911b31b42f165fc0ba549c57563d6d Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Mon, 6 Apr 2026 23:02:57 +0200 Subject: [PATCH 2/3] feat(cli): add spz port-forward command --- cli/skills/spz/SKILL.md | 8 ++ cli/src/index.ts | 65 +++++++++- cli/test/help.test.ts | 9 ++ cli/test/port-forward.test.ts | 224 ++++++++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 3 deletions(-) create mode 100644 cli/test/port-forward.test.ts diff --git a/cli/skills/spz/SKILL.md b/cli/skills/spz/SKILL.md index e53efcb..1e41dae 100644 --- a/cli/skills/spz/SKILL.md +++ b/cli/skills/spz/SKILL.md @@ -17,6 +17,7 @@ Typical cases: - suggest a DNS-safe random name for an instance - inspect an instance URL - attach to terminal access +- forward a local port into one instance - manage local `spz` profiles for different Spritz environments ## What Spritz is @@ -176,6 +177,12 @@ Open a terminal: spz terminal openclaw-tide-wind ``` +Forward a local port into one instance: + +```bash +spz port-forward openclaw-tide-wind --local 3000 --remote 3000 +``` + Use profiles: ```bash @@ -199,6 +206,7 @@ spz profile use staging - do not construct instance URLs yourself - use idempotency keys for any retried or externally triggered create operation - for service principals, expect create to succeed and list/delete to be denied unless extra scopes were granted +- prefer `spz port-forward` over raw `ssh -L` when you need local access to one private instance port ## Bundled skill usage diff --git a/cli/src/index.ts b/cli/src/index.ts index b6a0be4..4d2ef90 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -507,6 +507,7 @@ Usage: spritz open [--namespace ] spritz terminal [--namespace ] [--session ] [--transport ] [--print] spritz ssh [--namespace ] [--session ] [--transport ] [--print] + spritz port-forward [--namespace ] --local --remote [--print] spritz chat send (--instance | --conversation ) --message [--reason ] [--cwd ] [--title ] [--namespace <ns>] [--json] spritz profile list spritz profile current @@ -647,6 +648,21 @@ async function generateSSHKeypair() { return { dir, privateKeyPath, publicKey }; } +function parsePortFlag(flag: string): number { + const raw = argValue(flag)?.trim(); + if (!raw) { + throw new Error(`${flag} is required`); + } + if (!/^\d+$/.test(raw)) { + throw new Error(`${flag} must be a valid TCP port`); + } + const parsed = Number.parseInt(raw, 10); + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65535) { + throw new Error(`${flag} must be between 1 and 65535`); + } + return parsed; +} + function normalizeProfileName(value: string): string { const name = value.trim(); if (!name) { @@ -1139,9 +1155,14 @@ async function openTerminalWs(name: string, namespace: string | undefined, print } /** - * Opens a terminal session via SSH by minting a short-lived cert. + * Builds and executes an SSH command using a short-lived Spritz-minted cert. */ -async function openTerminalSSH(name: string, namespace: string | undefined, printOnly: boolean) { +async function openSSHConnection( + name: string, + namespace: string | undefined, + printOnly: boolean, + extraArgs: string[] = [], +) { const keypair = await generateSSHKeypair(); let keepTemp = false; try { @@ -1169,7 +1190,7 @@ async function openTerminalSSH(name: string, namespace: string | undefined, prin args.push('-o', 'StrictHostKeyChecking=accept-new'); } const port = data.port || 22; - args.push('-p', String(port), `${data.user}@${data.host}`); + args.push(...extraArgs, '-p', String(port), `${data.user}@${data.host}`); const commandLine = `${sshBinary} ${args.join(' ')}`; if (printOnly) { console.log(commandLine); @@ -1185,6 +1206,27 @@ async function openTerminalSSH(name: string, namespace: string | undefined, prin } } +/** + * Opens a terminal session via SSH by minting a short-lived cert. + */ +async function openTerminalSSH(name: string, namespace: string | undefined, printOnly: boolean) { + await openSSHConnection(name, namespace, printOnly); +} + +/** + * Opens a local loopback port forward to a loopback port inside one instance. + */ +async function openPortForward( + name: string, + namespace: string | undefined, + printOnly: boolean, + localPort: number, + remotePort: number, +) { + const forwardSpec = `127.0.0.1:${localPort}:127.0.0.1:${remotePort}`; + await openSSHConnection(name, namespace, printOnly, ['-N', '-L', forwardSpec]); +} + /** * Resolve namespace from CLI flags or active profile. */ @@ -1541,6 +1583,23 @@ async function main() { return; } + if (command === 'port-forward') { + const name = rest[0]; + if (!name) throw new Error('name is required'); + if (argValueInfo('--session').present) { + throw new Error('--session is not supported with port-forward'); + } + if (argValueInfo('--transport').present) { + throw new Error('--transport is not supported with port-forward'); + } + const ns = await resolveNamespace(); + const printOnly = hasFlag('--print'); + const localPort = parsePortFlag('--local'); + const remotePort = parsePortFlag('--remote'); + await openPortForward(name, ns, printOnly, localPort, remotePort); + return; + } + if (command === 'ssh' || command === 'terminal') { const name = rest[0]; if (!name) throw new Error('name is required'); diff --git a/cli/test/help.test.ts b/cli/test/help.test.ts index 81ef8fb..1eb8ae8 100644 --- a/cli/test/help.test.ts +++ b/cli/test/help.test.ts @@ -47,3 +47,12 @@ test('create help for agent audience prefers external owner guidance', async () assert.match(result.stdout, /tag the person who requested the instance/i); assert.match(result.stdout, /what was created and how to open it/i); }); + +test('top-level help lists port-forward command', async () => { + const result = await runCli(['--help']); + assert.equal(result.code, 0, result.stderr); + assert.match( + result.stdout, + /spritz port-forward <name> \[--namespace <ns>\] --local <port> --remote <port> \[--print\]/, + ); +}); diff --git a/cli/test/port-forward.test.ts b/cli/test/port-forward.test.ts new file mode 100644 index 0000000..63f5367 --- /dev/null +++ b/cli/test/port-forward.test.ts @@ -0,0 +1,224 @@ +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { mkdtempSync, writeFileSync, readFileSync } from 'node:fs'; +import http from 'node:http'; +import os from 'node:os'; +import test from 'node:test'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +const cliPath = path.join(__dirname, '..', 'src', 'index.ts'); + +function writeExecutable(filePath: string, contents: string) { + writeFileSync(filePath, contents, { encoding: 'utf8', mode: 0o755 }); +} + +function buildTestEnv(baseUrl: string, extraEnv: NodeJS.ProcessEnv = {}) { + const configDir = mkdtempSync(path.join(os.tmpdir(), 'spz-config-')); + return { + ...process.env, + SPRITZ_API_URL: baseUrl, + SPRITZ_BEARER_TOKEN: 'service-token', + SPRITZ_CONFIG_DIR: configDir, + ...extraEnv, + }; +} + +function listen(server: http.Server) { + return new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve)); +} + +function spawnCli(args: string[], env: NodeJS.ProcessEnv) { + return spawn(process.execPath, ['--import', 'tsx', cliPath, ...args], { + env, + stdio: ['ignore', 'pipe', 'pipe'], + }); +} + +test('port-forward --print prints the SSH command for the requested mapping', async (t) => { + let requestHeaders: http.IncomingHttpHeaders | null = null; + let requestPath = ''; + let requestMethod = ''; + let requestBody: any = null; + + const server = http.createServer((req, res) => { + requestHeaders = req.headers; + requestPath = req.url || ''; + requestMethod = req.method || ''; + const chunks: Buffer[] = []; + req.on('data', (chunk) => chunks.push(Buffer.from(chunk))); + req.on('end', () => { + requestBody = JSON.parse(Buffer.concat(chunks).toString('utf8')); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ + status: 'success', + data: { + host: '127.0.0.1', + user: 'spritz', + cert: 'ssh-ed25519-cert-v01@openssh.com AAAATEST', + port: 2222, + known_hosts: '[127.0.0.1]:2222 ssh-ed25519 AAAAKNOWNHOST', + }, + })); + }); + }); + await listen(server); + t.after(() => { + server.close(); + }); + const address = server.address(); + assert.ok(address && typeof address === 'object'); + + const tempDir = mkdtempSync(path.join(os.tmpdir(), 'spz-port-forward-')); + const fakeKeygen = path.join(tempDir, 'ssh-keygen'); + writeExecutable( + fakeKeygen, + `#!/usr/bin/env bash +set -euo pipefail +target="" +while (($#)); do + if [[ "$1" == "-f" ]]; then + target="$2" + shift 2 + continue + fi + shift +done +printf '%s\\n' 'PRIVATE KEY' > "$target" +printf '%s\\n' 'ssh-ed25519 AAAATEST generated@test' > "\${target}.pub" +chmod 600 "$target" "\${target}.pub" +`, + ); + + const child = spawnCli( + ['port-forward', 'devbox1', '--local', '3000', '--remote', '4000', '--print'], + buildTestEnv(`http://127.0.0.1:${address.port}/api`, { + SPRITZ_SSH_KEYGEN: fakeKeygen, + }), + ); + + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exitCode = await new Promise<number | null>((resolve) => child.on('exit', resolve)); + assert.equal(exitCode, 0, `spz port-forward --print should succeed: ${stderr}`); + + assert.equal(requestHeaders?.authorization, 'Bearer service-token'); + assert.equal(requestMethod, 'POST'); + assert.equal(requestPath, '/api/spritzes/devbox1/ssh'); + assert.deepEqual(requestBody, { + public_key: 'ssh-ed25519 AAAATEST generated@test\n', + }); + assert.match(stdout, / -N /); + assert.match(stdout, / -L 127\.0\.0\.1:3000:127\.0\.0\.1:4000 /); + assert.match(stdout, / -p 2222 /); + assert.match(stdout, /spritz@127\.0\.0\.1/); +}); + +test('port-forward executes the SSH client with the expected loopback mapping', async (t) => { + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on('data', (chunk) => chunks.push(Buffer.from(chunk))); + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ + status: 'success', + data: { + host: '127.0.0.1', + user: 'spritz', + cert: 'ssh-ed25519-cert-v01@openssh.com AAAATEST', + port: 2201, + known_hosts: '[127.0.0.1]:2201 ssh-ed25519 AAAAKNOWNHOST', + }, + })); + }); + }); + await listen(server); + t.after(() => { + server.close(); + }); + const address = server.address(); + assert.ok(address && typeof address === 'object'); + + const tempDir = mkdtempSync(path.join(os.tmpdir(), 'spz-port-forward-')); + const fakeKeygen = path.join(tempDir, 'ssh-keygen'); + const fakeSsh = path.join(tempDir, 'ssh'); + const sshArgsLog = path.join(tempDir, 'ssh-args.log'); + + writeExecutable( + fakeKeygen, + `#!/usr/bin/env bash +set -euo pipefail +target="" +while (($#)); do + if [[ "$1" == "-f" ]]; then + target="$2" + shift 2 + continue + fi + shift +done +printf '%s\\n' 'PRIVATE KEY' > "$target" +printf '%s\\n' 'ssh-ed25519 AAAATEST generated@test' > "\${target}.pub" +chmod 600 "$target" "\${target}.pub" +`, + ); + writeExecutable( + fakeSsh, + `#!/usr/bin/env bash +set -euo pipefail +printf '%s\\n' "$@" > "$SSH_ARGS_LOG" +`, + ); + + const child = spawnCli( + ['port-forward', 'devbox1', '--namespace', 'spritz', '--local', '3000', '--remote', '4000'], + buildTestEnv(`http://127.0.0.1:${address.port}/api`, { + SPRITZ_SSH_KEYGEN: fakeKeygen, + SPRITZ_SSH_BINARY: fakeSsh, + SSH_ARGS_LOG: sshArgsLog, + }), + ); + + let stderr = ''; + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exitCode = await new Promise<number | null>((resolve) => child.on('exit', resolve)); + assert.equal(exitCode, 0, `spz port-forward should succeed: ${stderr}`); + + const args = readFileSync(sshArgsLog, 'utf8').trim().split('\n'); + assert.ok(args.includes('-N')); + const localIndex = args.indexOf('-L'); + assert.notEqual(localIndex, -1); + assert.equal(args[localIndex + 1], '127.0.0.1:3000:127.0.0.1:4000'); + const portIndex = args.indexOf('-p'); + assert.notEqual(portIndex, -1); + assert.equal(args[portIndex + 1], '2201'); + assert.equal(args.at(-1), 'spritz@127.0.0.1'); +}); + +test('port-forward rejects missing remote port', async () => { + const child = spawnCli( + ['port-forward', 'devbox1', '--local', '3000'], + buildTestEnv('http://127.0.0.1:9/api'), + ); + + let stderr = ''; + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exitCode = await new Promise<number | null>((resolve) => child.on('exit', resolve)); + assert.notEqual(exitCode, 0, 'spz port-forward should reject missing --remote'); + assert.match(stderr, /--remote is required/); +}); From 2ccbc7645bb3cefa078e1d35d15ce9b96cda897e Mon Sep 17 00:00:00 2001 From: Onur Solmaz <onur@textcortex.com> Date: Mon, 6 Apr 2026 23:20:31 +0200 Subject: [PATCH 3/3] fix(api): support ssh port forwarding --- api/main.go | 3 + api/ssh_gateway.go | 242 ++++++++++++++++++++++++++++++++++++++-- api/ssh_gateway_test.go | 187 +++++++++++++++++++++++++++++++ 3 files changed, 425 insertions(+), 7 deletions(-) diff --git a/api/main.go b/api/main.go index d30000a..81e977a 100644 --- a/api/main.go +++ b/api/main.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log" + "net" "net/http" "net/url" "os" @@ -62,6 +63,8 @@ type server struct { instanceProxyTransport http.RoundTripper nameGeneratorFactory func(context.Context, string, string) (func() string, error) activityRecorder func(context.Context, string, string, time.Time) error + findRunningPodFunc func(context.Context, string, string, string) (*corev1.Pod, error) + openSSHPortForwardFunc func(context.Context, *corev1.Pod, uint32) (net.Conn, io.Closer, error) } func main() { diff --git a/api/ssh_gateway.go b/api/ssh_gateway.go index 024ff53..ca022c2 100644 --- a/api/ssh_gateway.go +++ b/api/ssh_gateway.go @@ -2,23 +2,46 @@ package main import ( "context" + "errors" "fmt" "io" "log" + "net" + "net/http" + "strconv" "strings" + "sync" "time" sshserver "github.com/gliderlabs/ssh" gossh "golang.org/x/crypto/ssh" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/portforward" "k8s.io/client-go/tools/remotecommand" + transportspdy "k8s.io/client-go/transport/spdy" spritzv1 "spritz.sh/operator/api/v1" ) const sshPrincipalDelimiter = ":" +type sshPortForwardActivityStartedKey struct{} + +type sshLocalForwardChannelData struct { + DestAddr string + DestPort uint32 + + OriginAddr string + OriginPort uint32 +} + +type closeFunc func() error + +func (f closeFunc) Close() error { + return f() +} + func formatSSHPrincipal(prefix, namespace, name string) string { return strings.Join([]string{prefix, namespace, name}, sshPrincipalDelimiter) } @@ -43,12 +66,7 @@ func (s *server) startSSHGateway(ctx context.Context) error { return nil } - server := &sshserver.Server{ - Addr: cfg.listenAddr, - Handler: s.handleSSHSession, - PublicKeyHandler: s.handleSSHAuth, - Version: "spritz", - } + server := s.newSSHGatewayServer() server.AddHostKey(cfg.hostSigner) errCh := make(chan error, 1) @@ -70,6 +88,21 @@ func (s *server) startSSHGateway(ctx context.Context) error { } } +func (s *server) newSSHGatewayServer() *sshserver.Server { + cfg := s.sshGateway + return &sshserver.Server{ + Addr: cfg.listenAddr, + Handler: s.handleSSHSession, + PublicKeyHandler: s.handleSSHAuth, + Version: "spritz", + ChannelHandlers: map[string]sshserver.ChannelHandler{ + "session": sshserver.DefaultSessionHandler, + "direct-tcpip": s.handleSSHPortForward, + }, + LocalPortForwardingCallback: s.allowSSHPortForwardDestination, + } +} + func (s *server) handleSSHAuth(ctx sshserver.Context, key sshserver.PublicKey) bool { cert, ok := key.(*gossh.Certificate) if !ok { @@ -114,7 +147,7 @@ func (s *server) handleSSHSession(sess sshserver.Session) { _ = sess.Exit(1) return } - s.startSSHActivityLoop(sess.Context(), spritz) + s.ensureSSHActivityLoop(sess.Context(), spritz) pty, winCh, hasPty := sess.Pty() sizeQueue := newTerminalSizeQueue() @@ -135,6 +168,201 @@ func (s *server) handleSSHSession(sess sshserver.Session) { _ = sess.Exit(0) } +func (s *server) handleSSHPortForward(srv *sshserver.Server, conn *gossh.ServerConn, newChan gossh.NewChannel, ctx sshserver.Context) { + var request sshLocalForwardChannelData + if err := gossh.Unmarshal(newChan.ExtraData(), &request); err != nil { + newChan.Reject(gossh.ConnectionFailed, "error parsing forward data: "+err.Error()) + return + } + if srv.LocalPortForwardingCallback == nil || !srv.LocalPortForwardingCallback(ctx, request.DestAddr, request.DestPort) { + newChan.Reject(gossh.Prohibited, "port forwarding is disabled") + return + } + + namespace, name, ok := parseSSHPrincipal(s.sshGateway.principalPrefix, ctx.User()) + if !ok { + log.Printf("spritz ssh: invalid forward principal value=%s", ctx.User()) + newChan.Reject(gossh.Prohibited, "invalid ssh principal") + return + } + + spritz := &spritzv1.Spritz{} + if err := s.client.Get(ctx, clientKey(namespace, name), spritz); err != nil { + log.Printf("spritz ssh: forward spritz not found name=%s namespace=%s err=%v", name, namespace, err) + newChan.Reject(gossh.ConnectionFailed, "spritz not ready") + return + } + + pod, err := s.findSSHGatewayPod(ctx, namespace, name, s.sshGateway.containerName) + if err != nil { + log.Printf("spritz ssh: forward pod not ready name=%s namespace=%s err=%v", name, namespace, err) + newChan.Reject(gossh.ConnectionFailed, "spritz not ready") + return + } + s.ensureSSHActivityLoop(ctx, spritz) + + upstream, cleanup, err := s.openSSHPortForward(ctx, pod, request.DestPort) + if err != nil { + log.Printf("spritz ssh: forward open failed name=%s namespace=%s port=%d err=%v", name, namespace, request.DestPort, err) + newChan.Reject(gossh.ConnectionFailed, "port forward unavailable") + return + } + + channel, requests, err := newChan.Accept() + if err != nil { + _ = upstream.Close() + _ = cleanup.Close() + return + } + go gossh.DiscardRequests(requests) + + var once sync.Once + closeAll := func() { + once.Do(func() { + _ = channel.Close() + _ = upstream.Close() + _ = cleanup.Close() + }) + } + + go func() { + defer closeAll() + _, _ = io.Copy(channel, upstream) + }() + go func() { + defer closeAll() + _, _ = io.Copy(upstream, channel) + }() +} + +func (s *server) allowSSHPortForwardDestination(ctx sshserver.Context, destinationHost string, destinationPort uint32) bool { + if !isLoopbackSSHForwardHost(destinationHost) { + log.Printf("spritz ssh: rejected forward user=%s host=%s port=%d", ctx.User(), destinationHost, destinationPort) + return false + } + return true +} + +func isLoopbackSSHForwardHost(host string) bool { + normalized := strings.TrimSpace(host) + normalized = strings.TrimPrefix(normalized, "[") + normalized = strings.TrimSuffix(normalized, "]") + if normalized == "" { + return false + } + if strings.EqualFold(normalized, "localhost") { + return true + } + ip := net.ParseIP(normalized) + return ip != nil && ip.IsLoopback() +} + +func (s *server) ensureSSHActivityLoop(ctx sshserver.Context, spritz *spritzv1.Spritz) { + if s == nil || spritz == nil { + return + } + ctx.Lock() + defer ctx.Unlock() + if started, ok := ctx.Value(sshPortForwardActivityStartedKey{}).(bool); ok && started { + return + } + ctx.SetValue(sshPortForwardActivityStartedKey{}, true) + s.startSSHActivityLoop(ctx, spritz) +} + +func (s *server) findSSHGatewayPod(ctx context.Context, namespace, name, container string) (*corev1.Pod, error) { + if s.findRunningPodFunc != nil { + return s.findRunningPodFunc(ctx, namespace, name, container) + } + return s.findRunningPod(ctx, namespace, name, container) +} + +func (s *server) openSSHPortForward(ctx context.Context, pod *corev1.Pod, remotePort uint32) (net.Conn, io.Closer, error) { + if s.openSSHPortForwardFunc != nil { + return s.openSSHPortForwardFunc(ctx, pod, remotePort) + } + if s.clientset == nil || s.restConfig == nil { + return nil, nil, errors.New("ssh port forwarding is not configured") + } + + req := s.clientset.CoreV1().RESTClient(). + Post(). + Resource("pods"). + Name(pod.Name). + Namespace(pod.Namespace). + SubResource("portforward") + transport, upgrader, err := transportspdy.RoundTripperFor(s.restConfig) + if err != nil { + return nil, nil, err + } + dialer := transportspdy.NewDialer(upgrader, &http.Client{Transport: transport}, http.MethodPost, req.URL()) + stopCh := make(chan struct{}) + readyCh := make(chan struct{}) + errCh := make(chan error, 1) + forwarder, err := portforward.NewOnAddresses( + dialer, + []string{"127.0.0.1"}, + []string{fmt.Sprintf("0:%d", remotePort)}, + stopCh, + readyCh, + io.Discard, + io.Discard, + ) + if err != nil { + close(stopCh) + return nil, nil, err + } + + go func() { + errCh <- forwarder.ForwardPorts() + }() + + select { + case <-readyCh: + case err := <-errCh: + close(stopCh) + return nil, nil, err + case <-ctx.Done(): + close(stopCh) + return nil, nil, ctx.Err() + } + + ports, err := forwarder.GetPorts() + if err != nil { + close(stopCh) + return nil, nil, err + } + if len(ports) != 1 { + close(stopCh) + return nil, nil, fmt.Errorf("unexpected forwarded port count: %d", len(ports)) + } + + localAddress := net.JoinHostPort("127.0.0.1", strconv.Itoa(int(ports[0].Local))) + upstream, err := (&net.Dialer{}).DialContext(ctx, "tcp", localAddress) + if err != nil { + close(stopCh) + return nil, nil, err + } + + var once sync.Once + cleanup := closeFunc(func() error { + once.Do(func() { + close(stopCh) + }) + return nil + }) + + go func() { + err := <-errCh + if err == nil || errors.Is(err, portforward.ErrLostConnectionToPod) || errors.Is(err, context.Canceled) { + return + } + log.Printf("spritz ssh: port-forward ended pod=%s namespace=%s remote_port=%d err=%v", pod.Name, pod.Namespace, remotePort, err) + }() + + return upstream, cleanup, nil +} + func sshActivityRefreshInterval(spec spritzv1.SpritzSpec, fallback time.Duration) time.Duration { interval := fallback if interval <= 0 { diff --git a/api/ssh_gateway_test.go b/api/ssh_gateway_test.go index 1eb5a8f..b420d2f 100644 --- a/api/ssh_gateway_test.go +++ b/api/ssh_gateway_test.go @@ -2,11 +2,19 @@ package main import ( "context" + "crypto/ed25519" + "crypto/rand" + "io" + "net" "sync/atomic" "testing" "time" + gossh "golang.org/x/crypto/ssh" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrlclientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" spritzv1 "spritz.sh/operator/api/v1" ) @@ -46,3 +54,182 @@ func TestStartSSHActivityLoopRefreshesWhileSessionIsOpen(t *testing.T) { t.Fatalf("expected repeated activity refreshes, got %d", calls.Load()) } } + +func TestIsLoopbackSSHForwardHost(t *testing.T) { + t.Parallel() + + cases := []struct { + host string + want bool + }{ + {host: "127.0.0.1", want: true}, + {host: "localhost", want: true}, + {host: "::1", want: true}, + {host: "[::1]", want: true}, + {host: "10.0.0.10", want: false}, + {host: "example.com", want: false}, + {host: "", want: false}, + } + + for _, tc := range cases { + if got := isLoopbackSSHForwardHost(tc.host); got != tc.want { + t.Fatalf("host %q => %t, want %t", tc.host, got, tc.want) + } + } +} + +func TestSSHGatewayPortForwardProxiesToInjectedUpstream(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + if err := spritzv1.AddToScheme(scheme); err != nil { + t.Fatalf("add spritz scheme: %v", err) + } + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("add core scheme: %v", err) + } + + caSigner := newTestSSHSigner(t) + hostSigner := newTestSSHSigner(t) + echoListener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen echo: %v", err) + } + defer echoListener.Close() + go func() { + conn, err := echoListener.Accept() + if err != nil { + return + } + defer conn.Close() + _, _ = io.Copy(conn, conn) + }() + + var activityCalls atomic.Int32 + var forwardedPort atomic.Int32 + spritz := &spritzv1.Spritz{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ssh-instance", + Namespace: "spritz-test", + }, + Spec: spritzv1.SpritzSpec{IdleTTL: "1m"}, + } + s := &server{ + client: ctrlclientfake.NewClientBuilder().WithScheme(scheme).WithObjects(spritz).Build(), + sshGateway: sshGatewayConfig{ + enabled: true, + listenAddr: "127.0.0.1:0", + user: "spritz", + principalPrefix: "spritz", + certTTL: time.Minute, + activityRefresh: time.Minute, + containerName: "spritz", + caSigner: caSigner, + hostSigner: hostSigner, + hostPublicKey: hostSigner.PublicKey(), + certChecker: &gossh.CertChecker{ + IsUserAuthority: func(auth gossh.PublicKey) bool { + return keysEqual(auth, caSigner.PublicKey()) + }, + }, + }, + activityRecorder: func(ctx context.Context, namespace, name string, when time.Time) error { + activityCalls.Add(1) + return nil + }, + findRunningPodFunc: func(ctx context.Context, namespace, name, container string) (*corev1.Pod, error) { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ssh-instance-pod", + Namespace: namespace, + }, + }, nil + }, + openSSHPortForwardFunc: func(ctx context.Context, pod *corev1.Pod, remotePort uint32) (net.Conn, io.Closer, error) { + forwardedPort.Store(int32(remotePort)) + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", echoListener.Addr().String()) + if err != nil { + return nil, nil, err + } + return conn, closeFunc(func() error { return nil }), nil + }, + } + + sshListener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen ssh: %v", err) + } + defer sshListener.Close() + + sshServer := s.newSSHGatewayServer() + sshServer.AddHostKey(hostSigner) + serverDone := make(chan error, 1) + go func() { + serverDone <- sshServer.Serve(sshListener) + }() + defer func() { + _ = sshServer.Close() + select { + case <-serverDone: + case <-time.After(200 * time.Millisecond): + } + }() + + principal := formatSSHPrincipal(s.sshGateway.principalPrefix, "spritz-test", "ssh-instance") + userSigner := newTestSSHSigner(t) + cert, err := s.signSSHCert(userSigner.PublicKey(), principal, "user-123") + if err != nil { + t.Fatalf("sign cert: %v", err) + } + certSigner, err := gossh.NewCertSigner(cert, userSigner) + if err != nil { + t.Fatalf("new cert signer: %v", err) + } + + client, err := gossh.Dial("tcp", sshListener.Addr().String(), &gossh.ClientConfig{ + User: principal, + Auth: []gossh.AuthMethod{gossh.PublicKeys(certSigner)}, + HostKeyCallback: gossh.InsecureIgnoreHostKey(), + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("dial ssh gateway: %v", err) + } + defer client.Close() + + forwardedConn, err := client.Dial("tcp", "127.0.0.1:3000") + if err != nil { + t.Fatalf("open direct-tcpip channel: %v", err) + } + defer forwardedConn.Close() + + if _, err := forwardedConn.Write([]byte("ping")); err != nil { + t.Fatalf("write forwarded conn: %v", err) + } + buffer := make([]byte, 4) + if _, err := io.ReadFull(forwardedConn, buffer); err != nil { + t.Fatalf("read forwarded conn: %v", err) + } + if string(buffer) != "ping" { + t.Fatalf("unexpected echoed payload %q", string(buffer)) + } + if got := forwardedPort.Load(); got != 3000 { + t.Fatalf("forwarded remote port = %d, want 3000", got) + } + if activityCalls.Load() == 0 { + t.Fatal("expected port forwarding to mark ssh activity") + } +} + +func newTestSSHSigner(t *testing.T) gossh.Signer { + t.Helper() + _, privateKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generate ed25519 key: %v", err) + } + signer, err := gossh.NewSignerFromKey(privateKey) + if err != nil { + t.Fatalf("new ssh signer: %v", err) + } + return signer +}