lib: gpo: Fixes issue with GPOPTIONS_BLOCK_INHERITANCE.

GP links with the GPOPTIONS_BLOCK_INHERITANCE option set
were blocking GPOs from the same link (i.e. an OU with
the flag set would block its own GPOs). This patch makes
sure the GPOs from the link are added to the list.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13046

Signed-off-by: Lutz Justen <ljusten@google.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>

diff --git a/libgpo/gpo_ldap.c b/libgpo/gpo_ldap.c
index 7ede12c3c21d9c0667b2f82eb9a7342e99e91b77..8fb5fdc40c1d67d3c2b610969201c78a7d4c7a0a 100644 (file)
--- a/libgpo/gpo_ldap.c
+++ b/libgpo/gpo_ldap.c
@@ -561,10 +561,16 @@ static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads,
                                         const struct security_token *token)
 {
        ADS_STATUS status;
-       int i;
-
-       for (i = 0; i < gp_link->num_links; i++) {
-
+       uint32_t count;
+
+       /*
+        * Note: DLIST_ADD pushes to the front,
+        * so loop from last to first to get the
+        * order right.
+        */
+       for (count = gp_link->num_links; count > 0; count--) {
+               /* NB. Index into arrays is one less than counter. */
+               uint32_t i = count - 1;
                struct GROUP_POLICY_OBJECT *new_gpo = NULL;
 
                if (gp_link->link_opts[i] & GPO_LINK_OPT_DISABLED) {
@@ -726,7 +732,15 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
                            const struct security_token *token,
                            struct GROUP_POLICY_OBJECT **gpo_list)
 {
-       /* (L)ocal (S)ite (D)omain (O)rganizational(U)nit */
+       /*
+        * Push GPOs to gpo_list so that the traversal order of the list matches
+        * the order of application:
+        * (L)ocal (S)ite (D)omain (O)rganizational(U)nit
+        * Within domains and OUs: parent-to-child.
+        * Since GPOs are pushed to the front of gpo_list, GPOs have to be
+        * pushed in the opposite order of application (OUs first, local last,
+        * child-to-parent).
+        */
 
        ADS_STATUS status;
        struct GP_LINK gp_link;
@@ -745,63 +759,18 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
 
        DEBUG(10,("ads_get_gpo_list: getting GPO list for [%s]\n", dn));
 
-       /* (L)ocal */
-       status = add_local_policy_to_gpo_list(mem_ctx, gpo_list,
-                                             GP_LINK_LOCAL);
-       if (!ADS_ERR_OK(status)) {
-               return status;
-       }
-
-       /* (S)ite */
-
-       /* are site GPOs valid for users as well ??? */
-       if (flags & GPO_LIST_FLAG_MACHINE) {
-
-               status = ads_site_dn_for_machine(ads, mem_ctx,
-                                                ads->config.ldap_server_name,
-                                                &site_dn);
-               if (!ADS_ERR_OK(status)) {
-                       return status;
-               }
-
-               DEBUG(10,("ads_get_gpo_list: query SITE: [%s] for GPOs\n",
-                       site_dn));
-
-               status = ads_get_gpo_link(ads, mem_ctx, site_dn, &gp_link);
-               if (ADS_ERR_OK(status)) {
-
-                       if (DEBUGLEVEL >= 100) {
-                               dump_gplink(&gp_link);
-                       }
-
-                       status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list,
-                                                       site_dn, &gp_link,
-                                                       GP_LINK_SITE,
-                                                       add_only_forced_gpos,
-                                                       token);
-                       if (!ADS_ERR_OK(status)) {
-                               return status;
-                       }
-
-                       if (flags & GPO_LIST_FLAG_SITEONLY) {
-                               return ADS_ERROR(LDAP_SUCCESS);
-                       }
-
-                       /* inheritance can't be blocked at the site level */
-               }
-       }
-
        tmp_dn = dn;
 
        while ((parent_dn = ads_parent_dn(tmp_dn)) &&
               (!strequal(parent_dn, ads_parent_dn(ads->config.bind_path)))) {
 
-               /* (D)omain */
 
-               /* An account can just be a member of one domain */
-               if (strncmp(parent_dn, "DC=", strlen("DC=")) == 0) {
+               /* (O)rganizational(U)nit */
 
-                       DEBUG(10,("ads_get_gpo_list: query DC: [%s] for GPOs\n",
+               /* An account can be a member of more OUs */
+               if (strncmp(parent_dn, "OU=", strlen("OU=")) == 0) {
+
+                       DEBUG(10,("ads_get_gpo_list: query OU: [%s] for GPOs\n",
                                parent_dn));
 
                        status = ads_get_gpo_link(ads, mem_ctx, parent_dn,
@@ -817,7 +786,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
                                                        gpo_list,
                                                        parent_dn,
                                                        &gp_link,
-                                                       GP_LINK_DOMAIN,
+                                                       GP_LINK_OU,
                                                        add_only_forced_gpos,
                                                        token);
                                if (!ADS_ERR_OK(status)) {
@@ -833,6 +802,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
                }
 
                tmp_dn = parent_dn;
+
        }
 
        /* reset dn again */
@@ -841,13 +811,12 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
        while ((parent_dn = ads_parent_dn(tmp_dn)) &&
               (!strequal(parent_dn, ads_parent_dn(ads->config.bind_path)))) {
 
+               /* (D)omain */
 
-               /* (O)rganizational(U)nit */
-
-               /* An account can be a member of more OUs */
-               if (strncmp(parent_dn, "OU=", strlen("OU=")) == 0) {
+               /* An account can just be a member of one domain */
+               if (strncmp(parent_dn, "DC=", strlen("DC=")) == 0) {
 
-                       DEBUG(10,("ads_get_gpo_list: query OU: [%s] for GPOs\n",
+                       DEBUG(10,("ads_get_gpo_list: query DC: [%s] for GPOs\n",
                                parent_dn));
 
                        status = ads_get_gpo_link(ads, mem_ctx, parent_dn,
@@ -863,7 +832,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
                                                        gpo_list,
                                                        parent_dn,
                                                        &gp_link,
-                                                       GP_LINK_OU,
+                                                       GP_LINK_DOMAIN,
                                                        add_only_forced_gpos,
                                                        token);
                                if (!ADS_ERR_OK(status)) {
@@ -879,8 +848,53 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads,
                }
 
                tmp_dn = parent_dn;
+       }
+
+       /* (S)ite */
+
+       /* are site GPOs valid for users as well ??? */
+       if (flags & GPO_LIST_FLAG_MACHINE) {
 
-       };
+               status = ads_site_dn_for_machine(ads, mem_ctx,
+                                                ads->config.ldap_server_name,
+                                                &site_dn);
+               if (!ADS_ERR_OK(status)) {
+                       return status;
+               }
+
+               DEBUG(10,("ads_get_gpo_list: query SITE: [%s] for GPOs\n",
+                       site_dn));
+
+               status = ads_get_gpo_link(ads, mem_ctx, site_dn, &gp_link);
+               if (ADS_ERR_OK(status)) {
+
+                       if (DEBUGLEVEL >= 100) {
+                               dump_gplink(&gp_link);
+                       }
+
+                       status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list,
+                                                       site_dn, &gp_link,
+                                                       GP_LINK_SITE,
+                                                       add_only_forced_gpos,
+                                                       token);
+                       if (!ADS_ERR_OK(status)) {
+                               return status;
+                       }
+
+                       if (flags & GPO_LIST_FLAG_SITEONLY) {
+                               return ADS_ERROR(LDAP_SUCCESS);
+                       }
+
+                       /* inheritance can't be blocked at the site level */
+               }
+       }
+
+       /* (L)ocal */
+       status = add_local_policy_to_gpo_list(mem_ctx, gpo_list,
+                                             GP_LINK_LOCAL);
+       if (!ADS_ERR_OK(status)) {
+               return status;
+       }
 
        return ADS_ERROR(LDAP_SUCCESS);
 }