From f7fd355dc1e92ac6e1c90184d1d680f0bf6a22fb Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Lopez <101330800+MarioRgzLpz@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:16:21 +0200 Subject: [PATCH] refactor(acm): Change certificates from list to dict in `acm_service` (#5420) --- .../acm_certificates_expiration_check.py | 2 +- ..._certificates_transparency_logs_enabled.py | 2 +- ...certificates_with_secure_key_algorithms.py | 2 +- .../providers/aws/services/acm/acm_service.py | 68 +++++----- .../acm_certificates_expiration_check_test.py | 127 +++++++++--------- ...ificates_transparency_logs_enabled_test.py | 102 +++++++------- ...ficates_with_secure_key_algorithms_test.py | 52 ++++--- .../aws/services/acm/acm_service_test.py | 36 ++--- 8 files changed, 190 insertions(+), 201 deletions(-) diff --git a/prowler/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check.py b/prowler/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check.py index 3cb5b7e398..ab684e3397 100644 --- a/prowler/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check.py +++ b/prowler/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check.py @@ -5,7 +5,7 @@ from prowler.providers.aws.services.acm.acm_client import acm_client class acm_certificates_expiration_check(Check): def execute(self): findings = [] - for certificate in acm_client.certificates: + for certificate in acm_client.certificates.values(): if certificate.in_use or acm_client.provider.scan_unused_services: report = Check_Report_AWS(self.metadata()) report.region = certificate.region diff --git a/prowler/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled.py b/prowler/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled.py index 7c067a9a41..19c24f82b7 100644 --- a/prowler/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled.py +++ b/prowler/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled.py @@ -5,7 +5,7 @@ from prowler.providers.aws.services.acm.acm_client import acm_client class acm_certificates_transparency_logs_enabled(Check): def execute(self): findings = [] - for certificate in acm_client.certificates: + for certificate in acm_client.certificates.values(): if certificate.in_use or acm_client.provider.scan_unused_services: report = Check_Report_AWS(self.metadata()) report.region = certificate.region diff --git a/prowler/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms.py b/prowler/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms.py index eb91295774..16f19ef061 100644 --- a/prowler/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms.py +++ b/prowler/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms.py @@ -5,7 +5,7 @@ from prowler.providers.aws.services.acm.acm_client import acm_client class acm_certificates_with_secure_key_algorithms(Check): def execute(self): findings = [] - for certificate in acm_client.certificates: + for certificate in acm_client.certificates.values(): if certificate.in_use or acm_client.provider.scan_unused_services: report = Check_Report_AWS(self.metadata()) report.region = certificate.region diff --git a/prowler/providers/aws/services/acm/acm_service.py b/prowler/providers/aws/services/acm/acm_service.py index d858464a30..b5d85c44dd 100644 --- a/prowler/providers/aws/services/acm/acm_service.py +++ b/prowler/providers/aws/services/acm/acm_service.py @@ -13,10 +13,12 @@ class ACM(AWSService): def __init__(self, provider): # Call AWSService's __init__ super().__init__(__class__.__name__, provider) - self.certificates = [] + self.certificates = {} self.__threading_call__(self._list_certificates) - self._describe_certificates() - self._list_tags_for_certificate() + self.__threading_call__(self._describe_certificates, self.certificates.values()) + self.__threading_call__( + self._list_tags_for_certificate, self.certificates.values() + ) def _list_certificates(self, regional_client): logger.info("ACM - Listing Certificates...") @@ -54,54 +56,50 @@ class ACM(AWSService): ).days else: certificate_expiration_time = 0 - self.certificates.append( - Certificate( - arn=certificate["CertificateArn"], - name=certificate["DomainName"], - id=certificate["CertificateArn"].split("/")[-1], - type=certificate["Type"], - key_algorithm=certificate["KeyAlgorithm"], - expiration_days=certificate_expiration_time, - in_use=certificate.get("InUse", False), - transparency_logging=False, - region=regional_client.region, - ) + self.certificates[certificate["CertificateArn"]] = Certificate( + arn=certificate["CertificateArn"], + name=certificate["DomainName"], + id=certificate["CertificateArn"].split("/")[-1], + type=certificate["Type"], + key_algorithm=certificate["KeyAlgorithm"], + expiration_days=certificate_expiration_time, + in_use=certificate.get("InUse", False), + transparency_logging=False, + region=regional_client.region, ) except Exception as error: logger.error( f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - def _describe_certificates(self): + def _describe_certificates(self, certificate): logger.info("ACM - Describing Certificates...") try: - for certificate in self.certificates: - regional_client = self.regional_clients[certificate.region] - response = regional_client.describe_certificate( - CertificateArn=certificate.arn - )["Certificate"] - if ( - response["Options"]["CertificateTransparencyLoggingPreference"] - == "ENABLED" - ): - certificate.transparency_logging = True + regional_client = self.regional_clients[certificate.region] + response = regional_client.describe_certificate( + CertificateArn=certificate.arn + )["Certificate"] + if ( + response["Options"]["CertificateTransparencyLoggingPreference"] + == "ENABLED" + ): + certificate.transparency_logging = True except Exception as error: logger.error( - f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + f"{certificate.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - def _list_tags_for_certificate(self): + def _list_tags_for_certificate(self, certificate): logger.info("ACM - List Tags...") try: - for certificate in self.certificates: - regional_client = self.regional_clients[certificate.region] - response = regional_client.list_tags_for_certificate( - CertificateArn=certificate.arn - )["Tags"] - certificate.tags = response + regional_client = self.regional_clients[certificate.region] + response = regional_client.list_tags_for_certificate( + CertificateArn=certificate.arn + )["Tags"] + certificate.tags = response except Exception as error: logger.error( - f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + f"{certificate.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) diff --git a/tests/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check_test.py b/tests/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check_test.py index 157d4366c2..f7336acd8f 100644 --- a/tests/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check_test.py +++ b/tests/providers/aws/services/acm/acm_certificates_expiration_check/acm_certificates_expiration_check_test.py @@ -11,7 +11,7 @@ DAYS_TO_EXPIRE_THRESHOLD = 7 class Test_acm_certificates_expiration_check: def test_no_acm_certificates(self): acm_client = mock.MagicMock - acm_client.certificates = [] + acm_client.certificates = {} with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", @@ -37,19 +37,18 @@ class Test_acm_certificates_expiration_check: in_use = True acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=expiration_days, - in_use=in_use, - transparency_logging=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=expiration_days, + in_use=in_use, + transparency_logging=True, + region=AWS_REGION, + ) acm_client.audit_config = {"days_to_expire_threshold": 7} @@ -87,19 +86,18 @@ class Test_acm_certificates_expiration_check: in_use = True acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=expiration_days, - in_use=in_use, - transparency_logging=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=expiration_days, + in_use=in_use, + transparency_logging=True, + region=AWS_REGION, + ) acm_client.audit_config = {"days_to_expire_threshold": 7} @@ -136,19 +134,18 @@ class Test_acm_certificates_expiration_check: in_use = True acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=expiration_days, - in_use=in_use, - transparency_logging=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=expiration_days, + in_use=in_use, + transparency_logging=True, + region=AWS_REGION, + ) acm_client.audit_config = {"days_to_expire_threshold": 7} @@ -184,19 +181,18 @@ class Test_acm_certificates_expiration_check: in_use = False acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=expiration_days, - in_use=in_use, - transparency_logging=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=expiration_days, + in_use=in_use, + transparency_logging=True, + region=AWS_REGION, + ) acm_client.audit_config = {"days_to_expire_threshold": 7} @@ -225,19 +221,18 @@ class Test_acm_certificates_expiration_check: in_use = False acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=expiration_days, - in_use=in_use, - transparency_logging=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=expiration_days, + in_use=in_use, + transparency_logging=True, + region=AWS_REGION, + ) acm_client.audit_config = {"days_to_expire_threshold": 7} diff --git a/tests/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled_test.py b/tests/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled_test.py index a95edabe41..f3f72629af 100644 --- a/tests/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled_test.py +++ b/tests/providers/aws/services/acm/acm_certificates_transparency_logs_enabled/acm_certificates_transparency_logs_enabled_test.py @@ -10,7 +10,7 @@ AWS_ACCOUNT_NUMBER = "123456789012" class Test_acm_certificates_transparency_logs_enabled: def test_no_acm_certificates(self): acm_client = mock.MagicMock - acm_client.certificates = [] + acm_client.certificates = {} with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", @@ -34,19 +34,18 @@ class Test_acm_certificates_transparency_logs_enabled: certificate_key_algorithm = "RSA-2048" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=True, - in_use=False, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=True, + in_use=False, + region=AWS_REGION, + ) acm_client.provider = mock.MagicMock(scan_unused_services=False) @@ -75,19 +74,18 @@ class Test_acm_certificates_transparency_logs_enabled: certificate_key_algorithm = "RSA-2048" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=True, - in_use=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=True, + in_use=True, + region=AWS_REGION, + ) with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", @@ -120,19 +118,18 @@ class Test_acm_certificates_transparency_logs_enabled: certificate_key_algorithm = "RSA-2048" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=False, - in_use=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=False, + in_use=True, + region=AWS_REGION, + ) with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", @@ -165,19 +162,18 @@ class Test_acm_certificates_transparency_logs_enabled: certificate_type = "IMPORTED" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=True, - in_use=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=True, + in_use=True, + region=AWS_REGION, + ) with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", diff --git a/tests/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms_test.py b/tests/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms_test.py index 0862893fb7..476f56b732 100644 --- a/tests/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms_test.py +++ b/tests/providers/aws/services/acm/acm_certificates_with_secure_key_algorithms/acm_certificates_with_secure_key_algorithms_test.py @@ -10,7 +10,7 @@ AWS_ACCOUNT_NUMBER = "123456789012" class Test_acm_certificates_with_secure_key_algorithms: def test_no_acm_certificates(self): acm_client = mock.MagicMock - acm_client.certificates = [] + acm_client.certificates = {} with mock.patch( "prowler.providers.aws.services.acm.acm_service.ACM", @@ -34,19 +34,18 @@ class Test_acm_certificates_with_secure_key_algorithms: certificate_key_algorithm = "RSA-2048" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=True, - in_use=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=True, + in_use=True, + region=AWS_REGION, + ) acm_client.audit_config = {"insecure_algorithm": ["RSA-1024"]} @@ -81,19 +80,18 @@ class Test_acm_certificates_with_secure_key_algorithms: certificate_key_algorithm = "RSA-1024" acm_client = mock.MagicMock - acm_client.certificates = [ - Certificate( - arn=certificate_arn, - id=certificate_id, - name=certificate_name, - type=certificate_type, - key_algorithm=certificate_key_algorithm, - expiration_days=365, - transparency_logging=False, - in_use=True, - region=AWS_REGION, - ) - ] + acm_client.certificates = {} + acm_client.certificates[certificate_arn] = Certificate( + arn=certificate_arn, + id=certificate_id, + name=certificate_name, + type=certificate_type, + key_algorithm=certificate_key_algorithm, + expiration_days=365, + transparency_logging=False, + in_use=True, + region=AWS_REGION, + ) acm_client.audit_config = {"insecure_algorithm": ["RSA-1024"]} diff --git a/tests/providers/aws/services/acm/acm_service_test.py b/tests/providers/aws/services/acm/acm_service_test.py index 32eef4993a..23ef4be67b 100644 --- a/tests/providers/aws/services/acm/acm_service_test.py +++ b/tests/providers/aws/services/acm/acm_service_test.py @@ -15,10 +15,10 @@ from tests.providers.aws.utils import ( # Mocking Access Analyzer Calls make_api_call = botocore.client.BaseClient._make_api_call -certificate_arn = f"arn:aws:acm:{AWS_REGION_US_EAST_1}:{AWS_ACCOUNT_NUMBER}:certificate/{str(uuid.uuid4())}" -certificate_name = "test-certificate.com" -certificate_type = "AMAZON_ISSUED" -certificate_key_algorithm = "RSA-4096" +CERTIFICATE_ARN = f"arn:aws:acm:{AWS_REGION_US_EAST_1}:{AWS_ACCOUNT_NUMBER}:certificate/{str(uuid.uuid4())}" +CERTIFICATE_NAME = "test-certificate.com" +CERTIFICATE_TYPE = "AMAZON_ISSUED" +CERTIFICATE_KEY_ALGORITHM = "RSA-4096" def mock_make_api_call(self, operation_name, kwargs): @@ -33,14 +33,14 @@ def mock_make_api_call(self, operation_name, kwargs): return { "CertificateSummaryList": [ { - "CertificateArn": certificate_arn, - "DomainName": certificate_name, + "CertificateArn": CERTIFICATE_ARN, + "DomainName": CERTIFICATE_NAME, "SubjectAlternativeNameSummaries": [ "test-certificate-2.com", ], "HasAdditionalSubjectAlternativeNames": False, "Status": "ISSUED", - "Type": certificate_type, + "Type": CERTIFICATE_TYPE, "KeyAlgorithm": "RSA-4096", "KeyUsages": ["DIGITAL_SIGNATURE"], "ExtendedKeyUsages": ["TLS_WEB_SERVER_AUTHENTICATION"], @@ -57,14 +57,14 @@ def mock_make_api_call(self, operation_name, kwargs): ] } if operation_name == "DescribeCertificate": - if kwargs["CertificateArn"] == certificate_arn: + if kwargs["CertificateArn"] == CERTIFICATE_ARN: return { "Certificate": { "Options": {"CertificateTransparencyLoggingPreference": "DISABLED"}, } } if operation_name == "ListTagsForCertificate": - if kwargs["CertificateArn"] == certificate_arn: + if kwargs["CertificateArn"] == CERTIFICATE_ARN: return { "Tags": [ {"Key": "test", "Value": "test"}, @@ -140,13 +140,15 @@ class Test_ACM_Service: aws_provider = set_mocked_aws_provider() acm = ACM(aws_provider) assert len(acm.certificates) == 1 - assert acm.certificates[0].arn == certificate_arn - assert acm.certificates[0].name == certificate_name - assert acm.certificates[0].type == certificate_type - assert acm.certificates[0].key_algorithm == certificate_key_algorithm - assert acm.certificates[0].expiration_days == 365 - assert acm.certificates[0].transparency_logging is False - assert acm.certificates[0].region == AWS_REGION_US_EAST_1 + assert acm.certificates[CERTIFICATE_ARN].arn == CERTIFICATE_ARN + assert acm.certificates[CERTIFICATE_ARN].name == CERTIFICATE_NAME + assert acm.certificates[CERTIFICATE_ARN].type == CERTIFICATE_TYPE + assert ( + acm.certificates[CERTIFICATE_ARN].key_algorithm == CERTIFICATE_KEY_ALGORITHM + ) + assert acm.certificates[CERTIFICATE_ARN].expiration_days == 365 + assert acm.certificates[CERTIFICATE_ARN].transparency_logging is False + assert acm.certificates[CERTIFICATE_ARN].region == AWS_REGION_US_EAST_1 # Test ACM List Tags # @mock_acm @@ -162,6 +164,6 @@ class Test_ACM_Service: aws_provider = set_mocked_aws_provider() acm = ACM(aws_provider) assert len(acm.certificates) == 1 - assert acm.certificates[0].tags == [ + assert acm.certificates[CERTIFICATE_ARN].tags == [ {"Key": "test", "Value": "test"}, ]