Implemented consistent checking of role and application exclusions ac…

…ross CAP policies. Removed PolicyConditionsMatch. Created unit test for 3.1 compliant usage of config file for user exclusions.
cisagov · Feb 4, 2025 · c06613f · c06613f
1 parent fa9d316
commit c06613f
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 64 deletions.
diff --git a/PowerShell/ScubaGear/Rego/AADConfig.rego b/PowerShell/ScubaGear/Rego/AADConfig.rego
@@ -16,10 +16,10 @@ import data.utils.aad.UserExclusionsFullyExempt
 import data.utils.aad.GroupExclusionsFullyExempt
 import data.utils.aad.Aad2P2Licenses
 import data.utils.aad.IsPhishingResistantMFA
-import data.utils.aad.PolicyConditionsMatch
 import data.utils.aad.CAPLINK
 import data.utils.aad.DomainReportDetails
 import data.utils.aad.INT_MAX
+import data.utils.key.Count
 
 
 #############
@@ -46,13 +46,21 @@ MEMBERUSER := "a0b1b346-4d3e-4e8b-98f8-753987be4970"
 LegacyAuthentication contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    # Match all simple conditions
-    PolicyConditionsMatch(CAPolicy, true) == true
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     "other" in CAPolicy.Conditions.ClientAppTypes
     "exchangeActiveSync" in CAPolicy.Conditions.ClientAppTypes
     "block" in CAPolicy.GrantControls.BuiltInControls
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.1.1v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.1.1v1") == true
 }
@@ -84,12 +92,20 @@ tests contains {
 BlockHighRisk contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    # Match all simple conditions
-    PolicyConditionsMatch(CAPolicy, true) == true
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     "high" in CAPolicy.Conditions.UserRiskLevels
     "block" in CAPolicy.GrantControls.BuiltInControls
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.2.1v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.2.1v1") == true
 }
@@ -137,12 +153,20 @@ tests contains {
 SignInBlocked contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    # Match all simple conditions
-    PolicyConditionsMatch(CAPolicy, true)
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     "high" in CAPolicy.Conditions.SignInRiskLevels
     "block" in CAPolicy.GrantControls.BuiltInControls
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.2.3v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.2.3v1") == true
 }
@@ -180,13 +204,21 @@ tests contains {
 PhishingResistantMFAPolicies contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    PolicyConditionsMatch(CAPolicy, true)
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
 
+    ### Conditional access checks specific to this policy
+    IsPhishingResistantMFA(CAPolicy) == true
+    ###
 
-    GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.1v1") == true
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.3.1v1") == true
-
-    IsPhishingResistantMFA(CAPolicy) == true
+    GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.1v1") == true
 }
 
 # Pass if at least 1 policy meets all conditions
@@ -215,11 +247,19 @@ AllMFA := NonSpecificMFAPolicies | PhishingResistantMFAPolicies
 NonSpecificMFAPolicies contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    # Match all simple conditions
-    PolicyConditionsMatch(CAPolicy, true)
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     "mfa" in CAPolicy.GrantControls.BuiltInControls
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.3.2v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.2v1") == true
 }
@@ -399,22 +439,24 @@ tests contains {
 PhishingResistantMFAPrivilegedRoles contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    PolicyConditionsMatch(CAPolicy, false)
+    ### Common checks for conditional access policies
+    ### We don't check IncludeUsers All because this is a role based policy
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     PrivRolesSet := ConvertToSetWithKey(input.privileged_roles, "RoleTemplateId")
     # Make sure all the necessary roles are included
     count(PrivRolesSet - ConvertToSet(CAPolicy.Conditions.Users.IncludeRoles)) == 0
+    IsPhishingResistantMFA(CAPolicy) == true
+    ###
 
-    # Confirm excluded roles do not contain any of the privileged roles
-    # (if it does, that means you are excluding it which leaves role unprotected)
-    # Ted thinks the next line is not needed
-    # count(PrivRolesSet & ConvertToSet(CAPolicy.Conditions.Users.ExcludeRoles)) == 0
-
-    # Only match policies with user and group exclusions if all exempted
-    GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.6v1") == true
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.3.6v1") == true
-
-    # Policy has only acceptable MFA
-    IsPhishingResistantMFA(CAPolicy) == true
+    GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.6v1") == true
 }
 
 # Pass if at least 1 policy meets all conditions
