Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Restrict instance SSM permissions #3918

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
});
});

Expand All @@ -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)(
Expand Down Expand Up @@ -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',
},
],
});
});

Expand All @@ -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',
},
],
});
});

Expand All @@ -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',
},
],
});
});

Expand Down
8 changes: 6 additions & 2 deletions lambdas/functions/control-plane/src/scale-runners/scale-up.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions lambdas/libs/aws-ssm-util/src/index.ts
Original file line number Diff line number Diff line change
@@ -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> {
Expand All @@ -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,
}),
);
}
7 changes: 6 additions & 1 deletion modules/runners/policies/instance-ssm-parameters-policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion modules/runners/policies/lambda-scale-up.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
{
"Effect": "Allow",
"Action": [
"ssm:PutParameter"
"ssm:PutParameter",
"ssm:AddTagsToResource"
],
"Resource": "*"
},
Expand Down
1 change: 1 addition & 0 deletions modules/runners/pool/policies/lambda-pool.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
{
"Effect": "Allow",
"Action": [
"ssm:AddTagsToResource",
"ssm:PutParameter"
],
"Resource": "*"
Expand Down
Loading