Skip to content

Commit

Permalink
Merge pull request #152 from Skyscanner/update-rules-to-use-context
Browse files Browse the repository at this point in the history 
Update all rules and documentation to include a filters context
  • Loading branch information
@ocrawford555
ocrawford555 committed Feb 15, 2021
2 parents 4c0d1b3 + 86a88b2 commit 1d80ad7
Show file tree
Hide file tree
Showing 38 changed files with 563 additions and 79 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
All notable changes to this project will be documented in this file.

## [0.23.3] - 2021-02-11
### Additions
- All rules now support filter contexts!
### Improvements
- Update `WildcardResourceRule` to allow for certain resources to be excluded.

Expand Down
15 changes: 14 additions & 1 deletion cfripper/rules/cloudformation_authentication.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ class CloudFormationAuthenticationRule(Rule):
Ref: "PasswordAuth"
...
````
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` | `Resource` | Resource that is being addressed |
"""

REASON = "Hardcoded credentials in {}"
Expand All @@ -46,5 +54,10 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
result = Result()
for logical_id, resource in cfmodel.Resources.items():
if resource.has_hardcoded_credentials():
self.add_failure_to_result(result, self.REASON.format(logical_id), resource_ids={logical_id})
self.add_failure_to_result(
result,
self.REASON.format(logical_id),
resource_ids={logical_id},
context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
)
return result
20 changes: 19 additions & 1 deletion cfripper/rules/ebs_volume_has_sse.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ class EBSVolumeHasSSERule(Rule):
}
}
````
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` | `Resource` | EC2 Volume that is being addressed |
"""

REASON = "EBS volume {} should have server-side encryption enabled"
Expand All @@ -42,5 +50,15 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
for logical_id, resource in cfmodel.Resources.items():
if resource.Type == "AWS::EC2::Volume":
if resource.Properties.get("Encrypted") != "true":
self.add_failure_to_result(result, self.REASON.format(logical_id), resource_ids={logical_id})
self.add_failure_to_result(
result,
self.REASON.format(logical_id),
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
},
)
return result
26 changes: 21 additions & 5 deletions cfripper/rules/hardcoded_RDS_password.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ class HardcodedRDSPasswordRule(Rule):
MasterUserPassword: !Ref 'MasterUserPassword'
...
````
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` | `Resource` | Resource that is being addressed |
"""

REASON_DEFAULT = "Default RDS {} password parameter (readable in plain-text) for {}."
Expand All @@ -60,7 +68,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
for logical_id, resource in cfmodel.Resources.items():
# flag insecure RDS Clusters.
if resource.Type == "AWS::RDS::DBCluster":
failure_added = self._failure_added(result, logical_id, resource)
failure_added = self._failure_added(result, logical_id, resource, extras)
if not failure_added:
password_protected_cluster_ids.append(logical_id)

Expand All @@ -76,20 +84,28 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
):
continue

self._failure_added(result, logical_id, resource)
self._failure_added(result, logical_id, resource, extras)
return result

def _failure_added(self, result: Result, logical_id: str, resource: GenericResource) -> bool:
def _failure_added(
self, result: Result, logical_id: str, resource: GenericResource, extras: Optional[Dict] = None
) -> bool:
master_user_password = resource.Properties.get("MasterUserPassword", Parameter.NO_ECHO_NO_DEFAULT)
resource_type = resource.Type.replace("AWS::RDS::DB", "")
if master_user_password == Parameter.NO_ECHO_WITH_DEFAULT:
self.add_failure_to_result(
result, self.REASON_DEFAULT.format(resource_type, logical_id), resource_ids={logical_id}
result,
self.REASON_DEFAULT.format(resource_type, logical_id),
resource_ids={logical_id},
context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
)
return True
elif master_user_password not in (Parameter.NO_ECHO_NO_DEFAULT, Parameter.NO_ECHO_WITH_VALUE):
self.add_failure_to_result(
result, self.REASON_MISSING_NOECHO.format(resource_type, logical_id), resource_ids={logical_id},
result,
self.REASON_MISSING_NOECHO.format(resource_type, logical_id),
resource_ids={logical_id},
context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
)
return True

Expand Down
63 changes: 57 additions & 6 deletions cfripper/rules/iam_roles.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
class IAMRolesOverprivilegedRule(Rule):
"""
Rule that checks for wildcards in resources for a set of actions and restricts managed policies.
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` | `IAMRole` | Resource that is being addressed |
|`statement` | `Optional[Statement]` | Statement being checked found in the Resource |
|`action` | `Optional[str]` | Action containing insecure permission with forbidden prefix |
"""

