feat(cond parser): add policy cond parser & apply in sqs public check (#2575)

.pre-commit-config.yaml

@@ -57,7 +57,7 @@ repos:
         args: ["--ignore=E266,W503,E203,E501,W605"]
 
   - repo: https://github.com/python-poetry/poetry
-    rev: 1.4.0 # add version here
+    rev: 1.5.1 # add version here
     hooks:
       - id: poetry-check
       - id: poetry-lock

poetry.lock

@@ -2378,8 +2378,7 @@ files = [
     {file = "ruamel.yaml.clib-0.2.7-cp310-cp310-win32.whl", hash = "sha256:763d65baa3b952479c4e972669f679fe490eee058d5aa85da483ebae2009d231"},
     {file = "ruamel.yaml.clib-0.2.7-cp310-cp310-win_amd64.whl", hash = "sha256:d000f258cf42fec2b1bbf2863c61d7b8918d31ffee905da62dede869254d3b8a"},
     {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-macosx_10_9_universal2.whl", hash = "sha256:045e0626baf1c52e5527bd5db361bc83180faaba2ff586e763d3d5982a876a9e"},
-    {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-macosx_13_0_arm64.whl", hash = "sha256:1a6391a7cabb7641c32517539ca42cf84b87b667bad38b78d4d42dd23e957c81"},
-    {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-manylinux2014_aarch64.whl", hash = "sha256:9c7617df90c1365638916b98cdd9be833d31d337dbcd722485597b43c4a215bf"},
+    {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-macosx_12_6_arm64.whl", hash = "sha256:721bc4ba4525f53f6a611ec0967bdcee61b31df5a56801281027a3a6d1c2daf5"},
     {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.manylinux_2_24_x86_64.whl", hash = "sha256:41d0f1fa4c6830176eef5b276af04c89320ea616655d01327d5ce65e50575c94"},
     {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-win32.whl", hash = "sha256:f6d3d39611ac2e4f62c3128a9eed45f19a6608670c5a2f4f07f24e8de3441d38"},
     {file = "ruamel.yaml.clib-0.2.7-cp311-cp311-win_amd64.whl", hash = "sha256:da538167284de58a52109a9b89b8f6a53ff8437dd6dc26d33b57bf6699153122"},

prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py

@@ -0,0 +1,39 @@
+def is_account_only_allowed_in_condition(
+    condition_statement: dict, source_account: str
+):
+    is_condition_valid = False
+    valid_condition_options = {
+        "StringEquals": "aws:SourceAccount",
+        "ArnLike": "aws:SourceArn",
+        "ArnEquals": "aws:SourceArn",
+    }
+    for condition_operator, condition_operator_key in valid_condition_options.items():
+        if condition_operator in condition_statement:
+            if condition_operator_key in condition_statement[condition_operator]:
+                # values are a list
+                if isinstance(
+                    condition_statement[condition_operator][condition_operator_key],
+                    list,
+                ):
+                    # if there is an arn/account without the source account -> we do not consider it safe
+                    # here by default we assume is true and look for false entries
+                    is_condition_valid = True
+                    for item in condition_statement[condition_operator][
+                        condition_operator_key
+                    ]:
+                        if source_account not in item:
+                            is_condition_valid = False
+                            break
+                # value is a string
+                elif isinstance(
+                    condition_statement[condition_operator][condition_operator_key], str
+                ):
+                    if (
+                        source_account
+                        in condition_statement[condition_operator][
+                            condition_operator_key
+                        ]
+                    ):
+                        is_condition_valid = True
+
+    return is_condition_valid

prowler/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible.py

@@ -1,4 +1,7 @@
 from prowler.lib.check.models import Check, Check_Report_AWS
+from prowler.providers.aws.lib.policy_condition_parser.policy_condition_parser import (
+    is_account_only_allowed_in_condition,
+)
 from prowler.providers.aws.services.sqs.sqs_client import sqs_client
 
 
@@ -28,14 +31,16 @@ class sqs_queues_not_publicly_accessible(Check):
                                 and "*" in statement["Principal"]["CanonicalUser"]
                             )
                         ):
-                            if "Condition" not in statement:
-                                report.status = "FAIL"
-                                report.status_extended = (
-                                    f"SQS queue {queue.id} policy with public access"
+                            if (
+                                "Condition" in statement
+                                and is_account_only_allowed_in_condition(
+                                    statement["Condition"], sqs_client.audited_account
                                 )
+                            ):
+                                report.status_extended = f"SQS queue {queue.id} is not public because its policy only allows access from the same account"
                             else:
                                 report.status = "FAIL"
-                                report.status_extended = f"SQS queue {queue.id} policy with public access but has a Condition"
+                                report.status_extended = f"SQS queue {queue.id} is public because its policy allows public access"
             findings.append(report)
 
         return findings

tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py

@@ -0,0 +1,123 @@
+from prowler.providers.aws.lib.policy_condition_parser.policy_condition_parser import (
+    is_account_only_allowed_in_condition,
+)
+
+AWS_ACCOUNT_NUMBER = "123456789012"
+
+
+class Test_policy_condition_parser:
+    def test_condition_parser_string_equals_list(self):
+        condition_statement = {"StringEquals": {"aws:SourceAccount": ["123456789012"]}}
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_string_equals_str(self):
+        condition_statement = {"StringEquals": {"aws:SourceAccount": "123456789012"}}
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_string_equals_list_not_valid(self):
+        condition_statement = {
+            "StringEquals": {"aws:SourceAccount": ["123456789012", "111222333444"]}
+        }
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_string_equals_str_not_valid(self):
+        condition_statement = {"StringEquals": {"aws:SourceAccount": "111222333444"}}
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnlike_list(self):
+        condition_statement = {
+            "ArnLike": {"aws:SourceArn": ["arn:aws:cloudtrail:*:123456789012:trail/*"]}
+        }
+
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnlike_list_not_valid(self):
+        condition_statement = {
+            "ArnLike": {
+                "aws:SourceArn": [
+                    "arn:aws:cloudtrail:*:123456789012:trail/*",
+                    "arn:aws:cloudtrail:*:111222333444:trail/*",
+                ]
+            }
+        }
+
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnlike_str(self):
+        condition_statement = {
+            "ArnLike": {"aws:SourceArn": "arn:aws:cloudtrail:*:123456789012:trail/*"}
+        }
+
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnlike_str_not_valid(self):
+        condition_statement = {
+            "ArnLike": {"aws:SourceArn": "arn:aws:cloudtrail:*:111222333444:trail/*"}
+        }
+
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnequals_list(self):
+        condition_statement = {
+            "ArnEquals": {
+                "aws:SourceArn": [
+                    "arn:aws:cloudtrail:eu-west-1:123456789012:trail/test"
+                ]
+            }
+        }
+
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnequals_list_not_valid(self):
+        condition_statement = {
+            "ArnEquals": {
+                "aws:SourceArn": [
+                    "arn:aws:cloudtrail:eu-west-1:123456789012:trail/test",
+                    "arn:aws:cloudtrail:eu-west-1:111222333444:trail/test",
+                ]
+            }
+        }
+
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnequals_str(self):
+        condition_statement = {
+            "ArnEquals": {
+                "aws:SourceArn": "arn:aws:cloudtrail:eu-west-1:123456789012:trail/test"
+            }
+        }
+
+        assert is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )
+
+    def test_condition_parser_arnequals_str_not_valid(self):
+        condition_statement = {
+            "ArnEquals": {
+                "aws:SourceArn": "arn:aws:cloudtrail:eu-west-1:111222333444:trail/test"
+            }
+        }
+
+        assert not is_account_only_allowed_in_condition(
+            condition_statement, AWS_ACCOUNT_NUMBER
+        )

tests/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible_test.py

@@ -38,7 +38,7 @@ test_public_policy = {
     ],
 }
 
-test_public_policy_with_condition = {
+test_public_policy_with_condition_not_valid = {
     "Version": "2012-10-17",
     "Id": "Queue1_Policy_UUID",
     "Statement": [
@@ -56,6 +56,21 @@ test_public_policy_with_condition = {
     ],
 }
 
+test_public_policy_with_condition = {
+    "Version": "2012-10-17",
+    "Id": "Queue1_Policy_UUID",
+    "Statement": [
+        {
+            "Sid": "Queue1_AnonymousAccess_ReceiveMessage",
+            "Effect": "Allow",
+            "Principal": "*",
+            "Action": "sqs:ReceiveMessage",
+            "Resource": topic_arn,
+            "Condition": {"StringEquals": {"aws:SourceAccount": "123456789012"}},
+        }
+    ],
+}
+
 
 class Test_sqs_queues_not_publicly_accessible:
     def test_no_queues(self):
@@ -123,13 +138,48 @@ class Test_sqs_queues_not_publicly_accessible:
             result = check.execute()
             assert len(result) == 1
             assert result[0].status == "FAIL"
-            assert search("policy with public access", result[0].status_extended)
+            assert search(
+                "is public because its policy allows public access",
+                result[0].status_extended,
+            )
             assert result[0].resource_id == queue_id
             assert result[0].resource_arn == "arn_test"
 
-    def test_queues_public_with_condition(self):
+    def test_queues_public_with_condition_not_valid(self):
         sqs_client = mock.MagicMock
         sqs_client.queues = []
+        sqs_client.audited_account = AWS_ACCOUNT_NUMBER
+        sqs_client.queues.append(
+            Queue(
+                id=queue_id,
+                region=AWS_REGION,
+                policy=test_public_policy_with_condition_not_valid,
+                arn="arn_test",
+            )
+        )
+        with mock.patch(
+            "prowler.providers.aws.services.sqs.sqs_service.SQS",
+            sqs_client,
+        ):
+            from prowler.providers.aws.services.sqs.sqs_queues_not_publicly_accessible.sqs_queues_not_publicly_accessible import (
+                sqs_queues_not_publicly_accessible,
+            )
+
+            check = sqs_queues_not_publicly_accessible()
+            result = check.execute()
+            assert len(result) == 1
+            assert result[0].status == "FAIL"
+            assert search(
+                "is public because its policy allows public access",
+                result[0].status_extended,
+            )
+            assert result[0].resource_id == queue_id
+            assert result[0].resource_arn == "arn_test"
+
+    def test_queues_public_with_condition_valid(self):
+        sqs_client = mock.MagicMock
+        sqs_client.queues = []
+        sqs_client.audited_account = AWS_ACCOUNT_NUMBER
         sqs_client.queues.append(
             Queue(
                 id=queue_id,
@@ -149,10 +199,10 @@ class Test_sqs_queues_not_publicly_accessible:
             check = sqs_queues_not_publicly_accessible()
             result = check.execute()
             assert len(result) == 1
-            assert result[0].status == "FAIL"
-            assert search(
-                "policy with public access but has a Condition",
-                result[0].status_extended,
+            assert result[0].status == "PASS"
+            assert (
+                result[0].status_extended
+                == f"SQS queue {queue_id} is not public because its policy only allows access from the same account"
             )
             assert result[0].resource_id == queue_id
             assert result[0].resource_arn == "arn_test"