From dce025b882db7247571b135e928afb47f069a60f Mon Sep 17 00:00:00 2001 From: Sumit Bose 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 --- 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