feat: Restrict instance SSM permissions (#3918)

## Restrict instance SSM permissions Previously, EC2 instances could read other instances' tokens (via path .../tokens/...) from SSM parameters. This PR restricts access to only read / delete tokens owned by the instances Co-authored-by: Blake Burkhart <bburky@bburky.com>
philips-labs · May 22, 2024 · 9399cf2 · 9399cf2
1 parent 93e8d27
commit 9399cf2
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 6 deletions.
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
@@ -338,6 +338,12 @@ describe('scaleUp with GHES', () => {
         Name: '/github-action-runners/default/runners/config/i-12345',
         Value: 'TEST_JIT_CONFIG_ORG',
         Type: 'SecureString',
+        Tags: [
+          {
+            Key: 'InstanceId',
+            Value: 'i-12345',
+          },
+        ],
       });
     });
 
@@ -353,6 +359,12 @@ describe('scaleUp with GHES', () => {
           '--url https://github.enterprise.something/Codertocat --token 1234abcd ' +
           '--labels label1,label2 --runnergroup Default',
         Type: 'SecureString',
+        Tags: [
+          {
+            Key: 'InstanceId',
+            Value: 'i-12345',
+          },
+        ],
       });
     });
     it.each(RUNNER_TYPES)(
@@ -708,6 +720,12 @@ describe('scaleUp with public GH', () => {
         Name: '/github-action-runners/default/runners/config/i-12345',
         Value: 'TEST_JIT_CONFIG_REPO',
         Type: 'SecureString',
+        Tags: [
+          {
+            Key: 'InstanceId',
+            Value: 'i-12345',
+          },
+        ],
       });
     });
 
@@ -724,6 +742,12 @@ describe('scaleUp with public GH', () => {
         Name: '/github-action-runners/default/runners/config/i-12345',
         Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --ephemeral',
         Type: 'SecureString',
+        Tags: [
+          {
+            Key: 'InstanceId',
+            Value: 'i-12345',
+          },
+        ],
       });
     });
 
@@ -741,6 +765,12 @@ describe('scaleUp with public GH', () => {
         Name: '/github-action-runners/default/runners/config/i-12345',
         Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --labels jit',
         Type: 'SecureString',
+        Tags: [
+          {
+            Key: 'InstanceId',
+            Value: 'i-12345',
+          },
+        ],
       });
     });
 

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -362,7 +362,9 @@ async function createRegistrationTokenConfig(
   });
 
   for (const instance of instances) {
-    await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerServiceConfig.join(' '), true);
+    await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerServiceConfig.join(' '), true, {
+      tags: [{ Key: 'InstanceId', Value: instance }],
+    });
     if (isDelay) {
       // Delay to prevent AWS ssm rate limits by being within the max throughput limit
       await delay(25);
@@ -405,7 +407,9 @@ async function createJitConfig(githubRunnerConfig: CreateGitHubRunnerConfig, ins
     logger.debug('Runner JIT config for ephemeral runner generated.', {
       instance: instance,
     });
-    await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerConfig.data.encoded_jit_config, true);
+    await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerConfig.data.encoded_jit_config, true, {
+      tags: [{ Key: 'InstanceId', Value: instance }],
+    });
     if (isDelay) {
       // Delay to prevent AWS ssm rate limits by being within the max throughput limit
       await delay(25);

diff --git a/lambdas/libs/aws-ssm-util/src/index.ts b/lambdas/libs/aws-ssm-util/src/index.ts
@@ -1,4 +1,4 @@
-import { GetParameterCommand, PutParameterCommand, SSMClient } from '@aws-sdk/client-ssm';
+import { GetParameterCommand, PutParameterCommand, SSMClient, Tag } from '@aws-sdk/client-ssm';
 import { getTracedAWSV3Client } from '@terraform-aws-github-runner/aws-powertools-util';
 
 export async function getParameter(parameter_name: string): Promise<string> {
@@ -7,13 +7,19 @@ export async function getParameter(parameter_name: string): Promise<string> {
     ?.Value as string;
 }
 
-export async function putParameter(parameter_name: string, parameter_value: string, secure: boolean): Promise<void> {
+export async function putParameter(
+  parameter_name: string,
+  parameter_value: string,
+  secure: boolean,
+  options: { tags?: Tag[] } = {},
+): Promise<void> {
   const client = getTracedAWSV3Client(new SSMClient({ region: process.env.AWS_REGION }));
   await client.send(
     new PutParameterCommand({
       Name: parameter_name,
       Value: parameter_value,
       Type: secure ? 'SecureString' : 'String',
+      Tags: options.tags,
     }),
   );
 }
diff --git a/modules/runners/policies/instance-ssm-parameters-policy.json b/modules/runners/policies/instance-ssm-parameters-policy.json
@@ -8,7 +8,12 @@
         "ssm:GetParameters",
         "ssm:GetParameter"
       ],
-      "Resource": "${arn_ssm_parameters_path_tokens}*"
+      "Resource": "${arn_ssm_parameters_path_tokens}/*",
+      "Condition": {
+        "StringLike": {
+          "ec2:SourceInstanceARN": "*/$${aws:ResourceTag/InstanceId}"
+        }
+      }
     },
     {
       "Effect": "Allow",

diff --git a/modules/runners/policies/lambda-scale-up.json b/modules/runners/policies/lambda-scale-up.json
@@ -22,7 +22,8 @@
         {
             "Effect": "Allow",
             "Action": [
-                "ssm:PutParameter"
+                "ssm:PutParameter",
+                "ssm:AddTagsToResource"
             ],
             "Resource": "*"
         },

diff --git a/modules/runners/pool/policies/lambda-pool.json b/modules/runners/pool/policies/lambda-pool.json
@@ -22,6 +22,7 @@
         {
             "Effect": "Allow",
             "Action": [
+                "ssm:AddTagsToResource",
                 "ssm:PutParameter"
             ],
             "Resource": "*"