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

Custom config file is not parsed correctly #147

Open
sckevmit opened this issue Sep 18, 2020 · 2 comments
Open

Custom config file is not parsed correctly #147

sckevmit opened this issue Sep 18, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@sckevmit
Copy link

Hello!

Example policy:

{
  "Version": "2012-10-17",
  "Statement": [
  {
      "Sid": "BucketAccess",
      "Effect": "Allow",
      "Action": [
        "s3:GetBucketLocation",
        "s3:GetBucketVersioning",
        "s3:GetEncryptionConfiguration",
        "s3:ListBucket",
        "s3:ListBucketMultipartUploads",
        "s3:ListBucketVersions"
      ],
      "Resource": "arn:aws:s3:::bucket-for-client-${aws:PrincipalTag/Namespace}-*"
    }
]
}

custom_config.yaml:

RESOURCE_MISMATCH:
  ignore_locations:
    - actions:
        - '.*s3:Describe.*'
        - '.*s3:Get.*'
        - '.*s3:List*.*'
 $ python3.7 parliament/cli.py  --file main.json --config custom_config.yaml

Traceback (most recent call last):
  File "/home/sdziech/.local/lib/python3.7/site-packages/parliament/cli.py", line 363, in <module>
    main()
  File "/home/sdziech/.local/lib/python3.7/site-packages/parliament/cli.py", line 348, in main
    if not is_finding_filtered(finding, args.minimum_severity):
  File "/home/sdziech/.local/lib/python3.7/site-packages/parliament/cli.py", line 57, in is_finding_filtered
    for location_to_ignore in make_list(locations_to_ignore):
  File "/home/sdziech/.local/lib/python3.7/site-packages/parliament/misc.py", line 18, in make_list
    line = v.line
AttributeError: 'list' object has no attribute 'line'

It looks like jsoncfg.node_is_array(config_node) doesn't recognize list as list.

>>> import jsoncfg
>>> d = ['.*s3:Describe.*', '.*s3:Get.*', '.*s3:List*.*']
>>> type(d)
<class 'list'>
>>> jsoncfg.node_is_array(d)
False
@0xdabbad00
Copy link
Collaborator

Looks like there are two issues at play:

  1. The make_list call is now primarily used for the given json policy and does not work for the yaml configuration file. This call here:
    for location_to_ignore in make_list(locations_to_ignore):
    can thus be changed to something like local_make_list that can be defined within that is_finding_filtered function as:
    def local_make_list(v):
        if not isinstance(v, list):
            return [v]
        return v
  1. The location info has been changed dramatically from its original format. As such, the for location_to_ignore loop inside is_finding_filtered needs to be something like:
for detail in local_make_list(finding.detail):
    if location_type == 'actions':
        if re.fullmatch(
            location_to_ignore.lower(),
            str(detail.get('action', '')).lower(),
        ):
            has_array_match = True

That will get to the correct elements within the finding structure, but because a single finding can now contain the issues related to multiple actions, filtering that becomes much more difficult.

For example, let's say we have the following policy:

{
  "Version": "2012-10-17",
  "Statement": [
  {
      "Sid": "BucketAccess",
      "Effect": "Allow",
      "Action": [
        "s3:GetBucketLocation",
        "s3:ListAllMyBuckets"
      ],
      "Resource": "arn:aws:s3:::bucket/object"
    }
]
}

This will generate the finding:

{"issue": "RESOURCE_MISMATCH", "title": "No resources match for the given action", "severity": "MEDIUM", "description": "", "detail": [{"action": "s3:GetBucketLocation", "required_format": "arn:*:s3:::*"}, {"action": "s3:ListAllMyBuckets", "required_format": "*"}], "location": {"line": 4, "column": 3, "filepath": "test.json"}}

Notice that the detail element contains both s3:GetBucketLocation and s3:ListAllMyBuckets. If we then have a config override file with:

RESOURCE_MISMATCH:
  ignore_locations:
  - actions: 
    - '.*s3:Get.*'

It will mute this entire finding. So we'd have to somehow extract out the action we want to mute maybe? This will be a mess to fix unfortunately, and I think the best way forward is likely for me to remove this ignore_locations functionality and remove it from the README.

@0xdabbad00 0xdabbad00 added the bug Something isn't working label Sep 18, 2020
@celik0311
Copy link

So Ive ran into a similar probelm;

When parsing an IAM role trust policy I found the below comment:

# IAM role trust policies, but those don't have Resource elements, so that would break other things

The above line leads me to believe that IAM role trust policies will raise findings like Statement contains neither Resource nor NotResource. I originally intended to just add those to the ignore_locations and then bumped into the issue discussed here. What is the workaround for this? Or should I just store those types of policies in a different location, one which parliament does not know of. TIA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants