feat(elasticache): add check `elasticache_redis_cluster_auth_enabled` #4830

HugoPBrito · 2024-08-22T10:21:24Z

Context

Redis AUTH is essential for securing access to Redis clusters by requiring a password for client commands, especially since Role-Based Access Control (RBAC) is not available in versions prior to 6.0. The control will fail if Redis AUTH is not enabled for these earlier versions, helping to enforce best practices for data security in environments where older Redis versions are still in use. For Redis versions 6.0 and later, RBAC is recommended, but this check specifically targets the need for AUTH in versions below 6.0.

Description

I have implemented a new check called elasticache_redis_cluster_auth_enabled to address a security concern in Amazon ElastiCache for Redis. This check ensures that replication groups running Redis versions earlier than 6.0 have Redis AUTH enabled.

Checklist

Are there new checks included in this PR? Yes / No
- If so, do we need to update permissions for the provider? Please review this carefully.
Review if the code is being covered by tests.
Review if code is being documented following this specification https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

HugoPBrito · 2024-08-22T10:26:52Z

I used the describe_cache_clusters method and filtered within Redis instead of describe_replication_groups because Boto3 provides the necessary attributes only through this method.

codecov · 2024-08-22T10:58:57Z

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.12%. Comparing base (9263ade) to head (97440d0).
Report is 31 commits behind head on master.

Files with missing lines	Patch %	Lines
...rs/aws/services/elasticache/elasticache_service.py	77.77%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4830      +/-   ##
==========================================
+ Coverage   89.00%   89.12%   +0.12%     
==========================================
  Files         965      974       +9     
  Lines       29526    29852     +326     
==========================================
+ Hits        26279    26607     +328     
+ Misses       3247     3245       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sergargar · 2024-08-23T14:37:22Z

..._cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.metadata.json

