GPO evaluation of primary group #7071

thalman · 2023-12-06T15:27:34Z

When we are evaluating GPO the SID of user's primary group is not returned in the list. This patch converts the value of origPrimaryGroupGidNumber attribute back to SID and that SID is added to the list of SIDs before evaluating the GPO rules.

sumit-bose · 2023-12-08T14:00:15Z

src/providers/ad/ad_gpo.c

+                                           SYSDB_PRIMARY_GROUP_GIDNUM,
+                                           0);
+    if (orig_gid != 0) {
+        sss_idmap_unix_to_sid(idmap_ctx, orig_gid, &orig_gid_sid);


Hi,

please check return code of sss_idmap_unix_to_sid() because it might return IDMAP_EXTERNAL. In this case there is no id-mapping for the domain the orig_gid is coming from but the values are stored in the group object. In this case you have to search the cache for the group with orig_gid and take the SID from the cached group.

bye,
Sumit

Thanks for explanation/hint, will do

sumit-bose · 2023-12-20T11:53:59Z

src/providers/ad/ad_gpo.c

+            return NULL;
+        }
+
+        sid = discard_const(ldb_msg_find_attr_as_string(msg, SYSDB_SID_STR, NULL));


Hi,

I would prefer to avoid discard_const() even if this would mean to declare another variable like e.g. const char *tmp;.

bye,
Sumit

sumit-bose

Hi,

thanks for the update, works for me, ACK.

bye,
Sumit

alexey-tikhonov · 2023-12-22T10:16:39Z

To be rebased on the top of #7107

danlavu

Tested, working.

