* fix RemoveSidForeignDomain() ; bug 252

* don't fall back to unmapped UNIX group for
  get_local_group_from_sid()
* remove an extra become/unbecome_root() pair
  from group enumeration

6 files changed, 121 insertions, 84 deletions

diff --git a/source/groupdb/mapping.c b/source/groupdb/mapping.c
index 8f534d779ef..b1c260581ee 100644
--- a/source/groupdb/mapping.c
+++ b/source/groupdb/mapping.c
@@ -547,27 +547,28 @@ BOOL get_domain_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 
 BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 {
-	struct group *grp;
-
 	if(!init_group_mapping()) {
 		DEBUG(0,("failed to initialize group mapping"));
 		return(False);
 	}
 
 	/* The group is in the mapping table */
-	if(pdb_getgrsid(map, sid)) {
-		if (map->sid_name_use!=SID_NAME_ALIAS) {
-			return False;
- 		}
+	
+	if( !pdb_getgrsid(map, sid) ) 
+		return False;
 		
-		if (map->gid==-1) {
-			return False;
-		}
-
-		if ( (grp=getgrgid(map->gid)) == NULL) {
-			return False;
-		}
-	} else {
+	if ( (map->sid_name_use != SID_NAME_ALIAS)
+		|| (map->gid == -1)
+		|| (getgrgid(map->gid) == NULL) ) 
+	{
+		return False;
+	} 		
+			
+#if 0 	/* JERRY */
+	/* local groups only exist in the group mapping DB so this 
+	   is not necessary */
+	   
+	else {
 		/* the group isn't in the mapping table.
 		 * make one based on the unix information */
 		uint32 alias_rid;
@@ -588,6 +589,7 @@ BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 
 		sid_copy(&map->sid, &sid);
 	}
+#endif
 
 	return True;
 }
diff --git a/source/include/rpc_samr.h b/source/include/rpc_samr.h
index 3b81042df37..787535d0e90 100644
--- a/source/include/rpc_samr.h
+++ b/source/include/rpc_samr.h
@@ -127,7 +127,7 @@ SamrTestPrivateFunctionsUser
 #define SAMR_UNKNOWN_2a        0x2a
 #define SAMR_UNKNOWN_2b        0x2b
 #define SAMR_GET_USRDOM_PWINFO 0x2c
-#define SAMR_REMOVE_USER_FOREIGN_DOMAIN        0x2d
+#define SAMR_REMOVE_SID_FOREIGN_DOMAIN        0x2d
 #define SAMR_UNKNOWN_2E        0x2e /* looks like an alias for SAMR_QUERY_DOMAIN_INFO */
 #define SAMR_UNKNOWN_2f        0x2f
 #define SAMR_QUERY_DISPINFO3   0x30 /* Alias for SAMR_QUERY_DISPINFO
@@ -1790,21 +1790,21 @@ typedef struct r_samr_chgpasswd_user_info
 } SAMR_R_CHGPASSWD_USER;
 
 
-/* SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN */
-typedef struct q_samr_remove_user_foreign_domain_info
+/* SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN */
+typedef struct q_samr_remove_sid_foreign_domain_info
 {
 	POLICY_HND dom_pol;   /* policy handle */
 	DOM_SID2 sid;         /* SID */
 
-} SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN;
+} SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN;
 
 
-/* SAMR_R_REMOVE_USER_FOREIGN_DOMAIN */
-typedef struct r_samr_remove_user_foreign_domain_info
+/* SAMR_R_REMOVE_SID_FOREIGN_DOMAIN */
+typedef struct r_samr_remove_sid_foreign_domain_info
 {
 	NTSTATUS status;         /* return status */
 
-} SAMR_R_REMOVE_USER_FOREIGN_DOMAIN;
+} SAMR_R_REMOVE_SID_FOREIGN_DOMAIN;
 
 
 