GRANULARITY = RuleGranularity.RESOURCE
Expand All @@ -23,11 +33,11 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
result = Result()
for logical_id, resource in cfmodel.Resources.items():
if isinstance(resource, IAMRole):
self.check_managed_policies(result, logical_id, resource)
self.check_inline_policies(result, logical_id, resource)
self.check_managed_policies(result, logical_id, resource, extras)
self.check_inline_policies(result, logical_id, resource, extras)
return result

def check_managed_policies(self, result: Result, logical_id: str, role: IAMRole):
def check_managed_policies(self, result: Result, logical_id: str, role: IAMRole, extras: Optional[Dict] = None):
"""Run the managed policies against a blacklist."""
if not role.Properties.ManagedPolicyArns:
return
Expand All @@ -38,9 +48,17 @@ def check_managed_policies(self, result: Result, logical_id: str, role: IAMRole)
result,
f"Role {logical_id} has forbidden Managed Policy {managed_policy_arn}",
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": role,
"statement": None,
"action": None,
},
)

def check_inline_policies(self, result: Result, logical_id: str, role: IAMRole):
def check_inline_policies(self, result: Result, logical_id: str, role: IAMRole, extras: Optional[Dict] = None):
"""Check conditional and non-conditional inline policies."""
if not role.Properties.Policies:
return
Expand All @@ -56,13 +74,29 @@ def check_inline_policies(self, result: Result, logical_id: str, role: IAMRole):
f"Role '{logical_id}' contains an insecure permission '{action}' in policy "
f"'{policy.PolicyName}'",
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": role,
"statement": statement,
"action": action,
},
)


class IAMRoleWildcardActionOnPolicyRule(Rule):
"""
Checks for use of wildcard characters in all IAM Role policies (including `AssumeRolePolicyDocument`)
and [AWS Managed Policies](https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_managed-vs-inline.html).
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` | `Union[IAMRole, IAMManagedPolicy]` | Resource that is being addressed |
"""

GRANULARITY = RuleGranularity.RESOURCE
Expand All @@ -75,7 +109,15 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
# check AssumeRolePolicyDocument.
if resource.Properties.AssumeRolePolicyDocument.allowed_actions_with(REGEX_WILDCARD_POLICY_ACTION):
self.add_failure_to_result(
result, self.REASON.format(logical_id, "AssumeRolePolicy"), resource_ids={logical_id},
result,
self.REASON.format(logical_id, "AssumeRolePolicy"),
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
},
)

# check other policies of the IAM role.
Expand All @@ -86,13 +128,22 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
result,
self.REASON.format(logical_id, f"{policy.PolicyName} policy"),
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
},
)

# check AWS::IAM::ManagedPolicy.
elif isinstance(resource, IAMManagedPolicy) and resource.Properties.PolicyDocument.allowed_actions_with(
REGEX_WILDCARD_POLICY_ACTION
):
self.add_failure_to_result(
result, self.REASON.format(logical_id, "AWS::IAM::ManagedPolicy"), resource_ids={logical_id},
result,
self.REASON.format(logical_id, "AWS::IAM::ManagedPolicy"),
resource_ids={logical_id},
context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
)
return result
22 changes: 21 additions & 1 deletion cfripper/rules/kms_key_wildcard_principal.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@
class KMSKeyWildcardPrincipalRule(Rule):
"""
Check for wildcards in principals in KMS Policies.
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` | `KMSKey` | Resource that is being addressed |
|`statement` | `Statement` | Statement being checked found in the Resource |
|`principal` | str | AWS Principal being checked found in the statement |
"""

GRANULARITY = RuleGranularity.RESOURCE
Expand All @@ -38,6 +48,16 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
)
else:
self.add_failure_to_result(
result, self.REASON.format(logical_id), resource_ids={logical_id}
result,
self.REASON.format(logical_id),
resource_ids={logical_id},
context={
"config": self._config,
"extras": extras,
"logical_id": logical_id,
"resource": resource,
"statement": statement,
"principal": principal,
},
)
return result
15 changes: 14 additions & 1 deletion cfripper/rules/managed_policy_on_user.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ class ManagedPolicyOnUserRule(Rule):
}
}
```
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` | `IAMManagedPolicy` | Resource that is being addressed |
"""

REASON = "IAM managed policy {} should not apply directly to users. Should be on group"
Expand All @@ -69,5 +77,10 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
result = Result()
for logical_id, resource in cfmodel.Resources.items():
if isinstance(resource, IAMManagedPolicy) and resource.Properties.Users:
self.add_failure_to_result(result, self.REASON.format(logical_id), resource_ids={logical_id})
self.add_failure_to_result(
result,
self.REASON.format(logical_id),
resource_ids={logical_id},
context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
)
return result

0 comments on commit 1d80ad7

Please sign in to comment.