dataall icon indicating copy to clipboard operation
dataall copied to clipboard

Remove access without constraints on the pivotRole

Open zsaltys opened this issue 1 year ago • 5 comments

The pivot role installed on AWS accounts has policies which are picked up by Checkov scanner where the role has unrestricted access. 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		: /deploy/pivot_role/pivotRole.yaml:49-218
Resource	: AWS::IAM::ManagedPolicy.PivotRolePolicy0
Guideline	: CKV_AWS_111 
CheckID		: CKV_AWS_111
CheckName	: Ensure IAM policies does not allow write access without constraints
File		: /deploy/pivot_role/pivotRole.yaml:220-338
Resource	: AWS::IAM::ManagedPolicy.PivotRolePolicy1
Guideline	: CKV_AWS_111 

Expected resolution

Please ensure that the pivotRole never grants an action which is completely unrestricted. Ideally everything should be restricted by asking for dataall prefix OR if it is an imported resource then it has to be solved in a way where the imported resource is mentioned specifically.

zsaltys avatar Nov 16 '23 17:11 zsaltys

Thanks for raising this issue @zsaltys. Data.all supports 2 ways of creating the pivotRole, via CloudFormation before linking Environment, or via CDK when linking Environment. Some of the pain points you referenced above with imported resources has been resolved with the CDK way of creating the pivotRole but may not be fully addressed with the CloudFormation way since it is a manual creation via upload stack to CloudFormation (this is why we suggest to use CDK to create PivotRole as best practice)

We should further evaluate if scanners are reporting any issues for the CDK pivot role creation and also see what ways we can restrict both pivotRole policies further - to take a look at this when some capacity frees up from the team

noah-paige avatar Nov 24 '23 19:11 noah-paige

@noah-paige let's use this ticket for the purpose of removing unconstrained access on the pivotRole installed by data.all. As an example I'll use this statement from the pivot role policies:

{
       "Action": [
        "iam:PutRolePolicy",
        "iam:DeleteRolePolicy"
       ],
       "Effect": "Allow",
       "Resource": "*",
       "Sid": "IAMRolePolicy"
      },

This allows the pivot role to modify any policy. I as owner of data.all on the infra account can easily write code to assume the pivot role through the graphql role and then effectively I can modify any policy on any account... Effectively if the pivotRole is ever compromised it gives attackers access to resources which were never created by data.all or imported to data.all. When we onboard users to data.all and they see such roles installed on their accounts it really scares them.

As general rule we need to make sure that pivotRole only has access to things that data.all itself created or things that were imported to data.all like kms keys, buckets, consumer roles.. and nothing else.

zsaltys avatar Jan 26 '24 17:01 zsaltys

Created the PR - https://github.com/data-dot-all/dataall/pull/1075 to restrict logging, and cloudformation stacks. The remaining exceptions will be taken up in a different ticket.

mourya-33 avatar Feb 23 '24 04:02 mourya-33

The pending items that are not fixed in this ticket are tracked here

https://github.com/data-dot-all/dataall/issues/1195 - RAM permissions https://github.com/data-dot-all/dataall/issues/1189 - Glue and KMS permissions

mourya-33 avatar Apr 19 '24 16:04 mourya-33

@zsaltys i updated the ticket with the details on pending items and their corresponding tickets. Can we close this?

mourya-33 avatar Apr 19 '24 16:04 mourya-33

@mourya-33 thanks

zsaltys avatar Jun 06 '24 09:06 zsaltys