From: Joseph Sutton Date: Mon, 12 Dec 2022 20:04:47 +0000 (+1300) Subject: auth: Correct primary group handling X-Git-Tag: talloc-2.4.1~1651 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c3a8fa20c79dfbc944b941d47586894d32fcedb;p=thirdparty%2Fsamba.git auth: Correct primary group handling Heretofore we have treated the primary group SID specially, storing it in a fixed position as the second element of the user_info_dc->sids array, and filtering out other copies in the PAC_LOGON_INFO base structure. This filtering has made it difficult to distinguish between the case where the primary group is a universal or global group, located in the base RIDs, and the case where it is a domain-local group, missing from the base RIDs; especially since the attributes of a domain-local primary group are lost by being stored in the PAC. Domain-local primary groups are normally disallowed by Windows, but are allowed by Samba, and so it is reasonable to support them with at least some measure of consistency. The second element of user_info_dc->sids is still reserved for the primary group's SID, but we no longer filter out any other copies in the array. The first two elements are no more than the SIDs of the user and the primary group respectively; and the remaining SIDs are as if taken without modification from arrays of SIDs in the PAC. user_info_dc->sids should therefore become a more faithful representation of the SIDs in the PAC. After adding resource SIDs to it with dsdb_expand_resource_groups(), we should have a result that more closely and in more cases matches that of Windows. Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c index 93a8c6e9bb0..fd94bdbc505 100644 --- a/auth/auth_sam_reply.c +++ b/auth/auth_sam_reply.c @@ -248,7 +248,7 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, sam->groups.count = 0; sam->groups.rids = NULL; - if (user_info_dc->num_sids > PRIMARY_GROUP_SID_INDEX) { + if (user_info_dc->num_sids > REMAINING_SIDS_INDEX) { size_t i; sam->groups.rids = talloc_array(mem_ctx, struct samr_RidWithAttribute, user_info_dc->num_sids); @@ -256,7 +256,7 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, if (sam->groups.rids == NULL) return NT_STATUS_NO_MEMORY; - for (i=PRIMARY_GROUP_SID_INDEX; inum_sids; i++) { + for (i=REMAINING_SIDS_INDEX; inum_sids; i++) { struct auth_SidAttr *group_sid = &user_info_dc->sids[i]; bool belongs_in_base = is_base_sid(group_sid, sam->domain_sid); @@ -692,10 +692,6 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx, user_info_dc->sids[PRIMARY_GROUP_SID_INDEX].attrs = SE_GROUP_DEFAULT_FLAGS; for (i = 0; i < base->groups.count; i++) { - /* Skip primary group, already added above */ - if (base->groups.rids[i].rid == base->primary_gid) { - continue; - } user_info_dc->sids[user_info_dc->num_sids].sid = *base->domain_sid; if (!sid_append_rid(&user_info_dc->sids[user_info_dc->num_sids].sid, base->groups.rids[i].rid)) { return NT_STATUS_INVALID_PARAMETER; diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index a264a2c59b6..9f1a81e883e 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -145,10 +145,4 @@ # # Group tests # -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_as_req_to_krbtgt.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_compression_as_req_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_compression_tgs_req_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_no_compression_as_req_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_no_compression_tgs_req_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_primary_domain_local_tgs_req_to_krbtgt.ad_dc ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_set_domain_local_primary_group.ad_dc diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c index 75918b440a3..c51f6f44bc3 100644 --- a/source3/lib/util_sid.c +++ b/source3/lib/util_sid.c @@ -129,9 +129,6 @@ NTSTATUS sid_array_from_info3(TALLOC_CTX *mem_ctx, for (i = 0; i < info3->base.groups.count; i++) { /* Don't add the primary group sid twice. */ - if (info3->base.primary_gid == info3->base.groups.rids[i].rid) { - continue; - } if (!sid_compose(&sid, info3->base.domain_sid, info3->base.groups.rids[i].rid)) { DEBUG(3, ("could not compose SID from additional group " diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 5d7b5c86ad4..ebdff19e67b 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -343,6 +343,7 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, struct auth_user_info_dc **_user_info_dc) { NTSTATUS status; + int ret; struct auth_user_info_dc *user_info_dc; struct auth_user_info *info; const char *str = NULL; @@ -350,8 +351,11 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, /* SIDs for the account and his primary group */ struct dom_sid *account_sid; struct dom_sid_buf buf; - const char *primary_group_dn; + const char *primary_group_dn_str = NULL; DATA_BLOB primary_group_blob; + struct ldb_dn *primary_group_dn = NULL; + struct ldb_message *primary_group_msg = NULL; + unsigned primary_group_type; /* SID structures for the expanded group memberships */ struct auth_SidAttr *sids = NULL; uint32_t num_sids = 0; @@ -359,6 +363,7 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, struct dom_sid *domain_sid; TALLOC_CTX *tmp_ctx; struct ldb_message_element *el; + static const char * const group_type_attrs[] = { "groupType", NULL }; user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); @@ -369,7 +374,12 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - sids = talloc_array(user_info_dc, struct auth_SidAttr, 2); + /* + * We'll typically store three SIDs: the SID of the user, the SID of the + * primary group, and a copy of the latter if it's not a resource + * group. Allocate enough memory for these three SIDs. + */ + sids = talloc_zero_array(user_info_dc, struct auth_SidAttr, 3); if (sids == NULL) { TALLOC_FREE(user_info_dc); return NT_STATUS_NO_MEMORY; @@ -406,16 +416,58 @@ _PUBLIC_ NTSTATUS authsam_make_user_info_dc(TALLOC_CTX *mem_ctx, return status; } - primary_group_dn = talloc_asprintf( + primary_group_dn_str = talloc_asprintf( tmp_ctx, "", dom_sid_str_buf(&sids[PRIMARY_GROUP_SID_INDEX].sid, &buf)); + if (primary_group_dn_str == NULL) { + TALLOC_FREE(user_info_dc); + return NT_STATUS_NO_MEMORY; + } + + /* Get the DN of the primary group. */ + primary_group_dn = ldb_dn_new(tmp_ctx, sam_ctx, primary_group_dn_str); if (primary_group_dn == NULL) { TALLOC_FREE(user_info_dc); return NT_STATUS_NO_MEMORY; } - primary_group_blob = data_blob_string_const(primary_group_dn); + /* + * Do a search for the primary group, for the purpose of checking + * whether it's a resource group. + */ + ret = dsdb_search_one(sam_ctx, tmp_ctx, + &primary_group_msg, + primary_group_dn, + LDB_SCOPE_BASE, + group_type_attrs, + 0, + NULL); + if (ret != LDB_SUCCESS) { + talloc_free(user_info_dc); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + + /* Check the type of the primary group. */ + primary_group_type = ldb_msg_find_attr_as_uint(primary_group_msg, "groupType", 0); + if (primary_group_type & GROUP_TYPE_RESOURCE_GROUP) { + /* + * If it's a resource group, we might as well indicate that in + * its attributes. At any rate, the primary group's attributes + * are unlikely to be used in the code, as there's nowhere to + * store them. + */ + sids[PRIMARY_GROUP_SID_INDEX].attrs |= SE_GROUP_RESOURCE; + } else { + /* + * The primary group is not a resource group. Make a copy of its + * SID to ensure it is added to the Base SIDs in the PAC. + */ + sids[REMAINING_SIDS_INDEX] = sids[PRIMARY_GROUP_SID_INDEX]; + ++num_sids; + } + + primary_group_blob = data_blob_string_const(primary_group_dn_str); /* Expands the primary group - this function takes in * memberOf-like values, so we fake one up with the diff --git a/source4/auth/system_session.c b/source4/auth/system_session.c index b6de6a140e3..ec6885a86e3 100644 --- a/source4/auth/system_session.c +++ b/source4/auth/system_session.c @@ -201,7 +201,7 @@ static NTSTATUS auth_domain_admin_user_info_dc(TALLOC_CTX *mem_ctx, user_info_dc = talloc_zero(mem_ctx, struct auth_user_info_dc); NT_STATUS_HAVE_NO_MEMORY(user_info_dc); - user_info_dc->num_sids = 7; + user_info_dc->num_sids = 8; user_info_dc->sids = talloc_array(user_info_dc, struct auth_SidAttr, user_info_dc->num_sids); user_info_dc->sids[PRIMARY_USER_SID_INDEX].sid = *domain_sid; @@ -212,21 +212,24 @@ static NTSTATUS auth_domain_admin_user_info_dc(TALLOC_CTX *mem_ctx, sid_append_rid(&user_info_dc->sids[PRIMARY_GROUP_SID_INDEX].sid, DOMAIN_RID_USERS); user_info_dc->sids[PRIMARY_GROUP_SID_INDEX].attrs = SE_GROUP_DEFAULT_FLAGS; - user_info_dc->sids[2].sid = global_sid_Builtin_Administrators; - user_info_dc->sids[2].attrs = SE_GROUP_DEFAULT_FLAGS; + /* Add the primary group again. */ + user_info_dc->sids[2] = user_info_dc->sids[PRIMARY_GROUP_SID_INDEX]; - user_info_dc->sids[3].sid = *domain_sid; - sid_append_rid(&user_info_dc->sids[3].sid, DOMAIN_RID_ADMINS); + user_info_dc->sids[3].sid = global_sid_Builtin_Administrators; user_info_dc->sids[3].attrs = SE_GROUP_DEFAULT_FLAGS; + user_info_dc->sids[4].sid = *domain_sid; - sid_append_rid(&user_info_dc->sids[4].sid, DOMAIN_RID_ENTERPRISE_ADMINS); + sid_append_rid(&user_info_dc->sids[4].sid, DOMAIN_RID_ADMINS); user_info_dc->sids[4].attrs = SE_GROUP_DEFAULT_FLAGS; user_info_dc->sids[5].sid = *domain_sid; - sid_append_rid(&user_info_dc->sids[5].sid, DOMAIN_RID_POLICY_ADMINS); + sid_append_rid(&user_info_dc->sids[5].sid, DOMAIN_RID_ENTERPRISE_ADMINS); user_info_dc->sids[5].attrs = SE_GROUP_DEFAULT_FLAGS; user_info_dc->sids[6].sid = *domain_sid; - sid_append_rid(&user_info_dc->sids[6].sid, DOMAIN_RID_SCHEMA_ADMINS); + sid_append_rid(&user_info_dc->sids[6].sid, DOMAIN_RID_POLICY_ADMINS); user_info_dc->sids[6].attrs = SE_GROUP_DEFAULT_FLAGS; + user_info_dc->sids[7].sid = *domain_sid; + sid_append_rid(&user_info_dc->sids[7].sid, DOMAIN_RID_SCHEMA_ADMINS); + user_info_dc->sids[7].attrs = SE_GROUP_DEFAULT_FLAGS; /* What should the session key be?*/ user_info_dc->user_session_key = data_blob_talloc(user_info_dc, NULL, 16);