Skip to content

Commit

Permalink
RDSSecurityGroupIngressOpenToWorldRule (#226)
Browse files Browse the repository at this point in the history
* RDSSecurityGroupIngressOpenToWorldRule

* Rename template files

* Improve naming

* Update cfripper/rules/rds_security_group.py

Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>

* Update cfripper/rules/rds_security_group.py

Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>

* Update tests/rules/test_RDSSecurityGroupIngressOpenToWorldRule.py

Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>

* Update tests/rules/test_RDSSecurityGroupIngressOpenToWorldRule.py

Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>

* fix lint

* bump pycfmodel in requirements

Co-authored-by: Ramon <w0rmr1d3r@users.noreply.github.com>
Co-authored-by: Ramon <ramon.guimera@skyscanner.net>
  • Loading branch information
3 people committed May 23, 2022
1 parent 8c8c924 commit 5dc729b
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Changelog
All notable changes to this project will be documented in this file.

## [1.10.0]
### Additions
- New rule: `RDSSecurityGroupIngressOpenToWorldRule`
### Updates
- Bumped minimum `pycfmodel` version to `0.20.0`

## [1.9.0]
### Improvements
- CFRipper is now compatible with Python3.10
Expand Down
2 changes: 2 additions & 0 deletions cfripper/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from cfripper.rules.managed_policy_on_user import ManagedPolicyOnUserRule
from cfripper.rules.policy_on_user import PolicyOnUserRule
from cfripper.rules.privilege_escalation import PrivilegeEscalationRule
from cfripper.rules.rds_security_group import RDSSecurityGroupIngressOpenToWorldRule
from cfripper.rules.s3_bucket_policy import S3BucketPolicyPrincipalRule
from cfripper.rules.s3_lifecycle_configuration import S3LifecycleConfigurationRule
from cfripper.rules.s3_object_versioning import S3ObjectVersioningRule
Expand Down Expand Up @@ -77,6 +78,7 @@
PartialWildcardPrincipalRule,
PolicyOnUserRule,
PrivilegeEscalationRule,
RDSSecurityGroupIngressOpenToWorldRule,
S3BucketPolicyPrincipalRule,
S3LifecycleConfigurationRule,
S3BucketPolicyWildcardActionRule,
Expand Down
93 changes: 93 additions & 0 deletions cfripper/rules/rds_security_group.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
from typing import Optional, Union

from pycfmodel.model.resources.security_group import RDSDBSecurityGroup
from pycfmodel.model.resources.security_group_ingress import RDSDBSecurityGroupIngress

from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
from cfripper.model.result import Result
from cfripper.rules import ResourceSpecificRule


class RDSSecurityGroupIngressOpenToWorldRule(ResourceSpecificRule):
"""
Checks if RDS native security groups have an ingress IP that is open to the world.
Fix:
Most security groups only need to be [accessed privately](https://en.wikipedia.org/wiki/Private_network), and
this can typically be done by specifying the CIDR of a Security Group's ingress to `10.0.0.0/8` or similar.
Unless required, do not use the following IP ranges in your Security Group Ingress:
- `0.0.0.0/0`.
- Any `/8` that does not start with 10.
- `172/8` or `192/8` (use `172.16/12` and `192.168/16` ranges, per RFC1918 specification).
As per RFC4193, `fd00::/8` IPv6 addresses should be used to define a private network.
Code example:
````yaml
Resources:
CompliantRDSSecurityGroup:
Type: AWS::RDS::DBSecurityGroup
Properties:
EC2VpcId: "vpc-id"
DBSecurityGroupIngress:
- CIDRIP: 10.0.0.0/8
GroupDescription: Compliant RDS security group
NonCompliantRDSSecurityGroup:
Type: AWS::RDS::DBSecurityGroup
Properties:
EC2VpcId: "vpc-id"
DBSecurityGroupIngress:
- CIDRIP: 0.0.0.0/0
GroupDescription: Risky RDS security group
````
Filters context:
| Parameter | Type | Description |
|:-----------------------:|:----------------------------------------------:|:--------------------------------------------------------------:|
|`config` | str | `config` variable available inside the rule |
|`extras` | str | `extras` variable available inside the rule |
|`logical_id` | str | ID used in Cloudformation to refer the resource being analysed |
|`resource` | `RDSDBSecurityGroup/RDSDBSecurityGroupIngress` | Resource that is being addressed |
|`ingress_obj` | `DBSecurityGroupIngressProp` | DBSecurityGroupIngress being checked found in the Resource |
"""

GRANULARITY = RuleGranularity.RESOURCE
RESOURCE_TYPES = (RDSDBSecurityGroup, RDSDBSecurityGroupIngress)
RULE_MODE = RuleMode.BLOCKING
RISK_VALUE = RuleRisk.HIGH
REASON = "RDS DB Security group {} should not have ingress open to the world"

def resource_invoke(
self,
resource: Union[RDSDBSecurityGroup, RDSDBSecurityGroupIngress],
logical_id: str,
extras: Optional[dict] = None,
) -> Result:
result = Result()

if isinstance(resource, RDSDBSecurityGroupIngress):
ingress_collection = [resource.Properties]
else: # isinstance(resource, RDSDBSecurityGroup)
ingress_collection = resource.Properties.DBSecurityGroupIngress

for ingress_entry in ingress_collection:
if ingress_entry.is_public():
self.add_failure_to_result(
result,
self.REASON.format(logical_id),
resource_ids={logical_id},
resource_types={resource.Type},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
"cirdip": ingress_entry.CIDRIP,
},
)
return result
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ cfn-flip==1.3.0
click==8.1.2
jmespath==1.0.0
pluggy==0.13.1
pycfmodel==0.18.0
pycfmodel==0.20.0
pydantic==1.9.0
pydash==4.7.6
python-dateutil==2.8.2
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"cfn_flip>=1.2.0",
"click>=8.0.0",
"pluggy~=0.13.1",
"pycfmodel>=0.18.0",
"pycfmodel>=0.20.0",
"pydash~=4.7.6",
"PyYAML>=4.2b1",
]
Expand Down
73 changes: 73 additions & 0 deletions tests/rules/test_RDSSecurityGroupIngressOpenToWorldRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
from pytest import fixture

