Policy conditions not enforced in resource policy?

[skuenzli]

I'm testing IAMSpy and it looks like policy conditions in resource policy are / may not be enforced. What behavior is expected?

I've created a failing test case here: https://github.com/skuenzli/IAMSpy/commit/3614947751b8507d1c5eed56956b06cbb600e0b7

But basically the test case uses:

• allow-testing-s3.json GAAD which grants no permissions to role testing via Identity policies
• a bucket policy that Allows s3:GetObject to principal * and narrows that with an aws:PrincipalArn condition

Statement:

{
          "Sid": "AllowNonExistentPrincipal",
          "Effect": "Allow",
          "Principal": "*",
          "Action": "s3:GetObject",
          "Resource": [
            "arn:aws:s3:::bucket",
            "arn:aws:s3:::bucket/*"
          ],
          "Condition": {
            "ArnEquals": {
              "aws:PrincipalArn": [
                "arn:aws:iam::111111111111:user/some-other-user"
              ]
            }
          }
        }

IAMSpy can_i reports the testing principal has the s3:GetObject permission to the bucket.

FWIW, I have tried both strict_conditions=True and False

Is this behavior expected?

AFAICT (from debug output in my private library integration), the condition is parsed from the statement in the resource policy.

[Skybound1]

Yup this is currently a known issue. So condition support has started, but the ties between the various condition keys haven't been fully setup yet. At the moment, it will enforce that the PrincipalArn as a string matches should multiple policies require them to be different, but it is yet to compare this against the source ARN. Once the SCP branch has been merged in, that is next on my todo list :)

[skuenzli]

Thanks for confirming the issue and those details. I will look into the issue and try to fix it in the meantime.

[skuenzli]

@Skybound1 - quick update:

I spent a couple days getting acquainted with IAMSpy and trying to figure out where to add/copy the condition key constraints.

It seems like parse.py's generate_evaluation_logic_checks needs something like:

    if source:
        if identity_identifier not in model_vars:
            constraints.append(identity_check == False)  # noqa: E712
        source_account = source.split(":")[4]
        constraints.append(z3.String("s_account") == z3.StringVal(source_account))
        # WIP add context keys to identity side of model so conditions in resource policy are respected
        # (using specific condition names below just for exploration/poc)
        constraints.extend([
            z3.Bool("condition_aws:PrincipalArn_exists") == True,  # noqa: E712
            z3.String("condition_aws:PrincipalArn") == z3.StringVal(source),
        ])

But I'm stuck and don't feel like I really understand what I'm doing.

If you can provide some guidance on how to implement this, or even some recommended reading about using z3 String expressions, I'm happy to work on this some more.

[Skybound1]

:thinking: TBH, considering the scale of work that needs to go into conditions, I don't think putting it into that function would be the cleanest, imho we should have a separate function that is called to generate all the condition constraints.

Also, so I would make a few tweaks to that:

1. It shouldn't be added as part of the if source, this constraint should always be in place, as it should also apply for say the who_can operation where the source isn't defined but computed by the model, but this constraint should still apply. I think this is just a constraint that is always added in. This constraint should just be linking the source ARN to the condition key. Policy conditions just compare the condition_key variable against whatever is defined in the policy, but as that isn't constrained anywhere, IAMSpy can just set that to whatever it wants. This is just to tell IAMSpy that this variable should always be linked to the source. There will be a tad bit we will need to change when it comes to assumed role ARNs vs role ARNs, but that's a problem for later I think.
2. I wouldn't enforce the exists == True, the condition_key_exists labels are used for the strict condition checking part of the tool, not for making sure the values are legit.
3. So there is a difference between z3.String and z3.StringVal. z3.String is for declaring and using a variable. So this will be something that we let the model play with, and the string we use as the argument is the name / label of that variable. z3.StringVal defines a constant, and the argument is the value the constant is set to. In this case, I don't think we need to define any constants, we just need to define a constraint saying whatever is in condition_aws:PrincipalArn is the same as whats in s (well named I know xD)... so that would be something like z3.String("condition_aws:PrincipalArn") == z3.String("s")

Annoyingly, I haven't found that many great docs on using Python Z3, so it's been a lot of experimenting myself, reading through general Z3 docs and trying to figure out how to conver that to Python, etc

[skuenzli]

Thanks for the detailed guidance.

I had similar thoughts that:

• the constraint should always apply, regardless of source; I was mostly just experimenting in the path that was executing.
• some well-named functions like apply_constraints_for_conditions might help modularize and clarify intent
• z3py docs have big gaps; I've taken to skimming questions tagged with z3py on StackOverflow since I'm not even sure how to ask/phrase questions for search

I'll try to give this another shot next week.