Skip to content
This repository was archived by the owner on Jan 16, 2025. It is now read-only.

Commit 9399cf2

Browse files
npalmbburky
andauthored
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 <[email protected]>
1 parent 93e8d27 commit 9399cf2

File tree

6 files changed

+53
-6
lines changed

6 files changed

+53
-6
lines changed

lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts

+30
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,12 @@ describe('scaleUp with GHES', () => {
338338
Name: '/github-action-runners/default/runners/config/i-12345',
339339
Value: 'TEST_JIT_CONFIG_ORG',
340340
Type: 'SecureString',
341+
Tags: [
342+
{
343+
Key: 'InstanceId',
344+
Value: 'i-12345',
345+
},
346+
],
341347
});
342348
});
343349

@@ -353,6 +359,12 @@ describe('scaleUp with GHES', () => {
353359
'--url https://github.enterprise.something/Codertocat --token 1234abcd ' +
354360
'--labels label1,label2 --runnergroup Default',
355361
Type: 'SecureString',
362+
Tags: [
363+
{
364+
Key: 'InstanceId',
365+
Value: 'i-12345',
366+
},
367+
],
356368
});
357369
});
358370
it.each(RUNNER_TYPES)(
@@ -708,6 +720,12 @@ describe('scaleUp with public GH', () => {
708720
Name: '/github-action-runners/default/runners/config/i-12345',
709721
Value: 'TEST_JIT_CONFIG_REPO',
710722
Type: 'SecureString',
723+
Tags: [
724+
{
725+
Key: 'InstanceId',
726+
Value: 'i-12345',
727+
},
728+
],
711729
});
712730
});
713731

@@ -724,6 +742,12 @@ describe('scaleUp with public GH', () => {
724742
Name: '/github-action-runners/default/runners/config/i-12345',
725743
Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --ephemeral',
726744
Type: 'SecureString',
745+
Tags: [
746+
{
747+
Key: 'InstanceId',
748+
Value: 'i-12345',
749+
},
750+
],
727751
});
728752
});
729753

@@ -741,6 +765,12 @@ describe('scaleUp with public GH', () => {
741765
Name: '/github-action-runners/default/runners/config/i-12345',
742766
Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --labels jit',
743767
Type: 'SecureString',
768+
Tags: [
769+
{
770+
Key: 'InstanceId',
771+
Value: 'i-12345',
772+
},
773+
],
744774
});
745775
});
746776

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,9 @@ async function createRegistrationTokenConfig(
362362
});
363363

364364
for (const instance of instances) {
365-
await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerServiceConfig.join(' '), true);
365+
await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerServiceConfig.join(' '), true, {
366+
tags: [{ Key: 'InstanceId', Value: instance }],
367+
});
366368
if (isDelay) {
367369
// Delay to prevent AWS ssm rate limits by being within the max throughput limit
368370
await delay(25);
@@ -405,7 +407,9 @@ async function createJitConfig(githubRunnerConfig: CreateGitHubRunnerConfig, ins
405407
logger.debug('Runner JIT config for ephemeral runner generated.', {
406408
instance: instance,
407409
});
408-
await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerConfig.data.encoded_jit_config, true);
410+
await putParameter(`${githubRunnerConfig.ssmTokenPath}/${instance}`, runnerConfig.data.encoded_jit_config, true, {
411+
tags: [{ Key: 'InstanceId', Value: instance }],
412+
});
409413
if (isDelay) {
410414
// Delay to prevent AWS ssm rate limits by being within the max throughput limit
411415
await delay(25);
+8-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { GetParameterCommand, PutParameterCommand, SSMClient } from '@aws-sdk/client-ssm';
1+
import { GetParameterCommand, PutParameterCommand, SSMClient, Tag } from '@aws-sdk/client-ssm';
22
import { getTracedAWSV3Client } from '@terraform-aws-github-runner/aws-powertools-util';
33

44
export async function getParameter(parameter_name: string): Promise<string> {
@@ -7,13 +7,19 @@ export async function getParameter(parameter_name: string): Promise<string> {
77
?.Value as string;
88
}
99

10-
export async function putParameter(parameter_name: string, parameter_value: string, secure: boolean): Promise<void> {
10+
export async function putParameter(
11+
parameter_name: string,
12+
parameter_value: string,
13+
secure: boolean,
14+
options: { tags?: Tag[] } = {},
15+
): Promise<void> {
1116
const client = getTracedAWSV3Client(new SSMClient({ region: process.env.AWS_REGION }));
1217
await client.send(
1318
new PutParameterCommand({
1419
Name: parameter_name,
1520
Value: parameter_value,
1621
Type: secure ? 'SecureString' : 'String',
22+
Tags: options.tags,
1723
}),
1824
);
1925
}

modules/runners/policies/instance-ssm-parameters-policy.json

+6-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88
"ssm:GetParameters",
99
"ssm:GetParameter"
1010
],
11-
"Resource": "${arn_ssm_parameters_path_tokens}*"
11+
"Resource": "${arn_ssm_parameters_path_tokens}/*",
12+
"Condition": {
13+
"StringLike": {
14+
"ec2:SourceInstanceARN": "*/$${aws:ResourceTag/InstanceId}"
15+
}
16+
}
1217
},
1318
{
1419
"Effect": "Allow",

modules/runners/policies/lambda-scale-up.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
{
2323
"Effect": "Allow",
2424
"Action": [
25-
"ssm:PutParameter"
25+
"ssm:PutParameter",
26+
"ssm:AddTagsToResource"
2627
],
2728
"Resource": "*"
2829
},

modules/runners/pool/policies/lambda-pool.json

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
{
2323
"Effect": "Allow",
2424
"Action": [
25+
"ssm:AddTagsToResource",
2526
"ssm:PutParameter"
2627
],
2728
"Resource": "*"

0 commit comments

Comments
 (0)