Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(acm): Add new check for insecure algorithms in certificates #4551

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c3ff8c3
chore(acm): Add RSA key length check
MarioRgzLpz Jul 26, 2024
52a4224
test(acm): Add RSA key length check tests
MarioRgzLpz Jul 26, 2024
8ea0807
test(acm): Add the new atributte key_algorithm to old tests
MarioRgzLpz Jul 26, 2024
4d9d8f3
feat(acm): Add a list of insecure algorithms instead of verifying onl…
MarioRgzLpz Jul 26, 2024
74786c7
fix(acm): Change the URLs for the SecurityHub ones
MarioRgzLpz Jul 29, 2024
c8d3954
feat(acm): Make the insecure algorithm a configurable list by default…
MarioRgzLpz Jul 29, 2024
e1da46b
refactor(acm): I change the check name to a more suitable one
MarioRgzLpz Jul 29, 2024
49dae85
fix(acm): Add the eks_rquire_log_types in config_test to resolve merg…
MarioRgzLpz Jul 29, 2024
b9e698f
Merge branch 'master' into PRWLR-4239-add-new-acm-check-for-rsa-key-l…
MrCloudSec Jul 29, 2024
42829a3
refactor(acm): I change the name of the check and some texts like the…
MarioRgzLpz Jul 30, 2024
629ecff
Merge branch
MarioRgzLpz Jul 30, 2024
ca090d8
chore(acm): Change the recommendation url to the SecurityHub one
MarioRgzLpz Jul 31, 2024
ee08174
chore(acm): Change insecure_algorithm to insecure_key_algorithm in th…
MarioRgzLpz Jul 31, 2024
9546adf
chore(acm): Change method _list_certificates so it returns all key al…
MarioRgzLpz Aug 5, 2024
9697ead
chore(acm): Added logic for checking only in use certificates and swa…
MarioRgzLpz Aug 5, 2024
e797248
Merge branch 'prowler-cloud:master' into PRWLR-4239-add-new-acm-check…
MarioRgzLpz Aug 5, 2024
b4dafb0
Merge branch 'prowler-cloud:master' into PRWLR-4239-add-new-acm-check…
MarioRgzLpz Aug 7, 2024
cf645a7
chore(acm): Add the key algorithm in the status extended and fix the …
MarioRgzLpz Aug 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"Provider": "aws",
"CheckID": "acm_certificates_rsa_key_length",
"CheckTitle": "Check if ACM Certificates use a secure RSA key size",
"CheckType": [
"Data Protection"
],
"ServiceName": "acm",
"SubServiceName": "",
"ResourceIdTemplate": "arn:partition:acm:region:account-id:certificate/resource-id",
"Severity": "high",
"ResourceType": "AwsCertificateManagerCertificate",
"Description": "Check if ACM Certificates use a secure RSA key size",
"Risk": "Certificates with weak RSA keys can be compromised.",
"RelatedUrl": "https://docs.aws.amazon.com/es_es/acm/latest/userguide/acm-certificate.html",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in commit 74786c7

"Remediation": {
"Code": {
"CLI": "",
"NativeIaC": "",
"Other": "",
"Terraform": ""
},
"Recommendation": {
"Text": "Ensure that all ACM certificates use RSA keys of at least 2048 bits. If any certificates use smaller keys, regenerate them with a secure key size and update any systems that rely on these certificates.",
"Url": "https://docs.aws.amazon.com/es_es/acm/latest/userguide/acm-certificate.html"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too, since the SecurityHub link also contains the Remediation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in commit 74786c7

}
},
"Categories": [],
"DependsOn": [],
"RelatedTo": [],
"Notes": ""
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from prowler.lib.check.models import Check, Check_Report_AWS
from prowler.providers.aws.services.acm.acm_client import acm_client


class acm_certificates_rsa_key_length(Check):
insecure_algorithms = ["RSA_1024"]

def execute(self):
findings = []
for certificate in acm_client.certificates:
report = Check_Report_AWS(self.metadata())
report.region = certificate.region
report.resource_id = certificate.id
report.resource_details = certificate.name
report.resource_arn = certificate.arn
report.resource_tags = certificate.tags

report.status = "PASS"
report.status_extended = f"ACM Certificate {certificate.id} for {certificate.name} meet minimum key size requirements."

if certificate.key_algorithm in self.insecure_algorithms:
report.status = "FAIL"
report.status_extended = f"ACM Certificate {certificate.id} for {certificate.name} uses RSA_1024 which is not secure enough."
findings.append(report)

