From cce088d19878b4796827957dc320e836bef7311b Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Thu, 21 Sep 2017 10:01:58 -0700 Subject: [PATCH 1/3] lib: gpo: Changes order to match GPO application order. The order of GPOs in a gpo_list generated by ads_get_gpo_list did not match the order of application. Since GPOs are pushed to the FRONT of gpo_list, GPOs have to be pushed in the opposite order of application. (Pushing to front is useful to get inheritance blocking right). Signed-off-by: Lutz Justen Reviewed-by: Jeremy Allison Reviewed-by: Guenther Deschner --- libgpo/gpo_ldap.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libgpo/gpo_ldap.c b/libgpo/gpo_ldap.c index 4533d61a1e3..7ede12c3c21 100644 --- a/libgpo/gpo_ldap.c +++ b/libgpo/gpo_ldap.c @@ -812,12 +812,6 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, dump_gplink(&gp_link); } - /* block inheritance from now on */ - if (gp_link.gp_opts & - GPOPTIONS_BLOCK_INHERITANCE) { - add_only_forced_gpos = true; - } - status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list, @@ -829,6 +823,12 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, if (!ADS_ERR_OK(status)) { return status; } + + /* block inheritance from now on */ + if (gp_link.gp_opts & + GPOPTIONS_BLOCK_INHERITANCE) { + add_only_forced_gpos = true; + } } } @@ -858,12 +858,6 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, dump_gplink(&gp_link); } - /* block inheritance from now on */ - if (gp_link.gp_opts & - GPOPTIONS_BLOCK_INHERITANCE) { - add_only_forced_gpos = true; - } - status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list, @@ -875,6 +869,12 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, if (!ADS_ERR_OK(status)) { return status; } + + /* block inheritance from now on */ + if (gp_link.gp_opts & + GPOPTIONS_BLOCK_INHERITANCE) { + add_only_forced_gpos = true; + } } } -- 2.11.0 From 9c3f10053dbf2773492089eb9452c35dff0df483 Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Thu, 21 Sep 2017 10:11:15 -0700 Subject: [PATCH 2/3] 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. Signed-off-by: Lutz Justen Reviewed-by: Jeremy Allison Reviewed-by: Guenther Deschner --- libgpo/gpo_ldap.c | 139 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 78 insertions(+), 61 deletions(-) diff --git a/libgpo/gpo_ldap.c b/libgpo/gpo_ldap.c index 7ede12c3c21..620687977e5 100644 --- a/libgpo/gpo_ldap.c +++ b/libgpo/gpo_ldap.c @@ -561,9 +561,18 @@ static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads, const struct security_token *token) { ADS_STATUS status; - int i; + uint32_t i; + + if (gp_link->num_links == 0) { + return ADS_ERROR(LDAP_SUCCESS); + } - for (i = 0; i < gp_link->num_links; i++) { + /* + * Note: DLIST_ADD pushes to the front, + * so loop from last to first to get the + * order right. + */ + for (i = gp_link->num_links - 1; i >= 0; i--) { struct GROUP_POLICY_OBJECT *new_gpo = NULL; @@ -726,7 +735,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 +762,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 +789,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 +805,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, } tmp_dn = parent_dn; + } /* reset dn again */ @@ -841,13 +814,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 +835,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 +851,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); } -- 2.11.0 From 7d0b3cbc596fcd6348b71aa40499a10b415227b5 Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Thu, 21 Sep 2017 10:32:05 -0700 Subject: [PATCH 3/3] lib: gpo: Put enforced GPOs at the end of the list. Enforced GPOs should be applied on top of all non-enforced GPOs, so that they override policies set in non-enforced GPOs. Signed-off-by: Lutz Justen Reviewed-by: Jeremy Allison Reviewed-by: Guenther Deschner --- libgpo/gpo_ldap.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/libgpo/gpo_ldap.c b/libgpo/gpo_ldap.c index 620687977e5..07aa9e3799e 100644 --- a/libgpo/gpo_ldap.c +++ b/libgpo/gpo_ldap.c @@ -554,6 +554,7 @@ ADS_STATUS ads_get_gpo(ADS_STRUCT *ads, static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, struct GROUP_POLICY_OBJECT **gpo_list, + struct GROUP_POLICY_OBJECT **forced_gpo_list, const char *link_dn, struct GP_LINK *gp_link, enum GPO_LINK_TYPE link_type, @@ -573,8 +574,10 @@ static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads, * order right. */ for (i = gp_link->num_links - 1; i >= 0; i--) { - + struct GROUP_POLICY_OBJECT **target_list = NULL; struct GROUP_POLICY_OBJECT *new_gpo = NULL; + bool is_forced = + (gp_link->link_opts[i] & GPO_LINK_OPT_ENFORCED) != 0; if (gp_link->link_opts[i] & GPO_LINK_OPT_DISABLED) { DEBUG(10,("skipping disabled GPO\n")); @@ -583,7 +586,7 @@ static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads, if (only_add_forced_gpos) { - if (!(gp_link->link_opts[i] & GPO_LINK_OPT_ENFORCED)) { + if (!is_forced) { DEBUG(10,("skipping nonenforced GPO link " "because GPOPTIONS_BLOCK_INHERITANCE " "has been set\n")); @@ -626,7 +629,8 @@ static ADS_STATUS add_gplink_to_gpo_list(ADS_STRUCT *ads, new_gpo->link = link_dn; new_gpo->link_type = link_type; - DLIST_ADD(*gpo_list, new_gpo); + target_list = is_forced ? forced_gpo_list : gpo_list; + DLIST_ADD(*target_list, new_gpo); DEBUG(10,("add_gplink_to_gplist: added GPLINK #%d %s " "to GPO list\n", i, gp_link->link_names[i])); @@ -725,24 +729,27 @@ static ADS_STATUS add_local_policy_to_gpo_list(TALLOC_CTX *mem_ctx, } /**************************************************************** - get the full list of GROUP_POLICY_OBJECTs for a given dn + Get the full list of GROUP_POLICY_OBJECTs for a given dn. ****************************************************************/ -ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, +static ADS_STATUS ads_get_gpo_list_internal(ADS_STRUCT *ads, TALLOC_CTX *mem_ctx, const char *dn, uint32_t flags, const struct security_token *token, - struct GROUP_POLICY_OBJECT **gpo_list) + struct GROUP_POLICY_OBJECT **gpo_list, + struct GROUP_POLICY_OBJECT **forced_gpo_list) { /* * 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. + * For different domains and OUs: parent-to-child. + * Within same level of domains and OUs: Link order. * 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). + * Forced GPOs are appended in the end since they override all others. */ ADS_STATUS status; @@ -751,6 +758,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, bool add_only_forced_gpos = false; ZERO_STRUCTP(gpo_list); + ZERO_STRUCTP(forced_gpo_list); if (!dn) { return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); @@ -787,6 +795,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list, + forced_gpo_list, parent_dn, &gp_link, GP_LINK_OU, @@ -833,6 +842,7 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list, + forced_gpo_list, parent_dn, &gp_link, GP_LINK_DOMAIN, @@ -875,8 +885,12 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, dump_gplink(&gp_link); } - status = add_gplink_to_gpo_list(ads, mem_ctx, gpo_list, - site_dn, &gp_link, + status = add_gplink_to_gpo_list(ads, + mem_ctx, + gpo_list, + forced_gpo_list, + site_dn, + &gp_link, GP_LINK_SITE, add_only_forced_gpos, token); @@ -902,4 +916,35 @@ ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, return ADS_ERROR(LDAP_SUCCESS); } +/**************************************************************** + Get the full list of GROUP_POLICY_OBJECTs for a given dn, wrapper + around ads_get_gpo_list_internal() that ensures correct ordering. +****************************************************************/ + +ADS_STATUS ads_get_gpo_list(ADS_STRUCT *ads, + TALLOC_CTX *mem_ctx, + const char *dn, + uint32_t flags, + const struct security_token *token, + struct GROUP_POLICY_OBJECT **gpo_list) +{ + struct GROUP_POLICY_OBJECT *forced_gpo_list = NULL; + ADS_STATUS status = ads_get_gpo_list_internal(ads, + mem_ctx, + dn, + flags, + token, + gpo_list, + &forced_gpo_list); + if (!ADS_ERR_OK(status)) { + return status; + } + /* + * Append |forced_gpo_list| at the end of |gpo_list|, + * so that forced GPOs are applied on top of non enforced GPOs. + */ + DLIST_CONCATENATE(*gpo_list, forced_gpo_list); + return status; +} + #endif /* HAVE_LDAP */ -- 2.11.0