Fix: S3 ACL analysis rule for WRITE permissions never triggers

cartography/data/jobs/analysis/aws_s3acl_analysis.json

@@ -1,22 +1,27 @@
 {
   "statements": [
     {
+      "__comment__": "READ -> ListBucket, ListBucketVersions, ListBucketMultipartUploads",
       "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'READ'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:ListBucket', 's3:ListBucketVersions', 's3:ListBucketMultipartUploads']",
       "iterative": false
     },
     {
-      "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'WRITE'\nAND (acl.ownerid = acl.granteeid)\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:DeleteObjectVersion']",
+      "__comment__": "WRITE -> PutObject",
+      "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'WRITE'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:PutObject']",
       "iterative": false
     },
     {
-      "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'READ_ACP'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:DeleteObjectVersion']",
+      "__comment__": "READ_ACP -> GetBucketAcl",
+      "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'READ_ACP'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:GetBucketAcl']",
       "iterative": false
     },
     {
+      "__comment__": "WRITE_ACP -> PutBucketAcl",
       "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'WRITE_ACP'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:PutBucketAcl']",
       "iterative": false
     },
     {
+      "__comment__": "FULL_CONTROL -> Pretty much everything",
       "query": "MATCH (acl:S3Acl)-[:APPLIES_TO]->(bucket:S3Bucket)<-[:RESOURCE]-(aws:AWSAccount{id: $AWS_ID})\nWHERE acl.uri IN ['http://acs.amazonaws.com/groups/global/AllUsers', 'http://acs.amazonaws.com/groups/global/AuthenticatedUsers'] AND acl.permission = 'FULL_CONTROL'\nSET bucket.anonymous_access = true, bucket.anonymous_actions = coalesce(bucket.anonymous_actions, []) + ['s3:ListBucket', 's3:ListBucketVersions', 's3:ListBucketMultipartUploads', 's3:PutObject', 's3:DeleteObject', 's3:DeleteObjectVersion', 's3:PutBucketAcl']",
       "iterative": false
     }],

cartography/intel/aws/s3.py

@@ -222,7 +222,12 @@ def _is_common_exception(e: Exception, bucket: Dict) -> bool:
 
 
 @timeit
-def _load_s3_acls(neo4j_session: neo4j.Session, acls: Dict, aws_account_id: str, update_tag: int) -> None:
+def _load_s3_acls(
+        neo4j_session: neo4j.Session,
+        acls: List[Dict[str, Any]],
+        aws_account_id: str,
+        update_tag: int,
+) -> None:
     """
     Ingest S3 ACL into neo4j.
     """

tests/data/aws/s3.py

@@ -23,6 +23,69 @@
     },
 }
 
+OPEN_BUCKET_ACLS = {
+    "bucket-1": {
+        "Owner": {
+            "DisplayName": "my-display-name-1",
+            "ID": "3cb8",
+        },
+        "Grants": [
+            {
+                "Grantee": {
+                    "DisplayName": "my-display-name-1",
+                    "ID": "3cb8",
+                    "Type": "CanonicalUser",
+                },
+                "Permission": "FULL_CONTROL",
+            },
+        ],
+    },
+    "bucket-2": {
+        "Owner": {
+            "DisplayName": "my-display-name-2",
+            "ID": "828a",
+        },
+        "Grants": [
+            {
+                "Grantee": {
+                    "Type": "Group",
+                    "URI": "http://acs.amazonaws.com/groups/global/AllUsers",
+                },
+                "Permission": "READ",
+            },
+            {
+                "Grantee": {
+                    "Type": "Group",
+                    "URI": "http://acs.amazonaws.com/groups/global/AuthenticatedUsers",
+                },
+                "Permission": "READ_ACP",
+            },
+        ],
+    },
+    "bucket-3": {
+        "Owner": {
+            "DisplayName": "my-display-name-2",
+            "ID": "828a",
+        },
+        "Grants": [
+            {
+                "Grantee": {
+                    "Type": "Group",
+                    "URI": "http://acs.amazonaws.com/groups/global/AllUsers",
+                },
+                "Permission": "WRITE_ACP",
+            },
+            {
+                "Grantee": {
+                    "Type": "Group",
+                    "URI": "http://acs.amazonaws.com/groups/global/AuthenticatedUsers",
+                },
+                "Permission": "WRITE",
+            },
+        ],
+    },
+}
+
 GET_ENCRYPTION = {
     'bucket': 'bucket-1',
     'default_encryption': True,

tests/integration/cartography/intel/aws/test_s3_acls.py

@@ -0,0 +1,51 @@
+from cartography.intel.aws.s3 import _load_s3_acls
+from cartography.intel.aws.s3 import load_s3_buckets
+from cartography.intel.aws.s3 import parse_acl
+from tests.data.aws.s3 import LIST_BUCKETS
+from tests.data.aws.s3 import OPEN_BUCKET_ACLS
+from tests.integration.cartography.intel.aws.iam.test_iam import _create_base_account
+from tests.integration.util import check_nodes
+
+TEST_ACCOUNT_ID = '000000000000'
+TEST_REGION = 'us-east-1'
+TEST_UPDATE_TAG = 123456789
+
+
+def test_load_s3_acls(neo4j_session):
+    """
+    Ensure that the S3 ACL that sets anonymous_access and anonymous_actions works.
+    """
+    # Arrange: hackily add s3 data to the test graph
+    _create_base_account(neo4j_session)
+    load_s3_buckets(neo4j_session, LIST_BUCKETS, TEST_ACCOUNT_ID, TEST_UPDATE_TAG)
+    parsed_acls = []
+    for bucket_name, acl in OPEN_BUCKET_ACLS.items():
+        acl = parse_acl(acl, bucket_name, TEST_ACCOUNT_ID)
+        if acl:
+            parsed_acls.extend(acl)
+
+    # Act: run the analysis job
+    _load_s3_acls(neo4j_session, parsed_acls, TEST_ACCOUNT_ID, TEST_UPDATE_TAG)
+
+    # Assert that the anonymous_access field is set as expected
+    assert check_nodes(neo4j_session, 'S3Bucket', ['name', 'anonymous_access']) == {
+        ('bucket-1', None),
+        ('bucket-3', True),
+        ('bucket-2', True),
+    }
+
+    # Assert that we properly set anonymous_actions on the S3Bucket based on the attached S3Acls
+    # (Can't use check_nodes() here because anonymous_actions is a list and is non-hashable.)
+    actual = neo4j_session.run(
+        """
+        MATCH (r:S3Bucket) RETURN r.name, r.anonymous_actions;
+        """,
+    )
+    actual_nodes = [
+        (n['r.name'], sorted(n['r.anonymous_actions']) if n['r.anonymous_actions'] else []) for n in actual
+    ]
+    assert sorted(actual_nodes) == [
+        ('bucket-1', []),
+        ('bucket-2', ['s3:GetBucketAcl', 's3:ListBucket', 's3:ListBucketMultipartUploads', 's3:ListBucketVersions']),
+        ('bucket-3', ['s3:PutBucketAcl', 's3:PutObject']),
+    ]