cloud-custodian / cloud-custodian

Rules engine for cloud security, cost optimization, and governance, DSL in yaml for policies to query, filter, and take actions on resources
https://cloudcustodian.io
Apache License 2.0
5.46k stars 1.49k forks source link

security group scan prefix list my account work, but other account can't #8054

Open ZenoRewn opened 1 year ago

ZenoRewn commented 1 year ago

Describe the bug

We had multi accounts. And I tried to enhanced our security group policy by aws prefix-list.

With the same custodian env, my account can return the expect result. Like Match SG who is using the prefix-list. But other accounts can't return the expect result.

What did you expect to happen?

Policy-A and Policy-B just prefix id is different. And I'm sure about those prefix id existed on corresponding account.

Policy-A:

policies:
  - name: prefix
    resource: security-group
    description: prefix
    filters:
    - type: ingress
      PrefixListIds:
        op: eq
        value: ["PrefixListId": "pl-aaaa"]

Policy-B:

policies:
  - name: prefix
    resource: security-group
    description: prefix
    filters:
    - type: ingress
      PrefixListIds:
        op: eq
        value: ["PrefixListId": "pl-bbb"]

---custodian run--- Account-A returns as below:

2022-12-06 15:56:31,547: c7n_org:INFO Ran account:account-a region:cn-northwest-1 policy:prefix matched:1 time:0.32
2022-12-06 15:56:31,557: c7n_org:INFO Policy resource counts Counter({'prefix': 1})
Account-B returns as below:
2022-12-06 15:58:34,071: c7n_org:INFO Policy resource counts Counter({'prefix': 0})

Cloud Provider

Amazon Web Services (AWS)

Cloud Custodian version and dependency information

Please copy/paste the following info along with any bug reports:

Custodian:   0.9.19
Python:      3.7.10 (default, Jun  3 2021, 00:02:01)
             [GCC 7.3.1 20180712 (Red Hat 7.3.1-13)]
Platform:    posix.uname_result(sysname='Linux', nodename='ip-172-31-36-106.cn-northwest-1.compute.internal', release='4.14.296-222.539.amzn2.x86_64', version='#1 SMP Wed Oct 26 20:36:53 UTC 2022', machine='x86_64')
Using venv:  True
Docker: False
Installed:

argcomplete==2.0.0
attrs==22.1.0
boto3==1.24.88
botocore==1.27.88
docutils==0.17.1
importlib-metadata==4.13.0
importlib-resources==5.9.0
jmespath==1.0.1
jsonschema==4.16.0
pkgutil-resolve-name==1.3.10
pyrsistent==0.18.1
python-dateutil==2.8.2
pyyaml==6.0
s3transfer==0.6.0
six==1.16.0
tabulate==0.8.10
typing-extensions==4.3.0
urllib3==1.26.12
zipp==3.8.1

Policy

policies:
  - name: prefix
    resource: security-group
    description: prefix
    filters:
    - type: ingress
      PrefixListIds:
        op: eq
        value: ["PrefixListId": "pl-aaaa"]

Relevant log/traceback output

---output-log---

2022-12-06 15:18:56,791 - custodian.policy - INFO - policy:prefix resource:security-group region:cn-northwest-1 count:0 time:1.28
2022-12-06 15:44:27,245 - custodian.policy - INFO - policy:prefix resource:security-group region:cn-northwest-1 count:0 time:0.29
2022-12-06 15:44:43,403 - custodian.policy - INFO - policy:prefix resource:security-group region:cn-northwest-1 count:0 time:0.13

---metadata---

{
  "policy": {
    "name": "prefix",
    "resource": "security-group",
    "description": "prefix",
    "filters": [
      {
        "type": "ingress",
        "PrefixListIds": {
          "PrefixListId": {
            "value": "pl-xxx"
          },
          "key": "PrefixListIds"
        }
      }
    ]
  },
  "version": "0.9.19",
  "execution": {
    "id": "xxxxx-xxxxx-xxxxx",
    "start": 1670312683.2638478,
    "end_time": 1670312683.4043925,
    "duration": 0.14054465293884277
  },
  "config": {
    "region": "cn-northwest-1",
    "regions": [
      "cn-northwest-1"
    ],
    "cache": "~/.cache/cloud-custodian.cache",
    "profile": null,
    "account_id": "216775306531",
    "assume_role": null,
    "external_id": null,
    "log_group": null,
    "tracer": null,
    "metrics_enabled": null,
    "metrics": null,
    "output_dir": "output",
    "cache_period": 0,
    "dryrun": true,
    "authorization_file": null,
    "subparser": "run",
    "config": null,
    "configs": [
      "p_yj.yml"
    ],
    "policy_filters": [],
    "resource_types": [],
    "verbose": null,
    "quiet": null,
    "debug": false,
    "skip_validation": false,
    "command": "c7n.commands.run",
    "vars": null
  },
  "sys-stats": {},
  "api-stats": {
    "ec2.DescribeSecurityGroups": 1
  },
  "metrics": [
    {
      "MetricName": "ResourceCount",
      "Timestamp": "2022-12-06T15:44:43.404127",
      "Value": 0,
      "Unit": "Count"
    },
    {
      "MetricName": "ResourceTime",
      "Timestamp": "2022-12-06T15:44:43.404144",
      "Value": 0.1300971508026123,
      "Unit": "Seconds"
    }
  ]
}


### Extra information or context

_No response_
ZenoRewn commented 1 year ago

Resource: security-group CATEGORY: Filter Item: PrefixListIds

---above can work to filter prefix list--- But When you are allow all traffic policy with prefixlist, it can't work properly Still finding workaround or how to filter all traffic with prefixlistid.

policies:
  - name: prefix
    resource: security-group
    description: This works for filter prefix list. But not on all traffic.
    filters:
      - type: ingress
        PrefixListIds: [PrefixListId: "pl-0bb6fa005053d29a9"]
ajkerrigan commented 1 year ago

If your existing policy works for finding the prefix lists, you should be able to add an IpProtocol key to catch the 'all traffic' case:

policies:
  - name: prefix
    resource: security-group
    filters:
      - type: ingress
        IpProtocol: '-1'
        PrefixListIds: [PrefixListId: "pl-0bb6fa005053d29a9"]

Note that you do need '-1' in quotes since the API response is a string. Without quotes it'll treat -1 as an integer and fail to match.

While testing this locally, I did notice that the fanciness of the ingress filter won't match a policy like yours if there's a description on the security group rule that has the prefix list association. One way to work around that is by using a value filter instead, so your policy would become:

policies:
  - name: prefix
    resource: security-group
    filters:
      - type: value
        key: |
          IpPermissions[?
            IpProtocol == '-1'
            && PrefixListIds[?PrefixListId == 'pl-0bb6fa005053d29a9']
          ]
        value: not-null

That's unfortunately an uglier policy, but it does seem to more reliably capture your intent.

ZenoRewn commented 1 year ago

Hi ajkerrigan@,

Thanks for your supporting. And Happy 2023 New Year~

I tried your policy and it's worked. If I need to filter the ingress all traffic, seems like value filter is the only option. Correct? If I don't add 'IpProtocol' then the policy can't filter ingress all traffic security group rules.