Fix: S3 ACL analysis rule for WRITE permissions never triggers #1128
Conversation
`acl.uri` and `acl.granteeid` are mutually exclusive as per https://github.com/lyft/cartography/blob/f613c32959f4bbdab53f83b2a03532c62e180e8d/cartography/intel/aws/s3.py#L554-L578. So the "WRITE" rule would never trigger. And as these rules are all about detecting _anonymous_ access permissions, there shouldn't be any clauses checking that the grantee is the owner. And as per AWS' documentation, the `WRITE` permission allows the `s3:PutObject` action, so that should be added to `anonymous_actions`. (`s3:DeleteObjectVersion` is only ever granted to bucket owners.) For the "READ_ACP" rule the granted anonymous actions are incorrect.
jhecking
changed the title
|
Hi @achantavy, do you mind taking a look at this please? This is a second issue I found while reviewing S3 access control related features in Cartography. We've fixed this in our local copy of Cartography, but I would like to see this merged upstream as well so we don't have to maintain this patch on our end. Thx. |
|
Hey @jhecking, thanks for the PR and sorry it took so long to review. Your write-up and changes make sense. Analysis jobs scare me a bit in that we don't have test coverage on them so I took some time to add them here. I tried to fetch your branch and add tests to this PR but I messed up my git commands and it didn't work the way I thought -oops. So, I'm going to close this one and open #1244. You'll still get the credit in the commit history as a coauthor. Thanks again for the PR. |
#1128 + tests. See that original PR for full context. Reviewer notes: the main change is here: 13feb15. Everything else is tests to make sure the analysis job works. --------- Co-authored-by: Jan Hecking <[email protected]>
…graphy-cncf#1244) cartography-cncf#1128 + tests. See that original PR for full context. Reviewer notes: the main change is here: cartography-cncf@13feb15. Everything else is tests to make sure the analysis job works. --------- Co-authored-by: Jan Hecking <[email protected]>
The S3 ACL analysis rule for WRITE permissions granted by an ACL can never trigger because of an invalid query statement. The query checks that
acl.uriis set to a particular value and thatacl.ownerid = acl.granteeidbutacl.uriandacl.granteeidare mutually exclusive as per https://github.com/lyft/cartography/blob/f613c32959f4bbdab53f83b2a03532c62e180e8d/cartography/intel/aws/s3.py#L554-L578 So the "WRITE" rule would never trigger. And as these rules are all about detecting anonymous access permissions, there shouldn't be any clauses checking that the grantee is the owner.Furthermore, as per AWS' documentation, the
WRITEpermission allows thes3:PutObjectaction, so that should be added toanonymous_actions. (s3:DeleteObjectVersionis only ever granted to bucket owners.)For the "READ_ACP" rule the granted anonymous actions are incorrect.