diff --git a/source/rpc_parse/parse_samr.c b/source/rpc_parse/parse_samr.c
index 5e3502b2422..607c9ecf640 100644
--- a/source/rpc_parse/parse_samr.c
+++ b/source/rpc_parse/parse_samr.c
@@ -181,9 +181,9 @@ BOOL samr_io_r_lookup_domain(const char *desc, SAMR_R_LOOKUP_DOMAIN * r_u,
 reads or writes a structure.
 ********************************************************************/
 
-void init_samr_q_remove_user_foreign_domain(SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN * q_u, POLICY_HND *dom_pol, DOM_SID *sid)
+void init_samr_q_remove_sid_foreign_domain(SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN * q_u, POLICY_HND *dom_pol, DOM_SID *sid)
 {
-	DEBUG(5, ("samr_init_samr_q_remove_user_foreign_domain\n"));
+	DEBUG(5, ("samr_init_samr_q_remove_sid_foreign_domain\n"));
 
 	q_u->dom_pol = *dom_pol;
 	init_dom_sid2(&q_u->sid, sid);
@@ -193,13 +193,13 @@ void init_samr_q_remove_user_foreign_domain(SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN *
 reads or writes a structure.
 ********************************************************************/
 
-BOOL samr_io_q_remove_user_foreign_domain(const char *desc, SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN * q_u,
+BOOL samr_io_q_remove_sid_foreign_domain(const char *desc, SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN * q_u,
 			  prs_struct *ps, int depth)
 {
 	if (q_u == NULL)
 		return False;
 
-	prs_debug(ps, depth, desc, "samr_io_q_remove_user_foreign_domain");
+	prs_debug(ps, depth, desc, "samr_io_q_remove_sid_foreign_domain");
 	depth++;
 
 	if(!prs_align(ps))
@@ -221,13 +221,13 @@ BOOL samr_io_q_remove_user_foreign_domain(const char *desc, SAMR_Q_REMOVE_USER_F
 reads or writes a structure.
 ********************************************************************/
 
-BOOL samr_io_r_remove_user_foreign_domain(const char *desc, SAMR_R_REMOVE_USER_FOREIGN_DOMAIN * r_u,
+BOOL samr_io_r_remove_sid_foreign_domain(const char *desc, SAMR_R_REMOVE_SID_FOREIGN_DOMAIN * r_u,
 			  prs_struct *ps, int depth)
 {
 	if (r_u == NULL)
 		return False;
 
-	prs_debug(ps, depth, desc, "samr_io_r_remove_user_foreign_domain");
+	prs_debug(ps, depth, desc, "samr_io_r_remove_sid_foreign_domain");
 	depth++;
 
 	if(!prs_align(ps))
diff --git a/source/rpc_server/srv_samr.c b/source/rpc_server/srv_samr.c
index a0f62c20fca..971f5ed40ce 100644
--- a/source/rpc_server/srv_samr.c
+++ b/source/rpc_server/srv_samr.c
@@ -1343,13 +1343,13 @@ static BOOL api_samr_open_group(pipes_struct *p)
 }
 
 /*******************************************************************
- api_samr_remove_user_foreign_domain
+ api_samr_remove_sid_foreign_domain
  ********************************************************************/
 
-static BOOL api_samr_remove_user_foreign_domain(pipes_struct *p)
+static BOOL api_samr_remove_sid_foreign_domain(pipes_struct *p)
 {
-	SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN q_u;
-	SAMR_R_REMOVE_USER_FOREIGN_DOMAIN r_u;
+	SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN q_u;
+	SAMR_R_REMOVE_SID_FOREIGN_DOMAIN r_u;
 
 	prs_struct *data = &p->in_data.data;
 	prs_struct *rdata = &p->out_data.rdata;
@@ -1357,15 +1357,15 @@ static BOOL api_samr_remove_user_foreign_domain(pipes_struct *p)
 	ZERO_STRUCT(q_u);
 	ZERO_STRUCT(r_u);
 
-	if (!samr_io_q_remove_user_foreign_domain("", &q_u, data, 0)) {
-		DEBUG(0,("api_samr_remove_user_foreign_domain: unable to unmarshall SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN.\n"));
+	if (!samr_io_q_remove_sid_foreign_domain("", &q_u, data, 0)) {
+		DEBUG(0,("api_samr_remove_sid_foreign_domain: unable to unmarshall SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN.\n"));
 		return False;
 	}
 
-	r_u.status = _samr_remove_user_foreign_domain(p, &q_u, &r_u);
+	r_u.status = _samr_remove_sid_foreign_domain(p, &q_u, &r_u);
 
-	if (!samr_io_r_remove_user_foreign_domain("", &r_u, rdata, 0)) {
-		DEBUG(0,("api_samr_remove_user_foreign_domain: unable to marshall SAMR_R_REMOVE_USER_FOREIGN_DOMAIN.\n"));
+	if (!samr_io_r_remove_sid_foreign_domain("", &r_u, rdata, 0)) {
+		DEBUG(0,("api_samr_remove_sid_foreign_domain: unable to marshall SAMR_R_REMOVE_SID_FOREIGN_DOMAIN.\n"));
 		return False;
 	}
 
@@ -1483,7 +1483,7 @@ static struct api_struct api_samr_cmds [] =
       {"SAMR_OPEN_ALIAS"        , SAMR_OPEN_ALIAS       , api_samr_open_alias       },
       {"SAMR_OPEN_GROUP"        , SAMR_OPEN_GROUP       , api_samr_open_group       },
       {"SAMR_OPEN_DOMAIN"       , SAMR_OPEN_DOMAIN      , api_samr_open_domain      },
-      {"SAMR_REMOVE_USER_FOREIGN_DOMAIN"       , SAMR_REMOVE_USER_FOREIGN_DOMAIN      , api_samr_remove_user_foreign_domain      },
+      {"SAMR_REMOVE_SID_FOREIGN_DOMAIN"       , SAMR_REMOVE_SID_FOREIGN_DOMAIN      , api_samr_remove_sid_foreign_domain      },
       {"SAMR_LOOKUP_DOMAIN"     , SAMR_LOOKUP_DOMAIN    , api_samr_lookup_domain    },
       
       {"SAMR_QUERY_SEC_OBJECT"  , SAMR_QUERY_SEC_OBJECT , api_samr_query_sec_obj    },
diff --git a/source/rpc_server/srv_samr_nt.c b/source/rpc_server/srv_samr_nt.c
index d3da830991f..7be9b41ee90 100644
--- a/source/rpc_server/srv_samr_nt.c
+++ b/source/rpc_server/srv_samr_nt.c
@@ -1384,8 +1384,6 @@ NTSTATUS _samr_lookup_names(pipes_struct *p, SAMR_Q_LOOKUP_NAMES *q_u, SAMR_R_LO
 
 	DEBUG(5,("_samr_lookup_names: looking name on SID %s\n", sid_to_string(sid_str, &pol_sid)));
 	
-	become_root(); /* local_lookup_name can require root privs */
-
 	for (i = 0; i < num_rids; i++) {
 		fstring name;
             	DOM_SID sid;
@@ -1421,8 +1419,6 @@ NTSTATUS _samr_lookup_names(pipes_struct *p, SAMR_Q_LOOKUP_NAMES *q_u, SAMR_R_LO
             	}
 	}
 
-	unbecome_root();
-
 	init_samr_r_lookup_names(p->mem_ctx, r_u, num_rids, rid, (uint32 *)type, r_u->status);
 
 	DEBUG(5,("_samr_lookup_names: %d\n", __LINE__));
@@ -4250,75 +4246,114 @@ NTSTATUS _samr_open_group(pipes_struct *p, SAMR_Q_OPEN_GROUP *q_u, SAMR_R_OPEN_G
 }
 
 /*********************************************************************
- _samr_remove_user_foreign_domain
+ _samr_remove_sid_foreign_domain
 *********************************************************************/
 
-NTSTATUS _samr_remove_user_foreign_domain(pipes_struct *p, 
-                                          SAMR_Q_REMOVE_USER_FOREIGN_DOMAIN *q_u, 
-                                          SAMR_R_REMOVE_USER_FOREIGN_DOMAIN *r_u)
+NTSTATUS _samr_remove_sid_foreign_domain(pipes_struct *p, 
+                                          SAMR_Q_REMOVE_SID_FOREIGN_DOMAIN *q_u, 
+                                          SAMR_R_REMOVE_SID_FOREIGN_DOMAIN *r_u)
 {
-	DOM_SID			user_sid, dom_sid;
+	DOM_SID			delete_sid, alias_sid;
 	SAM_ACCOUNT 		*sam_pass=NULL;
 	uint32 			acc_granted;
+	GROUP_MAP 		map;
+	BOOL			is_user = False;
+	NTSTATUS		result;
+	enum SID_NAME_USE	type = SID_NAME_UNKNOWN;
 	
-	sid_copy( &user_sid, &q_u->sid.sid );
+	sid_copy( &delete_sid, &q_u->sid.sid );
 	
-	DEBUG(5,("_samr_remove_user_foreign_domain: removing user [%s]\n",
-		sid_string_static(&user_sid)));
+	DEBUG(5,("_samr_remove_sid_foreign_domain: removing SID [%s]\n",
+		sid_string_static(&delete_sid)));
 		
 	/* Find the policy handle. Open a policy on it. */
 	
-	if (!get_lsa_policy_samr_sid(p, &q_u->dom_pol, &dom_sid, &acc_granted)) 
+	if (!get_lsa_policy_samr_sid(p, &q_u->dom_pol, &alias_sid, &acc_granted)) 
 		return NT_STATUS_INVALID_HANDLE;
+	
+	result = access_check_samr_function(acc_granted, STD_RIGHT_DELETE_ACCESS, 
+		"_samr_remove_sid_foreign_domain");
 		
-	if (!NT_STATUS_IS_OK(r_u->status = access_check_samr_function(acc_granted, 
-		STD_RIGHT_DELETE_ACCESS, "_samr_remove_user_foreign_domain"))) 
-	{
-		return r_u->status;
-	}
+	if (!NT_STATUS_IS_OK(result)) 
+		return result;
+			
+	DEBUG(8, ("_samr_remove_sid_foreign_domain:sid is %s\n", 
+		sid_string_static(&alias_sid)));
 		
-	if ( !sid_check_is_in_our_domain(&user_sid) ) {
-		DEBUG(5,("_samr_remove_user_foreign_domain: user not is our domain!\n"));
-		return NT_STATUS_NO_SUCH_USER;
+	/* make sure we can handle this */
+	
+	if ( sid_check_is_domain(&alias_sid) )
+		type = SID_NAME_DOM_GRP;
+	else if ( sid_check_is_builtin(&alias_sid) )
+		type = SID_NAME_ALIAS;
+	
+	if ( type == SID_NAME_UNKNOWN ) {
+		DEBUG(10, ("_samr_remove_sid_foreign_domain: can't operate on what we don't own!\n"));
+		return NT_STATUS_OK;
 	}
 
 	/* check if the user exists before trying to delete */
 	
 	pdb_init_sam(&sam_pass);
 	
-	if ( !pdb_getsampwsid(sam_pass, &user_sid) ) {
+	if ( pdb_getsampwsid(sam_pass, &delete_sid) ) {
+		is_user = True;
+	} else {
+		/* maybe it is a group */
+		if( !pdb_getgrsid(&map, delete_sid) ) {
+			DEBUG(3,("_samr_remove_sid_foreign_domain: %s is not a user or a group!\n",
+				sid_string_static(&delete_sid)));
+			result = NT_STATUS_INVALID_SID;
+			goto done;
+		}
+	}
 	
-		DEBUG(5,("_samr_remove_user_foreign_domain:User %s doesn't exist.\n", 
-			sid_string_static(&user_sid)));
-			
-		pdb_free_sam(&sam_pass);
+	/* we can only delete a user from a group since we don't have 
+	   nested groups anyways.  So in the latter case, just say OK */
+	   
+	if ( is_user ) {
+		GROUP_MAP	*mappings = NULL;
+		uint32		num_groups, i;
+		struct group	*grp2;
 		
-		return NT_STATUS_NO_SUCH_USER;
-	}
+		if ( pdb_enum_group_mapping(type, &mappings, &num_groups, False) && num_groups>0 ) {
+		
+			/* interate over the groups */
+			for ( i=0; i<num_groups; i++ ) {
 
-	/*
-	 * delete the unix side
-	 * 
-	 * note: we don't check if the delete really happened
-	 * as the script is not necessary present
-	 * and maybe the sysadmin doesn't want to delete the unix side
-	 */
-	 
-	smb_delete_user(pdb_get_username(sam_pass));
+				grp2 = getgrgid(mappings[i].gid);
 
-	/* and delete the samba side */
-	
-	if ( !pdb_delete_sam_account(sam_pass) ) {
-	
-		DEBUG(5,("_samr_delete_dom_user:Failed to delete entry for user %s.\n", pdb_get_username(sam_pass)));
-		pdb_free_sam(&sam_pass);
-		
-		return NT_STATUS_CANNOT_DELETE;
+				if ( !grp2 ) {
+					DEBUG(0,("_samr_remove_sid_foreign_domain: group mapping without UNIX group!\n"));
+					continue;
+				}
+			
+				if ( !user_in_unix_group_list(pdb_get_username(sam_pass), grp2->gr_name) )
+					continue;
+				
+				smb_delete_user_group(grp2->gr_name, pdb_get_username(sam_pass));
+				
+				if ( user_in_unix_group_list(pdb_get_username(sam_pass), grp2->gr_name) ) {
+					/* should we fail here ? */
+					DEBUG(0,("_samr_remove_sid_foreign_domain: Delete user [%s] from group [%s] failed!\n",
+						pdb_get_username(sam_pass), grp2->gr_name ));
+					continue;
+				}
+					
+				DEBUG(10,("_samr_remove_sid_foreign_domain: Removed user [%s] from group [%s]!\n",
+					pdb_get_username(sam_pass), grp2->gr_name ));
+			}
+			
+			SAFE_FREE(mappings);
+		}
 	}
 	
+	result = NT_STATUS_OK;
+done:
+
 	pdb_free_sam(&sam_pass);
 
-	return NT_STATUS_OK;
+	return result;
 }
 
 /*******************************************************************
diff --git a/source/rpc_server/srv_util.c b/source/rpc_server/srv_util.c
index d5b87b7c10d..c2395e6faec 100644
--- a/source/rpc_server/srv_util.c
+++ b/source/rpc_server/srv_util.c
@@ -382,7 +382,7 @@ BOOL get_domain_user_groups(TALLOC_CTX *ctx, int *numgroups, DOM_GID **pgids, SA
  done:
 	*pgids=gids;
 	*numgroups=cur_gid;
-	safe_free(map);
+	SAFE_FREE(map);
 
 	return True;
 }