from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
from cfripper.model.result import Failure
from cfripper.rules import RDSSecurityGroupIngressOpenToWorldRule
from tests.utils import compare_lists_of_failures, get_cfmodel_from


@fixture()
def rds_sg():
return get_cfmodel_from("rules/RDSSecurityGroupIngressOpenToWorldRule/rds_sg.yaml").resolve()


@fixture()
def rds_sg_ingress():
return get_cfmodel_from("rules/RDSSecurityGroupIngressOpenToWorldRule/rds_sg_ingress.yaml").resolve()


def test_dangerous_rds_securitygroup(rds_sg):
rule = RDSSecurityGroupIngressOpenToWorldRule(None)
result = rule.invoke(rds_sg)

assert not result.valid
assert compare_lists_of_failures(
result.failures,
[
Failure(
actions=None,
granularity=RuleGranularity.RESOURCE,
reason=(
"RDS DB Security group NonCompliantRDSSecurityGroup " + "should not have ingress open to the world"
),
risk_value=RuleRisk.HIGH,
rule="RDSSecurityGroupIngressOpenToWorldRule",
rule_mode=RuleMode.BLOCKING,
resource_ids={"NonCompliantRDSSecurityGroup"},
resource_types={"AWS::RDS::DBSecurityGroup"},
)
],
)


def test_dangerous_rds_securitygroup_ingress(rds_sg_ingress):
rule = RDSSecurityGroupIngressOpenToWorldRule(None)
result = rule.invoke(rds_sg_ingress)

assert not result.valid
assert compare_lists_of_failures(
result.failures,
[
Failure(
actions=None,
granularity=RuleGranularity.RESOURCE,
reason=(
"RDS DB Security group NonCompliantRDSSecurityGroupIngress "
+ "should not have ingress open to the world"
),
risk_value=RuleRisk.HIGH,
rule="RDSSecurityGroupIngressOpenToWorldRule",
rule_mode=RuleMode.BLOCKING,
resource_ids={"NonCompliantRDSSecurityGroupIngress"},
resource_types={"AWS::RDS::DBSecurityGroupIngress"},
)
],
)


def test_rule_supports_filter_config(rds_sg, default_allow_all_config):
rule = RDSSecurityGroupIngressOpenToWorldRule(default_allow_all_config)
result = rule.invoke(rds_sg)

assert result.valid
assert compare_lists_of_failures(result.failures, [])
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Resources:
CompliantRDSSecurityGroup:
Type: AWS::RDS::DBSecurityGroup
Properties:
EC2VpcId: "vpc-id"
DBSecurityGroupIngress:
- CIDRIP: 10.0.0.0/8
GroupDescription: Compliant RDS security group
NonCompliantRDSSecurityGroup:
Type: AWS::RDS::DBSecurityGroup
Properties:
EC2VpcId: "vpc-id"
DBSecurityGroupIngress:
- CIDRIP: 0.0.0.0/0
GroupDescription: Risky RDS security group
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Resources:
CompliantRDSSecurityGroupIngress:
Type: AWS::RDS::DBSecurityGroupIngress
Properties:
CIDRIP: 10.0.0.0/8
DBSecurityGroupName: RDS security group ingress open to a private range
NonCompliantRDSSecurityGroupIngress:
Type: AWS::RDS::DBSecurityGroupIngress
Properties:
CIDRIP: 0.0.0.0/0
DBSecurityGroupName: Risky RDS security group ingress

0 comments on commit 5dc729b

Please sign in to comment.