From 9147a45e2f29f9246f2ca859f22daeba9b1a2337 Mon Sep 17 00:00:00 2001 From: John Mastron <14130495+mtronrd@users.noreply.github.com> Date: Wed, 19 Jun 2024 01:21:50 -0700 Subject: [PATCH] fix(aws): aws check and metadata fixes (#4251) Co-authored-by: John Mastron Co-authored-by: Pepe Fagoaga --- ..._ebs_volume_snapshots_exists.metadata.json | 4 +- .../ec2_instance_managed_by_ssm.py | 30 +-- ...s_encryption_at_rest_enabled.metadata.json | 2 +- ...pics_not_publicly_accessible.metadata.json | 2 +- .../providers/aws/services/ssm/ssm_service.py | 5 + .../ec2_instance_managed_by_ssm_test.py | 174 ++++++++++++++++++ 6 files changed, 199 insertions(+), 18 deletions(-) diff --git a/prowler/providers/aws/services/ec2/ec2_ebs_volume_snapshots_exists/ec2_ebs_volume_snapshots_exists.metadata.json b/prowler/providers/aws/services/ec2/ec2_ebs_volume_snapshots_exists/ec2_ebs_volume_snapshots_exists.metadata.json index 26325e1210..fa8e660aa8 100644 --- a/prowler/providers/aws/services/ec2/ec2_ebs_volume_snapshots_exists/ec2_ebs_volume_snapshots_exists.metadata.json +++ b/prowler/providers/aws/services/ec2/ec2_ebs_volume_snapshots_exists/ec2_ebs_volume_snapshots_exists.metadata.json @@ -6,10 +6,10 @@ "Data Protection" ], "ServiceName": "ec2", - "SubServiceName": "snapshot", + "SubServiceName": "volume", "ResourceIdTemplate": "arn:partition:service:region:account-id:resource-id", "Severity": "medium", - "ResourceType": "AwsEc2Snapshot", + "ResourceType": "AwsEc2Volume", "Description": "Check if EBS snapshots exists.", "Risk": "Ensure that your EBS volumes (available or in-use) have recent snapshots (taken weekly) available for point-in-time recovery for a better, more reliable data backup strategy.", "RelatedUrl": "https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSSnapshots.html", diff --git a/prowler/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm.py b/prowler/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm.py index 69af056edd..f35578ef6e 100644 --- a/prowler/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm.py +++ b/prowler/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm.py @@ -7,21 +7,23 @@ class ec2_instance_managed_by_ssm(Check): def execute(self): findings = [] for instance in ec2_client.instances: - if instance.state != "terminated": - report = Check_Report_AWS(self.metadata()) - report.region = instance.region - report.resource_arn = instance.arn - report.resource_tags = instance.tags - report.status = "PASS" + report = Check_Report_AWS(self.metadata()) + report.region = instance.region + report.resource_arn = instance.arn + report.resource_tags = instance.tags + report.status = "PASS" + report.status_extended = ( + f"EC2 Instance {instance.id} is managed by Systems Manager." + ) + report.resource_id = instance.id + # instances not running should pass the check + if instance.state in ["pending", "terminated", "stopped"]: + report.status_extended = f"EC2 Instance {instance.id} is unmanaged by Systems Manager because it is {instance.state}." + elif not ssm_client.managed_instances.get(instance.id): + report.status = "FAIL" report.status_extended = ( - f"EC2 Instance {instance.id} is managed by Systems Manager." + f"EC2 Instance {instance.id} is not managed by Systems Manager." ) - report.resource_id = instance.id - if not ssm_client.managed_instances.get(instance.id): - report.status = "FAIL" - report.status_extended = ( - f"EC2 Instance {instance.id} is not managed by Systems Manager." - ) - findings.append(report) + findings.append(report) return findings diff --git a/prowler/providers/aws/services/sns/sns_topics_kms_encryption_at_rest_enabled/sns_topics_kms_encryption_at_rest_enabled.metadata.json b/prowler/providers/aws/services/sns/sns_topics_kms_encryption_at_rest_enabled/sns_topics_kms_encryption_at_rest_enabled.metadata.json index ad1e000a93..0826f89219 100644 --- a/prowler/providers/aws/services/sns/sns_topics_kms_encryption_at_rest_enabled/sns_topics_kms_encryption_at_rest_enabled.metadata.json +++ b/prowler/providers/aws/services/sns/sns_topics_kms_encryption_at_rest_enabled/sns_topics_kms_encryption_at_rest_enabled.metadata.json @@ -7,7 +7,7 @@ "SubServiceName": "", "ResourceIdTemplate": "arn:aws:sns:region:account-id:topic", "Severity": "high", - "ResourceType": "AwsSNSTopic", + "ResourceType": "AwsSnsTopic", "Description": "Ensure there are no SNS Topics unencrypted", "Risk": "If not enabled sensitive information at rest is not protected.", "RelatedUrl": "https://docs.aws.amazon.com/sns/latest/dg/sns-server-side-encryption.html", diff --git a/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.metadata.json b/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.metadata.json index 7572ab4f4c..d6d7ad3a3f 100644 --- a/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.metadata.json +++ b/prowler/providers/aws/services/sns/sns_topics_not_publicly_accessible/sns_topics_not_publicly_accessible.metadata.json @@ -7,7 +7,7 @@ "SubServiceName": "", "ResourceIdTemplate": "arn:aws:sns:region:account-id:topic", "Severity": "high", - "ResourceType": "AwsSNSTopic", + "ResourceType": "AwsSnsTopic", "Description": "Check if SNS topics have policy set as Public", "Risk": "Publicly accessible services could expose sensitive data to bad actors.", "RelatedUrl": "https://docs.aws.amazon.com/config/latest/developerguide/sns-topic-policy.html", diff --git a/prowler/providers/aws/services/ssm/ssm_service.py b/prowler/providers/aws/services/ssm/ssm_service.py index 7c50c1ab89..015ce81ed3 100644 --- a/prowler/providers/aws/services/ssm/ssm_service.py +++ b/prowler/providers/aws/services/ssm/ssm_service.py @@ -1,4 +1,5 @@ import json +import time from enum import Enum from typing import Optional @@ -145,6 +146,10 @@ class SSM(AWSService): id=resource_id, region=regional_client.region, ) + # boto3 does not properly handle throttling exceptions for + # ssm:DescribeInstanceInformation when there are large numbers of instances + # AWS support recommends manually reducing frequency of requests + time.sleep(0.1) except Exception as error: logger.error( diff --git a/tests/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm_test.py b/tests/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm_test.py index d6b3ce0c0c..6325d064d7 100644 --- a/tests/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm_test.py +++ b/tests/providers/aws/services/ec2/ec2_instance_managed_by_ssm/ec2_instance_managed_by_ssm_test.py @@ -157,3 +157,177 @@ class Test_ec2_instance_managed_by_ssm_test: == f"EC2 Instance {instance.id} is managed by Systems Manager." ) assert result[0].resource_id == instance.id + + @mock_aws + def test_ec2_instance_managed_by_ssm_running(self): + ec2 = resource("ec2", region_name=AWS_REGION_US_EAST_1) + instances_pending = ec2.create_instances( + ImageId=EXAMPLE_AMI_ID, + MinCount=2, + MaxCount=2, + UserData="This is some user_data", + ) + instance_managed = ec2.Instance(instances_pending[0].id) + instance_unmanaged = ec2.Instance(instances_pending[1].id) + assert instance_managed.state["Name"] == "running" + assert instance_unmanaged.state["Name"] == "running" + + ssm_client = mock.MagicMock + ssm_client.managed_instances = { + instance_managed.id: ManagedInstance( + arn=f"arn:aws:ec2:{AWS_REGION_US_EAST_1}:{AWS_ACCOUNT_NUMBER}:instance/{instance_managed.id}", + id=instance_managed.id, + region=AWS_REGION_US_EAST_1, + ) + } + + from prowler.providers.aws.services.ec2.ec2_service import EC2 + + aws_provider = set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_service.SSM", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_client.ssm_client", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm.ec2_client", + new=EC2(aws_provider), + ): + # Test Check + from prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm import ( + ec2_instance_managed_by_ssm, + ) + + check = ec2_instance_managed_by_ssm() + results = check.execute() + + assert len(results) == 2 + for result in results: + if result.resource_id == instance_managed.id: + assert result.status == "PASS" + assert result.region == AWS_REGION_US_EAST_1 + assert result.resource_tags is None + assert ( + result.status_extended + == f"EC2 Instance {instance_managed.id} is managed by Systems Manager." + ) + + if result.resource_id == instance_unmanaged.id: + assert result.status == "FAIL" + assert result.region == AWS_REGION_US_EAST_1 + assert result.resource_tags is None + assert ( + result.status_extended + == f"EC2 Instance {instance_unmanaged.id} is not managed by Systems Manager." + ) + + @mock_aws + def test_ec2_instance_managed_by_ssm_stopped(self): + ec2 = resource("ec2", region_name=AWS_REGION_US_EAST_1) + instances_pending = ec2.create_instances( + ImageId=EXAMPLE_AMI_ID, + MinCount=1, + MaxCount=1, + UserData="This is some user_data", + ) + instances_pending[0].stop() + instance = ec2.Instance(instances_pending[0].id) + assert instance.state["Name"] == "stopped" + + ssm_client = mock.MagicMock + ssm_client.managed_instances = {} + + from prowler.providers.aws.services.ec2.ec2_service import EC2 + + aws_provider = set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_service.SSM", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_client.ssm_client", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm.ec2_client", + new=EC2(aws_provider), + ): + # Test Check + from prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm import ( + ec2_instance_managed_by_ssm, + ) + + check = ec2_instance_managed_by_ssm() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].region == AWS_REGION_US_EAST_1 + assert result[0].resource_tags is None + assert ( + result[0].status_extended + == f"EC2 Instance {instance.id} is unmanaged by Systems Manager because it is stopped." + ) + + @mock_aws + def test_ec2_instance_managed_by_ssm_terminated(self): + ec2 = resource("ec2", region_name=AWS_REGION_US_EAST_1) + instances_pending = ec2.create_instances( + ImageId=EXAMPLE_AMI_ID, + MinCount=1, + MaxCount=1, + UserData="This is some user_data", + ) + instances_pending[0].terminate() + instance = ec2.Instance(instances_pending[0].id) + assert instance.state["Name"] == "terminated" + + ssm_client = mock.MagicMock + ssm_client.managed_instances = {} + + from prowler.providers.aws.services.ec2.ec2_service import EC2 + + aws_provider = set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ) + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=aws_provider, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_service.SSM", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ssm.ssm_client.ssm_client", + new=ssm_client, + ), mock.patch( + "prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm.ec2_client", + new=EC2(aws_provider), + ): + # Test Check + from prowler.providers.aws.services.ec2.ec2_instance_managed_by_ssm.ec2_instance_managed_by_ssm import ( + ec2_instance_managed_by_ssm, + ) + + check = ec2_instance_managed_by_ssm() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].region == AWS_REGION_US_EAST_1 + assert result[0].resource_tags is None + assert ( + result[0].status_extended + == f"EC2 Instance {instance.id} is unmanaged by Systems Manager because it is terminated." + )