r22390: Patchset sent to samba-technical to address the winbind
authorGerald Carter <jerry@samba.org>
Thu, 19 Apr 2007 22:26:09 +0000 (22:26 +0000)
committerGerald (Jerry) Carter <jerry@samba.org>
Wed, 10 Oct 2007 17:19:30 +0000 (12:19 -0500)
loop when allocating a new id for a SID:

auth_util.patch Revert create_local_token() to
the 3.0.24 codebase

idmap_type.patch Have the caller fillin the
id_map.xid.type field when
resolving a SID so that if we allocate
a new id, we know what type to use

winbindd_api.patch Remove the WINBINDD_SIDS_TO_XIDS calls
from the public winbindd interface
for the 3.0.25 release

idmap_rid.patch Cleanup the idmap_rid backend to not
call back into winbindd to resolve
the SID in order to verify it's type.

source/auth/auth_util.c
source/include/idmap.h
source/include/smb.h
source/nsswitch/idmap.c
source/nsswitch/idmap_rid.c
source/nsswitch/idmap_util.c
source/nsswitch/winbindd.c
source/nsswitch/winbindd_dual.c

index 0de3bf23252f7fe0721a598f9774ec6f09c6f33a..336daa906d0e81cad662487b0ded50a23fe09b8a 100644 (file)
@@ -637,9 +637,7 @@ static NTSTATUS log_nt_token(TALLOC_CTX *tmp_ctx, NT_USER_TOKEN *token)
 NTSTATUS create_local_token(auth_serversupplied_info *server_info)
 {
        TALLOC_CTX *mem_ctx;
-       struct id_map *ids;
        NTSTATUS status;
-       BOOL wb = True;
        size_t i;
        
 
@@ -686,46 +684,20 @@ NTSTATUS create_local_token(auth_serversupplied_info *server_info)
        server_info->groups = NULL;
 
        /* Start at index 1, where the groups start. */
-       ids = talloc_zero_array(mem_ctx, struct id_map, server_info->ptok->num_sids);
-       for (i = 0; i < server_info->ptok->num_sids-1; i++) {
-               ids[i].sid = &server_info->ptok->user_sids[i + 1]; /* store the sids */
-       }
-
-       if (!winbind_sids_to_unixids(ids, server_info->ptok->num_sids-1)) {
-               DEBUG(2, ("Query to map secondary SIDs failed!\n"));
-               if (!winbind_ping()) {
-                       DEBUG(2, ("Winbindd is not running, will try to map SIDs one by one with legacy code\n"));
-                       wb = False;
-               }
-       }
 
-       for (i = 0; i < server_info->ptok->num_sids-1; i++) {
-               gid_t agid;
+       for (i=1; i<server_info->ptok->num_sids; i++) {
+               gid_t gid;
+               DOM_SID *sid = &server_info->ptok->user_sids[i];
 
-               if (wb) {
-                       if (ids[i].status != ID_MAPPED) {
-                               DEBUG(10, ("Could not convert SID %s to gid, "
-                                          "ignoring it\n", sid_string_static(ids[i].sid)));
-                               continue;
-                       }
-                       if (ids[i].xid.type == ID_TYPE_UID) {
-                               DEBUG(10, ("SID %s is a User ID (%u) not a Group ID, "
-                                          "ignoring it\n", sid_string_static(ids[i].sid), ids[i].xid.id));
-                               continue;
-                       }
-                       agid = (gid_t)ids[i].xid.id;
-               } else {
-                       if (! sid_to_gid(ids[i].sid, &agid)) {
-                               continue;
-                       }
-               }
-               if (!add_gid_to_array_unique(server_info, agid, &server_info->groups,
-                                       &server_info->n_groups)) {
-                       TALLOC_FREE(mem_ctx);
-                       return NT_STATUS_NO_MEMORY;
+               if (!sid_to_gid(sid, &gid)) {
+                       DEBUG(10, ("Could not convert SID %s to gid, "
+                                  "ignoring it\n", sid_string_static(sid)));
+                       continue;
                }
+               add_gid_to_array_unique(server_info, gid, &server_info->groups,
+                                       &server_info->n_groups);
        }
-
+       
        debug_nt_user_token(DBGC_AUTH, 10, server_info->ptok);
 
        status = log_nt_token(mem_ctx, server_info->ptok);
index 472358a2305629b5aefbe40dc1d85c3b4f3106ef..f4926f1e5c4bd2bf464d4e64b275a5d8a46c839e 100644 (file)
@@ -52,8 +52,14 @@ struct idmap_methods {
        /* Called when backend is first loaded */
        NTSTATUS (*init)(struct idmap_domain *dom);
 
+       /* Map an array of uids/gids to SIDs.  The caller specifies
+          the uid/gid and type. Gets back the SID. */
        NTSTATUS (*unixids_to_sids)(struct idmap_domain *dom, struct id_map **ids);
+
+       /* Map an arry of SIDs to uids/gids.  The caller sets the SID
+          and type and gets back a uid or gid. */
        NTSTATUS (*sids_to_unixids)(struct idmap_domain *dom, struct id_map **ids);
+
        NTSTATUS (*set_mapping)(struct idmap_domain *dom, const struct id_map *map);
        NTSTATUS (*remove_mapping)(struct idmap_domain *dom, const struct id_map *map);
 
index f85201ed303fcafe3a9d4ca329aa61a32dfe5406..6c44db5a3485115f7b6ce1538a9845348377fa56 100644 (file)
@@ -276,13 +276,14 @@ typedef struct dom_sid {
 #define dom_sid28 dom_sid
 
 enum id_mapping {
-       ID_UNKNOWN,
+       ID_UNKNOWN = 0,
        ID_MAPPED,
        ID_UNMAPPED,
        ID_EXPIRED
 };
 
 enum id_type {
+       ID_TYPE_NOT_SPECIFIED = 0,
        ID_TYPE_UID,
        ID_TYPE_GID
 };
index 82b8f3d592e5635d2be90ecb641d1c6feb0693df..530e03089d0d364b43066ebe6047dfc576dd4da2 100644 (file)
@@ -91,24 +91,6 @@ static struct idmap_alloc_methods *get_alloc_methods(struct idmap_alloc_backend
        return NULL;
 }
 
-/* part of a quick hack to avoid loops, need to be sorted out correctly later on */
-static BOOL idmap_in_own_child;
-
-static BOOL idmap_is_in_own_child(void)
-{
-       return idmap_in_own_child;
-}
-
-void reset_idmap_in_own_child(void)
-{
-       idmap_in_own_child = False;
-}
-
-void set_idmap_in_own_child(void)
-{
-       idmap_in_own_child = True;
-}
-
 BOOL idmap_is_offline(void)
 {
        return ( lp_winbind_offline_logon() &&
@@ -855,9 +837,6 @@ static NTSTATUS idmap_new_mapping(TALLOC_CTX *ctx, struct id_map *map)
 {
        NTSTATUS ret;
        struct idmap_domain *dom;
-       char *domname, *name;
-       enum lsa_SidType sid_type;
-       BOOL wbret;
 
        /* If we are offline we cannot lookup SIDs, deny mapping */
        if (idmap_is_offline()) {
@@ -869,70 +848,46 @@ static NTSTATUS idmap_new_mapping(TALLOC_CTX *ctx, struct id_map *map)
                return NT_STATUS_NONE_MAPPED;
        }
 
-       /* quick hack to make things work, will need proper fix later on */     
-       if (idmap_is_in_own_child()) {
-               /* by default calls to winbindd are disabled
-                  the following call will not recurse so this is safe */
-               winbind_on();
-               wbret = winbind_lookup_sid(ctx, map->sid,
-                                               (const char **)&domname,
-                                               (const char **)&name,
-                                               &sid_type);
-               winbind_off();
-       } else {
-               wbret = winbindd_lookup_name_by_sid(ctx, map->sid,
-                                                       &domname,
-                                                       &name,
-                                                       &sid_type);
-       }
-
        /* check if this is a valid SID and then map it */
-       if (wbret) {
-               switch (sid_type) {
-               case SID_NAME_USER:
-                       ret = idmap_allocate_uid(&map->xid);
-                       if ( ! NT_STATUS_IS_OK(ret)) {
-                               /* can't allocate id, let's just leave it unmapped */
-                               DEBUG(2, ("uid allocation failed! Can't create mapping\n"));
-                               return NT_STATUS_NONE_MAPPED;
-                       }
-                       break;
-               case SID_NAME_DOM_GRP:
-               case SID_NAME_ALIAS:
-               case SID_NAME_WKN_GRP:
-                       ret = idmap_allocate_gid(&map->xid);
-                       if ( ! NT_STATUS_IS_OK(ret)) {
-                               /* can't allocate id, let's just leave it unmapped */
-                               DEBUG(2, ("gid allocation failed! Can't create mapping\n"));
-                               return NT_STATUS_NONE_MAPPED;
-                       }
-                       break;
-               default:
-                       /* invalid sid, let's just leave it unmapped */
-                       DEBUG(10, ("SID %s is UNKNOWN, skip mapping\n", sid_string_static(map->sid)));
+       switch (map->xid.type) {
+       case ID_TYPE_UID:
+               ret = idmap_allocate_uid(&map->xid);
+               if ( ! NT_STATUS_IS_OK(ret)) {
+                       /* can't allocate id, let's just leave it unmapped */
+                       DEBUG(2, ("uid allocation failed! Can't create mapping\n"));
                        return NT_STATUS_NONE_MAPPED;
                }
+               break;
+       case ID_TYPE_GID:
+               ret = idmap_allocate_gid(&map->xid);
+               if ( ! NT_STATUS_IS_OK(ret)) {
+                       /* can't allocate id, let's just leave it unmapped */
+                       DEBUG(2, ("gid allocation failed! Can't create mapping\n"));
+                       return NT_STATUS_NONE_MAPPED;
+               }
+               break;
+       default:
+               /* invalid sid, let's just leave it unmapped */
+               DEBUG(3,("idmap_new_mapping: Refusing to create a "
+                        "mapping for an unspecified ID type.\n"));             
+               return NT_STATUS_NONE_MAPPED;
+       }
 
-               /* ok, got a new id, let's set a mapping */
-               map->status = ID_MAPPED;
+       /* ok, got a new id, let's set a mapping */
+       map->status = ID_MAPPED;
 
-               DEBUG(10, ("Setting mapping: %s <-> %s %lu\n",
-                          sid_string_static(map->sid),
-                          (map->xid.type == ID_TYPE_UID) ? "UID" : "GID",
-                          (unsigned long)map->xid.id));
-               ret = dom->methods->set_mapping(dom, map);
+       DEBUG(10, ("Setting mapping: %s <-> %s %lu\n",
+                  sid_string_static(map->sid),
+                  (map->xid.type == ID_TYPE_UID) ? "UID" : "GID",
+                  (unsigned long)map->xid.id));
+       ret = dom->methods->set_mapping(dom, map);
 
-               if ( ! NT_STATUS_IS_OK(ret)) {
-                       /* something wrong here :-( */
-                       DEBUG(2, ("Failed to commit mapping\n!"));
+       if ( ! NT_STATUS_IS_OK(ret)) {
+               /* something wrong here :-( */
+               DEBUG(2, ("Failed to commit mapping\n!"));
 
-                       /* TODO: would it make sense to have an "unalloc_id function?" */
+               /* TODO: would it make sense to have an "unalloc_id function?" */
 
-                       return NT_STATUS_NONE_MAPPED;
-               }
-       } else {
-               DEBUG(2,("Invalid SID, not mapping %s (type %d)\n",
-                               sid_string_static(map->sid), sid_type));
                return NT_STATUS_NONE_MAPPED;
        }
 
@@ -1439,6 +1394,8 @@ void idmap_dump_maps(char *logfile)
                                (unsigned long)maps[i].xid.id,
                                sid_string_static(maps[i].sid));
                        break;
+               case ID_TYPE_NOT_SPECIFIED:
+                       break;
                }
        }
 
index 298d6fed355f364a60b9259485ace585893b3e7c..8e016879b8c9e170278d8161fae8c4158cbd7a0a 100644 (file)
@@ -37,7 +37,7 @@ struct idmap_rid_context {
   we support multiple domains in the new idmap
  *****************************************************************************/
 
-static NTSTATUS idmap_rid_initialize(struct idmap_domain *dom, const char *compat_params)
+static NTSTATUS idmap_rid_initialize(struct idmap_domain *dom)
 {
        NTSTATUS ret;
        struct idmap_rid_context *ctx;
@@ -86,9 +86,6 @@ failed:
 
 static NTSTATUS idmap_rid_id_to_sid(TALLOC_CTX *memctx, struct idmap_rid_context *ctx, struct id_map *map)
 {
-       const char *domname, *name;
-       enum lsa_SidType sid_type;
-       BOOL ret;
        struct winbindd_domain *domain; 
 
        /* apply filters before checking */
@@ -104,45 +101,9 @@ static NTSTATUS idmap_rid_id_to_sid(TALLOC_CTX *memctx, struct idmap_rid_context
        
        sid_compose(map->sid, &domain->sid, map->xid.id - ctx->low_id + ctx->base_rid);
 
-       /* by default calls to winbindd are disabled
-          the following call will not recurse so this is safe */
-       winbind_on();
-       ret = winbind_lookup_sid(memctx, map->sid, &domname, &name, &sid_type);
-       winbind_off();
-
-       if (ret) {
-               switch (sid_type) {
-               case SID_NAME_USER:
-                       if (map->xid.type != ID_TYPE_UID) {
-                               /* wrong type */
-                               map->status = ID_UNMAPPED;
-                               DEBUG(5, ("Resulting SID is of wrong ID type\n"));
-                               return NT_STATUS_NONE_MAPPED;
-                       }
-                       break;
-               case SID_NAME_DOM_GRP:
-               case SID_NAME_ALIAS:
-               case SID_NAME_WKN_GRP:
-                       if (map->xid.type != ID_TYPE_GID) {
-                               /* wrong type */
-                               map->status = ID_UNMAPPED;
-                               DEBUG(5, ("Resulting SID is of wrong ID type\n"));
-                               return NT_STATUS_NONE_MAPPED;
-                       }
-                       break;
-               default:
-                       /* invalid sid?? */
-                       map->status = ID_UNKNOWN;
-                       DEBUG(10, ("SID %s is UNKNOWN, skip mapping\n", sid_string_static(map->sid)));
-                       return NT_STATUS_NONE_MAPPED;
-               }
-       } else {
-               /* TODO: how do we known if the lookup was negative
-                * or something just failed? */
-               map->status = ID_UNMAPPED;
-               DEBUG(2, ("Failed: to resolve SID\n"));
-               return NT_STATUS_UNSUCCESSFUL;
-       }
+       /* We **really** should have some way of validating 
+          the SID exists and is the correct type here.  But 
+          that is a deficiency in the idmap_rid design. */
 
        map->status = ID_MAPPED;
 
@@ -155,46 +116,13 @@ static NTSTATUS idmap_rid_id_to_sid(TALLOC_CTX *memctx, struct idmap_rid_context
 
 static NTSTATUS idmap_rid_sid_to_id(TALLOC_CTX *memctx, struct idmap_rid_context *ctx, struct id_map *map)
 {
-       const char *domname, *name;
-       enum lsa_SidType sid_type;
        uint32_t rid;
-       BOOL ret;
 
        sid_peek_rid(map->sid, &rid);
        map->xid.id = rid - ctx->base_rid + ctx->low_id;
 
-       /* by default calls to winbindd are disabled
-          the following call will not recurse so this is safe */
-       winbind_on();
-       /* check if this is a valid SID and set the type */
-       ret = winbind_lookup_sid(memctx, map->sid, &domname, &name, &sid_type);
-       winbind_off();
-
-       if (ret) {
-               switch (sid_type) {
-               case SID_NAME_USER:
-                       map->xid.type = ID_TYPE_UID;
-                       break;
-               case SID_NAME_DOM_GRP:
-               case SID_NAME_ALIAS:
-               case SID_NAME_WKN_GRP:
-                       map->xid.type = ID_TYPE_GID;
-                       break;
-               default:
-                       /* invalid sid, let's just leave it unmapped */
-                       DEBUG(10, ("SID %s is UNKNOWN, skip mapping\n", sid_string_static(map->sid)));
-                       map->status = ID_UNKNOWN;
-                       return NT_STATUS_NONE_MAPPED;
-               }
-       } else {
-               /* TODO: how do we known if the lookup was negative
-                * or something just failed? */
-               map->status = ID_UNMAPPED;
-               DEBUG(2, ("Failed: to resolve SID\n"));
-               return NT_STATUS_UNSUCCESSFUL;
-       }
-
        /* apply filters before returning result */
+
        if ((map->xid.id < ctx->low_id) || (map->xid.id > ctx->high_id)) {
                DEBUG(5, ("Requested id (%u) out of range (%u - %u). Filtered!\n",
                                map->xid.id, ctx->low_id, ctx->high_id));
@@ -202,6 +130,10 @@ static NTSTATUS idmap_rid_sid_to_id(TALLOC_CTX *memctx, struct idmap_rid_context
                return NT_STATUS_NONE_MAPPED;
        }
 
+       /* We **really** should have some way of validating 
+          the SID exists and is the correct type here.  But 
+          that is a deficiency in the idmap_rid design. */
+
        map->status = ID_MAPPED;
 
        return NT_STATUS_OK;
index 540dafaa7346f3d169383944c62c59df9565e17d..40a5fb854bef275bd95985fb818d3ad771e3b115 100644 (file)
@@ -105,18 +105,24 @@ NTSTATUS idmap_sid_to_uid(DOM_SID *sid, uid_t *uid)
        DEBUG(10,("idmap_sid_to_uid: sid = [%s]\n", sid_string_static(sid)));
 
        map.sid = sid;
+       map.xid.type = ID_TYPE_UID;     
        
        maps[0] = &map;
        maps[1] = NULL;
        
        ret = idmap_sids_to_unixids(maps);
        if ( ! NT_STATUS_IS_OK(ret)) {
-               DEBUG(10, ("error mapping sid [%s] to uid\n", sid_string_static(sid)));
+               DEBUG(10, ("error mapping sid [%s] to uid\n", 
+                          sid_string_static(sid)));
                return ret;
        }
 
        if ((map.status != ID_MAPPED) || (map.xid.type != ID_TYPE_UID)) {
-               DEBUG(10, ("sid [%s] not mapped to an uid [%u,%u,%u]\n", sid_string_static(sid), map.status, map.xid.type, map.xid.id));
+               DEBUG(10, ("sid [%s] not mapped to an uid [%u,%u,%u]\n", 
+                          sid_string_static(sid), 
+                          map.status, 
+                          map.xid.type, 
+                          map.xid.id));
                return NT_STATUS_NONE_MAPPED;
        }
 
@@ -139,18 +145,24 @@ NTSTATUS idmap_sid_to_gid(DOM_SID *sid, gid_t *gid)
        DEBUG(10,("idmap_sid_to_gid: sid = [%s]\n", sid_string_static(sid)));
 
        map.sid = sid;
+       map.xid.type = ID_TYPE_GID;
        
        maps[0] = &map;
        maps[1] = NULL;
        
        ret = idmap_sids_to_unixids(maps);
        if ( ! NT_STATUS_IS_OK(ret)) {
-               DEBUG(10, ("error mapping sid [%s] to gid\n", sid_string_static(sid)));
+               DEBUG(10, ("error mapping sid [%s] to gid\n", 
+                          sid_string_static(sid)));
                return ret;
        }
 
        if ((map.status != ID_MAPPED) || (map.xid.type != ID_TYPE_GID)) {
-               DEBUG(10, ("sid [%s] not mapped to an gid [%u,%u,%u]\n", sid_string_static(sid), map.status, map.xid.type, map.xid.id));
+               DEBUG(10, ("sid [%s] not mapped to an gid [%u,%u,%u]\n", 
+                          sid_string_static(sid), 
+                          map.status, 
+                          map.xid.type, 
+                          map.xid.id));
                return NT_STATUS_NONE_MAPPED;
        }
 
index b17172f4f27a67775137f565ce86fafcd190cffe..fcedaebb273e9314c083103cf95c9485699af8fa 100644 (file)
@@ -248,7 +248,9 @@ static struct winbindd_dispatch_table {
        { WINBINDD_SID_TO_GID, winbindd_sid_to_gid, "SID_TO_GID" },
        { WINBINDD_UID_TO_SID, winbindd_uid_to_sid, "UID_TO_SID" },
        { WINBINDD_GID_TO_SID, winbindd_gid_to_sid, "GID_TO_SID" },
+#if 0   /* DISABLED until we fix the interface in Samba 3.0.26 --jerry */
        { WINBINDD_SIDS_TO_XIDS, winbindd_sids_to_unixids, "SIDS_TO_XIDS" },
+#endif  /* end DISABLED */
        { WINBINDD_ALLOCATE_UID, winbindd_allocate_uid, "ALLOCATE_UID" },
        { WINBINDD_ALLOCATE_GID, winbindd_allocate_gid, "ALLOCATE_GID" },
        { WINBINDD_SET_MAPPING, winbindd_set_mapping, "SET_MAPPING" },
@@ -1010,9 +1012,6 @@ int main(int argc, char **argv, char **envp)
 
        namecache_enable();
 
-       /* quick hack to avoid a loop in idmap, proper fix later */
-       reset_idmap_in_own_child();
-
        /* Winbind daemon initialisation */
 
        if ( ! NT_STATUS_IS_OK(idmap_init_cache()) ) {
index 20cf55b51d012e179c4fd8797d8013bc94c0a554..8d475e6c9f9e95e7a179ecf7c71b8fc035e4c049 100644 (file)
@@ -364,7 +364,9 @@ static struct winbindd_child_dispatch_table child_dispatch_table[] = {
        { WINBINDD_CHECK_MACHACC,        winbindd_dual_check_machine_acct,    "CHECK_MACHACC" },
        { WINBINDD_DUAL_SID2UID,         winbindd_dual_sid2uid,               "DUAL_SID2UID" },
        { WINBINDD_DUAL_SID2GID,         winbindd_dual_sid2gid,               "DUAL_SID2GID" },
+#if 0   /* DISABLED until we fix the interface in Samba 3.0.26 --jerry */
        { WINBINDD_DUAL_SIDS2XIDS,       winbindd_dual_sids2xids,             "DUAL_SIDS2XIDS" },
+#endif  /* end DISABLED */
        { WINBINDD_DUAL_UID2SID,         winbindd_dual_uid2sid,               "DUAL_UID2SID" },
        { WINBINDD_DUAL_GID2SID,         winbindd_dual_gid2sid,               "DUAL_GID2SID" },
        { WINBINDD_DUAL_UID2NAME,        winbindd_dual_uid2name,              "DUAL_UID2NAME" },
@@ -921,9 +923,6 @@ static BOOL fork_domain_child(struct winbindd_child *child)
                        child);
        }
 
-       /* quick hack to avoid a loop in idmap, proper fix later */
-       set_idmap_in_own_child();
-
        while (1) {
 
                int ret;