diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ba78ed62..21f2ccb0 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/poetry.lock b/poetry.lock index c0814ef7..d40eb8f2 100644 --- a/poetry.lock +++ b/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"}, diff --git a/prowler/providers/aws/lib/policy_condition_parser/__init__.py b/prowler/providers/aws/lib/policy_condition_parser/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py b/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py new file mode 100644 index 00000000..9c9f9cc0 --- /dev/null +++ b/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 diff --git a/prowler/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible.py b/prowler/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible.py index 5d4e7159..38a4e185 100644 --- a/prowler/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible.py +++ b/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 diff --git a/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py b/tests/providers/aws/lib/policy_condition_parser/policy_condition_parser_test.py new file mode 100644 index 00000000..60f20b46 --- /dev/null +++ b/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 + ) diff --git a/tests/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible_test.py b/tests/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible_test.py index e163ccdc..0e1ff6b6 100644 --- a/tests/providers/aws/services/sqs/sqs_queues_not_publicly_accessible/sqs_queues_not_publicly_accessible_test.py +++ b/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"