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(aws): Add new check to ensure RDS instances are not using default database engine ports #4973

Conversation

danibarranqueroo
Copy link
Member

@danibarranqueroo danibarranqueroo commented Sep 10, 2024

Context

This new check verifies if an instance uses a port other than the default port of the database engine. The control fails if the RDS instance uses the default port or a non-default port that is not configured in the security group.

Since RDS instances belong to VPC Security Groups, which are managed by EC2, I've used this service to iterate over all the security groups and verify if an instance belongs to any of them. If so, I've checked that the security group has the port that the instance is listening to open. With this implementation, I can verify that if the instance uses a non-default port, it's configured correctly in the security group.

Description

Added rds_instance_not_default_port check with its unit tests.

Checklist

License

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

@github-actions github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.10%. Comparing base (21ac395) to head (9a012bf).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4973      +/-   ##
==========================================
+ Coverage   89.05%   89.10%   +0.04%     
==========================================
  Files         968      969       +1     
  Lines       29618    29642      +24     
==========================================
+ Hits        26376    26412      +36     
+ Misses       3242     3230      -12     

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

@danibarranqueroo danibarranqueroo marked this pull request as draft September 10, 2024 16:35
@danibarranqueroo danibarranqueroo marked this pull request as ready for review September 11, 2024 10:35
@@ -0,0 +1,32 @@
{
"Provider": "aws",
"CheckID": "rds_instance_not_default_port",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"CheckID": "rds_instance_not_default_port",
"CheckID": "rds_instance_non_default_port",

"CheckID": "rds_instance_not_default_port",
"CheckTitle": "Check if RDS instances are using non-default ports.",
"CheckType": [
"Software and Configuration Checks, AWS Security Best Practices"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Software and Configuration Checks, AWS Security Best Practices"
"Software and Configuration Checks", "AWS Security Best Practices"

Comment on lines 47 to 52
if check_security_group(
ingress_rule,
"tcp",
[db_instance.port],
any_address=True,
):
Copy link
Member

Choose a reason for hiding this comment

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

This will only check if the port is public, not if it can be accessed by an IP address, is that what we want?

Copy link
Member

Choose a reason for hiding this comment

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

I would only check the ports for each instance in db_instance.port.

class rds_instance_not_default_port(Check):
def execute(self):
findings = []
default_ports = {
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get this list from?

@MrCloudSec MrCloudSec self-requested a review September 19, 2024 14:14
@MrCloudSec MrCloudSec merged commit 4318396 into prowler-cloud:master Sep 19, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/aws Issues/PRs related with the AWS provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants