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

aws-cdk: S3 set publicReadAccess: true, fails deploy because of default deny public access policy #29564

Closed
irobinsonDandH opened this issue Mar 20, 2024 · 3 comments · Fixed by #29632
Assignees
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@irobinsonDandH
Copy link

irobinsonDandH commented Mar 20, 2024

Describe the bug

So, if you make a new s3 bucket

const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
    }
  })

While this is fine code and you can deploy it will fail in the middle with a generic access denied error not telling you what stopped it even if you are full admin. This happens due to the default deny all public access rule.

Expected Behavior

So if you make a new s3 bucket

const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
    }
  })

it will create the s3 bucket with the policy and set the deny public access to false for all 4 options

Current Behavior

Fails with access denied error while creating the bucket and doesn't say that it's because of the policy.

Reproduction Steps

Use the following code changing the bucket name to something unique.

const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
    }
  })

npx cdk deploy app

Possible Solution

A possible solution would be if you use publicReadAccess: true set all blockPublicAccess to false implicitly same if you use the grantPublicAccess() function.

or state in the documentation that you have to set blockPublicAceess to false and give a better error back.

blockPublicAccess: {
blockPublicAcls: false,
blockPublicPolicy: false,
ignorePublicAcls: false,
restrictPublicBuckets:false,
}

Additional Information/Context

No response

CDK CLI Version

2.124

Framework Version

No response

Node.js Version

18

OS

Debian

Language

TypeScript

Language Version

5.3.3

Other information

Current workaround is adding

blockPublicAccess: {
blockPublicAcls: false,
blockPublicPolicy: false,
ignorePublicAcls: false,
restrictPublicBuckets:false,
}

but I feel that publicReadAccess: true should just handle the bucket level permissions fully.
@irobinsonDandH irobinsonDandH added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 20, 2024
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Mar 20, 2024
@pahud
Copy link
Contributor

pahud commented Mar 21, 2024

A possible solution would be if you use publicReadAccess: true set all blockPublicAccess to false implicitly same if you use the grantPublicAccess() function.
or state in the documentation that you have to set blockPublicAceess to false and give a better error back.

Yes we should do that with either of the suggested solutions.

At this moment the CFN is returning 403.

Resource handler returned message: "Access Denied (Service: S3, Status Code: 403, Request ID: NN0VTK51S1ER1XTQ, Extended Request ID: 9wQETvOzgpnTdjif2tnSvNuKsPF4wj2qzHgZdnT9fs5bHsNslLeN7kZI1yppemvKD2X+jpt
+zIE=)" (RequestToken: d4971ffd-f0d8-8229-750e-3193540cdafd, HandlerErrorCode: AccessDenied)

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 21, 2024
@GavinZZ GavinZZ self-assigned this Mar 27, 2024
@GavinZZ GavinZZ assigned GavinZZ and unassigned GavinZZ May 14, 2024
@mergify mergify bot closed this as completed in #29632 May 28, 2024
mergify bot pushed a commit that referenced this issue May 28, 2024
…ied 403 (#29632)

### Issue # (if applicable)

Closes #29564

### Reason for this change

if you make a new s3 bucket
```
const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
})
```
While this is fine code and you can deploy it will fail in the middle with a generic access denied error not telling you what stopped it even if you are full admin. This happens due to the default deny all public access rule.

### Description of changes

When users only enable `publicReadAccess` without configuring `blockPublicAccess` to disable it, we will raise an exception and throw an more appropriate error message for easier diagnosis. 

We do not want to directly disable `blockPublicAccess` as it feels like a weird behaviour.

### Description of how you validated changes

New unit tests and updated integ tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

atanaspam pushed a commit to atanaspam/aws-cdk that referenced this issue Jun 3, 2024
…ied 403 (aws#29632)

### Issue # (if applicable)

Closes aws#29564

### Reason for this change

if you make a new s3 bucket
```
const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
})
```
While this is fine code and you can deploy it will fail in the middle with a generic access denied error not telling you what stopped it even if you are full admin. This happens due to the default deny all public access rule.

### Description of changes

When users only enable `publicReadAccess` without configuring `blockPublicAccess` to disable it, we will raise an exception and throw an more appropriate error message for easier diagnosis. 

We do not want to directly disable `blockPublicAccess` as it feels like a weird behaviour.

### Description of how you validated changes

New unit tests and updated integ tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@kennethaasan
Copy link

Same issue here. This is confusing and hard to understand how to fix unless you find this issue.

vdahlberg pushed a commit to vdahlberg/aws-cdk that referenced this issue Jun 10, 2024
…ied 403 (aws#29632)

### Issue # (if applicable)

Closes aws#29564

### Reason for this change

if you make a new s3 bucket
```
const staticBucket = new aws_s3.Bucket(s3Stack, `static-Bucket`, {
    bucketName: `static-bucket`,
    publicReadAccess: true,
})
```
While this is fine code and you can deploy it will fail in the middle with a generic access denied error not telling you what stopped it even if you are full admin. This happens due to the default deny all public access rule.

### Description of changes

When users only enable `publicReadAccess` without configuring `blockPublicAccess` to disable it, we will raise an exception and throw an more appropriate error message for easier diagnosis. 

We do not want to directly disable `blockPublicAccess` as it feels like a weird behaviour.

### Description of how you validated changes

New unit tests and updated integ tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants