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
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
@@ -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'),
});
@@ -206,6 +207,8 @@ export class Build extends BuildBase {
},
});

resource.node.addDependency(this.role);

this.buildId = resource.ref;
}

23 changes: 18 additions & 5 deletions packages/@aws-cdk/aws-gamelift/lib/content.ts
Original file line number Diff line number Diff line change
@@ -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;

}

@@ -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,
@@ -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', {
@@ -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})`);
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-gamelift/package.json
Original file line number Diff line number Diff line change
@@ -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",
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
@@ -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;
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
@@ -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',
],
},
],
],
},
},
],
},
@@ -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,
});
@@ -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',
},
],
},
],
},
],
},
],
],
},
},
],
},
@@ -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,
});
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
@@ -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}"
}
}
Loading