svn commit: samba r13441 - in trunk/source: auth lib smbd

[vlendec at samba.org]

Author: vlendec
Date: 2006-02-10 23:00:35 +0000 (Fri, 10 Feb 2006)
New Revision: 13441

WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=13441

Log:
For the well-known reasons user_in_group is broken (winbind can't always
reliably tell). Replace two uses by an appropriate check going via
create_token_from_username. 

Sounds expensive and probably is, but user_in_group is potentially much more
expensive as it lists all group members and checks for membership. Potentially
even much more expensive.

The change in auth_sam is for the "+" in the list of allowed
workstations. This only makes sense on for workstations defined locally
anyway, thus unix_in_group->unix_in_user_group.

Volker

Modified:
   trunk/source/auth/auth_sam.c
   trunk/source/auth/auth_util.c
   trunk/source/lib/username.c
   trunk/source/smbd/posix_acls.c
   trunk/source/smbd/service.c


Changeset:
Modified: trunk/source/auth/auth_sam.c
===================================================================
--- trunk/source/auth/auth_sam.c	2006-02-10 21:16:30 UTC (rev 13440)
+++ trunk/source/auth/auth_sam.c	2006-02-10 23:00:35 UTC (rev 13441)
@@ -192,7 +192,7 @@
 			if (tok[0] == '+') {
 				DEBUG(10,("sam_account_ok: checking for workstation %s in group: %s\n", 
 					machine_name, tok + 1));
-				if (user_in_group(machine_name, tok + 1)) {
+				if (user_in_unix_group(machine_name, tok + 1)) {
 					invalid_ws = False;
 					break;
 				}

Modified: trunk/source/auth/auth_util.c
===================================================================
--- trunk/source/auth/auth_util.c	2006-02-10 21:16:30 UTC (rev 13440)
+++ trunk/source/auth/auth_util.c	2006-02-10 23:00:35 UTC (rev 13441)
@@ -1021,6 +1021,46 @@
 }
 
 /***************************************************************************
+ Build upon create_token_from_username:
+
+ Expensive helper function to figure out whether a user given its name is
+ member of a particular group.
+
+ (Justification: Before this function existed, the callers of this function
+  called user_in_group() which was potentially even more expensive as
+  it lists all group members which can be *huge* -- vl )
+
+***************************************************************************/
+BOOL username_in_group(const char *username, const DOM_SID *group_sid)
+{
+	NTSTATUS status;
+	uid_t uid;
+	gid_t gid;
+	char *found_username;
+	struct nt_user_token *token;
+	BOOL result;
+
+	TALLOC_CTX *mem_ctx;
+
+	mem_ctx = talloc_new(NULL);
+	if (mem_ctx == NULL) {
+		DEBUG(0, ("talloc_new failed\n"));
+		return False;
+	}
+
+	status = create_token_from_username(mem_ctx, username, False,
+					    &uid, &gid, &found_username,
+					    &token);
+
+	result = nt_token_check_sid(group_sid, token);
+
+	talloc_free(mem_ctx);
+	return result;
+	
+}
+
+
+/***************************************************************************
  Make (and fill) a user_info struct from a Kerberos PAC logon_info by
  conversion to a SAM_ACCOUNT
 ***************************************************************************/

Modified: trunk/source/lib/username.c
===================================================================
--- trunk/source/lib/username.c	2006-02-10 21:16:30 UTC (rev 13440)
+++ trunk/source/lib/username.c	2006-02-10 23:00:35 UTC (rev 13441)
@@ -529,7 +529,7 @@
  Check if a user is in a group list. Ask winbind first, then use UNIX.
 ****************************************************************************/
 
-BOOL user_in_group(const char *user, const char *gname)
+static BOOL user_in_group(const char *user, const char *gname)
 {
 	BOOL winbind_answered = False;
 	BOOL ret;

Modified: trunk/source/smbd/posix_acls.c
===================================================================
--- trunk/source/smbd/posix_acls.c	2006-02-10 21:16:30 UTC (rev 13440)
+++ trunk/source/smbd/posix_acls.c	2006-02-10 23:00:35 UTC (rev 13441)
@@ -1015,7 +1015,6 @@
 static BOOL uid_entry_in_group( canon_ace *uid_ace, canon_ace *group_ace )
 {
 	fstring u_name;
-	fstring g_name;
 
 	/* "Everyone" always matches every uid. */
 
@@ -1028,14 +1027,7 @@
 		return True;
 
 	fstrcpy(u_name, uidtoname(uid_ace->unix_ug.uid));
-	fstrcpy(g_name, gidtoname(group_ace->unix_ug.gid));
-
-	/*
-	 * Due to the winbind interfaces we need to do this via names,
-	 * not uids/gids.
-	 */
-
-	return user_in_group(u_name, g_name);
+	return username_in_group(u_name, &group_ace->trustee);
 }
 
 /****************************************************************************

Modified: trunk/source/smbd/service.c
===================================================================
--- trunk/source/smbd/service.c	2006-02-10 21:16:30 UTC (rev 13440)
+++ trunk/source/smbd/service.c	2006-02-10 23:00:35 UTC (rev 13441)
@@ -473,7 +473,7 @@
 	 */
 
 	if (force_user && user_must_be_member) {
-		if (user_in_group(username, groupname)) {
+		if (username_in_group(username, &group_sid)) {
 			sid_copy(pgroup_sid, &group_sid);
 			*pgid = gid;
 			DEBUG(3,("Forced group %s for member %s\n",