From 1045ffe48932ec8e9fa24ad3cc3f18dbe69c7f37 Mon Sep 17 00:00:00 2001 From: Prowler Bot Date: Fri, 13 Dec 2024 14:07:57 +0100 Subject: [PATCH] fix(aws): set unique resource IDs (#6192) Co-authored-by: Sergio Garcia --- .../backup_recovery_point_encrypted.py | 6 +-- .../aws/services/backup/backup_service.py | 2 + ...egion_enabled_logging_management_events.py | 2 +- .../route53_dangling_ip_subdomain_takeover.py | 4 +- .../backup_recovery_point_encrypted_test.py | 53 +++++++++++-------- ...e53_dangling_ip_subdomain_takeover_test.py | 25 +++++++-- 6 files changed, 60 insertions(+), 32 deletions(-) diff --git a/prowler/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted.py b/prowler/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted.py index dd41668217f..7dadc609e5f 100644 --- a/prowler/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted.py +++ b/prowler/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted.py @@ -8,14 +8,14 @@ def execute(self): for recovery_point in backup_client.recovery_points: report = Check_Report_AWS(self.metadata()) report.region = recovery_point.backup_vault_region - report.resource_id = recovery_point.backup_vault_name + report.resource_id = recovery_point.id report.resource_arn = recovery_point.arn report.resource_tags = recovery_point.tags report.status = "FAIL" - report.status_extended = f"Backup Recovery Point {recovery_point.arn} for Backup Vault {recovery_point.backup_vault_name} is not encrypted at rest." + report.status_extended = f"Backup Recovery Point {recovery_point.id} for Backup Vault {recovery_point.backup_vault_name} is not encrypted at rest." if recovery_point.encrypted: report.status = "PASS" - report.status_extended = f"Backup Recovery Point {recovery_point.arn} for Backup Vault {recovery_point.backup_vault_name} is encrypted at rest." + report.status_extended = f"Backup Recovery Point {recovery_point.id} for Backup Vault {recovery_point.backup_vault_name} is encrypted at rest." findings.append(report) diff --git a/prowler/providers/aws/services/backup/backup_service.py b/prowler/providers/aws/services/backup/backup_service.py index 8690ae4ecc2..5ac0095ce69 100644 --- a/prowler/providers/aws/services/backup/backup_service.py +++ b/prowler/providers/aws/services/backup/backup_service.py @@ -195,6 +195,7 @@ def _list_recovery_points(self, regional_client): self.recovery_points.append( RecoveryPoint( arn=arn, + id=arn.split(":")[-1], backup_vault_name=backup_vault.name, encrypted=recovery_point.get( "IsEncrypted", False @@ -246,6 +247,7 @@ class BackupReportPlan(BaseModel): class RecoveryPoint(BaseModel): arn: str + id: str backup_vault_name: str encrypted: bool backup_vault_region: str diff --git a/prowler/providers/aws/services/cloudtrail/cloudtrail_multi_region_enabled_logging_management_events/cloudtrail_multi_region_enabled_logging_management_events.py b/prowler/providers/aws/services/cloudtrail/cloudtrail_multi_region_enabled_logging_management_events/cloudtrail_multi_region_enabled_logging_management_events.py index 0f95378d409..86f76cdfcba 100644 --- a/prowler/providers/aws/services/cloudtrail/cloudtrail_multi_region_enabled_logging_management_events/cloudtrail_multi_region_enabled_logging_management_events.py +++ b/prowler/providers/aws/services/cloudtrail/cloudtrail_multi_region_enabled_logging_management_events/cloudtrail_multi_region_enabled_logging_management_events.py @@ -48,7 +48,7 @@ def execute(self): report.resource_id = trail.name report.resource_arn = trail.arn report.resource_tags = trail.tags - report.region = trail.home_region + report.region = region report.status = "PASS" if trail.is_multiregion: report.status_extended = f"Trail {trail.name} from home region {trail.home_region} is multi-region, is logging and have management events enabled." diff --git a/prowler/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover.py b/prowler/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover.py index daa54e55627..e8e6f60e2fd 100644 --- a/prowler/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover.py +++ b/prowler/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover.py @@ -29,7 +29,9 @@ def execute(self) -> Check_Report_AWS: # Check if record is an IP Address if validate_ip_address(record): report = Check_Report_AWS(self.metadata()) - report.resource_id = f"{record_set.hosted_zone_id}/{record}" + report.resource_id = ( + f"{record_set.hosted_zone_id}/{record_set.name}/{record}" + ) report.resource_arn = route53_client.hosted_zones[ record_set.hosted_zone_id ].arn diff --git a/tests/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted_test.py b/tests/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted_test.py index dd49821133e..029a4826c04 100644 --- a/tests/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted_test.py +++ b/tests/providers/aws/services/backup/backup_recovery_point_encrypted/backup_recovery_point_encrypted_test.py @@ -94,12 +94,15 @@ def test_no_backup_recovery_points(self): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", - new=Backup(aws_provider), + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", + new=Backup(aws_provider), + ), ): # Test Check from prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted import ( @@ -124,12 +127,15 @@ def test_backup_recovery_points_not_encrypted(self): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", - new=Backup(aws_provider), + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", + new=Backup(aws_provider), + ), ): # Test Check from prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted import ( @@ -142,9 +148,9 @@ def test_backup_recovery_points_not_encrypted(self): assert len(result) == 1 assert result[0].status == "FAIL" assert result[0].status_extended == ( - "Backup Recovery Point arn:aws:backup:eu-west-1:123456789012:recovery-point:1 for Backup Vault Test Vault is not encrypted at rest." + "Backup Recovery Point 1 for Backup Vault Test Vault is not encrypted at rest." ) - assert result[0].resource_id == "Test Vault" + assert result[0].resource_id == "1" assert ( result[0].resource_arn == "arn:aws:backup:eu-west-1:123456789012:recovery-point:1" @@ -165,12 +171,15 @@ def test_backup_recovery_points_encrypted(self): aws_provider = set_mocked_aws_provider([AWS_REGION_EU_WEST_1]) - with mock.patch( - "prowler.providers.common.provider.Provider.get_global_provider", - return_value=aws_provider, - ), mock.patch( - "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", - new=Backup(aws_provider), + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), + mock.patch( + "prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted.backup_client", + new=Backup(aws_provider), + ), ): # Test Check from prowler.providers.aws.services.backup.backup_recovery_point_encrypted.backup_recovery_point_encrypted import ( @@ -183,9 +192,9 @@ def test_backup_recovery_points_encrypted(self): assert len(result) == 1 assert result[0].status == "PASS" assert result[0].status_extended == ( - "Backup Recovery Point arn:aws:backup:eu-west-1:123456789012:recovery-point:1 for Backup Vault Test Vault is encrypted at rest." + "Backup Recovery Point 1 for Backup Vault Test Vault is encrypted at rest." ) - assert result[0].resource_id == "Test Vault" + assert result[0].resource_id == "1" assert ( result[0].resource_arn == "arn:aws:backup:eu-west-1:123456789012:recovery-point:1" diff --git a/tests/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover_test.py b/tests/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover_test.py index 97a104be13e..8d05e64a6e5 100644 --- a/tests/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover_test.py +++ b/tests/providers/aws/services/route53/route53_dangling_ip_subdomain_takeover/route53_dangling_ip_subdomain_takeover_test.py @@ -131,7 +131,10 @@ def test_hosted_zone_private_record(self): ) assert ( result[0].resource_id - == zone_id.replace("/hostedzone/", "") + "/192.168.1.1" + == zone_id.replace("/hostedzone/", "") + + "/" + + record_set_name + + "/192.168.1.1" ) assert ( result[0].resource_arn @@ -196,7 +199,10 @@ def test_hosted_zone_external_record(self): ) assert ( result[0].resource_id - == zone_id.replace("/hostedzone/", "") + "/17.5.7.3" + == zone_id.replace("/hostedzone/", "") + + "/" + + record_set_name + + "/17.5.7.3" ) assert ( result[0].resource_arn @@ -261,7 +267,10 @@ def test_hosted_zone_dangling_public_record(self): ) assert ( result[0].resource_id - == zone_id.replace("/hostedzone/", "") + "/54.152.12.70" + == zone_id.replace("/hostedzone/", "") + + "/" + + record_set_name + + "/54.152.12.70" ) assert ( result[0].resource_arn @@ -330,7 +339,10 @@ def test_hosted_zone_eip_record(self): ) assert ( result[0].resource_id - == zone_id.replace("/hostedzone/", "") + "/17.5.7.3" + == zone_id.replace("/hostedzone/", "") + + "/" + + record_set_name + + "/17.5.7.3" ) assert ( result[0].resource_arn @@ -405,7 +417,10 @@ def test_hosted_zone_eni_record(self): ) assert ( result[0].resource_id - == zone_id.replace("/hostedzone/", "") + "/17.5.7.3" + == zone_id.replace("/hostedzone/", "") + + "/" + + record_set_name + + "/17.5.7.3" ) assert ( result[0].resource_arn