diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 831b19c..661f72f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -29,8 +29,7 @@ updates: commit-message: prefix: "deps" - # pnpm, which this ecosystem reads. ssh2 is held at the version patches/ is made for: a bump of - # it fails test/ssh2.security.test.ts until the patch is made again for the new version. + # pnpm, which this ecosystem reads. - package-ecosystem: npm directory: / schedule: @@ -38,8 +37,6 @@ updates: day: monday cooldown: default-days: 7 - ignore: - - dependency-name: ssh2 groups: js: patterns: ["*"] diff --git a/CLAUDE.md b/CLAUDE.md index c8654da..48d4a66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,11 +25,12 @@ path. | `src/logger.ts` | The output channel. | | `src/spin/authority.ts` | `spin+` parsed. | | `src/spin/resolver.ts` | The VS Code resolver: this editor's commit, progress, the local port, a refusal shown with its action. | -| `src/spin/open.ts` | The flow, with no VS Code in it so `test/e2e` drives it: `spin code`, `spin ssh-session`, ssh2 over `spin tunnel`, the server started. | +| `src/spin/open.ts` | The flow, with no VS Code in it so `test/e2e` drives it: the OpenSSH checked, `spin code`, `spin ssh-session`, ssh over `spin tunnel`, the server started. | | `src/spin/cli.ts` | The spin CLI, the only client of an installation. | | `src/spin/contract.ts` | The CLI's JSON answers. | -| `src/spin/keys.ts` | The workspace's certificate as an ssh2 key; the host key pinned to the launch claim's, as ssh-session answers it. | -| `src/spin/connection.ts` | The ssh2 connection: exec, a channel, the loopback port to the server. | +| `src/spin/openssh.ts` | The ssh on the PATH held to OpenSSH 9.9 or newer: the one key exchange a workspace accepts, `mlkem768x25519-sha256`. | +| `src/spin/keys.ts` | The host key pinned to the launch claim's, as ssh-session answers it. | +| `src/spin/connection.ts` | The system `ssh`: a ControlMaster over `spin tunnel`, exec through it, and the loopback port to the server (`ssh -W`). | | `src/spin/serverStart.ts` | The server from the extension disk, never downloaded, and its answer read. | | `src/spin/refusal.ts` | What a person is shown. | | `src/typings/` | VS Code's proposed `resolvers` API, as Microsoft wrote it. | @@ -47,7 +48,7 @@ path. ## Build & verify ```bash -go tool task install # pnpm through sfw; ssh2 patched as it installs +go tool task install # pnpm through sfw go tool task lint # biome go tool task typecheck # tsc --noEmit go tool task test # vitest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b46037f..0cdfcda 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,10 +5,11 @@ green. ## Developing -Needs Go (the tools are pinned in `go.mod` and `versions.yaml`), Node 22 and pnpm. +Needs Go (the tools are pinned in `go.mod` and `versions.yaml`), Node 22, pnpm, and OpenSSH 9.9 +or newer - `ssh`, `sshd` and `ssh-keygen`. ```sh -go tool task install # dependencies, through Socket Firewall; ssh2 is patched as it installs +go tool task install # dependencies, through Socket Firewall go tool task lint # Biome go tool task typecheck # tsc over src/ and test/ go tool task test # unit tests (Vitest), with vscode replaced by test/mocks/vscode.ts @@ -17,10 +18,11 @@ go tool task ci # what CI runs go tool task test:e2e SPIN= VSCODE_COMMIT= # the Spinbox resolver, below ``` -The unit tests need OpenSSH's `sshd` and `ssh-keygen`: `test/spin/keys.sshd.test.ts` runs a -real sshd that, like a workspace's, admits certificates only, and `test/spin/resolve.test.ts` -opens `spin+` from the extension's activation to a request its server answers, with -a stand-in CLI and workspace. +The unit tests run OpenSSH: `test/sshd.ts` starts a real sshd set up as a workspace's - it admits +certificates only and accepts the key exchange `mlkem768x25519-sha256` alone, which takes +OpenSSH 9.9 - `test/spin/connection.sshd.test.ts` connects to it with this machine's `ssh` as the +extension does, and `test/spin/resolve.test.ts` opens `spin+` from the extension's +activation to a request its server answers, with a stand-in CLI and that sshd as the workspace. ## The Spinbox resolver against a real installation @@ -34,11 +36,3 @@ other. `test/e2e/cli.e2e.test.ts` needs only the CLI (`SPIN_E2E_CLI`): the refus with no installation. F5 in VS Code runs the extension in an Extension Development Host (`.vscode/launch.json`). - -## ssh2 - -ssh2 1.17.0 lacks the ECDSA signature fix jeanp413's ssh2 fork carried, so pnpm patches it as it -installs (`patches/ssh2@1.17.0.patch`, `patchedDependencies` in `pnpm-workspace.yaml`), and -`test/ssh2.security.test.ts` holds the installed tree to the fix and to strict key exchange -(Terrapin). A bump of ssh2 installs unpatched, which that test refuses: make the patch again with -`pnpm patch ssh2@` and `pnpm patch-commit`, or delete it when ssh2 has the fix. diff --git a/README.md b/README.md index 642a533..41592d9 100644 --- a/README.md +++ b/README.md @@ -11,16 +11,19 @@ workspace uses. ## How a workspace opens +- `ssh -V` says this machine's OpenSSH is 9.9 or newer (below); - `spin code --no-open --vscode-commit --start` has the workspace running with the VS Code server for this commit on the extension disk the installation publishes; - `spin ssh-session -o json` opens a session, checking the workspace's launch claim, and answers the host key it names, the certificate the workspace signed for this machine's key and the - session's name; the connection is carried by `spin tunnel `, accepts that host key - only, and logs in with that certificate; + session's name; this machine's `ssh` connects, carried by `spin tunnel ` as its + ProxyCommand, accepts that host key only, and logs in with that certificate - one connection + (a ControlMaster) that everything after rides on; - the server is started from the extension disk, and **never downloaded**: a workspace without it is refused with what to do; -- VS Code reaches the server through a port on this machine's loopback; +- VS Code reaches the server through a port on this machine's loopback, each of its + connections an `ssh -W` through that one; - whatever refuses - a key the installation has not enrolled (`Spin-Refused: key-not-enrolled`), a profile not signed in, an installation that does not answer - is shown in VS Code with the CLI's words and the one thing to do. @@ -28,6 +31,24 @@ workspace uses. The CLI is the setting `remote.spin.cliPath`, or the one the Spinbox extension carries, or `spin` on the PATH. `Spinbox: Show Connection Log` shows what each open did. +### OpenSSH 9.9 or newer, on Linux or macOS + +A workspace's sshd accepts one key exchange, the post-quantum `mlkem768x25519-sha256`, which +OpenSSH added in 9.9, and refuses an older client with `no matching key exchange method found`. +The `ssh` on the PATH is asked its version first, and an older one is refused before anything +else runs, in VS Code: + +``` +Spinbox could not open my-workspace: ssh is OpenSSH 9.6 (OpenSSH_9.6p1 Ubuntu-3ubuntu13.20); a +workspace needs OpenSSH 9.9 or newer. Its sshd accepts only the post-quantum key exchange +mlkem768x25519-sha256, which OpenSSH added in 9.9, and refuses older clients. Update OpenSSH (on +macOS, 'brew install openssh') and open the workspace again. +``` + +An `ssh -V` it cannot read is let through, with a line in the log. The connection reads none of +your `~/.ssh/config`. Windows is refused: there is no spin CLI for it, and OpenSSH for Windows +has no ControlMaster. + ## Install The extension is `spin-stack.spinbox-remote`. It resolves a remote authority through VS Code's diff --git a/Taskfile.yml b/Taskfile.yml index 2b4f129..9265c4b 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -12,7 +12,7 @@ vars: tasks: install: - desc: Install the dependencies from the lockfile, through Socket Firewall (sfw). ssh2 is patched as pnpm installs it (patches/). + desc: Install the dependencies from the lockfile, through Socket Firewall (sfw). cmds: - sfw pnpm install --frozen-lockfile diff --git a/esbuild.mjs b/esbuild.mjs index 93c5272..b51b558 100644 --- a/esbuild.mjs +++ b/esbuild.mjs @@ -1,6 +1,4 @@ -// Bundles the extension into lib/extension.js. vscode is VS Code's own, never bundled. ssh2's -// native crypto binding and cpu-features are optional: ssh2 requires each inside a try and falls -// back to Node's crypto, so leaving them external ships a bundle with no native code. +// Bundles the extension into lib/extension.js. vscode is VS Code's own, never bundled. import { build } from "esbuild" await build({ @@ -10,7 +8,7 @@ await build({ platform: "node", format: "cjs", target: "node22", - external: ["vscode", "cpu-features", "*.node"], + external: ["vscode"], sourcemap: false, minify: true, logLevel: "warning", diff --git a/package.json b/package.json index 46d631a..b10f9ff 100644 --- a/package.json +++ b/package.json @@ -27,14 +27,9 @@ "test:e2e": "vitest run --config vitest.e2e.config.mts", "package": "vsce package --no-dependencies --out spinbox-remote.vsix" }, - "dependencies": { - "ssh2": "1.17.0" - }, "devDependencies": { "@biomejs/biome": "2.6.3", "@types/node": "22.20.5", - "@types/ssh2": "0.5.52", - "@types/ssh2-streams": "0.1.13", "@types/vscode": "1.146.0", "@vscode/vsce": "4.1.0", "esbuild": "0.28.2", diff --git a/patches/ssh2@1.17.0.patch b/patches/ssh2@1.17.0.patch deleted file mode 100644 index f419dbc..0000000 --- a/patches/ssh2@1.17.0.patch +++ /dev/null @@ -1,93 +0,0 @@ -diff --git a/lib/protocol/Protocol.js b/lib/protocol/Protocol.js -index 7302488102161b5b627f6f089d502caa6df0f356..4ed9619fe5b14cd8e03b4b8795df81557edc052a 100644 ---- a/lib/protocol/Protocol.js -+++ b/lib/protocol/Protocol.js -@@ -697,7 +697,14 @@ class Protocol { - } - - cbSign(packet, (signature) => { -- signature = convertSignature(signature, keyType); -+ // An OpenSSH certificate (`-cert-v01@openssh.com`) is offered -+ // under its own name, and signs as the key it certifies: the signature -+ // is that key's, named by that key's algorithm (PROTOCOL.certkeys). -+ const sigAlgo = keyAlgo.replace(/-cert-v01@openssh\.com$/, ''); -+ const sigAlgoLen = Buffer.byteLength(sigAlgo); -+ signature = convertSignature( -+ signature, keyType.replace(/-cert-v01@openssh\.com$/, '') -+ ); - if (signature === false) - throw new Error('Error while converting handshake signature'); - -@@ -705,7 +712,7 @@ class Protocol { - p = this._packetRW.write.allocStart; - packet = this._packetRW.write.alloc( - 1 + 4 + userLen + 4 + 14 + 4 + 9 + 1 + 4 + algoLen + 4 + pubKeyLen + 4 -- + 4 + algoLen + 4 + sigLen -+ + 4 + sigAlgoLen + 4 + sigLen - ); - - // TODO: simply copy from original "packet" to new `packet` to avoid -@@ -729,12 +736,12 @@ class Protocol { - writeUInt32BE(packet, pubKeyLen, p += algoLen); - packet.set(pubKey, p += 4); - -- writeUInt32BE(packet, 4 + algoLen + 4 + sigLen, p += pubKeyLen); -+ writeUInt32BE(packet, 4 + sigAlgoLen + 4 + sigLen, p += pubKeyLen); - -- writeUInt32BE(packet, algoLen, p += 4); -- packet.utf8Write(keyAlgo, p += 4, algoLen); -+ writeUInt32BE(packet, sigAlgoLen, p += 4); -+ packet.utf8Write(sigAlgo, p += 4, sigAlgoLen); - -- writeUInt32BE(packet, sigLen, p += algoLen); -+ writeUInt32BE(packet, sigLen, p += sigAlgoLen); - packet.set(signature, p += 4); - - // Servers shouldn't send packet type 60 in response to signed publickey -diff --git a/lib/protocol/utils.js b/lib/protocol/utils.js -index 26f4cab6de85953ccc7f9e7af35c4fa0905e8ca0..5c0ba76ab4cc1b1789115ec83c956ebfe96ac61b 100644 ---- a/lib/protocol/utils.js -+++ b/lib/protocol/utils.js -@@ -158,6 +158,25 @@ function writeUInt32BE(buf, value, offset) { - return offset; - } - -+// jeanp413/ssh2@a169f62's "Fix sigSSHToASN1": a big-endian integer as a DER -+// INTEGER - a zero in front when the high bit is set, and no zeros that do not -+// carry the sign. -+function convertToMpint(buf) { -+ const prependZeroCount = buf[0] & 0x80 ? 1 : 0; -+ let skipZeroCount = 0; -+ for (let i = 0; i < buf.length - 1 && buf[i] === 0; i++) { -+ if ((buf[i + 1] & 0x80) === 0) -+ skipZeroCount++; -+ } -+ const newBuf = Buffer.allocUnsafe( -+ buf.length + prependZeroCount - skipZeroCount -+ ); -+ buf.copy(newBuf, prependZeroCount, skipZeroCount, buf.length); -+ if (prependZeroCount) -+ newBuf[0] = 0; -+ return newBuf; -+} -+ - const utilBufferParser = makeBufferParser(); - - module.exports = { -@@ -270,10 +289,14 @@ module.exports = { - if (r === undefined || s === undefined) - return; - -+ // OpenSSL wants minimal, positive DER INTEGERs: without this, an -+ // ECDSA host key fails verification whenever the high bit of r or s -+ // is set. After the check above, so a truncated signature returns -+ // undefined rather than throwing. - const asnWriter = new Ber.Writer(); - asnWriter.startSequence(); -- asnWriter.writeBuffer(r, Ber.Integer); -- asnWriter.writeBuffer(s, Ber.Integer); -+ asnWriter.writeBuffer(convertToMpint(r), Ber.Integer); -+ asnWriter.writeBuffer(convertToMpint(s), Ber.Integer); - asnWriter.endSequence(); - return asnWriter.buffer; - } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index aace808..e0c052e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,16 +4,9 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false -patchedDependencies: - ssh2@1.17.0: e19040bc894509e68f83e869b83add27d179a70f36b2497d741536d727fe6937 - importers: .: - dependencies: - ssh2: - specifier: 1.17.0 - version: 1.17.0(patch_hash=e19040bc894509e68f83e869b83add27d179a70f36b2497d741536d727fe6937) devDependencies: '@biomejs/biome': specifier: 2.6.3 @@ -21,12 +14,6 @@ importers: '@types/node': specifier: 22.20.5 version: 22.20.5 - '@types/ssh2': - specifier: 0.5.52 - version: 0.5.52 - '@types/ssh2-streams': - specifier: 0.1.13 - version: 0.1.13 '@types/vscode': specifier: 1.146.0 version: 1.146.0 @@ -570,12 +557,6 @@ packages: '@types/node@22.20.5': resolution: {integrity: sha512-U2+DNr+wSjpsTS/wZGYHq7GcwfuSmKiKvoPvK22zwTlRhU91yOniN4qRR5KhIjvif7ysw/dz/hKmfDH0Ris4aA==} - '@types/ssh2-streams@0.1.13': - resolution: {integrity: sha512-faHyY3brO9oLEA0QlcO8N2wT7R0+1sHWZvQ+y3rMLwdY1ZyS1z0W3t65j9PqT4HmQ6ALzNe7RZlNuCNE0wBSWA==} - - '@types/ssh2@0.5.52': - resolution: {integrity: sha512-lbLLlXxdCZOSJMCInKH2+9V/77ET2J6NPQHpFI0kda61Dd1KglJs+fPQBchizmzYSOJBgdTajhPqBO1xxLywvg==} - '@types/vscode@1.146.0': resolution: {integrity: sha512-OtU25faeM997QQsW+D07FN9cX06j+EEI4aQaPw/UO3HWlCM39N1W7fG8ewhdZKluut31KffyMsmDzJzgMu6t8Q==} @@ -774,9 +755,6 @@ packages: resolution: {integrity: sha512-TQf59BsZnytt8GdJKLPfUZ54g/iaUL2OWDSFCCvMOhsHduDQxO8xC4PNeyIkVcA5KwL2phPSv0douC0fgWzmnA==} engines: {node: '>= 20'} - asn1@0.2.6: - resolution: {integrity: sha512-ix/FxPn0MDjeyJ7i/yoHGFt/EX6LyNbxSEhPPXODPL+KB0VPk86UYfL0lMdy+KCnv+fmvIzySwaK5COwqVbWTQ==} - assertion-error@2.0.1: resolution: {integrity: sha512-Izi8RQcffqCeNVgFigKli1ssklIbpHnCYc6AknXGYoB6grJqyeby7jv12JUQgmTAnIDnbck1uxksT4dzN3PWBA==} engines: {node: '>=12'} @@ -788,9 +766,6 @@ packages: resolution: {integrity: sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==} engines: {node: 18 || 20 || >=22} - bcrypt-pbkdf@1.0.2: - resolution: {integrity: sha512-qeFIXtP4MSoi6NLqO12WfqARWWuCKi2Rn/9hJLEmtB5yTNr9DqFWkJRCf2qShWzPeAMRnOgCrq0sg/KLv5ES9w==} - binaryextensions@6.11.0: resolution: {integrity: sha512-sXnYK/Ij80TO3lcqZVV2YgfKN5QjUWIRk/XSm2J/4bd/lPko3lvk0O4ZppH6m+6hB2/GTu+ptNwVFe1xh+QLQw==} engines: {node: '>=4'} @@ -808,10 +783,6 @@ packages: buffer-equal-constant-time@1.0.1: resolution: {integrity: sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==} - buildcheck@0.0.7: - resolution: {integrity: sha512-lHblz4ahamxpTmnsk+MNTRWsjYKv965MwOrSJyeD588rR3Jcu7swE+0wN5F+PbL5cjgu/9ObkhfzEPuofEMwLA==} - engines: {node: '>=10.0.0'} - bundle-name@4.1.1: resolution: {integrity: sha512-DdH81/zPLVS11EUgWq3tEu/xn+EzljlMYooDNdzWEnFha3R3NBMpMV1UqYIpjYHV/SgpFKMIX1Oh7o06SjM/oA==} engines: {node: '>=18'} @@ -836,10 +807,6 @@ packages: resolution: {integrity: sha512-Vw8qHK3bZM9y/P10u3Vib8o/DdkvA2OtPtZvD871QKjy74Wj1WSKFILMPRPSdUSx5RFK1arlJzEtA4PkFgnbuA==} engines: {node: '>=18'} - cpu-features@0.0.10: - resolution: {integrity: sha512-9IkYqtX3YHPCzoVg1Py+o9057a3i0fp7S530UWokCSaFVTc7CwXPRiOjRjBQQ18ZCNafx78YfnG+HALxtVmOGA==} - engines: {node: '>=10.0.0'} - debug@4.4.3: resolution: {integrity: sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==} engines: {node: '>=6.0'} @@ -1120,9 +1087,6 @@ packages: mute-stream@0.0.8: resolution: {integrity: sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==} - nan@2.30.0: - resolution: {integrity: sha512-P5mokDkSvCGBz6mZDD/2l+w9QoICRXdCyZ9ONGmTePr40oihJg3Kpn0YU3twKd6PJnwSlLBWNQeOT6yWEV2c4A==} - nanoid@3.3.20: resolution: {integrity: sha512-uKdg2G3GNCKQn9byYOpxbGqrT2fGO5KRt5J/8b3pok8rT6qxGWF6hxMyJiEYtAf+FVyYuD9hRaDqX5uPFYJ4ZQ==} engines: {node: ^10 || ^12 || ^13.7 || ^14 || >=15.0.1} @@ -1197,9 +1161,6 @@ packages: safe-buffer@5.2.1: resolution: {integrity: sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==} - safer-buffer@2.1.2: - resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} - sax@1.6.1: resolution: {integrity: sha512-42tBVwLWnaQvW5zc4HbZrTuWccECCZfBi92FDuwtqxasH+JbPB3/FOKb1m222K42R4WxuxzzMsTswfzgtSu64Q==} engines: {node: '>=11.0.0'} @@ -1232,10 +1193,6 @@ packages: resolution: {integrity: sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==} engines: {node: '>=0.10.0'} - ssh2@1.17.0: - resolution: {integrity: sha512-wPldCk3asibAjQ/kziWQQt1Wh3PgDFpC0XpwclzKcdT1vql6KeYxf5LIt4nlFkUeR8WuphYMKqUA56X4rjbfgQ==} - engines: {node: '>=10.16.0'} - std-env@4.3.0: resolution: {integrity: sha512-OtU/EgQ1kIm5KwqQpBC6ZEMXrZRui11w8zgfTWp8cdO9B8OaPsbA8bTHO2P+HNo1VlUTGMVBwPhydu6poeXiag==} @@ -1265,9 +1222,6 @@ packages: resolution: {integrity: sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==} engines: {node: '>=0.6.11 <=0.7.0 || >=0.7.3'} - tweetnacl@0.14.5: - resolution: {integrity: sha512-KXXFFdAbFXY4geFIwoyNK+f5Z1b7swfXABfL7HXCmoIWMKU3dmS26672A4EeQtDzLKy7SXmfBu51JolvEKwtGA==} - typed-rest-client@1.8.11: resolution: {integrity: sha512-5UvfMpd1oelmUPRbbaVnq+rHP7ng2cE4qoQkQeAqxRL6PklkxsM0g32/HL0yfvruK6ojQ5x8EE+HF4YV6DtuCA==} @@ -1795,15 +1749,6 @@ snapshots: dependencies: undici-types: 6.21.0 - '@types/ssh2-streams@0.1.13': - dependencies: - '@types/node': 22.20.5 - - '@types/ssh2@0.5.52': - dependencies: - '@types/node': 22.20.5 - '@types/ssh2-streams': 0.1.13 - '@types/vscode@1.146.0': {} '@typescript/typescript-aix-ppc64@7.0.2': @@ -1959,10 +1904,6 @@ snapshots: agent-base@9.0.0: {} - asn1@0.2.6: - dependencies: - safer-buffer: 2.1.2 - assertion-error@2.0.1: {} azure-devops-node-api@12.5.0: @@ -1972,10 +1913,6 @@ snapshots: balanced-match@4.0.4: {} - bcrypt-pbkdf@1.0.2: - dependencies: - tweetnacl: 0.14.5 - binaryextensions@6.11.0: dependencies: editions: 6.22.0 @@ -1990,9 +1927,6 @@ snapshots: buffer-equal-constant-time@1.0.1: {} - buildcheck@0.0.7: - optional: true - bundle-name@4.1.1: dependencies: run-applescript: 7.1.0 @@ -2013,12 +1947,6 @@ snapshots: commander@12.1.0: {} - cpu-features@0.0.10: - dependencies: - buildcheck: 0.0.7 - nan: 2.30.0 - optional: true - debug@4.4.3: dependencies: ms: 2.1.3 @@ -2283,9 +2211,6 @@ snapshots: mute-stream@0.0.8: {} - nan@2.30.0: - optional: true - nanoid@3.3.20: {} object-inspect@1.13.4: {} @@ -2364,8 +2289,6 @@ snapshots: safe-buffer@5.2.1: {} - safer-buffer@2.1.2: {} - sax@1.6.1: {} semver@7.8.6: {} @@ -2402,14 +2325,6 @@ snapshots: source-map-js@1.2.2: {} - ssh2@1.17.0(patch_hash=e19040bc894509e68f83e869b83add27d179a70f36b2497d741536d727fe6937): - dependencies: - asn1: 0.2.6 - bcrypt-pbkdf: 1.0.2 - optionalDependencies: - cpu-features: 0.0.10 - nan: 2.30.0 - std-env@4.3.0: {} structured-source@4.0.0: @@ -2433,8 +2348,6 @@ snapshots: tunnel@0.0.6: {} - tweetnacl@0.14.5: {} - typed-rest-client@1.8.11: dependencies: qs: 6.17.1 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index db73123..ef37eeb 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -10,13 +10,3 @@ allowBuilds: # Its postinstall picks the platform's vsce-sign binary, which signs a package for the # Marketplace: vsce package runs without it, and nothing here signs yet. '@vscode/vsce-sign': false - # ssh2 builds an optional native crypto binding and cpu-features probes the CPU; the bundle - # leaves both out (esbuild.mjs) and ssh2 falls back to Node's crypto, so neither is built. - ssh2: false - cpu-features: false - -# ssh2 1.17.0 still lacks the ECDSA signature fix jeanp413's ssh2 fork carried (patches/ has it, -# and test/ssh2.security.test.ts holds the installed tree to it). Keyed by the exact version: -# a bump installs unpatched until the patch is made again for it, which that test refuses. -patchedDependencies: - ssh2@1.17.0: patches/ssh2@1.17.0.patch diff --git a/src/spin/cli.ts b/src/spin/cli.ts index a21100c..dbac8cf 100644 --- a/src/spin/cli.ts +++ b/src/spin/cli.ts @@ -1,11 +1,11 @@ // The spin CLI, the resolver's only client of a Spinbox installation: `spin code` to have the // workspace running with the server for this editor's commit, `spin ssh-session` to open a -// session into it, and `spin tunnel` to carry the connection. The CLI keeps the credentials; -// this extension holds none. +// session into it, and `spin tunnel` to carry the connection, as ssh's ProxyCommand. The CLI +// keeps the credentials; this extension holds none. import { spawn } from 'child_process'; -import { Duplex } from 'stream'; import { type Answer, type CodeResult, readCodeAnswer, readSessionAnswer, type SessionResult } from './contract'; +import { escapeShellArg } from './serverStart'; export interface CodeOptions { workspace: string; @@ -25,7 +25,8 @@ interface Ran { export class SpinCli { constructor( private readonly binary: string, - private readonly env: NodeJS.ProcessEnv = process.env, + /** The CLI's environment, and the ssh's that runs it as ProxyCommand. */ + readonly env: NodeJS.ProcessEnv = process.env, ) { } private run(args: string[], timeoutMs: number): Promise { @@ -76,15 +77,11 @@ export class SpinCli { return readSessionAnswer(ran.stdout, ran.stderr, ran.exitCode); } - /** The connection to the workspace's sshd, carried by `spin tunnel` over the session ssh-session opened. */ - tunnel(session: string, onStderr: (line: string) => void): Duplex { - const child = spawn(this.binary, ['tunnel', session], { env: this.env, stdio: ['pipe', 'pipe', 'pipe'] }); - child.stderr.on('data', (d: Buffer) => d.toString().split(/\r?\n/).filter(Boolean).forEach(onStderr)); - const stream = Duplex.from({ readable: child.stdout, writable: child.stdin }); - // The proxy outlives nothing it carries: closing the stream ends it, and its end the stream. - stream.on('close', () => child.kill()); - child.on('exit', () => stream.destroy()); - child.on('error', e => stream.destroy(e)); - return stream; + /** + * ssh's ProxyCommand that carries the session ssh-session opened: `spin tunnel `, as + * the shell ssh runs it through reads it back. + */ + tunnelCommand(session: string): string { + return `${escapeShellArg(this.binary)} tunnel ${escapeShellArg(session)}`; } } diff --git a/src/spin/connection.ts b/src/spin/connection.ts index 97a7573..80eaf42 100644 --- a/src/spin/connection.ts +++ b/src/spin/connection.ts @@ -1,52 +1,205 @@ -// The ssh2 connection to a workspace's sshd, over the stream `spin tunnel` carries, and the -// local port VS Code reaches the workspace's server through. It began as -// sanketbajoria/ssh2-promise's SSHConnection, by way of jeanp413/open-remote-ssh. +// The connection to a workspace's sshd: this machine's own OpenSSH, so it negotiates the one key +// exchange the workspace accepts (openssh.ts), carried by `spin tunnel` as its ProxyCommand, held +// to the host key the launch claim names, and logged in with the certificate the workspace signed. +// It began as sanketbajoria/ssh2-promise's SSHConnection, by way of jeanp413/open-remote-ssh. +// +// One login, a ControlMaster, that everything rides on: the server's start, and each connection +// VS Code makes to the server, as `ssh -W` through it. A session is carried once (`spin tunnel`), +// so a second login would be a second session, certificate and claim check. OpenSSH for Windows +// has no ControlMaster, and there is no spin CLI for Windows, so it is refused first (open.ts). +import { type ChildProcess, spawn } from 'child_process'; +import * as fs from 'fs'; import * as net from 'net'; -import { Client, type ClientChannel, type ConnectConfig } from 'ssh2'; +import * as path from 'path'; +import { Duplex } from 'stream'; +import type { PinnedKey } from './keys'; +import { KEX_ALGORITHM } from './openssh'; + +export interface SSHLogin { + user: string; + /** What carries the connection: ssh runs it through a shell, as ProxyCommand. */ + proxyCommand: string; + /** The private key, or the .pub of a key ssh-agent holds. */ + identity: string; + /** The OpenSSH certificate the workspace signed over identity's public key. */ + certificate: string; + /** The only key the workspace's sshd may answer with. */ + hostKey: PinnedKey; + /** ssh's environment: the CLI's, which its ProxyCommand runs in. */ + env: NodeJS.ProcessEnv; + log: (line: string) => void; + readyTimeoutMs: number; +} + +/** The name ssh is given for the workspace: its known_hosts line's. The ProxyCommand routes. */ +const HOST = 'spinbox-workspace'; + +/** + * Where a connection keeps its known_hosts, certificate and control socket, in a directory of its + * own (mkdtemp: 0700, a name nobody predicts): /tmp, since a unix socket's address holds 104 bytes + * on macOS, whose os.tmpdir() is a long path, and ssh binds a name 17 longer before renaming it. + */ +const TMP = '/tmp'; + +/** A value as ssh's configuration reads it: a % is one of its tokens unless doubled. */ +function literal(value: string): string { + return value.replace(/%/g, '%%'); +} + +export interface SSHFiles { + knownHosts: string; + certificate: string; + controlPath: string; +} + +/** + * ssh's options for a connection, every run alike - the master and each client of it - so a + * client that finds no master is the same connection, held the same way, and not another. Its + * session is carried once, so such a client fails rather than logs in again. + */ +export function sshOptions(login: Pick, files: SSHFiles): string[] { + return [ + // Nothing of the person's own config: a Host * there - a ProxyJump, a ControlMaster, a + // forwarded agent - would change what this connection is. + '-F', '/dev/null', + '-o', `KexAlgorithms=${KEX_ALGORITHM}`, + '-o', `ProxyCommand=${literal(login.proxyCommand)}`, + // The host key: the one line in a known_hosts of this connection's own, and no other file. + '-o', `HostKeyAlias=${HOST}`, + '-o', `UserKnownHostsFile=${literal(files.knownHosts)}`, + '-o', 'GlobalKnownHostsFile=/dev/null', + '-o', 'StrictHostKeyChecking=yes', + '-o', 'UpdateHostKeys=no', + // The login: this key, as the certificate, and nothing else - no prompt, no password. + '-i', literal(login.identity), + '-o', `CertificateFile=${literal(files.certificate)}`, + '-o', 'IdentitiesOnly=yes', + '-o', 'PreferredAuthentications=publickey', + '-o', 'BatchMode=yes', + '-o', `ControlPath=${literal(files.controlPath)}`, + // Not ERROR: ssh says "no matching key exchange method found" at INFO, and that is the + // one refusal this connection exists to make clear. What it says goes to the log. + '-o', 'LogLevel=INFO', + ]; +} export default class SSHConnection { - private readonly client = new Client(); private ready: Promise | undefined; + private dir: string | undefined; + private options: string[] = []; + private master: ChildProcess | undefined; private readonly servers: net.Server[] = []; - constructor(private readonly config: ConnectConfig) { } + constructor(private readonly login: SSHLogin) { } + + private get destination(): string { + return `${this.login.user}@${HOST}`; + } connect(): Promise { - this.ready ??= new Promise((resolve, reject) => { - this.client - .on('ready', () => resolve()) - .on('error', reject) - // ssh2 says an error before it closes: a close with none is a peer that hung up. - .on('close', () => reject(new Error('the workspace closed the connection before it was ready'))) - .connect(this.config); - }); + this.ready ??= this.start(); return this.ready; } - async exec(command: string): Promise<{ stdout: string; stderr: string }> { - await this.connect(); - return new Promise((resolve, reject) => { - this.client.exec(command, (err, stream) => { - if (err) { - return reject(err); - } - let stdout = ''; - let stderr = ''; - stream.on('close', () => resolve({ stdout, stderr })) - .on('data', (d: Buffer) => { stdout += d.toString(); }) - .stderr.on('data', (d: Buffer) => { stderr += d.toString(); }); + private async start(): Promise { + const dir = fs.mkdtempSync(path.join(TMP, 'spinbox-')); + this.dir = dir; + const files: SSHFiles = { + knownHosts: path.join(dir, 'known_hosts'), + certificate: path.join(dir, 'id-cert.pub'), + controlPath: path.join(dir, 'mux'), + }; + const { hostKey } = this.login; + fs.writeFileSync(files.knownHosts, `${HOST} ${hostKey.type} ${hostKey.blob.toString('base64')}\n`, { mode: 0o600 }); + fs.writeFileSync(files.certificate, `${this.login.certificate.trim()}\n`, { mode: 0o600 }); + this.options = sshOptions(this.login, files); + + const master = this.ssh(['-M', '-N', '-o', 'ControlPersist=no'], ['ignore', 'ignore', 'pipe']); + this.master = master; + let said = ''; + master.stderr?.on('data', (d: Buffer) => { + said += d.toString(); + this.logLines('ssh', d); + }); + let gone = false; + // What it said is why: a host key not the claim's, a certificate refused, no key exchange + // in common, a session the tunnel could not carry. + const ended = new Promise((_, reject) => { + master.on('error', e => reject(new Error(`running ssh: ${e.message}`))); + master.on('close', code => { + gone = true; + reject(new Error(`ssh could not connect to the workspace (exit ${code}): ${said.trim() || 'it said nothing'}`)); }); }); + // An end after the connection is up is close()'s, or VS Code's to notice. + ended.catch(() => undefined); + // The master opens its control socket once it has logged in. + const up = (async () => { + const deadline = Date.now() + this.login.readyTimeoutMs; + while (!gone && Date.now() < deadline) { + if (await this.status(['-O', 'check']) === 0) { + return; + } + await new Promise(r => setTimeout(r, 100)); + } + if (!gone) { + throw new Error(`ssh did not connect to the workspace within ${this.login.readyTimeoutMs / 1000} s`); + } + })(); + try { + await Promise.race([up, ended]); + } catch (e) { + await this.close(); + throw e; + } + } + + /** ssh with this connection's options, args before the destination and after them. */ + private ssh(before: string[], stdio: ('ignore' | 'pipe')[], after: string[] = []): ChildProcess { + return spawn('ssh', [...this.options, ...before, this.destination, ...after], { env: this.login.env, stdio }); + } + + private logLines(who: string, d: Buffer) { + for (const line of d.toString().split(/\r?\n/).filter(Boolean)) { + this.login.log(`${who}: ${line}`); + } + } + + private status(before: string[]): Promise { + return new Promise(resolve => { + const child = this.ssh(before, ['ignore', 'ignore', 'ignore']); + child.on('error', () => resolve(null)); + child.on('close', code => resolve(code)); + }); } - async forwardOut(srcIP: string, srcPort: number, destIP: string, destPort: number): Promise { + async exec(command: string): Promise<{ stdout: string; stderr: string; code: number | null }> { await this.connect(); return new Promise((resolve, reject) => { - this.client.forwardOut(srcIP, srcPort, destIP, destPort, (err, stream) => err ? reject(err) : resolve(stream)); + const child = this.ssh(['-o', 'ControlMaster=no'], ['ignore', 'pipe', 'pipe'], [command]); + let stdout = ''; + let stderr = ''; + child.stdout?.on('data', (d: Buffer) => { stdout += d.toString(); }); + child.stderr?.on('data', (d: Buffer) => { stderr += d.toString(); }); + child.on('error', e => reject(new Error(`running ssh: ${e.message}`))); + child.on('close', code => resolve({ stdout, stderr, code })); }); } + /** A stream to remotePort on the workspace's loopback, through the master (`ssh -W`). */ + async forwardOut(remotePort: number): Promise { + await this.connect(); + const child = this.ssh(['-o', 'ControlMaster=no', '-W', `127.0.0.1:${remotePort}`], ['pipe', 'pipe', 'pipe']); + child.stderr?.on('data', (d: Buffer) => this.logLines('ssh -W', d)); + // Not destroyed when ssh exits: what it wrote before is still to be read, and the end of + // its stdout is the end of the stream. + const stream = Duplex.from({ readable: child.stdout, writable: child.stdin }); + stream.on('close', () => child.kill()); + child.on('error', e => stream.destroy(e)); + return stream; + } + /** * A port on this machine's loopback, chosen by the system, whose every connection is carried * to remotePort on the workspace's loopback. Only this machine reaches it. @@ -54,7 +207,7 @@ export default class SSHConnection { async forwardLocal(remotePort: number): Promise { await this.connect(); const server = net.createServer(socket => { - this.forwardOut('127.0.0.1', 0, '127.0.0.1', remotePort).then(stream => { + this.forwardOut(remotePort).then(stream => { // An 'error' on either end left unhandled would end the extension host: the // other end is destroyed instead. stream.on('error', () => socket.destroy()); @@ -69,8 +222,22 @@ export default class SSHConnection { }); } + /** Ends the master, which ends every client of it and the tunnel, and removes its files. */ async close(): Promise { - await Promise.all(this.servers.splice(0).map(s => new Promise(resolve => s.close(() => resolve())))); - this.client.end(); + // Not waited for: a server closes once its connections have, which the master's end ends. + for (const s of this.servers.splice(0)) { + s.close(); + } + const master = this.master; + this.master = undefined; + if (master && master.exitCode === null && master.signalCode === null) { + const closed = new Promise(resolve => master.once('close', () => resolve())); + master.kill(); + await closed; + } + if (this.dir) { + fs.rmSync(this.dir, { recursive: true, force: true }); + this.dir = undefined; + } } } diff --git a/src/spin/keys.ts b/src/spin/keys.ts index a339557..c7dc4b0 100644 --- a/src/spin/keys.ts +++ b/src/spin/keys.ts @@ -1,10 +1,6 @@ -// The two keys a Spinbox connection is held to: the certificate the workspace signed for this -// machine's key, which is how its sshd lets a person in (AuthorizedKeysFile none, -// TrustedUserCAKeys: spin's internal/sshauth), and the host key the workspace's launch claim -// names, which is the only key it may answer with. - -import { utils, type AuthHandlerResult, type ConnectConfig } from 'ssh2'; -import type { ParsedKey as Ssh2StreamsKey } from 'ssh2-streams'; +// The host key a Spinbox connection is held to: the one the workspace's launch claim names, which +// is the only key its sshd may answer with. ssh is given it as the one line of a known_hosts of +// the connection's own (connection.ts), written from the key as parsed here, never as answered. export interface PinnedKey { type: string; @@ -30,68 +26,3 @@ export function parseHostKey(line: string): PinnedKey { } return { type, blob }; } - -/** ssh2 1.x's parsed key, which the typings this tree pins (ssh2-streams') describe in part. */ -export type ParsedKey = Ssh2StreamsKey & { isPrivateKey(): boolean }; - -const CERT_SUFFIX = '-cert-v01@openssh.com'; - -// RSA is left out on purpose: ssh2 signs an ssh-rsa key with SHA-1 unless the server lists -// rsa-sha2 for that very key type, which it does not for a certificate, and OpenSSH refuses SHA-1. -const CERTIFIED_TYPES = new Set(['ssh-ed25519', 'ecdsa-sha2-nistp256', 'ecdsa-sha2-nistp384', 'ecdsa-sha2-nistp521']); - -/** - * privateKey, presented as the certificate certificateLine (an id-cert.pub line): offered under - * the certificate's name and blob, signed with the private key. ssh2 has no certificates of its - * own; patches/ssh2@1.17.0.patch names the signature by the certified key's algorithm, as - * OpenSSH expects. - */ -export function certificateKey(privateKey: ParsedKey, certificateLine: string): ParsedKey { - const [certType, base64] = certificateLine.trim().split(/\s+/); - if (!certType?.endsWith(CERT_SUFFIX) || !base64) { - throw new Error('the certificate is not an OpenSSH certificate line'); - } - const keyType = certType.slice(0, -CERT_SUFFIX.length); - if (!CERTIFIED_TYPES.has(keyType)) { - throw new Error(`a ${keyType} certificate cannot be used: Spinbox signs ed25519 and ecdsa keys`); - } - if (privateKey.type !== keyType || !privateKey.isPrivateKey()) { - throw new Error(`the certificate is for a ${keyType} key, and the identity is a ${privateKey.type} key`); - } - const blob = Buffer.from(base64, 'base64'); - return Object.create(privateKey, { - type: { value: certType }, - getPublicSSH: { value: () => blob }, - }) as ParsedKey; -} - -/** ssh2's authHandler that offers key, once, and then gives up: there is nothing else to try. */ -export function offerOnce(username: string, key: ParsedKey): NonNullable { - let offered = false; - return (_methodsLeft, _partialSuccess, callback) => { - if (offered) { - return callback(false); - } - offered = true; - const offer: AuthHandlerResult = { type: 'publickey', username, key }; - callback(offer); - }; -} - -/** Reads the identity at path as a private key, refusing what is not one. */ -export function parsePrivateKey(data: Buffer, path: string): ParsedKey { - const parsed = utils.parseKey(data); - const key = (Array.isArray(parsed) ? parsed[0] : parsed) as ParsedKey | Error | undefined; - if (!key || key instanceof Error || !key.isPrivateKey()) { - throw new Error(`${path} is not a private key this extension can read${key instanceof Error ? `: ${key.message}` : ''}`); - } - return key; -} - -/** - * ssh2's hostVerifier for a connection that may meet only pinned: with no hostHash, ssh2 hands - * it the key's wire encoding, which has to be pinned's byte for byte. - */ -export function pinnedHostVerifier(pinned: PinnedKey): (key: Buffer) => boolean { - return (key: Buffer) => Buffer.isBuffer(key) && key.equals(pinned.blob); -} diff --git a/src/spin/open.ts b/src/spin/open.ts index 5929350..006430d 100644 --- a/src/spin/open.ts +++ b/src/spin/open.ts @@ -1,15 +1,15 @@ // Opens a Spinbox workspace for VS Code, with no VS Code in it so the e2e harness drives it as -// the resolver does: the workspace running with this commit's server (`spin code`), a session -// opened by the CLI (`spin ssh-session`), ssh2 over the CLI's tunnel (`spin tunnel`) held to the -// launch claim's host key and authenticated by the workspace's certificate, and the server -// started from the extension disk. Every refusal is a SpinRefusal saying what to do. +// the resolver does: this machine's OpenSSH asked whether it can connect at all, the workspace +// running with this commit's server (`spin code`), a session opened by the CLI +// (`spin ssh-session`), ssh over the CLI's tunnel (`spin tunnel`) held to the launch claim's host +// key and authenticated by the workspace's certificate, and the server started from the extension +// disk. Every refusal is a SpinRefusal saying what to do. import * as crypto from 'crypto'; -import * as fs from 'fs'; -import type { ConnectConfig } from 'ssh2'; import type { SpinCli } from './cli'; import SSHConnection from './connection'; -import { certificateKey, offerOnce, parseHostKey, parsePrivateKey, pinnedHostVerifier } from './keys'; +import { parseHostKey } from './keys'; +import { checkOpenSSH } from './openssh'; import { refusalOf, SpinRefusal } from './refusal'; import { readServerStart, serverStartScript, ServerStartError, shCommand } from './serverStart'; @@ -34,6 +34,20 @@ export interface OpenedWorkspace { const WORKSPACE_USER = 'root'; export async function openWorkspace(o: OpenOptions): Promise { + // Before the workspace is started for a connection that could not be made. + if (process.platform === 'win32') { + throw new SpinRefusal({ + code: 'platform_unsupported', + message: 'Spinbox opens workspaces from Linux and macOS: the spin CLI is built for those alone, ' + + 'and OpenSSH for Windows has no ControlMaster, which the connection to the server rides on.', + temporary: false, + }); + } + const ssh = await checkOpenSSH(o.cli.env); + if (ssh.warning) { + o.log(ssh.warning); + } + o.log(`spin code ${o.workspace} (VS Code ${o.version ?? ''} ${o.commit})`); const answer = await o.cli.code({ workspace: o.workspace, commit: o.commit, version: o.version, start: o.start }); if ('error' in answer) { @@ -54,16 +68,15 @@ export async function openWorkspace(o: OpenOptions): Promise { if ('error' in session) { throw new SpinRefusal(refusalOf(session.error)); } - const { identity, certificate, host_key: hostKey } = session.ok; - const pinned = parseHostKey(hostKey); - const key = certificateKey(parsePrivateKey(fs.readFileSync(identity), identity), certificate); const connection = new SSHConnection({ - sock: o.cli.tunnel(session.ok.session, line => o.log(`spin tunnel: ${line}`)), - username: WORKSPACE_USER, - readyTimeout: o.readyTimeoutMs ?? 60_000, - strictVendor: false, - hostVerifier: pinnedHostVerifier(pinned) as unknown as ConnectConfig['hostVerifier'], - authHandler: offerOnce(WORKSPACE_USER, key), + user: WORKSPACE_USER, + proxyCommand: o.cli.tunnelCommand(session.ok.session), + identity: session.ok.identity, + certificate: session.ok.certificate, + hostKey: parseHostKey(session.ok.host_key), + env: o.cli.env, + log: o.log, + readyTimeoutMs: o.readyTimeoutMs ?? 60_000, }); try { await connection.connect(); diff --git a/src/spin/openssh.ts b/src/spin/openssh.ts new file mode 100644 index 0000000..b9fc4e6 --- /dev/null +++ b/src/spin/openssh.ts @@ -0,0 +1,84 @@ +// The OpenSSH a Spinbox connection runs. A workspace's sshd accepts one key exchange, +// mlkem768x25519-sha256 (spin-stack/spin-machine#245): post-quantum, at what a curve25519 login +// costs. OpenSSH has it from 9.9, and an older client is refused with "no matching key exchange +// method found", which says nothing of why - so the version is asked first, and an older one is +// refused here in words that do, as the spin CLI refuses it (cmd/cli/remote/openssh.go). + +import { execFile } from 'child_process'; +import { SpinRefusal } from './refusal'; + +export const KEX_ALGORITHM = 'mlkem768x25519-sha256'; + +const MIN_OPENSSH = { major: 9, minor: 9 }; +const MIN = `${MIN_OPENSSH.major}.${MIN_OPENSSH.minor}`; + +/** The version in what `ssh -V` prints: OpenSSH_9.6p1 Ubuntu-..., OpenSSH_9.9p2, OpenSSH_for_Windows_9.5p1. */ +const OPENSSH = /\bOpenSSH(?:_for_Windows)?_(\d+)\.(\d+)/; + +/** + * What `ssh -V` printing banner means for connecting: a refusal of an OpenSSH older than 9.9, and + * a warning where banner names no OpenSSH version at all. That one is let through: a client this + * cannot read - a vendor's build, a wrapper - may well negotiate the key exchange, and refusing it + * would lock out somebody whose ssh works; the warning says what its refusal would mean. + */ +export function checkBanner(banner: string): { warning?: string } { + const said = banner.trim(); + const match = OPENSSH.exec(said); + if (!match) { + return { + warning: `could not tell which OpenSSH ssh is (ssh -V said ${JSON.stringify(said)}). A workspace accepts only the ` + + `post-quantum key exchange ${KEX_ALGORITHM}, which OpenSSH has from ${MIN}: an older ssh is refused with ` + + '"no matching key exchange method found"', + }; + } + const major = Number(match[1]); + const minor = Number(match[2]); + if (major < MIN_OPENSSH.major || (major === MIN_OPENSSH.major && minor < MIN_OPENSSH.minor)) { + throw new SpinRefusal({ + code: 'ssh_outdated', + message: `ssh is OpenSSH ${major}.${minor} (${said.split(',')[0]}); a workspace needs OpenSSH ${MIN} or newer. ` + + `Its sshd accepts only the post-quantum key exchange ${KEX_ALGORITHM}, which OpenSSH added in ${MIN}, ` + + 'and refuses older clients. Update OpenSSH (on macOS, \'brew install openssh\') and open the workspace again.', + temporary: false, + }); + } + return {}; +} + +/** + * Each PATH's ssh, asked once: a reconnect does not ask again. A refusal is not kept, so an ssh + * updated after it is asked again on the next open, as the refusal says to. + */ +const checked = new Map>(); + +/** + * Refuses the ssh env's PATH runs when it cannot connect to a workspace - none, or one older than + * OpenSSH 9.9 - before anything is asked of the installation. One whose version it cannot read + * passes, with a warning to log. + */ +export function checkOpenSSH(env: NodeJS.ProcessEnv): Promise<{ warning?: string }> { + const key = env.PATH ?? ''; + let check = checked.get(key); + if (!check) { + check = new Promise((resolve, reject) => { + // -V prints to stderr, and exits 0. + execFile('ssh', ['-V'], { env, timeout: 10_000 }, (err, stdout, stderr) => { + if ((err as NodeJS.ErrnoException | null)?.code === 'ENOENT') { + return reject(new SpinRefusal({ + code: 'ssh_not_found', + message: `a workspace is reached with OpenSSH ${MIN} or newer, and there is no ssh on this machine's PATH`, + temporary: false, + })); + } + try { + resolve(checkBanner(`${stderr}${stdout}`)); + } catch (e) { + reject(e); + } + }); + }); + checked.set(key, check); + check.catch(() => checked.delete(key)); + } + return check; +} diff --git a/src/spin/serverStart.ts b/src/spin/serverStart.ts index d6da802..20bcc28 100644 --- a/src/spin/serverStart.ts +++ b/src/spin/serverStart.ts @@ -105,6 +105,6 @@ export function shCommand(script: string): string { } /** value as one POSIX shell word: single-quoted, each ' inside closed, escaped and reopened. */ -function escapeShellArg(value: string): string { +export function escapeShellArg(value: string): string { return `'${value.replace(/'/g, `'\\''`)}'`; } diff --git a/test/e2e/spin.e2e.test.ts b/test/e2e/spin.e2e.test.ts index 08ccb14..0107704 100644 --- a/test/e2e/spin.e2e.test.ts +++ b/test/e2e/spin.e2e.test.ts @@ -59,7 +59,7 @@ async function refusalOf(p: Promise) { /** An HTTP request to the server through the connection, answering its status line. */ async function statusLine(o: OpenedWorkspace): Promise { - const channel = await o.connection.forwardOut('127.0.0.1', 0, '127.0.0.1', o.port); + const channel = await o.connection.forwardOut(o.port); return new Promise((resolve, reject) => { let data = ''; channel.on('data', (d: Buffer) => { data += d.toString(); }); diff --git a/test/spin/connection.sshd.test.ts b/test/spin/connection.sshd.test.ts new file mode 100644 index 0000000..72e4d94 --- /dev/null +++ b/test/spin/connection.sshd.test.ts @@ -0,0 +1,126 @@ +import * as fs from 'fs'; +import * as http from 'http'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import SSHConnection, { type SSHLogin } from '../../src/spin/connection'; +import { parseHostKey } from '../../src/spin/keys'; +import { escapeShellArg } from '../../src/spin/serverStart'; +import { keygen, type Sshd, startSshd } from '../sshd'; + +// The connection, this machine's OpenSSH, against a real sshd set up as a workspace's is: only a +// certificate the workspace's CA signed lets anyone in, only the host key the claim names is +// accepted, and only mlkem768x25519-sha256 is negotiated. The ProxyCommand stands in for +// `spin tunnel`: it carries the connection to the sshd's port. + +const user = os.userInfo().username; +let dir: string; +let workspace: Sshd; +let classical: Sshd; +let web: http.Server; +let webPort: number; +const opened: SSHConnection[] = []; + +// `spin tunnel` as ssh runs it: stdin and stdout carried to a port. +const PROXY = `const s = require('net').connect(Number(process.argv[2]), '127.0.0.1'); +process.stdin.pipe(s); s.pipe(process.stdout); s.on('close', () => process.exit(0)); s.on('error', () => process.exit(1)); +`; + +beforeAll(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'spin-connection-')); + keygen('-t', 'ed25519', '-N', '', '-f', path.join(dir, 'ca')); + keygen('-t', 'ed25519', '-N', '', '-f', path.join(dir, 'host')); + for (const [name, type] of [['user-ed25519', 'ed25519'], ['user-ecdsa', 'ecdsa'], ['stranger', 'ed25519']]) { + keygen('-t', type, '-N', '', '-f', path.join(dir, name)); + } + keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', user, '-V', '+5m', path.join(dir, 'user-ed25519.pub')); + keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', user, '-V', '+5m', path.join(dir, 'user-ecdsa.pub')); + // A certificate for another principal: signed by the same CA, for nobody here. + keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', 'someone-else', '-V', '+5m', path.join(dir, 'stranger.pub')); + fs.writeFileSync(path.join(dir, 'proxy.js'), PROXY); + workspace = await startSshd(dir, 'workspace'); + // The same sshd as an OpenSSH before 9.9 would meet it: curve25519, and no ML-KEM. + classical = await startSshd(dir, 'classical', { kex: 'curve25519-sha256' }); + web = http.createServer((_req, res) => res.end('the server')); + webPort = await new Promise(resolve => web.listen(0, '127.0.0.1', () => resolve((web.address() as net.AddressInfo).port))); +}, 30_000); + +afterAll(async () => { + await Promise.all(opened.map(c => c.close())); + workspace?.process.kill(); + classical?.process.kill(); + web?.close(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +function hostKey(file = 'host.pub') { + return parseHostKey(fs.readFileSync(path.join(dir, file), 'utf8').trim().split(' ').slice(0, 2).join(' ')); +} + +function login(name: string, o: Partial & { port?: number; certificate?: string } = {}): SSHConnection { + const said: string[] = []; + const c = new SSHConnection({ + user, + proxyCommand: `${escapeShellArg(process.execPath)} ${escapeShellArg(path.join(dir, 'proxy.js'))} ${o.port ?? workspace.port}`, + identity: path.join(dir, name), + certificate: o.certificate ?? fs.readFileSync(path.join(dir, `${name}-cert.pub`), 'utf8'), + hostKey: o.hostKey ?? hostKey(), + env: process.env, + log: line => said.push(line), + readyTimeoutMs: 20_000, + }); + opened.push(c); + return c; +} + +/** What connecting and running `echo ok` answers: the output, or the error. */ +async function run(c: SSHConnection): Promise { + try { + await c.connect(); + return (await c.exec('echo ok')).stdout.trim(); + } catch (e) { + return `error: ${(e as Error).message}`; + } +} + +describe('a connection to a workspace sshd', () => { + it.each(['user-ed25519', 'user-ecdsa'])('logs in with %s presented as its certificate', async (name) => { + expect(await run(login(name))).toBe('ok'); + }, 30_000); + + it('is refused for the same key without its certificate', async () => { + const out = await run(login('user-ed25519', { certificate: fs.readFileSync(path.join(dir, 'user-ed25519.pub'), 'utf8') })); + expect(out).toMatch(/^error: ssh could not connect to the workspace .*Permission denied \(publickey\)/s); + }, 30_000); + + it('is refused for a certificate for another principal', async () => { + expect(await run(login('stranger'))).toMatch(/^error: ssh could not connect to the workspace .*Permission denied \(publickey\)/s); + }, 30_000); + + it('hangs up on a host whose key is not the pinned one', async () => { + const out = await run(login('user-ed25519', { hostKey: hostKey('ca.pub') })); + expect(out).toMatch(/^error: ssh could not connect to the workspace .*Host key verification failed/s); + }, 30_000); + + it('negotiates ML-KEM or nothing', async () => { + const out = await run(login('user-ed25519', { port: classical.port })); + expect(out).toMatch(/^error: ssh could not connect to the workspace .*no matching key exchange method found/s); + }, 30_000); + + it('carries a loopback port to the workspace\'s, for as long as it is open', async () => { + const c = login('user-ed25519'); + const port = await c.forwardLocal(webPort); + for (let i = 0; i < 2; i++) { + const body = await new Promise((resolve, reject) => { + http.get({ host: '127.0.0.1', port, path: '/' }, res => { + let b = ''; + res.on('data', (d: Buffer) => { b += d.toString(); }).on('end', () => resolve(b)); + }).on('error', reject); + }); + expect(body).toBe('the server'); + } + await c.close(); + await expect(c.exec('echo ok').then(r => r.code)).resolves.not.toBe(0); + }, 30_000); +}); diff --git a/test/spin/keys.sshd.test.ts b/test/spin/keys.sshd.test.ts deleted file mode 100644 index 82d5ff1..0000000 --- a/test/spin/keys.sshd.test.ts +++ /dev/null @@ -1,166 +0,0 @@ -import { execFileSync, spawn, type ChildProcess } from 'child_process'; -import * as fs from 'fs'; -import * as net from 'net'; -import * as os from 'os'; -import * as path from 'path'; -import { Client, type ConnectConfig } from 'ssh2'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; -import { certificateKey, offerOnce, parseHostKey, parsePrivateKey, pinnedHostVerifier } from '../../src/spin/keys'; - -// Against a real OpenSSH sshd set up as a workspace's is (spin's internal/sshauth): -// AuthorizedKeysFile none, so only a certificate a trusted CA signed lets anyone in. ssh-keygen -// makes the CA, the host key and the certificates, as the workspace's supervisor does. - -const user = os.userInfo().username; -let dir: string; -let sshd: ChildProcess; -let port: number; - -function keygen(...args: string[]) { - execFileSync('ssh-keygen', ['-q', ...args], { stdio: 'ignore' }); -} - -function freePort(): Promise { - return new Promise((resolve, reject) => { - const s = net.createServer().listen(0, '127.0.0.1', () => { - const p = (s.address() as net.AddressInfo).port; - s.close(() => resolve(p)); - }).on('error', reject); - }); -} - -async function waitForPort(p: number) { - for (let i = 0; i < 100; i++) { - const open = await new Promise(resolve => { - const c = net.connect(p, '127.0.0.1', () => { c.destroy(); resolve(true); }).on('error', () => resolve(false)); - }); - if (open) { - return; - } - await new Promise(r => setTimeout(r, 50)); - } - throw new Error(`sshd did not listen on ${p}`); -} - -beforeAll(async () => { - dir = fs.mkdtempSync(path.join(os.tmpdir(), 'spin-keys-')); - keygen('-t', 'ed25519', '-N', '', '-f', path.join(dir, 'ca')); - keygen('-t', 'ed25519', '-N', '', '-f', path.join(dir, 'host')); - for (const [name, type] of [['user-ed25519', 'ed25519'], ['user-ecdsa', 'ecdsa'], ['stranger', 'ed25519']]) { - keygen('-t', type, '-N', '', '-f', path.join(dir, name)); - } - keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', user, '-V', '+5m', path.join(dir, 'user-ed25519.pub')); - keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', user, '-V', '+5m', path.join(dir, 'user-ecdsa.pub')); - // A certificate for another principal: signed by the same CA, for nobody here. - keygen('-s', path.join(dir, 'ca'), '-I', 'spin-test', '-n', 'someone-else', '-V', '+5m', path.join(dir, 'stranger.pub')); - port = await freePort(); - fs.writeFileSync(path.join(dir, 'sshd_config'), [ - `ListenAddress 127.0.0.1:${port}`, - `HostKey ${path.join(dir, 'host')}`, - `TrustedUserCAKeys ${path.join(dir, 'ca.pub')}`, - 'AuthorizedKeysFile none', - 'PasswordAuthentication no', - 'KbdInteractiveAuthentication no', - 'UsePAM no', - 'StrictModes no', - `PidFile ${path.join(dir, 'sshd.pid')}`, - 'LogLevel ERROR', - '', - ].join('\n')); - // OpenSSH 9.8+ shuts out a source after failed logins, and the refusals below are failed - // logins from 127.0.0.1: without this, a test after them would meet a closed door. An older - // sshd (Ubuntu 24.04's 9.6) has no penalties, and refuses the option. - fs.appendFileSync(path.join(dir, 'sshd_config'), 'PerSourcePenalties no\n'); - try { - execFileSync('/usr/sbin/sshd', ['-t', '-f', path.join(dir, 'sshd_config')], { stdio: 'ignore' }); - } catch { - const config = fs.readFileSync(path.join(dir, 'sshd_config'), 'utf8'); - fs.writeFileSync(path.join(dir, 'sshd_config'), config.replace('PerSourcePenalties no\n', '')); - } - sshd = spawn('/usr/sbin/sshd', ['-D', '-e', '-f', path.join(dir, 'sshd_config')], { stdio: ['ignore', 'ignore', 'pipe'] }); - let said = ''; - sshd.stderr?.on('data', (d: Buffer) => { said += d.toString(); }); - sshd.on('error', e => { said += e.message; }); - await waitForPort(port).catch(e => { - throw new Error(`${(e as Error).message}: ${said || `exit ${sshd.exitCode}`}`); - }); -}, 30_000); - -afterAll(() => { - sshd?.kill(); - fs.rmSync(dir, { recursive: true, force: true }); -}); - -/** Connects as user and runs `echo ok`, answering what the outside sees: the output, or the error. */ -function run(config: ConnectConfig): Promise { - return new Promise(resolve => { - const c = new Client(); - c.on('ready', () => { - c.exec('echo ok', (err, stream) => { - if (err) { - c.end(); - return resolve(`exec: ${err.message}`); - } - let out = ''; - stream.on('data', (d: Buffer) => { out += d.toString(); }).on('close', () => { c.end(); resolve(out.trim()); }); - }); - }).on('error', e => resolve(`error: ${e.message}`)).connect({ host: '127.0.0.1', port, username: user, readyTimeout: 10_000, ...config }); - }); -} - -function pinnedHost() { - return parseHostKey(fs.readFileSync(path.join(dir, 'host.pub'), 'utf8').trim().split(' ').slice(0, 2).join(' ')); -} - -function identity(name: string) { - return parsePrivateKey(fs.readFileSync(path.join(dir, name)), name); -} - -function cert(name: string) { - return fs.readFileSync(path.join(dir, `${name}-cert.pub`), 'utf8'); -} - -describe('a workspace sshd that admits certificates only', () => { - it.each(['user-ed25519', 'user-ecdsa'])('lets in %s presented as its certificate', async (name) => { - const out = await run({ - hostVerifier: pinnedHostVerifier(pinnedHost()) as unknown as ConnectConfig['hostVerifier'], - authHandler: offerOnce(user, certificateKey(identity(name), cert(name))), - }); - expect(out).toBe('ok'); - }); - - it('refuses the same key without its certificate', async () => { - const out = await run({ - hostVerifier: pinnedHostVerifier(pinnedHost()) as unknown as ConnectConfig['hostVerifier'], - authHandler: offerOnce(user, identity('user-ed25519')), - }); - expect(out).toMatch(/^error: All configured authentication methods failed/); - }); - - it('refuses a certificate for another principal', async () => { - const out = await run({ - hostVerifier: pinnedHostVerifier(pinnedHost()) as unknown as ConnectConfig['hostVerifier'], - authHandler: offerOnce(user, certificateKey(identity('stranger'), cert('stranger'))), - }); - expect(out).toMatch(/^error: All configured authentication methods failed/); - }); - - it('hangs up on a host whose key is not the pinned one', async () => { - const other = { type: 'ssh-ed25519', blob: Buffer.from(fs.readFileSync(path.join(dir, 'ca.pub'), 'utf8').split(' ')[1], 'base64') }; - const out = await run({ - hostVerifier: pinnedHostVerifier(other) as unknown as ConnectConfig['hostVerifier'], - authHandler: offerOnce(user, certificateKey(identity('user-ed25519'), cert('user-ed25519'))), - }); - expect(out).toMatch(/^error: .*verification failed/i); - }); -}); - -describe('certificateKey', () => { - it('refuses a certificate for another kind of key', () => { - expect(() => certificateKey(identity('user-ecdsa'), cert('user-ed25519'))).toThrow(/for a ssh-ed25519 key, and the identity is a ecdsa/); - }); - - it('refuses what is not a certificate', () => { - expect(() => certificateKey(identity('user-ed25519'), fs.readFileSync(path.join(dir, 'user-ed25519.pub'), 'utf8'))).toThrow(/not an OpenSSH certificate/); - }); -}); diff --git a/test/spin/openssh.test.ts b/test/spin/openssh.test.ts new file mode 100644 index 0000000..8b1ebc3 --- /dev/null +++ b/test/spin/openssh.test.ts @@ -0,0 +1,118 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { sshOptions } from '../../src/spin/connection'; +import { checkBanner, checkOpenSSH } from '../../src/spin/openssh'; +import { SpinRefusal } from '../../src/spin/refusal'; + +// A workspace's sshd accepts mlkem768x25519-sha256 alone (spin-stack/spin-machine#245), which +// OpenSSH has from 9.9: an older ssh is refused before anything is asked, in words that say why, +// and every ssh this extension runs asks for it and is held to the claim's host key. + +function refusalOf(f: () => unknown) { + try { + f(); + } catch (e) { + if (e instanceof SpinRefusal) { + return e.refusal; + } + throw e; + } + return undefined; +} + +describe('checkBanner', () => { + it.each([ + ['Ubuntu 24.04\'s', 'OpenSSH_9.6p1 Ubuntu-3ubuntu13.20, OpenSSL 3.0.13 30 Jan 2024', 'ssh is OpenSSH 9.6 (OpenSSH_9.6p1 Ubuntu-3ubuntu13.20)'], + ['the last without it', 'OpenSSH_9.8p1, LibreSSL 3.3.6', 'ssh is OpenSSH 9.8 (OpenSSH_9.8p1)'], + ['Windows\'', 'OpenSSH_for_Windows_9.5p1, LibreSSL 3.8.2', 'ssh is OpenSSH 9.5 (OpenSSH_for_Windows_9.5p1)'], + ['an old major', 'OpenSSH_8.9p1 Ubuntu-3ubuntu0.10, OpenSSL 3.0.2 15 Mar 2022', 'ssh is OpenSSH 8.9'], + ])('refuses %s, saying what it is, what is needed and why', (_name, banner, says) => { + const refusal = refusalOf(() => checkBanner(`${banner}\n`)); + expect(refusal?.code).toBe('ssh_outdated'); + expect(refusal?.temporary).toBe(false); + expect(refusal?.message).toContain(says); + expect(refusal?.message).toContain('a workspace needs OpenSSH 9.9 or newer'); + expect(refusal?.message).toContain('post-quantum key exchange mlkem768x25519-sha256'); + expect(refusal?.message).toContain('refuses older clients'); + }); + + it.each([ + ['macOS\'s 9.9', 'OpenSSH_9.9p2, LibreSSL 3.3.6'], + ['Windows\' 9.9', 'OpenSSH_for_Windows_9.9p1, LibreSSL 3.8.2'], + ['a 10', 'OpenSSH_10.2p1 Ubuntu-2ubuntu3.8, OpenSSL 3.5.5 27 Jan 2026'], + ['a minor past 9', 'OpenSSH_9.10p1'], + ])('lets %s through, saying nothing', (_name, banner) => { + expect(checkBanner(banner)).toEqual({}); + }); + + it.each([['not OpenSSH', 'Dropbear v2024.86'], ['nothing', '']])('lets %s through, warned', (_name, banner) => { + expect(checkBanner(banner).warning).toMatch(/^could not tell which OpenSSH ssh is .*no matching key exchange method found/); + }); +}); + +describe('checkOpenSSH', () => { + let bin: string; + beforeEach(() => { + bin = fs.mkdtempSync(path.join(os.tmpdir(), 'spin-openssh-')); + }); + afterEach(() => { + fs.rmSync(bin, { recursive: true, force: true }); + }); + + function ssh(banner: string) { + fs.writeFileSync(path.join(bin, 'ssh'), `#!/bin/sh\necho '${banner}' >&2\n`, { mode: 0o755 }); + } + + it('asks the ssh on the PATH, and asks again after a refusal, as the refusal says to', async () => { + ssh('OpenSSH_9.6p1 Ubuntu-3ubuntu13.20, OpenSSL 3.0.13 30 Jan 2024'); + await expect(checkOpenSSH({ PATH: bin })).rejects.toMatchObject({ refusal: { code: 'ssh_outdated' } }); + ssh('OpenSSH_10.2p1, OpenSSL 3.5.5 27 Jan 2026'); + await expect(checkOpenSSH({ PATH: bin })).resolves.toEqual({}); + }); + + it('refuses a PATH with no ssh', async () => { + await expect(checkOpenSSH({ PATH: bin })).rejects.toMatchObject({ refusal: { code: 'ssh_not_found' } }); + }); +}); + +describe('sshOptions', () => { + const options = sshOptions( + { proxyCommand: '\'/opt/spin\' tunnel \'ABC\'', identity: '/home/me/.ssh/id_ed25519' }, + { knownHosts: '/tmp/spinbox-x/known_hosts', certificate: '/tmp/spinbox-x/id-cert.pub', controlPath: '/tmp/spinbox-x/mux' }, + ); + + function option(name: string): string | undefined { + const at = options.findIndex((o, i) => options[i - 1] === '-o' && o.startsWith(`${name}=`)); + return at < 0 ? undefined : options[at].slice(name.length + 1); + } + + it('asks for ML-KEM alone, carried by the tunnel', () => { + expect(option('KexAlgorithms')).toBe('mlkem768x25519-sha256'); + expect(option('ProxyCommand')).toBe('\'/opt/spin\' tunnel \'ABC\''); + }); + + it('accepts the one host key it was given, from the one file it is in, and learns none', () => { + expect(option('UserKnownHostsFile')).toBe('/tmp/spinbox-x/known_hosts'); + expect(option('GlobalKnownHostsFile')).toBe('/dev/null'); + expect(option('StrictHostKeyChecking')).toBe('yes'); + expect(option('UpdateHostKeys')).toBe('no'); + expect(option('HostKeyAlias')).toBe('spinbox-workspace'); + }); + + it('logs in with the certificate alone, asking nobody, and reads no config of the person\'s', () => { + expect(options.slice(0, 2)).toEqual(['-F', '/dev/null']); + expect(options[options.indexOf('-i') + 1]).toBe('/home/me/.ssh/id_ed25519'); + expect(option('CertificateFile')).toBe('/tmp/spinbox-x/id-cert.pub'); + expect(option('IdentitiesOnly')).toBe('yes'); + expect(option('BatchMode')).toBe('yes'); + }); + + it('doubles a % in what ssh would read as a token', () => { + const odd = sshOptions({ proxyCommand: '\'/opt/100%/spin\' tunnel \'ABC\'', identity: '/home/me/%u' }, + { knownHosts: '/k', certificate: '/c', controlPath: '/m' }); + expect(odd).toContain('ProxyCommand=\'/opt/100%%/spin\' tunnel \'ABC\''); + expect(odd[odd.indexOf('-i') + 1]).toBe('/home/me/%%u'); + }); +}); diff --git a/test/spin/resolve.test.ts b/test/spin/resolve.test.ts index 44eea7b..bfb66b3 100644 --- a/test/spin/resolve.test.ts +++ b/test/spin/resolve.test.ts @@ -4,35 +4,29 @@ import * as http from 'http'; import * as net from 'net'; import * as os from 'os'; import * as path from 'path'; -import { Server } from 'ssh2'; import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; import type * as vscodeTypes from 'vscode'; import { activate } from '../../src/extension'; import { configOverrides, env, ResolvedAuthority, RemoteAuthorityResolverError, resolvers, shownErrors } from '../mocks/vscode'; +import { keygen, type Sshd, startSshd } from '../sshd'; // `spin+` from the extension's activation to a request answered by the workspace's // server, with the CLI and the workspace standing in: a spin that answers `code` and -// `ssh-session`, and carries `tunnel` to an ssh2 server that admits only the -// workspace's certificate and answers the server start as the workspace would. What OpenSSH -// makes of the certificate is keys.sshd.test.ts's, and what the real CLI and workspace do is -// test/e2e's. +// `ssh-session`, and carries `tunnel` to OpenSSH's sshd, set up as a workspace's (test/sshd.ts), +// whose every exec answers the server start as the workspace would. The ssh is this machine's, +// through a wrapper that records what it was run with and logs in as this user, the only one a +// test's sshd lets in. What the real CLI and workspace do is test/e2e's. const COMMIT = '69047191fc427f2f92828279aae1d90d8eeda7ab'; const TOKEN = 'the-servers-token'; -const CERT_TYPE = 'ssh-ed25519-cert-v01@openssh.com'; const SESSION = 'ORWU6TGORHKIKCHPMWRLHNZSR4'; let dir: string; let home: string; let fixture: string; -let sshd: Server; +let sshd: Sshd; let codeServer: http.Server; -const savedHome = process.env.HOME; -const said: string[] = []; - -function keygen(...args: string[]) { - execFileSync('ssh-keygen', ['-q', ...args], { stdio: 'ignore' }); -} +const saved = { HOME: process.env.HOME, PATH: process.env.PATH }; function listen(server: net.Server): Promise { return new Promise(resolve => server.listen(0, '127.0.0.1', () => resolve((server.address() as net.AddressInfo).port))); @@ -60,6 +54,22 @@ if (cmd === 'code' || cmd === 'ssh-session') { } `; +// The ssh the resolver runs: this machine's, recording its arguments, as this user rather than +// root. The rotation keeps every argument as one word. +const sshWrapper = (real: string) => `#!/bin/sh +[ "$1" = -V ] || printf '%s\\036' "$*" >> "$FAKE_SPIN_FIXTURE/ssh-calls" +for a; do shift; case "$a" in root@*) set -- "$@" "$TEST_SSH_USER@\${a#root@}";; *) set -- "$@" "$a";; esac; done +exec ${real} "$@" +`; + +// The workspace's every exec: the server start's answer, for the id the script names, and what it +// was asked kept in said. +const FORCE_COMMAND = (codePort: number) => `#!/bin/sh +printf '%s\\n' "$SSH_ORIGINAL_COMMAND" >> "${fixture}/said" +id=$(printf '%s\\n' "$SSH_ORIGINAL_COMMAND" | sed -n 's/.*echo "\\(spinbox-[0-9a-f]*\\): start".*/\\1/p' | head -n 1) +printf '%s: start\\nexitCode==0==\\nlisteningOn==%s==\\nconnectionToken==%s==\\n%s: end\\n' "$id" ${codePort} ${TOKEN} "$id" +`; + function answerCode(answer: unknown, exit = 0) { fs.writeFileSync(path.join(fixture, 'code.json'), JSON.stringify({ answer, exit })); } @@ -75,65 +85,49 @@ function calls(): string[][] { return fs.readFileSync(path.join(fixture, 'calls'), 'utf8').trim().split('\n').map(l => JSON.parse(l) as string[]); } +/** Each run of ssh, its arguments in a line - the server's start a script of several - ended by \x1e. */ +function sshCalls(): string[] { + return fs.readFileSync(path.join(fixture, 'ssh-calls'), 'utf8').split('\x1e').filter(Boolean); +} + beforeAll(async () => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'spin-resolve-')); home = path.join(dir, 'home'); fixture = path.join(dir, 'fixture'); fs.mkdirSync(path.join(home, '.ssh'), { recursive: true }); - fs.mkdirSync(fixture); + fs.mkdirSync(path.join(fixture, 'bin'), { recursive: true }); const identity = path.join(home, '.ssh', 'id_ed25519'); + const user = os.userInfo().username; keygen('-t', 'ed25519', '-N', '', '-f', identity); keygen('-t', 'ed25519', '-N', '', '-f', path.join(fixture, 'ca')); keygen('-t', 'ed25519', '-N', '', '-f', path.join(fixture, 'host')); - keygen('-s', path.join(fixture, 'ca'), '-I', 'spin-test', '-n', 'root', '-V', '+5m', `${identity}.pub`); + // For this user, whom the wrapper logs in as: a workspace's certificate names root. + keygen('-s', path.join(fixture, 'ca'), '-I', 'spin-test', '-n', user, '-V', '+5m', `${identity}.pub`); fs.renameSync(`${identity}-cert.pub`, path.join(fixture, 'user-cert.pub')); opensSession = () => answerSession({ workspace_id: 'w1', session: SESSION, identity, certificate: fs.readFileSync(path.join(fixture, 'user-cert.pub'), 'utf8').trim(), host_key: fs.readFileSync(path.join(fixture, 'host.pub'), 'utf8').trim().split(' ').slice(0, 2).join(' '), }); - const certBlob = Buffer.from(fs.readFileSync(path.join(fixture, 'user-cert.pub'), 'utf8').split(' ')[1], 'base64'); codeServer = http.createServer((req, res) => { res.writeHead(req.url === `/version?tkn=${TOKEN}` ? 200 : 403).end(COMMIT); }); const codePort = await listen(codeServer); + fs.writeFileSync(path.join(fixture, 'workspace.sh'), FORCE_COMMAND(codePort), { mode: 0o755 }); + sshd = await startSshd(fixture, 'sshd', { forceCommand: path.join(fixture, 'workspace.sh') }); + fs.writeFileSync(path.join(fixture, 'port'), String(sshd.port)); - sshd = new Server({ hostKeys: [fs.readFileSync(path.join(fixture, 'host'))] }, client => { - client.on('authentication', ctx => { - if (ctx.method === 'publickey' && ctx.username === 'root' && ctx.key.algo === CERT_TYPE && ctx.key.data.equals(certBlob)) { - return ctx.accept(); - } - ctx.reject(['publickey']); - }).on('ready', () => { - client.on('session', accept => { - accept().on('exec', (acceptExec, _reject, info) => { - said.push(info.command); - const id = /echo "(spinbox-[0-9a-f]+): start"/.exec(info.command)?.[1]; - const stream = acceptExec(); - stream.write(`${id}: start\nexitCode==0==\nlisteningOn==${codePort}==\nconnectionToken==${TOKEN}==\n${id}: end\n`); - stream.exit(0); - stream.end(); - }); - }).on('tcpip', (acceptTcp, reject, info) => { - if (info.destIP !== '127.0.0.1' || info.destPort !== codePort) { - reject(); - return; - } - const channel = acceptTcp(); - const socket = net.connect(codePort, '127.0.0.1'); - channel.pipe(socket).pipe(channel); - }); - }).on('error', () => undefined); - }); - fs.writeFileSync(path.join(fixture, 'port'), String(await listen(sshd as unknown as net.Server))); - + const real = execFileSync('sh', ['-c', 'command -v ssh'], { encoding: 'utf8' }).trim(); + fs.writeFileSync(path.join(fixture, 'bin', 'ssh'), sshWrapper(real), { mode: 0o755 }); const spin = path.join(fixture, 'spin'); fs.writeFileSync(spin, FAKE_SPIN, { mode: 0o755 }); env.appCommit = COMMIT; configOverrides.set('remote.spin.cliPath', spin); process.env.HOME = home; + process.env.PATH = `${path.join(fixture, 'bin')}:${saved.PATH}`; process.env.FAKE_SPIN_FIXTURE = fixture; + process.env.TEST_SSH_USER = user; // Each test asserts on its own resolve: the subscriptions are disposed in afterAll. activate(context); }, 30_000); @@ -141,7 +135,9 @@ beforeAll(async () => { const context = { subscriptions: [] as { dispose(): unknown }[] } as unknown as vscodeTypes.ExtensionContext; afterEach(() => { - fs.rmSync(path.join(fixture, 'calls'), { force: true }); + for (const f of ['calls', 'ssh-calls']) { + fs.rmSync(path.join(fixture, f), { force: true }); + } shownErrors.length = 0; }); @@ -149,10 +145,12 @@ afterAll(async () => { for (const s of context.subscriptions) { s.dispose(); } - process.env.HOME = savedHome; + process.env.HOME = saved.HOME; + process.env.PATH = saved.PATH; delete process.env.FAKE_SPIN_FIXTURE; + delete process.env.TEST_SSH_USER; configOverrides.clear(); - sshd?.close(); + sshd?.process.kill(); codeServer?.close(); fs.rmSync(dir, { recursive: true, force: true }); }); @@ -192,7 +190,17 @@ describe('spin+', () => { ['ssh-session', 'unit-ws', '-o', 'json'], ['tunnel', SESSION], ]); - expect(said[said.length - 1]).toContain(`/run/spin-stack/vsc-${COMMIT.slice(0, 12)}/servers/${COMMIT}/server/bin/code-server`); + const said = fs.readFileSync(path.join(fixture, 'said'), 'utf8'); + expect(said).toContain(`/run/spin-stack/vsc-${COMMIT.slice(0, 12)}/servers/${COMMIT}/server/bin/code-server`); + // Every ssh - the login, the server's start, the request's way in - asked for ML-KEM alone + // and held the workspace to the claim's key. + const ran = sshCalls(); + expect(ran.length).toBeGreaterThanOrEqual(3); + for (const run of ran) { + expect(run).toContain('-o KexAlgorithms=mlkem768x25519-sha256'); + expect(run).toContain('-o StrictHostKeyChecking=yes'); + } + expect(ran.some(r => /-W 127\.0\.0\.1:\d+ root@/.test(r)), 'the request went through ssh -W').toBe(true); }, 30_000); it('refuses a workspace that answers with another host key than the session\'s', async () => { @@ -220,6 +228,28 @@ describe('spin+', () => { expect(calls().map(c => c[0])).toEqual(ran); }, 30_000); + it('refuses an ssh older than OpenSSH 9.9 before running anything, and says why', async () => { + const old = path.join(dir, 'old'); + fs.mkdirSync(old, { recursive: true }); + fs.writeFileSync(path.join(old, 'ssh'), '#!/bin/sh\necho \'OpenSSH_9.6p1 Ubuntu-3ubuntu13.20, OpenSSL 3.0.13 30 Jan 2024\' >&2\n', { mode: 0o755 }); + const path_ = process.env.PATH; + process.env.PATH = `${old}:${saved.PATH}`; + try { + const failed = await resolve('spin+unit-ws').catch((e: unknown) => e); + expect(failed).toBeInstanceOf(RemoteAuthorityResolverError); + expect((failed as RemoteAuthorityResolverError).temporary).toBe(false); + } finally { + process.env.PATH = path_; + } + expect(shownErrors).toEqual([[ + 'Spinbox could not open unit-ws: ssh is OpenSSH 9.6 (OpenSSH_9.6p1 Ubuntu-3ubuntu13.20); a workspace needs OpenSSH 9.9 or newer. ' + + 'Its sshd accepts only the post-quantum key exchange mlkem768x25519-sha256, which OpenSSH added in 9.9, and refuses older ' + + 'clients. Update OpenSSH (on macOS, \'brew install openssh\') and open the workspace again.', + { modal: true }, + ]]); + expect(fs.existsSync(path.join(fixture, 'calls')), 'the CLI was asked for a workspace this ssh cannot reach').toBe(false); + }, 30_000); + it('refuses an authority that is not a workspace before running anything', async () => { await expect(resolve('spin+../etc')).rejects.toThrow(/not a Spinbox workspace/); expect(fs.existsSync(path.join(fixture, 'calls'))).toBe(false); diff --git a/test/ssh2.security.test.ts b/test/ssh2.security.test.ts deleted file mode 100644 index 21fdaa0..0000000 --- a/test/ssh2.security.test.ts +++ /dev/null @@ -1,151 +0,0 @@ -import { createRequire } from 'node:module'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; -import { describe, expect, it } from 'vitest'; - -// Real-protocol behaviour of the INSTALLED ssh2 tree, pulled in through -// createRequire (ssh2 is CommonJS and its deep internals have no ESM entry) — -// this file never reimplements ssh2 code; it exercises the exact bytes -// pnpm installed (and patched) in node_modules/ssh2, the bytes the extension's -// SSH transport actually runs. It pins the two runtime properties the old git -// fork (jeanp413/ssh2@a169f62) existed for, so a silent downgrade back to the -// fork — or an ssh2 bump that left patches/ behind — fails here rather than on -// a live connection: -// -// 1. Terrapin (CVE-2023-48795): the client KEXINIT proposal must advertise -// strict-kex (`kex-strict-c-v00@openssh.com`) next to `ext-info-c`. The -// fork's proposal ended at `ext-info-c`; npm's ssh2 has carried the -// strict-kex marker since 1.11.0. -// -// 2. ECDSA host-key verification: sigSSHToASN1 must hand OpenSSL minimal, -// positive DER INTEGERs (the fork's "Fix sigSSHToASN1", re-applied to the -// registry's ssh2 by patches/ssh2@1.17.0.patch), and a truncated signature must return -// `undefined` — never throw. The fork converted r/s before its -// truncated-signature check, so a malformed signature escaped as a -// TypeError from inside the handshake; the patched tree checks first and -// fails closed, which the kex layer turns into a clean "Malformed -// signature" disconnect. - -const repoRoot = path.resolve(__dirname, '..'); -// Anchored at the root manifest so the require resolves from this repo's -// node_modules — the installed tree, not any vendored or hoisted copy. -const require = createRequire(path.join(repoRoot, 'package.json')); - -/** The piece of ssh2's kex module this suite reads. */ -interface KexModule { - DEFAULT_KEXINIT_CLIENT: { - lists: { - kex: { - array: string[]; - }; - }; - }; -} - -/** The piece of ssh2's protocol utils module this suite drives. */ -interface ProtocolUtilsModule { - sigSSHToASN1: (sig: Buffer, type: string) => Buffer | undefined; -} - -const kex = require('ssh2/lib/protocol/kex.js') as KexModule; -const { sigSSHToASN1 } = require('ssh2/lib/protocol/utils.js') as ProtocolUtilsModule; - -/** One SSH wire binary string: uint32 length prefix + payload. */ -function sshString(value: Buffer): Buffer { - const framed = Buffer.alloc(4 + value.length); - framed.writeUInt32BE(value.length, 0); - value.copy(framed, 4); - return framed; -} - -/** An ECDSA signature body as it travels on the SSH wire: string r, string s. */ -function ecdsaSignatureBody(r: Buffer, s: Buffer): Buffer { - return Buffer.concat([sshString(r), sshString(s)]); -} - -describe('installed ssh2 (real protocol behaviour)', () => { - it('is exactly the version the root manifest pins', () => { - const pkg = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')) as { - dependencies: Record; - }; - const manifestPin = pkg.dependencies.ssh2; - const installedVersion = (require('ssh2/package.json') as { version: string }).version; - expect(installedVersion, 'node_modules/ssh2 must be reinstalled to match the manifest pin').toBe(manifestPin); - expect(installedVersion, 'the installed version must be an exact x.y.z release').toMatch(/^\d+\.\d+\.\d+$/); - }); - - // The strict-kex marker in the CLIENT proposal is the Terrapin mitigation: - // once offered and accepted, both peers reset sequence numbers after - // NEWKEYS and authenticate the pre-negotiation exchange. The git fork's - // proposal ended at `ext-info-c` — no marker — so this assertion cannot - // pass on the fork tree: it pins the fix itself, not just a version number. - it('the client KEXINIT proposal advertises strict-kex alongside ext-info-c', () => { - const proposal = kex.DEFAULT_KEXINIT_CLIENT.lists.kex.array; - expect(proposal, 'the client KEXINIT proposal must offer ext-info-c').toContain('ext-info-c'); - expect( - proposal, - 'the client KEXINIT proposal must offer kex-strict-c-v00@openssh.com (CVE-2023-48795) — the git-fork tree cannot pass this' - ).toContain('kex-strict-c-v00@openssh.com'); - }); - - // nistp256 r/s are 32 bytes; when the top byte's high bit is set, a DER - // INTEGER needs a 0x00 pad to stay positive — exactly the case npm's - // unpatched ssh2 writes raw (OpenSSL then reads a negative integer and - // verification fails) and the fork fixed via convertToMpint. - it('converts high-bit ECDSA r/s to minimal positive DER INTEGERs', () => { - const r = Buffer.concat([Buffer.from([0x80]), Buffer.alloc(31, 0xab)]); - const s = Buffer.concat([Buffer.from([0x90]), Buffer.alloc(31, 0xcd)]); - const sig = sigSSHToASN1(ecdsaSignatureBody(r, s), 'ecdsa-sha2-nistp256'); - - // SEQUENCE (70 content bytes) { INTEGER (33) 0x00||r; INTEGER (33) 0x00||s } - const expected = Buffer.concat([ - Buffer.from([0x30, 70]), - Buffer.from([0x02, 0x21, 0x00]), r, - Buffer.from([0x02, 0x21, 0x00]), s, - ]); - expect(sig, 'high-bit r/s must each gain the 0x00 positive pad (33-byte INTEGERs)').toEqual(expected); - }); - - it('strips non-significant leading zeros but keeps a semantically required one', () => { - // r: two leading zeros before a byte with the high bit clear — both - // dropped, that is the "minimal" half of minimal positive mpint. - // s: a leading zero before a high byte (0x80) is significant — dropping - // it would flip the integer's sign — so it must survive untouched. - const r = Buffer.from([0x00, 0x00, 0x7f, 0x1c, 0x55]); - const s = Buffer.from([0x00, 0x80, 0x33]); - const sig = sigSSHToASN1(ecdsaSignatureBody(r, s), 'ecdsa-sha2-nistp256'); - - // SEQUENCE (10 content bytes) { INTEGER 0x7f,0x1c,0x55; INTEGER 0x00,0x80,0x33 } - const expected = Buffer.concat([ - Buffer.from([0x30, 10]), - Buffer.from([0x02, 0x03, 0x7f, 0x1c, 0x55]), - Buffer.from([0x02, 0x03, 0x00, 0x80, 0x33]), - ]); - expect(sig, 'leading zeros must be stripped (r) or kept exactly when significant (s)').toEqual(expected); - }); - - // The fork's fatal ordering: it ran convertToMpint on r/s BEFORE its - // undefined-check, so a truncated signature threw TypeError mid-handshake. - // The patched tree must keep the check first: return undefined, no throw. - it.each([ - ['s declares more bytes than remain', Buffer.concat([ - sshString(Buffer.alloc(32, 0xab)), - Buffer.from([0x00, 0x00, 0x00, 0x20]), - ])], - ['r declares more bytes than remain', Buffer.concat([ - Buffer.from([0x00, 0x00, 0x00, 0x20]), - Buffer.alloc(8, 0xcd), - ])], - ['s\u2019s length prefix itself is cut short', sshString(Buffer.alloc(32, 0xab))], - ] as const)('returns undefined — never throws — when %s', (_label, sig) => { - let asn1: Buffer | undefined; - let threw: unknown; - try { - asn1 = sigSSHToASN1(sig, 'ecdsa-sha2-nistp256'); - } catch (error) { - threw = error; - } - expect(threw, 'a truncated signature must fail closed, not throw (the git fork threw TypeError here)').toBeUndefined(); - expect(asn1, 'a truncated signature must yield no ASN.1 output').toBeUndefined(); - }); -}); diff --git a/test/sshd.ts b/test/sshd.ts new file mode 100644 index 0000000..a86cc13 --- /dev/null +++ b/test/sshd.ts @@ -0,0 +1,81 @@ +// OpenSSH's own sshd, configured as a workspace's is (spin's internal/sshauth, and the image's +// key exchange, spin-stack/spin-machine#245): certificates from one CA and nothing else, and +// mlkem768x25519-sha256 alone - so what a test proves of a connection is what sshd decides. It +// runs as this user, and so lets in only this user. + +import { execFileSync, spawn, type ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as net from 'net'; +import * as path from 'path'; + +export function keygen(...args: string[]) { + execFileSync('ssh-keygen', ['-q', ...args], { stdio: 'ignore' }); +} + +function freePort(): Promise { + return new Promise((resolve, reject) => { + const s = net.createServer().listen(0, '127.0.0.1', () => { + const p = (s.address() as net.AddressInfo).port; + s.close(() => resolve(p)); + }).on('error', reject); + }); +} + +async function waitForPort(p: number) { + for (let i = 0; i < 100; i++) { + const open = await new Promise(resolve => { + const c = net.connect(p, '127.0.0.1', () => { c.destroy(); resolve(true); }).on('error', () => resolve(false)); + }); + if (open) { + return; + } + await new Promise(r => setTimeout(r, 50)); + } + throw new Error(`sshd did not listen on ${p}`); +} + +export interface Sshd { + port: number; + process: ChildProcess; +} + +/** + * sshd in dir, presenting dir/host and trusting certificates dir/ca signs. kex is what it accepts + * (the workspace's by default); forceCommand, when given, is what every exec runs instead. + */ +export async function startSshd(dir: string, name: string, o: { kex?: string; forceCommand?: string } = {}): Promise { + const port = await freePort(); + const config = path.join(dir, `${name}_config`); + fs.writeFileSync(config, [ + `ListenAddress 127.0.0.1:${port}`, + `HostKey ${path.join(dir, 'host')}`, + `TrustedUserCAKeys ${path.join(dir, 'ca.pub')}`, + 'AuthorizedKeysFile none', + 'PasswordAuthentication no', + 'KbdInteractiveAuthentication no', + 'UsePAM no', + 'StrictModes no', + `KexAlgorithms ${o.kex ?? 'mlkem768x25519-sha256'}`, + // A refused login is one from 127.0.0.1, and a penalised source has the next test's + // refused before it is asked. + 'PerSourcePenalties no', + ...(o.forceCommand ? [`ForceCommand ${o.forceCommand}`] : []), + `PidFile ${path.join(dir, `${name}.pid`)}`, + 'LogLevel ERROR', + '', + ].join('\n')); + // Both lines take OpenSSH 9.9 or newer, as a workspace's sshd is: an older one is no stand-in. + try { + execFileSync('/usr/sbin/sshd', ['-t', '-f', config], { stdio: 'pipe' }); + } catch (e) { + throw new Error(`this sshd refuses a workspace's config (OpenSSH 9.9 or newer takes it): ${(e as { stderr?: Buffer }).stderr?.toString() ?? e}`); + } + const sshd = spawn('/usr/sbin/sshd', ['-D', '-e', '-f', config], { stdio: ['ignore', 'ignore', 'pipe'] }); + let said = ''; + sshd.stderr?.on('data', (d: Buffer) => { said += d.toString(); }); + sshd.on('error', e => { said += e.message; }); + await waitForPort(port).catch(e => { + throw new Error(`${(e as Error).message}: ${said || `exit ${sshd.exitCode}`}`); + }); + return { port, process: sshd }; +} diff --git a/tsconfig.json b/tsconfig.json index 3eaf424..c46c003 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "lib": ["ES2022"], "module": "preserve", "moduleResolution": "bundler", - "types": ["node", "ssh2", "ssh2-streams", "vscode"], + "types": ["node", "vscode"], "isolatedModules": true, "noEmit": true, "strict": true,