From dce025b882db7247571b135e928afb47f069a60f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Thu, 27 Feb 2020 06:54:21 +0100
Subject: [PATCH] GPO: fix link order in a SOM

GPOs of the same OU were applied in the wrong order. Details about how
GPOs should be processed can be found e.g. at
https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/dn581922(v%3Dws.11)

Resolves: https://github.com/SSSD/sssd/issues/5103

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
---
 src/providers/ad/ad_gpo.c | 59 +++++++++++++++++++++++++++++----------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index bbe8d8a1e..1524c4bfc 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -3511,14 +3511,19 @@ ad_gpo_process_som_recv(struct tevent_req *req,
  * - GPOs linked to an OU will be applied after GPOs linked to a Domain,
  *   which will be applied after GPOs linked to a Site.
  * - multiple GPOs linked to a single SOM are applied in their link order
- *   (i.e. 1st GPO linked to SOM is applied after 2nd GPO linked to SOM, etc).
+ *   (i.e. 1st GPO linked to SOM is applied before 2nd GPO linked to SOM, etc).
  * - enforced GPOs are applied after unenforced GPOs.
  *
  * As such, the _candidate_gpos output's dn fields looks like (in link order):
- * [unenforced {Site, Domain, OU}; enforced {Site, Domain, OU}]
+ * [unenforced {Site, Domain, OU}; enforced {OU, Domain, Site}]
  *
  * Note that in the case of conflicting policy settings, GPOs appearing later
- * in the list will trump GPOs appearing earlier in the list.
+ * in the list will trump GPOs appearing earlier in the list. Therefore the
+ * enforced GPOs are applied in revers order after the unenforced GPOs to
+ * make sure the enforced setting form the highest level will be applied.
+ *
+ * GPO processing details can be found e.g. at
+ * https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/dn581922(v%3Dws.11)
  */
 static errno_t
 ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
@@ -3542,6 +3547,7 @@ ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
     int i = 0;
     int j = 0;
     int ret;
+    size_t som_count = 0;
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
@@ -3568,6 +3574,7 @@ ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
         }
         i++;
     }
+    som_count = i;
 
     num_candidate_gpos = num_enforced + num_unenforced;
 
@@ -3590,9 +3597,43 @@ ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    i = som_count -1 ;
+    while (i >= 0) {
+        gp_som = som_list[i];
+
+        /* For unenforced_gpo_dns the most specific GPOs with the highest
+         * priority should be the last. We start with the top-level SOM and go
+         * down to the most specific one and add the unenforced following the
+         * gplink_list where the GPO with the highest priority comes last. */
+        j = 0;
+        while (gp_som && gp_som->gplink_list && gp_som->gplink_list[j]) {
+                gp_gplink = gp_som->gplink_list[j];
+
+                if (!gp_gplink->enforced) {
+                    unenforced_gpo_dns[unenforced_idx] =
+                        talloc_steal(unenforced_gpo_dns, gp_gplink->gpo_dn);
+
+                    if (unenforced_gpo_dns[unenforced_idx] == NULL) {
+                        ret = ENOMEM;
+                        goto done;
+                    }
+                    unenforced_idx++;
+                }
+                j++;
+        }
+        i--;
+    }
+
     i = 0;
     while (som_list[i]) {
         gp_som = som_list[i];
+
+        /* For enforced GPOs we start processing with the most specific SOM to
+         * make sur enforced GPOs from higher levels override to lower level
+         * ones. According to the 'Group Policy Inheritance' tab in the
+         * Windows 'Goup Policy Management' utility in the same SOM the link
+         * order is still observed and an enforced GPO with a lower link order
+         * value still overrides an enforced GPO with a higher link order. */
         j = 0;
         while (gp_som && gp_som->gplink_list && gp_som->gplink_list[j]) {
             gp_gplink = gp_som->gplink_list[j];
@@ -3610,16 +3651,6 @@ ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
                     goto done;
                 }
                 enforced_idx++;
-            } else {
-
-                unenforced_gpo_dns[unenforced_idx] =
-                    talloc_steal(unenforced_gpo_dns, gp_gplink->gpo_dn);
-
-                if (unenforced_gpo_dns[unenforced_idx] == NULL) {
-                    ret = ENOMEM;
-                    goto done;
-                }
-                unenforced_idx++;
             }
             j++;
         }
@@ -3638,7 +3669,7 @@ ad_gpo_populate_candidate_gpos(TALLOC_CTX *mem_ctx,
     }
 
     gpo_dn_idx = 0;
-    for (i = num_unenforced - 1; i >= 0; i--) {
+    for (i = 0; i < num_unenforced; i++) {
         candidate_gpos[gpo_dn_idx] = talloc_zero(candidate_gpos, struct gp_gpo);
         if (candidate_gpos[gpo_dn_idx] == NULL) {
             ret = ENOMEM;
-- 
2.21.3