Repository: cloud-custodian/cloud-custodian

Issue:
aws - VPC Security Group filters "used" / "unused" and ECS Scheduled Tasks
I just found that those filters were not taking into account the ECS Scheduled Tasks (which do have a Security Group info).

I'm preparing a Pull Request with a proposed workaround.

Discussion:
kapilt: simplified and added backfilled a unit test
kapilt: docker build failure was unrelated, due to a python cryptography release without wheels/binaries.

Patch:
diff --git a/c7n/resources/awslambda.py b/c7n/resources/awslambda.py
index be593671e..be91fdaa4 100644
--- a/c7n/resources/awslambda.py
+++ b/c7n/resources/awslambda.py
@@ -21,6 +21,7 @@ from concurrent.futures import as_completed
 
 from c7n.actions import BaseAction, RemovePolicyBase, ModifyVpcSecurityGroupsAction
 from c7n.filters import CrossAccountAccessFilter, ValueFilter
+from c7n.filters.kms import KmsRelatedFilter
 import c7n.filters.vpc as net_filters
 from c7n.manager import resources
 from c7n import query
@@ -257,6 +258,28 @@ class LambdaCrossAccountAccessFilter(CrossAccountAccessFilter):
             resources, event)
 
 
+@AWSLambda.filter_registry.register('kms-key')
+class KmsFilter(KmsRelatedFilter):
+    """
+    Filter a resource by its associcated kms key and optionally the aliasname
+    of the kms key by using 'c7n:AliasName'
+
+    :example:
+
+        .. code-block:: yaml
+
+            policies:
+                - name: lambda-kms-key-filters
+                  resource: aws.lambda
+                  filters:
+                    - type: kms-key
+                      key: c7n:AliasName
+                      value: "^(alias/aws/lambda)"
+                      op: regex
+    """
+    RelatedIdsExpression = 'KMSKeyArn'
+
+
 @AWSLambda.action_registry.register('remove-statements')
 class RemovePolicyStatement(RemovePolicyBase):
     """Action to remove policy/permission statements from lambda functions.
diff --git a/c7n/resources/vpc.py b/c7n/resources/vpc.py
index c7a2adf1c..cea0d2885 100644
--- a/c7n/resources/vpc.py
+++ b/c7n/resources/vpc.py
@@ -688,7 +688,7 @@ class SGUsage(Filter):
         return list(itertools.chain(
             *[self.manager.get_resource_manager(m).get_permissions()
              for m in
-             ['lambda', 'eni', 'launch-config', 'security-group']]))
+             ['lambda', 'eni', 'launch-config', 'security-group', 'event-rule-target']]))
 
     def filter_peered_refs(self, resources):
         if not resources:
@@ -705,14 +705,18 @@ class SGUsage(Filter):
             "%d of %d groups w/ peered refs", len(peered_ids), len(resources))
         return [r for r in resources if r['GroupId'] not in peered_ids]
 
+    def get_scanners(self):
+        return (
+            ("nics", self.get_eni_sgs),
+            ("sg-perm-refs", self.get_sg_refs),
+            ('lambdas', self.get_lambda_sgs),
+            ("launch-configs", self.get_launch_config_sgs),
+            ("ecs-cwe", self.get_ecs_cwe_sgs)
+        )
+
     def scan_groups(self):
         used = set()
-        for kind, scanner in (
-                ("nics", self.get_eni_sgs),
-                ("sg-perm-refs", self.get_sg_refs),
-                ('lambdas', self.get_lambda_sgs),
-                ("launch-configs", self.get_launch_config_sgs),
-        ):
+        for kind, scanner in self.get_scanners():
             sg_ids = scanner()
             new_refs = sg_ids.difference(used)
             used = used.union(sg_ids)
@@ -735,7 +739,7 @@ class SGUsage(Filter):
 
     def get_lambda_sgs(self):
         sg_ids = set()
-        for func in self.manager.get_resource_manager('lambda').resources():
+        for func in self.manager.get_resource_manager('lambda').resources(augment=False):
             if 'VpcConfig' not in func:
                 continue
             for g in func['VpcConfig']['SecurityGroupIds']:
@@ -758,6 +762,16 @@ class SGUsage(Filter):
                         sg_ids.add(g['GroupId'])
         return sg_ids
 
+    def get_ecs_cwe_sgs(self):
+        sg_ids = set()
+        expr = jmespath.compile(
+            'EcsParameters.NetworkConfiguration.awsvpcConfiguration.SecurityGroups[]')
+        for rule in self.manager.get_resource_manager('event-rule-target').resources():
+            ids = expr.search(rule)
+            if ids:
+                sg_ids.update(ids)
+        return sg_ids
+
 
 @SecurityGroup.filter_registry.register('unused')
 class UnusedSecurityGroup(SGUsage):
@@ -768,7 +782,10 @@ class UnusedSecurityGroup(SGUsage):
     lambdas as they may not have extant resources in the vpc at a
     given moment. We also find any security group with references from
     other security group either within the vpc or across peered
-    connections.
+    connections. Also checks cloud watch event targeting ecs.
+
+    Checks - enis, lambda, launch-configs, sg rule refs, and ecs cwe
+    targets.
 
     Note this filter does not support classic security groups atm.
 
@@ -781,6 +798,7 @@ class UnusedSecurityGroup(SGUsage):
                 resource: security-group
                 filters:
                   - unused
+
     """
     schema = type_schema('unused')
 


