refactor: Separate CSP-specific functionality from generic SSH code (#…

…122) For maintainability, anything CSP-specific for SSH should be contained within the CSP's SSH provider object. I also sorted the provider properties.
p0-security · Oct 1, 2024 · 3eff3a9 · 3eff3a9
1 parent 91440e3
commit 3eff3a9
Show file tree

Hide file tree

Showing 4 changed files with 203 additions and 144 deletions.
diff --git a/src/plugins/aws/ssh.ts b/src/plugins/aws/ssh.ts
@@ -31,34 +31,38 @@ const MAX_SSH_RETRIES = 30;
 /** The name of the SessionManager port forwarding document. This document is managed by AWS.  */
 const START_SSH_SESSION_DOCUMENT_NAME = "AWS-StartSSHSession";
 
+/**There are 2 cases of unprovisioned access in AWS
+ * 1. SSM:StartSession action is missing either on the SSM document (AWS-StartSSHSession) or the EC2 instance
+ * 2. Temporary error when issuing an SCP command
+ *
+ * 1: results in UNAUTHORIZED_START_SESSION_MESSAGE
+ * 2: results in CONNECTION_CLOSED_MESSAGE
+ */
+const unprovisionedAccessPatterns = [
+  /** Matches the error message that AWS SSM prints when access is not propagated */
+  // Note that the resource will randomly be either the SSM document or the EC2 instance
+  {
+    pattern:
+      /An error occurred \(AccessDeniedException\) when calling the StartSession operation: User: arn:aws:sts::.*:assumed-role\/P0GrantsRole.* is not authorized to perform: ssm:StartSession on resource: arn:aws:.*:.*:.* because no identity-based policy allows the ssm:StartSession action/,
+  },
+  /**
+   * Matches the following error messages that AWS SSM pints when ssh authorized
+   * key access hasn't propagated to the instance yet.
+   * - Connection closed by UNKNOWN port 65535
+   * - scp: Connection closed
+   * - kex_exchange_identification: Connection closed by remote host
+   */
+  {
+    pattern: /\bConnection closed\b.*\b(?:by UNKNOWN port \d+|by remote host)?/,
+  },
+] as const;
+
 export const awsSshProvider: SshProvider<
   AwsSshPermissionSpec,
   undefined,
   AwsSshRequest,
   AwsCredentials
 > = {
-  requestToSsh: (request) => {
-    const { permission, generated } = request;
-    const { instanceId, accountId, region } = permission.spec;
-    const { idc, ssh, name } = generated;
-    const { linuxUserName } = ssh;
-    const common = { linuxUserName, accountId, region, id: instanceId };
-    return !idc
-      ? { ...common, role: name, type: "aws", access: "role" }
-      : {
-          ...common,
-          idc,
-          permissionSet: name,
-          type: "aws",
-          access: "idc",
-        };
-  },
-  toCliRequest: async (request) => ({ ...request, cliLocalData: undefined }),
-  ensureInstall: async () => {
-    if (!(await ensureSsmInstall())) {
-      throw "Please try again after installing the required AWS utilities";
-    }
-  },
   cloudProviderLogin: async (authn, request) => {
     const { config } = await getAwsConfig(authn, request.accountId);
     if (!config.login?.type || config.login?.type === "iam") {
@@ -71,6 +75,19 @@ export const awsSshProvider: SshProvider<
         ? await assumeRoleWithOktaSaml(authn, request as AwsSshRoleRequest)
         : throwAssertNever(config.login);
   },
+
+  ensureInstall: async () => {
+    if (!(await ensureSsmInstall())) {
+      throw "Please try again after installing the required AWS utilities";
+    }
+  },
+
+  friendlyName: "AWS",
+
+  maxRetries: MAX_SSH_RETRIES,
+
+  preTestAccessPropagationArgs: () => undefined,
+
   proxyCommand: (request) => {
     return [
       "aws",
@@ -86,6 +103,7 @@ export const awsSshProvider: SshProvider<
       '"portNumber=%p"',
     ];
   },
+
   reproCommands: (request) => {
     // TODO: Add manual commands for IDC login
     if (request.access !== "idc") {
@@ -95,7 +113,25 @@ export const awsSshProvider: SshProvider<
     }
     return undefined;
   },
-  preTestAccessPropagationArgs: () => undefined,
-  maxRetries: MAX_SSH_RETRIES,
-  friendlyName: "AWS",
+
+  requestToSsh: (request) => {
+    const { permission, generated } = request;
+    const { instanceId, accountId, region } = permission.spec;
+    const { idc, ssh, name } = generated;
+    const { linuxUserName } = ssh;
+    const common = { linuxUserName, accountId, region, id: instanceId };
+    return !idc
+      ? { ...common, role: name, type: "aws", access: "role" }
+      : {
+          ...common,
+          idc,
+          permissionSet: name,
+          type: "aws",
+          access: "idc",
+        };
+  },
+
+  toCliRequest: async (request) => ({ ...request, cliLocalData: undefined }),
+
+  unprovisionedAccessPatterns,
 };
diff --git a/src/plugins/google/ssh.ts b/src/plugins/google/ssh.ts
@@ -20,35 +20,75 @@ import { GcpSshPermissionSpec, GcpSshRequest } from "./types";
  */
 const MAX_SSH_RETRIES = 120;
 
+/**
+ * There are 7 cases of unprovisioned access in Google Cloud.
+ * These are all potentially subject to propagation delays.
+ * 1. The linux user name is not present in the user's Google Workspace profile `posixAccounts` attribute
+ * 2. The public key is not present in the user's Google Workspace profile `sshPublicKeys` attribute
+ * 3. The user cannot act as the service account of the compute instance
+ * 4. The user cannot tunnel through the IAP tunnel to the instance
+ * 5. The user doesn't have osLogin or osAdminLogin role to the instance
+ * 5.a. compute.instances.get permission is missing
+ * 5.b. compute.instances.osLogin permission is missing
+ * 6. compute.instances.osAdminLogin is not provisioned but compute.instances.osLogin is - happens when a user upgrades existing access to sudo
+ * 7: Rare occurrence, the exact conditions so far undetermined (together with CONNECTION_CLOSED_MESSAGE)
+ *
+ * 1, 2, 3 (yes!), 5b: result in PUBLIC_KEY_DENIED_MESSAGE
+ * 4: results in UNAUTHORIZED_TUNNEL_USER_MESSAGE and also CONNECTION_CLOSED_MESSAGE
+ * 5a: results in UNAUTHORIZED_INSTANCES_GET_MESSAGE
+ * 6: results in SUDO_MESSAGE
+ * 7: results in DESTINATION_READ_ERROR and also CONNECTION_CLOSED_MESSAGE
+ */
+const unprovisionedAccessPatterns = [
+  { pattern: /Permission denied \(publickey\)/ },
+  {
+    // The output of `sudo -v` when the user is not allowed to run sudo
+    pattern: /Sorry, user .+ may not run sudo on .+/,
+  },
+  { pattern: /Error while connecting \[4033: 'not authorized'\]/ },
+  {
+    pattern: /Required 'compute\.instances\.get' permission/,
+    validationWindowMs: 30e3,
+  },
+  { pattern: /Error while connecting \[4010: 'destination read failed'\]/ },
+] as const;
+
 export const gcpSshProvider: SshProvider<
   GcpSshPermissionSpec,
   { linuxUserName: string },
   GcpSshRequest
 > = {
-  requestToSsh: (request) => {
-    return {
-      id: request.permission.spec.instanceName,
-      projectId: request.permission.spec.projectId,
-      zone: request.permission.spec.zone,
-      linuxUserName: request.cliLocalData.linuxUserName,
-      type: "gcloud",
-    };
-  },
-  toCliRequest: async (request, options) => ({
-    ...request,
-    cliLocalData: {
-      linuxUserName: await importSshKey(
-        request.permission.spec.publicKey,
-        options
-      ),
-    },
-  }),
+  // TODO support login with Google Cloud
+  cloudProviderLogin: async () => undefined,
+
   ensureInstall: async () => {
     if (!(await ensureGcpSshInstall())) {
       throw "Please try again after installing the required GCP utilities";
     }
   },
-  cloudProviderLogin: async () => undefined, // TODO @ENG-2284 support login with Google Cloud
+
+  friendlyName: "Google Cloud",
+
+  loginRequiredMessage:
+    "Please login to Google Cloud CLI with 'gcloud auth login'",
+
+  loginRequiredPattern: /You do not currently have an active account selected/,
+
+  maxRetries: MAX_SSH_RETRIES,
+
+  preTestAccessPropagationArgs: (cmdArgs) => {
+    if (isSudoCommand(cmdArgs)) {
+      return {
+        ...cmdArgs,
+        // `sudo -v` prints `Sorry, user <user> may not run sudo on <hostname>.` to stderr when user is not a sudoer.
+        // It prints nothing to stdout when user is a sudoer - which is important because we don't want any output from the pre-test.
+        command: "sudo",
+        arguments: ["-v"],
+      };
+    }
+    return undefined;
+  },
+
   proxyCommand: (request) => {
     return [
       "gcloud",
@@ -65,19 +105,28 @@ export const gcpSshProvider: SshProvider<
       `--project=${request.projectId}`,
     ];
   },
-  reproCommands: () => undefined, // TODO @ENG-2284 support login with Google Cloud
-  preTestAccessPropagationArgs: (cmdArgs) => {
-    if (isSudoCommand(cmdArgs)) {
-      return {
-        ...cmdArgs,
-        // `sudo -v` prints `Sorry, user <user> may not run sudo on <hostname>.` to stderr when user is not a sudoer.
-        // It prints nothing to stdout when user is a sudoer - which is important because we don't want any output from the pre-test.
-        command: "sudo",
-        arguments: ["-v"],
-      };
-    }
-    return undefined;
+
+  reproCommands: () => undefined,
+
+  requestToSsh: (request) => {
+    return {
+      id: request.permission.spec.instanceName,
+      projectId: request.permission.spec.projectId,
+      zone: request.permission.spec.zone,
+      linuxUserName: request.cliLocalData.linuxUserName,
+      type: "gcloud",
+    };
   },
-  maxRetries: MAX_SSH_RETRIES,
-  friendlyName: "Google Cloud",
+
+  unprovisionedAccessPatterns,
+
+  toCliRequest: async (request, options) => ({
+    ...request,
+    cliLocalData: {
+      linuxUserName: await importSshKey(
+        request.permission.spec.publicKey,
+        options
+      ),
+    },
+  }),
 };