@@ -0,0 +1,32 @@
+{
+  "Provider": "aws",
+  "CheckID": "elasticache_redis_cluster_below_v6_auth_enabled",


Suggested change

"CheckID": "elasticache_redis_cluster_below_v6_auth_enabled",

"CheckID": "elasticache_redis_cluster_auth_enabled",

sergargar · 2024-08-23T14:38:42Z

..._cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.metadata.json

+{
+  "Provider": "aws",
+  "CheckID": "elasticache_redis_cluster_below_v6_auth_enabled",
+  "CheckTitle": "Ensure Elasticache Elasticache Redis replication groups under version 6.0 have Redis OSS AUTH Enabled.",


Please, don't hardcore versions in the metadata, it could change in the future.

Suggested change

"CheckTitle": "Ensure Elasticache Elasticache Redis replication groups under version 6.0 have Redis OSS AUTH Enabled.",

"CheckTitle": "Ensure Elasticache Elasticache Redis replication groups of earlier versions should have Redis OSS AUTH enabled.",

sergargar · 2024-08-23T14:38:58Z

..._cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.metadata.json

+  "CheckType": [
+    "AWS Foundational Security Best Practices v1.0.0"
+  ],


Please, use the types from https://docs.aws.amazon.com/securityhub/latest/userguide/asff-required-attributes.html#Types

sergargar · 2024-08-23T14:39:28Z

...cache_redis_cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.py

+        for cluster in elasticache_client.clusters.values():
+            if (
+                cluster.engine == "redis" and int(cluster.engine_version[0]) < 6
+            ):  # major.minor.patch


What is the comment for?

It was thought to clarify the string format of the version stored in engine_version. If it is unclear or unnecessary we can remove it.

sergargar · 2024-08-23T14:40:12Z

prowler/providers/aws/services/elasticache/elasticache_service.py

@@ -41,6 +41,10 @@ def _describe_cache_clusters(self, regional_client):
                            auto_minor_version_upgrade=cache_cluster.get(
                                "AutoMinorVersionUpgrade", False
                            ),
+                            engine_version=cache_cluster.get("EngineVersion", None),


The None value would raise an error in the check.

sergargar · 2024-08-23T14:44:50Z

...cache_redis_cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.py

Please, add another MANUAL finding for clusters of later versions saying to implement Redis ACL(Role bases access control) in the cluster.

sergargar · 2024-08-23T16:14:46Z

...cache_redis_cluster_below_v6_auth_enabled/elasticache_redis_cluster_below_v6_auth_enabled.py

+class elasticache_redis_cluster_below_v6_auth_enabled(Check):
+    def execute(self):
+        findings = []
+        for cluster in elasticache_client.clusters.values():


Please, use the replication_groups since we have to check it at that level and update the metadata.

puchy22 · 2024-08-28T15:48:33Z

.../elasticache_redis_cluster_auth_enabled/elasticache_redis_cluster_auth_enabled.metadata.json

+  "SubServiceName": "",
+  "ResourceIdTemplate": "arn:partition:service:region:account-id:resource-id",
+  "Severity": "medium",
+  "ResourceType": "AWSElastiCacheClusters",


I think this ResourceType is not supported by AWS, please refer to the docs

puchy22 · 2024-08-28T19:02:54Z

.../elasticache_redis_cluster_auth_enabled/elasticache_redis_cluster_auth_enabled.metadata.json

@@ -0,0 +1,32 @@
+{
+  "Provider": "aws",
+  "CheckID": "elasticache_redis_cluster_auth_enabled",


I don't understand why names redis_cluster if you are giving findings about replication groups

I handled it this way because all replication group checks were done that way. Making these corrections should imply renaming the checkID and outputs of all other replication group checks as well.

puchy22 · 2024-08-28T19:06:40Z

...elasticache/elasticache_redis_cluster_auth_enabled/elasticache_redis_cluster_auth_enabled.py

+            report.resource_tags = repl_group.tags
+
+            cluster = repl_group.member_clusters[0]
+            report.status = "PASS"


Suggested change

report.status = "PASS"

report.status = "MANUAL"

puchy22 · 2024-08-28T19:14:43Z

...elasticache/elasticache_redis_cluster_auth_enabled/elasticache_redis_cluster_auth_enabled.py

+            report.resource_arn = repl_group.arn
+            report.resource_tags = repl_group.tags
+
+            cluster = repl_group.member_clusters[0]


Is there no other way to get the version of the replication cluster? It's weird to get it from the first cluster, it's a bit arbitrary.

I´ll update this in a better way. I thought MemberClusters returned primary node in first position, but I think that´s incorrect. I´ll take the primary node now (whose version is replicated in replica nodes) to make this non arbitrary at all.

puchy22 · 2024-08-28T19:18:37Z

prowler/providers/aws/services/elasticache/elasticache_service.py

@@ -41,6 +41,10 @@ def _describe_cache_clusters(self, regional_client):
                            auto_minor_version_upgrade=cache_cluster.get(
                                "AutoMinorVersionUpgrade", False
                            ),
+                            engine_version=cache_cluster.get("EngineVersion", "None"),


2 things here:

If the default value "None" reach to the check it will be: int("None") which is a problem,

Why not store this values as a floating point instead a str?

puchy22 · 2024-08-28T19:19:50Z

prowler/providers/aws/services/elasticache/elasticache_service.py

@@ -90,6 +94,16 @@ def _describe_replication_groups(self, regional_client):
                    if not self.audit_resources or (
                        is_resource_filtered(replication_arn, self.audit_resources)
                    ):
+                        member_clusters = repl_group.get("MemberClusters", [])
+                        cluster_list = []
+                        if member_clusters:


Suggested change

if member_clusters:

…rtitions

…tion

puchy22 · 2024-09-11T09:02:05Z

prowler/providers/aws/services/elasticache/elasticache_service.py

+                                    primary_id = primary_node["CacheClusterId"]
+                                    break
+
+                        except Exception as error:


Please, could you manage the specific Exception instead just generic one

...eplication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.metadata.json

…ion-groups-of-earlier-redis-versions-should-have-redis-auth-enabled

sergargar · 2024-09-16T15:40:01Z

prowler/providers/aws/services/elasticache/elasticache_service.py

+                        version = cache_cluster.get("EngineVersion", "0.0")
+                        version = float(version[:3])
+


Suggested change

version = cache_cluster.get("EngineVersion", "0.0")

version = float(version[:3])

version = version.parse(cache_cluster.get("EngineVersion", "0.0"))

Please, use version function from the packaging library.

from packaging import version

sergargar · 2024-09-16T15:40:53Z

...che_redis_replication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.py

+            report.status = "MANUAL"
+            report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{repl_group.engine_version}) does not have to use AUTH, but it should have Redis ACL configured."
+
+            if repl_group.engine_version < 6.0:


Suggested change

if repl_group.engine_version < 6.0:

if repl_group.engine_version < version.parse("6.0"):

sergargar · 2024-09-16T15:51:08Z

prowler/providers/aws/services/elasticache/elasticache_service.py

+                        # Get primary cluster
+                        try:
+                            for node_group in repl_group["NodeGroups"][0]:
+                                primary_node = next(
+                                    (
+                                        node
+                                        for node in node_group["NodeGroupMembers"]
+                                        if node["CurrentRole"] == "primary"
+                                    ),
+                                    None,
+                                )
+                                if primary_node:
+                                    primary_id = primary_node["CacheClusterId"]
+                                    break
+
+                        except Exception as error:
+                            primary_node = repl_group["NodeGroups"][0][
+                                "NodeGroupMembers"
+                            ][0]
+                            primary_id = primary_node["CacheClusterId"]
+                            logger.error(
+                                f"{regional_client.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
+                            )


Please, get the nodes from MemberClusters, and verify within the check if those members have the compliant configuration.

sergargar · 2024-09-16T15:52:44Z

prowler/providers/aws/services/elasticache/elasticache_service.py

+                            engine_version=version,
+                            auth_token_enabled=repl_group.get(
+                                "AuthTokenEnabled", False


Please, only store the member_clusters attribute.

sergargar · 2024-09-17T14:55:56Z

...che_redis_replication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.py

+            report.resource_arn = repl_group.arn
+            report.resource_tags = repl_group.tags
+
+            for cluster in repl_group.member_clusters:


Please, can you show in the status extended the member clusters that are not compliant (if any).

sergargar · 2024-09-17T14:56:30Z

...che_redis_replication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.py

+
+                    else:
+                        report.status = "PASS"
+                        report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) does have AUTH enabled."


Suggested change

report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) does have AUTH enabled."

report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) has all member clusters with AUTH enabled."

