JHT came up with a nasty (broken) torture case in preparing examples for

his book.

This prompted me to look at the code that reads the unix group list.  This
code did a lot of name -> uid -> name -> sid translations, which caused
problems.  Instead, we now do just name->sid

I also cleaned up some interfaces, and client tools.

Andrew Bartlett
(This used to be commit f9e59f8bc06fae7e5c8cb0980947f78942dc25c0)

diff --git a/source3/groupdb/mapping.c b/source3/groupdb/mapping.c
index 08ac6a25a53c524e068d1cd37f371c3b07096671..97abbd46e317eb0e75059f3928c86ceeea88d4a2 100644 (file)
--- a/source3/groupdb/mapping.c
+++ b/source3/groupdb/mapping.c
@@ -551,7 +551,7 @@ BOOL get_domain_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 
 /* get a local (alias) group from it's SID */
 
-BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
+BOOL get_local_group_from_sid(DOM_SID *sid, GROUP_MAP *map)
 {
        BOOL ret;
        
@@ -562,7 +562,7 @@ BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 
        /* The group is in the mapping table */
        become_root();
-       ret = pdb_getgrsid(map, sid);
+       ret = pdb_getgrsid(map, *sid);
        unbecome_root();
        
        if ( !ret )
@@ -585,7 +585,7 @@ BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
                uint32 alias_rid;
                struct group *grp;
 
-               sid_peek_rid(&sid, &alias_rid);
+               sid_peek_rid(sid, &alias_rid);
                map->gid=pdb_group_rid_to_gid(alias_rid);
                
                grp = getgrgid(map->gid);
@@ -599,7 +599,7 @@ BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
                fstrcpy(map->nt_name, grp->gr_name);
                fstrcpy(map->comment, "Local Unix Group");
 
-               sid_copy(&map->sid, &sid);
+               sid_copy(&map->sid, sid);
        }
 #endif
 
@@ -608,7 +608,7 @@ BOOL get_local_group_from_sid(DOM_SID sid, GROUP_MAP *map)
 
 /* get a builtin group from it's SID */
 
-BOOL get_builtin_group_from_sid(DOM_SID sid, GROUP_MAP *map)
+BOOL get_builtin_group_from_sid(DOM_SID *sid, GROUP_MAP *map)
 {
        struct group *grp;
        BOOL ret;
@@ -620,7 +620,7 @@ BOOL get_builtin_group_from_sid(DOM_SID sid, GROUP_MAP *map)
        }
 
        become_root();
-       ret = pdb_getgrsid(map, sid);
+       ret = pdb_getgrsid(map, *sid);
        unbecome_root();
        
        if ( !ret )
@@ -690,7 +690,7 @@ BOOL get_group_from_gid(gid_t gid, GROUP_MAP *map)
  Get the member users of a group and
  all the users who have that group as primary.
             
- give back an array of uid
+ give back an array of SIDS
  return the grand number of users
 
 
@@ -698,21 +698,21 @@ BOOL get_group_from_gid(gid_t gid, GROUP_MAP *map)
 
 ****************************************************************************/
         
