Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecurityHub finding doesn't work for rds-cluster-snapshot resource in config-rule mode #5780

Closed
darrendao opened this issue May 18, 2020 · 3 comments · Fixed by #5802
Closed
Labels

Comments

@darrendao
Copy link
Collaborator

Describe the bug
SecurityHub post-finding doesn't work on rds-cluster-snapshot resource in config-rule policy mode. This is because the id/name/arn for rds-cluster-snapshot is slightly different when coming from AWS Config vs when doing Describe

To Reproduce
Steps to reproduce the behavior:

  • Set up a policy to run in config-rule mode for rds-cluster-snapshot resource, filter on StorageEncrypted=false with action = post-finding to security hub.
  • For example:
policies:
  - name: ddao-rds-cluster-snapshot
    resource: rds-cluster-snapshot
    mode:
       type: config-rule
       timeout: 300
       role: arn:aws:iam::123456789012:role/CustodianRole
    filters:
      - type: value
        key: "StorageEncrypted"
        value: false
    action:
      - type: post-finding
        severity_normalized: 30 
        types:
          - "Software and Configuration Checks/AWS Security Best Practices"
  • Create a snapshot for an encrypted RDS DB cluster

Expected behavior
A finding should be sent to security hub regarding the unencrypted RDS cluster snapshot

Actual behavior
Cloud Custodian threw the following error:

{
  "errorMessage": "'DBClusterSnapshotIdentifier'",
  "errorType": "KeyError",
  "stackTrace": [
    "  File \"/var/task/custodian_policy.py\", line 6, in run\n    return handler.dispatch_event(event, context)\n",
    "  File \"/var/task/c7n/handler.py\", line 176, in dispatch_event\n    p.push(event, context)\n",
    "  File \"/var/task/c7n/policy.py\", line 1130, in push\n    return mode.run(event, lambda_ctx)\n",
    "  File \"/var/task/c7n/policy.py\", line 854, in run\n    resources = super(ConfigRuleMode, self).run(event, lambda_context)\n",
    "  File \"/var/task/c7n/policy.py\", line 461, in run\n    return self.run_resource_set(event, resources)\n",
    "  File \"/var/task/c7n/policy.py\", line 491, in run_resource_set\n    results = action.process(resources)\n",
    "  File \"/var/task/c7n/resources/securityhub.py\", line 419, in process\n    finding = self.get_finding(\n",
    "  File \"/var/task/c7n/resources/securityhub.py\", line 467, in get_finding\n    [r[model.id] for r in resources]))).encode(\n",
    "  File \"/var/task/c7n/resources/securityhub.py\", line 467, in <listcomp>\n    [r[model.id] for r in resources]))).encode(\n"
  ]
}

Additional context
I did some digging into the code and here's the problem: For rds-cluster-snapshot, we have its name and id set to DBClusterSnapshotIdentifier (

name = id = 'DBClusterSnapshotIdentifier'
), but when the policy is running in config-rule mode, AWS Config has the resource name/id as dbclusterSnapshotIdentifier , which then gets turned into DbclusterSnapshotIdentifier when we do resolve_resource. Then, in the get_finding method of securityhub, we expect the dictionary to have DBClusterSnapshotIdentifier as the key, but it's not there so things blow up.
[r[model.id] for r in resources]))).encode(

I was thinking of submitting a PR to do a quick fix like this

@resources.register('rds-cluster-snapshot')
class RDSClusterSnapshot(QueryResourceManager):
    """Resource manager for RDS cluster snapshots.
    """

    class resource_type(TypeInfo):
        service = 'rds'
        arn_type = 'cluster-snapshot'
        arn_separator = ':'
        arn = 'DBClusterSnapshotArn'
        enum_spec = (
            'describe_db_cluster_snapshots', 'DBClusterSnapshots', None)
        name = id = 'DBClusterSnapshotIdentifier'
        date = 'SnapshotCreateTime'
        universal_tagging = object()
        config_type = 'AWS::RDS::DBClusterSnapshot'
        permissions_enum = ('rds:DescribeDBClusterSnapshots',)

    source_mapping = {
        'describe': DescribeClusterSnapshot,
        'config': ConfigSource
    }

    def get_source(self, source_type):
        # Fixing id, name and arn of the resource for when it comes from AWS Config
        if source_type == 'config':
            self.resource_type.id = 'DbclusterSnapshotIdentifier'
            self.resource_type.name = 'DbclusterSnapshotIdentifier'
            self.resource_type.arn = 'DbclusterSnapshotArn'
        return super().get_source(source_type)

but I think the problem might be there in other resources as well and was thinking if we need to have a more generic or standard fix. Ultimately, the problem is with AWS Config having different conventions for naming things. I wonder if the proper fix is to do something like what you have started doing with config_type vs cfn_type, meaning we now have config_id, config_name, config_arn vs the regular id, name and arn.

Background (please complete the following information):

  • OS: [e.g. OSX 10.15]
  • Python Version: [e.g. python 3.8.1]
  • Custodian Version: latest from master
  • Cloud Provider: aws
  • Policy: See above
  • Traceback: [if applicable, please exclude sensitive/account information]
  • custodian version --debug output
$ custodian version --debug
Custodian:   0.8.46.1
Python:      3.7.6 (default, Dec 30 2019, 19:38:36) 
             [Clang 10.0.0 (clang-1000.11.45.5)]
Platform:    posix.uname_result(sysname='Darwin', nodename='SDGL1607c2b92.local', release='17.7.0', version='Darwin Kernel Version 17.7.0: Sun Dec  1 19:19:56 PST 2019; root:xnu-4570.71.63~1/RELEASE_X86_64', machine='x86_64')
Using venv:  False
Docker: False
PYTHONPATH: 
[            '/Users/ddao/sandbox/c7n/custodian/bin',
             '/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python37.zip',
             '/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7',
             '/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/lib-dynload',
             '/Users/ddao/sandbox/c7n/custodian/lib/python3.7/site-packages']
@darrendao darrendao changed the title SecurityHub finding doesn't work on rds-cluster-snapshot resource in config-rule mode SecurityHub finding doesn't work for rds-cluster-snapshot resource in config-rule mode May 18, 2020
@kapilt
Copy link
Collaborator

kapilt commented May 19, 2020

we use a config source to normalize the bespoke idiosyncratic formatting of config to describe format. ie the goal is that policies are isomorphic wrt to execution mode without filter/action changes. The extant automated conversion for config is likely turning this to Db instead of DB, which needs customization for this and other DB prefixed fields.

@darrendao
Copy link
Collaborator Author

@kapilt Maybe update the camelResource() method in utils.py to something like this?

def camelResource(obj):
    """Some sources from apis return lowerCased where as describe calls

    always return TitleCase, this function turns the former to the later
    """
    if not isinstance(obj, dict):
        return obj
    for k in list(obj.keys()):
        v = obj.pop(k)
        if len(k) > 3 and k.startswith("db"):
            k = "%s%s" % (k[0:3].upper(), k[3:])
        else:
            k = "%s%s" % (k[0].upper(), k[1:])
        obj[k] = v
        if isinstance(v, dict):
            camelResource(v)
        elif isinstance(v, list):
            list(map(camelResource, v))
    return obj

Might have some undesire side effects though...

@kapilt
Copy link
Collaborator

kapilt commented May 27, 2020

@darrendao we don't put resource specific hacks/workarounds in generic utilities, the right place for this normalization is a config source subclass for rds snapshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants