[Bug]: Logic error when using trust_account_ids in the config file

[Fennerr]

Steps to Reproduce

I took a look at the logic, and there seems to be 2 different bugs. One in vpc_endpoint_connections_trust_boundaries.py logic, and one in the is_account_only_allowed_in_condition logic Disclaimer that I didnt spend that long on this, but took a look at the code, and did some debugging on is_account_only_allowed_in_condition's logic - I might be missing something, but Im fairly confident in my analysis.

Here is the source code for the check: https://github.com/prowler-cloud/prowler/blob/master/prowler/providers/aws/services/vpc/vpc_endpoint_connections_trust_boundaries/vpc_endpoint_connections_trust_boundaries.py#L5 What we are interested in is the check for a condition on line 36, and then invoking is_account_only_allowed_in_condition on line 38 (similar logic on line 71 and 103) Also worth noting line 14 where trusted_account_ids is obtained from the config file, and the current account is added to the list on line 16

In order to pass the check (access_from_trusted_accounts = True) we would need to have all the account_id values inside trusted_account_ids return true when the condition block and the account_id is passed to is_account_only_allowed_in_condition

Taking a look at is_account_only_allowed_in_condition from here: https://github.com/prowler-cloud/prowler/blob/master/prowler/providers/aws/lib/policy_condition_parser/policy_condition_parser.py

The line 21 defines a map between valid condition operators, and valid condition values. Skipping ahead, the important part that we need to pass is either the for loop on line 60 if the input is a list, or line 70 if its a string

Lets take a look at these 2 cases: If its a string (we perform the check on line 70) So in this case, the condition element is not a list. Ie, in python:

condition_statement = {
    "StringLike": {
        "AWS:SourceAccount": "111122223333"
    }
}

is_account_only_allowed_in_condition(condition_statement,"111122223333") returns True is_account_only_allowed_in_condition(condition_statement,"111122225555") returns False Seems like everything is working correctly.

But if you go back to vpc_endpoint_connections_trust_boundaries.py, you will see that on line 37 this check needs to pass for ALL trusted_account_ids This seems to be a logic mistake. If you put the following in the config file:

trusted_account_ids: ["111122223333", "098765432109"]

And have the same condition statement as above, the first iteration of the for loop will pass, but the 2nd iteration of the for loop will fail, access_from_trusted_accounts will be set to False, and the break statement will be hit => we will fail the check when it should pass

2. If its a list (we perform the check on line 60) Lets take a look at is_account_only_allowed_in_condition. If the condition statement is like this: ( I know users arnt applicable in this case - I just took the example condition statement from this: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-logic-multiple-context-keys-or-values.html - but the logic still applies)

condition_statement = {
    "ArnLike": {
        "AWS:PrincipalArn":  [
            "arn:aws:iam::111122223333:user/Ana",
            "arn:aws:iam::111122223333:user/Mary"
        ]
    }
}

is_account_only_allowed_in_condition(condition_statement,"111122223333") will result in a True result Okay cool, that seems to check out. But similar to the above issue regarding a string, when the trusted account ids in vpc_endpoint_connections_trust_boundaries.py are being looped through, the next iteration will send 098765432109, the result will be false, access_from_trusted_accounts will be set to False, and the break statement will be hit => we will fail the check when it should pass

But there is also an issue with line 60 in is_account_only_allowed_in_condition. If we have listed 098765432109 and 111122223333 as trusted accounts (as in the above example config file line), then we should be able to do something like this

condition_statement = {
    "ArnLike": {
        "AWS:PrincipalArn":  [
            "arn:aws:iam::111122223333:user/Ana",
            "arn:aws:iam::098765432109:user/Mary"
        ]
    }
}

But now is_account_only_allowed_in_condition(condition_statement,"111122223333") would fail, because each item in the list needs to pass the check on line 61 is_account_only_allowed_in_condition(condition_statement,"098765432109") would also fail due to the same reasons

Solution: Seems like we need a separate implementation for is_account_only_allowed_in_condition that can take a list of trusted_account_ids in as a param for 1: check if ANY of the trusted account ids pass the check on line 66 for 2: determine if there are any items in the list that do not fall within the trust account ids - and remove the for loop from line 60 (and 71/103) in vpc_endpoint_connections_trust_boundaries.py

Expected behavior

The check should pass if the condition statement only contains account ids, or principals within accounts, that are listed in the trusted_account_ids list in the config file

Actual Result with Screenshots or Logs

I did not actually run this - just investigated the feedback in the ask-a-question channel in slack.

How did you install Prowler?

From pip package (pip install prowler)

Environment Resource

Other: Did not actually run it. Pulled some logic into individual files for debugging purposes

OS used

Debian

Prowler version

Latest

Pip version

NA

Context

Original response in slack: https://prowler-workspace.slack.com/archives/C0451NDLC4X/p1699276325100359 Original bug report in slack: https://prowler-workspace.slack.com/archives/C0451NDLC4X/p1699268921121009

[sergargar]

Hi @Fennerr , thank you so much for all the study and information! Recently, @mtronrd fixed most of this issues in https://github.com/prowler-cloud/prowler/pull/3006, can you try it again in master?

[jfagoagas]

Hi @Fennerr, could you please test this out again in the master branch and let us know if this is fixed?

Thanks!

[jfagoagas]

I’m closing this issue. Please feel free to reopen it if you notice that behaviour again.

Thanks for using Prowler 🚀