sergargar · 2024-09-17T14:57:19Z

...che_redis_replication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.py

+                        report.status = "PASS"
+                        report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) does have AUTH enabled."
+                else:
+                    report.status = "MANUAL"


Can member cluster have different versions?

If not, leave it as it is.

As far as I'm concerned, every node must have the exact same version inside a replication group.

…ion-groups-of-earlier-redis-versions-should-have-redis-auth-enabled

sergargar · 2024-09-19T17:00:43Z

prowler/providers/aws/services/elasticache/elasticache_service.py

+                        # Get first cluster version as they all have the same unless an upgrade is being made
+                        member_clusters = repl_group.get("MemberClusters", [])
+                        if member_clusters:
+                            cluster_arn = f"arn:aws:s3:::{member_clusters[0]}"


Please, verify this ARN and add a test for this case.

...eplication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.metadata.json

@sergargar

Reviewed by @sergargar

HugoPBrito added 6 commits August 21, 2024 13:06

fix(elasticache): fixed error in metadata description

a328b02

fix(elasticache): fixed error in metadata checktitle

d1dd13a

fix(elasticache): fixed error in metadata recommendation-text

e8f0a3d

feat(elasticache): expanded cluster model and testing

2a1fb68

feat(elasticache): Added check logic and testing

412f688

feat(aws): Correction of check results message

63efb1c

HugoPBrito requested review from a team as code owners August 22, 2024 10:21

github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Aug 22, 2024

fix(elasticache): changed error in metadata description

aad2ec8

sergargar reviewed Aug 23, 2024

View reviewed changes

HugoPBrito added 4 commits August 27, 2024 12:18

feat(elasticache): title and description changed in metadata

0f7b95e

feat(elasticache): resolved most comments

a0b289e

feat(elasticache): changed check name and minor modifications

6b01746

feat(elasticache): renewed logic and updated model and testing

c08ef95

puchy22 changed the title ~~feat(elasticache): Ensure Redis clusters below v6.0 have AUTH enabled~~ feat(elasticache): add check elasticache_redis_cluster_auth_enabled Aug 28, 2024

puchy22 requested changes Aug 28, 2024

View reviewed changes

HugoPBrito added 5 commits August 29, 2024 13:40

feat(elasticache): resolved comments

a7b8000

feat(elasticache): improved logic and adapted testing

48f6f6d

fix(elasticache): handled error in replication groups with several pa…

79603de

…rtitions

fix(elasticache): minor correction

4faddef

chore(elasticache): Only iterate one nodegroup

81ebc75

fix(elasticache): restored previous commit due to better error preven…

7bcaee8

…tion

puchy22 previously requested changes Sep 11, 2024

View reviewed changes

sergargar reviewed Sep 11, 2024

View reviewed changes

...eplication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.metadata.json Outdated Show resolved Hide resolved

Merge branch 'master' into PRWLR-4509-elasti-cache-redis-oss-replicat…

a18b7ad

…ion-groups-of-earlier-redis-versions-should-have-redis-auth-enabled

sergargar reviewed Sep 16, 2024

View reviewed changes

