Skip to content

Commit

Permalink
ENG-2660: Print selected SSH output to stderr (#120)
Browse files Browse the repository at this point in the history
* If debug is not enabled, it will now still print selected SSH output
to stderr:
  - "Authenticated to" ... message
  - "port forwarding failed" messages
  - "Sorry, user {user} may not run sudo..." messages (on pre-test only)
* Print an error message when SSH pre-test fails.

Examples:

./p0 ssh dev-instance-fabian-joya --provider gcloud -- -NR
'*:8080:localhost:8088' -o 'GatewayPorts yes'
Authenticated to dev-instance-fabian-joya (via proxy) using "publickey".
Warning: remote port forwarding failed for listen port 8080

./p0 ssh dev-instance-fabian-joya --sudo        
Failed to establish an SSH session. Use the --debug option to see
additional details.
  • Loading branch information
fabgo authored Oct 3, 2024
1 parent f3f18bf commit fe72810
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 8 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@p0security/cli",
"version": "0.11.1",
"version": "0.11.2",
"description": "Execute infra CLI commands with P0 grants",
"main": "index.ts",
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/aws/ssh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
*
* Each attempt consumes ~ 1 s.
*/
const MAX_SSH_RETRIES = 30;
const MAX_SSH_RETRIES = 6;

/** The name of the SessionManager port forwarding document. This document is managed by AWS. */
const START_SSH_SESSION_DOCUMENT_NAME = "AWS-StartSSHSession";
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/google/ssh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { GcpSshPermissionSpec, GcpSshRequest } from "./types";
*
* The length of each attempt varies based on the type of error from a few seconds to < 1s
*/
const MAX_SSH_RETRIES = 120;
const MAX_SSH_RETRIES = 24;

/**
* There are 7 cases of unprovisioned access in Google Cloud.
Expand Down
41 changes: 36 additions & 5 deletions src/plugins/ssh/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,15 @@ const RETRY_DELAY_MS = 5000;
const accessPropagationGuard = (
provider: SshProvider,
child: ChildProcessByStdio<null, null, Readable>,
debug?: boolean
options: SpawnSshNodeOptions
) => {
let isEphemeralAccessDeniedException = false;
let isLoginException = false;
const beforeStart = Date.now();

child.stderr.on("data", (chunk) => {
const chunkString: string = chunk.toString("utf-8");

if (debug) print2(chunkString);
parseAndPrintSshOutputToStderr(chunkString, options);

const match = provider.unprovisionedAccessPatterns.find((message) =>
chunkString.match(message.pattern)
Expand Down Expand Up @@ -88,6 +87,36 @@ const accessPropagationGuard = (
};
};

/**
* Parses and prints a chunk of SSH output to stderr.
*
* If debug is enabled, all output is printed. Otherwise, only selected messages are printed.
*
* @param chunkString the chunk to print
* @param options SSH spawn options
*/
const parseAndPrintSshOutputToStderr = (
chunkString: string,
options: SpawnSshNodeOptions
) => {
const lines = chunkString.split("\n");
const isPreTest = options.isAccessPropagationPreTest;

for (const line of lines) {
if (options.debug) {
print2(line);
} else {
if (!isPreTest && line.includes("Authenticated to")) {
// We want to let the user know that they successfully authenticated
print2(line);
} else if (!isPreTest && line.includes("port forwarding failed")) {
// We also want to let the user know if port forwarding failed
print2(line);
}
}
}
};

const spawnChildProcess = (
credential: AwsCredentials | undefined,
command: string,
Expand Down Expand Up @@ -143,7 +172,7 @@ async function spawnSshNode(
const { isAccessPropagated, isLoginException } = accessPropagationGuard(
provider,
child,
options.debug
options
);

const exitListener = child.on("exit", (code) => {
Expand Down Expand Up @@ -265,8 +294,9 @@ const addCommonArgs = (args: CommandArgs, proxyCommand: string[]) => {
sshOptions.push("-o", `ProxyCommand=${proxyCommand.join(" ")}`);
}

// Force verbose output from SSH so we can parse the output
const verboseOptionExists = sshOptions.some((opt) => opt === "-v");
if (!verboseOptionExists && args.debug) {
if (!verboseOptionExists) {
sshOptions.push("-v");
}
};
Expand Down Expand Up @@ -325,6 +355,7 @@ const preTestAccessPropagationIfNeeded = async <
: undefined
) => {
const testCmdArgs = sshProvider.preTestAccessPropagationArgs(cmdArgs);

// Pre-testing comes at a performance cost because we have to execute another ssh subprocess after
// a successful test. Only do when absolutely necessary.
if (testCmdArgs) {
Expand Down

0 comments on commit fe72810

Please sign in to comment.