Skip to content

Commit 4a5f0f2

Browse files
committed
auth: remember the origin of sids from the PAC
So far the conversion from TGT PAC to struct auth_user_info_dc back to TGS PAC looses the information in what part of the PAC_LOGON_INFO a sid was stored. With this change we let make_user_info_dc_{netlogon_validation,pac}() remember this, so that auth_convert_user_info_dc_sam{baseinfo,info6}() can rebuild the information into the desired parts of the PAC_LOGON_INFO. This was found and fixed for sid filter related tests, but it turns out that it already fixes a few tests from samba.tests.krb5.device_tests. All other places get an implicit AUTH_SID_ORIGIN_UNKNOWN (=0), which means we use the same logic as before. Signed-off-by: Stefan Metzmacher <[email protected]> Reviewed-by: Jennifer Sutton <[email protected]>
1 parent 551e236 commit 4a5f0f2

File tree

3 files changed

+30
-5
lines changed

3 files changed

+30
-5
lines changed

auth/auth_sam_reply.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@
2929
static bool is_base_sid(const struct auth_SidAttr *sid,
3030
const struct dom_sid *domain_sid)
3131
{
32+
if (sid->origin == AUTH_SID_ORIGIN_BASE) {
33+
goto check_domain;
34+
}
35+
36+
if (sid->origin != AUTH_SID_ORIGIN_UNKNOWN) {
37+
return false;
38+
}
39+
3240
if (sid->attrs & SE_GROUP_RESOURCE) {
3341
/*
3442
* Resource groups don't belong in the base
@@ -37,6 +45,7 @@ static bool is_base_sid(const struct auth_SidAttr *sid,
3745
return false;
3846
}
3947

48+
check_domain:
4049
/*
4150
* This SID belongs in the base structure only if it's in the account's
4251
* domain.
@@ -145,6 +154,13 @@ static NTSTATUS store_sid(struct netr_SidAttr *sids,
145154
const uint32_t allocated_resource_groups,
146155
const enum auth_group_inclusion group_inclusion)
147156
{
157+
if (sid->origin == AUTH_SID_ORIGIN_BASE) {
158+
return NT_STATUS_OK;
159+
}
160+
if (sid->origin == AUTH_SID_ORIGIN_EXTRA) {
161+
goto store_in_extra;
162+
}
163+
148164
/* See if it's a resource SID. */
149165
if (sid->attrs & SE_GROUP_RESOURCE) {
150166
/*
@@ -176,7 +192,7 @@ static NTSTATUS store_sid(struct netr_SidAttr *sids,
176192
return NT_STATUS_INVALID_PARAMETER;
177193
}
178194
}
179-
195+
store_in_extra:
180196
/* Just store the SID in Extra SIDs. */
181197
return store_extra_sid(sids,
182198
sidcount,
@@ -734,6 +750,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx,
734750
user_info_dc->sids[PRIMARY_USER_SID_INDEX] = (struct auth_SidAttr) {
735751
.sid = tmpsid,
736752
.attrs = SE_GROUP_DEFAULT_FLAGS,
753+
.origin = AUTH_SID_ORIGIN_BASE,
737754
};
738755

739756
tmpsid = *base->domain_sid;
@@ -750,6 +767,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx,
750767
user_info_dc->sids[PRIMARY_GROUP_SID_INDEX] = (struct auth_SidAttr) {
751768
.sid = tmpsid,
752769
.attrs = SE_GROUP_DEFAULT_FLAGS,
770+
.origin = AUTH_SID_ORIGIN_BASE,
753771
};
754772

755773
user_info_dc->num_sids = PRIMARY_SIDS_COUNT;
@@ -765,6 +783,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx,
765783
bgrps[user_info_dc->num_sids] = (struct auth_SidAttr) {
766784
.sid = tmpsid,
767785
.attrs = base->groups.rids[i].attributes,
786+
.origin = AUTH_SID_ORIGIN_BASE,
768787
};
769788
user_info_dc->num_sids++;
770789
}
@@ -779,6 +798,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx,
779798
dgrps[user_info_dc->num_sids] = (struct auth_SidAttr) {
780799
.sid = *sids[i].sid,
781800
.attrs = sids[i].attributes,
801+
.origin = AUTH_SID_ORIGIN_EXTRA,
782802
};
783803
user_info_dc->num_sids++;
784804
}
@@ -921,6 +941,7 @@ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx,
921941
rgrps[user_info_dc->num_sids] = (struct auth_SidAttr) {
922942
.sid = tmpsid,
923943
.attrs = rg->groups.rids[i].attributes,
944+
.origin = AUTH_SID_ORIGIN_RESOURCE,
924945
};
925946
user_info_dc->num_sids++;
926947
}

librpc/idl/auth.idl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,17 @@ interface auth
116116
AUTH_EXCLUDE_RESOURCE_GROUPS = 4
117117
} auth_group_inclusion;
118118

119+
typedef [nopush,nopull] enum {
120+
AUTH_SID_ORIGIN_UNKNOWN = 0,
121+
AUTH_SID_ORIGIN_BASE = 1,
122+
AUTH_SID_ORIGIN_EXTRA = 2,
123+
AUTH_SID_ORIGIN_RESOURCE = 3
124+
} auth_sid_origin;
125+
119126
typedef [nopush,nopull] struct {
120127
dom_sid sid;
121128
security_GroupAttrs attrs;
129+
auth_sid_origin origin;
122130
} auth_SidAttr;
123131

124132
/* This is the interim product of the auth subsystem, before

selftest/knownfail_heimdal_kdc.d/device-info

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,3 @@
33
#
44
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_add_transitive_domain_local_groups_to_service_compressed.ad_dc
55
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_add_transitive_domain_local_groups_to_service_uncompressed.ad_dc
6-
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_base_sid_resource_attrs_to_krbtgt.ad_dc
7-
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_base_sid_resource_attrs_to_service.ad_dc
8-
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_extra_sids_to_krbtgt.ad_dc
9-
^samba.tests.krb5.device_tests.samba.tests.krb5.device_tests.DeviceTests.test_device_info_extra_sids_to_service.ad_dc

0 commit comments

Comments
 (0)