This patch fixes one of my longest-standing pet hates with Samba :-).

When we look see if a user is in a list, and we try to 'expand' an @group, we
should lookup the user's own list of groups, rather than looking for all the
members of a group.

I'm sure this will fix some nasty performance issues, particularly on large
domains etc.  In particular, this avoids contacting winbind at all, if the
group is not a winbind group.

(This caused a deadlock on my winbind-on-PDC setup).

The groups list always includes the user's primary group, as per the
getgrouplist manpage, and my recent changes to our implementation.

Andrew Bartlett
(This used to be commit 9be21976f7662ebe6eb92fff7cecbdb352eca334)

4 files changed, 21 insertions, 21 deletions

diff --git a/source3/smbd/password.c b/source3/smbd/password.c
index 5274028db4a..784c1525c89 100644
--- a/source3/smbd/password.c
+++ b/source3/smbd/password.c
@@ -267,7 +267,7 @@ void add_session_user(const char *user)
 /****************************************************************************
 check if a username is valid
 ****************************************************************************/
-BOOL user_ok(const char *user,int snum)
+BOOL user_ok(const char *user,int snum, gid_t *groups, size_t n_groups)
 {
 	char **valid, **invalid;
 	BOOL ret;
@@ -278,7 +278,7 @@ BOOL user_ok(const char *user,int snum)
 	if (lp_invalid_users(snum)) {
 		str_list_copy(&invalid, lp_invalid_users(snum));
 		if (invalid && str_list_substitute(invalid, "%S", lp_servicename(snum))) {
-			ret = !user_in_list(user, (const char **)invalid);
+			ret = !user_in_list(user, (const char **)invalid, groups, n_groups);
 		}
 	}
 	if (invalid)
@@ -287,7 +287,7 @@ BOOL user_ok(const char *user,int snum)
 	if (ret && lp_valid_users(snum)) {
 		str_list_copy(&valid, lp_valid_users(snum));
 		if (valid && str_list_substitute(valid, "%S", lp_servicename(snum))) {
-			ret = user_in_list(user, (const char **)valid);
+			ret = user_in_list(user, (const char **)valid, groups, n_groups);
 		}
 	}
 	if (valid)
@@ -296,7 +296,7 @@ BOOL user_ok(const char *user,int snum)
 	if (ret && lp_onlyuser(snum)) {
 		char **user_list = str_list_make (lp_username(snum), NULL);
 		if (user_list && str_list_substitute(user_list, "%S", lp_servicename(snum))) {
-			ret = user_in_list(user, (const char **)user_list);
+			ret = user_in_list(user, (const char **)user_list, groups, n_groups);
 		}
 		if (user_list) str_list_free (&user_list);
 	}
@@ -315,7 +315,7 @@ static char *validate_group(char *group, DATA_BLOB password,int snum)
 		setnetgrent(group);
 		while (getnetgrent(&host, &user, &domain)) {
 			if (user) {
-				if (user_ok(user, snum) && 
+				if (user_ok(user, snum, NULL, 0) && 
 				    password_ok(user,password)) {
 					endnetgrent();
 					return(user);
@@ -370,7 +370,7 @@ static char *validate_group(char *group, DATA_BLOB password,int snum)
 			while (*member) {
 				static fstring name;
 				fstrcpy(name,member);
-				if (user_ok(name,snum) &&
+				if (user_ok(name,snum, NULL, 0) &&
 				    password_ok(name,password)) {
 					endgrent();
 					return(&name[0]);
@@ -429,7 +429,7 @@ BOOL authorise_login(int snum, fstring user, DATA_BLOB password,
 		     auser = strtok(NULL,LIST_SEP)) {
 			fstring user2;
 			fstrcpy(user2,auser);
-			if (!user_ok(user2,snum))
+			if (!user_ok(user2,snum, NULL, 0))
 				continue;
 			
 			if (password_ok(user2,password)) {
@@ -464,7 +464,7 @@ and given password ok (%s)\n", user));
 			} else {
 				fstring user2;
 				fstrcpy(user2,auser);
-				if (user_ok(user2,snum) && password_ok(user2,password)) {
+				if (user_ok(user2,snum, NULL, 0) && password_ok(user2,password)) {
 					ok = True;
 					fstrcpy(user,user2);
 					DEBUG(3,("authorise_login: ACCEPTED: user list username \
@@ -489,7 +489,7 @@ and given password ok (%s)\n", user));
 		*guest = True;
 	}
 
-	if (ok && !user_ok(user,snum)) {
+	if (ok && !user_ok(user, snum, NULL, 0)) {
 		DEBUG(0,("authorise_login: rejected invalid user %s\n",user));
 		ok = False;
 	}
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 5069db8097b..2739f73b0ac 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -573,7 +573,7 @@ static BOOL uid_entry_in_group( canon_ace *uid_ace, canon_ace *group_ace )
 	 * not uids/gids.
 	 */
 
-	return user_in_group_list(u_name, g_name );
+	return user_in_group_list(u_name, g_name, NULL, 0);
 }
 
 /****************************************************************************
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 2a41a6db1c5..f9d84872d71 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -258,7 +258,7 @@ static NTSTATUS share_sanity_checks(int snum, pstring dev)
 /****************************************************************************
  readonly share?
 ****************************************************************************/
-static void set_read_only(connection_struct *conn) 
+static void set_read_only(connection_struct *conn, gid_t *groups, size_t n_groups)
 {
 	char **list;
 	char *service = lp_servicename(conn->service);
@@ -271,7 +271,7 @@ static void set_read_only(connection_struct *conn)
 		if (!str_list_substitute(list, "%S", service)) {
 			DEBUG(0, ("ERROR: read list substitution failed\n"));
 		}
-		if (user_in_list(conn->user, (const char **)list))
+		if (user_in_list(conn->user, (const char **)list, groups, n_groups))
 			conn->read_only = True;
 		str_list_free(&list);
 	}
@@ -281,7 +281,7 @@ static void set_read_only(connection_struct *conn)
 		if (!str_list_substitute(list, "%S", service)) {
 			DEBUG(0, ("ERROR: write list substitution failed\n"));
 		}
-		if (user_in_list(conn->user, (const char **)list))
+		if (user_in_list(conn->user, (const char **)list, groups, n_groups))
 			conn->read_only = False;
 		str_list_free(&list);
 	}
@@ -291,7 +291,7 @@ static void set_read_only(connection_struct *conn)
 /****************************************************************************
   admin user check
 ****************************************************************************/
-static void set_admin_user(connection_struct *conn) 
+static void set_admin_user(connection_struct *conn, gid_t *groups, size_t n_groups)
 {
 	/* admin user check */
 	
@@ -299,7 +299,7 @@ static void set_admin_user(connection_struct *conn)
 	   marked read_only. Changed as I don't think this is needed,
 	   but old code left in case there is a problem here.
 	*/
-	if (user_in_list(conn->user,lp_admin_users(conn->service)) 
+	if (user_in_list(conn->user,lp_admin_users(conn->service), groups, n_groups) 
 #if 0
 	    && !conn->read_only
 #endif
@@ -370,7 +370,7 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 				      return NULL;
 			}
 		} else {
-			if (!user_ok(vuser->user.unix_name, snum)) {
+			if (!user_ok(vuser->user.unix_name, snum, vuser->groups, vuser->n_groups)) {
 				DEBUG(2, ("user '%s' (from session setup) not permitted to access this share (%s)", vuser->user.unix_name, lp_servicename(snum)));
 				conn_free(conn);
 				*status = NT_STATUS_ACCESS_DENIED;
@@ -427,9 +427,9 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 	string_set(&conn->user,user);
 	conn->nt_user_token = NULL;
 	
-	set_read_only(conn);
+	set_read_only(conn, vuser ? vuser->groups : NULL, vuser ? vuser->n_groups : 0);
 	
-	set_admin_user(conn);
+	set_admin_user(conn, vuser ? vuser->groups : NULL, vuser ? vuser->n_groups : 0);
 
 	/*
 	 * If force user is true, then store the
@@ -499,7 +499,7 @@ static connection_struct *make_connection_snum(int snum, user_struct *vuser,
 			 * Otherwise, the meaning of the '+' would be ignored.
 			 */
 			if (conn->force_user && user_must_be_member) {
-				if (user_in_group_list( user, gname )) {
+				if (user_in_group_list( user, gname, NULL, 0)) {
 						conn->gid = gid;
 						DEBUG(3,("Forced group %s for member %s\n",gname,user));
 				}
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index e7c00ba4560..4ebee75a159 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -60,7 +60,7 @@ BOOL change_to_guest(void)
 
 static BOOL check_user_ok(connection_struct *conn, user_struct *vuser,int snum)
 {
-	int i;
+	unsigned i;
 	for (i=0;i<conn->vuid_cache.entries && i< VUID_CACHE_SIZE;i++)
 		if (conn->vuid_cache.list[i] == vuser->vuid)
 			return(True);
@@ -70,7 +70,7 @@ static BOOL check_user_ok(connection_struct *conn, user_struct *vuser,int snum)
 		return False;
 	}
 	
-	if (!user_ok(vuser->user.unix_name,snum))
+	if (!user_ok(vuser->user.unix_name,snum, vuser->groups, vuser->n_groups))
 		return(False);
 
 	if (!share_access_check(conn, snum, vuser, conn->read_only ? FILE_READ_DATA : FILE_WRITE_DATA)) {