@@ -440,14 +482,22 @@ tests contains {
 ManagedDeviceAuth contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    PolicyConditionsMatch(CAPolicy, true) == true
+    ### Common checks for conditional access policies
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Contains(CAPolicy.Conditions.Applications.IncludeApplications, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    Count(CAPolicy.Conditions.Applications.ExcludeApplications) == 0
+    CAPolicy.State == "enabled"
+    ###
 
+    ### Conditional access checks specific to this policy
     "compliantDevice" in CAPolicy.GrantControls.BuiltInControls
     "domainJoinedDevice" in CAPolicy.GrantControls.BuiltInControls
     count(CAPolicy.GrantControls.BuiltInControls) == 2
     CAPolicy.GrantControls.Operator == "OR"
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.3.7v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.7v1") == true
 }
@@ -470,21 +520,28 @@ tests contains {
 # MS.AAD.3.8v1
 #--
 
-# If policy matches basic conditions, & needed strings
-# are in bult in controls, save the policy name
+# Checks to ensure a managed device is required to perform MFA registration
 RequireManagedDeviceMFA contains CAPolicy.DisplayName if {
     some CAPolicy in input.conditional_access_policies
 
-    PolicyConditionsMatch(CAPolicy, true)
+    ### Common checks for conditional access policies
+    ### We don't check IncludeApplications and ExcludeApplications because they are not relevant when you have an IncludeUserActions node
+    Contains(CAPolicy.Conditions.Users.IncludeUsers, "All") == true
+    Count(CAPolicy.Conditions.Users.ExcludeRoles) == 0
+    CAPolicy.State == "enabled"
+    ###
+
+    ### Conditional access checks specific to this policy
     Contains(CAPolicy.Conditions.Applications.IncludeUserActions, "urn:user:registersecurityinfo") == true
 
     Conditions := [
         "compliantDevice" in CAPolicy.GrantControls.BuiltInControls,
         "domainJoinedDevice" in CAPolicy.GrantControls.BuiltInControls,
     ]
     count(FilterArray(Conditions, true)) > 0
+    ###
 
-    # Only match policies with user and group exclusions if all exempted
+    # Only match policies with user and group exclusions per the confile file
     UserExclusionsFullyExempt(CAPolicy, "MS.AAD.3.8v1") == true
     GroupExclusionsFullyExempt(CAPolicy, "MS.AAD.3.8v1") == true
 }

diff --git a/PowerShell/ScubaGear/Rego/Utils/AAD.rego b/PowerShell/ScubaGear/Rego/Utils/AAD.rego
@@ -144,36 +144,6 @@ GroupExclusionsFullyExempt(Policy, PolicyID) := true if {
 # General AAD Functions #
 #########################
 
-# Internal helper rule to handle the conditional logic for "All" users
-Check_For_All_Users(Policy, CheckForAllUsers) if {
-    # If false is passed then do not implement the check
-    CheckForAllUsers == false
-} {
-    CheckForAllUsers == true
-    Contains(Policy.Conditions.Users.IncludeUsers, "All") == true
-}
-
-# PolicyConditionsMatch returns true if conditional access policy matches all conditions below:
-# IncludeUsers = All (you can bypass this by passing false in the CheckForAllUsers parameter)
-# IncludeApplications = All
-# ExcludeRoles is empty
-# ExcludeApplications is empty
-# Policy state = enabled
-
-PolicyConditionsMatch(Policy, CheckForAllUsers) := true if {
-    # If CheckForAllUsers is true then check for "All" users
-    Check_For_All_Users(Policy, CheckForAllUsers)
-
-    Contains(Policy.Conditions.Applications.IncludeApplications, "All") == true
-    Count(Policy.Conditions.Users.ExcludeRoles) == 0
-    Count(Policy.Conditions.Applications.ExcludeApplications) == 0
-    Policy.State == "enabled"
-
-    # Uncomment this line of code when we want to check for external or guest users
-    # Object.get() protects against undefined errors
-    # Count(object.get(Policy, ["Conditions", "Users", "ExcludeGuestsOrExternalUsers", "GuestOrExternalUserTypes"], null)) == 0
-} else := false
-
 # Save the Allowed MFA items as a set, check if there are any MFA
 # items allowed besides the acceptable ones & if there is at least
 # 1 MFA item allowed. Return true

diff --git a/PowerShell/ScubaGear/Testing/Unit/Rego/AAD/AADConfig_03_test.rego b/PowerShell/ScubaGear/Testing/Unit/Rego/AAD/AADConfig_03_test.rego
@@ -70,6 +70,7 @@ test_PhishingResistantMFAExcludeApp_Incorrect if {
     TestResult("MS.AAD.3.1v1", Output, ReportDetailStr, false) == true
 }
 
+# User / Group exclusions tests
 test_PhishingResistantMFAExcludeUser_Incorrect if {
     CAP := json.patch(ConditionalAccessPolicies,
                 [{"op": "add", "path": "Conditions/Users/ExcludeUsers", "value": ["me"]}])
@@ -91,6 +92,25 @@ test_PhishingResistantMFAExcludeGroup_Incorrect if {
         "0 conditional access policy(s) found that meet(s) all requirements. <a href='#caps'>View all CA policies</a>."
     TestResult("MS.AAD.3.1v1", Output, ReportDetailStr, false) == true
 }
+
+# Make sure user and group exclusions defined in config file pass the policy
+test_PhishingResistantMFAUserGroupExclusion_Correct if {
+    CAP := json.patch(ConditionalAccessPolicies,
+                [{"op": "add", "path": "Conditions/Users/ExcludeUsers", "value": ["49b4dcdf-1f90-41a7c3609b425-9dd7-5e3", "39b4dcdf-1f90-41a7c3609b425-9dd7-5e2"]},
+                {"op": "add", "path": "Conditions/Users/ExcludeGroups", "value": ["59b4dcdf-1f90-41a7c3609b425-9dd7-5e3", "69b4dcdf-1f90-41a7c3609b425-9dd7-5e3"]}])
+
+    Output := aad.tests with input.conditional_access_policies as [CAP]
+                        with input.scuba_config.Aad["MS.AAD.3.1v1"] as ScubaConfig
+                        with input.scuba_config.Aad["MS.AAD.3.1v1"].CapExclusions.Users as ["49b4dcdf-1f90-41a7c3609b425-9dd7-5e3", "39b4dcdf-1f90-41a7c3609b425-9dd7-5e2"]
+                        with input.scuba_config.Aad["MS.AAD.3.1v1"].CapExclusions.Groups as ["59b4dcdf-1f90-41a7c3609b425-9dd7-5e3", "69b4dcdf-1f90-41a7c3609b425-9dd7-5e3"]
+
+    ReportDetailStr := concat("", [
+        "1 conditional access policy(s) found that meet(s) all requirements:",
+        "<br/>Test Policy. <a href='#caps'>View all CA policies</a>."
+    ])
+
+    TestResult("MS.AAD.3.1v1", Output, ReportDetailStr, true) == true
+}
 #--
 
 #