From fe728103d0429ebfbcfb4ab2daebbd5f06322244 Mon Sep 17 00:00:00 2001 From: fabgo Date: Thu, 3 Oct 2024 09:18:39 -0700 Subject: [PATCH] ENG-2660: Print selected SSH output to stderr (#120) * 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. --- package.json | 2 +- src/plugins/aws/ssh.ts | 2 +- src/plugins/google/ssh.ts | 2 +- src/plugins/ssh/index.ts | 41 ++++++++++++++++++++++++++++++++++----- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 91beb4c..9099338 100644 --- a/package.json +++ b/package.json @@ -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": { diff --git a/src/plugins/aws/ssh.ts b/src/plugins/aws/ssh.ts index 669b80c..056720d 100644 --- a/src/plugins/aws/ssh.ts +++ b/src/plugins/aws/ssh.ts @@ -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"; diff --git a/src/plugins/google/ssh.ts b/src/plugins/google/ssh.ts index b7280d9..889fe67 100644 --- a/src/plugins/google/ssh.ts +++ b/src/plugins/google/ssh.ts @@ -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. diff --git a/src/plugins/ssh/index.ts b/src/plugins/ssh/index.ts index d84ddbd..4155abb 100644 --- a/src/plugins/ssh/index.ts +++ b/src/plugins/ssh/index.ts @@ -49,7 +49,7 @@ const RETRY_DELAY_MS = 5000; const accessPropagationGuard = ( provider: SshProvider, child: ChildProcessByStdio, - debug?: boolean + options: SpawnSshNodeOptions ) => { let isEphemeralAccessDeniedException = false; let isLoginException = false; @@ -57,8 +57,7 @@ const accessPropagationGuard = ( 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) @@ -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, @@ -143,7 +172,7 @@ async function spawnSshNode( const { isAccessPropagated, isLoginException } = accessPropagationGuard( provider, child, - options.debug + options ); const exitListener = child.on("exit", (code) => { @@ -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"); } }; @@ -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) {