From 74e37307f70e29a20ab96fb9cfa299dfc6ca3dc3 Mon Sep 17 00:00:00 2001 From: John Mastron <14130495+mtronrd@users.noreply.github.com> Date: Fri, 10 Nov 2023 00:33:18 -0800 Subject: [PATCH] fix(SQS): fix invalid SQS ARNs (#3016) Co-authored-by: John Mastron --- .../providers/aws/services/sqs/sqs_service.py | 12 +++- ...sqs_queues_not_publicly_accessible_test.py | 66 +++++++++++-------- ...ues_server_side_encryption_enabled_test.py | 25 ++++--- .../aws/services/sqs/sqs_service_test.py | 16 +++++ 4 files changed, 78 insertions(+), 41 deletions(-) diff --git a/prowler/providers/aws/services/sqs/sqs_service.py b/prowler/providers/aws/services/sqs/sqs_service.py index 924f1dae..bdc3e6a9 100644 --- a/prowler/providers/aws/services/sqs/sqs_service.py +++ b/prowler/providers/aws/services/sqs/sqs_service.py @@ -23,16 +23,23 @@ class SQS(AWSService): logger.info("SQS - describing queues...") try: list_queues_paginator = regional_client.get_paginator("list_queues") - for page in list_queues_paginator.paginate(): + # The SQS API uses nonstandard pagination + # you must specify a PageSize if there are more than 1000 queues + for page in list_queues_paginator.paginate( + PaginationConfig={"PageSize": 1000} + ): if "QueueUrls" in page: for queue in page["QueueUrls"]: - arn = f"arn:{self.audited_partition}:sqs:{regional_client.region}:{self.audited_account}:{queue}" + # the queue name is the last path segment of the url + queue_name = queue.split("/")[-1] + arn = f"arn:{self.audited_partition}:sqs:{regional_client.region}:{self.audited_account}:{queue_name}" if not self.audit_resources or ( is_resource_filtered(arn, self.audit_resources) ): self.queues.append( Queue( arn=arn, + name=queue_name, id=queue, region=regional_client.region, ) @@ -122,6 +129,7 @@ class SQS(AWSService): class Queue(BaseModel): id: str + name: str arn: str region: str policy: dict = None 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 e53778f2..cb679d26 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 @@ -7,8 +7,11 @@ from prowler.providers.aws.services.sqs.sqs_service import Queue AWS_REGION = "eu-west-1" AWS_ACCOUNT_NUMBER = "123456789012" -queue_id = str(uuid4()) -topic_arn = f"arn:aws:sqs:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:{queue_id}" +test_queue_name = str(uuid4()) +test_queue_url = ( + f"https://sqs.{AWS_REGION}.amazonaws.com/{AWS_ACCOUNT_NUMBER}/{test_queue_name}" +) +test_queue_arn = f"arn:aws:sqs:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:{test_queue_name}" test_restricted_policy = { "Version": "2012-10-17", @@ -19,7 +22,7 @@ test_restricted_policy = { "Effect": "Allow", "Principal": {"AWS": {AWS_ACCOUNT_NUMBER}}, "Action": "sqs:ReceiveMessage", - "Resource": topic_arn, + "Resource": test_queue_arn, } ], } @@ -33,7 +36,7 @@ test_public_policy = { "Effect": "Allow", "Principal": "*", "Action": "sqs:ReceiveMessage", - "Resource": topic_arn, + "Resource": test_queue_arn, } ], } @@ -47,7 +50,7 @@ test_public_policy_with_condition_same_account_not_valid = { "Effect": "Allow", "Principal": "*", "Action": "sqs:ReceiveMessage", - "Resource": topic_arn, + "Resource": test_queue_arn, "Condition": { "DateGreaterThan": {"aws:CurrentTime": "2009-01-31T12:00Z"}, "DateLessThan": {"aws:CurrentTime": "2009-01-31T15:00Z"}, @@ -65,7 +68,7 @@ test_public_policy_with_condition_same_account = { "Effect": "Allow", "Principal": "*", "Action": "sqs:ReceiveMessage", - "Resource": topic_arn, + "Resource": test_queue_arn, "Condition": { "StringEquals": {"aws:SourceAccount": f"{AWS_ACCOUNT_NUMBER}"} }, @@ -82,7 +85,7 @@ test_public_policy_with_condition_diff_account = { "Effect": "Allow", "Principal": "*", "Action": "sqs:ReceiveMessage", - "Resource": topic_arn, + "Resource": test_queue_arn, "Condition": {"StringEquals": {"aws:SourceAccount": "111122223333"}}, } ], @@ -110,10 +113,11 @@ class Test_sqs_queues_not_publicly_accessible: sqs_client.queues = [] sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, policy=test_restricted_policy, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -129,8 +133,8 @@ class Test_sqs_queues_not_publicly_accessible: assert len(result) == 1 assert result[0].status == "PASS" assert search("is not public", result[0].status_extended) - assert result[0].resource_id == queue_id - assert result[0].resource_arn == "arn_test" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn assert result[0].resource_tags == [] assert result[0].region == AWS_REGION @@ -139,10 +143,11 @@ class Test_sqs_queues_not_publicly_accessible: sqs_client.queues = [] sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, policy=test_public_policy, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -161,8 +166,8 @@ class Test_sqs_queues_not_publicly_accessible: "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" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn assert result[0].resource_tags == [] assert result[0].region == AWS_REGION @@ -172,10 +177,11 @@ class Test_sqs_queues_not_publicly_accessible: sqs_client.audited_account = AWS_ACCOUNT_NUMBER sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, policy=test_public_policy_with_condition_same_account_not_valid, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -194,8 +200,8 @@ class Test_sqs_queues_not_publicly_accessible: "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" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn assert result[0].resource_tags == [] assert result[0].region == AWS_REGION @@ -205,10 +211,11 @@ class Test_sqs_queues_not_publicly_accessible: sqs_client.audited_account = AWS_ACCOUNT_NUMBER sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, policy=test_public_policy_with_condition_same_account, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -225,10 +232,10 @@ class Test_sqs_queues_not_publicly_accessible: 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." + == f"SQS queue {test_queue_url} 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" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn assert result[0].resource_tags == [] assert result[0].region == AWS_REGION @@ -238,10 +245,11 @@ class Test_sqs_queues_not_publicly_accessible: sqs_client.audited_account = AWS_ACCOUNT_NUMBER sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, policy=test_public_policy_with_condition_diff_account, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -258,9 +266,9 @@ class Test_sqs_queues_not_publicly_accessible: assert result[0].status == "FAIL" assert ( result[0].status_extended - == f"SQS queue {queue_id} is public because its policy allows public access, and the condition does not limit access to resources within the same account." + == f"SQS queue {test_queue_url} is public because its policy allows public access, and the condition does not limit access to resources within the same account." ) - assert result[0].resource_id == queue_id - assert result[0].resource_arn == "arn_test" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn assert result[0].resource_tags == [] assert result[0].region == AWS_REGION diff --git a/tests/providers/aws/services/sqs/sqs_queues_server_side_encryption_enabled/sqs_queues_server_side_encryption_enabled_test.py b/tests/providers/aws/services/sqs/sqs_queues_server_side_encryption_enabled/sqs_queues_server_side_encryption_enabled_test.py index ff1f8808..7afbb48a 100644 --- a/tests/providers/aws/services/sqs/sqs_queues_server_side_encryption_enabled/sqs_queues_server_side_encryption_enabled_test.py +++ b/tests/providers/aws/services/sqs/sqs_queues_server_side_encryption_enabled/sqs_queues_server_side_encryption_enabled_test.py @@ -8,8 +8,11 @@ AWS_REGION = "eu-west-1" AWS_ACCOUNT_NUMBER = "123456789012" test_kms_key_id = str(uuid4()) -queue_id = str(uuid4()) -topic_arn = f"arn:aws:sqs:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:{queue_id}" +test_queue_name = str(uuid4()) +test_queue_url = ( + f"https://sqs.{AWS_REGION}.amazonaws.com/{AWS_ACCOUNT_NUMBER}/{test_queue_name}" +) +test_queue_arn = f"arn:aws:sqs:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:{test_queue_name}" class Test_sqs_queues_server_side_encryption_enabled: @@ -33,10 +36,11 @@ class Test_sqs_queues_server_side_encryption_enabled: sqs_client.queues = [] sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, kms_key_id=test_kms_key_id, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -52,17 +56,18 @@ class Test_sqs_queues_server_side_encryption_enabled: assert len(result) == 1 assert result[0].status == "PASS" assert search("is using Server Side Encryption", result[0].status_extended) - assert result[0].resource_id == queue_id - assert result[0].resource_arn == "arn_test" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn def test_queues_no_encryption(self): sqs_client = mock.MagicMock sqs_client.queues = [] sqs_client.queues.append( Queue( - id=queue_id, + id=test_queue_url, + name=test_queue_name, region=AWS_REGION, - arn="arn_test", + arn=test_queue_arn, ) ) with mock.patch( @@ -80,5 +85,5 @@ class Test_sqs_queues_server_side_encryption_enabled: assert search( "is not using Server Side Encryption", result[0].status_extended ) - assert result[0].resource_id == queue_id - assert result[0].resource_arn == "arn_test" + assert result[0].resource_id == test_queue_url + assert result[0].resource_arn == test_queue_arn diff --git a/tests/providers/aws/services/sqs/sqs_service_test.py b/tests/providers/aws/services/sqs/sqs_service_test.py index c3a71dc5..72779a56 100644 --- a/tests/providers/aws/services/sqs/sqs_service_test.py +++ b/tests/providers/aws/services/sqs/sqs_service_test.py @@ -110,9 +110,25 @@ class Test_SQS_Service: sqs = SQS(audit_info) assert len(sqs.queues) == 1 assert sqs.queues[0].id == queue["QueueUrl"] + assert sqs.queues[0].name == test_queue + assert sqs.queues[0].name == sqs.queues[0].arn.split(":")[-1] + assert sqs.queues[0].name == sqs.queues[0].id.split("/")[-1] + assert sqs.queues[0].arn == test_queue_arn assert sqs.queues[0].region == AWS_REGION assert sqs.queues[0].tags == [{"test": "test"}] + # moto does not properly mock this and is hardcoded to return 1000 queues + # so this test currently always fails + # @mock_sqs + # # Test SQS list queues for over 1000 queues + # def test__list_queues__pagination_over_a_thousand(self): + # sqs_client = client("sqs", region_name=AWS_REGION) + # for i in range(0,1050): + # sqs_client.create_queue(QueueName=f"{test_queue}-{i}", tags={"test": "test"}) + # audit_info = self.set_mocked_audit_info() + # sqs = SQS(audit_info) + # assert len(sqs.queues) > 1000 + @mock_sqs # Test SQS list queues def test__get_queue_attributes__(self):