AAD Rego policies that examine conditional access are not checking for guest user exclusions or application exclusions

[tkol2022]

🐛 Summary

The AAD Rego policies that examine conditional access configurations are not checking to see if the user excluded any guest users or excluded any cloud applications. An adversary could configure some guest user exclusions (to say an MFA policy) as a way to circumvent the security enforcement and ScubaGear would still pass the tenant. The scope of this issue is to augment the existing Rego code to include these checks. A side problem is that the affected policies in the AADConfig.rego file are not referencing the AAD helper rulesets consistently which will also be corrected with this work.

This issue affects the following policies in the AADConfig.rego file:

• MS.AAD.1.1v1, MS.AAD.2.1v1, MS.AAD.2.3v1, MS.AAD.3.1v1, MS.AAD.3.2v1, MS.AAD.3.6v1, MS.AAD.3.7v1, MS.AAD.3.8v1

To reproduce

Steps to reproduce the behavior:

1. Open one of the conditional access policies related to Scuba such as the policy that disables legacy authentication (AAD 1.1).
2. Click on the Users page and then exclude all guest users.
3. Run ScubaGear and you will see that it incorrectly passes the policy check.
4. Open the conditional access policy related to AAD 2.1
5. Click on the Target resources page and then exclude some cloud apps such as MS Admin Portals.
6. Run ScubaGear and you will see that it incorrectly passes the policy check.

Implementation Notes

The PolicyConditionsMatch ruleset in the Rego helper file Utils/AAD.Rego is where the logic checks for the respective exclusions should be coded and this ruleset should be referenced from all AAD policies in AADConfig.Rego that examine conditional access to ensure that we have a consistent implementation. Currently PolicyConditionsMatch is only referenced by some of the AAD policies in the AADConfig.Rego file. I included instructions to ensure we consistently call the helper rulesets in the list below.

• [ ] Go through each of the affected AADConfig.Rego policies noted at the top of this issue and ensure that they call the helper ruleset PolicyConditionsMatch instead of implementing the same or similar logic. In the screenshots below I have included notes on what we do NOT want, versus what we want to clarify.

• [ ] Modify the PolicyConditionsMatch ruleset so that guest user and application exclusions are not allowed.

• [ ] Write new unit tests for the affected policies that ensure the guest user and application exclusions are not allowed (produce a fail)

[tkol2022]

A placed this as high priority since the tool can incorrectly produce a Pass when the user has configured the types of exclusions described in this issue, therefore bypassing the intended enforcement of the policy.

[schrolla]

Review against epic #754 to see if these conditions should be included in that epic.

[tkol2022]

Review against epic #754 to see if these conditions should be included in that epic.

I added it.

[tkol2022]

Scope refinement

This comment slightly modifies the original requirements and adds more context by providing screenshots of the conditional access admin portal for reference.

The following requirements are updated:

• Modify the PolicyConditionsMatch ruleset so that guest user, directory roles and application exclusions are not allowed. I added directory roles since that was missing in the initial requirements.

Example guest user and directory role exclusions are provided in the screenshot below:

Example application exclusions are provided in the screenshot below:

[tkol2022]

Final scope change

The implemented changes in the PR vary slightly from the most recent scope definition. The code now prohibits role exclusions and application exclusions. We are not prohibiting guest user exclusions yet because we are working with an agency and Microsoft to refine the requirements on guest users.

We also removed PolicyConditionsMatch so any comments about that are OBE now.