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

fix(gamelift): restrict policy to access Script / Build content in S3 #22767

Merged
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
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-gamelift/lib/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export class Build extends BuildBase {
throw new Error(`Build name can not be longer than 1024 characters but has ${props.buildName.length} characters.`);
}
}

this.role = props.role ?? new iam.Role(this, 'ServiceRole', {
assumedBy: new iam.ServicePrincipal('gamelift.amazonaws.com'),
});
Expand All @@ -206,6 +207,8 @@ export class Build extends BuildBase {
},
});

resource.node.addDependency(this.role);

this.buildId = resource.ref;
}

Expand Down
23 changes: 18 additions & 5 deletions packages/@aws-cdk/aws-gamelift/lib/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export abstract class Content {
/**
* Called when the Build is initialized to allow this object to bind
*/
public abstract bind(scope: Construct, grantable: iam.IGrantable): ContentConfig;
public abstract bind(scope: Construct, role: iam.IRole): ContentConfig;

}

Expand All @@ -58,8 +58,14 @@ export class S3Content extends Content {
}
}

public bind(_scope: Construct, grantable: iam.IGrantable): ContentConfig {
this.bucket.grantRead(grantable, this.key);
public bind(_scope: Construct, role: iam.IRole): ContentConfig {
// Adding permission to access specific content
role.addToPrincipalPolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
resources: [this.bucket.arnForObjects(this.key)],
actions: ['s3:GetObject', 's3:GetObjectVersion'],
}));

return {
s3Location: {
bucketName: this.bucket.bucketName,
Expand All @@ -83,7 +89,7 @@ export class AssetContent extends Content {
super();
}

public bind(scope: Construct, grantable: iam.IGrantable): ContentConfig {
public bind(scope: Construct, role: iam.IRole): ContentConfig {
// If the same AssetContent is used multiple times, retain only the first instantiation.
if (!this.asset) {
this.asset = new s3_assets.Asset(scope, 'Content', {
Expand All @@ -94,7 +100,14 @@ export class AssetContent extends Content {
throw new Error(`Asset is already associated with another stack '${cdk.Stack.of(this.asset).stackName}'. ` +
'Create a new Content instance for every stack.');
}
this.asset.grantRead(grantable);
const bucket = s3.Bucket.fromBucketName(scope, 'AssetBucket', this.asset.s3BucketName);

// Adding permission to access specific content
role.addToPrincipalPolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
resources: [bucket.arnForObjects(this.asset.s3ObjectKey)],
actions: ['s3:GetObject', 's3:GetObjectVersion'],
}));

if (!this.asset.isZipArchive) {
throw new Error(`Asset must be a .zip file or a directory (${this.path})`);
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-gamelift/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
Expand Down
41 changes: 13 additions & 28 deletions packages/@aws-cdk/aws-gamelift/test/build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,37 +47,22 @@ describe('build', () => {
const contentBucketName = 'bucketname';
const contentBucketAccessStatement = {
Action: [
's3:GetObject*',
's3:GetBucket*',
's3:List*',
's3:GetObject',
's3:GetObjectVersion',
],
Effect: 'Allow',
Resource: [
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
`:s3:::${contentBucketName}`,
],
],
},
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
`:s3:::${contentBucketName}/content`,
],
Resource: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
`:s3:::${contentBucketName}/content`,
],
},
],
],
},
};
let contentBucket: s3.IBucket;
let content: gamelift.Content;
Expand Down
122 changes: 57 additions & 65 deletions packages/@aws-cdk/aws-gamelift/test/content.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,37 +38,22 @@ describe('Code', () => {
Statement: [
{
Action: [
's3:GetObject*',
's3:GetBucket*',
's3:List*',
's3:GetObject',
's3:GetObjectVersion',
],
Effect: 'Allow',
Resource: [
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::bucketname',
],
],
},
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::bucketname/content',
],
Resource: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::bucketname/content',
],
},
],
],
},
},
],
},
Expand All @@ -88,7 +73,7 @@ describe('Code', () => {
content = gamelift.Content.fromAsset(directoryPath);
});

test("with valid and existing file path and bound to job sets job's script location and permissions stack metadata", () => {
test('with valid and existing file path and bound to script location and permissions stack metadata', () => {
new gamelift.Build(stack, 'Build1', {
content: content,
});
Expand Down Expand Up @@ -146,44 +131,52 @@ describe('Code', () => {
Statement: [
{
Action: [
's3:GetObject*',
's3:GetBucket*',
's3:List*',
's3:GetObject',
's3:GetObjectVersion',
],
Effect: 'Allow',
Resource: [
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters6019bfc8ab05a24b0ae9b5d8f4585cbfc7d1c30a23286d0b25ce7066a368a5d7S3Bucket72AA8348',
},
],
],
},
{
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters6019bfc8ab05a24b0ae9b5d8f4585cbfc7d1c30a23286d0b25ce7066a368a5d7S3Bucket72AA8348',
},
'/*',
],
Resource: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':s3:::',
{
Ref: 'AssetParameters6019bfc8ab05a24b0ae9b5d8f4585cbfc7d1c30a23286d0b25ce7066a368a5d7S3Bucket72AA8348',
},
'/',
{
'Fn::Select': [
0,
{
'Fn::Split': [
'||',
{
Ref: 'AssetParameters6019bfc8ab05a24b0ae9b5d8f4585cbfc7d1c30a23286d0b25ce7066a368a5d7S3VersionKey720D3160',
},
],
},
],
},
{
'Fn::Select': [
1,
{
'Fn::Split': [
'||',
{
Ref: 'AssetParameters6019bfc8ab05a24b0ae9b5d8f4585cbfc7d1c30a23286d0b25ce7066a368a5d7S3VersionKey720D3160',
},
],
},
],
},
],
},
],
],
},
},
],
},
Expand Down Expand Up @@ -257,7 +250,6 @@ describe('Code', () => {
};

expect(stack.node.metadata.find(m => m.type === 'aws:cdk:asset')).toBeDefined();
// Job1 and Job2 use reuse the asset
Template.fromStack(stack).hasResourceProperties('AWS::GameLift::Build', {
StorageLocation,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "21.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
"path": "BuildDefaultTestDeployAssert0688841C.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
}
}
},
"9c561e93c7a2947a15dba683670660e922cf493e17b2a6f8ca03cf221442c222": {
"56a977de7626326c13fb108674329fc1a0952d0c525384c951169c7c75812e47": {
"source": {
"path": "aws-gamelift-build.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "9c561e93c7a2947a15dba683670660e922cf493e17b2a6f8ca03cf221442c222.json",
"objectKey": "56a977de7626326c13fb108674329fc1a0952d0c525384c951169c7c75812e47.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Loading