(2024-01-08 11:20:02): [be[domain-8l9d.com]] [ad_gpo_access_check] (0x0400): [RID#34] user_sid = S-1-5-21-1477186385-3752140279-2633954784-1122
(2024-01-08 11:20:02): [be[domain-8l9d.com]] [ad_gpo_access_check] (0x0400): [RID#34] group_sids[0] = S-1-5-21-1477186385-3752140279-2633954784-513
(2024-01-08 11:20:02): [be[domain-8l9d.com]] [ad_gpo_access_check] (0x0400): [RID#34] group_sids[1] = S-1-5-21-1477186385-3752140279-2633954784-1113
(2024-01-08 11:20:02): [be[domain-8l9d.com]] [ad_gpo_access_check] (0x0400): [RID#34] group_sids[2] = S-1-5-11

alexey-tikhonov · 2024-01-09T16:18:38Z

@sumit-bose, @danlavu, please re-review after rebase.

thalman · 2024-01-09T16:22:45Z

JFI, The rebase went smoothly, I had to add one more change when calling ad_gpo_get_sids.

When we are evaluating GPO the SID of user's primary group is not returned in the list. This patch converts the value of origPrimaryGroupGidNumber attribute back to SID and that SID is added to the list of SIDs before evaluating the GPO rules.

thalman · 2024-01-09T16:58:26Z

src/providers/ad/ad_gpo.c

+    ret = ad_gpo_get_sids(tmp_ctx, host_fqdn, host_domain, idmap_ctx, &host_sid,
                          &host_group_sids, &host_group_size);


@sumit-bose, @danlavu, This has been added since last review

danlavu · 2024-01-09T19:39:51Z

Tested.
unpatched

(2024-01-09 14:36:28): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#6] CURRENT USER: (2024-01-09 14:36:28): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#6] user_sid = S-1-5-21-453157533-3668325555-2162132090-1121 (2024-01-09 14:36:28): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#6] group_sids[0] = S-1-5-21-453157533-3668325555-2162132090-513 (2024-01-09 14:36:28): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#6] group_sids[1] = S-1-5-11

patched

(2024-01-09 14:38:05): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#7] CURRENT USER: (2024-01-09 14:38:05): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#7] user_sid = S-1-5-21-453157533-3668325555-2162132090-1121 (2024-01-09 14:38:05): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#7] group_sids[0] = S-1-5-21-453157533-3668325555-2162132090-513 (2024-01-09 14:38:05): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#7] group_sids[1] = S-1-5-11 (2024-01-09 14:38:05): [be[domain-2875.com]] [ad_gpo_access_check] (0x0400): [RID#7] group_sids[2] = S-1-5-21-453157533-3668325555-2162132090-513

Working as expected, approved.

sumit-bose

Hi,

rebased and updated version still works for me, ACK.

bye,
Sumit

alexey-tikhonov · 2024-01-10T08:38:18Z

Pushed PR: #7071

master
- ecb0c63 - GPO evaluation of primary group
sssd-2-9
- 05de56d - GPO evaluation of primary group

thalman requested a review from sumit-bose December 6, 2023 15:27

alexey-tikhonov added the backport-to-stable label Dec 7, 2023

andreboscatto assigned danlavu and sumit-bose Dec 7, 2023

andreboscatto requested a review from danlavu December 7, 2023 13:51

alexey-tikhonov added the Waiting for review label Dec 7, 2023

sumit-bose reviewed Dec 8, 2023

View reviewed changes

thalman marked this pull request as draft December 11, 2023 07:53

thalman force-pushed the gpoprimarygroup branch from dccf804 to d9365bc Compare December 12, 2023 09:14

thalman marked this pull request as ready for review December 12, 2023 09:35

sumit-bose reviewed Dec 20, 2023

View reviewed changes

sumit-bose added Changes requested and removed Waiting for review labels Dec 20, 2023

thalman force-pushed the gpoprimarygroup branch from d9365bc to c53e7a9 Compare December 20, 2023 15:32

thalman added Waiting for review and removed Changes requested labels Dec 20, 2023

sumit-bose approved these changes Dec 21, 2023

View reviewed changes

danlavu approved these changes Jan 8, 2024

View reviewed changes

thalman force-pushed the gpoprimarygroup branch from c53e7a9 to b4794ed Compare January 9, 2024 16:16

alexey-tikhonov requested review from danlavu and sumit-bose January 9, 2024 16:18

thalman force-pushed the gpoprimarygroup branch from b4794ed to 31bb4b9 Compare January 9, 2024 16:40

GPO evaluation of primary group

8fa54e6

When we are evaluating GPO the SID of user's primary group is not returned in the list. This patch converts the value of origPrimaryGroupGidNumber attribute back to SID and that SID is added to the list of SIDs before evaluating the GPO rules.

thalman force-pushed the gpoprimarygroup branch from 31bb4b9 to 8fa54e6 Compare January 9, 2024 16:57

thalman commented Jan 9, 2024

View reviewed changes

danlavu approved these changes Jan 9, 2024

View reviewed changes

sumit-bose approved these changes Jan 10, 2024

View reviewed changes

sumit-bose added Accepted and removed Waiting for review labels Jan 10, 2024

alexey-tikhonov added the Ready to push Ready to push label Jan 10, 2024

alexey-tikhonov added Pushed and removed Accepted Ready to push Ready to push labels Jan 10, 2024

alexey-tikhonov closed this Jan 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GPO evaluation of primary group #7071

GPO evaluation of primary group #7071

thalman commented Dec 6, 2023

sumit-bose Dec 8, 2023

thalman Dec 11, 2023

thalman Dec 12, 2023

sumit-bose Dec 20, 2023

thalman Dec 20, 2023

sumit-bose left a comment

alexey-tikhonov commented Dec 22, 2023

danlavu left a comment

alexey-tikhonov commented Jan 9, 2024

thalman commented Jan 9, 2024 •

edited

Loading

thalman Jan 9, 2024

danlavu commented Jan 9, 2024 •

edited

Loading

sumit-bose left a comment

alexey-tikhonov commented Jan 10, 2024

		ret = ad_gpo_get_sids(tmp_ctx, host_fqdn, host_domain, idmap_ctx, &host_sid,
		&host_group_sids, &host_group_size);

GPO evaluation of primary group #7071

GPO evaluation of primary group #7071

Conversation

thalman commented Dec 6, 2023

sumit-bose Dec 8, 2023

Choose a reason for hiding this comment

thalman Dec 11, 2023

Choose a reason for hiding this comment

thalman Dec 12, 2023

Choose a reason for hiding this comment

sumit-bose Dec 20, 2023

Choose a reason for hiding this comment

thalman Dec 20, 2023

Choose a reason for hiding this comment

sumit-bose left a comment

Choose a reason for hiding this comment

alexey-tikhonov commented Dec 22, 2023

danlavu left a comment

Choose a reason for hiding this comment

alexey-tikhonov commented Jan 9, 2024

thalman commented Jan 9, 2024 • edited Loading

thalman Jan 9, 2024

Choose a reason for hiding this comment

danlavu commented Jan 9, 2024 • edited Loading

sumit-bose left a comment

Choose a reason for hiding this comment

alexey-tikhonov commented Jan 10, 2024

thalman commented Jan 9, 2024 •

edited

Loading

danlavu commented Jan 9, 2024 •

edited

Loading