From 482f2f2290f28da2099e57288652c3d3434d0a7d Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 1 Aug 2024 09:26:01 -0400 Subject: [PATCH 01/32] feat: auth handler skeleton --- client/src/connection/ssh/index.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 60b5cfad7..267531929 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -2,7 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 import { l10n } from "vscode"; -import { Client, ClientChannel, ConnectConfig } from "ssh2"; +import { + AuthHandlerMiddleware, + Client, + ClientChannel, + ConnectConfig, + NextAuthHandler, +} from "ssh2"; import { BaseConfig, RunResult } from ".."; import { updateStatusBarItem } from "../../components/StatusBarItem"; @@ -72,7 +78,7 @@ export class SSHSession extends Session { port: this._config.port, username: this._config.username, readyTimeout: sasLaunchTimeout, - agent: process.env.SSH_AUTH_SOCK || undefined, + authHandler: this.authHandler, }; this.conn @@ -235,4 +241,13 @@ export class SSHSession extends Session { this.stream.write(`${execArgs} ${this._config.saspath} ${execSasOpts} \n`); }; + + private authHandler: AuthHandlerMiddleware = ( + authsLeft: AuthenticationType[], + partialSuccess: boolean, + next: NextAuthHandler, + ) => { + //TODO: auth handler implementation + // We need to support username/password, private key, and agent + }; } From bda97f195a859b61e1b0051ab61fb559a14f5595 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 1 Aug 2024 09:45:35 -0400 Subject: [PATCH 02/32] fix: type on authsLeft --- client/src/connection/ssh/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 267531929..b1e532e29 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -4,6 +4,7 @@ import { l10n } from "vscode"; import { AuthHandlerMiddleware, + AuthenticationType, Client, ClientChannel, ConnectConfig, From 86840c7f4431ef9ee8476205b75faa96c654c516 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 1 Aug 2024 12:49:15 -0400 Subject: [PATCH 03/32] chore: progress on auth handler --- client/src/connection/ssh/index.ts | 273 ++++++++++++++++++++++------- 1 file changed, 214 insertions(+), 59 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index b1e532e29..c4e0d7078 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -1,7 +1,9 @@ // Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { l10n } from "vscode"; +import { l10n, window } from "vscode"; +import { CancellationTokenSource } from "vscode-languageclient"; +import { readFileSync } from "fs"; import { AuthHandlerMiddleware, AuthenticationType, @@ -9,15 +11,18 @@ import { ClientChannel, ConnectConfig, NextAuthHandler, + utils, } from "ssh2"; import { BaseConfig, RunResult } from ".."; +import { getSecretStorage } from "../../components/ExtensionContext"; import { updateStatusBarItem } from "../../components/StatusBarItem"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; const endCode = "--vscode-sas-extension-submit-end--"; -const sasLaunchTimeout = 10000; +const SECRET_STORAGE_NAMESPACE = "SSH_SECRET_STORAGE"; +const sasLaunchTimeout = 600000; let sessionInstance: SSHSession; export interface Config extends BaseConfig { @@ -25,6 +30,7 @@ export interface Config extends BaseConfig { username: string; saspath: string; port: number; + privateKeyFilePath: string; } export function getSession(c: Config): Session { @@ -40,20 +46,26 @@ export function getSession(c: Config): Session { sessionInstance.config = c; return sessionInstance; } - +type SSHSessionAuthMethod = "password" | "keyboard-interactive" | "publickey"; export class SSHSession extends Session { - private conn: Client; - private stream: ClientChannel | undefined; + private _conn: Client; + private _stream: ClientChannel | undefined; private _config: Config; - private resolve: ((value?) => void) | undefined; - private reject: ((reason?) => void) | undefined; - private html5FileName = ""; - private timer: NodeJS.Timeout; + private _resolve: ((value?) => void) | undefined; + private _reject: ((reason?) => void) | undefined; + private _html5FileName = ""; + private _timer: NodeJS.Timeout; + private _authMethods: SSHSessionAuthMethod[]; //auth methods that this session can support + private _cancellationSource: CancellationTokenSource | undefined; + private _secretStorage; + private _passwordKey: string; constructor(c?: Config) { super(); this._config = c; - this.conn = new Client(); + this._conn = new Client(); + this._authMethods = ["publickey", "password", "keyboard-interactive"]; + this._secretStorage = getSecretStorage(SECRET_STORAGE_NAMESPACE); } public sessionId? = (): string => { @@ -62,15 +74,16 @@ export class SSHSession extends Session { set config(newValue: Config) { this._config = newValue; + this._passwordKey = `${newValue.host}${newValue.username}`; } protected establishConnection = (): Promise => { return new Promise((pResolve, pReject) => { - this.resolve = pResolve; - this.reject = pReject; + this._resolve = pResolve; + this._reject = pReject; - if (this.stream) { - this.resolve?.({}); + if (this._stream) { + this._resolve?.({}); return; } @@ -79,75 +92,78 @@ export class SSHSession extends Session { port: this._config.port, username: this._config.username, readyTimeout: sasLaunchTimeout, + debug: (msg) => { + console.log(msg); + }, authHandler: this.authHandler, }; - this.conn + this._conn .on("ready", () => { - this.conn.shell(this.onShell); + this._conn.shell(this.onShell); }) .on("error", this.onConnectionError); - this.setTimer(); - this.conn.connect(cfg); + // this.setTimer(); + this._conn.connect(cfg); }); }; public run = (code: string): Promise => { - this.html5FileName = ""; + this._html5FileName = ""; return new Promise((_resolve, _reject) => { - this.resolve = _resolve; - this.reject = _reject; + this._resolve = _resolve; + this._reject = _reject; - this.stream?.write(`${code}\n`); - this.stream?.write(`%put ${endCode};\n`); + this._stream?.write(`${code}\n`); + this._stream?.write(`%put ${endCode};\n`); }); }; public close = (): void | Promise => { - if (!this.stream) { + if (!this._stream) { return; } - this.stream.write("endsas;\n"); - this.stream.close(); + this._stream.write("endsas;\n"); + this._stream.close(); }; private onConnectionError = (err: Error) => { this.clearTimer(); - this.reject?.(err); + this._reject?.(err); }; private setTimer = (): void => { this.clearTimer(); - this.timer = setTimeout(() => { - this.reject?.( + this._timer = setTimeout(() => { + this._reject?.( new Error( l10n.t("Failed to connect to Session. Check profile settings."), ), ); - this.timer = undefined; + this._timer = undefined; this.close(); }, sasLaunchTimeout); }; private clearTimer = (): void => { - this.timer && clearTimeout(this.timer); - this.timer = undefined; + this._timer && clearTimeout(this._timer); + this._timer = undefined; }; private getResult = (): void => { const runResult: RunResult = {}; - if (!this.html5FileName) { - this.resolve?.(runResult); + if (!this._html5FileName) { + this._resolve?.(runResult); return; } let fileContents = ""; - this.conn.exec( - `cat ${this.html5FileName}.htm`, + this._conn.exec( + `cat ${this._html5FileName}.htm`, (err: Error, s: ClientChannel) => { if (err) { - this.reject?.(err); + this._reject?.(err); return; } @@ -164,28 +180,28 @@ export class SSHSession extends Session { runResult.title = l10n.t("Result"); } } - this.resolve?.(runResult); + this._resolve?.(runResult); }); }, ); }; private onStreamClose = (): void => { - this.stream = undefined; - this.resolve = undefined; - this.reject = undefined; - this.html5FileName = ""; - this.timer = undefined; - this.conn.end(); + this._stream = undefined; + this._resolve = undefined; + this._reject = undefined; + this._html5FileName = ""; + this._timer = undefined; + this._conn.end(); updateStatusBarItem(false); }; private onStreamData = (data: Buffer): void => { const output = data.toString().trimEnd(); - if (this.timer && output.endsWith("?")) { + if (this._timer && output.endsWith("?")) { this.clearTimer(); - this.resolve?.(); + this._resolve?.(); updateStatusBarItem(true); return; } @@ -200,9 +216,9 @@ export class SSHSession extends Session { this.getResult(); } if (!(line.endsWith("?") || line.endsWith(">"))) { - this.html5FileName = extractOutputHtmlFileName( + this._html5FileName = extractOutputHtmlFileName( line, - this.html5FileName, + this._html5FileName, ); this._onExecutionLogFn?.([{ type: "normal", line }]); } @@ -211,17 +227,17 @@ export class SSHSession extends Session { private onShell = (err: Error, s: ClientChannel): void => { if (err) { - this.reject?.(err); + this._reject?.(err); return; } - this.stream = s; - if (!this.stream) { - this.reject?.(err); + this._stream = s; + if (!this._stream) { + this._reject?.(err); return; } - this.stream.on("close", this.onStreamClose); - this.stream.on("data", this.onStreamData); + this._stream.on("close", this.onStreamClose); + this._stream.on("data", this.onStreamData); const resolvedEnv: string[] = [ "env", @@ -240,15 +256,154 @@ export class SSHSession extends Session { } const execSasOpts: string = resolvedSasOpts.join(" "); - this.stream.write(`${execArgs} ${this._config.saspath} ${execSasOpts} \n`); + this._stream.write(`${execArgs} ${this._config.saspath} ${execSasOpts} \n`); + }; + + private promptForPassphrase = async (): Promise => { + //TODO: need to think about whether these should be stored in secret storage + // I'm leaning towrds no, but it's worth considering. Intial thought is that + // if users want to persist a passphrase, that the ssh-agent should be used, + // which seems to be inline with other solutions. Otherwise, the passphrase + // should be entered each time a session is established. + const passphrase = await window.showInputBox({ + prompt: l10n.t("Enter the passphrase for the private key."), + password: true, + }); + return passphrase; + }; + private promptForPassword = async (): Promise => { + //TODO: we need to properly manage the lifecycle of the password secret + const storedPassword = await this._secretStorage.get(this._passwordKey); + if (storedPassword) { + return storedPassword; + } + + const source = new CancellationTokenSource(); + this._cancellationSource = source; + const pw = await window.showInputBox( + { + ignoreFocusOut: true, + password: true, + prompt: l10n.t("Enter your password for this connection."), + title: l10n.t("Password Required"), + }, + this._cancellationSource.token, + ); + return pw; }; private authHandler: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], - partialSuccess: boolean, - next: NextAuthHandler, + _partialSuccess: boolean, + cb: NextAuthHandler, ) => { - //TODO: auth handler implementation - // We need to support username/password, private key, and agent + if (!authsLeft) { + cb("none"); //sending none will usually prompt the server to send supported auth methods + return; + } + + if (this._authMethods.length === 0) { + //if we're out of auth methods to try, then reject with an error + this._reject?.( + new Error(l10n.t("Could not authenticate to the SSH server.")), + ); + + //returning false will stop the auth process + return false; + } + + //otherwise, fetch the next auth method to try + const authMethod = this._authMethods.shift(); + + //make sure the auth method is supported by the server + if (authsLeft.includes(authMethod)) { + switch (authMethod) { + //TODO: this is ugly and needs to be broken up into smaller functions + // one function per type of auth method would be a good start + case "publickey": { + //user set a keyfile path in profile config + //check for passphrase, prompt if necessary + //and then attempt to auth + if (this._config.privateKeyFilePath) { + const keyContents = readFileSync(this._config.privateKeyFilePath); + const parsedKeyResult = utils.parseKey(keyContents); + // key is encrypted, prompt for passphrase + if ( + parsedKeyResult instanceof Error && + parsedKeyResult.message === + "Encrypted OpenSSH private key detected, but no passphrase given" + ) { + this.promptForPassphrase().then((passphrase) => { + //parse the keyfile using the passphrase + const parsedKeyContentsResult = utils.parseKey( + keyContents, + passphrase, + ); + + //TODO: refactor typechecking into one place, maybe a utility function + if (!(parsedKeyContentsResult instanceof Error)) { + cb({ + type: "publickey", + key: parsedKeyContentsResult, + passphrase: passphrase, + username: this._config.username, + }); + } + }); + } else { + //TODO: refactor typechecking into one place, maybe a utility function + if (!(parsedKeyResult instanceof Error)) { + cb({ + type: "publickey", + key: parsedKeyResult, + username: this._config.username, + }); + } + } + } else if (process.env.SSH_AUTH_SOCK) { + //attempt to auth using ssh-agent + cb({ + type: "agent", + agent: process.env.SSH_AUTH_SOCK, + username: this._config.username, + }); + } + break; + } + case "password": { + this.promptForPassword().then((pw) => { + cb({ + type: "password", + password: pw, + username: this._config.username, + }); + }); + break; + } + + case "keyboard-interactive": { + cb({ + type: "keyboard-interactive", + username: this._config.username, + prompt: (_name, _instructions, _instructionsLang, prompts, cb) => { + if (prompts.length === 1 && prompts[0].prompt === "Password:") { + this.promptForPassword().then((pw) => { + cb([pw]); + }); + } else { + cb([]); + } + }, + }); + break; + } + default: + //TODO: not sure if cb with "none" is the right thing to do here + cb("none"); + } + } else { + console.warn(`Server does not support ${authMethod} auth method.`); + cb("none"); + } }; } From 74040255c8f8ccc0d117d0fd3b40e062af6614b1 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 1 Aug 2024 12:53:35 -0400 Subject: [PATCH 04/32] fix: brain working faster than fingers --- client/src/connection/ssh/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index c4e0d7078..13712ca67 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -261,7 +261,7 @@ export class SSHSession extends Session { private promptForPassphrase = async (): Promise => { //TODO: need to think about whether these should be stored in secret storage - // I'm leaning towrds no, but it's worth considering. Intial thought is that + // I'm leaning towards no, but it's worth considering. Initial thought is that // if users want to persist a passphrase, that the ssh-agent should be used, // which seems to be inline with other solutions. Otherwise, the passphrase // should be entered each time a session is established. @@ -325,6 +325,7 @@ export class SSHSession extends Session { //check for passphrase, prompt if necessary //and then attempt to auth if (this._config.privateKeyFilePath) { + //TODO: handle "file not found" errors const keyContents = readFileSync(this._config.privateKeyFilePath); const parsedKeyResult = utils.parseKey(keyContents); // key is encrypted, prompt for passphrase From 69b66642cb85a053f455c59d0b7bcdcbc932f067 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 08:57:30 -0400 Subject: [PATCH 05/32] feat: progress on userpass --- client/src/connection/LineParser.ts | 40 ++++++++++++++++ client/src/connection/ssh/const.ts | 11 +++++ client/src/connection/ssh/index.ts | 74 +++++++++++++---------------- client/src/connection/ssh/types.ts | 29 +++++++++++ 4 files changed, 113 insertions(+), 41 deletions(-) create mode 100644 client/src/connection/LineParser.ts create mode 100644 client/src/connection/ssh/const.ts create mode 100644 client/src/connection/ssh/types.ts diff --git a/client/src/connection/LineParser.ts b/client/src/connection/LineParser.ts new file mode 100644 index 000000000..002a44f3a --- /dev/null +++ b/client/src/connection/LineParser.ts @@ -0,0 +1,40 @@ +// Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +export class LineParser { + protected processedLines: string[] = []; + protected capturingLine: boolean = false; + + public constructor( + protected startTag: string, + protected endTag: string, + protected returnNonProcessedLines: boolean, + ) {} + + public processLine(line: string): string | undefined { + if (line.includes(this.startTag) || this.capturingLine) { + this.processedLines.push(line); + this.capturingLine = true; + if (line.includes(this.endTag)) { + return this.processedLine(); + } + return; + } + + return this.returnNonProcessedLines ? line : undefined; + } + + protected processedLine(): string { + this.capturingLine = false; + const fullError = this.processedLines + .join("") + .replace(this.startTag, "") + .replace(this.endTag, ""); + this.processedLines = []; + return fullError; + } + + public isCapturingLine(): boolean { + return this.capturingLine; + } +} diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts new file mode 100644 index 000000000..bbfc73562 --- /dev/null +++ b/client/src/connection/ssh/const.ts @@ -0,0 +1,11 @@ +// Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +export const WORK_DIR_START_TAG = ""; +export const WORK_DIR_END_TAG = ""; +export const SAS_LAUNCH_TIMEOUT = 600000; +export const SUPPORTED_AUTH_METHODS = [ + "publickey", + "password", + "keyboard-interactive", +]; diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 13712ca67..5ee2f5663 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -15,13 +15,12 @@ import { } from "ssh2"; import { BaseConfig, RunResult } from ".."; -import { getSecretStorage } from "../../components/ExtensionContext"; import { updateStatusBarItem } from "../../components/StatusBarItem"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; +import { SUPPORTED_AUTH_METHODS } from "./const"; const endCode = "--vscode-sas-extension-submit-end--"; -const SECRET_STORAGE_NAMESPACE = "SSH_SECRET_STORAGE"; const sasLaunchTimeout = 600000; let sessionInstance: SSHSession; @@ -46,7 +45,6 @@ export function getSession(c: Config): Session { sessionInstance.config = c; return sessionInstance; } -type SSHSessionAuthMethod = "password" | "keyboard-interactive" | "publickey"; export class SSHSession extends Session { private _conn: Client; private _stream: ClientChannel | undefined; @@ -54,18 +52,16 @@ export class SSHSession extends Session { private _resolve: ((value?) => void) | undefined; private _reject: ((reason?) => void) | undefined; private _html5FileName = ""; - private _timer: NodeJS.Timeout; - private _authMethods: SSHSessionAuthMethod[]; //auth methods that this session can support + private _sessionReady: boolean; + private _authMethods: AuthenticationType[]; //auth methods that this session can support private _cancellationSource: CancellationTokenSource | undefined; - private _secretStorage; - private _passwordKey: string; constructor(c?: Config) { super(); this._config = c; this._conn = new Client(); this._authMethods = ["publickey", "password", "keyboard-interactive"]; - this._secretStorage = getSecretStorage(SECRET_STORAGE_NAMESPACE); + this._sessionReady = false; } public sessionId? = (): string => { @@ -74,7 +70,6 @@ export class SSHSession extends Session { set config(newValue: Config) { this._config = newValue; - this._passwordKey = `${newValue.host}${newValue.username}`; } protected establishConnection = (): Promise => { @@ -104,7 +99,6 @@ export class SSHSession extends Session { }) .on("error", this.onConnectionError); - // this.setTimer(); this._conn.connect(cfg); }); }; @@ -130,28 +124,10 @@ export class SSHSession extends Session { }; private onConnectionError = (err: Error) => { - this.clearTimer(); + this._sessionReady = false; this._reject?.(err); }; - private setTimer = (): void => { - this.clearTimer(); - this._timer = setTimeout(() => { - this._reject?.( - new Error( - l10n.t("Failed to connect to Session. Check profile settings."), - ), - ); - this._timer = undefined; - this.close(); - }, sasLaunchTimeout); - }; - - private clearTimer = (): void => { - this._timer && clearTimeout(this._timer); - this._timer = undefined; - }; - private getResult = (): void => { const runResult: RunResult = {}; if (!this._html5FileName) { @@ -191,7 +167,8 @@ export class SSHSession extends Session { this._resolve = undefined; this._reject = undefined; this._html5FileName = ""; - this._timer = undefined; + this._sessionReady = false; + this._authMethods = undefined; this._conn.end(); updateStatusBarItem(false); }; @@ -199,8 +176,8 @@ export class SSHSession extends Session { private onStreamData = (data: Buffer): void => { const output = data.toString().trimEnd(); - if (this._timer && output.endsWith("?")) { - this.clearTimer(); + if (!this._sessionReady && output.endsWith("?")) { + this._sessionReady = true; this._resolve?.(); updateStatusBarItem(true); return; @@ -272,12 +249,6 @@ export class SSHSession extends Session { return passphrase; }; private promptForPassword = async (): Promise => { - //TODO: we need to properly manage the lifecycle of the password secret - const storedPassword = await this._secretStorage.get(this._passwordKey); - if (storedPassword) { - return storedPassword; - } - const source = new CancellationTokenSource(); this._cancellationSource = source; const pw = await window.showInputBox( @@ -289,6 +260,12 @@ export class SSHSession extends Session { }, this._cancellationSource.token, ); + + // user cancelled password dialog + if (!pw) { + this._resolve?.({}); + } + return pw; }; @@ -302,6 +279,10 @@ export class SSHSession extends Session { return; } + if (!this._authMethods) { + this._authMethods = authsLeft; + } + if (this._authMethods.length === 0) { //if we're out of auth methods to try, then reject with an error this._reject?.( @@ -316,7 +297,7 @@ export class SSHSession extends Session { const authMethod = this._authMethods.shift(); //make sure the auth method is supported by the server - if (authsLeft.includes(authMethod)) { + if (SUPPORTED_AUTH_METHODS.includes(authMethod)) { switch (authMethod) { //TODO: this is ugly and needs to be broken up into smaller functions // one function per type of auth method would be a good start @@ -325,8 +306,19 @@ export class SSHSession extends Session { //check for passphrase, prompt if necessary //and then attempt to auth if (this._config.privateKeyFilePath) { - //TODO: handle "file not found" errors - const keyContents = readFileSync(this._config.privateKeyFilePath); + let keyContents: Buffer; + + try { + keyContents = readFileSync(this._config.privateKeyFilePath); + } catch (e) { + l10n.t( + "Error reading private key file: {filePath}, error: {message}", + { + filePath: this._config.privateKeyFilePath, + message: e.message, + }, + ); + } const parsedKeyResult = utils.parseKey(keyContents); // key is encrypted, prompt for passphrase if ( diff --git a/client/src/connection/ssh/types.ts b/client/src/connection/ssh/types.ts new file mode 100644 index 000000000..76bfb2f00 --- /dev/null +++ b/client/src/connection/ssh/types.ts @@ -0,0 +1,29 @@ +import { BaseConfig, LogLineTypeEnum } from ".."; + +export interface Config extends BaseConfig { + host: string; + username: string; + saspath: string; + port: number; + privateKeyFilePath: string; +} + +export const LogLineTypes: LogLineTypeEnum[] = [ + "normal", + "hilighted", + "source", + "title", + "byline", + "footnote", + "error", + "warning", + "note", + "message", +]; + +export enum LineCodes { + ResultsFetchedCode = "--vscode-sas-extension-results-fetched--", + RunCancelledCode = "--vscode-sas-extension-run-cancelled--", + RunEndCode = "--vscode-sas-extension-submit-end--", + LogLineType = "--vscode-sas-extension-log-line-type--", +} From cc667a4eb91abbadb839edd21144fb7e568d41ce Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 10:07:44 -0400 Subject: [PATCH 06/32] refactor: auth state cleanup --- client/src/connection/ssh/index.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 5ee2f5663..39a9d50f0 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -124,7 +124,7 @@ export class SSHSession extends Session { }; private onConnectionError = (err: Error) => { - this._sessionReady = false; + this.clearAuthState(); this._reject?.(err); }; @@ -167,8 +167,7 @@ export class SSHSession extends Session { this._resolve = undefined; this._reject = undefined; this._html5FileName = ""; - this._sessionReady = false; - this._authMethods = undefined; + this.clearAuthState(); this._conn.end(); updateStatusBarItem(false); }; @@ -269,6 +268,14 @@ export class SSHSession extends Session { return pw; }; + /** + * Resets the SSH auth state. + */ + private clearAuthState = (): void => { + this._authMethods = undefined; + this._sessionReady = false; + }; + private authHandler: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], _partialSuccess: boolean, @@ -289,6 +296,7 @@ export class SSHSession extends Session { new Error(l10n.t("Could not authenticate to the SSH server.")), ); + this.clearAuthState(); //returning false will stop the auth process return false; } From 38a575a17f3dbe31a961378f7272fd766af34d95 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 11:29:11 -0400 Subject: [PATCH 07/32] refactor: move auth details to separate module --- client/src/connection/ssh/auth.ts | 171 +++++++++++++++++++++++++++++ client/src/connection/ssh/index.ts | 133 +++------------------- 2 files changed, 187 insertions(+), 117 deletions(-) create mode 100644 client/src/connection/ssh/auth.ts diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts new file mode 100644 index 000000000..ba13a361f --- /dev/null +++ b/client/src/connection/ssh/auth.ts @@ -0,0 +1,171 @@ +import { CancellationTokenSource, l10n, window } from "vscode"; + +import { readFileSync } from "fs"; +import { NextAuthHandler, utils } from "ssh2"; + +let _cancellationSource: CancellationTokenSource | undefined; + +/** + * Authenticate to the server using the password method. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for the password + * @param username the user name to use for the connection + */ +export const passwordAuth = ( + cb: NextAuthHandler, + resolve: ((value?) => void) | undefined, + username: string, +) => { + promptForPassword(resolve).then((pw) => { + cb({ + type: "password", + password: pw, + username: username, + }); + }); +}; + +/** + * Authenticate to the server using the keyboard-interactive method. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for authentication + * @param username the user name to use for the connection + */ +export const keyboardInteractiveAuth = ( + cb: NextAuthHandler, + resolve: ((value?) => void) | undefined, + username: string, +) => { + cb({ + type: "keyboard-interactive", + username: username, + prompt: (_name, _instructions, _instructionsLang, prompts, cb) => { + if (prompts.length === 1 && prompts[0].prompt === "Password:") { + promptForPassword(resolve).then((pw) => { + cb([pw]); + }); + } else { + cb([]); + } + }, + }); +}; + +/** + * Authenticate to the server using the ssh-agent. See the extension README for more information on how to set up the ssh-agent. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param username the user name to use for the connection + */ +export const sshAgentAuth = (cb: NextAuthHandler, username: string) => { + //attempt to auth using ssh-agent + cb({ + type: "agent", + agent: process.env.SSH_AUTH_SOCK, + username: username, + }); +}; + +/** + * Authenticate to the server using a private key file. + * If a private key file is defined in the connection profile, this function will read the file and use it to authenticate to the server. + * If the key is encrypted, the user will be prompted for the passphrase. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for authentication + * @param privateKeyFilePath the path to the private key file defined in the connection profile + * @param username the user name to use for the connection + */ +export const privateKeyAuth = ( + cb: NextAuthHandler, + resolve: ((value?) => void) | undefined, + privateKeyFilePath: string, + username: string, +) => { + let keyContents: Buffer; + try { + keyContents = readFileSync(privateKeyFilePath); + } catch (e) { + l10n.t("Error reading private key file: {filePath}, error: {message}", { + filePath: privateKeyFilePath, + message: e.message, + }); + } + //check for passphrase, prompt if necessary + //and then attempt to auth + const parsedKeyResult = utils.parseKey(keyContents); + const hasParseError = parsedKeyResult instanceof Error; + const passphraseRequired = + hasParseError && + parsedKeyResult.message === + "Encrypted OpenSSH private key detected, but no passphrase given"; + // key is encrypted, prompt for passphrase + if (passphraseRequired) { + promptForPassphrase(resolve).then((passphrase) => { + //parse the keyfile using the passphrase + const reparsedKeyContentsResult = utils.parseKey(keyContents, passphrase); + + if (!(reparsedKeyContentsResult instanceof Error)) { + cb({ + type: "publickey", + key: reparsedKeyContentsResult, + passphrase: passphrase, + username: username, + }); + } + }); + } else { + if (!hasParseError) { + cb({ + type: "publickey", + key: parsedKeyResult, + username: username, + }); + } + } +}; + +/** + * Prompt the user for a passphrase. + * @param resolve a function that resolves the promise that is waiting for authentication + * @returns the passphrase entered by the user + */ +const promptForPassphrase = async (resolve): Promise => { + const passphrase = await window.showInputBox({ + prompt: l10n.t("Enter the passphrase for the private key."), + password: true, + }); + + // user cancelled password dialog + if (!passphrase) { + resolve?.({}); + } + + return passphrase; +}; + +/** + * Prompt the user for a password. + * @param resolve a function that resolves the promise that is waiting for authentication + * @returns the password entered by the user + */ +const promptForPassword = async ( + resolve: ((value?) => void) | undefined, +): Promise => { + const source = new CancellationTokenSource(); + _cancellationSource = source; + const pw = await window.showInputBox( + { + ignoreFocusOut: true, + password: true, + prompt: l10n.t("Enter your password for this connection."), + title: l10n.t("Password Required"), + }, + _cancellationSource.token, + ); + + // user cancelled password dialog + if (!pw) { + resolve?.({}); + } + + return pw; +}; diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 39a9d50f0..6d3b8a063 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -1,9 +1,7 @@ // Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { l10n, window } from "vscode"; -import { CancellationTokenSource } from "vscode-languageclient"; +import { l10n } from "vscode"; -import { readFileSync } from "fs"; import { AuthHandlerMiddleware, AuthenticationType, @@ -11,13 +9,18 @@ import { ClientChannel, ConnectConfig, NextAuthHandler, - utils, } from "ssh2"; import { BaseConfig, RunResult } from ".."; import { updateStatusBarItem } from "../../components/StatusBarItem"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; +import { + keyboardInteractiveAuth, + passwordAuth, + privateKeyAuth, + sshAgentAuth, +} from "./auth"; import { SUPPORTED_AUTH_METHODS } from "./const"; const endCode = "--vscode-sas-extension-submit-end--"; @@ -54,7 +57,6 @@ export class SSHSession extends Session { private _html5FileName = ""; private _sessionReady: boolean; private _authMethods: AuthenticationType[]; //auth methods that this session can support - private _cancellationSource: CancellationTokenSource | undefined; constructor(c?: Config) { super(); @@ -235,39 +237,6 @@ export class SSHSession extends Session { this._stream.write(`${execArgs} ${this._config.saspath} ${execSasOpts} \n`); }; - private promptForPassphrase = async (): Promise => { - //TODO: need to think about whether these should be stored in secret storage - // I'm leaning towards no, but it's worth considering. Initial thought is that - // if users want to persist a passphrase, that the ssh-agent should be used, - // which seems to be inline with other solutions. Otherwise, the passphrase - // should be entered each time a session is established. - const passphrase = await window.showInputBox({ - prompt: l10n.t("Enter the passphrase for the private key."), - password: true, - }); - return passphrase; - }; - private promptForPassword = async (): Promise => { - const source = new CancellationTokenSource(); - this._cancellationSource = source; - const pw = await window.showInputBox( - { - ignoreFocusOut: true, - password: true, - prompt: l10n.t("Enter your password for this connection."), - title: l10n.t("Password Required"), - }, - this._cancellationSource.token, - ); - - // user cancelled password dialog - if (!pw) { - this._resolve?.({}); - } - - return pw; - }; - /** * Resets the SSH auth state. */ @@ -307,99 +276,29 @@ export class SSHSession extends Session { //make sure the auth method is supported by the server if (SUPPORTED_AUTH_METHODS.includes(authMethod)) { switch (authMethod) { - //TODO: this is ugly and needs to be broken up into smaller functions - // one function per type of auth method would be a good start case "publickey": { //user set a keyfile path in profile config - //check for passphrase, prompt if necessary - //and then attempt to auth if (this._config.privateKeyFilePath) { - let keyContents: Buffer; - - try { - keyContents = readFileSync(this._config.privateKeyFilePath); - } catch (e) { - l10n.t( - "Error reading private key file: {filePath}, error: {message}", - { - filePath: this._config.privateKeyFilePath, - message: e.message, - }, - ); - } - const parsedKeyResult = utils.parseKey(keyContents); - // key is encrypted, prompt for passphrase - if ( - parsedKeyResult instanceof Error && - parsedKeyResult.message === - "Encrypted OpenSSH private key detected, but no passphrase given" - ) { - this.promptForPassphrase().then((passphrase) => { - //parse the keyfile using the passphrase - const parsedKeyContentsResult = utils.parseKey( - keyContents, - passphrase, - ); - - //TODO: refactor typechecking into one place, maybe a utility function - if (!(parsedKeyContentsResult instanceof Error)) { - cb({ - type: "publickey", - key: parsedKeyContentsResult, - passphrase: passphrase, - username: this._config.username, - }); - } - }); - } else { - //TODO: refactor typechecking into one place, maybe a utility function - if (!(parsedKeyResult instanceof Error)) { - cb({ - type: "publickey", - key: parsedKeyResult, - username: this._config.username, - }); - } - } + privateKeyAuth( + cb, + this._resolve, + this._config.privateKeyFilePath, + this._config.username, + ); } else if (process.env.SSH_AUTH_SOCK) { - //attempt to auth using ssh-agent - cb({ - type: "agent", - agent: process.env.SSH_AUTH_SOCK, - username: this._config.username, - }); + sshAgentAuth(cb, this._config.username); } break; } case "password": { - this.promptForPassword().then((pw) => { - cb({ - type: "password", - password: pw, - username: this._config.username, - }); - }); + passwordAuth(cb, this._resolve, this._config.username); break; } - case "keyboard-interactive": { - cb({ - type: "keyboard-interactive", - username: this._config.username, - prompt: (_name, _instructions, _instructionsLang, prompts, cb) => { - if (prompts.length === 1 && prompts[0].prompt === "Password:") { - this.promptForPassword().then((pw) => { - cb([pw]); - }); - } else { - cb([]); - } - }, - }); + keyboardInteractiveAuth(cb, this._resolve, this._config.username); break; } default: - //TODO: not sure if cb with "none" is the right thing to do here cb("none"); } } else { From f0016cbb98f36c5f85cfeafea3d6f667ef63f580 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 11:58:40 -0400 Subject: [PATCH 08/32] feat: keepalive and max unanswered settings --- client/src/connection/ssh/const.ts | 4 ++++ client/src/connection/ssh/index.ts | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index bbfc73562..943bb86c6 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -1,6 +1,10 @@ // Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +const SECOND = 1000; +export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to send SSH-level keepalive packets to the server. Set to 0 to disable. +// 720 * 60 seconds = 43200 seconds = 12 hours +export const KEEPALIVE_UNANSWERED_THRESHOLD = 720; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. export const WORK_DIR_START_TAG = ""; export const WORK_DIR_END_TAG = ""; export const SAS_LAUNCH_TIMEOUT = 600000; diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 6d3b8a063..1510f6ebc 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -21,7 +21,7 @@ import { privateKeyAuth, sshAgentAuth, } from "./auth"; -import { SUPPORTED_AUTH_METHODS } from "./const"; +import { KEEPALIVE_INTERVAL, SUPPORTED_AUTH_METHODS } from "./const"; const endCode = "--vscode-sas-extension-submit-end--"; const sasLaunchTimeout = 600000; @@ -89,6 +89,8 @@ export class SSHSession extends Session { port: this._config.port, username: this._config.username, readyTimeout: sasLaunchTimeout, + keepaliveInterval: KEEPALIVE_INTERVAL, + keepaliveCountMax: 1440, debug: (msg) => { console.log(msg); }, From 21284e41d53e3bffcc637cb9c38daadf654f3040 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 12:00:23 -0400 Subject: [PATCH 09/32] use const for keepalive --- client/src/connection/ssh/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 1510f6ebc..e9d725bf1 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -21,7 +21,11 @@ import { privateKeyAuth, sshAgentAuth, } from "./auth"; -import { KEEPALIVE_INTERVAL, SUPPORTED_AUTH_METHODS } from "./const"; +import { + KEEPALIVE_INTERVAL, + KEEPALIVE_UNANSWERED_THRESHOLD, + SUPPORTED_AUTH_METHODS, +} from "./const"; const endCode = "--vscode-sas-extension-submit-end--"; const sasLaunchTimeout = 600000; @@ -90,7 +94,7 @@ export class SSHSession extends Session { username: this._config.username, readyTimeout: sasLaunchTimeout, keepaliveInterval: KEEPALIVE_INTERVAL, - keepaliveCountMax: 1440, + keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, debug: (msg) => { console.log(msg); }, From 8aa0c23bd4faa8d6714081fcd9bc6fd8bc095e22 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 16:42:50 -0400 Subject: [PATCH 10/32] refactor: remove redundant timeout test --- client/src/components/profile.ts | 1 + client/src/connection/ssh/const.ts | 2 +- client/src/connection/ssh/index.ts | 18 ++++++------------ client/test/connection/ssh/index.test.ts | 22 ---------------------- 4 files changed, 8 insertions(+), 35 deletions(-) diff --git a/client/src/components/profile.ts b/client/src/components/profile.ts index 9ef9766d4..264a6adb2 100644 --- a/client/src/components/profile.ts +++ b/client/src/components/profile.ts @@ -88,6 +88,7 @@ export interface SSHProfile extends BaseProfile { saspath: string; port: number; username: string; + privateKeyFilePath?: string; } export interface COMProfile extends BaseProfile { diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index 943bb86c6..f63181842 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -7,7 +7,7 @@ export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to export const KEEPALIVE_UNANSWERED_THRESHOLD = 720; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. export const WORK_DIR_START_TAG = ""; export const WORK_DIR_END_TAG = ""; -export const SAS_LAUNCH_TIMEOUT = 600000; +export const SAS_LAUNCH_TIMEOUT = 10000; export const SUPPORTED_AUTH_METHODS = [ "publickey", "password", diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index e9d725bf1..76d2b1f95 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -24,11 +24,11 @@ import { import { KEEPALIVE_INTERVAL, KEEPALIVE_UNANSWERED_THRESHOLD, + SAS_LAUNCH_TIMEOUT, SUPPORTED_AUTH_METHODS, } from "./const"; +import { LineCodes } from "./types"; -const endCode = "--vscode-sas-extension-submit-end--"; -const sasLaunchTimeout = 600000; let sessionInstance: SSHSession; export interface Config extends BaseConfig { @@ -36,16 +36,10 @@ export interface Config extends BaseConfig { username: string; saspath: string; port: number; - privateKeyFilePath: string; + privateKeyFilePath?: string; } export function getSession(c: Config): Session { - if (!process.env.SSH_AUTH_SOCK) { - throw new Error( - l10n.t("SSH_AUTH_SOCK not set. Check Environment Variables."), - ); - } - if (!sessionInstance) { sessionInstance = new SSHSession(); } @@ -92,7 +86,7 @@ export class SSHSession extends Session { host: this._config.host, port: this._config.port, username: this._config.username, - readyTimeout: sasLaunchTimeout, + readyTimeout: SAS_LAUNCH_TIMEOUT, keepaliveInterval: KEEPALIVE_INTERVAL, keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, debug: (msg) => { @@ -119,7 +113,7 @@ export class SSHSession extends Session { this._reject = _reject; this._stream?.write(`${code}\n`); - this._stream?.write(`%put ${endCode};\n`); + this._stream?.write(`%put ${LineCodes.RunEndCode};\n`); }); }; @@ -195,7 +189,7 @@ export class SSHSession extends Session { if (!line) { return; } - if (line.endsWith(endCode)) { + if (line.endsWith(LineCodes.RunEndCode)) { // run completed this.getResult(); } diff --git a/client/test/connection/ssh/index.test.ts b/client/test/connection/ssh/index.test.ts index ddc217052..7f55ea4f2 100644 --- a/client/test/connection/ssh/index.test.ts +++ b/client/test/connection/ssh/index.test.ts @@ -124,28 +124,6 @@ describe("ssh connection", () => { await session.setup(); }, "Shell Connection Failed"); }); - - it("rejects on ready timeout", async () => { - const config = { - host: "host", - username: "username", - port: 22, - saspath: "/path/to/sas_u8", - sasOptions: [], - agentSocket: "/agent/socket", - }; - - sandbox.stub(Client.prototype, "connect").callsFake(function () { - sandbox.clock.tick(50 * seconds); - this.emit("ready"); - return undefined; - }); - - session = new SSHSession(config); - await assertThrowsAsync(async () => { - await session.setup(); - }, "Failed to connect to Session. Check profile settings"); - }); }); describe("run", () => { From 91a6d0fea916540249ff27d540467134e92000ba Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 5 Sep 2024 16:45:54 -0400 Subject: [PATCH 11/32] chore: copyright header --- client/src/connection/ssh/auth.ts | 2 ++ client/src/connection/ssh/types.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index ba13a361f..b54df4899 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -1,3 +1,5 @@ +// Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 import { CancellationTokenSource, l10n, window } from "vscode"; import { readFileSync } from "fs"; diff --git a/client/src/connection/ssh/types.ts b/client/src/connection/ssh/types.ts index 76bfb2f00..28af06a94 100644 --- a/client/src/connection/ssh/types.ts +++ b/client/src/connection/ssh/types.ts @@ -1,3 +1,5 @@ +// Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 import { BaseConfig, LogLineTypeEnum } from ".."; export interface Config extends BaseConfig { From 0607d64f12efe11396d5051daad505ac26a340d4 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Tue, 10 Sep 2024 14:38:33 -0400 Subject: [PATCH 12/32] refactor for testability, auth tests --- client/src/connection/ssh/auth.ts | 344 ++++++++++++++---------- client/src/connection/ssh/index.ts | 22 +- client/test/connection/ssh/auth.test.ts | 152 +++++++++++ 3 files changed, 363 insertions(+), 155 deletions(-) create mode 100644 client/test/connection/ssh/auth.test.ts diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index b54df4899..a17ed0962 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -1,173 +1,233 @@ // Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { CancellationTokenSource, l10n, window } from "vscode"; +import { l10n, window } from "vscode"; import { readFileSync } from "fs"; -import { NextAuthHandler, utils } from "ssh2"; - -let _cancellationSource: CancellationTokenSource | undefined; +import { NextAuthHandler, ParsedKey, Prompt, utils } from "ssh2"; /** - * Authenticate to the server using the password method. - * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. - * @param resolve a function that resolves the promise that is waiting for the password - * @param username the user name to use for the connection + * Abstraction for presenting authentication prompts to the user. */ -export const passwordAuth = ( - cb: NextAuthHandler, - resolve: ((value?) => void) | undefined, - username: string, -) => { - promptForPassword(resolve).then((pw) => { - cb({ - type: "password", - password: pw, - username: username, +export interface AuthPresenter { + /** + * Prompt the user for a passphrase. + * @returns the passphrase entered by the user + */ + presentPasswordPrompt: () => Promise; + /** + * Prompt the user for a password. + * @returns the password entered by the user + */ + presentPassphrasePrompt: () => Promise; + /** + * Present multiple prompts to the user. + * This scenario can happen when the server sends multiple input prompts to the user during keyboard-interactive authentication. + * Auth setups involving MFA or PAM can trigger this scenario. + * One input box will be presented for each prompt. + * @param prompts an array of prompts to present to the user + * @returns array of answers to the prompts + */ + presentMultiplePrompts: (prompts: Prompt[]) => Promise; +} + +class AuthPresenterImpl implements AuthPresenter { + presentPasswordPrompt = async (): Promise => { + return this.presentSecurePrompt( + l10n.t("Enter your password for this connection"), + l10n.t("Password Required"), + ); + }; + + presentPassphrasePrompt = async (): Promise => { + return this.presentSecurePrompt( + l10n.t("Enter the passphrase for the private key"), + ); + }; + + presentMultiplePrompts = async (prompts: Prompt[]): Promise => { + const answers: string[] = []; + for (const prompt of prompts) { + this.presentSecurePrompt(prompt.prompt).then((answer) => { + answers.push(answer); + }); + } + return answers; + }; + + /** + * Present a secure prompt to the user. + * @param prompt the prompt to display to the user + * @param title optional title for the prompt + * @returns the user's response to the prompt + */ + private presentSecurePrompt = async ( + prompt: string, + title?: string, + ): Promise => { + return window.showInputBox({ + ignoreFocusOut: true, + prompt: prompt, + title: title, + password: true, }); - }); -}; + }; +} /** - * Authenticate to the server using the keyboard-interactive method. - * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. - * @param resolve a function that resolves the promise that is waiting for authentication - * @param username the user name to use for the connection + * Handles the authentication process for the ssh connection. + * */ -export const keyboardInteractiveAuth = ( - cb: NextAuthHandler, - resolve: ((value?) => void) | undefined, - username: string, -) => { - cb({ - type: "keyboard-interactive", - username: username, - prompt: (_name, _instructions, _instructionsLang, prompts, cb) => { - if (prompts.length === 1 && prompts[0].prompt === "Password:") { - promptForPassword(resolve).then((pw) => { - cb([pw]); - }); - } else { - cb([]); - } - }, - }); -}; +export class AuthHandler { + private _authPresenter: AuthPresenter; + private _keyParser: KeyParser; -/** - * Authenticate to the server using the ssh-agent. See the extension README for more information on how to set up the ssh-agent. - * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. - * @param username the user name to use for the connection - */ -export const sshAgentAuth = (cb: NextAuthHandler, username: string) => { - //attempt to auth using ssh-agent - cb({ - type: "agent", - agent: process.env.SSH_AUTH_SOCK, - username: username, - }); -}; + constructor(authPresenter?: AuthPresenter, keyParser?: KeyParser) { + this._authPresenter = authPresenter; + this._keyParser = keyParser; -/** - * Authenticate to the server using a private key file. - * If a private key file is defined in the connection profile, this function will read the file and use it to authenticate to the server. - * If the key is encrypted, the user will be prompted for the passphrase. - * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. - * @param resolve a function that resolves the promise that is waiting for authentication - * @param privateKeyFilePath the path to the private key file defined in the connection profile - * @param username the user name to use for the connection - */ -export const privateKeyAuth = ( - cb: NextAuthHandler, - resolve: ((value?) => void) | undefined, - privateKeyFilePath: string, - username: string, -) => { - let keyContents: Buffer; - try { - keyContents = readFileSync(privateKeyFilePath); - } catch (e) { - l10n.t("Error reading private key file: {filePath}, error: {message}", { - filePath: privateKeyFilePath, - message: e.message, - }); + if (!authPresenter) { + this._authPresenter = new AuthPresenterImpl(); + } + if (!keyParser) { + this._keyParser = new KeyParserImpl(); + } } - //check for passphrase, prompt if necessary - //and then attempt to auth - const parsedKeyResult = utils.parseKey(keyContents); - const hasParseError = parsedKeyResult instanceof Error; - const passphraseRequired = - hasParseError && - parsedKeyResult.message === - "Encrypted OpenSSH private key detected, but no passphrase given"; - // key is encrypted, prompt for passphrase - if (passphraseRequired) { - promptForPassphrase(resolve).then((passphrase) => { - //parse the keyfile using the passphrase - const reparsedKeyContentsResult = utils.parseKey(keyContents, passphrase); - - if (!(reparsedKeyContentsResult instanceof Error)) { + + /** + * Authenticate to the server using the password method. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for the password + * @param username the user name to use for the connection + */ + passwordAuth = (cb: NextAuthHandler, username: string) => { + this._authPresenter.presentPasswordPrompt().then((pw) => { + cb({ + type: "password", + password: pw, + username: username, + }); + }); + }; + + /** + * Authenticate to the server using the keyboard-interactive method. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for authentication + * @param username the user name to use for the connection + */ + keyboardInteractiveAuth = (cb: NextAuthHandler, username: string) => { + cb({ + type: "keyboard-interactive", + username: username, + prompt: (_name, _instructions, _instructionsLang, prompts, promptCb) => { + // often, the server will only send a single prompt for the password. + // however, PAM can send multiple prompts, so we need to handle that case + this._authPresenter + .presentMultiplePrompts(prompts) + .then((answers) => promptCb(answers)); + }, + }); + }; + + /** + * Authenticate to the server using the ssh-agent. See the extension README for more information on how to set up the ssh-agent. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param username the user name to use for the connection + */ + sshAgentAuth = (cb: NextAuthHandler, username: string) => { + cb({ + type: "agent", + agent: process.env.SSH_AUTH_SOCK, + username: username, + }); + }; + + /** + * Authenticate to the server using a private key file. + * If a private key file is defined in the connection profile, this function will read the file and use it to authenticate to the server. + * If the key is encrypted, the user will be prompted for the passphrase. + * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. + * @param resolve a function that resolves the promise that is waiting for authentication + * @param privateKeyFilePath the path to the private key file defined in the connection profile + * @param username the user name to use for the connection + */ + privateKeyAuth = ( + cb: NextAuthHandler, + privateKeyFilePath: string, + username: string, + ) => { + // first, try to parse the key file without a passphrase + const parsedKeyResult = this._keyParser.parseKey(privateKeyFilePath); + const hasParseError = parsedKeyResult instanceof Error; + const passphraseRequired = + hasParseError && + parsedKeyResult.message === + "Encrypted OpenSSH private key detected, but no passphrase given"; + // key is encrypted, prompt for passphrase + if (passphraseRequired) { + this._authPresenter.presentPassphrasePrompt().then((passphrase) => { + //parse the keyfile using the passphrase + const passphrasedKeyContentsResult = this._keyParser.parseKey( + privateKeyFilePath, + passphrase, + ); + + if (passphrasedKeyContentsResult instanceof Error) { + throw passphrasedKeyContentsResult; + } cb({ type: "publickey", - key: reparsedKeyContentsResult, + key: passphrasedKeyContentsResult, passphrase: passphrase, username: username, }); + }); + } else { + if (hasParseError) { + throw parsedKeyResult; } - }); - } else { - if (!hasParseError) { cb({ type: "publickey", key: parsedKeyResult, username: username, }); } - } -}; + }; +} /** - * Prompt the user for a passphrase. - * @param resolve a function that resolves the promise that is waiting for authentication - * @returns the passphrase entered by the user + * Parses a private key file. */ -const promptForPassphrase = async (resolve): Promise => { - const passphrase = await window.showInputBox({ - prompt: l10n.t("Enter the passphrase for the private key."), - password: true, - }); - - // user cancelled password dialog - if (!passphrase) { - resolve?.({}); - } - - return passphrase; -}; +export interface KeyParser { + /** + * Parse the private key file. + * If a passphrase is specified, the key will be decrypted using the passphrase. + * @param privateKeyPath the path to the private key file + * @param passphrase the passphrase to decrypt the key if applicable + * @returns the parsed key or an error if the key could not be parsed + */ + parseKey: (privateKeyPath: string, passphrase?: string) => ParsedKey | Error; +} -/** - * Prompt the user for a password. - * @param resolve a function that resolves the promise that is waiting for authentication - * @returns the password entered by the user - */ -const promptForPassword = async ( - resolve: ((value?) => void) | undefined, -): Promise => { - const source = new CancellationTokenSource(); - _cancellationSource = source; - const pw = await window.showInputBox( - { - ignoreFocusOut: true, - password: true, - prompt: l10n.t("Enter your password for this connection."), - title: l10n.t("Password Required"), - }, - _cancellationSource.token, - ); - - // user cancelled password dialog - if (!pw) { - resolve?.({}); - } +class KeyParserImpl implements KeyParser { + private readKeyFile = (privateKeyPath: string): Buffer => { + try { + return readFileSync(privateKeyPath); + } catch (e) { + throw new Error( + l10n.t("Error reading private key file: {filePath}, error: {message}", { + filePath: privateKeyPath, + message: e.message, + }), + ); + } + }; - return pw; -}; + public parseKey = ( + privateKeyPath: string, + passphrase?: string, + ): ParsedKey | Error => { + const keyContents = this.readKeyFile(privateKeyPath); + return utils.parseKey(keyContents, passphrase); + }; +} diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 76d2b1f95..084ff2feb 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -15,12 +15,7 @@ import { BaseConfig, RunResult } from ".."; import { updateStatusBarItem } from "../../components/StatusBarItem"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; -import { - keyboardInteractiveAuth, - passwordAuth, - privateKeyAuth, - sshAgentAuth, -} from "./auth"; +import { AuthHandler } from "./auth"; import { KEEPALIVE_INTERVAL, KEEPALIVE_UNANSWERED_THRESHOLD, @@ -55,6 +50,7 @@ export class SSHSession extends Session { private _html5FileName = ""; private _sessionReady: boolean; private _authMethods: AuthenticationType[]; //auth methods that this session can support + private _authHandler: AuthHandler; constructor(c?: Config) { super(); @@ -62,6 +58,7 @@ export class SSHSession extends Session { this._conn = new Client(); this._authMethods = ["publickey", "password", "keyboard-interactive"]; this._sessionReady = false; + this._authHandler = new AuthHandler(); } public sessionId? = (): string => { @@ -92,7 +89,7 @@ export class SSHSession extends Session { debug: (msg) => { console.log(msg); }, - authHandler: this.authHandler, + authHandler: this.handleSSHAuthentication, }; this._conn @@ -245,7 +242,7 @@ export class SSHSession extends Session { this._sessionReady = false; }; - private authHandler: AuthHandlerMiddleware = ( + private handleSSHAuthentication: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], _partialSuccess: boolean, cb: NextAuthHandler, @@ -279,23 +276,22 @@ export class SSHSession extends Session { case "publickey": { //user set a keyfile path in profile config if (this._config.privateKeyFilePath) { - privateKeyAuth( + this._authHandler.privateKeyAuth( cb, - this._resolve, this._config.privateKeyFilePath, this._config.username, ); } else if (process.env.SSH_AUTH_SOCK) { - sshAgentAuth(cb, this._config.username); + this._authHandler.sshAgentAuth(cb, this._config.username); } break; } case "password": { - passwordAuth(cb, this._resolve, this._config.username); + this._authHandler.passwordAuth(cb, this._config.username); break; } case "keyboard-interactive": { - keyboardInteractiveAuth(cb, this._resolve, this._config.username); + this._authHandler.keyboardInteractiveAuth(cb, this._config.username); break; } default: diff --git a/client/test/connection/ssh/auth.test.ts b/client/test/connection/ssh/auth.test.ts new file mode 100644 index 000000000..4f9efcaab --- /dev/null +++ b/client/test/connection/ssh/auth.test.ts @@ -0,0 +1,152 @@ +import { expect } from "chai"; +import * as fs from "fs"; +import { KeyboardInteractiveAuthMethod, ParsedKey } from "ssh2"; +import sinon, { stubInterface } from "ts-sinon"; + +import { + AuthHandler, + AuthPresenter, + KeyParser, +} from "../../../src/connection/ssh/auth"; + +describe("ssh connection auth handler", () => { + let authHandler: AuthHandler; + + describe("sshAgentAuth", () => { + beforeEach(() => { + process.env.SSH_AUTH_SOCK = "socketPath"; + }); + + it("pass socket path to the callback", async () => { + const cb = sinon.stub(); + const username = "username"; + const socketPath = "socketPath"; + + const presenter = stubInterface(); + authHandler = new AuthHandler(presenter); + + await authHandler.sshAgentAuth(cb, username); + + sinon.assert.calledWith(cb, { + type: "agent", + agent: socketPath, + username: username, + }); + }); + }); + + describe("privateKeyAuth", () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + }); + afterEach(() => { + sandbox.restore(); + }); + + it("should pass the key contents to the callback for an unencrypted key", async () => { + const cb = sandbox.stub(); + const username = "username"; + const privateKeyFilePath = "privateKeyFilePath"; + + sandbox + .stub(fs, "readFileSync") + .callsFake(() => Buffer.from("keyContents")); + + const presenter = stubInterface(); + const keyParser = stubInterface(); + + const key = stubInterface(); + keyParser.parseKey.returns(key); + authHandler = new AuthHandler(presenter, keyParser); + + await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); + + sinon.assert.calledWith(cb, { + type: "publickey", + key: key, + username: username, + }); + }); + }); + + it("should pass the key contents and passphrase to the callback for an encrypted key", async () => { + const cb = sinon.stub(); + const username = "username"; + const passphrase = "passphrase"; + const privateKeyFilePath = "privateKeyFilePath"; + + const presenter = stubInterface(); + const keyParser = stubInterface(); + + const key = stubInterface(); + keyParser.parseKey + .withArgs(privateKeyFilePath) + .returns( + new Error( + "Encrypted OpenSSH private key detected, but no passphrase given", + ), + ); + keyParser.parseKey.withArgs(privateKeyFilePath, passphrase).returns(key); + presenter.presentPassphrasePrompt.resolves(passphrase); + + authHandler = new AuthHandler(presenter, keyParser); + + await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); + + sinon.assert.calledWith(cb, { + type: "publickey", + key: key, + passphrase: passphrase, + username: username, + }); + }); + + describe("passwordAuth", () => { + it("should pass the password to the callback", async () => { + const cb = sinon.stub(); + const username = "username"; + const pw = "password"; + + const presenter = stubInterface(); + presenter.presentPasswordPrompt.resolves(pw); + + authHandler = new AuthHandler(presenter); + + await authHandler.passwordAuth(cb, username); + + sinon.assert.calledWith(cb, { + type: "password", + password: pw, + username: username, + }); + }); + }); + + describe("keyboardAuth", () => { + it("should present input prompts and pass the answers to the callback", async () => { + const promptCbStub = sinon.stub(); + const cb = (auth: KeyboardInteractiveAuthMethod) => { + expect(auth.type).to.equal("keyboard-interactive"); + expect(auth.username).to.equal("username"); + auth.prompt( + "name", + "instruction", + "lang", + [{ prompt: "question1" }, { prompt: "question2" }], + promptCbStub, + ); + }; + + const answers = ["answer1", "answer2"]; + const presenter = stubInterface(); + presenter.presentMultiplePrompts.resolves(answers); + + authHandler = new AuthHandler(presenter); + + await authHandler.keyboardInteractiveAuth(cb, "username"); + sinon.assert.calledWith(promptCbStub, answers); + }); + }); +}); From 6ac98e8b5dc859cb45dfdec9d21a146e8efc2f35 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 12 Sep 2024 08:35:04 -0400 Subject: [PATCH 13/32] feat: set working path to work dir --- client/src/connection/ssh/index.ts | 56 ++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 084ff2feb..e7d73eedd 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -13,6 +13,7 @@ import { import { BaseConfig, RunResult } from ".."; import { updateStatusBarItem } from "../../components/StatusBarItem"; +import { LineParser } from "../LineParser"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; import { AuthHandler } from "./auth"; @@ -21,6 +22,8 @@ import { KEEPALIVE_UNANSWERED_THRESHOLD, SAS_LAUNCH_TIMEOUT, SUPPORTED_AUTH_METHODS, + WORK_DIR_END_TAG, + WORK_DIR_START_TAG, } from "./const"; import { LineCodes } from "./types"; @@ -51,6 +54,8 @@ export class SSHSession extends Session { private _sessionReady: boolean; private _authMethods: AuthenticationType[]; //auth methods that this session can support private _authHandler: AuthHandler; + private _workDirectory: string; + private _workDirectoryParser: LineParser; constructor(c?: Config) { super(); @@ -59,6 +64,11 @@ export class SSHSession extends Session { this._authMethods = ["publickey", "password", "keyboard-interactive"]; this._sessionReady = false; this._authHandler = new AuthHandler(); + this._workDirectoryParser = new LineParser( + WORK_DIR_START_TAG, + WORK_DIR_END_TAG, + false, + ); } public sessionId? = (): string => { @@ -135,7 +145,7 @@ export class SSHSession extends Session { } let fileContents = ""; this._conn.exec( - `cat ${this._html5FileName}.htm`, + `cat ${this._workDirectory}/${this._html5FileName}.htm`, (err: Error, s: ClientChannel) => { if (err) { this._reject?.(err); @@ -167,16 +177,47 @@ export class SSHSession extends Session { this._reject = undefined; this._html5FileName = ""; this.clearAuthState(); + this._workDirectory = undefined; this._conn.end(); updateStatusBarItem(false); }; + private fetchWorkDirectory = (line: string): string => { + let foundWorkDirectory = ""; + if ( + !line.includes(`%put ${WORK_DIR_START_TAG};`) && + !line.includes(`%put &workDir;`) && + !line.includes(`%put ${WORK_DIR_END_TAG};`) + ) { + foundWorkDirectory = this._workDirectoryParser.processLine(line); + } else { + // If the line is the put statement, we don't need to log that + return; + } + // We don't want to output any of the captured lines + if (this._workDirectoryParser.isCapturingLine()) { + return; + } + + return foundWorkDirectory || ""; + }; + private resolveSystemVars = (): void => { + const code = `%let workDir = %sysfunc(pathname(work)); + %put ${WORK_DIR_START_TAG}; + %put &workDir; + %put ${WORK_DIR_END_TAG}; + %let rc = %sysfunc(dlgcdir("&workDir")); + run; + `; + this._stream.write(code); + }; private onStreamData = (data: Buffer): void => { const output = data.toString().trimEnd(); if (!this._sessionReady && output.endsWith("?")) { this._sessionReady = true; this._resolve?.(); + this.resolveSystemVars(); updateStatusBarItem(true); return; } @@ -186,17 +227,26 @@ export class SSHSession extends Session { if (!line) { return; } - if (line.endsWith(LineCodes.RunEndCode)) { + const trimmedLine = line.trim(); + if (trimmedLine.endsWith(LineCodes.RunEndCode)) { // run completed this.getResult(); } - if (!(line.endsWith("?") || line.endsWith(">"))) { + if (!(trimmedLine.endsWith("?") || trimmedLine.endsWith(">"))) { this._html5FileName = extractOutputHtmlFileName( line, this._html5FileName, ); this._onExecutionLogFn?.([{ type: "normal", line }]); } + + if (this._sessionReady && !this._workDirectory) { + const foundWorkDir = this.fetchWorkDirectory(line); + if (foundWorkDir) { + const match = foundWorkDir.match(/\/[^\s\r]+/); + this._workDirectory = match ? match[0] : ""; + } + } }); }; From a092524aa485587761df265c0d9cdc7b902007c1 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Fri, 13 Sep 2024 10:56:57 -0400 Subject: [PATCH 14/32] metadata for private key, various fixes found during testing --- client/src/components/profile.ts | 16 +++++ client/src/connection/ssh/auth.ts | 11 ++-- client/src/connection/ssh/const.ts | 2 +- client/src/connection/ssh/index.ts | 2 +- .../test/components/profile/profile.test.ts | 10 +++ client/test/connection/ssh/auth.test.ts | 64 +++++++++---------- package.json | 5 ++ package.nls.json | 1 + 8 files changed, 72 insertions(+), 39 deletions(-) diff --git a/client/src/components/profile.ts b/client/src/components/profile.ts index 264a6adb2..8b181cc32 100644 --- a/client/src/components/profile.ts +++ b/client/src/components/profile.ts @@ -581,6 +581,14 @@ export class ProfileConfig { return; } + profileClone.privateKeyFilePath = await createInputTextBox( + ProfilePromptType.PrivateKeyFilePath, + profileClone.privateKeyFilePath, + ); + if (profileClone.privateKeyFilePath === undefined) { + return; + } + await this.upsertProfile(name, profileClone); } else if (profileClone.connectionType === ConnectionType.COM) { profileClone.sasOptions = []; @@ -658,6 +666,7 @@ export enum ProfilePromptType { SASPath, Port, Username, + PrivateKeyFilePath, } /** @@ -795,6 +804,13 @@ const input: ProfilePromptInput = { placeholder: l10n.t("Enter your username"), description: l10n.t("Enter your SAS server username."), }, + [ProfilePromptType.PrivateKeyFilePath]: { + title: l10n.t("Private Key File Path (optional)"), + placeholder: l10n.t("Enter the private key file path"), + description: l10n.t( + "Enter the local path of the private key file. Leave empty to use SSH Agent.", + ), + }, }; /** diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index a17ed0962..f71594ccd 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -13,7 +13,7 @@ export interface AuthPresenter { * Prompt the user for a passphrase. * @returns the passphrase entered by the user */ - presentPasswordPrompt: () => Promise; + presentPasswordPrompt: (username: string) => Promise; /** * Prompt the user for a password. * @returns the password entered by the user @@ -31,9 +31,9 @@ export interface AuthPresenter { } class AuthPresenterImpl implements AuthPresenter { - presentPasswordPrompt = async (): Promise => { + presentPasswordPrompt = async (username: string): Promise => { return this.presentSecurePrompt( - l10n.t("Enter your password for this connection"), + l10n.t("Enter the password for user: {username}", { username }), l10n.t("Password Required"), ); }; @@ -41,6 +41,7 @@ class AuthPresenterImpl implements AuthPresenter { presentPassphrasePrompt = async (): Promise => { return this.presentSecurePrompt( l10n.t("Enter the passphrase for the private key"), + l10n.t("Passphrase Required"), ); }; @@ -100,7 +101,7 @@ export class AuthHandler { * @param username the user name to use for the connection */ passwordAuth = (cb: NextAuthHandler, username: string) => { - this._authPresenter.presentPasswordPrompt().then((pw) => { + this._authPresenter.presentPasswordPrompt(username).then((pw) => { cb({ type: "password", password: pw, @@ -162,7 +163,7 @@ export class AuthHandler { const passphraseRequired = hasParseError && parsedKeyResult.message === - "Encrypted OpenSSH private key detected, but no passphrase given"; + "Encrypted private OpenSSH key detected, but no passphrase given"; // key is encrypted, prompt for passphrase if (passphraseRequired) { this._authPresenter.presentPassphrasePrompt().then((passphrase) => { diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index f63181842..f32ca572a 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -7,7 +7,7 @@ export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to export const KEEPALIVE_UNANSWERED_THRESHOLD = 720; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. export const WORK_DIR_START_TAG = ""; export const WORK_DIR_END_TAG = ""; -export const SAS_LAUNCH_TIMEOUT = 10000; +export const SAS_LAUNCH_TIMEOUT = 60000; export const SUPPORTED_AUTH_METHODS = [ "publickey", "password", diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index e7d73eedd..579d23292 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -298,7 +298,7 @@ export class SSHSession extends Session { cb: NextAuthHandler, ) => { if (!authsLeft) { - cb("none"); //sending none will usually prompt the server to send supported auth methods + cb("none"); //sending none will prompt the server to send supported auth methods return; } diff --git a/client/test/components/profile/profile.test.ts b/client/test/components/profile/profile.test.ts index 3ef72a670..e278d0a18 100644 --- a/client/test/components/profile/profile.test.ts +++ b/client/test/components/profile/profile.test.ts @@ -92,6 +92,7 @@ describe("Profiles", async function () { sasPath: "sasPath", sasOptions: ["-nonews"], connectionType: "ssh", + privateKeyFilePath: "/private/key/file/path", }, }, }; @@ -654,6 +655,7 @@ describe("Profiles", async function () { sasOptions: ["-nonews"], saspath: "/sas/path", username: "username", + privateKeyFilePath: "/private/key/file/path", }; // Arrange // Act @@ -969,6 +971,14 @@ describe("Profiles", async function () { wantDescription: "Enter your SAS server username.", wantPlaceHolder: "Enter your username", }, + { + name: "Private Key File Path", + prompt: ProfilePromptType.PrivateKeyFilePath, + wantTitle: "Private Key File Path (optional)", + wantDescription: + "Enter the local path of the private key file. Leave empty to use SSH Agent.", + wantPlaceHolder: "Enter the private key file path", + }, ]; testCases.forEach((testCase) => { diff --git a/client/test/connection/ssh/auth.test.ts b/client/test/connection/ssh/auth.test.ts index 4f9efcaab..f29637c83 100644 --- a/client/test/connection/ssh/auth.test.ts +++ b/client/test/connection/ssh/auth.test.ts @@ -45,6 +45,38 @@ describe("ssh connection auth handler", () => { sandbox.restore(); }); + it("should pass the key contents and passphrase to the callback for an encrypted key", async () => { + const cb = sinon.stub(); + const username = "username"; + const passphrase = "passphrase"; + const privateKeyFilePath = "privateKeyFilePath"; + + const presenter = stubInterface(); + const keyParser = stubInterface(); + + const key = stubInterface(); + keyParser.parseKey + .withArgs(privateKeyFilePath) + .returns( + new Error( + "Encrypted private OpenSSH key detected, but no passphrase given", + ), + ); + keyParser.parseKey.withArgs(privateKeyFilePath, passphrase).returns(key); + presenter.presentPassphrasePrompt.resolves(passphrase); + + authHandler = new AuthHandler(presenter, keyParser); + + await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); + + sinon.assert.calledWith(cb, { + type: "publickey", + key: key, + passphrase: passphrase, + username: username, + }); + }); + it("should pass the key contents to the callback for an unencrypted key", async () => { const cb = sandbox.stub(); const username = "username"; @@ -71,38 +103,6 @@ describe("ssh connection auth handler", () => { }); }); - it("should pass the key contents and passphrase to the callback for an encrypted key", async () => { - const cb = sinon.stub(); - const username = "username"; - const passphrase = "passphrase"; - const privateKeyFilePath = "privateKeyFilePath"; - - const presenter = stubInterface(); - const keyParser = stubInterface(); - - const key = stubInterface(); - keyParser.parseKey - .withArgs(privateKeyFilePath) - .returns( - new Error( - "Encrypted OpenSSH private key detected, but no passphrase given", - ), - ); - keyParser.parseKey.withArgs(privateKeyFilePath, passphrase).returns(key); - presenter.presentPassphrasePrompt.resolves(passphrase); - - authHandler = new AuthHandler(presenter, keyParser); - - await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); - - sinon.assert.calledWith(cb, { - type: "publickey", - key: key, - passphrase: passphrase, - username: username, - }); - }); - describe("passwordAuth", () => { it("should pass the password to the callback", async () => { const cb = sinon.stub(); diff --git a/package.json b/package.json index a00334296..43069d129 100644 --- a/package.json +++ b/package.json @@ -321,6 +321,11 @@ "description": "%configuration.SAS.connectionProfiles.profiles.ssh.port%", "exclusiveMinimum": 1, "exclusiveMaximum": 65535 + }, + "privateKeyFilePath": { + "type": "string", + "default": "", + "description": "%configuration.SAS.connectionProfiles.profiles.ssh.privateKeyFilePath%" } } } diff --git a/package.nls.json b/package.nls.json index 78324ea13..0b6663b1e 100644 --- a/package.nls.json +++ b/package.nls.json @@ -51,6 +51,7 @@ "configuration.SAS.connectionProfiles.profiles.ssh.port": "SAS SSH Connection port", "configuration.SAS.connectionProfiles.profiles.ssh.saspath": "SAS SSH Connection executable path", "configuration.SAS.connectionProfiles.profiles.ssh.username": "SAS SSH Connection username", + "configuration.SAS.connectionProfiles.profiles.ssh.privateKeyFilePath": "SAS SSH Connection private key file path", "configuration.SAS.flowConversionMode": "Choose the conversion mode for notebooks", "configuration.SAS.flowConversionModeNode": "Convert each notebook cell to a node", "configuration.SAS.flowConversionModeSwimlane": "Convert each notebook cell to a swimlane", From debafaf17d2d8e92fe5627837a44c1040fd539cd Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Tue, 17 Sep 2024 11:18:53 -0400 Subject: [PATCH 15/32] fix: minor cleanup --- client/src/connection/ssh/auth.ts | 5 ++--- client/src/connection/ssh/index.ts | 15 ++++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index f71594ccd..77803e65d 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -48,9 +48,8 @@ class AuthPresenterImpl implements AuthPresenter { presentMultiplePrompts = async (prompts: Prompt[]): Promise => { const answers: string[] = []; for (const prompt of prompts) { - this.presentSecurePrompt(prompt.prompt).then((answer) => { - answers.push(answer); - }); + const answer = await this.presentSecurePrompt(prompt.prompt); + answers.push(answer); } return answers; }; diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 579d23292..2efa9dcda 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -39,9 +39,8 @@ export interface Config extends BaseConfig { export function getSession(c: Config): Session { if (!sessionInstance) { - sessionInstance = new SSHSession(); + sessionInstance = new SSHSession(c, new Client()); } - sessionInstance.config = c; return sessionInstance; } export class SSHSession extends Session { @@ -57,10 +56,10 @@ export class SSHSession extends Session { private _workDirectory: string; private _workDirectoryParser: LineParser; - constructor(c?: Config) { + constructor(c?: Config, client?: Client) { super(); this._config = c; - this._conn = new Client(); + this._conn = client; this._authMethods = ["publickey", "password", "keyboard-interactive"]; this._sessionReady = false; this._authHandler = new AuthHandler(); @@ -96,12 +95,17 @@ export class SSHSession extends Session { readyTimeout: SAS_LAUNCH_TIMEOUT, keepaliveInterval: KEEPALIVE_INTERVAL, keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, + debug: (msg) => { console.log(msg); }, authHandler: this.handleSSHAuthentication, }; + if (!this._conn) { + this._conn = new Client(); + } + this._conn .on("ready", () => { this._conn.shell(this.onShell); @@ -179,6 +183,7 @@ export class SSHSession extends Session { this.clearAuthState(); this._workDirectory = undefined; this._conn.end(); + sessionInstance = undefined; updateStatusBarItem(false); }; @@ -227,7 +232,7 @@ export class SSHSession extends Session { if (!line) { return; } - const trimmedLine = line.trim(); + const trimmedLine = line.trimEnd(); if (trimmedLine.endsWith(LineCodes.RunEndCode)) { // run completed this.getResult(); From e0e26dd89b8e82ad1da8727f1bacb4f35883e0ad Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 18 Sep 2024 10:57:00 -0400 Subject: [PATCH 16/32] fix: bug fixes found during mfa testing --- client/src/connection/ssh/auth.ts | 45 +++++++++++---- client/src/connection/ssh/const.ts | 12 ++-- client/src/connection/ssh/index.ts | 89 ++++++++++++++---------------- 3 files changed, 78 insertions(+), 68 deletions(-) diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index 77803e65d..8be028adb 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -27,29 +27,46 @@ export interface AuthPresenter { * @param prompts an array of prompts to present to the user * @returns array of answers to the prompts */ - presentMultiplePrompts: (prompts: Prompt[]) => Promise; + presentMultiplePrompts: ( + username: string, + prompts: Prompt[], + ) => Promise; } class AuthPresenterImpl implements AuthPresenter { presentPasswordPrompt = async (username: string): Promise => { - return this.presentSecurePrompt( + return this.presentPrompt( l10n.t("Enter the password for user: {username}", { username }), l10n.t("Password Required"), + true, ); }; presentPassphrasePrompt = async (): Promise => { - return this.presentSecurePrompt( + return this.presentPrompt( l10n.t("Enter the passphrase for the private key"), l10n.t("Passphrase Required"), + true, ); }; - presentMultiplePrompts = async (prompts: Prompt[]): Promise => { + presentMultiplePrompts = async ( + username: string, + prompts: Prompt[], + ): Promise => { const answers: string[] = []; for (const prompt of prompts) { - const answer = await this.presentSecurePrompt(prompt.prompt); - answers.push(answer); + const answer = await this.presentPrompt( + undefined, + l10n.t("User {username} {prompt}", { + username, + prompt: prompt.prompt, + }), + !prompt.echo, + ); + if (answer) { + answers.push(answer); + } } return answers; }; @@ -58,17 +75,19 @@ class AuthPresenterImpl implements AuthPresenter { * Present a secure prompt to the user. * @param prompt the prompt to display to the user * @param title optional title for the prompt + * @param isSecureInput whether the input should be hidden * @returns the user's response to the prompt */ - private presentSecurePrompt = async ( + private presentPrompt = async ( prompt: string, title?: string, + isSecureInput?: boolean, ): Promise => { return window.showInputBox({ ignoreFocusOut: true, prompt: prompt, title: title, - password: true, + password: isSecureInput, }); }; } @@ -119,18 +138,20 @@ export class AuthHandler { cb({ type: "keyboard-interactive", username: username, - prompt: (_name, _instructions, _instructionsLang, prompts, promptCb) => { + prompt: (_name, _instructions, _instructionsLang, prompts, finish) => { // often, the server will only send a single prompt for the password. // however, PAM can send multiple prompts, so we need to handle that case this._authPresenter - .presentMultiplePrompts(prompts) - .then((answers) => promptCb(answers)); + .presentMultiplePrompts(username, prompts) + .then((answers) => { + finish(answers); + }); }, }); }; /** - * Authenticate to the server using the ssh-agent. See the extension README for more information on how to set up the ssh-agent. + * Authenticate to the server using the ssh-agent. See the extension Docs for more information on how to set up the ssh-agent. * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. * @param username the user name to use for the connection */ diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index f32ca572a..4c53e1752 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -2,14 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 const SECOND = 1000; +const MINUTE = 60 * SECOND; +const HOUR = 60 * MINUTE; export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to send SSH-level keepalive packets to the server. Set to 0 to disable. -// 720 * 60 seconds = 43200 seconds = 12 hours -export const KEEPALIVE_UNANSWERED_THRESHOLD = 720; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. +export const KEEPALIVE_UNANSWERED_THRESHOLD = 12 * HOUR; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. export const WORK_DIR_START_TAG = ""; export const WORK_DIR_END_TAG = ""; -export const SAS_LAUNCH_TIMEOUT = 60000; -export const SUPPORTED_AUTH_METHODS = [ - "publickey", - "password", - "keyboard-interactive", -]; +export const CONNECT_READY_TIMEOUT = 5 * MINUTE; //allow extra time due to possible prompting diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 2efa9dcda..5add4a14e 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -18,10 +18,9 @@ import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; import { AuthHandler } from "./auth"; import { + CONNECT_READY_TIMEOUT, KEEPALIVE_INTERVAL, KEEPALIVE_UNANSWERED_THRESHOLD, - SAS_LAUNCH_TIMEOUT, - SUPPORTED_AUTH_METHODS, WORK_DIR_END_TAG, WORK_DIR_START_TAG, } from "./const"; @@ -51,7 +50,6 @@ export class SSHSession extends Session { private _reject: ((reason?) => void) | undefined; private _html5FileName = ""; private _sessionReady: boolean; - private _authMethods: AuthenticationType[]; //auth methods that this session can support private _authHandler: AuthHandler; private _workDirectory: string; private _workDirectoryParser: LineParser; @@ -60,7 +58,6 @@ export class SSHSession extends Session { super(); this._config = c; this._conn = client; - this._authMethods = ["publickey", "password", "keyboard-interactive"]; this._sessionReady = false; this._authHandler = new AuthHandler(); this._workDirectoryParser = new LineParser( @@ -92,7 +89,7 @@ export class SSHSession extends Session { host: this._config.host, port: this._config.port, username: this._config.username, - readyTimeout: SAS_LAUNCH_TIMEOUT, + readyTimeout: CONNECT_READY_TIMEOUT, keepaliveInterval: KEEPALIVE_INTERVAL, keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, @@ -107,6 +104,7 @@ export class SSHSession extends Session { } this._conn + .on("close", this.onConnectionClose) .on("ready", () => { this._conn.shell(this.onShell); }) @@ -136,6 +134,12 @@ export class SSHSession extends Session { this._stream.close(); }; + private onConnectionClose = () => { + if (!this._sessionReady) { + this._reject?.(new Error(l10n.t("Could not connect to the SAS server."))); + } + }; + private onConnectionError = (err: Error) => { this.clearAuthState(); this._reject?.(err); @@ -206,6 +210,7 @@ export class SSHSession extends Session { return foundWorkDirectory || ""; }; + private resolveSystemVars = (): void => { const code = `%let workDir = %sysfunc(pathname(work)); %put ${WORK_DIR_START_TAG}; @@ -216,6 +221,7 @@ export class SSHSession extends Session { `; this._stream.write(code); }; + private onStreamData = (data: Buffer): void => { const output = data.toString().trimEnd(); @@ -293,68 +299,55 @@ export class SSHSession extends Session { * Resets the SSH auth state. */ private clearAuthState = (): void => { - this._authMethods = undefined; this._sessionReady = false; }; private handleSSHAuthentication: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], _partialSuccess: boolean, - cb: NextAuthHandler, + nextAuth: NextAuthHandler, ) => { if (!authsLeft) { - cb("none"); //sending none will prompt the server to send supported auth methods + nextAuth("none"); //sending none will prompt the server to send supported auth methods return; } - if (!this._authMethods) { - this._authMethods = authsLeft; - } - - if (this._authMethods.length === 0) { - //if we're out of auth methods to try, then reject with an error + if (authsLeft.length === 0) { this._reject?.( new Error(l10n.t("Could not authenticate to the SSH server.")), ); - this.clearAuthState(); - //returning false will stop the auth process - return false; + return false; //returning false will stop the authentication process } - //otherwise, fetch the next auth method to try - const authMethod = this._authMethods.shift(); - - //make sure the auth method is supported by the server - if (SUPPORTED_AUTH_METHODS.includes(authMethod)) { - switch (authMethod) { - case "publickey": { - //user set a keyfile path in profile config - if (this._config.privateKeyFilePath) { - this._authHandler.privateKeyAuth( - cb, - this._config.privateKeyFilePath, - this._config.username, - ); - } else if (process.env.SSH_AUTH_SOCK) { - this._authHandler.sshAgentAuth(cb, this._config.username); - } - break; - } - case "password": { - this._authHandler.passwordAuth(cb, this._config.username); - break; + const authMethod = authsLeft.shift(); + switch (authMethod) { + case "publickey": { + //user set a keyfile path in profile config + if (this._config.privateKeyFilePath) { + this._authHandler.privateKeyAuth( + nextAuth, + this._config.privateKeyFilePath, + this._config.username, + ); + } else if (process.env.SSH_AUTH_SOCK) { + this._authHandler.sshAgentAuth(nextAuth, this._config.username); } - case "keyboard-interactive": { - this._authHandler.keyboardInteractiveAuth(cb, this._config.username); - break; - } - default: - cb("none"); + break; } - } else { - console.warn(`Server does not support ${authMethod} auth method.`); - cb("none"); + case "password": { + this._authHandler.passwordAuth(nextAuth, this._config.username); + break; + } + case "keyboard-interactive": { + this._authHandler.keyboardInteractiveAuth( + nextAuth, + this._config.username, + ); + break; + } + default: + nextAuth(authMethod); } }; } From 51ce78aaa8aebd393711b128334b335ff9033101 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Wed, 18 Sep 2024 14:08:47 -0400 Subject: [PATCH 17/32] doc: ssh auth refactor --- .../docs/Configurations/Profiles/sas9ssh.md | 56 +++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/website/docs/Configurations/Profiles/sas9ssh.md b/website/docs/Configurations/Profiles/sas9ssh.md index 0e646f47b..4f3c344c5 100644 --- a/website/docs/Configurations/Profiles/sas9ssh.md +++ b/website/docs/Configurations/Profiles/sas9ssh.md @@ -4,7 +4,7 @@ sidebar_position: 4 # SAS 9.4 (remote - SSH) Connection Profile -For a secure connection to SAS 9.4 (remote - SSH) server, a public / private SSH key pair is required. The socket defined in the environment variable `SSH_AUTH_SOCK` is used to communicate with ssh-agent to authenticate the SSH session. The private key must be registered with the ssh-agent. The steps for configuring SSH follow. +This connection method uses SSH to authenticate to a SAS Server and run SAS Code using [Interactive Line Mode](https://go.documentation.sas.com/doc/en/pgmsascdc/9.4_3.5/hostunx/n16ui9f6dacn8pn1t0y2hgxgi7wa.htm). A number of methods are available to create a secure connection to the SAS 9.4 server. :::note @@ -18,18 +18,29 @@ A SAS 9.4 (remote – SSH) connection profile includes the following parameters: `"connectionType": "ssh"` -| Name | Description | Additional Notes | -| ---------- | ------------------------------------ | -------------------------------------------------------------------------- | -| `host` | SSH Server Host | Appears when hovering over the status bar. | -| `username` | SSH Server Username | The username to establish the SSH connection to the server. | -| `port` | SSH Server Port | The SSH port of the SSH server. The default value is 22. | -| `saspath` | Path to SAS Executable on the server | Must be a fully qualified path on the SSH server to a SAS executable file. | +| Name | Description | Additional Notes | +| -------------------- | ------------------------------------ | ----------------------------------------------------------------------------- | +| `host` | SSH Server Host | Appears when hovering over the status bar. | +| `username` | SSH Server Username | The username to establish the SSH connection to the server. | +| `port` | SSH Server Port | The SSH port of the SSH server. The default value is 22. | +| `saspath` | Path to SAS Executable on the server | Must be a fully qualified path on the SSH server to a SAS executable file. | +| `privateKeyFilePath` | SSH Private Key File (optional) | Must be a fully qualified path on the same machine that VSCode is running on. | -## Required setup for connection to SAS 9.4 (remote - SSH) +## Authenticating to a SAS Server -In order to configure the connection between VS Code and SAS 9, you must configure OpenSSH. Follow the steps below to complete the setup. +The extension will attempt to authenticate to the SAS Server over ssh using the auth methods specified in the SSH Server configuration defined on the SAS Server. The extension currently supports using the SSH auth methods listed below: -### Windows +- [Publickey](#publickey) +- [Password](#password) +- [Keyboard Interactive](#keyboard-interactive) + +### Publickey + +#### SSH Agent + +When using publickey SSH authentication, The extension can be configured to use keys defined in the SSH Agent. The socket defined in the environment variable `SSH_AUTH_SOCK` is used to communicate with ssh-agent to authenticate the SSH session. The private key must be registered with the ssh-agent when using this method. The steps for configuring SSH follow. Follow the steps below to complete the setup. + +##### Windows 1. Enable OpenSSH for Windows using these [instructions](https://learn.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse?tabs=gui). @@ -80,7 +91,7 @@ Note: the default path to the SAS executable (saspath) is /opt/sasinside/SASHome 9. Add the public part of the keypair to the SAS server. Add the contents of the key file to the ~/.ssh/authorized_keys file. -### Mac +##### Mac 1. Start ssh-agent in the background: @@ -118,3 +129,26 @@ Host host.machine.name ``` 6. Add the public part of the keypair to the SAS server. Add the contents of the key file to the ~/.ssh/authorized_keys file. + +### Connection Profile + +A private key can optionally be specified in the `privateKeyFilePath` field in the connection profile for SAS 9.4 (remote - SSH). This is useful for auth setups where the SSH Agent cannot be used. If a private key file contains a passphrase, the user will be prompted to enter it during each Session creation for which it is used. + +```json +"ssh_test": { + "connectionType": "ssh", + "host": "host.machine.name", + "saspath": "/path/to/sas/executable", + "username": "username", + "port": 22, + "privateKeyFilePath": "/path/to/privatekey/file" +} +``` + +### Password + +Enter the password using the secure input prompt during each Session creation. To authenticate without using a password, configure the extension using one of the Publickey setups. + +### Keyboard Interactive + +Enter the response to each question using the secure input prompts during each Session creation. From ea17b7554c9090c05c893abe452a74becde45891 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 19 Sep 2024 08:08:51 -0400 Subject: [PATCH 18/32] rename Connection Profile to Private Key File Path --- website/docs/Configurations/Profiles/sas9ssh.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/Configurations/Profiles/sas9ssh.md b/website/docs/Configurations/Profiles/sas9ssh.md index 4f3c344c5..522dfb6f8 100644 --- a/website/docs/Configurations/Profiles/sas9ssh.md +++ b/website/docs/Configurations/Profiles/sas9ssh.md @@ -130,7 +130,7 @@ Host host.machine.name 6. Add the public part of the keypair to the SAS server. Add the contents of the key file to the ~/.ssh/authorized_keys file. -### Connection Profile +#### Private Key File Path A private key can optionally be specified in the `privateKeyFilePath` field in the connection profile for SAS 9.4 (remote - SSH). This is useful for auth setups where the SSH Agent cannot be used. If a private key file contains a passphrase, the user will be prompted to enter it during each Session creation for which it is used. From 3e1f267322b20378f756f30d9132a82f4a10f652 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 19 Sep 2024 08:10:36 -0400 Subject: [PATCH 19/32] refactor: remove unused types --- client/src/connection/ssh/types.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/client/src/connection/ssh/types.ts b/client/src/connection/ssh/types.ts index 28af06a94..092e0a064 100644 --- a/client/src/connection/ssh/types.ts +++ b/client/src/connection/ssh/types.ts @@ -1,6 +1,6 @@ // Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { BaseConfig, LogLineTypeEnum } from ".."; +import { BaseConfig } from ".."; export interface Config extends BaseConfig { host: string; @@ -10,19 +10,6 @@ export interface Config extends BaseConfig { privateKeyFilePath: string; } -export const LogLineTypes: LogLineTypeEnum[] = [ - "normal", - "hilighted", - "source", - "title", - "byline", - "footnote", - "error", - "warning", - "note", - "message", -]; - export enum LineCodes { ResultsFetchedCode = "--vscode-sas-extension-results-fetched--", RunCancelledCode = "--vscode-sas-extension-run-cancelled--", From 76a271f3e526a476e63d776d5cecb539100ab04b Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 19 Sep 2024 08:11:47 -0400 Subject: [PATCH 20/32] refactor: use one LineParser --- client/src/connection/itc/LineParser.ts | 40 ------------------------- client/src/connection/itc/index.ts | 2 +- 2 files changed, 1 insertion(+), 41 deletions(-) delete mode 100644 client/src/connection/itc/LineParser.ts diff --git a/client/src/connection/itc/LineParser.ts b/client/src/connection/itc/LineParser.ts deleted file mode 100644 index 002a44f3a..000000000 --- a/client/src/connection/itc/LineParser.ts +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -export class LineParser { - protected processedLines: string[] = []; - protected capturingLine: boolean = false; - - public constructor( - protected startTag: string, - protected endTag: string, - protected returnNonProcessedLines: boolean, - ) {} - - public processLine(line: string): string | undefined { - if (line.includes(this.startTag) || this.capturingLine) { - this.processedLines.push(line); - this.capturingLine = true; - if (line.includes(this.endTag)) { - return this.processedLine(); - } - return; - } - - return this.returnNonProcessedLines ? line : undefined; - } - - protected processedLine(): string { - this.capturingLine = false; - const fullError = this.processedLines - .join("") - .replace(this.startTag, "") - .replace(this.endTag, ""); - this.processedLines = []; - return fullError; - } - - public isCapturingLine(): boolean { - return this.capturingLine; - } -} diff --git a/client/src/connection/itc/index.ts b/client/src/connection/itc/index.ts index 20d2e9416..61c58bbcb 100644 --- a/client/src/connection/itc/index.ts +++ b/client/src/connection/itc/index.ts @@ -18,9 +18,9 @@ import { getSecretStorage, } from "../../components/ExtensionContext"; import { updateStatusBarItem } from "../../components/StatusBarItem"; +import { LineParser } from "../LineParser"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; -import { LineParser } from "./LineParser"; import { ERROR_END_TAG, ERROR_START_TAG, From 7a2c0b4d2721451508128fc8e05d67a5e15250a0 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 19 Sep 2024 08:41:47 -0400 Subject: [PATCH 21/32] refactor: shorter prompting for private key file path --- client/src/components/profile.ts | 11 ++++++----- client/test/components/profile/profile.test.ts | 5 ++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/client/src/components/profile.ts b/client/src/components/profile.ts index 8b181cc32..f790114f1 100644 --- a/client/src/components/profile.ts +++ b/client/src/components/profile.ts @@ -585,7 +585,10 @@ export class ProfileConfig { ProfilePromptType.PrivateKeyFilePath, profileClone.privateKeyFilePath, ); - if (profileClone.privateKeyFilePath === undefined) { + if ( + profileClone.privateKeyFilePath === undefined || + profileClone.privateKeyFilePath === "" + ) { return; } @@ -806,10 +809,8 @@ const input: ProfilePromptInput = { }, [ProfilePromptType.PrivateKeyFilePath]: { title: l10n.t("Private Key File Path (optional)"), - placeholder: l10n.t("Enter the private key file path"), - description: l10n.t( - "Enter the local path of the private key file. Leave empty to use SSH Agent.", - ), + placeholder: l10n.t("Enter the local private key file path"), + description: l10n.t("Leave empty to use SSH Agent or password."), }, }; diff --git a/client/test/components/profile/profile.test.ts b/client/test/components/profile/profile.test.ts index e278d0a18..d1d63fd51 100644 --- a/client/test/components/profile/profile.test.ts +++ b/client/test/components/profile/profile.test.ts @@ -975,9 +975,8 @@ describe("Profiles", async function () { name: "Private Key File Path", prompt: ProfilePromptType.PrivateKeyFilePath, wantTitle: "Private Key File Path (optional)", - wantDescription: - "Enter the local path of the private key file. Leave empty to use SSH Agent.", - wantPlaceHolder: "Enter the private key file path", + wantDescription: "Leave empty to use SSH Agent or password.", + wantPlaceHolder: "Enter the local private key file path", }, ]; From 430f80bf39a090995922b068aaf31ddb4257d4c5 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Fri, 20 Sep 2024 09:27:23 -0400 Subject: [PATCH 22/32] fix: remove console logging for debug msgs --- client/src/connection/ssh/index.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 5add4a14e..3c257fcac 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -92,10 +92,6 @@ export class SSHSession extends Session { readyTimeout: CONNECT_READY_TIMEOUT, keepaliveInterval: KEEPALIVE_INTERVAL, keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, - - debug: (msg) => { - console.log(msg); - }, authHandler: this.handleSSHAuthentication, }; From 04444a735da125568a1bfba2e2cbd5fdedba0875 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 23 Sep 2024 11:30:33 -0400 Subject: [PATCH 23/32] fix: use a more reasonable unanswered threshhold --- client/src/connection/ssh/const.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index 4c53e1752..791db9ddc 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -3,9 +3,9 @@ const SECOND = 1000; const MINUTE = 60 * SECOND; -const HOUR = 60 * MINUTE; export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to send SSH-level keepalive packets to the server. Set to 0 to disable. -export const KEEPALIVE_UNANSWERED_THRESHOLD = 12 * HOUR; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. +export const KEEPALIVE_UNANSWERED_THRESHOLD = + (15 * MINUTE) / KEEPALIVE_INTERVAL; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. export const WORK_DIR_START_TAG = ""; export const WORK_DIR_END_TAG = ""; export const CONNECT_READY_TIMEOUT = 5 * MINUTE; //allow extra time due to possible prompting From 5f9ec571cab6b1675c81821a773ee47bbdc85589 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 23 Sep 2024 15:00:31 -0400 Subject: [PATCH 24/32] doc: update prompt wording --- client/src/components/profile.ts | 2 +- client/test/components/profile/profile.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/src/components/profile.ts b/client/src/components/profile.ts index f790114f1..a3fbff76f 100644 --- a/client/src/components/profile.ts +++ b/client/src/components/profile.ts @@ -810,7 +810,7 @@ const input: ProfilePromptInput = { [ProfilePromptType.PrivateKeyFilePath]: { title: l10n.t("Private Key File Path (optional)"), placeholder: l10n.t("Enter the local private key file path"), - description: l10n.t("Leave empty to use SSH Agent or password."), + description: l10n.t("To use the SSH Agent or a password, leave blank."), }, }; diff --git a/client/test/components/profile/profile.test.ts b/client/test/components/profile/profile.test.ts index d1d63fd51..60bf40622 100644 --- a/client/test/components/profile/profile.test.ts +++ b/client/test/components/profile/profile.test.ts @@ -975,7 +975,7 @@ describe("Profiles", async function () { name: "Private Key File Path", prompt: ProfilePromptType.PrivateKeyFilePath, wantTitle: "Private Key File Path (optional)", - wantDescription: "Leave empty to use SSH Agent or password.", + wantDescription: "To use the SSH Agent or a password, leave blank.", wantPlaceHolder: "Enter the local private key file path", }, ]; From d3c9cd20a7ed32da89318826b0ede0239c067780 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Thu, 26 Sep 2024 17:14:19 -0400 Subject: [PATCH 25/32] fix: only write private key file key if there is an input value --- client/src/components/profile.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/client/src/components/profile.ts b/client/src/components/profile.ts index a3fbff76f..50438153b 100644 --- a/client/src/components/profile.ts +++ b/client/src/components/profile.ts @@ -581,15 +581,13 @@ export class ProfileConfig { return; } - profileClone.privateKeyFilePath = await createInputTextBox( + const keyPath = await createInputTextBox( ProfilePromptType.PrivateKeyFilePath, profileClone.privateKeyFilePath, ); - if ( - profileClone.privateKeyFilePath === undefined || - profileClone.privateKeyFilePath === "" - ) { - return; + + if (keyPath) { + profileClone.privateKeyFilePath = keyPath; } await this.upsertProfile(name, profileClone); From 6d28a434f55d7535083c89846984f14e7253d591 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 30 Sep 2024 09:55:24 -0400 Subject: [PATCH 26/32] fix: connection close lifecycle bug --- client/src/connection/ssh/index.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 3c257fcac..1f98973be 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -131,9 +131,13 @@ export class SSHSession extends Session { }; private onConnectionClose = () => { - if (!this._sessionReady) { - this._reject?.(new Error(l10n.t("Could not connect to the SAS server."))); - } + this._stream = undefined; + this._resolve = undefined; + this._reject = undefined; + this._html5FileName = ""; + this._workDirectory = undefined; + this.clearAuthState(); + sessionInstance = undefined; }; private onConnectionError = (err: Error) => { @@ -176,14 +180,7 @@ export class SSHSession extends Session { }; private onStreamClose = (): void => { - this._stream = undefined; - this._resolve = undefined; - this._reject = undefined; - this._html5FileName = ""; - this.clearAuthState(); - this._workDirectory = undefined; this._conn.end(); - sessionInstance = undefined; updateStatusBarItem(false); }; From a0b7f88e5137f05462e2c7e7290d69bf8cd4d0aa Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 14 Oct 2024 15:35:21 -0400 Subject: [PATCH 27/32] fix: better error handling --- client/src/connection/ssh/auth.ts | 50 ++++++++++++++-------------- client/src/connection/ssh/index.ts | 53 +++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index 8be028adb..18d0887bb 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -118,13 +118,12 @@ export class AuthHandler { * @param resolve a function that resolves the promise that is waiting for the password * @param username the user name to use for the connection */ - passwordAuth = (cb: NextAuthHandler, username: string) => { - this._authPresenter.presentPasswordPrompt(username).then((pw) => { - cb({ - type: "password", - password: pw, - username: username, - }); + passwordAuth = async (cb: NextAuthHandler, username: string) => { + const pw = await this._authPresenter.presentPasswordPrompt(username); + cb({ + type: "password", + password: pw, + username: username, }); }; @@ -134,7 +133,7 @@ export class AuthHandler { * @param resolve a function that resolves the promise that is waiting for authentication * @param username the user name to use for the connection */ - keyboardInteractiveAuth = (cb: NextAuthHandler, username: string) => { + keyboardInteractiveAuth = async (cb: NextAuthHandler, username: string) => { cb({ type: "keyboard-interactive", username: username, @@ -172,7 +171,7 @@ export class AuthHandler { * @param privateKeyFilePath the path to the private key file defined in the connection profile * @param username the user name to use for the connection */ - privateKeyAuth = ( + privateKeyAuth = async ( cb: NextAuthHandler, privateKeyFilePath: string, username: string, @@ -186,22 +185,23 @@ export class AuthHandler { "Encrypted private OpenSSH key detected, but no passphrase given"; // key is encrypted, prompt for passphrase if (passphraseRequired) { - this._authPresenter.presentPassphrasePrompt().then((passphrase) => { - //parse the keyfile using the passphrase - const passphrasedKeyContentsResult = this._keyParser.parseKey( - privateKeyFilePath, - passphrase, - ); - - if (passphrasedKeyContentsResult instanceof Error) { - throw passphrasedKeyContentsResult; - } - cb({ - type: "publickey", - key: passphrasedKeyContentsResult, - passphrase: passphrase, - username: username, - }); + const passphrase = await this._authPresenter.presentPassphrasePrompt(); + + //parse the keyfile using the passphrase + const passphrasedKeyContentsResult = this._keyParser.parseKey( + privateKeyFilePath, + passphrase, + ); + + if (passphrasedKeyContentsResult instanceof Error) { + throw passphrasedKeyContentsResult; + } + + cb({ + type: "publickey", + key: passphrasedKeyContentsResult, + passphrase: passphrase, + username: username, }); } else { if (hasParseError) { diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 1f98973be..361fb5b55 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -53,6 +53,7 @@ export class SSHSession extends Session { private _authHandler: AuthHandler; private _workDirectory: string; private _workDirectoryParser: LineParser; + private _authsLeft: AuthenticationType[]; constructor(c?: Config, client?: Client) { super(); @@ -65,6 +66,7 @@ export class SSHSession extends Session { WORK_DIR_END_TAG, false, ); + this._authsLeft = []; } public sessionId? = (): string => { @@ -124,6 +126,7 @@ export class SSHSession extends Session { public close = (): void | Promise => { if (!this._stream) { + this.disposeResources(); return; } this._stream.write("endsas;\n"); @@ -131,6 +134,14 @@ export class SSHSession extends Session { }; private onConnectionClose = () => { + if (!this._sessionReady) { + this._reject?.(new Error(l10n.t("Could not connect to the SAS server."))); + } + + this.disposeResources(); + }; + + private disposeResources = () => { this._stream = undefined; this._resolve = undefined; this._reject = undefined; @@ -138,6 +149,7 @@ export class SSHSession extends Session { this._workDirectory = undefined; this.clearAuthState(); sessionInstance = undefined; + this._authsLeft = []; }; private onConnectionError = (err: Error) => { @@ -297,7 +309,7 @@ export class SSHSession extends Session { private handleSSHAuthentication: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], - _partialSuccess: boolean, + partialSuccess: boolean, nextAuth: NextAuthHandler, ) => { if (!authsLeft) { @@ -313,30 +325,47 @@ export class SSHSession extends Session { return false; //returning false will stop the authentication process } - const authMethod = authsLeft.shift(); + if (this._authsLeft.length === 0 || partialSuccess) { + this._authsLeft = authsLeft; + } + + const authMethod = this._authsLeft.shift(); + switch (authMethod) { case "publickey": { //user set a keyfile path in profile config if (this._config.privateKeyFilePath) { - this._authHandler.privateKeyAuth( - nextAuth, - this._config.privateKeyFilePath, - this._config.username, - ); + this._authHandler + .privateKeyAuth( + nextAuth, + this._config.privateKeyFilePath, + this._config.username, + ) + .catch((e) => { + this._reject?.(e); + return false; + }); } else if (process.env.SSH_AUTH_SOCK) { this._authHandler.sshAgentAuth(nextAuth, this._config.username); } break; } case "password": { - this._authHandler.passwordAuth(nextAuth, this._config.username); + this._authHandler + .passwordAuth(nextAuth, this._config.username) + .catch((e) => { + this._reject?.(e); + return false; + }); break; } case "keyboard-interactive": { - this._authHandler.keyboardInteractiveAuth( - nextAuth, - this._config.username, - ); + this._authHandler + .keyboardInteractiveAuth(nextAuth, this._config.username) + .catch((e) => { + this._reject?.(e); + return false; + }); break; } default: From 9e9650276565a852bf3a04f767517d52baab6e0d Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 14 Oct 2024 15:43:02 -0400 Subject: [PATCH 28/32] DCO Remediation Commit for Joe Morris I, Joe Morris , hereby add my Signed-off-by to this commit: 482f2f2290f28da2099e57288652c3d3434d0a7d I, Joe Morris , hereby add my Signed-off-by to this commit: bda97f195a859b61e1b0051ab61fb559a14f5595 I, Joe Morris , hereby add my Signed-off-by to this commit: 86840c7f4431ef9ee8476205b75faa96c654c516 I, Joe Morris , hereby add my Signed-off-by to this commit: 74040255c8f8ccc0d117d0fd3b40e062af6614b1 I, Joe Morris , hereby add my Signed-off-by to this commit: 69b66642cb85a053f455c59d0b7bcdcbc932f067 I, Joe Morris , hereby add my Signed-off-by to this commit: cc667a4eb91abbadb839edd21144fb7e568d41ce I, Joe Morris , hereby add my Signed-off-by to this commit: 38a575a17f3dbe31a961378f7272fd766af34d95 I, Joe Morris , hereby add my Signed-off-by to this commit: f0016cbb98f36c5f85cfeafea3d6f667ef63f580 I, Joe Morris , hereby add my Signed-off-by to this commit: 21284e41d53e3bffcc637cb9c38daadf654f3040 I, Joe Morris , hereby add my Signed-off-by to this commit: 8aa0c23bd4faa8d6714081fcd9bc6fd8bc095e22 I, Joe Morris , hereby add my Signed-off-by to this commit: 91a6d0fea916540249ff27d540467134e92000ba I, Joe Morris , hereby add my Signed-off-by to this commit: 0607d64f12efe11396d5051daad505ac26a340d4 I, Joe Morris , hereby add my Signed-off-by to this commit: 6ac98e8b5dc859cb45dfdec9d21a146e8efc2f35 I, Joe Morris , hereby add my Signed-off-by to this commit: a092524aa485587761df265c0d9cdc7b902007c1 I, Joe Morris , hereby add my Signed-off-by to this commit: debafaf17d2d8e92fe5627837a44c1040fd539cd I, Joe Morris , hereby add my Signed-off-by to this commit: e0e26dd89b8e82ad1da8727f1bacb4f35883e0ad I, Joe Morris , hereby add my Signed-off-by to this commit: 51ce78aaa8aebd393711b128334b335ff9033101 I, Joe Morris , hereby add my Signed-off-by to this commit: ea17b7554c9090c05c893abe452a74becde45891 I, Joe Morris , hereby add my Signed-off-by to this commit: 3e1f267322b20378f756f30d9132a82f4a10f652 I, Joe Morris , hereby add my Signed-off-by to this commit: 76a271f3e526a476e63d776d5cecb539100ab04b I, Joe Morris , hereby add my Signed-off-by to this commit: 7a2c0b4d2721451508128fc8e05d67a5e15250a0 I, Joe Morris , hereby add my Signed-off-by to this commit: 430f80bf39a090995922b068aaf31ddb4257d4c5 I, Joe Morris , hereby add my Signed-off-by to this commit: 04444a735da125568a1bfba2e2cbd5fdedba0875 I, Joe Morris , hereby add my Signed-off-by to this commit: 5f9ec571cab6b1675c81821a773ee47bbdc85589 I, Joe Morris , hereby add my Signed-off-by to this commit: d3c9cd20a7ed32da89318826b0ede0239c067780 I, Joe Morris , hereby add my Signed-off-by to this commit: 6d28a434f55d7535083c89846984f14e7253d591 I, Joe Morris , hereby add my Signed-off-by to this commit: a0b7f88e5137f05462e2c7e7290d69bf8cd4d0aa Signed-off-by: Joe Morris --- client/src/connection/ssh/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 361fb5b55..4a335cae4 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -309,7 +309,7 @@ export class SSHSession extends Session { private handleSSHAuthentication: AuthHandlerMiddleware = ( authsLeft: AuthenticationType[], - partialSuccess: boolean, + partialSuccess: boolean, //used in scenarios which require multiple auth methods to denote partial success nextAuth: NextAuthHandler, ) => { if (!authsLeft) { From 6e464402422436e98f8ead38581cfa991f6cbd72 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Tue, 15 Oct 2024 20:58:47 -0400 Subject: [PATCH 29/32] fix: further error handling enhancements --- client/src/connection/ssh/auth.ts | 59 ++++++----- client/src/connection/ssh/index.ts | 132 +++++++++++++----------- client/test/connection/ssh/auth.test.ts | 35 ++++--- 3 files changed, 124 insertions(+), 102 deletions(-) diff --git a/client/src/connection/ssh/auth.ts b/client/src/connection/ssh/auth.ts index 18d0887bb..0aa9b6498 100644 --- a/client/src/connection/ssh/auth.ts +++ b/client/src/connection/ssh/auth.ts @@ -3,7 +3,15 @@ import { l10n, window } from "vscode"; import { readFileSync } from "fs"; -import { NextAuthHandler, ParsedKey, Prompt, utils } from "ssh2"; +import { + AgentAuthMethod, + KeyboardInteractiveAuthMethod, + ParsedKey, + PasswordAuthMethod, + Prompt, + PublicKeyAuthMethod, + utils, +} from "ssh2"; /** * Abstraction for presenting authentication prompts to the user. @@ -118,13 +126,13 @@ export class AuthHandler { * @param resolve a function that resolves the promise that is waiting for the password * @param username the user name to use for the connection */ - passwordAuth = async (cb: NextAuthHandler, username: string) => { + passwordAuth = async (username: string): Promise => { const pw = await this._authPresenter.presentPasswordPrompt(username); - cb({ + return { type: "password", password: pw, username: username, - }); + }; }; /** @@ -133,8 +141,10 @@ export class AuthHandler { * @param resolve a function that resolves the promise that is waiting for authentication * @param username the user name to use for the connection */ - keyboardInteractiveAuth = async (cb: NextAuthHandler, username: string) => { - cb({ + keyboardInteractiveAuth = async ( + username: string, + ): Promise => { + return { type: "keyboard-interactive", username: username, prompt: (_name, _instructions, _instructionsLang, prompts, finish) => { @@ -146,7 +156,7 @@ export class AuthHandler { finish(answers); }); }, - }); + }; }; /** @@ -154,12 +164,12 @@ export class AuthHandler { * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server. * @param username the user name to use for the connection */ - sshAgentAuth = (cb: NextAuthHandler, username: string) => { - cb({ + sshAgentAuth = (username: string): AgentAuthMethod => { + return { type: "agent", agent: process.env.SSH_AUTH_SOCK, username: username, - }); + }; }; /** @@ -172,10 +182,9 @@ export class AuthHandler { * @param username the user name to use for the connection */ privateKeyAuth = async ( - cb: NextAuthHandler, privateKeyFilePath: string, username: string, - ) => { + ): Promise => { // first, try to parse the key file without a passphrase const parsedKeyResult = this._keyParser.parseKey(privateKeyFilePath); const hasParseError = parsedKeyResult instanceof Error; @@ -186,7 +195,6 @@ export class AuthHandler { // key is encrypted, prompt for passphrase if (passphraseRequired) { const passphrase = await this._authPresenter.presentPassphrasePrompt(); - //parse the keyfile using the passphrase const passphrasedKeyContentsResult = this._keyParser.parseKey( privateKeyFilePath, @@ -195,23 +203,24 @@ export class AuthHandler { if (passphrasedKeyContentsResult instanceof Error) { throw passphrasedKeyContentsResult; + } else { + return { + type: "publickey", + key: passphrasedKeyContentsResult, + passphrase: passphrase, + username: username, + }; } - - cb({ - type: "publickey", - key: passphrasedKeyContentsResult, - passphrase: passphrase, - username: username, - }); } else { if (hasParseError) { throw parsedKeyResult; + } else { + return { + type: "publickey", + key: parsedKeyResult, + username: username, + }; } - cb({ - type: "publickey", - key: parsedKeyResult, - username: username, - }); } }; } diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 4a335cae4..585aa1e51 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -3,12 +3,16 @@ import { l10n } from "vscode"; import { + AgentAuthMethod, AuthHandlerMiddleware, AuthenticationType, Client, ClientChannel, ConnectConfig, + KeyboardInteractiveAuthMethod, NextAuthHandler, + PasswordAuthMethod, + PublicKeyAuthMethod, } from "ssh2"; import { BaseConfig, RunResult } from ".."; @@ -87,6 +91,7 @@ export class SSHSession extends Session { return; } + const authHandlerFn = this.handleSSHAuthentication(); const cfg: ConnectConfig = { host: this._config.host, port: this._config.port, @@ -94,7 +99,9 @@ export class SSHSession extends Session { readyTimeout: CONNECT_READY_TIMEOUT, keepaliveInterval: KEEPALIVE_INTERVAL, keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD, - authHandler: this.handleSSHAuthentication, + authHandler: (methodsLeft, partialSuccess, callback) => ( + authHandlerFn(methodsLeft, partialSuccess, callback), undefined + ), }; if (!this._conn) { @@ -305,71 +312,74 @@ export class SSHSession extends Session { */ private clearAuthState = (): void => { this._sessionReady = false; + this._authsLeft = []; }; - private handleSSHAuthentication: AuthHandlerMiddleware = ( - authsLeft: AuthenticationType[], - partialSuccess: boolean, //used in scenarios which require multiple auth methods to denote partial success - nextAuth: NextAuthHandler, - ) => { - if (!authsLeft) { - nextAuth("none"); //sending none will prompt the server to send supported auth methods - return; - } - - if (authsLeft.length === 0) { - this._reject?.( - new Error(l10n.t("Could not authenticate to the SSH server.")), - ); - this.clearAuthState(); - return false; //returning false will stop the authentication process - } + private handleSSHAuthentication = (): AuthHandlerMiddleware => { + //The ssh2 library supports sending false to stop the authentication process + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + const END_AUTH = false as unknown as AuthenticationType; + return async ( + authsLeft: AuthenticationType[], + partialSuccess: boolean, //used in scenarios which require multiple auth methods to denote partial success + nextAuth: NextAuthHandler, + ) => { + if (!authsLeft) { + return nextAuth({ type: "none", username: this._config.username }); //sending none will prompt the server to send supported auth methods + } else { + if (authsLeft.length === 0) { + return nextAuth(END_AUTH); + } - if (this._authsLeft.length === 0 || partialSuccess) { - this._authsLeft = authsLeft; - } + if (this._authsLeft.length === 0 || partialSuccess) { + this._authsLeft = authsLeft; + } - const authMethod = this._authsLeft.shift(); - - switch (authMethod) { - case "publickey": { - //user set a keyfile path in profile config - if (this._config.privateKeyFilePath) { - this._authHandler - .privateKeyAuth( - nextAuth, - this._config.privateKeyFilePath, - this._config.username, - ) - .catch((e) => { - this._reject?.(e); - return false; - }); - } else if (process.env.SSH_AUTH_SOCK) { - this._authHandler.sshAgentAuth(nextAuth, this._config.username); + const authMethod = this._authsLeft.shift(); + + try { + let authPayload: + | PublicKeyAuthMethod + | AgentAuthMethod + | PasswordAuthMethod + | KeyboardInteractiveAuthMethod; + + switch (authMethod) { + case "publickey": { + //user set a keyfile path in profile config + if (this._config.privateKeyFilePath) { + authPayload = await this._authHandler.privateKeyAuth( + this._config.privateKeyFilePath, + this._config.username, + ); + } else if (process.env.SSH_AUTH_SOCK) { + authPayload = this._authHandler.sshAgentAuth( + this._config.username, + ); + } + break; + } + case "password": { + authPayload = await this._authHandler.passwordAuth( + this._config.username, + ); + break; + } + case "keyboard-interactive": { + authPayload = await this._authHandler.keyboardInteractiveAuth( + this._config.username, + ); + break; + } + default: + nextAuth(authMethod); + } + return nextAuth(authPayload); + } catch (e) { + this._reject?.(e); + return nextAuth(END_AUTH); } - break; } - case "password": { - this._authHandler - .passwordAuth(nextAuth, this._config.username) - .catch((e) => { - this._reject?.(e); - return false; - }); - break; - } - case "keyboard-interactive": { - this._authHandler - .keyboardInteractiveAuth(nextAuth, this._config.username) - .catch((e) => { - this._reject?.(e); - return false; - }); - break; - } - default: - nextAuth(authMethod); - } + }; }; } diff --git a/client/test/connection/ssh/auth.test.ts b/client/test/connection/ssh/auth.test.ts index f29637c83..986ef2500 100644 --- a/client/test/connection/ssh/auth.test.ts +++ b/client/test/connection/ssh/auth.test.ts @@ -18,16 +18,15 @@ describe("ssh connection auth handler", () => { }); it("pass socket path to the callback", async () => { - const cb = sinon.stub(); const username = "username"; const socketPath = "socketPath"; const presenter = stubInterface(); authHandler = new AuthHandler(presenter); - await authHandler.sshAgentAuth(cb, username); + const agentPayload = authHandler.sshAgentAuth(username); - sinon.assert.calledWith(cb, { + expect(agentPayload).to.deep.equal({ type: "agent", agent: socketPath, username: username, @@ -46,7 +45,6 @@ describe("ssh connection auth handler", () => { }); it("should pass the key contents and passphrase to the callback for an encrypted key", async () => { - const cb = sinon.stub(); const username = "username"; const passphrase = "passphrase"; const privateKeyFilePath = "privateKeyFilePath"; @@ -67,9 +65,12 @@ describe("ssh connection auth handler", () => { authHandler = new AuthHandler(presenter, keyParser); - await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); + const pkPayload = await authHandler.privateKeyAuth( + privateKeyFilePath, + username, + ); - sinon.assert.calledWith(cb, { + expect(pkPayload).to.deep.equal({ type: "publickey", key: key, passphrase: passphrase, @@ -78,7 +79,6 @@ describe("ssh connection auth handler", () => { }); it("should pass the key contents to the callback for an unencrypted key", async () => { - const cb = sandbox.stub(); const username = "username"; const privateKeyFilePath = "privateKeyFilePath"; @@ -93,9 +93,11 @@ describe("ssh connection auth handler", () => { keyParser.parseKey.returns(key); authHandler = new AuthHandler(presenter, keyParser); - await authHandler.privateKeyAuth(cb, privateKeyFilePath, username); - - sinon.assert.calledWith(cb, { + const pkPayload = await authHandler.privateKeyAuth( + privateKeyFilePath, + username, + ); + expect(pkPayload).to.deep.equal({ type: "publickey", key: key, username: username, @@ -105,7 +107,6 @@ describe("ssh connection auth handler", () => { describe("passwordAuth", () => { it("should pass the password to the callback", async () => { - const cb = sinon.stub(); const username = "username"; const pw = "password"; @@ -114,9 +115,9 @@ describe("ssh connection auth handler", () => { authHandler = new AuthHandler(presenter); - await authHandler.passwordAuth(cb, username); + const pwPayload = await authHandler.passwordAuth(username); - sinon.assert.calledWith(cb, { + expect(pwPayload).to.deep.equal({ type: "password", password: pw, username: username, @@ -126,7 +127,9 @@ describe("ssh connection auth handler", () => { describe("keyboardAuth", () => { it("should present input prompts and pass the answers to the callback", async () => { - const promptCbStub = sinon.stub(); + const promptCbStub = (answers: string[]) => { + expect(answers).to.deep.equal(["answer1", "answer2"]); + }; const cb = (auth: KeyboardInteractiveAuthMethod) => { expect(auth.type).to.equal("keyboard-interactive"); expect(auth.username).to.equal("username"); @@ -145,8 +148,8 @@ describe("ssh connection auth handler", () => { authHandler = new AuthHandler(presenter); - await authHandler.keyboardInteractiveAuth(cb, "username"); - sinon.assert.calledWith(promptCbStub, answers); + const kbPayload = await authHandler.keyboardInteractiveAuth("username"); + cb(kbPayload); }); }); }); From 2195eb967441272ae57c83a553f52dcf15a77c50 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Fri, 18 Oct 2024 08:31:37 -0400 Subject: [PATCH 30/32] fix: work dir on single line --- client/src/connection/ssh/index.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 585aa1e51..d35ae3a38 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -204,17 +204,7 @@ export class SSHSession extends Session { }; private fetchWorkDirectory = (line: string): string => { - let foundWorkDirectory = ""; - if ( - !line.includes(`%put ${WORK_DIR_START_TAG};`) && - !line.includes(`%put &workDir;`) && - !line.includes(`%put ${WORK_DIR_END_TAG};`) - ) { - foundWorkDirectory = this._workDirectoryParser.processLine(line); - } else { - // If the line is the put statement, we don't need to log that - return; - } + const foundWorkDirectory = this._workDirectoryParser.processLine(line); // We don't want to output any of the captured lines if (this._workDirectoryParser.isCapturingLine()) { return; @@ -225,9 +215,7 @@ export class SSHSession extends Session { private resolveSystemVars = (): void => { const code = `%let workDir = %sysfunc(pathname(work)); - %put ${WORK_DIR_START_TAG}; - %put &workDir; - %put ${WORK_DIR_END_TAG}; + %put ${WORK_DIR_START_TAG}&workDir${WORK_DIR_END_TAG}; %let rc = %sysfunc(dlgcdir("&workDir")); run; `; From cde43f065bd1ddeda4281fee5577014d5439222e Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 21 Oct 2024 11:19:23 -0400 Subject: [PATCH 31/32] fix:parse structure log output for work dir --- client/src/connection/{ => itc}/LineParser.ts | 0 client/src/connection/itc/index.ts | 2 +- client/src/connection/ssh/const.ts | 4 +- client/src/connection/ssh/index.ts | 75 ++++++++----------- 4 files changed, 36 insertions(+), 45 deletions(-) rename client/src/connection/{ => itc}/LineParser.ts (100%) diff --git a/client/src/connection/LineParser.ts b/client/src/connection/itc/LineParser.ts similarity index 100% rename from client/src/connection/LineParser.ts rename to client/src/connection/itc/LineParser.ts diff --git a/client/src/connection/itc/index.ts b/client/src/connection/itc/index.ts index 61c58bbcb..20d2e9416 100644 --- a/client/src/connection/itc/index.ts +++ b/client/src/connection/itc/index.ts @@ -18,9 +18,9 @@ import { getSecretStorage, } from "../../components/ExtensionContext"; import { updateStatusBarItem } from "../../components/StatusBarItem"; -import { LineParser } from "../LineParser"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; +import { LineParser } from "./LineParser"; import { ERROR_END_TAG, ERROR_START_TAG, diff --git a/client/src/connection/ssh/const.ts b/client/src/connection/ssh/const.ts index 791db9ddc..99d6ef584 100644 --- a/client/src/connection/ssh/const.ts +++ b/client/src/connection/ssh/const.ts @@ -6,6 +6,6 @@ const MINUTE = 60 * SECOND; export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to send SSH-level keepalive packets to the server. Set to 0 to disable. export const KEEPALIVE_UNANSWERED_THRESHOLD = (15 * MINUTE) / KEEPALIVE_INTERVAL; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection. -export const WORK_DIR_START_TAG = ""; -export const WORK_DIR_END_TAG = ""; +export const WORK_DIR_START_TAG = "WORKDIR"; +export const WORK_DIR_END_TAG = "WORKDIREND"; export const CONNECT_READY_TIMEOUT = 5 * MINUTE; //allow extra time due to possible prompting diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index d35ae3a38..358a57bff 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -17,7 +17,6 @@ import { import { BaseConfig, RunResult } from ".."; import { updateStatusBarItem } from "../../components/StatusBarItem"; -import { LineParser } from "../LineParser"; import { Session } from "../session"; import { extractOutputHtmlFileName } from "../util"; import { AuthHandler } from "./auth"; @@ -56,7 +55,6 @@ export class SSHSession extends Session { private _sessionReady: boolean; private _authHandler: AuthHandler; private _workDirectory: string; - private _workDirectoryParser: LineParser; private _authsLeft: AuthenticationType[]; constructor(c?: Config, client?: Client) { @@ -65,11 +63,6 @@ export class SSHSession extends Session { this._conn = client; this._sessionReady = false; this._authHandler = new AuthHandler(); - this._workDirectoryParser = new LineParser( - WORK_DIR_START_TAG, - WORK_DIR_END_TAG, - false, - ); this._authsLeft = []; } @@ -181,19 +174,23 @@ export class SSHSession extends Session { s.on("data", (data) => { fileContents += data.toString(); - }).on("close", (code) => { - const rc: number = code; - - if (rc === 0) { - //Make sure that the html has a valid body - //TODO #185: should this be refactored into a shared location? - if (fileContents.search('<*id="IDX*.+">') !== -1) { - runResult.html5 = fileContents; - runResult.title = l10n.t("Result"); + }) + .on("close", (code) => { + const rc: number = code; + + if (rc === 0) { + //Make sure that the html has a valid body + //TODO #185: should this be refactored into a shared location? + if (fileContents.search('<*id="IDX*.+">') !== -1) { + runResult.html5 = fileContents; + runResult.title = l10n.t("Result"); + } } - } - this._resolve?.(runResult); - }); + this._resolve?.(runResult); + }) + .on("error", (err) => { + console.log(err); + }); }, ); }; @@ -203,22 +200,15 @@ export class SSHSession extends Session { updateStatusBarItem(false); }; - private fetchWorkDirectory = (line: string): string => { - const foundWorkDirectory = this._workDirectoryParser.processLine(line); - // We don't want to output any of the captured lines - if (this._workDirectoryParser.isCapturingLine()) { - return; - } - - return foundWorkDirectory || ""; - }; - private resolveSystemVars = (): void => { - const code = `%let workDir = %sysfunc(pathname(work)); - %put ${WORK_DIR_START_TAG}&workDir${WORK_DIR_END_TAG}; - %let rc = %sysfunc(dlgcdir("&workDir")); - run; - `; + const code = `%let wd = %sysfunc(pathname(work)); + %let rc = %sysfunc(dlgcdir("&wd")); + data _null_; length x $ 4096; + file STDERR; + x = resolve('&wd'); put '${WORK_DIR_START_TAG}' x '${WORK_DIR_END_TAG}'; + run; + + `; this._stream.write(code); }; @@ -233,6 +223,15 @@ export class SSHSession extends Session { return; } + if (this._sessionReady && !this._workDirectory) { + const match = output.match( + `${WORK_DIR_START_TAG}(/[\\s\\S]*?)${WORK_DIR_END_TAG}`, + ); + if (match && match.length > 1) { + this._workDirectory = match[1].trimEnd().replace(/(\r\n|\n|\r)/gm, ""); + } + } + const outputLines = output.split(/\n|\r\n/); outputLines.forEach((line) => { if (!line) { @@ -250,14 +249,6 @@ export class SSHSession extends Session { ); this._onExecutionLogFn?.([{ type: "normal", line }]); } - - if (this._sessionReady && !this._workDirectory) { - const foundWorkDir = this.fetchWorkDirectory(line); - if (foundWorkDir) { - const match = foundWorkDir.match(/\/[^\s\r]+/); - this._workDirectory = match ? match[0] : ""; - } - } }); }; From 6cd548a0176bdc8feda4f52acbe3819e7a134533 Mon Sep 17 00:00:00 2001 From: Joe Morris Date: Mon, 21 Oct 2024 11:24:31 -0400 Subject: [PATCH 32/32] chore: remove logging --- client/src/connection/ssh/index.ts | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/client/src/connection/ssh/index.ts b/client/src/connection/ssh/index.ts index 358a57bff..428477cfd 100644 --- a/client/src/connection/ssh/index.ts +++ b/client/src/connection/ssh/index.ts @@ -174,23 +174,19 @@ export class SSHSession extends Session { s.on("data", (data) => { fileContents += data.toString(); - }) - .on("close", (code) => { - const rc: number = code; - - if (rc === 0) { - //Make sure that the html has a valid body - //TODO #185: should this be refactored into a shared location? - if (fileContents.search('<*id="IDX*.+">') !== -1) { - runResult.html5 = fileContents; - runResult.title = l10n.t("Result"); - } + }).on("close", (code) => { + const rc: number = code; + + if (rc === 0) { + //Make sure that the html has a valid body + //TODO #185: should this be refactored into a shared location? + if (fileContents.search('<*id="IDX*.+">') !== -1) { + runResult.html5 = fileContents; + runResult.title = l10n.t("Result"); } - this._resolve?.(runResult); - }) - .on("error", (err) => { - console.log(err); - }); + } + this._resolve?.(runResult); + }); }, ); };