2 files changed, 132 insertions, 86 deletions

diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
index 14494cbeec..d523055b9d 100644
--- a/source3/passdb/lookup_sid.c
+++ b/source3/passdb/lookup_sid.c
@@ -1514,3 +1514,127 @@ bool sid_to_gid(const struct dom_sid *psid, gid_t *pgid)
 	store_gid_sid_cache(psid, *pgid);
 	return true;
 }
+
+/**
+ * @brief This function gets the primary group SID mapping the primary
+ *        GID of the user as obtained by an actual getpwnam() call.
+ *        This is necessary to avoid issues with arbitrary group SIDs
+ *        stored in passdb. We try as hard as we can to get the SID
+ *        corresponding to the GID, including trying group mapping.
+ *        If nothing else works, we will force "Domain Users" as the
+ *        primary group.
+ *        This is needed because we must always be able to lookup the
+ *        primary group SID, so we cannot settle for an arbitrary SID.
+ *
+ *        This call can be expensive. Use with moderation.
+ *        If you have a "samu" struct around use pdb_get_group_sid()
+ *        instead as it does properly cache results.
+ *
+ * @param mem_ctx[in]     The memory context iused to allocate the result.
+ * @param username[in]    The user's name
+ * @param _pwd[in|out]    If available, pass in user's passwd struct.
+ *                        It will contain a tallocated passwd if NULL was
+ *                        passed in.
+ * @param _group_sid[out] The user's Primary Group SID
+ *
+ * @return NTSTATUS error code.
+ */
+NTSTATUS get_primary_group_sid(TALLOC_CTX *mem_ctx,
+				const char *username,
+				struct passwd **_pwd,
+				struct dom_sid **_group_sid)
+{
+	TALLOC_CTX *tmp_ctx;
+	bool need_lookup_sid = false;
+	struct dom_sid *group_sid;
+	struct passwd *pwd = *_pwd;
+
+	tmp_ctx = talloc_new(mem_ctx);
+	if (!tmp_ctx) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	if (!pwd) {
+		pwd = Get_Pwnam_alloc(mem_ctx, username);
+		if (!pwd) {
+			DEBUG(0, ("Failed to find a Unix account for %s",
+				  username));
+			TALLOC_FREE(tmp_ctx);
+			return NT_STATUS_NO_SUCH_USER;
+		}
+	}
+
+	group_sid = talloc_zero(mem_ctx, struct dom_sid);
+	if (!group_sid) {
+		TALLOC_FREE(tmp_ctx);
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	gid_to_sid(group_sid, pwd->pw_gid);
+	if (!is_null_sid(group_sid)) {
+		struct dom_sid domain_sid;
+		uint32_t rid;
+
+		/* We need a sid within our domain */
+		sid_copy(&domain_sid, group_sid);
+		sid_split_rid(&domain_sid, &rid);
+		if (sid_equal(&domain_sid, get_global_sam_sid())) {
+			/*
+			 * As shortcut for the expensive lookup_sid call
+			 * compare the domain sid part
+			 */
+			switch (rid) {
+			case DOMAIN_RID_ADMINS:
+			case DOMAIN_RID_USERS:
+				goto done;
+			default:
+				need_lookup_sid = true;
+				break;
+			}
+		} else {
+			/* Try group mapping */
+			ZERO_STRUCTP(group_sid);
+			if (pdb_gid_to_sid(pwd->pw_gid, group_sid)) {
+				need_lookup_sid = true;
+			}
+		}
+	}
+
+	/* We must verify that this is a valid SID that resolves to a
+	 * group of the correct type */
+	if (need_lookup_sid) {
+		enum lsa_SidType type = SID_NAME_UNKNOWN;
+		bool lookup_ret;
+
+		DEBUG(10, ("do lookup_sid(%s) for group of user %s\n",
+			   sid_string_dbg(group_sid), username));
+
+		/* Now check that it's actually a domain group and
+		 * not something else */
+		lookup_ret = lookup_sid(tmp_ctx, group_sid,
+					NULL, NULL, &type);
+
+		if (lookup_ret && (type == SID_NAME_DOM_GRP)) {
+			goto done;
+		}
+
+		DEBUG(3, ("Primary group %s for user %s is"
+			  " a %s and not a domain group\n",
+			  sid_string_dbg(group_sid), username,
+			  sid_type_lookup(type)));
+	}
+
+	/* Everything else, failed.
+	 * Just set it to the 'Domain Users' RID of 513 which will
+	   always resolve to a name */
+	DEBUG(3, ("Forcing Primary Group to 'Domain Users' for %s\n",
+		  username));
+
+	sid_compose(group_sid, get_global_sam_sid(), DOMAIN_RID_USERS);
+
+done:
+	*_pwd = talloc_move(mem_ctx, &pwd);
+	*_group_sid = talloc_move(mem_ctx, &group_sid);
+	TALLOC_FREE(tmp_ctx);
+	return NT_STATUS_OK;
+}
diff --git a/source3/passdb/pdb_get_set.c b/source3/passdb/pdb_get_set.c
index 57d95138e3..3e2510e74c 100644
--- a/source3/passdb/pdb_get_set.c
+++ b/source3/passdb/pdb_get_set.c
@@ -182,105 +182,27 @@ const struct dom_sid *pdb_get_user_sid(const struct samu *sampass)
 
 const struct dom_sid *pdb_get_group_sid(struct samu *sampass)
 {
-	struct dom_sid *gsid;
-	struct passwd *pwd;
-	bool need_lookup_sid = false;
+	NTSTATUS status;
 
 	/* Return the cached group SID if we have that */
-	if ( sampass->group_sid ) {
+	if (sampass->group_sid) {
 		return sampass->group_sid;
 	}
 
-	/* generate the group SID from the user's primary Unix group */
-
-	if ( !(gsid  = TALLOC_ZERO_P( sampass, struct dom_sid )) ) {
-		return NULL;
-	}
-
 	/* No algorithmic mapping, meaning that we have to figure out the
 	   primary group SID according to group mapping and the user SID must
 	   be a newly allocated one.  We rely on the user's Unix primary gid.
 	   We have no choice but to fail if we can't find it. */
-
-	if ( sampass->unix_pw ) {
-		pwd = sampass->unix_pw;
-	} else {
-		pwd = Get_Pwnam_alloc( sampass, pdb_get_username(sampass) );
-		sampass->unix_pw = pwd;
-	}
-
-	if ( !pwd ) {
-		DEBUG(0,("pdb_get_group_sid: Failed to find Unix account for %s\n", pdb_get_username(sampass) ));
+	status = get_primary_group_sid(sampass,
+					pdb_get_username(sampass),
+					&sampass->unix_pw,
+					&sampass->group_sid);
+	if (!NT_STATUS_IS_OK(status)) {
 		return NULL;
 	}
 
-	gid_to_sid(gsid, pwd->pw_gid);
-	if (!is_null_sid(gsid)) {
-		struct dom_sid dgsid;
-		uint32_t rid;
-
-		sid_copy(&dgsid, gsid);
-		sid_split_rid(&dgsid, &rid);
-		if (sid_equal(&dgsid, get_global_sam_sid())) {
-			/*
-			 * As shortcut for the expensive lookup_sid call
-			 * compare the domain sid part
-			 */
-			switch (rid) {
-			case DOMAIN_RID_ADMINS:
-			case DOMAIN_RID_USERS:
-				sampass->group_sid = gsid;
-				return sampass->group_sid;
-			default:
-				need_lookup_sid = true;
-				break;
-			}
-		} else {
-			ZERO_STRUCTP(gsid);
-			if (pdb_gid_to_sid(pwd->pw_gid, gsid)) {
-				need_lookup_sid = true;
-			}
-		}
-	}
-
-	if (need_lookup_sid) {
-		enum lsa_SidType type = SID_NAME_UNKNOWN;
-		TALLOC_CTX *mem_ctx;
-		bool lookup_ret;
-		const struct dom_sid *usid = pdb_get_user_sid(sampass);
-
-		mem_ctx = talloc_init("pdb_get_group_sid");
-		if (!mem_ctx) {
-			return NULL;
-		}
-
-		DEBUG(10,("do lookup_sid(%s) for group of user %s\n",
-			  sid_string_dbg(gsid), sid_string_dbg(usid)));
-
-		/* Now check that it's actually a domain group and not something else */
-
-		lookup_ret = lookup_sid(mem_ctx, gsid, NULL, NULL, &type);
-
-		TALLOC_FREE( mem_ctx );
-
-		if ( lookup_ret && (type == SID_NAME_DOM_GRP) ) {
-			sampass->group_sid = gsid;
-			return sampass->group_sid;
-		}
-
-		DEBUG(3, ("Primary group %s for user %s is a %s and not a domain group\n",
-			sid_string_dbg(gsid), pwd->pw_name, sid_type_lookup(type)));
-	}
-
-	/* Just set it to the 'Domain Users' RID of 513 which will
-	   always resolve to a name */
-
-	sid_compose(gsid, get_global_sam_sid(), DOMAIN_RID_USERS);
-
-	sampass->group_sid = gsid;
-
 	return sampass->group_sid;
-}	
+}
 
 /**
  * Get flags showing what is initalised in the struct samu