Skip to content

Commit

Permalink
First iteration on a new filter system to allow more granular configu…
Browse files Browse the repository at this point in the history
…ration (#110)

* new_filter_system: add filter class and builder + tests

* new_filter_system: add pydash lobrary

* new_filter_system: Add disabled in RuleMode

* new_filter_system: remove test code

* new_filter_system: add whitelisted to model

* new_filter_system: add support to base model

* new_filter_system: add support for rule config

* new_filter_system: update docs config

* new_filter_system: add rule_config

* new_filter_system: add docs

* new_filter_system: remove default conf

* new_filter_system: update changelog and docs

* new_filter_system: upgrade version

* new_filter_system: update changelog

* new_filter_system: update docs

* new_filter_system: apply comments from PR
  • Loading branch information
oscarbc96 authored Mar 26, 2020
1 parent 99f1a85 commit da12775
Show file tree
Hide file tree
Showing 13 changed files with 385 additions and 28 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
# Changelog
All notable changes to this project will be documented in this file.

## [0.16.0] - 2020-03-27
### Improvements
- Add new `RuleConfig`, allows to overwrite the default behaviour of the rule changing rule mode and risk value.
- Add new `Filter`, allows setting custom rule configuration to matching coincidences.
- New RuleModes supported: `RuleMode.DISABLED` and `RuleMode.WHITELISTED`.
### Breaking changes
- Class variables `Rule.RULE_MODE` and `Rule.RISK_VALUE` should be changed to use properties `rule_mode` and `risk_value`. These properties take in consideration the custom config that might be applied.
- If rule mode is `DISABLED` or `WHITELISTED`; methods `add_failure_to_result` and `add_warning_to_result` will have no effect.
- `add_failure_to_result` and `add_warning_to_result` accepts a new optional parameter named `context`. This variable is going to be evaluated by filters defined in the custom config.

## [0.15.1] - 2020-03-26
### Improvements
- `SecurityGroupOpenToWorldRule` and `SecurityGroupIngressOpenToWorldRule` are now more accurately scoped to block
Expand Down
2 changes: 1 addition & 1 deletion cfripper/__version__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
VERSION = (0, 15, 1)
VERSION = (0, 16, 0)

__version__ = ".".join(map(str, VERSION))
11 changes: 11 additions & 0 deletions cfripper/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import re
from typing import List

from .rule_config import RuleConfig
from .whitelist import AWS_ELASTICACHE_BACKUP_CANONICAL_IDS, AWS_ELB_LOGS_ACCOUNT_IDS
from .whitelist import rule_to_action_whitelist as default_rule_to_action_whitelist
from .whitelist import rule_to_resource_whitelist as default_rule_to_resource_whitelist
Expand Down Expand Up @@ -99,6 +100,7 @@ def __init__(
stack_whitelist=None,
rule_to_action_whitelist=None,
rule_to_resource_whitelist=None,
rules_config=None,
):
self.project_name = project_name
self.service_name = service_name
Expand Down Expand Up @@ -138,6 +140,15 @@ def __init__(

# Set up a string list of allowed principals. If kept empty it will allow any AWS principal
self.aws_principals = aws_principals if aws_principals is not None else []
self.rules_config = rules_config if rules_config is not None else {}

def get_rule_config(self, rule_name: str) -> RuleConfig:
rule_config = self.rules_config.get(rule_name)
if rule_config is None:
return RuleConfig()
elif isinstance(rule_config, RuleConfig):
return rule_config
return RuleConfig(**rule_config)

def get_whitelisted_actions(self, rule_name: str) -> List[str]:
allowed_actions = []
Expand Down
53 changes: 53 additions & 0 deletions cfripper/config/filter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import operator
import re
from typing import Any, Callable, Dict, List, Optional, Union

from pydantic import BaseModel, validator
from pydash.objects import get

from cfripper.model.enums import RuleMode, RuleRisk

IMPLEMENTED_FILTER_FUNCTIONS = {
"eq": lambda *args, **kwargs: operator.eq(*args),
"ne": lambda *args, **kwargs: operator.ne(*args),
"lt": lambda *args, **kwargs: operator.lt(*args),
"gt": lambda *args, **kwargs: operator.gt(*args),
"le": lambda *args, **kwargs: operator.le(*args),
"ge": lambda *args, **kwargs: operator.ge(*args),
"not": lambda *args, **kwargs: operator.not_(*args),
"or": lambda *args, **kwargs: any(args),
"and": lambda *args, **kwargs: all(args),
"in": lambda a, b, *args, **kwargs: operator.contains(b, a),
"regex": lambda *args, **kwargs: bool(re.match(*args)),
"ref": lambda param_name, *args, **kwargs: get(kwargs, param_name),
}


def is_resolvable_dict(value: Any) -> bool:
return isinstance(value, dict) and len(value) == 1 and next(iter(value)) in IMPLEMENTED_FILTER_FUNCTIONS


def build_evaluator(tree: Union[str, int, float, bool, List, Dict]) -> Callable:
if is_resolvable_dict(tree):
function_name, nodes = list(tree.items())[0]
if not isinstance(nodes, list):
nodes = [nodes]
nodes = [build_evaluator(node) for node in nodes]
function_resolver = IMPLEMENTED_FILTER_FUNCTIONS[function_name]
return lambda kwargs: function_resolver(*[node(kwargs) for node in nodes], **kwargs)

return lambda kwargs: tree


class Filter(BaseModel):
reason: str = ""
eval: Union[Dict, Callable]
rule_mode: Optional[RuleMode] = None
risk_value: Optional[RuleRisk] = None

@validator("eval", pre=True)
def set_eval(cls, eval):
return build_evaluator(eval)

def __call__(self, **kwargs):
return self.eval(kwargs)
12 changes: 12 additions & 0 deletions cfripper/config/rule_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from typing import List, Optional

from pydantic import BaseModel

from cfripper.config.filter import Filter
from cfripper.model.enums import RuleMode, RuleRisk


class RuleConfig(BaseModel):
rule_mode: Optional[RuleMode] = None
risk_value: Optional[RuleRisk] = None
filters: List[Filter] = []
2 changes: 2 additions & 0 deletions cfripper/model/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class RuleMode(str, Enum):
BLOCKING = "BLOCKING"
MONITOR = "MONITOR"
DEBUG = "DEBUG"
WHITELISTED = "WHITELISTED"
DISABLED = "DISABLED"


class RuleRisk(str, Enum):
Expand Down
67 changes: 48 additions & 19 deletions cfripper/rules/base_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from pycfmodel.model.cf_model import CFModel

from cfripper.config.config import Config
from cfripper.config.rule_config import RuleConfig
from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
from cfripper.model.result import Failure, Result

Expand All @@ -33,6 +34,18 @@ class Rule(ABC):
def __init__(self, config: Optional[Config]):
self._config = config if config else Config()

@property
def rule_config(self) -> RuleConfig:
return self._config.get_rule_config(self.__class__.__name__)

@property
def rule_mode(self) -> RuleMode:
return self.rule_config.rule_mode or self.RULE_MODE

@property
def risk_value(self) -> RuleRisk:
return self.rule_config.risk_value or self.RISK_VALUE

@abstractmethod
def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
pass
Expand All @@ -46,16 +59,24 @@ def add_failure_to_result(
actions: Optional[Set] = None,
risk_value: Optional[RuleRisk] = None,
rule_mode: Optional[RuleMode] = None,
context: Optional[Dict] = None,
):
result.add_failure(
rule=type(self).__name__,
reason=reason,
rule_mode=rule_mode or self.RULE_MODE,
risk_value=risk_value or self.RISK_VALUE,
resource_ids=resource_ids,
actions=actions,
granularity=granularity or self.GRANULARITY,
)
rule_mode = rule_mode or self.rule_mode
risk_value = risk_value or self.risk_value
for fltr in self.rule_config.filters:
if fltr(**context):
risk_value = fltr.risk_value or risk_value
rule_mode = fltr.rule_mode or rule_mode
if rule_mode not in (RuleMode.DISABLED, RuleMode.WHITELISTED):
result.add_failure(
rule=type(self).__name__,
reason=reason,
rule_mode=rule_mode,
risk_value=risk_value,
resource_ids=resource_ids,
actions=actions,
granularity=granularity or self.GRANULARITY,
)

def add_warning_to_result(
self,
Expand All @@ -66,17 +87,25 @@ def add_warning_to_result(
actions: Optional[Set] = None,
risk_value: Optional[RuleRisk] = None,
rule_mode: Optional[RuleMode] = None,
context: Optional[Dict] = None,
):
warning = Failure(
rule=type(self).__name__,
reason=reason,
granularity=granularity or self.GRANULARITY,
resource_ids=resource_ids,
actions=actions,
risk_value=risk_value or self.RISK_VALUE,
rule_mode=rule_mode or self.RULE_MODE,
)
result.add_warning(warning)
rule_mode = rule_mode or self.rule_mode
risk_value = risk_value or self.risk_value
for fltr in self.rule_config.filters:
if fltr(**context):
risk_value = fltr.risk_value or risk_value
rule_mode = fltr.rule_mode or rule_mode
if rule_mode not in (RuleMode.DISABLED, RuleMode.WHITELISTED):
warning = Failure(
rule=type(self).__name__,
reason=reason,
granularity=granularity or self.GRANULARITY,
resource_ids=resource_ids,
actions=actions,
risk_value=risk_value,
rule_mode=rule_mode,
)
result.add_warning(warning)


class PrincipalCheckingRule(Rule):
Expand Down
2 changes: 1 addition & 1 deletion docs/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def process_paragraph(paragraph: str) -> str:


def parse_doc_string(doc):
sections = ["Risk", "Fix", "Code for fix"]
sections = ["Risk", "Fix", "Code for fix", "Filters context"]
result = OrderedDict()
pattern_for_paragraphs = regex_for_splitting_paragraphs(sections)
paragraphs = pattern_for_paragraphs.split(doc.strip())
Expand Down
59 changes: 59 additions & 0 deletions docs/rule_config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
Allows to overwrite the default behaviour of the rule, such as changing the rule mode and risk value. It accepts a more
granular configuration using the filter.

{{ inline_source('cfripper.config.rule_config.RuleConfig') }}

## Filters

When adding a failure or warning it will check if there is a filter that matches the current context and set the new
risk or mode. Context depends on each rule and is available inside each rule's documentation.
The object accepts a reason parameter to say why that filter exists.

{{ inline_source('cfripper.config.filter.Filter') }}

!!! warning
Only available for the following rules:

- Not supported yet

### Filter preference

Following the cascade style, takes preference always the last value set following this structure:

```
Rule Standard -> Rule Config -> Filter #1 -> ... -> Filter #N
```


### Implemented filter functions
| Function | Description | Example |
|:----------:|:---------------------------------------------------------------------------:|:---------------------------------------:|
| `eq` | Same as a == b | `{"eq": ["string", "string"]}` |
| `ne` | Same as a != b | `{"ne": ["string", "not_that_string"]}` |
| `lt` | Same as a < b | `{"lt": [0, 1]}` |
| `gt` | Same as a > b | `{"gt": [1, 0]}` |
| `le` | Same as a <= b | `{"le": [1, 1]}` |
| `ge` | Same as a >= b | `{"ge": [1, 1]}` |
| `not` | Same as not a | `{"not": True}` |
| `or` | True if any arg is True | `{"or": [False, True]}` |
| `and` | True if all args are True | `{"and": [True, True]}` |
| `in` | Same as a in b | `{"in": ["b", ["a", "b"]]}` |
| `regex` | True if b match pattern a | `{"regex": [r"^\d+$", "5"]}` |
| `ref` | Get the value at any depth of the context based on the path described by a. | `{"ref": "param_a.param_b"}` |

### Examples

Disable the rule if the role name is prefixed with `sandbox-` and the principal equals `arn:aws:iam::123456789012:role/test-role`.
```python3
Filter(
reason="",
rule_mode=RuleMode.DISABLED,
eval={
"and": [
{"regex": ["^sandbox-.*$", {"ref": "resource.Properties.RoleName"}]},
{"eq": [{"ref": "principal"}, "arn:aws:iam::123456789012:role/test-role"]},
]
},
)
```

2 changes: 2 additions & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ nav:
- Home: index.md
- CLI: cli.md
- Rules: rules.md
- Rule Config: rule_config.md
- Changelog: changelog.md
- Contributing: contributing.md
- Code of conduct: code_of_conduct.md

markdown_extensions:
- admonition
- codehilite
- toc:
permalink: true
Expand Down
13 changes: 7 additions & 6 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@
#
# make freeze
#
boto3==1.12.13
botocore==1.15.13 # via boto3, s3transfer
cfn-flip==1.2.2
click==7.0 # via cfn-flip
boto3==1.12.29 # via cfripper (setup.py)
botocore==1.15.29 # via boto3, s3transfer
cfn-flip==1.2.2 # via cfripper (setup.py)
click==7.1.1 # via cfn-flip
docutils==0.15.2 # via botocore
jmespath==0.9.5 # via boto3, botocore
pycfmodel==0.7.0
pycfmodel==0.7.0 # via cfripper (setup.py)
pydantic==1.4 # via pycfmodel
pydash==4.7.6 # via cfripper (setup.py)
python-dateutil==2.8.1 # via botocore
pyyaml==5.3
pyyaml==5.3.1 # via cfn-flip, cfripper (setup.py)
s3transfer==0.3.3 # via boto3
six==1.14.0 # via cfn-flip, python-dateutil
urllib3==1.25.8 # via botocore
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

project_root_path = Path(__file__).parent

install_requires = ["boto3>=1.4.7,<2", "PyYAML>=4.2b1", "pycfmodel>=0.7.0", "cfn_flip>=1.2.0"]
install_requires = ["boto3>=1.4.7,<2", "cfn_flip>=1.2.0", "pycfmodel>=0.7.0", "pydash~=4.7.6", "PyYAML>=4.2b1"]

dev_requires = [
"black==19.10b0",
Expand Down
Loading

0 comments on commit da12775

Please sign in to comment.