feat(elasticache): resolved comments

003c82a

sergargar reviewed Sep 17, 2024

View reviewed changes

HugoPBrito and others added 5 commits September 18, 2024 18:30

Merge branch 'master' into PRWLR-4509-elasti-cache-redis-oss-replicat…

66aeb8f

…ion-groups-of-earlier-redis-versions-should-have-redis-auth-enabled

fix(elasticache): fix automatic failover

25958b6

chore:stage changes

dd3363a

feat(elasticache): resolved comments

9dc0b9e

chore: enhance check and metadata

73ec237

sergargar reviewed Sep 19, 2024

View reviewed changes

HugoPBrito and others added 2 commits September 20, 2024 10:18

fix(elasticache): fixed mistaken arn and tested in real infraestructure

3fb54a8

fix: remove print

42b2cdd

sergargar reviewed Sep 20, 2024

View reviewed changes

...eplication_group_auth_enabled/elasticache_redis_replication_group_auth_enabled.metadata.json Outdated Show resolved Hide resolved

fix: update resource type

97440d0

sergargar self-requested a review September 20, 2024 16:12

sergargar approved these changes Sep 20, 2024

View reviewed changes

sergargar merged commit c6daa60 into master Sep 20, 2024
11 checks passed

sergargar deleted the PRWLR-4509-elasti-cache-redis-oss-replication-groups-of-earlier-redis-versions-should-have-redis-auth-enabled branch September 20, 2024 16:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticache): add check `elasticache_redis_cluster_auth_enabled` #4830

feat(elasticache): add check `elasticache_redis_cluster_auth_enabled` #4830

HugoPBrito commented Aug 22, 2024 •

edited by puchy22

Loading

HugoPBrito commented Aug 22, 2024

codecov bot commented Aug 22, 2024 •

edited

Loading

sergargar Aug 23, 2024

sergargar Aug 23, 2024

sergargar Aug 23, 2024

sergargar Aug 23, 2024

HugoPBrito Aug 23, 2024

sergargar Aug 23, 2024

sergargar Aug 23, 2024

sergargar Aug 23, 2024

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

HugoPBrito Aug 29, 2024 •

edited

Loading

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

HugoPBrito Aug 29, 2024

puchy22 Aug 28, 2024

puchy22 Aug 28, 2024

puchy22 Sep 11, 2024

sergargar Sep 16, 2024

sergargar Sep 16, 2024

sergargar Sep 16, 2024

sergargar Sep 16, 2024

sergargar Sep 16, 2024

sergargar Sep 17, 2024

sergargar Sep 17, 2024

sergargar Sep 17, 2024

sergargar Sep 17, 2024

HugoPBrito Sep 18, 2024

sergargar Sep 19, 2024

HugoPBrito Sep 20, 2024 •

edited

Loading

	"CheckID": "elasticache_redis_cluster_below_v6_auth_enabled",
	"CheckID": "elasticache_redis_cluster_auth_enabled",

	"CheckTitle": "Ensure Elasticache Elasticache Redis replication groups under version 6.0 have Redis OSS AUTH Enabled.",
	"CheckTitle": "Ensure Elasticache Elasticache Redis replication groups of earlier versions should have Redis OSS AUTH enabled.",

		version = cache_cluster.get("EngineVersion", "0.0")
		version = float(version[:3])

	version = cache_cluster.get("EngineVersion", "0.0")
	version = float(version[:3])
	version = version.parse(cache_cluster.get("EngineVersion", "0.0"))

	if repl_group.engine_version < 6.0:
	if repl_group.engine_version < version.parse("6.0"):

	report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) does have AUTH enabled."
	report.status_extended = f"Elasticache Redis replication group {repl_group.id}(v{cluster.engine_version}) has all member clusters with AUTH enabled."

feat(elasticache): add check elasticache_redis_cluster_auth_enabled #4830

feat(elasticache): add check elasticache_redis_cluster_auth_enabled #4830

Conversation

HugoPBrito commented Aug 22, 2024 • edited by puchy22 Loading

Context

Description

Checklist

License

HugoPBrito commented Aug 22, 2024

codecov bot commented Aug 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HugoPBrito Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HugoPBrito Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

feat(elasticache): add check `elasticache_redis_cluster_auth_enabled` #4830

feat(elasticache): add check `elasticache_redis_cluster_auth_enabled` #4830

HugoPBrito commented Aug 22, 2024 •

edited by puchy22

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading

HugoPBrito Aug 29, 2024 •

edited

Loading

HugoPBrito Sep 20, 2024 •

edited

Loading