Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: username and password support for SSH connection type #1126

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
482f2f2
feat: auth handler skeleton
smorrisj Aug 1, 2024
bda97f1
fix: type on authsLeft
smorrisj Aug 1, 2024
86840c7
chore: progress on auth handler
smorrisj Aug 1, 2024
7404025
fix: brain working faster than fingers
smorrisj Aug 1, 2024
54d8d66
Merge branch 'main' into feat/ssh-userpass
smorrisj Aug 28, 2024
36a778c
Merge branch 'main' into feat/ssh-userpass
smorrisj Sep 4, 2024
69b6664
feat: progress on userpass
smorrisj Sep 5, 2024
cc667a4
refactor: auth state cleanup
smorrisj Sep 5, 2024
38a575a
refactor: move auth details to separate module
smorrisj Sep 5, 2024
f0016cb
feat: keepalive and max unanswered settings
smorrisj Sep 5, 2024
21284e4
use const for keepalive
smorrisj Sep 5, 2024
8aa0c23
refactor: remove redundant timeout test
smorrisj Sep 5, 2024
91a6d0f
chore: copyright header
smorrisj Sep 5, 2024
0607d64
refactor for testability, auth tests
smorrisj Sep 10, 2024
6ac98e8
feat: set working path to work dir
smorrisj Sep 12, 2024
a092524
metadata for private key, various fixes found during testing
smorrisj Sep 13, 2024
978fdb4
Merge branch 'main' into feat/ssh-userpass
smorrisj Sep 13, 2024
debafaf
fix: minor cleanup
smorrisj Sep 17, 2024
e0e26dd
fix: bug fixes found during mfa testing
smorrisj Sep 18, 2024
51ce78a
doc: ssh auth refactor
smorrisj Sep 18, 2024
9879746
Merge branch 'main' into feat/ssh-userpass
smorrisj Sep 18, 2024
ea17b75
rename Connection Profile to Private Key File Path
smorrisj Sep 19, 2024
3e1f267
refactor: remove unused types
smorrisj Sep 19, 2024
76a271f
refactor: use one LineParser
smorrisj Sep 19, 2024
7a2c0b4
refactor: shorter prompting for private key file path
smorrisj Sep 19, 2024
430f80b
fix: remove console logging for debug msgs
smorrisj Sep 20, 2024
04444a7
fix: use a more reasonable unanswered threshhold
smorrisj Sep 23, 2024
5f9ec57
doc: update prompt wording
smorrisj Sep 23, 2024
d3c9cd2
fix: only write private key file key if there is an input value
smorrisj Sep 26, 2024
6d28a43
fix: connection close lifecycle bug
smorrisj Sep 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions client/src/components/profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface SSHProfile extends BaseProfile {
saspath: string;
port: number;
username: string;
privateKeyFilePath?: string;
}

export interface COMProfile extends BaseProfile {
Expand Down Expand Up @@ -580,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 = [];
Expand Down Expand Up @@ -657,6 +666,7 @@ export enum ProfilePromptType {
SASPath,
Port,
Username,
PrivateKeyFilePath,
}

/**
Expand Down Expand Up @@ -794,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.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we want to say Leave empty to use SSH Agent or password.?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was already unsure about having such a long description. I think we could shorten it a bit. See 7a2c0b4. Will that work?

),
},
};

/**
Expand Down
40 changes: 40 additions & 0 deletions client/src/connection/LineParser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

export class LineParser {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to remove the itc/LineParser.ts in favor of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching. I had originally promoted LineParser up higher. A merge must have errantly added it back. Fixed in 76a271f.

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;
}
}
254 changes: 254 additions & 0 deletions client/src/connection/ssh/auth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,254 @@
// Copyright © 2022-2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { l10n, window } from "vscode";

import { readFileSync } from "fs";
import { NextAuthHandler, ParsedKey, Prompt, utils } from "ssh2";

/**
* Abstraction for presenting authentication prompts to the user.
*/
export interface AuthPresenter {
/**
* Prompt the user for a passphrase.
* @returns the passphrase entered by the user
*/
presentPasswordPrompt: (username: string) => Promise<string>;
/**
* Prompt the user for a password.
* @returns the password entered by the user
*/
presentPassphrasePrompt: () => Promise<string>;
/**
* 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: (
username: string,
prompts: Prompt[],
) => Promise<string[]>;
}

class AuthPresenterImpl implements AuthPresenter {
presentPasswordPrompt = async (username: string): Promise<string> => {
return this.presentPrompt(
l10n.t("Enter the password for user: {username}", { username }),
l10n.t("Password Required"),
true,
);
};

presentPassphrasePrompt = async (): Promise<string> => {
return this.presentPrompt(
l10n.t("Enter the passphrase for the private key"),
l10n.t("Passphrase Required"),
true,
);
};

presentMultiplePrompts = async (
username: string,
prompts: Prompt[],
): Promise<string[]> => {
const answers: string[] = [];
for (const prompt of prompts) {
const answer = await this.presentPrompt(
undefined,
l10n.t("User {username} {prompt}", {
username,
prompt: prompt.prompt,
}),
!prompt.echo,
);
if (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
* @param isSecureInput whether the input should be hidden
* @returns the user's response to the prompt
*/
private presentPrompt = async (
prompt: string,
title?: string,
isSecureInput?: boolean,
): Promise<string> => {
return window.showInputBox({
ignoreFocusOut: true,
prompt: prompt,
title: title,
password: isSecureInput,
});
};
}

/**
* Handles the authentication process for the ssh connection.
*
*/
export class AuthHandler {
private _authPresenter: AuthPresenter;
private _keyParser: KeyParser;

constructor(authPresenter?: AuthPresenter, keyParser?: KeyParser) {
this._authPresenter = authPresenter;
this._keyParser = keyParser;

if (!authPresenter) {
this._authPresenter = new AuthPresenterImpl();
}
if (!keyParser) {
this._keyParser = new KeyParserImpl();
}
}

/**
* 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(username).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, 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(username, prompts)
.then((answers) => {
finish(answers);
});
},
});
};

/**
* 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
*/
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 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,
});
});
} else {
if (hasParseError) {
throw parsedKeyResult;
}
cb({
type: "publickey",
key: parsedKeyResult,
username: username,
});
}
};
}

/**
* Parses a private key file.
*/
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;
}

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,
}),
);
}
};

public parseKey = (
privateKeyPath: string,
passphrase?: string,
): ParsedKey | Error => {
const keyContents = this.readKeyFile(privateKeyPath);
return utils.parseKey(keyContents, passphrase);
};
}
11 changes: 11 additions & 0 deletions client/src/connection/ssh/const.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright © 2024, SAS Institute Inc., Cary, NC, USA. All Rights Reserved.
// 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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chunky any thoughts on going with these values for keepalive and max unanswered?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smorrisj 60 seconds is a reasonable figure - I think I usually set it to 30, but I have no strong feelings as long as it's well inside 90.
The unanswered_threshold, I confess to be unsure about appropriate behaviour. If the comment is right, it's measured in "pings" not in "time". So if you're aiming for 12 hours, it should be (12 * HOUR / KEEPALIVE_INTERVAL). Either way, because this is happening at the SSH/networking level, then 12 hours is probably way too long. Maybe 15 minutes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chunky. Must have forgot my morning coffee when making the first pass on that value =) See 04444a7.

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 = "<WorkDirectory>";
export const WORK_DIR_END_TAG = "</WorkDirectory>";
export const CONNECT_READY_TIMEOUT = 5 * MINUTE; //allow extra time due to possible prompting
Loading