Fix(extra771): jq fail when policy action is an array

[lazize]

Context

jq filter was failing when policy Action was an array.

Description

Fix jq select condition to handle Action as string or as array.
Add error handling.
When fail, print policies as just one line.

Fix issue #1026

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[lazize]

It will also fix partial issues from #1019

It will not expose policy in multiple lines anymore.

But it still needs to fix comma issue inside policy.

[lazize]

$ echo '
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "rule0",
            "Effect": "Allow",
            "Principal": "*",
            "Action": "s3:*",
            "Resource": [
                "arn:aws:s3:::xxxx",
                "arn:aws:s3:::xxxx/*"
            ],
            "Condition": {
                "StringEquals": {
                "aws:PrincipalOrgID": "o-abc123"
                }
            }
        },
        {
            "Sid": "rule1",
            "Effect": "Allow",
            "Principal": "*",
            "Action": [
                "s3:GetObject",
                "s3:PutObject",
                "s3:ListBucket"
            ],
            "Resource": [
                "arn:aws:s3:::xxxx",
                "arn:aws:s3:::xxxx/*"
            ],
            "Condition": {
                "StringEquals": {
                "aws:PrincipalOrgID": "o-abc123"
                }
            }
        },
        {
            "Sid": "rule2",
            "Effect": "Allow",
            "Principal": "*",
            "Action": "s3:PutObject",
            "Resource": [
                "arn:aws:s3:::xxxx",
                "arn:aws:s3:::xxxx/*"
            ]
        },
        {
            "Sid": "rule3",
            "Effect": "Allow",
            "Principal": "*",
            "Action": [
                "s3:GetObject",
                "s3:PutObject",
                "s3:ListBucket"
            ],
            "Resource": [
                "arn:aws:s3:::xxxx",
                "arn:aws:s3:::xxxx/*"
            ]
        }
    ]
}' | jq --compact-output '.Statement[]|select(
.Effect=="Allow" and
(
    ( (.Principal|type == "object") and (.Principal.AWS == "*") ) or
    ( (.Principal|type == "string") and (.Principal == "*") )
) and
(
    ( (.Action|type == "string") and (.Action|startswith("s3:Put")) ) or
    ( (.Action|type == "string") and (.Action|startswith("s3:*")) ) or
    ( (.Action|type == "array") and (.Action[]|startswith("s3:Put")) ) or
    ( (.Action|type == "array") and (.Action[]|startswith("s3:*")) )
) and
.Condition == null)' | tr '\n' ' '
{"Sid":"rule2","Effect":"Allow","Principal":"*","Action":"s3:PutObject","Resource":["arn:aws:s3:::xxxx","arn:aws:s3:::xxxx/*"]} {"Sid":"rule3","Effect":"Allow","Principal":"*","Action":["s3:GetObject","s3:PutObject","s3:ListBucket"],"Resource":["arn:aws:s3:::xxxx","arn:aws:s3:::xxxx/*"]}

[toniblyx]

@lazize when I test it with the suggested policy I get this error:
jq: error (at <stdin>:1): startswith() requires string inputs.
Also, can you clarify the comment about the comma?

[lazize]

@toniblyx Did you test the echo | jq | tr command above? Did it fail? I tested it right now and it works fine for me.
If you give me more details I can try to reproduce and fix any issue. But unfortunately I can't test it on my own bucket, I can't make it public (read or write).

This is the output it returns me, one long string with violating policy.
But note the policy contains comma, because it is a json.
{"Sid":"rule2","Effect":"Allow","Principal":"*","Action":"s3:PutObject","Resource":["arn:aws:s3:::xxxx","arn:aws:s3:::xxxx/*"]} {"Sid":"rule3","Effect":"Allow","Principal":"*","Action":["s3:GetObject","s3:PutObject","s3:ListBucket"],"Resource":["arn:aws:s3:::xxxx","arn:aws:s3:::xxxx/*"]}

