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.

Sign up for GitHub

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

Jump to bottom

Remove access without constraints on various policies #877

Closed
zsaltys opened this issue Nov 16, 2023 · 5 comments
Closed

Remove access without constraints on various policies #877

zsaltys opened this issue Nov 16, 2023 · 5 comments
Labels
effort: medium priority: medium

Comments

@zsaltys
Copy link
Contributor

zsaltys commented Nov 16, 2023

There are various policies installed by data.all granting unrestricted access to resources. This gets picked up by Checkov security scanner. By unrestricted I mean granting actions on resource '*'.

This is the list of these actions picked up:

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/dataall-cdk-template-cicd-stack.template.json:3106-3140
Resource	: AWS::IAM::Policy.dataallcdktemplatecdkpipelinePipelineBuildSynthCdkBuildProjectPolicyDocument0F8F4003
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/dataall-cdk-template-cicd-stack.template.json:3605-3639
Resource	: AWS::IAM::Policy.dataallcdktemplatecdkpipelinePipelinedataallstagingdbmigrationstageMigrateDBPolicyDocumentFD518241
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/dataall-cdk-template-cicd-stack.template.json:4630-4752
Resource	: AWS::IAM::Policy.dataallcdktemplatecdkpipelineAssetsFileRoleDefaultPolicyADA3EF5E
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-backend-stage/dataallcdktemplatecicdstackdataallstagingbackendstagebackendstackDbMigration96E13135.nested.template.json:554-585
Resource	: AWS::IAM::Policy.DBMigrationCBProjectstagingPolicyDocument690B6300
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-backend-stage/dataallcdktemplatecicdstackdataallstagingbackendstagebackendstackLambdas760AD7F8.nested.template.json:310-442
Resource	: AWS::IAM::Role.dataallstagingesproxyrole91A7EBFB
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-backend-stage/dataallcdktemplatecicdstackdataallstagingbackendstagebackendstackLambdas760AD7F8.nested.template.json:879-1011
Resource	: AWS::IAM::Role.dataallstaginggraphqlrole479A99FC
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-backend-stage/dataallcdktemplatecicdstackdataallstagingbackendstagebackendstackLambdas760AD7F8.nested.template.json:1467-1599
Resource	: AWS::IAM::Role.dataallstagingawsworkerroleFDFEE189
Guideline	: CKV_AWS_111 


CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-backend-stage/dataallcdktemplatecicdstackdataallstagingbackendstagebackendstackLambdas760AD7F8.nested.template.json:2925-2951
Resource	: AWS::IAM::Policy.LogRetentionaae0aa3c5b4d4f87b02d85b201efdd8aServiceRoleDefaultPolicyADDA7DEB
Guideline	: CKV_AWS_111 

CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /cdk.out/assembly-dataall-cdk-template-cicd-stack-dataall-staging-cloudfront-stage/dataallcdktemplatecicdstackdataallstagingcloudfrontstagecloudfrontstackCloudFront17AF3D7E.nested.template.json:1128-1206
Resource	: AWS::IAM::Policy.S3DeploymentRolestagingDefaultPolicy4672E484
Guideline	: CKV_AWS_111

Expected resolution

Please ensure that the these policies do not grant completely unrestricted access to resources. Ideally everything should be restricted by asking for dataall prefix.
@noah-paige
Copy link
Contributor

Thanks for raising this issue @zsaltys, we will have to investigate which roles Checkov is reporting as non-compliant and ensure we can restrict them further, but yes theoretically every write operation should be able to be limited to at least some form of naming convention based on the type of data.all configuration

We will do some further investigation on the above and report back with our findings

@noah-paige noah-paige mentioned this issue Jan 10, 2024
Merged
@mourya-33 mourya-33 mentioned this issue Mar 30, 2024
Merged
@mourya-33
Copy link
Contributor

Created a PR - #1134 for this

  1. Cloudfront changes to policy are currently in the PR.
  2. DB, lambda and main stack changes are in progress

dlpzx pushed a commit that referenced this issue Apr 15, 2024
@mourya-33 @noah-paige
Implement least privilege principle for cloudfront, lambda and db mig…
af46a25 
…ration stacks (#1134)

…front permissions

### Feature or Bugfix
- Bugfix

### Relates
[- <URL or Ticket>](#877)

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)? N/A
  - Is the input sanitized? N/A 
- What precautions are you taking before deserializing the data you
consume? N/A
  - Is injection prevented by parametrizing queries? N/A
  - Have you ensured no `eval` or similar functions are used? N/A
- Does this PR introduce any functionality or component that requires
authorization? N/A
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
N/A
  - Are you logging failed auth attempts? N/A
- Are you using or adding any cryptographic features? N/A
  - Do you use a standard proven implementations? yes
- Are the used keys controlled by the customer? Where are they stored?
N/A
- Are you introducing any new policies/roles/users? N/A
- Have you used the least-privilege principle? How? Yes, by removing the
* for cloudfront permissions and explicitly specifying the distribution
id arn.


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

---------

Co-authored-by: Noah Paige <noahpaig@amazon.com>
@mourya-33 mourya-33 mentioned this issue Apr 18, 2024
Open
@zsaltys
Copy link
Contributor Author

zsaltys commented Jun 6, 2024

@mourya-33 can tis be closed? Have we solved anything we can with this specific ticket?

@mourya-33
Copy link
Contributor

I am closing this @zsaltys , Except for the AssetFileRole and S3BucketDeployment Role, rest of them are addressed in this.

The AssetFileRole needs checkov baselining and is tracked here - #1188
For the bucket deployment role, we have a separate PFR for CDK

@mourya-33
Copy link
Contributor

@zsaltys i dont seem to have permissions to close this since you are the owner for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
effort: medium priority: medium
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zsaltys @noah-paige @anmolsgandhi @mourya-33