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>

1 files changed, 2 insertions, 6 deletions

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; 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;