Skip to content

Commit

Permalink
Add S3LifecycleConfiguration rule (#188)
Browse files Browse the repository at this point in the history
Co-authored-by: Oliver Crawford <oliver.crawford@skyscanner.net>
  • Loading branch information
ocrawford555 and Oliver Crawford committed Aug 19, 2021
1 parent 0b46775 commit 71dcd6a
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 1 deletion.
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.8] - 2021-08-18
### Improvements
- Add `S3LifecycleConfiguraton` rule

## [1.0.7] - 2021-08-16
### Improvements
- Add `KMSKeyEnabledKeyRotation` rule
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 = (1, 0, 7)
VERSION = (1, 0, 8)

__version__ = ".".join(map(str, VERSION))
2 changes: 2 additions & 0 deletions cfripper/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from cfripper.rules.policy_on_user import PolicyOnUserRule
from cfripper.rules.privilege_escalation import PrivilegeEscalationRule
from cfripper.rules.s3_bucket_policy import S3BucketPolicyPrincipalRule
from cfripper.rules.s3_lifecycle_configuration import S3LifecycleConfigurationRule
from cfripper.rules.s3_public_access import (
S3BucketPublicReadAclAndListStatementRule,
S3BucketPublicReadAclRule,
Expand Down Expand Up @@ -61,6 +62,7 @@
PolicyOnUserRule,
PrivilegeEscalationRule,
S3BucketPolicyPrincipalRule,
S3LifecycleConfigurationRule,
S3BucketPolicyWildcardActionRule,
S3BucketPublicReadAclAndListStatementRule,
S3BucketPublicReadWriteAclRule,
Expand Down
63 changes: 63 additions & 0 deletions cfripper/rules/s3_lifecycle_configuration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
__all__ = ["S3LifecycleConfigurationRule"]

from typing import Dict, Optional

from pycfmodel.model.cf_model import CFModel

from cfripper.model.enums import RuleGranularity, RuleRisk
from cfripper.model.result import Result
from cfripper.rules.base_rules import Rule


class S3LifecycleConfigurationRule(Rule):
"""
Checks for the presence of `LifecycleConfiguration` on S3 buckets.
These rules can help with security, compliance, and reduce AWS Costs. The rule does not
check the specific rules contained with the `LifecycleConfiguration` key.
Fix:
Add `LifecycleConfiguration` property to the S3 Bucket as defined in
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-s3-bucket-lifecycleconfig.html.
Code for fix:
An example rule is included within the configuration.
````yml
Resources:
S3Bucket:
Type: AWS::S3::Bucket
Properties:
...
LifecycleConfiguration:
Rules:
- Status: Enabled
Prefix: logs/
ExpirationInDays: 7
...
````
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` | `S3BucketPolicy` | Resource that is being addressed |
|`bucket_name` | str | Name of the S3 bucket being analysed |
"""

GRANULARITY = RuleGranularity.RESOURCE
REASON = "S3 Bucket {} is required to contain a LifecycleConfiguration property"
RISK_VALUE = RuleRisk.LOW

def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
result = Result()
for logical_id, resource in cfmodel.resources_filtered_by_type(("AWS::S3::Bucket",)).items():
if hasattr(resource, "Properties") and resource.Properties.get("LifecycleConfiguration") is None:
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
57 changes: 57 additions & 0 deletions tests/rules/test_S3LifecycleConfigurationRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import pytest
from pytest import fixture

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


@fixture()
def bad_template_no_configuration():
return get_cfmodel_from("rules/S3LifecycleConfiguration/bad_template_no_configurations.yaml").resolve()


@pytest.mark.parametrize(
"template_path",
[
"rules/S3LifecycleConfiguration/good_template.yaml",
"rules/S3LifecycleConfiguration/allowed_template_malformed_lifecycle_rules.yaml",
],
)
def test_no_failures_are_raised(template_path):
rule = S3LifecycleConfigurationRule(None)
result = rule.invoke(get_cfmodel_from(template_path).resolve())

assert result.valid
assert compare_lists_of_failures(result.failures, [])


def test_failures_are_raised(bad_template_no_configuration):
rule = S3LifecycleConfigurationRule(Config())
result = rule.invoke(bad_template_no_configuration)

assert not result.valid
assert compare_lists_of_failures(
result.failures,
[
Failure(
granularity=RuleGranularity.RESOURCE,
reason="S3 Bucket OutputBucket is required to contain a LifecycleConfiguration property",
risk_value=RuleRisk.LOW,
rule="S3LifecycleConfigurationRule",
rule_mode=RuleMode.BLOCKING,
actions=None,
resource_ids={"OutputBucket"},
)
],
)


def test_rule_supports_filter_config(bad_template_no_configuration, default_allow_all_config):
rule = S3LifecycleConfigurationRule(default_allow_all_config)
result = rule.invoke(bad_template_no_configuration)

assert result.valid
assert compare_lists_of_failures(result.failures, [])
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Resources:
OutputBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: "foo"
AccessControl: BucketOwnerFullControl
LifecycleConfiguration:
# This is not valid for LifecycleConfiguration, but CFRipper will not parse it right now.
- aa
- bb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Resources:
OutputBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: "foo"
AccessControl: BucketOwnerFullControl
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Resources:
OutputBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: "foo"
AccessControl: BucketOwnerFullControl
LifecycleConfiguration:
Rules:
- Status: Enabled
Prefix: logs/
ExpirationInDays: !Ref LogsExpirationInDays
- Status: Enabled
Prefix: output/
ExpirationInDays: !Ref ModelsExpirationInDays
- AbortIncompleteMultipartUpload:
DaysAfterInitiation: 7
Status: Enabled

0 comments on commit 71dcd6a

Please sign in to comment.