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 <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c
index 93a8c6e9bb05e7d6fda013e7eaf483a8b138d270..fd94bdbc5057f3baedbf7ab091c3fb3e2b42a9b0 100644 (file)
--- 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; i<user_info_dc->num_sids; i++) {
+               for (i=REMAINING_SIDS_INDEX; i<user_info_dc->num_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 a264a2c59b60fb513244f4c15e9ae47be09af04c..9f1a81e883eb12a76b08046915091d49a288eb66 100644 (file)
--- 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 75918b440a3d440ef77a92e57c367d1e658ac3af..c51f6f44bc33c58efd46b71ccd5ced77d0252733 100644 (file)
--- 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 5d7b5c86ad459d5889a2d3fd4bf42d2571825570..ebdff19e67bb73cecb14e49f31abb63c1a8475a2 100644 (file)
--- 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,
                "<SID=%s>",
                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 b6de6a140e34434dd2dcc54739c5ed6704dd7d89..ec6885a86e3b9294f8a0640aec364d8c38a0ee10 100644 (file)
--- 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);