Test patch:
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/kms.DescribeKey_1.json b/tests/data/placebo/test_lambda_kms_key_filter/kms.DescribeKey_1.json
new file mode 100644
index 000000000..dc406d831
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/kms.DescribeKey_1.json
@@ -0,0 +1,31 @@
+{
+    "status_code": 200,
+    "data": {
+        "KeyMetadata": {
+            "AWSAccountId": "644160558196",
+            "KeyId": "36812ccf-daaf-49b1-a24b-0eef254ffe41",
+            "Arn": "arn:aws:kms:us-east-1:644160558196:key/36812ccf-daaf-49b1-a24b-0eef254ffe41",
+            "CreationDate": {
+                "__class__": "datetime",
+                "year": 2016,
+                "month": 7,
+                "day": 24,
+                "hour": 4,
+                "minute": 51,
+                "second": 26,
+                "microsecond": 199000
+            },
+            "Enabled": true,
+            "Description": "Trails for the Skunk",
+            "KeyUsage": "ENCRYPT_DECRYPT",
+            "KeyState": "Enabled",
+            "Origin": "AWS_KMS",
+            "KeyManager": "CUSTOMER",
+            "CustomerMasterKeySpec": "SYMMETRIC_DEFAULT",
+            "EncryptionAlgorithms": [
+                "SYMMETRIC_DEFAULT"
+            ]
+        },
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_1.json b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_1.json
new file mode 100644
index 000000000..35baf402e
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_1.json
@@ -0,0 +1,14 @@
+{
+    "status_code": 200,
+    "data": {
+        "Aliases": [
+            {
+                "AliasName": "alias/skunk/trails",
+                "AliasArn": "arn:aws:kms:us-east-1:644160558196:alias/skunk/trails",
+                "TargetKeyId": "36812ccf-daaf-49b1-a24b-0eef254ffe41"
+            }
+        ],
+        "Truncated": false,
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_2.json b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_2.json
new file mode 100644
index 000000000..35baf402e
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListAliases_2.json
@@ -0,0 +1,14 @@
+{
+    "status_code": 200,
+    "data": {
+        "Aliases": [
+            {
+                "AliasName": "alias/skunk/trails",
+                "AliasArn": "arn:aws:kms:us-east-1:644160558196:alias/skunk/trails",
+                "TargetKeyId": "36812ccf-daaf-49b1-a24b-0eef254ffe41"
+            }
+        ],
+        "Truncated": false,
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/kms.ListKeys_1.json b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListKeys_1.json
new file mode 100644
index 000000000..05030d269
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/kms.ListKeys_1.json
@@ -0,0 +1,13 @@
+{
+    "status_code": 200,
+    "data": {
+        "Keys": [
+            {
+                "KeyId": "36812ccf-daaf-49b1-a24b-0eef254ffe41",
+                "KeyArn": "arn:aws:kms:us-east-1:644160558196:key/36812ccf-daaf-49b1-a24b-0eef254ffe41"
+            }
+        ],
+        "Truncated": false,
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/lambda.ListFunctions_1.json b/tests/data/placebo/test_lambda_kms_key_filter/lambda.ListFunctions_1.json
new file mode 100644
index 000000000..e0fe5a510
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/lambda.ListFunctions_1.json
@@ -0,0 +1,32 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {},
+        "Functions": [
+            {
+                "FunctionName": "test",
+                "FunctionArn": "arn:aws:lambda:us-east-1:644160558196:function:test",
+                "Runtime": "nodejs12.x",
+                "Role": "arn:aws:iam::644160558196:role/sphere12",
+                "Handler": "index.handler",
+                "CodeSize": 304,
+                "Description": "",
+                "Timeout": 3,
+                "MemorySize": 128,
+                "LastModified": "2020-04-21T01:57:58.231+0000",
+                "CodeSha256": "k33TggGtygLrDDrEecfHuvfuVd+6bRxCFv7lErxWOJ0=",
+                "Version": "$LATEST",
+                "Environment": {
+                    "Variables": {
+                        "test": "test"
+                    }
+                },
+                "KMSKeyArn": "arn:aws:kms:us-east-1:644160558196:key/36812ccf-daaf-49b1-a24b-0eef254ffe41",
+                "TracingConfig": {
+                    "Mode": "PassThrough"
+                },
+                "RevisionId": "4ce18eaf-7e19-4446-b8b4-fe773a79379f"
+            }
+        ]
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_lambda_kms_key_filter/tagging.GetResources_1.json b/tests/data/placebo/test_lambda_kms_key_filter/tagging.GetResources_1.json
new file mode 100644
index 000000000..8b704d185
--- /dev/null
+++ b/tests/data/placebo/test_lambda_kms_key_filter/tagging.GetResources_1.json
@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_security_group_ecs_unused/config.SelectResourceConfig_1.json b/tests/data/placebo/test_security_group_ecs_unused/config.SelectResourceConfig_1.json
new file mode 100644
index 000000000..2ab8f39b6
--- /dev/null
+++ b/tests/data/placebo/test_security_group_ecs_unused/config.SelectResourceConfig_1.json
@@ -0,0 +1,19 @@
+{
+    "status_code": 200,
+    "data": {
+        "Results": [
+            "{\"configuration\":{\"groupName\":\"apichanges-task\",\"groupId\":\"sg-0f026884bba48e350\",\"vpcId\":\"vpc-39831640\",\"ipPermissionsEgress\":[{\"ipRanges\":[\"0.0.0.0/0\"],\"prefixListIds\":[],\"userIdGroupPairs\":[],\"ipProtocol\":\"-1\",\"ipv4Ranges\":[{\"description\":\"Allows task to establish connections to all resources\",\"cidrIp\":\"0.0.0.0/0\"}],\"ipv6Ranges\":[]}],\"description\":\"Limit connections from internal resources while allowing apichanges-task to connect to all external resources\",\"ownerId\":\"644160558196\",\"ipPermissions\":[],\"tags\":[{\"value\":\"AWSAPIChanges\",\"key\":\"App\"}]},\"supplementaryConfiguration\":{}}"
+        ],
+        "QueryInfo": {
+            "SelectFields": [
+                {
+                    "Name": "configuration"
+                },
+                {
+                    "Name": "supplementaryConfiguration"
+                }
+            ]
+        },
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_security_group_ecs_unused/events.ListRules_1.json b/tests/data/placebo/test_security_group_ecs_unused/events.ListRules_1.json
new file mode 100644
index 000000000..b49c5332e
--- /dev/null
+++ b/tests/data/placebo/test_security_group_ecs_unused/events.ListRules_1.json
@@ -0,0 +1,16 @@
+{
+    "status_code": 200,
+    "data": {
+        "Rules": [
+            {
+                "Name": "apichanges-build",
+                "Arn": "arn:aws:events:us-east-1:644160558196:rule/apichanges-build",
+                "State": "ENABLED",
+                "Description": "Runs fargate task apichanges: rate(6 hours)",
+                "ScheduleExpression": "rate(6 hours)",
+                "EventBusName": "default"
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
diff --git a/tests/data/placebo/test_security_group_ecs_unused/events.ListTargetsByRule_1.json b/tests/data/placebo/test_security_group_ecs_unused/events.ListTargetsByRule_1.json
new file mode 100644
index 000000000..2c6247eca
--- /dev/null
+++ b/tests/data/placebo/test_security_group_ecs_unused/events.ListTargetsByRule_1.json
@@ -0,0 +1,31 @@
+{
+    "status_code": 200,
+    "data": {
+        "Targets": [
+            {
+                "Id": "apichanges-build-target",
+                "Arn": "arn:aws:ecs:us-east-1:644160558196:cluster/apichanges",
+                "RoleArn": "arn:aws:iam::644160558196:role/apichanges-events",
+                "Input": "{}",
+                "EcsParameters": {
+                    "TaskDefinitionArn": "arn:aws:ecs:us-east-1:644160558196:task-definition/apichanges:6",
+                    "TaskCount": 1,
+                    "LaunchType": "FARGATE",
+                    "NetworkConfiguration": {
+                        "awsvpcConfiguration": {
+                            "Subnets": [
+                                "subnet-8e261cb2"
+                            ],
+                            "SecurityGroups": [
+                                "sg-0f026884bba48e350"
+                            ],
+                            "AssignPublicIp": "ENABLED"
+                        }
+                    },
+                    "PlatformVersion": "LATEST"
+                }
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_security_group_unused/events.ListRules_1.json b/tests/data/placebo/test_security_group_unused/events.ListRules_1.json
new file mode 100644
index 000000000..67342edda
--- /dev/null
+++ b/tests/data/placebo/test_security_group_unused/events.ListRules_1.json
@@ -0,0 +1,7 @@
+{
+    "status_code": 200,
+    "data": {
+        "Rules": [],
+        "ResponseMetadata": {}
+    }
+}
diff --git a/tests/data/placebo/test_security_group_used/events.ListRules_1.json b/tests/data/placebo/test_security_group_used/events.ListRules_1.json
new file mode 100644
index 000000000..67342edda
--- /dev/null
+++ b/tests/data/placebo/test_security_group_used/events.ListRules_1.json
@@ -0,0 +1,7 @@
+{
+    "status_code": 200,
+    "data": {
+        "Rules": [],
+        "ResponseMetadata": {}
+    }
+}
diff --git a/tests/test_lambda.py b/tests/test_lambda.py
index 6bc3f3f76..6c3df5f57 100644
--- a/tests/test_lambda.py
+++ b/tests/test_lambda.py
@@ -526,3 +526,28 @@ class TestModifyVpcSecurityGroupsAction(BaseTest):
                 groups = ['sg-12121212', 'sg-34343434']
                 updatefunc(FunctionName='badname', VpcConfig={'SecurityGroupIds': groups})
                 updatefunc.assert_called_once()
+
+    def test_lambda_kms_alias(self):
+        session_factory = self.replay_flight_data("test_lambda_kms_key_filter")
+        kms = session_factory().client('kms')
+        p = self.load_policy(
+            {
+                "name": "lambda-kms-alias",
+                "resource": "lambda",
+                "filters": [
+                    {
+                        'FunctionName': "test"
+                    },
+                    {
+                        "type": "kms-key",
+                        "key": "c7n:AliasName",
+                        "value": "alias/skunk/trails",
+                    }
+                ]
+            },
+            session_factory=session_factory,
+        )
+        resources = p.run()
+        self.assertTrue(len(resources), 1)
+        aliases = kms.list_aliases(KeyId=resources[0]['KMSKeyArn'])
+        self.assertEqual(aliases['Aliases'][0]['AliasName'], 'alias/skunk/trails')
diff --git a/tests/test_vpc.py b/tests/test_vpc.py
index 64e0f43d8..4075f0bbe 100644
--- a/tests/test_vpc.py
+++ b/tests/test_vpc.py
@@ -1125,6 +1125,24 @@ class SecurityGroupTest(BaseTest):
             set([r["GroupId"] for r in resources]),
         )
 
+    def test_unused_ecs(self):
+        factory = self.replay_flight_data("test_security_group_ecs_unused")
+        p = self.load_policy(
+            {'name': 'sg-xyz',
+             'source': 'config',
+             'query': [
+                 {'clause': "resourceId ='sg-0f026884bba48e350'"}],
+             'resource': 'security-group',
+             'filters': ['unused']},
+            session_factory=factory)
+        unused = p.resource_manager.filters[0]
+        self.patch(
+            unused,
+            'get_scanners',
+            lambda: (('ecs-cwe', unused.get_ecs_cwe_sgs),))
+        resources = p.run()
+        assert resources == []
+
     def test_unused(self):
         factory = self.replay_flight_data("test_security_group_unused")
         p = self.load_policy(