Skip to content

Commit

Permalink
Wildcard service improvements (#191)
Browse files Browse the repository at this point in the history
* Add regex, tests and broken wildcard rule test

* Fix policy and test

* Update changelog
  • Loading branch information
jsoucheiron committed Sep 9, 2021
1 parent 71dcd6a commit ecd60bd
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Changelog
All notable changes to this project will be documented in this file.

## [1.0.9] - 2021-09-09
### Improvements
- `WildCardResourceRule` is now triggered by resources that only limit by service (ex: `arn:aws:s3:::*`)

## [1.0.8] - 2021-08-18
### Improvements
- Add `S3LifecycleConfiguraton` rule
Expand Down
15 changes: 15 additions & 0 deletions cfripper/config/regex.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,21 @@
"""
REGEX_ARN = re.compile(r"^arn:aws:(\w+):(\w*):(\d*):(.+)$")


"""
Check for arns that allow the full aws service range in a particular service
Valid:
- arn:aws:s3:::*
- arn:aws:iam:*:*:*
- arn:aws:*:::*
Invalid:
- potato
- arn:aws:s3:::my_corporate_bucket
- arn:aws:iam::437628376:root
"""
REGEX_WILDCARD_ARN = re.compile(r"^arn:aws:([*\w]+):(\*)?:(\*)?:([*?]+)$")


"""
Check for iam arns
It has 2 groups. The first one for account id, the last one for resource id
Expand Down
7 changes: 5 additions & 2 deletions cfripper/rules/wildcard_resource_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from pycfmodel.model.resources.resource import Resource

from cfripper.cloudformation_actions_only_accepts_wildcard import CLOUDFORMATION_ACTIONS_ONLY_ACCEPTS_WILDCARD
from cfripper.config.regex import REGEX_IS_STAR
from cfripper.config.regex import REGEX_IS_STAR, REGEX_WILDCARD_ARN
from cfripper.model.enums import RuleGranularity, RuleMode
from cfripper.model.result import Result
from cfripper.rules.base_rules import ResourceSpecificRule
Expand Down Expand Up @@ -67,7 +67,10 @@ def resource_invoke(self, resource: Resource, logical_id: str, extras: Optional[
def _check_policy_document(
self, result: Result, logical_id: str, policy_document: PolicyDocument, policy_name: Optional[str], extras: Dict
):
for statement in policy_document.statements_with(REGEX_IS_STAR):
statements_to_review = policy_document.statements_with(REGEX_IS_STAR) + policy_document.statements_with(
REGEX_WILDCARD_ARN
)
for statement in statements_to_review:
self._check_statement(result, logical_id, policy_name, statement, extras=extras)

def _check_statement(
Expand Down
6 changes: 6 additions & 0 deletions tests/config/test_regex.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
REGEX_IAM_ARN,
REGEX_IS_STAR,
REGEX_STS_ARN,
REGEX_WILDCARD_ARN,
REGEX_WILDCARD_POLICY_ACTION,
)

Expand Down Expand Up @@ -47,6 +48,11 @@
(REGEX_ARN, "arn:aws:iam::437628376:root", True),
(REGEX_ARN, "arn:aws:s3:::my_corporate_bucket", True),
(REGEX_ARN, "potato", False),
(REGEX_WILDCARD_ARN, "arn:aws:s3:::*", True),
(REGEX_WILDCARD_ARN, "arn:aws:iam:*:*:*", True),
(REGEX_WILDCARD_ARN, "arn:aws:*:::*", True),
(REGEX_WILDCARD_ARN, "arn:aws:s3:::my_corporate_bucket", False),
(REGEX_WILDCARD_ARN, "arn:aws:iam::437628376:root", False),
(REGEX_IAM_ARN, "arn:aws:iam::437628376:not-root", True),
(REGEX_IAM_ARN, "arn:aws:iam::437628376:root", True),
(REGEX_IAM_ARN, "arn:aws:s3:::my_corporate_bucket", False),
Expand Down
28 changes: 28 additions & 0 deletions tests/rules/test_WildcardResourceRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ def iam_policy_with_wildcard_resource_and_wildcard_action_and_condition():
).resolve()


@pytest.fixture()
def policy_with_s3_wildcard_and_all_buckets():
model = get_cfmodel_from("rules/WildcardResourceRule/policy_with_s3_wildcard_and_all_buckets.json")
return model.resolve()


@pytest.fixture()
def user_and_policy_with_wildcard_resource():
return get_cfmodel_from("rules/WildcardResourceRule/multiple_resources_with_wildcard_resources.json").resolve()
Expand Down Expand Up @@ -655,6 +661,28 @@ def test_multiple_resources_with_wildcard_resources_are_detected(user_and_policy
)


def test_policy_s3_wildcard_and_all_buckets(policy_with_s3_wildcard_and_all_buckets):
rule = WildcardResourceRule(None)
rule._config.stack_name = "stack3"
rule.all_cf_actions = set()
result = rule.invoke(policy_with_s3_wildcard_and_all_buckets)

assert result.valid is False
assert (
Failure(
granularity="ACTION",
reason='"RolePolicy" is using a wildcard resource in "Policy for something." for "s3:PutObject"',
risk_value="MEDIUM",
rule="WildcardResourceRule",
rule_mode="BLOCKING",
actions={"s3:*"},
resource_ids={"RolePolicy"},
)
in result.failures
)
assert 100 < len(result.failures)


def test_policy_document_with_wildcard_resource_without_policy_name_is_detected():
with pytest.raises(pydantic.ValidationError):
get_cfmodel_from("rules/WildcardResourceRule/iam_policy_with_wildcard_resource_without_policy_name.json")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"Resources": {
"RolePolicy": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyName": "Policy for something.",
"PolicyDocument": {
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": "s3:*",
"Resource": "arn:aws:s3:::*"
}
]
},
"Roles": [
{
"Ref": "RootRole"
}
]
}
}
}
}

0 comments on commit ecd60bd

Please sign in to comment.