Skip to content

Commit

Permalink
fix(gamelift): restrict policy to access Script / Build content in S3 (
Browse files Browse the repository at this point in the history
…#22767)

Restrict S3 policy to restrict access to script or build content stored in S3.

Integration tests has been upgraded to new integ-tests version
----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
stevehouel committed Nov 8, 2022
1 parent 6c606d0 commit c936002
Show file tree
Hide file tree
Showing 22 changed files with 730 additions and 485 deletions.
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

0 comments on commit c936002

Please sign in to comment.