From d0d5461a67c1ba0fceff571ec90c772c26e01b83 Mon Sep 17 00:00:00 2001 From: Anmol Sethi Date: Mon, 27 Apr 2020 09:27:45 -0400 Subject: [PATCH 1/2] Remove SSH server Closes #1502 --- src/node/cli.ts | 5 - src/node/entry.ts | 29 +----- src/node/ssh/server.ts | 110 ---------------------- src/node/ssh/sftp.ts | 201 ----------------------------------------- src/node/ssh/ssh.ts | 122 ------------------------- src/node/util.ts | 6 -- test/cli.test.ts | 1 - 7 files changed, 1 insertion(+), 473 deletions(-) delete mode 100644 src/node/ssh/server.ts delete mode 100644 src/node/ssh/sftp.ts delete mode 100644 src/node/ssh/ssh.ts diff --git a/src/node/cli.ts b/src/node/cli.ts index 8feaf982..0bb0364e 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -31,8 +31,6 @@ export interface Args extends VsArgs { readonly open?: boolean readonly port?: number readonly socket?: string - readonly "ssh-host-key"?: string - readonly "disable-ssh"?: boolean readonly version?: boolean readonly force?: boolean readonly "list-extensions"?: boolean @@ -99,9 +97,6 @@ const options: Options> = { version: { type: "boolean", short: "v", description: "Display version information." }, _: { type: "string[]" }, - "disable-ssh": { type: "boolean", description: "Disable the SSH server." }, - "ssh-host-key": { type: "string", path: true, description: "SSH server host key." }, - "user-data-dir": { type: "string", path: true, description: "Path to the user data directory." }, "extensions-dir": { type: "string", path: true, description: "Path to the extensions directory." }, "builtin-extensions-dir": { type: "string", path: true }, diff --git a/src/node/entry.ts b/src/node/entry.ts index 26a235cf..49fff6f4 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -11,8 +11,7 @@ import { UpdateHttpProvider } from "./app/update" import { VscodeHttpProvider } from "./app/vscode" import { Args, optionDescriptions, parse } from "./cli" import { AuthType, HttpServer, HttpServerOptions } from "./http" -import { SshProvider } from "./ssh/server" -import { generateCertificate, generatePassword, generateSshHostKey, hash, open } from "./util" +import { generateCertificate, generatePassword, hash, open } from "./util" import { ipcMain, wrap } from "./wrapper" process.on("uncaughtException", (error) => { @@ -101,32 +100,6 @@ const main = async (args: Args): Promise => { logger.info(`Automatic updates are ${update.enabled ? "enabled" : "disabled"}`) - let sshHostKey = args["ssh-host-key"] - if (!args["disable-ssh"] && !sshHostKey) { - try { - sshHostKey = await generateSshHostKey() - } catch (error) { - logger.error("Unable to start SSH server", field("error", error.message)) - } - } - - let sshPort: number | undefined - if (!args["disable-ssh"] && sshHostKey) { - const sshProvider = httpServer.registerHttpProvider("/ssh", SshProvider, sshHostKey) - try { - sshPort = await sshProvider.listen() - } catch (error) { - logger.warn(`SSH server: ${error.message}`) - } - } - - if (typeof sshPort !== "undefined") { - logger.info(`SSH server listening on localhost:${sshPort}`) - logger.info(" - To disable use `--disable-ssh`") - } else { - logger.info("SSH server disabled") - } - if (serverAddress && !options.socket && args.open) { // The web socket doesn't seem to work if browsing with 0.0.0.0. const openAddress = serverAddress.replace(/:\/\/0.0.0.0/, "://localhost") diff --git a/src/node/ssh/server.ts b/src/node/ssh/server.ts deleted file mode 100644 index 7ab99a6a..00000000 --- a/src/node/ssh/server.ts +++ /dev/null @@ -1,110 +0,0 @@ -import * as http from "http" -import * as net from "net" -import * as ssh from "ssh2" -import * as ws from "ws" -import * as fs from "fs" -import { logger } from "@coder/logger" -import safeCompare from "safe-compare" -import { HttpProvider, HttpResponse, HttpProviderOptions, Route } from "../http" -import { HttpCode } from "../../common/http" -import { forwardSshPort, fillSshSession } from "./ssh" -import { hash } from "../util" - -export class SshProvider extends HttpProvider { - private readonly wss = new ws.Server({ noServer: true }) - private sshServer: ssh.Server - - public constructor(options: HttpProviderOptions, hostKeyPath: string) { - super(options) - const hostKey = fs.readFileSync(hostKeyPath) - this.sshServer = new ssh.Server({ hostKeys: [hostKey] }, this.handleSsh) - - this.sshServer.on("error", (err) => { - logger.trace(`SSH server error: ${err.stack}`) - }) - } - - public async listen(): Promise { - return new Promise((resolve, reject) => { - this.sshServer.once("error", reject) - this.sshServer.listen(() => { - resolve(this.sshServer.address().port) - }) - }) - } - - public async handleRequest(): Promise { - // SSH has no HTTP endpoints - return { code: HttpCode.NotFound } - } - - public handleWebSocket( - _route: Route, - request: http.IncomingMessage, - socket: net.Socket, - head: Buffer, - ): Promise { - // Create a fake websocket to the sshServer - const sshSocket = net.connect(this.sshServer.address().port, "localhost") - - return new Promise((resolve) => { - this.wss.handleUpgrade(request, socket, head, (ws) => { - // Send SSH data to WS as compressed binary - sshSocket.on("data", (data) => { - ws.send(data, { - binary: true, - compress: true, - fin: true, - }) - }) - - // Send WS data to SSH as buffer - ws.on("message", (msg) => { - // Buffer.from is cool with all types, but casting as string keeps typing simple - sshSocket.write(Buffer.from(msg as string)) - }) - - ws.on("error", (err) => { - logger.error(`SSH websocket error: ${err.stack}`) - }) - - resolve() - }) - }) - } - - /** - * Determine how to handle incoming SSH connections. - */ - private handleSsh = (client: ssh.Connection, info: ssh.ClientInfo): void => { - logger.debug(`Incoming SSH connection from ${info.ip}`) - client.on("authentication", (ctx) => { - // Allow any auth to go through if we have no password - if (!this.options.password) { - return ctx.accept() - } - - // Otherwise require the same password as code-server - if (ctx.method === "password") { - if ( - safeCompare(this.options.password, hash(ctx.password)) || - safeCompare(this.options.password, ctx.password) - ) { - return ctx.accept() - } - } - - // Reject, letting them know that password is the only method we allow - ctx.reject(["password"]) - }) - client.on("tcpip", forwardSshPort) - client.on("session", fillSshSession) - client.on("error", (err) => { - // Don't bother logging Keepalive errors, they probably just disconnected - if (err.message === "Keepalive timeout") { - return logger.debug("SSH client keepalive timeout") - } - logger.error(`SSH client error: ${err.stack}`) - }) - } -} diff --git a/src/node/ssh/sftp.ts b/src/node/ssh/sftp.ts deleted file mode 100644 index baa5e865..00000000 --- a/src/node/ssh/sftp.ts +++ /dev/null @@ -1,201 +0,0 @@ -/** - * Provides utilities for handling SSH connections - */ -import * as fs from "fs" -import * as path from "path" -import * as ssh from "ssh2" -import { FileEntry, SFTPStream } from "ssh2-streams" - -/** - * Fills out all the functionality of SFTP using fs. - */ -export function fillSftpStream(accept: () => SFTPStream): void { - const sftp = accept() - - let oid = 0 - const fds: { [key: number]: boolean } = {} - const ods: { - [key: number]: { - path: string - read: boolean - } - } = {} - - const sftpStatus = (reqID: number, err?: NodeJS.ErrnoException | null): boolean => { - let code = ssh.SFTP_STATUS_CODE.OK - if (err) { - if (err.code === "EACCES") { - code = ssh.SFTP_STATUS_CODE.PERMISSION_DENIED - } else if (err.code === "ENOENT") { - code = ssh.SFTP_STATUS_CODE.NO_SUCH_FILE - } else { - code = ssh.SFTP_STATUS_CODE.FAILURE - } - } - return sftp.status(reqID, code) - } - - sftp.on("OPEN", (reqID, filename) => { - fs.open(filename, "w", (err, fd) => { - if (err) { - return sftpStatus(reqID, err) - } - fds[fd] = true - const buf = Buffer.alloc(4) - buf.writeUInt32BE(fd, 0) - return sftp.handle(reqID, buf) - }) - }) - - sftp.on("OPENDIR", (reqID, path) => { - const buf = Buffer.alloc(4) - const id = oid++ - buf.writeUInt32BE(id, 0) - ods[id] = { - path, - read: false, - } - sftp.handle(reqID, buf) - }) - - sftp.on("READDIR", (reqID, handle) => { - const od = handle.readUInt32BE(0) - if (!ods[od]) { - return sftp.status(reqID, ssh.SFTP_STATUS_CODE.NO_SUCH_FILE) - } - if (ods[od].read) { - sftp.status(reqID, ssh.SFTP_STATUS_CODE.EOF) - return - } - return fs.readdir(ods[od].path, (err, files) => { - if (err) { - return sftpStatus(reqID, err) - } - return Promise.all( - files.map((f) => { - return new Promise((resolve, reject) => { - const fullPath = path.join(ods[od].path, f) - fs.stat(fullPath, (err, stats) => { - if (err) { - return reject(err) - } - - resolve({ - filename: f, - longname: fullPath, - attrs: { - atime: stats.atimeMs, - gid: stats.gid, - mode: stats.mode, - size: stats.size, - mtime: stats.mtimeMs, - uid: stats.uid, - }, - }) - }) - }) - }), - ) - .then((files) => { - sftp.name(reqID, files) - ods[od].read = true - }) - .catch(() => { - sftp.status(reqID, ssh.SFTP_STATUS_CODE.FAILURE) - }) - }) - }) - - sftp.on("WRITE", (reqID, handle, offset, data) => { - const fd = handle.readUInt32BE(0) - if (!fds[fd]) { - return sftp.status(reqID, ssh.SFTP_STATUS_CODE.NO_SUCH_FILE) - } - return fs.write(fd, data, offset, (err) => sftpStatus(reqID, err)) - }) - - sftp.on("CLOSE", (reqID, handle) => { - const fd = handle.readUInt32BE(0) - if (!fds[fd]) { - if (ods[fd]) { - delete ods[fd] - return sftp.status(reqID, ssh.SFTP_STATUS_CODE.OK) - } - return sftp.status(reqID, ssh.SFTP_STATUS_CODE.NO_SUCH_FILE) - } - return fs.close(fd, (err) => sftpStatus(reqID, err)) - }) - - sftp.on("STAT", (reqID, path) => { - fs.stat(path, (err, stats) => { - if (err) { - return sftpStatus(reqID, err) - } - return sftp.attrs(reqID, { - atime: stats.atime.getTime(), - gid: stats.gid, - mode: stats.mode, - mtime: stats.mtime.getTime(), - size: stats.size, - uid: stats.uid, - }) - }) - }) - - sftp.on("MKDIR", (reqID, path) => { - fs.mkdir(path, (err) => sftpStatus(reqID, err)) - }) - - sftp.on("LSTAT", (reqID, path) => { - fs.lstat(path, (err, stats) => { - if (err) { - return sftpStatus(reqID, err) - } - return sftp.attrs(reqID, { - atime: stats.atimeMs, - gid: stats.gid, - mode: stats.mode, - mtime: stats.mtimeMs, - size: stats.size, - uid: stats.uid, - }) - }) - }) - - sftp.on("REMOVE", (reqID, path) => { - fs.unlink(path, (err) => sftpStatus(reqID, err)) - }) - - sftp.on("RMDIR", (reqID, path) => { - fs.rmdir(path, (err) => sftpStatus(reqID, err)) - }) - - sftp.on("REALPATH", (reqID, path) => { - fs.realpath(path, (pathErr, resolved) => { - if (pathErr) { - return sftpStatus(reqID, pathErr) - } - fs.stat(path, (statErr, stat) => { - if (statErr) { - return sftpStatus(reqID, statErr) - } - sftp.name(reqID, [ - { - filename: resolved, - longname: resolved, - attrs: { - mode: stat.mode, - uid: stat.uid, - gid: stat.gid, - size: stat.size, - atime: stat.atime.getTime(), - mtime: stat.mtime.getTime(), - }, - }, - ]) - return - }) - return - }) - }) -} diff --git a/src/node/ssh/ssh.ts b/src/node/ssh/ssh.ts deleted file mode 100644 index e0a87398..00000000 --- a/src/node/ssh/ssh.ts +++ /dev/null @@ -1,122 +0,0 @@ -/** - * Provides utilities for handling SSH connections - */ -import * as net from "net" -import * as cp from "child_process" -import * as ssh from "ssh2" -import * as nodePty from "node-pty" -import { fillSftpStream } from "./sftp" - -/** - * Fills out all of the functionality of SSH using node equivalents. - */ -export function fillSshSession(accept: () => ssh.Session): void { - let pty: nodePty.IPty | undefined - let activeProcess: cp.ChildProcess - let ptyInfo: ssh.PseudoTtyInfo | undefined - const env: { [key: string]: string } = {} - - const session = accept() - - // Run a command, stream back the data - const cmd = (command: string, channel: ssh.ServerChannel): void => { - if (ptyInfo) { - // Remove undefined and project env vars - // keysToRemove taken from sanitizeProcessEnvironment - const keysToRemove = [/^ELECTRON_.+$/, /^GOOGLE_API_KEY$/, /^VSCODE_.+$/, /^SNAP(|_.*)$/] - const env = Object.keys(process.env).reduce((prev, k) => { - if (process.env[k] === undefined) { - return prev - } - const val = process.env[k] as string - if (keysToRemove.find((rx) => val.search(rx))) { - return prev - } - prev[k] = val - return prev - }, {} as { [key: string]: string }) - - pty = nodePty.spawn(command, [], { - cols: ptyInfo.cols, - rows: ptyInfo.rows, - env, - }) - pty.onData((d) => channel.write(d)) - pty.on("exit", (exitCode) => { - channel.exit(exitCode) - channel.close() - }) - channel.on("data", (d: string) => pty && pty.write(d)) - return - } - - const proc = cp.spawn(command, { shell: true }) - proc.stdout.on("data", (d) => channel.stdout.write(d)) - proc.stderr.on("data", (d) => channel.stderr.write(d)) - proc.on("exit", (exitCode) => { - channel.exit(exitCode || 0) - channel.close() - }) - channel.stdin.on("data", (d: unknown) => proc.stdin.write(d)) - channel.stdin.on("close", () => proc.stdin.end()) - } - - session.on("pty", (accept, _, info) => { - ptyInfo = info - accept && accept() - }) - - session.on("shell", (accept) => { - cmd(process.env.SHELL || "/usr/bin/env bash", accept()) - }) - - session.on("exec", (accept, _, info) => { - cmd(info.command, accept()) - }) - - session.on("sftp", fillSftpStream) - - session.on("signal", (accept, _, info) => { - accept && accept() - process.kill((pty || activeProcess).pid, info.name) - }) - - session.on("env", (accept, _reject, info) => { - accept && accept() - env[info.key] = info.value - }) - - session.on("auth-agent", (accept) => { - accept() - }) - - session.on("window-change", (accept, reject, info) => { - if (pty) { - pty.resize(info.cols, info.rows) - accept && accept() - } else { - reject() - } - }) -} - -/** - * Pipes a requested port over SSH - */ -export function forwardSshPort( - accept: () => ssh.ServerChannel, - reject: () => boolean, - info: ssh.TcpipRequestInfo, -): void { - const fwdSocket = net.createConnection(info.destPort, info.destIP) - fwdSocket.on("error", () => reject()) - fwdSocket.on("connect", () => { - const channel = accept() - channel.pipe(fwdSocket) - channel.on("close", () => fwdSocket.end()) - fwdSocket.pipe(channel) - fwdSocket.on("close", () => channel.close()) - fwdSocket.on("error", () => channel.end()) - fwdSocket.on("end", () => channel.end()) - }) -} diff --git a/src/node/util.ts b/src/node/util.ts index 1d649454..44e27be0 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -44,12 +44,6 @@ export const generateCertificate = async (): Promise<{ cert: string; certKey: st return paths } -export const generateSshHostKey = async (): Promise => { - // Just reuse the SSL cert as the SSH host key - const { certKey } = await generateCertificate() - return certKey -} - export const generatePassword = async (length = 24): Promise => { const buffer = Buffer.alloc(Math.ceil(length / 2)) await util.promisify(crypto.randomFill)(buffer) diff --git a/test/cli.test.ts b/test/cli.test.ts index aab12684..2b222bc6 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -117,7 +117,6 @@ describe("cli", () => { assert.throws(() => parse(["--auth=", "--log=debug"]), /--auth requires a value/) assert.throws(() => parse(["--auth", "--log"]), /--auth requires a value/) assert.throws(() => parse(["--auth", "--invalid"]), /--auth requires a value/) - assert.throws(() => parse(["--ssh-host-key"]), /--ssh-host-key requires a value/) }) it("should error if value is invalid", () => { From 181e0ea6c889440cc478303d0d39d02fc42b6adc Mon Sep 17 00:00:00 2001 From: Anmol Sethi Date: Tue, 28 Apr 2020 14:04:56 -0400 Subject: [PATCH 2/2] Remove ssh2 dep --- package.json | 3 --- yarn.lock | 40 ++-------------------------------------- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/package.json b/package.json index 0ead146d..0bf8d2a8 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,6 @@ "@types/safe-compare": "^1.1.0", "@types/semver": "^7.1.0", "@types/tar-fs": "^1.16.2", - "@types/ssh2": "0.5.39", - "@types/ssh2-streams": "^0.1.6", "@types/tar-stream": "^1.6.1", "@types/ws": "^6.0.4", "@typescript-eslint/eslint-plugin": "^2.0.0", @@ -59,7 +57,6 @@ "pem": "^1.14.2", "safe-compare": "^1.1.4", "semver": "^7.1.3", - "ssh2": "^0.8.7", "tar": "^6.0.1", "tar-fs": "^2.0.0", "ws": "^7.2.0" diff --git a/yarn.lock b/yarn.lock index 07042359..c259a40e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -944,21 +944,6 @@ dependencies: "@types/node" "*" -"@types/ssh2-streams@*", "@types/ssh2-streams@^0.1.6": - version "0.1.6" - resolved "https://registry.yarnpkg.com/@types/ssh2-streams/-/ssh2-streams-0.1.6.tgz#1ff5b1fe50c15f282efe9fd092c68494f8702bc2" - integrity sha512-NB+SYftagfHTDPdgVyvSZCeg5MURyHECd/PycGIW9hwhnEbTFxIdDbFtf3jxUO3rRnfr0lfdtkZFiv28T1cAsg== - dependencies: - "@types/node" "*" - -"@types/ssh2@0.5.39": - version "0.5.39" - resolved "https://registry.yarnpkg.com/@types/ssh2/-/ssh2-0.5.39.tgz#e39ab7e38fc01337f66237ed6c42237ef3e58e3b" - integrity sha512-MtX4tr6Jtdn/JPVsQytjB/NBeOd7JfCyf/l79KOdkUYL+r4GXUXcySX1n8W4O61fnQdljTBXEPJJ2dhnGhi2xg== - dependencies: - "@types/node" "*" - "@types/ssh2-streams" "*" - "@types/tar-fs@^1.16.2": version "1.16.3" resolved "https://registry.yarnpkg.com/@types/tar-fs/-/tar-fs-1.16.3.tgz#425b2b817c405d13d051f36ec6ec6ebd25e31069" @@ -1243,7 +1228,7 @@ asn1.js@^4.0.0: inherits "^2.0.1" minimalistic-assert "^1.0.0" -asn1@~0.2.0, asn1@~0.2.3: +asn1@~0.2.3: version "0.2.4" resolved "https://registry.yarnpkg.com/asn1/-/asn1-0.2.4.tgz#8d2475dfab553bb33e77b54e59e880bb8ce23136" integrity sha512-jxwzQpLQjSmWXgwaCZE9Nz+glAG01yF1QnWgbhGwHI5A6FRIEY6IVqtHhIepHqI7/kyEyQEagBC5mBEFlIYvdg== @@ -1378,7 +1363,7 @@ base@^0.11.1: mixin-deep "^1.2.0" pascalcase "^0.1.1" -bcrypt-pbkdf@^1.0.0, bcrypt-pbkdf@^1.0.2: +bcrypt-pbkdf@^1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.2.tgz#a4301d389b6a43f9b67ff3ca11a3f6637e360e9e" integrity sha1-pDAdOJtqQ/m2f/PKEaP2Y342Dp4= @@ -6304,22 +6289,6 @@ sprintf-js@~1.0.2: resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.0.3.tgz#04e6926f662895354f3dd015203633b857297e2c" integrity sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw= -ssh2-streams@~0.4.9: - version "0.4.9" - resolved "https://registry.yarnpkg.com/ssh2-streams/-/ssh2-streams-0.4.9.tgz#d3d92155410001437d27119d9c023b303cbe2309" - integrity sha512-glMQKeYKuA+rLaH16fJC3nZMV1BWklbxuYCR4C5/LlBSf2yaoNRpPU7Ul702xsi5nvYjIx9XDkKBJwrBjkDynw== - dependencies: - asn1 "~0.2.0" - bcrypt-pbkdf "^1.0.2" - streamsearch "~0.1.2" - -ssh2@^0.8.7: - version "0.8.8" - resolved "https://registry.yarnpkg.com/ssh2/-/ssh2-0.8.8.tgz#1d9815e287faef623ae2b7db32e674dadbef4664" - integrity sha512-egJVQkf3sbjECTY6rCeg8rgV/fab6S/7E5kpYqHT3Fe/YpfJbLYeA1qTcB2d+LRUUAjqKi7rlbfWkaP66YdpAQ== - dependencies: - ssh2-streams "~0.4.9" - sshpk@^1.7.0: version "1.16.1" resolved "https://registry.yarnpkg.com/sshpk/-/sshpk-1.16.1.tgz#fb661c0bef29b39db40769ee39fa70093d6f6877" @@ -6409,11 +6378,6 @@ stream-http@^2.7.2: to-arraybuffer "^1.0.0" xtend "^4.0.0" -streamsearch@~0.1.2: - version "0.1.2" - resolved "https://registry.yarnpkg.com/streamsearch/-/streamsearch-0.1.2.tgz#808b9d0e56fc273d809ba57338e929919a1a9f1a" - integrity sha1-gIudDlb8Jz2Am6VzOOkpkZoanxo= - "string-width@^1.0.2 || 2": version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e"