From f36d23c9a70a819d44f0368736cbecfccf1f6c55 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Lopez <101330800+MarioRgzLpz@users.noreply.github.com> Date: Tue, 22 Oct 2024 20:08:13 +0200 Subject: [PATCH] feat(elb): add new check `elb_desync_mitigation_mode` (#5500) Co-authored-by: Sergio --- .../elb_desync_mitigation_mode/__init__.py | 0 .../elb_desync_mitigation_mode.metadata.json | 32 +++ .../elb_desync_mitigation_mode.py | 26 ++ .../providers/aws/services/elb/elb_service.py | 11 +- ...isteners_use_acm_certificate.metadata.json | 4 +- .../elb_ssl_listeners_use_acm_certificate.py | 3 +- .../elbv2_desync_mitigation_mode.py | 9 +- .../elb_desync_mitigation_mode_test.py | 272 ++++++++++++++++++ .../aws/services/elb/elb_service_test.py | 1 + .../elbv2_desync_mitigation_mode_test.py | 4 +- 10 files changed, 351 insertions(+), 11 deletions(-) create mode 100644 prowler/providers/aws/services/elb/elb_desync_mitigation_mode/__init__.py create mode 100644 prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.metadata.json create mode 100644 prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.py create mode 100644 tests/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode_test.py diff --git a/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/__init__.py b/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.metadata.json b/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.metadata.json new file mode 100644 index 0000000000..f6d873fbd6 --- /dev/null +++ b/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.metadata.json @@ -0,0 +1,32 @@ +{ + "Provider": "aws", + "CheckID": "elb_desync_mitigation_mode", + "CheckTitle": "Classic Load Balancer should be configured with defensive or strictest desync mitigation mode", + "CheckType": [ + "Software and Configuration Checks/AWS Security Best Practices" + ], + "ServiceName": "elb", + "SubServiceName": "", + "ResourceIdTemplate": "arn:aws:elasticloadbalancing:{region}:{account-id}:loadbalancer/{load-balancer-name}", + "Severity": "medium", + "ResourceType": "AwsElbLoadBalancer", + "Description": "This control checks whether a Classic Load Balancer is configured with defensive or strictest desync mitigation mode. The control fails if the Classic Load Balancer isn't configured with defensive or strictest desync mitigation mode.", + "Risk": "HTTP Desync issues can lead to request smuggling, making applications vulnerable to attacks such as request queue or cache poisoning, which could result in credential hijacking or unauthorized command execution.", + "RelatedUrl": "https://docs.aws.amazon.com/config/latest/developerguide/clb-desync-mode-check.html", + "Remediation": { + "Code": { + "CLI": "aws elb modify-load-balancer-attributes --load-balancer-name --load-balancer-attributes '{\"DesyncMitigationMode\":\"defensive\"}'", + "NativeIaC": "", + "Other": "https://docs.aws.amazon.com/securityhub/latest/userguide/elb-controls.html#elb-14", + "Terraform": "" + }, + "Recommendation": { + "Text": "Configure the Classic Load Balancer with defensive or strictest desync mitigation mode to prevent security issues caused by HTTP desync.", + "Url": "https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/config-desync-mitigation-mode.html#update-desync-mitigation-mode" + } + }, + "Categories": [], + "DependsOn": [], + "RelatedTo": [], + "Notes": "" +} diff --git a/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.py b/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.py new file mode 100644 index 0000000000..4897a73cda --- /dev/null +++ b/prowler/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode.py @@ -0,0 +1,26 @@ +from prowler.lib.check.models import Check, Check_Report_AWS +from prowler.providers.aws.services.elb.elb_client import elb_client + + +class elb_desync_mitigation_mode(Check): + def execute(self): + findings = [] + for lb_arn, lb in elb_client.loadbalancers.items(): + report = Check_Report_AWS(self.metadata()) + report.region = lb.region + report.resource_id = lb.name + report.resource_arn = lb_arn + report.resource_tags = lb.tags + if ( + lb.desync_mitigation_mode == "defensive" + or lb.desync_mitigation_mode == "strictest" + ): + report.status = "PASS" + report.status_extended = f"ELB {lb.name} has desync mitigation mode set to {lb.desync_mitigation_mode}." + else: + report.status = "FAIL" + report.status_extended = f"ELB {lb.name} has desync mitigation mode set to {lb.desync_mitigation_mode}, not to strictest or defensive." + + findings.append(report) + + return findings diff --git a/prowler/providers/aws/services/elb/elb_service.py b/prowler/providers/aws/services/elb/elb_service.py index 9de5632c08..5ab828a348 100644 --- a/prowler/providers/aws/services/elb/elb_service.py +++ b/prowler/providers/aws/services/elb/elb_service.py @@ -36,9 +36,9 @@ class ELB(AWSService): Listener( protocol=listener["Listener"]["Protocol"], policies=listener["PolicyNames"], - certificate_arn=listener["Listener"][ - "SSLCertificateId" - ], + certificate_arn=listener["Listener"].get( + "SSLCertificateId", "" + ), ) ) @@ -70,6 +70,10 @@ class ELB(AWSService): load_balancer.connection_draining = attributes.get( "ConnectionDraining", {} ).get("Enabled", False) + additional_attributes = attributes.get("AdditionalAttributes", []) + for attribute in additional_attributes: + if attribute["Key"] == "elb.http.desyncmitigationmode": + load_balancer.desync_mitigation_mode = attribute["Value"] except Exception as error: logger.error( @@ -109,4 +113,5 @@ class LoadBalancer(BaseModel): cross_zone_load_balancing: Optional[bool] availability_zones: set[str] connection_draining: Optional[bool] + desync_mitigation_mode: Optional[str] tags: Optional[list] = [] diff --git a/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.metadata.json b/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.metadata.json index 22a64fc5bf..3dbc823ec5 100644 --- a/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.metadata.json +++ b/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.metadata.json @@ -5,11 +5,11 @@ "CheckType": [ "Software and Configuration Checks/Vulnerabilities/NIST 800-53 Controls (USA)" ], - "ServiceName": "elasticloadbalancing", + "ServiceName": "elb", "SubServiceName": "", "ResourceIdTemplate": "arn:aws:elasticloadbalancing:{region}:{account-id}:loadbalancer/{loadbalancer-name}", "Severity": "medium", - "ResourceType": "AwsElasticLoadBalancingLoadBalancer", + "ResourceType": "AwsElbLoadBalancer", "Description": "This control checks whether the Classic Load Balancer uses HTTPS/SSL certificates provided by AWS Certificate Manager (ACM). The control fails if the Classic Load Balancer does not use a certificate provided by ACM.", "Risk": "If Classic Load Balancers are not using ACM certificates, it increases the risk of using self-signed or expired certificates, which can impact secure communication and lead to compliance issues.", "RelatedUrl": "https://docs.aws.amazon.com/config/latest/developerguide/elb-acm-certificate-required.html", diff --git a/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.py b/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.py index 04ac122d38..5c2acfffee 100644 --- a/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.py +++ b/prowler/providers/aws/services/elb/elb_ssl_listeners_use_acm_certificate/elb_ssl_listeners_use_acm_certificate.py @@ -17,7 +17,8 @@ class elb_ssl_listeners_use_acm_certificate(Check): report.status_extended = f"ELB {lb.name} HTTPS/SSL listeners are using certificates managed by ACM." for listener in lb.listeners: if ( - listener.protocol in secure_protocols + listener.certificate_arn + and listener.protocol in secure_protocols and acm_client.certificates[listener.certificate_arn].type != "AMAZON_ISSUED" ): diff --git a/prowler/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode.py b/prowler/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode.py index 048c73612d..a9ee709a8f 100644 --- a/prowler/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode.py +++ b/prowler/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode.py @@ -14,12 +14,15 @@ class elbv2_desync_mitigation_mode(Check): report.resource_tags = lb.tags report.status = "PASS" report.status_extended = f"ELBv2 ALB {lb.name} is configured with correct desync mitigation mode." - if lb.desync_mitigation_mode != "strictest": + if ( + lb.desync_mitigation_mode != "strictest" + or lb.desync_mitigation_mode != "defensive" + ): if lb.drop_invalid_header_fields == "false": report.status = "FAIL" - report.status_extended = f"ELBv2 ALB {lb.name} does not have desync mitigation mode set as strictest and is not dropping invalid header fields." + report.status_extended = f"ELBv2 ALB {lb.name} does not have desync mitigation mode set as strictest/defensive and is not dropping invalid header fields." elif lb.drop_invalid_header_fields == "true": - report.status_extended = f"ELBv2 ALB {lb.name} does not have desync mitigation mode set as strictest but is dropping invalid header fields." + report.status_extended = f"ELBv2 ALB {lb.name} does not have desync mitigation mode set as strictest/defensive but is dropping invalid header fields." findings.append(report) return findings diff --git a/tests/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode_test.py b/tests/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode_test.py new file mode 100644 index 0000000000..500bdc0dcb --- /dev/null +++ b/tests/providers/aws/services/elb/elb_desync_mitigation_mode/elb_desync_mitigation_mode_test.py @@ -0,0 +1,272 @@ +from unittest import mock + +import botocore +from boto3 import client, resource +from moto import mock_aws + +from tests.providers.aws.utils import ( + AWS_REGION_EU_WEST_1, + AWS_REGION_EU_WEST_1_AZA, + AWS_REGION_US_EAST_1, + set_mocked_aws_provider, +) + +make_api_call = botocore.client.BaseClient._make_api_call + + +def mock_make_api_call(self, operation_name, kwarg): + if operation_name == "DescribeLoadBalancerAttributes": + if kwarg["LoadBalancerName"] == "my-lb-strictest": + return { + "LoadBalancerAttributes": { + "CrossZoneLoadBalancing": {"Enabled": True}, + "AccessLog": { + "Enabled": False, + "EmitInterval": 60, + }, + "ConnectionDraining": {"Enabled": False, "Timeout": 300}, + "ConnectionSettings": { + "IdleTimeout": 60, + }, + "AdditionalAttributes": [ + { + "Key": "elb.http.desyncmitigationmode", + "Value": "strictest", + } + ], + } + } + if kwarg["LoadBalancerName"] == "my-lb-defensive": + return { + "LoadBalancerAttributes": { + "CrossZoneLoadBalancing": {"Enabled": True}, + "AccessLog": { + "Enabled": False, + "EmitInterval": 60, + }, + "ConnectionDraining": {"Enabled": False, "Timeout": 300}, + "ConnectionSettings": { + "IdleTimeout": 60, + }, + "AdditionalAttributes": [ + { + "Key": "elb.http.desyncmitigationmode", + "Value": "defensive", + } + ], + } + } + if kwarg["LoadBalancerName"] == "my-lb-monitor": + return { + "LoadBalancerAttributes": { + "CrossZoneLoadBalancing": {"Enabled": True}, + "AccessLog": { + "Enabled": False, + "EmitInterval": 60, + }, + "ConnectionDraining": {"Enabled": False, "Timeout": 300}, + "ConnectionSettings": { + "IdleTimeout": 60, + }, + "AdditionalAttributes": [ + { + "Key": "elb.http.desyncmitigationmode", + "Value": "monitor", + } + ], + } + } + + return make_api_call(self, operation_name, kwarg) + + +class Test_elb_desync_mitigation_mode: + @mock_aws + def test_elb_no_balancers(self): + from prowler.providers.aws.services.elb.elb_service import ELB + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ), + ), mock.patch( + "prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode.elb_client", + new=ELB( + set_mocked_aws_provider([AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1]) + ), + ): + # Test Check + from prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode import ( + elb_desync_mitigation_mode, + ) + + check = elb_desync_mitigation_mode() + result = check.execute() + + assert len(result) == 0 + + @mock_aws + @mock.patch("botocore.client.BaseClient._make_api_call", new=mock_make_api_call) + def test_elb_with_monitor_desync_mode(self): + elb = client("elb", region_name=AWS_REGION_EU_WEST_1) + ec2 = resource("ec2", region_name=AWS_REGION_EU_WEST_1) + + security_group = ec2.create_security_group( + GroupName="sg01", Description="Test security group sg01" + ) + + elb.create_load_balancer( + LoadBalancerName="my-lb-monitor", + Listeners=[ + {"Protocol": "tcp", "LoadBalancerPort": 80, "InstancePort": 8080}, + {"Protocol": "http", "LoadBalancerPort": 81, "InstancePort": 9000}, + ], + AvailabilityZones=[AWS_REGION_EU_WEST_1_AZA], + Scheme="internal", + SecurityGroups=[security_group.id], + ) + + from prowler.providers.aws.services.elb.elb_service import ELB + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ), + ), mock.patch( + "prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode.elb_client", + new=ELB( + set_mocked_aws_provider([AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1]) + ), + ): + from prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode import ( + elb_desync_mitigation_mode, + ) + + check = elb_desync_mitigation_mode() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "ELB my-lb-monitor has desync mitigation mode set to monitor, not to strictest or defensive." + ) + assert result[0].resource_id == "my-lb-monitor" + assert ( + result[0].resource_arn + == "arn:aws:elasticloadbalancing:eu-west-1:123456789012:loadbalancer/my-lb-monitor" + ) + assert result[0].region == AWS_REGION_EU_WEST_1 + assert result[0].resource_tags == [] + + @mock_aws + @mock.patch("botocore.client.BaseClient._make_api_call", new=mock_make_api_call) + def test_elb_with_defensive_desync_mode(self): + elb = client("elb", region_name=AWS_REGION_EU_WEST_1) + ec2 = resource("ec2", region_name=AWS_REGION_EU_WEST_1) + + security_group = ec2.create_security_group( + GroupName="sg01", Description="Test security group sg01" + ) + + elb.create_load_balancer( + LoadBalancerName="my-lb-defensive", + Listeners=[ + {"Protocol": "tcp", "LoadBalancerPort": 80, "InstancePort": 8080}, + {"Protocol": "http", "LoadBalancerPort": 81, "InstancePort": 9000}, + ], + AvailabilityZones=[AWS_REGION_EU_WEST_1_AZA], + Scheme="internal", + SecurityGroups=[security_group.id], + ) + + from prowler.providers.aws.services.elb.elb_service import ELB + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ), + ), mock.patch( + "prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode.elb_client", + new=ELB( + set_mocked_aws_provider([AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1]) + ), + ): + from prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode import ( + elb_desync_mitigation_mode, + ) + + check = elb_desync_mitigation_mode() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + result[0].status_extended + == "ELB my-lb-defensive has desync mitigation mode set to defensive." + ) + assert result[0].resource_id == "my-lb-defensive" + assert ( + result[0].resource_arn + == "arn:aws:elasticloadbalancing:eu-west-1:123456789012:loadbalancer/my-lb-defensive" + ) + assert result[0].region == AWS_REGION_EU_WEST_1 + assert result[0].resource_tags == [] + + @mock_aws + @mock.patch("botocore.client.BaseClient._make_api_call", new=mock_make_api_call) + def test_elb_with_strictest_desync_mode(self): + elb = client("elb", region_name=AWS_REGION_EU_WEST_1) + ec2 = resource("ec2", region_name=AWS_REGION_EU_WEST_1) + + security_group = ec2.create_security_group( + GroupName="sg01", Description="Test security group sg01" + ) + + elb.create_load_balancer( + LoadBalancerName="my-lb-strictest", + Listeners=[ + {"Protocol": "tcp", "LoadBalancerPort": 80, "InstancePort": 8080}, + {"Protocol": "http", "LoadBalancerPort": 81, "InstancePort": 9000}, + ], + AvailabilityZones=[AWS_REGION_EU_WEST_1_AZA], + Scheme="internal", + SecurityGroups=[security_group.id], + ) + + from prowler.providers.aws.services.elb.elb_service import ELB + + with mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_aws_provider( + [AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1] + ), + ), mock.patch( + "prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode.elb_client", + new=ELB( + set_mocked_aws_provider([AWS_REGION_EU_WEST_1, AWS_REGION_US_EAST_1]) + ), + ): + from prowler.providers.aws.services.elb.elb_desync_mitigation_mode.elb_desync_mitigation_mode import ( + elb_desync_mitigation_mode, + ) + + check = elb_desync_mitigation_mode() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + result[0].status_extended + == "ELB my-lb-strictest has desync mitigation mode set to strictest." + ) + assert result[0].resource_id == "my-lb-strictest" + assert ( + result[0].resource_arn + == "arn:aws:elasticloadbalancing:eu-west-1:123456789012:loadbalancer/my-lb-strictest" + ) + assert result[0].region == AWS_REGION_EU_WEST_1 + assert result[0].resource_tags == [] diff --git a/tests/providers/aws/services/elb/elb_service_test.py b/tests/providers/aws/services/elb/elb_service_test.py index 608beed7f2..d855281d99 100644 --- a/tests/providers/aws/services/elb/elb_service_test.py +++ b/tests/providers/aws/services/elb/elb_service_test.py @@ -136,6 +136,7 @@ class Test_ELB_Service: assert elb.loadbalancers[elb_arn].access_logs assert elb.loadbalancers[elb_arn].cross_zone_load_balancing assert elb.loadbalancers[elb_arn].connection_draining + assert elb.loadbalancers[elb_arn].desync_mitigation_mode is None # Test ELB Describe Tags @mock_aws diff --git a/tests/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode_test.py b/tests/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode_test.py index 92868118f4..ab81399b8f 100644 --- a/tests/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode_test.py +++ b/tests/providers/aws/services/elbv2/elbv2_desync_mitigation_mode/elbv2_desync_mitigation_mode_test.py @@ -101,7 +101,7 @@ class Test_elbv2_desync_mitigation_mode: assert len(result) == 1 assert result[0].status == "FAIL" assert search( - "does not have desync mitigation mode set as strictest and is not dropping invalid header fields", + "does not have desync mitigation mode set as strictest/defensive and is not dropping invalid header fields", result[0].status_extended, ) assert result[0].resource_id == "my-lb" @@ -169,7 +169,7 @@ class Test_elbv2_desync_mitigation_mode: assert len(result) == 1 assert result[0].status == "PASS" assert search( - "does not have desync mitigation mode set as strictest but is dropping invalid header fields", + "does not have desync mitigation mode set as strictest/defensive but is dropping invalid header fields", result[0].status_extended, ) assert result[0].resource_id == "my-lb"