From 36da6cb5847df2754e8f9223e0784da6013c572b Mon Sep 17 00:00:00 2001 From: Gerald Carter Date: Thu, 19 Apr 2007 22:26:09 +0000 Subject: r22390: Patchset sent to samba-technical to address the winbind 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. (This used to be commit 3b24dae9e73b244540a68b631b428a4d0f57440b) --- source3/auth/auth_util.c | 48 ++++------------- source3/include/idmap.h | 6 +++ source3/include/smb.h | 3 +- source3/nsswitch/idmap.c | 111 ++++++++++++--------------------------- source3/nsswitch/idmap_rid.c | 86 ++++-------------------------- source3/nsswitch/idmap_util.c | 20 +++++-- source3/nsswitch/winbindd.c | 5 +- source3/nsswitch/winbindd_dual.c | 5 +- 8 files changed, 81 insertions(+), 203 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 0de3bf2325..336daa906d 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -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; iptok->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); diff --git a/source3/include/idmap.h b/source3/include/idmap.h index 472358a230..f4926f1e5c 100644 --- a/source3/include/idmap.h +++ b/source3/include/idmap.h @@ -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); diff --git a/source3/include/smb.h b/source3/include/smb.h index f85201ed30..6c44db5a34 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -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 }; diff --git a/source3/nsswitch/idmap.c b/source3/nsswitch/idmap.c index 82b8f3d592..530e03089d 100644 --- a/source3/nsswitch/idmap.c +++ b/source3/nsswitch/idmap.c @@ -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; } } diff --git a/source3/nsswitch/idmap_rid.c b/source3/nsswitch/idmap_rid.c index 298d6fed35..8e016879b8 100644 --- a/source3/nsswitch/idmap_rid.c +++ b/source3/nsswitch/idmap_rid.c @@ -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; diff --git a/source3/nsswitch/idmap_util.c b/source3/nsswitch/idmap_util.c index 540dafaa73..40a5fb854b 100644 --- a/source3/nsswitch/idmap_util.c +++ b/source3/nsswitch/idmap_util.c @@ -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] = ↦ 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] = ↦ 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; } diff --git a/source3/nsswitch/winbindd.c b/source3/nsswitch/winbindd.c index b17172f4f2..fcedaebb27 100644 --- a/source3/nsswitch/winbindd.c +++ b/source3/nsswitch/winbindd.c @@ -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()) ) { diff --git a/source3/nsswitch/winbindd_dual.c b/source3/nsswitch/winbindd_dual.c index 20cf55b51d..8d475e6c9f 100644 --- a/source3/nsswitch/winbindd_dual.c +++ b/source3/nsswitch/winbindd_dual.c @@ -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; -- cgit