* 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
(This used to be commit c0f34b42a6a4af09ae4b76721bc350784d87f686)

3 files changed, 92 insertions, 57 deletions

diff --git a/source3/rpc_server/srv_samr.c b/source3/rpc_server/srv_samr.c
index a0f62c20fc..971f5ed40c 100644
--- a/source3/rpc_server/srv_samr.c
+++ b/source3/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/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index d3da830991..7be9b41ee9 100644
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/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/source3/rpc_server/srv_util.c b/source3/rpc_server/srv_util.c
index d5b87b7c10..c2395e6fae 100644
--- a/source3/rpc_server/srv_util.c
+++ b/source3/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;
 }