return findings
2 changes: 2 additions & 0 deletions prowler/providers/aws/services/acm/acm_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def __list_certificates__(self, regional_client):
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,
Expand Down Expand Up @@ -98,6 +99,7 @@ class Certificate(BaseModel):
name: str
id: str
type: str
key_algorithm: str
tags: Optional[list] = []
expiration_days: int
in_use: bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def test_acm_certificate_expirated(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"
expiration_days = 5
in_use = True

Expand All @@ -42,6 +43,7 @@ def test_acm_certificate_expirated(self):
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,
Expand Down Expand Up @@ -80,6 +82,7 @@ def test_acm_certificate_expirated_long_time(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"
expiration_days = -400
in_use = True

Expand All @@ -90,6 +93,7 @@ def test_acm_certificate_expirated_long_time(self):
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,
Expand Down Expand Up @@ -127,6 +131,7 @@ def test_acm_certificate_not_expirated(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"
expiration_days = 365
in_use = True

Expand All @@ -137,6 +142,7 @@ def test_acm_certificate_not_expirated(self):
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,
Expand Down Expand Up @@ -173,6 +179,7 @@ def test_acm_certificate_not_in_use(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"
expiration_days = 365
in_use = False

Expand All @@ -183,6 +190,7 @@ def test_acm_certificate_not_in_use(self):
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,
Expand Down Expand Up @@ -212,6 +220,7 @@ def test_acm_certificate_not_in_use_expired_scan_unused_services(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"
expiration_days = -400
in_use = False

Expand All @@ -222,6 +231,7 @@ def test_acm_certificate_not_in_use_expired_scan_unused_services(self):
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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import uuid
from unittest import mock

from prowler.providers.aws.services.acm.acm_service import Certificate

AWS_REGION = "us-east-1"
AWS_ACCOUNT_NUMBER = "123456789012"


class Test_acm_certificates_rsa_key_length:
def test_no_acm_certificates(self):
acm_client = mock.MagicMock
acm_client.certificates = []

with mock.patch(
"prowler.providers.aws.services.acm.acm_service.ACM",
new=acm_client,
):
# Test Check
from prowler.providers.aws.services.acm.acm_certificates_rsa_key_length.acm_certificates_rsa_key_length import (
acm_certificates_rsa_key_length,
)

check = acm_certificates_rsa_key_length()
result = check.execute()

assert len(result) == 0

def test_acm_certificate_valid_key_length(self):
certificate_id = str(uuid.uuid4())
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
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,
)
]

with mock.patch(
"prowler.providers.aws.services.acm.acm_service.ACM",
new=acm_client,
):
# Test Check
from prowler.providers.aws.services.acm.acm_certificates_rsa_key_length.acm_certificates_rsa_key_length import (
acm_certificates_rsa_key_length,
)

check = acm_certificates_rsa_key_length()
result = check.execute()

assert len(result) == 1
assert result[0].status == "PASS"
assert (
result[0].status_extended
== f"ACM Certificate {certificate_id} for {certificate_name} meet minimum key size requirements."
)
assert result[0].resource_id == certificate_id
assert result[0].resource_arn == certificate_arn
assert result[0].region == AWS_REGION
assert result[0].resource_tags == []

def test_acm_certificate_short_RSA_key(self):
certificate_id = str(uuid.uuid4())
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
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,
)
]

with mock.patch(
"prowler.providers.aws.services.acm.acm_service.ACM",
new=acm_client,
):
# Test Check
from prowler.providers.aws.services.acm.acm_certificates_rsa_key_length.acm_certificates_rsa_key_length import (
acm_certificates_rsa_key_length,
)

check = acm_certificates_rsa_key_length()
result = check.execute()

assert len(result) == 1
assert result[0].status == "FAIL"
assert (
result[0].status_extended
== f"ACM Certificate {certificate_id} for {certificate_name} uses RSA_1024 which is not secure enough."
)
assert result[0].resource_id == certificate_id
assert result[0].resource_arn == certificate_arn
assert result[0].region == AWS_REGION
assert result[0].resource_tags == []
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def test_acm_certificate_with_logging(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"

acm_client = mock.MagicMock
acm_client.certificates = [
Expand All @@ -39,6 +40,7 @@ def test_acm_certificate_with_logging(self):
id=certificate_id,
name=certificate_name,
type=certificate_type,
key_algorithm=certificate_key_algorithm,
expiration_days=365,
transparency_logging=True,
in_use=True,
Expand Down Expand Up @@ -74,6 +76,7 @@ def test_acm_certificate_without_logging(self):
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_type = "AMAZON_ISSUED"
certificate_key_algorithm = "RSA_2048"

acm_client = mock.MagicMock
acm_client.certificates = [
Expand All @@ -82,6 +85,7 @@ def test_acm_certificate_without_logging(self):
id=certificate_id,
name=certificate_name,
type=certificate_type,
key_algorithm=certificate_key_algorithm,
expiration_days=365,
transparency_logging=False,
in_use=True,
Expand Down Expand Up @@ -116,6 +120,7 @@ def test_acm_certificate_imported(self):
certificate_id = str(uuid.uuid4())
certificate_arn = f"arn:aws:acm:{AWS_REGION}:{AWS_ACCOUNT_NUMBER}:certificate/{certificate_id}"
certificate_name = "test-certificate.com"
certificate_key_algorithm = "RSA_2048"
certificate_type = "IMPORTED"

acm_client = mock.MagicMock
Expand All @@ -125,6 +130,7 @@ def test_acm_certificate_imported(self):
id=certificate_id,
name=certificate_name,
type=certificate_type,
key_algorithm=certificate_key_algorithm,
expiration_days=365,
transparency_logging=True,
in_use=True,
Expand Down
2 changes: 2 additions & 0 deletions tests/providers/aws/services/acm/acm_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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):
Expand Down Expand Up @@ -142,6 +143,7 @@ def test__list_and_describe_certificates__(self):
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
Expand Down