-BOOL get_uid_list_of_group(gid_t gid, uid_t **uid, int *num_uids)
+BOOL get_sid_list_of_group(gid_t gid, DOM_SID **sids, int *num_sids)
 {
        struct group *grp;
        struct passwd *pwd;
        int i=0;
        char *gr;
-       uid_t *u;
+       DOM_SID *s;
  
        if(!init_group_mapping()) {
                DEBUG(0,("failed to initialize group mapping"));
                return(False);
        }
 
-       *num_uids = 0;
-       *uid=NULL;
+       *num_sids = 0;
+       *sids=NULL;
        
        if ( (grp=getgrgid(gid)) == NULL)
                return False;
@@ -721,39 +721,74 @@ BOOL get_uid_list_of_group(gid_t gid, uid_t **uid, int *num_uids)
        DEBUG(10, ("getting members\n"));
         
        while (gr && (*gr != (char)'\0')) {
-               u = Realloc((*uid), sizeof(uid_t)*(*num_uids+1));
-               if (!u) {
-                       DEBUG(0,("get_uid_list_of_group: unable to enlarge uid list!\n"));
+               SAM_ACCOUNT *group_member_acct = NULL;
+               BOOL found_user;
+               s = Realloc((*sids), sizeof(**sids)*(*num_sids+1));
+               if (!s) {
+                       DEBUG(0,("get_uid_list_of_group: unable to enlarge SID list!\n"));
                        return False;
                }
-               else (*uid) = u;
+               else (*sids) = s;
+
+               if (!NT_STATUS_IS_OK(pdb_init_sam(&group_member_acct))) {
+                       continue;
+               }
 
-               if( (pwd=getpwnam_alloc(gr)) !=NULL) {
-                       (*uid)[*num_uids]=pwd->pw_uid;
-                       (*num_uids)++;
-                       passwd_free(&pwd);
+               become_root();
+               found_user = pdb_getsampwnam(group_member_acct, gr);
+               unbecome_root();
+       
+               if (found_user) {
+                       sid_copy(&(*sids)[*num_sids], pdb_get_user_sid(group_member_acct));
+                       (*num_sids)++;
                }
+       
+               pdb_free_sam(&group_member_acct);
+
                gr = grp->gr_mem[++i];
        }
-       DEBUG(10, ("got [%d] members\n", *num_uids));
+       DEBUG(10, ("got [%d] members\n", *num_sids));
+
+       winbind_off();
 
        setpwent();
        while ((pwd=getpwent()) != NULL) {
                if (pwd->pw_gid==gid) {
-                       u = Realloc((*uid), sizeof(uid_t)*(*num_uids+1));
-                       if (!u) {
-                               DEBUG(0,("get_uid_list_of_group: unable to enlarge uid list!\n"));
+                       SAM_ACCOUNT *group_member_acct = NULL;
+                       BOOL found_user;
+                       s = Realloc((*sids), sizeof(**sids)*(*num_sids+1));
+                       if (!s) {
+                               DEBUG(0,("get_sid_list_of_group: unable to enlarge SID list!\n"));
+                               winbind_on();
                                return False;
                        }
-                       else (*uid) = u;
-                       (*uid)[*num_uids]=pwd->pw_uid;
-
-                       (*num_uids)++;
+                       else (*sids) = s;
+                       
+                       if (!NT_STATUS_IS_OK(pdb_init_sam(&group_member_acct))) {
+                               continue;
+                       }
+                       
+                       become_root();
+                       found_user = pdb_getsampwnam(group_member_acct, pwd->pw_name);
+                       unbecome_root();
+                       
+                       if (found_user) {
+                               sid_copy(&(*sids)[*num_sids], pdb_get_user_sid(group_member_acct));
+                               (*num_sids)++;
+                       } else {
+                               DEBUG(4,("get_sid_list_of_group: User %s [uid == %lu] has no samba account\n",
+                                        pwd->pw_name, (unsigned long)pwd->pw_uid));
+                               if (algorithmic_uid_to_sid(&(*sids)[*num_sids], pwd->pw_uid))
+                                       (*num_sids)++;
+                       }
+       
+                       pdb_free_sam(&group_member_acct);
                }
        }
        endpwent();
-       DEBUG(10, ("got primary groups, members: [%d]\n", *num_uids));
+       DEBUG(10, ("got primary groups, members: [%d]\n", *num_sids));
 
+       winbind_on();
         return True;
 }
 

diff --git a/source3/passdb/passdb.c b/source3/passdb/passdb.c
index 616ebf2252b11351efbb8b064c3f69ca483fc2a6..a3f8fbc3500f32edb475cc137f869404dccfc8c8 100644 (file)
--- a/source3/passdb/passdb.c
+++ b/source3/passdb/passdb.c
@@ -1074,7 +1074,7 @@ BOOL local_password_change(const char *user_name, int local_flags,
  Convert a uid to SID - algorithmic.
 ****************************************************************************/
 
-static DOM_SID *algorithmic_uid_to_sid(DOM_SID *psid, uid_t uid)
+DOM_SID *algorithmic_uid_to_sid(DOM_SID *psid, uid_t uid)
 {
        if ( !lp_enable_rid_algorithm() )
                return NULL;

diff --git a/source3/rpc_server/srv_samr_nt.c b/source3/rpc_server/srv_samr_nt.c
index 4d4dfed47c9d9c04a8226e3f6e527111a153ca65..be5f197198e518b6496a03a04bc72eafc1fe8874 100644 (file)
--- a/source3/rpc_server/srv_samr_nt.c
+++ b/source3/rpc_server/srv_samr_nt.c
@@ -3176,18 +3176,15 @@ NTSTATUS _samr_query_aliasmem(pipes_struct *p, SAMR_Q_QUERY_ALIASMEM *q_u, SAMR_
        int i;
 
        GROUP_MAP map;
-       int num_uids = 0;
+       int num_sids = 0;
        DOM_SID2 *sid;
-       uid_t *uid=NULL;
+       DOM_SID *sids=NULL;
 
        DOM_SID alias_sid;
        DOM_SID als_sid;
        uint32 alias_rid;
        fstring alias_sid_str;
-       DOM_SID temp_sid;
 
-       SAM_ACCOUNT *sam_user = NULL;
-       BOOL check;
        uint32 acc_granted;
 
        /* find the policy handle.  open a policy on it. */
@@ -3207,65 +3204,40 @@ NTSTATUS _samr_query_aliasmem(pipes_struct *p, SAMR_Q_QUERY_ALIASMEM *q_u, SAMR_
 
        if (sid_equal(&alias_sid, &global_sid_Builtin)) {
                DEBUG(10, ("lookup on Builtin SID (S-1-5-32)\n"));
-               if(!get_builtin_group_from_sid(als_sid, &map))
+               if(!get_builtin_group_from_sid(&als_sid, &map))
                        return NT_STATUS_NO_SUCH_ALIAS;
        } else {
                if (sid_equal(&alias_sid, get_global_sam_sid())) {
                        DEBUG(10, ("lookup on Server SID\n"));
-                       if(!get_local_group_from_sid(als_sid, &map))
+                       if(!get_local_group_from_sid(&als_sid, &map)) {
+                               fstring alias_sid_string;
+                               DEBUG(10, ("Alias %s not found\n", sid_to_string(alias_sid_string, &als_sid))); 
                                return NT_STATUS_NO_SUCH_ALIAS;
+                       }
                }
        }
 
-       if(!get_uid_list_of_group(map.gid, &uid, &num_uids))
+       if (!get_sid_list_of_group(map.gid, &sids, &num_sids)) {
+               fstring alias_sid_string;
+               DEBUG(10, ("Alias %s found, but member list unavailable\n", sid_to_string(alias_sid_string, &als_sid))); 
                return NT_STATUS_NO_SUCH_ALIAS;
+       }
 
        DEBUG(10, ("sid is %s\n", alias_sid_str));
-       sid = (DOM_SID2 *)talloc_zero(p->mem_ctx, sizeof(DOM_SID2) * num_uids); 
-       if (num_uids!=0 && sid == NULL) 
+       sid = (DOM_SID2 *)talloc_zero(p->mem_ctx, sizeof(DOM_SID2) * num_sids); 
+       if (num_sids!=0 && sid == NULL) {
+               SAFE_FREE(sids);
                return NT_STATUS_NO_MEMORY;
+       }
 
-       for (i = 0; i < num_uids; i++) {
-               struct passwd *pass;
-               uint32 rid;
-
-               sid_copy(&temp_sid, get_global_sam_sid());
-
-               pass = getpwuid_alloc(uid[i]);
-               if (!pass) continue;
-
-               if (!NT_STATUS_IS_OK(pdb_init_sam(&sam_user))) {
-                       passwd_free(&pass);
-                       continue;
-               }
-
-               become_root();
-               check = pdb_getsampwnam(sam_user, pass->pw_name);
-               unbecome_root();
-       
-               if (check != True) {
-                       pdb_free_sam(&sam_user);
-                       passwd_free(&pass);
-                       continue;
-               }
-       
-               rid = pdb_get_user_rid(sam_user);
-               if (rid == 0) {
-                       pdb_free_sam(&sam_user);
-                       passwd_free(&pass);
-                       continue;
-               }
-
-               pdb_free_sam(&sam_user);
-               passwd_free(&pass);
-
-               sid_append_rid(&temp_sid, rid);
-               
-               init_dom_sid2(&sid[i], &temp_sid);
+       for (i = 0; i < num_sids; i++) {
+               init_dom_sid2(&sid[i], &sids[i]);
        }
 
        DEBUG(10, ("sid is %s\n", alias_sid_str));
-       init_samr_r_query_aliasmem(r_u, num_uids, sid, NT_STATUS_OK);
+       init_samr_r_query_aliasmem(r_u, num_sids, sid, NT_STATUS_OK);
+
+       SAFE_FREE(sids);
 
        return NT_STATUS_OK;
 }
@@ -3276,20 +3248,19 @@ NTSTATUS _samr_query_aliasmem(pipes_struct *p, SAMR_Q_QUERY_ALIASMEM *q_u, SAMR_
 
 NTSTATUS _samr_query_groupmem(pipes_struct *p, SAMR_Q_QUERY_GROUPMEM *q_u, SAMR_R_QUERY_GROUPMEM *r_u)
 {
-       int num_uids = 0;
+       int num_sids = 0;
+       int final_num_sids = 0;
        int i;
        DOM_SID group_sid;
        uint32 group_rid;
        fstring group_sid_str;
-       uid_t *uid=NULL;
+       DOM_SID *sids=NULL;
        
        GROUP_MAP map;
 
        uint32 *rid=NULL;
        uint32 *attr=NULL;
 
-       SAM_ACCOUNT *sam_user = NULL;
-       BOOL check;
        uint32 acc_granted;
 
        /* find the policy handle.  open a policy on it. */
@@ -3316,52 +3287,32 @@ NTSTATUS _samr_query_groupmem(pipes_struct *p, SAMR_Q_QUERY_GROUPMEM *q_u, SAMR_
        if(!get_domain_group_from_sid(group_sid, &map))
                return NT_STATUS_NO_SUCH_GROUP;
 
-       if(!get_uid_list_of_group(map.gid, &uid, &num_uids))
+       if(!get_sid_list_of_group(map.gid, &sids, &num_sids))
                return NT_STATUS_NO_SUCH_GROUP;
 
-       rid=talloc_zero(p->mem_ctx, sizeof(uint32)*num_uids);
-       attr=talloc_zero(p->mem_ctx, sizeof(uint32)*num_uids);
+       rid=talloc_zero(p->mem_ctx, sizeof(uint32)*num_sids);
+       attr=talloc_zero(p->mem_ctx, sizeof(uint32)*num_sids);
        
-       if (num_uids!=0 && (rid==NULL || attr==NULL))
+       if (num_sids!=0 && (rid==NULL || attr==NULL))
                return NT_STATUS_NO_MEMORY;
        
-       for (i=0; i<num_uids; i++) {
-               struct passwd *pass;
+       for (i=0; i<num_sids; i++) {
                uint32 urid;
-
-               pass = getpwuid_alloc(uid[i]);
-               if (!pass) continue;
-
-               if (!NT_STATUS_IS_OK(pdb_init_sam(&sam_user))) {
-                       passwd_free(&pass);
-                       continue;
-               }
-
-               become_root();
-               check = pdb_getsampwnam(sam_user, pass->pw_name);
-               unbecome_root();
-       
-               if (check != True) {
-                       pdb_free_sam(&sam_user);
-                       passwd_free(&pass);
-                       continue;
-               }
-       
-               urid = pdb_get_user_rid(sam_user);
-               if (urid == 0) {
-                       pdb_free_sam(&sam_user);
-                       passwd_free(&pass);
-                       continue;
+               
+               if (sid_peek_check_rid(get_global_sam_sid(), &sids[i], &urid)) {
+                       rid[final_num_sids] = urid;
+                       attr[final_num_sids] = SID_NAME_USER;
+                       final_num_sids++;
+               } else {
+                       fstring user_sid_str, domain_sid_str;
+                       DEBUG(1, ("_samr_query_groupmem: SID %s in group %s is not in our domain %s\n",
+                                 sid_to_string(user_sid_str, &sids[i]), 
+                                 sid_to_string(group_sid_str, &group_sid),
+                                 sid_to_string(domain_sid_str, get_global_sam_sid())));
                }
-
-               pdb_free_sam(&sam_user);
-               passwd_free(&pass);
-
-               rid[i] = urid;
-               attr[i] = SID_NAME_USER;                
        }
 
-       init_samr_r_query_groupmem(r_u, num_uids, rid, attr, NT_STATUS_OK);
+       init_samr_r_query_groupmem(r_u, final_num_sids, rid, attr, NT_STATUS_OK);
 
        return NT_STATUS_OK;
 }
@@ -3397,13 +3348,13 @@ NTSTATUS _samr_add_aliasmem(pipes_struct *p, SAMR_Q_ADD_ALIASMEM *q_u, SAMR_R_AD
 
        if (sid_compare(&alias_sid, get_global_sam_sid())>0) {
                DEBUG(10, ("adding member on Server SID\n"));
-               if(!get_local_group_from_sid(alias_sid, &map))
+               if(!get_local_group_from_sid(&alias_sid, &map))
                        return NT_STATUS_NO_SUCH_ALIAS;
        
        } else {
                if (sid_compare(&alias_sid, &global_sid_Builtin)>0) {
                        DEBUG(10, ("adding member on BUILTIN SID\n"));
-                       if( !get_local_group_from_sid(alias_sid, &map))
+                       if( !get_local_group_from_sid(&alias_sid, &map))
                                return NT_STATUS_NO_SUCH_ALIAS;
 
                } else
@@ -3494,7 +3445,7 @@ NTSTATUS _samr_del_aliasmem(pipes_struct *p, SAMR_Q_DEL_ALIASMEM *q_u, SAMR_R_DE
                return NT_STATUS_NO_SUCH_ALIAS;
        }
 
-       if( !get_local_group_from_sid(alias_sid, &map))
+       if( !get_local_group_from_sid(&alias_sid, &map))
                return NT_STATUS_NO_SUCH_ALIAS;
 
        if ((grp=getgrgid(map.gid)) == NULL)
@@ -3882,7 +3833,7 @@ NTSTATUS _samr_delete_dom_alias(pipes_struct *p, SAMR_Q_DELETE_DOM_ALIAS *q_u, S
 
        DEBUG(10, ("lookup on Local SID\n"));
 
-       if(!get_local_group_from_sid(alias_sid, &map))
+       if(!get_local_group_from_sid(&alias_sid, &map))
                return NT_STATUS_NO_SUCH_ALIAS;
 
        gid=map.gid;
@@ -4042,8 +3993,8 @@ NTSTATUS _samr_query_groupinfo(pipes_struct *p, SAMR_Q_QUERY_GROUPINFO *q_u, SAM
 {
        DOM_SID group_sid;
        GROUP_MAP map;
-       uid_t *uid=NULL;
-       int num_uids=0;
+       DOM_SID *sids=NULL;
+       int num_sids=0;
        GROUP_INFO_CTR *ctr;
        uint32 acc_granted;
        BOOL ret;
@@ -4068,10 +4019,10 @@ NTSTATUS _samr_query_groupinfo(pipes_struct *p, SAMR_Q_QUERY_GROUPINFO *q_u, SAM
        switch (q_u->switch_level) {
                case 1:
                        ctr->switch_value1 = 1;
-                       if(!get_uid_list_of_group(map.gid, &uid, &num_uids))
+                       if(!get_sid_list_of_group(map.gid, &sids, &num_sids))
                                return NT_STATUS_NO_SUCH_GROUP;
-                       init_samr_group_info1(&ctr->group.info1, map.nt_name, map.comment, num_uids);
-                       SAFE_FREE(uid);
+                       init_samr_group_info1(&ctr->group.info1, map.nt_name, map.comment, num_sids);
+                       SAFE_FREE(sids);
                        break;
                case 3:
                        ctr->switch_value1 = 3;
@@ -4153,7 +4104,7 @@ NTSTATUS _samr_set_aliasinfo(pipes_struct *p, SAMR_Q_SET_ALIASINFO *q_u, SAMR_R_
                return r_u->status;
        }
                
-       if (!get_local_group_from_sid(group_sid, &map))
+       if (!get_local_group_from_sid(&group_sid, &map))
                return NT_STATUS_NO_SUCH_GROUP;
        
        ctr=&q_u->ctr;

diff --git a/source3/utils/net_rpc.c b/source3/utils/net_rpc.c
index 04a03307740b2959db25f5e215804342fa05b5e3..b28365274c066c21a53d98e2e504a291c7fdbb1d 100644 (file)
--- a/source3/utils/net_rpc.c
+++ b/source3/utils/net_rpc.c
@@ -1158,7 +1158,7 @@ rpc_group_members_internals(const DOM_SID *domain_sid, struct cli_state *cli,
        if (!NT_STATUS_IS_OK(result))
                goto done;
 
-       do {
+       while (num_members > 0) {
                int this_time = 512;
 
                if (num_members < this_time)
@@ -1177,8 +1177,7 @@ rpc_group_members_internals(const DOM_SID *domain_sid, struct cli_state *cli,
 
                num_members -= this_time;
                group_rids += 512;
-
-       } while (num_members > 0);
+       }
 
  done:
        return result;

diff --git a/source3/utils/net_rpc_samsync.c b/source3/utils/net_rpc_samsync.c
index d1c8300a4975184717c72a130bc2390bf005721e..e97a362acce631f754bdef92414e1d57d89731fc 100644 (file)
--- a/source3/utils/net_rpc_samsync.c
+++ b/source3/utils/net_rpc_samsync.c
@@ -783,13 +783,13 @@ fetch_alias_mem(uint32 rid, SAM_ALIAS_MEM_INFO *delta, DOM_SID dom_sid)
 
        if (sid_equal(&dom_sid, &global_sid_Builtin)) {
                sid_type = SID_NAME_WKN_GRP;
-               if (!get_builtin_group_from_sid(group_sid, &map, False)) {
+               if (!get_builtin_group_from_sid(&group_sid, &map, False)) {
                        DEBUG(0, ("Could not find builtin group %s\n", sid_string_static(&group_sid)));
                        return NT_STATUS_NO_SUCH_GROUP;
                }
        } else {
                sid_type = SID_NAME_ALIAS;
-               if (!get_local_group_from_sid(group_sid, &map, False)) {
+               if (!get_local_group_from_sid(&group_sid, &map, False)) {
                        DEBUG(0, ("Could not find local group %s\n", sid_string_static(&group_sid)));
                        return NT_STATUS_NO_SUCH_GROUP;
                }