When we run prowler and ask it to generate CSV output, this json string with comma will break CSV result file.

[toniblyx]

yeah, the command works fine for me, the check itself doesn't. This is part of the debug output

+ for bucket in $LIST_OF_BUCKETS
++ /usr/local/bin/aws s3api --profile dev get-bucket-policy --region us-west-2 --bucket issue1031 --output json --query Policy
+ BUCKET_POLICY_STATEMENTS='"{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"rule0\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:*\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule1\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule2\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:PutObject\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]},{\"Sid\":\"rule3\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]}]}"'
+ [[ "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"rule0\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:*\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule1\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule2\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:PutObject\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]},{\"Sid\":\"rule3\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]}]}" == *GetBucketPolicy* ]]
++ jq --arg arn arn:aws:s3:::issue1031 'fromjson | .Statement[]|select(.Effect=="Allow" and (((.Principal|type == "object") and .Principal.AWS == "*") or ((.Principal|type == "string") and .Principal == "*")) and (.Action|startswith("s3:Put") or startswith("s3:*")) and .Condition == null)'
++ echo '"{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"rule0\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:*\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule1\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"],\"Condition\":{\"StringEquals\":{\"aws:PrincipalOrgID\":\"o-abc123\"}}},{\"Sid\":\"rule2\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:PutObject\",\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]},{\"Sid\":\"rule3\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":[\"s3:GetObject\",\"s3:PutObject\",\"s3:ListBucket\"],\"Resource\":[\"arn:aws:s3:::issue1031\",\"arn:aws:s3:::issue1031/*\"]}]}"'
jq: error (at <stdin>:1): startswith() requires string inputs
+ BUCKET_POLICY_BAD_STATEMENTS=
+ [[ '' != '' ]]
+ textPass 'Bucket issue1031 has S3 bucket policy which does not allow public write access' us-east-1 issue1031
+ CHECK_RESULT=PASS
+ CHECK_RESULT_EXTENDED='Bucket issue1031 has S3 bucket policy which does not allow public write access'
+ CHECK_RESOURCE_ID=issue1031

See the jq error there.

[lazize]

Strange, the jq command you are running is the original one, not the one I changed. See it below.

This is from your output above:

++ jq --arg arn arn:aws:s3:::issue1031 'fromjson | .Statement[]|select(
    .Effect=="Allow" and 
    (
        ((.Principal|type == "object") and .Principal.AWS == "*") or 
        ((.Principal|type == "string") and .Principal == "*")
    ) and
    (
        .Action|startswith("s3:Put") or startswith("s3:*")
    ) and
    .Condition == null
)'

This is the command I pushed on this PR:

jq --compact-output --arg arn "arn:${AWS_PARTITION}:s3:::$bucket" 'fromjson | .Statement[]|select(
    .Effect=="Allow" and
    (
        ( (.Principal|type == "object") and (.Principal.AWS == "*") ) or
        ( (.Principal|type == "string") and (.Principal == "*") )
    ) and
    (
        ( (.Action|type == "string") and (.Action|startswith("s3:Put")) ) or
        ( (.Action|type == "string") and (.Action|startswith("s3:*")) ) or
        ( (.Action|type == "array") and (.Action[]|startswith("s3:Put")) ) or
        ( (.Action|type == "array") and (.Action[]|startswith("s3:*")) )
    ) and
    .Condition == null
  )'

Note the difference between several OR conditions.

[toniblyx]

OK, I get it now. I don't see the need to include the policy output to the check output. That could be removed or redacted as you suggest in line 57.

[toniblyx]

Can you update this PR with the line BUCKET_POLICY_BAD_STATEMENTS=$(echo ${BUCKET_POLICY_BAD_STATEMENTS}" | sed 's|,|[comma]|g') enabled? That makes more sense. Thanks!

[toniblyx]

LGTM

[toniblyx]

Thanks @lazize, great work!