From 9b78af1f64015ae63948de565754ad8f6af66cbe Mon Sep 17 00:00:00 2001 From: Gerald Carter Date: Wed, 30 May 2007 19:47:35 +0000 Subject: r23244: Fix loop with nscd and NSS recusive calls. > Here's the problem I hit: > > getgrnam("foo") -> nscd -> NSS -> winbindd -> > winbindd_passdb.c:nam_to_sid() -> lookup_global_sam_name() -> > getgrnam("foo") -> nscd -> .... > > This is in the SAMBA_3_0 specifically but in theory could happen > SAMBA_3_0_25 (or 26) for an unknown group. > > The attached patch passes down enough state for the > name_to_sid() call to be able to determine the originating > winbindd cmd that came into the parent. So we can avoid > making more NSS calls if the original call came in trough NSS > so we don't deadlock ? But you should still service > lookupname() calls which are needed for example when > doing the token access checks for a "valid groups" from > smb.conf. > > I've got this in testing now. The problem has shown up with the > DsProvider on OS X and with nscd on SOlaris and Linux. (This used to be commit bcc8a3290aaa0d2620e9d391ffbbf65541f6d742) --- source3/include/smb.h | 15 +++++++++------ source3/nsswitch/winbindd.h | 1 + source3/nsswitch/winbindd_async.c | 6 ++++-- source3/nsswitch/winbindd_cache.c | 4 +++- source3/nsswitch/winbindd_group.c | 4 ++-- source3/nsswitch/winbindd_nss.h | 2 ++ source3/nsswitch/winbindd_passdb.c | 18 +++++++++++++++--- source3/nsswitch/winbindd_reconnect.c | 5 +++-- source3/nsswitch/winbindd_rpc.c | 11 ++++++----- source3/nsswitch/winbindd_sid.c | 3 ++- source3/nsswitch/winbindd_user.c | 3 ++- source3/nsswitch/winbindd_util.c | 4 +++- source3/passdb/lookup_sid.c | 8 ++++---- 13 files changed, 56 insertions(+), 28 deletions(-) (limited to 'source3') diff --git a/source3/include/smb.h b/source3/include/smb.h index abdb3f37ba..7a3b487f08 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -247,12 +247,15 @@ typedef uint64_t NTTIME; #define SID_MAX_SIZE ((size_t)(8+(MAXSUBAUTHS*4))) -#define LOOKUP_NAME_ISOLATED 1 /* Look up unqualified names */ -#define LOOKUP_NAME_REMOTE 2 /* Ask others */ -#define LOOKUP_NAME_ALL (LOOKUP_NAME_ISOLATED|LOOKUP_NAME_REMOTE) - -#define LOOKUP_NAME_GROUP 4 /* (unused) This is a NASTY hack for valid users = @foo - * where foo also exists in as user. */ +#define LOOKUP_NAME_ISOLATED 0x00000001 /* Look up unqualified names */ +#define LOOKUP_NAME_REMOTE 0x00000002 /* Ask others */ +#define LOOKUP_NAME_GROUP 0x00000004 /* (unused) This is a NASTY hack for + valid users = @foo where foo also + exists in as user. */ +#define LOOKUP_NAME_EXPLICIT 0x00000008 /* Only include + explicitly mapped names and not + the Unix {User,Group} domain */ +#define LOOKUP_NAME_ALL (LOOKUP_NAME_ISOLATED|LOOKUP_NAME_REMOTE) /** * @brief Security Identifier diff --git a/source3/nsswitch/winbindd.h b/source3/nsswitch/winbindd.h index e98c859405..3d71aff0cd 100644 --- a/source3/nsswitch/winbindd.h +++ b/source3/nsswitch/winbindd.h @@ -244,6 +244,7 @@ struct winbindd_methods { /* convert one user or group name to a sid */ NTSTATUS (*name_to_sid)(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, + enum winbindd_cmd orig_cmd, const char *domain_name, const char *name, DOM_SID *sid, diff --git a/source3/nsswitch/winbindd_async.c b/source3/nsswitch/winbindd_async.c index b674ef6cee..dbe4ad012b 100644 --- a/source3/nsswitch/winbindd_async.c +++ b/source3/nsswitch/winbindd_async.c @@ -952,6 +952,7 @@ void winbindd_lookupname_async(TALLOC_CTX *mem_ctx, void (*cont)(void *private_data, BOOL success, const DOM_SID *sid, enum lsa_SidType type), + enum winbindd_cmd orig_cmd, void *private_data) { struct winbindd_request request; @@ -966,6 +967,7 @@ void winbindd_lookupname_async(TALLOC_CTX *mem_ctx, ZERO_STRUCT(request); request.cmd = WINBINDD_LOOKUPNAME; + request.original_cmd = orig_cmd; fstrcpy(request.data.name.dom_name, dom_name); fstrcpy(request.data.name.name, name); @@ -977,7 +979,7 @@ void winbindd_lookupname_async(TALLOC_CTX *mem_ctx, s->dom_name = talloc_strdup( s, dom_name ); s->name = talloc_strdup( s, name ); - s->caller_private_data = private_data; + s->caller_private_data = private_data; do_async_domain(mem_ctx, domain, &request, lookupname_recv, (void *)cont, s); @@ -1012,7 +1014,7 @@ enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain, name_domain, lp_winbind_separator(), name_user)); /* Lookup name from DC using lsa_lookup_names() */ - if (!winbindd_lookup_sid_by_name(state->mem_ctx, domain, name_domain, + if (!winbindd_lookup_sid_by_name(state->mem_ctx, state->request.original_cmd, domain, name_domain, name_user, &sid, &type)) { return WINBINDD_ERROR; } diff --git a/source3/nsswitch/winbindd_cache.c b/source3/nsswitch/winbindd_cache.c index aa38bb6971..f9b045e10c 100644 --- a/source3/nsswitch/winbindd_cache.c +++ b/source3/nsswitch/winbindd_cache.c @@ -1355,6 +1355,7 @@ skip_save: /* convert a single name to a sid in a domain */ static NTSTATUS name_to_sid(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, + enum winbindd_cmd orig_cmd, const char *domain_name, const char *name, DOM_SID *sid, @@ -1402,7 +1403,8 @@ do_query: DEBUG(10,("name_to_sid: [Cached] - doing backend query for name for domain %s\n", domain->name )); - status = domain->backend->name_to_sid(domain, mem_ctx, domain_name, name, sid, type); + status = domain->backend->name_to_sid(domain, mem_ctx, orig_cmd, + domain_name, name, sid, type); /* and save it */ refresh_sequence_number(domain, False); diff --git a/source3/nsswitch/winbindd_group.c b/source3/nsswitch/winbindd_group.c index 1500324583..05aeef2612 100644 --- a/source3/nsswitch/winbindd_group.c +++ b/source3/nsswitch/winbindd_group.c @@ -529,7 +529,7 @@ void winbindd_getgrnam(struct winbindd_cli_state *state) ws_name_replace( name_group, WB_REPLACE_CHAR ); winbindd_lookupname_async( state->mem_ctx, domain->name, name_group, - getgrnam_recv, state ); + getgrnam_recv, WINBINDD_GETGRNAM, state ); } struct getgrsid_state { @@ -1324,7 +1324,7 @@ void winbindd_getgroups(struct winbindd_cli_state *state) /* Get rid and name type from name. The following costs 1 packet */ winbindd_lookupname_async(state->mem_ctx, s->domname, s->username, - getgroups_usersid_recv, s); + getgroups_usersid_recv, WINBINDD_GETGROUPS, s); } static void getgroups_usersid_recv(void *private_data, BOOL success, diff --git a/source3/nsswitch/winbindd_nss.h b/source3/nsswitch/winbindd_nss.h index 96c4b1161c..746b98bf79 100644 --- a/source3/nsswitch/winbindd_nss.h +++ b/source3/nsswitch/winbindd_nss.h @@ -230,6 +230,8 @@ typedef struct winbindd_gr { struct winbindd_request { uint32 length; enum winbindd_cmd cmd; /* Winbindd command to execute */ + enum winbindd_cmd original_cmd; /* Original Winbindd command + issued to parent process */ pid_t pid; /* pid of calling process */ uint32 flags; /* flags relavant to a given request */ fstring domain_name; /* name of domain for which the request applies */ diff --git a/source3/nsswitch/winbindd_passdb.c b/source3/nsswitch/winbindd_passdb.c index 2a61908f0e..8b8c4c66c6 100644 --- a/source3/nsswitch/winbindd_passdb.c +++ b/source3/nsswitch/winbindd_passdb.c @@ -93,16 +93,28 @@ static NTSTATUS enum_local_groups(struct winbindd_domain *domain, /* convert a single name to a sid in a domain */ static NTSTATUS name_to_sid(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, + enum winbindd_cmd original_cmd, const char *domain_name, const char *name, DOM_SID *sid, enum lsa_SidType *type) { + uint32 flags = LOOKUP_NAME_ALL; + + switch ( original_cmd ) { + case WINBINDD_LOOKUPNAME: + /* This call is ok */ + break; + default: + /* Avoid any NSS calls in the lookup_name by default */ + flags |= LOOKUP_NAME_EXPLICIT; + DEBUG(10,("winbindd_passdb: limiting name_to_sid() to explicit mappings\n")); + break; + } + DEBUG(10, ("Finding name %s\n", name)); - if ( !lookup_name( mem_ctx, name, LOOKUP_NAME_ALL, - NULL, NULL, sid, type ) ) - { + if ( !lookup_name( mem_ctx, name, flags, NULL, NULL, sid, type ) ) { return NT_STATUS_NONE_MAPPED; } diff --git a/source3/nsswitch/winbindd_reconnect.c b/source3/nsswitch/winbindd_reconnect.c index 6b484bdd06..70e5579109 100644 --- a/source3/nsswitch/winbindd_reconnect.c +++ b/source3/nsswitch/winbindd_reconnect.c @@ -84,6 +84,7 @@ static NTSTATUS enum_local_groups(struct winbindd_domain *domain, /* convert a single name to a sid in a domain */ static NTSTATUS name_to_sid(struct winbindd_domain *domain, TALLOC_CTX *mem_ctx, + enum winbindd_cmd orig_cmd, const char *domain_name, const char *name, DOM_SID *sid, @@ -91,12 +92,12 @@ static NTSTATUS name_to_sid(struct winbindd_domain *domain, { NTSTATUS result; - result = msrpc_methods.name_to_sid(domain, mem_ctx, + result = msrpc_methods.name_to_sid(domain, mem_ctx, orig_cmd, domain_name, name, sid, type); if (NT_STATUS_EQUAL(result, NT_STATUS_UNSUCCESSFUL)) - result = msrpc_methods.name_to_sid(domain, mem_ctx, + result = msrpc_methods.name_to_sid(domain, mem_ctx, orig_cmd, domain_name, name, sid, type); diff --git a/source3/nsswitch/winbindd_rpc.c b/source3/nsswitch/winbindd_rpc.c index 4a6448348e..03aa42012e 100644 --- a/source3/nsswitch/winbindd_rpc.c +++ b/source3/nsswitch/winbindd_rpc.c @@ -255,11 +255,12 @@ static NTSTATUS enum_local_groups(struct winbindd_domain *domain, /* convert a single name to a sid in a domain */ NTSTATUS msrpc_name_to_sid(struct winbindd_domain *domain, - TALLOC_CTX *mem_ctx, - const char *domain_name, - const char *name, - DOM_SID *sid, - enum lsa_SidType *type) + TALLOC_CTX *mem_ctx, + enum winbindd_cmd original_cmd, + const char *domain_name, + const char *name, + DOM_SID *sid, + enum lsa_SidType *type) { NTSTATUS result; DOM_SID *sids = NULL; diff --git a/source3/nsswitch/winbindd_sid.c b/source3/nsswitch/winbindd_sid.c index 881aa1af79..147917f978 100644 --- a/source3/nsswitch/winbindd_sid.c +++ b/source3/nsswitch/winbindd_sid.c @@ -103,7 +103,8 @@ void winbindd_lookupname(struct winbindd_cli_state *state) name_domain, lp_winbind_separator(), name_user)); winbindd_lookupname_async(state->mem_ctx, name_domain, name_user, - lookupname_recv, state); + lookupname_recv, WINBINDD_LOOKUPNAME, + state); } static void lookupname_recv(void *private_data, BOOL success, diff --git a/source3/nsswitch/winbindd_user.c b/source3/nsswitch/winbindd_user.c index 48b470cdd0..ef37e86dec 100644 --- a/source3/nsswitch/winbindd_user.c +++ b/source3/nsswitch/winbindd_user.c @@ -400,7 +400,8 @@ void winbindd_getpwnam(struct winbindd_cli_state *state) /* Get rid and name type from name. The following costs 1 packet */ winbindd_lookupname_async(state->mem_ctx, domname, username, - getpwnam_name2sid_recv, state); + getpwnam_name2sid_recv, WINBINDD_GETPWNAM, + state); } static void getpwnam_name2sid_recv(void *private_data, BOOL success, diff --git a/source3/nsswitch/winbindd_util.c b/source3/nsswitch/winbindd_util.c index c3f8e91734..082ec4440b 100644 --- a/source3/nsswitch/winbindd_util.c +++ b/source3/nsswitch/winbindd_util.c @@ -941,6 +941,7 @@ struct winbindd_domain *find_lookup_domain_from_name(const char *domain_name) /* Lookup a sid in a domain from a name */ BOOL winbindd_lookup_sid_by_name(TALLOC_CTX *mem_ctx, + enum winbindd_cmd orig_cmd, struct winbindd_domain *domain, const char *domain_name, const char *name, DOM_SID *sid, @@ -949,7 +950,8 @@ BOOL winbindd_lookup_sid_by_name(TALLOC_CTX *mem_ctx, NTSTATUS result; /* Lookup name */ - result = domain->methods->name_to_sid(domain, mem_ctx, domain_name, name, sid, type); + result = domain->methods->name_to_sid(domain, mem_ctx, orig_cmd, + domain_name, name, sid, type); /* Return sid and type if lookup successful */ if (!NT_STATUS_IS_OK(result)) { diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c index dbc0e75e59..dd7ffa8d81 100644 --- a/source3/passdb/lookup_sid.c +++ b/source3/passdb/lookup_sid.c @@ -102,7 +102,7 @@ BOOL lookup_name(TALLOC_CTX *mem_ctx, goto ok; } - if (strequal(domain, unix_users_domain_name())) { + if (!(flags & LOOKUP_NAME_EXPLICIT) && strequal(domain, unix_users_domain_name())) { if (lookup_unix_user_name(name, &sid)) { type = SID_NAME_USER; goto ok; @@ -111,7 +111,7 @@ BOOL lookup_name(TALLOC_CTX *mem_ctx, return False; } - if (strequal(domain, unix_groups_domain_name())) { + if (!(flags & LOOKUP_NAME_EXPLICIT) && strequal(domain, unix_groups_domain_name())) { if (lookup_unix_group_name(name, &sid)) { type = SID_NAME_DOM_GRP; goto ok; @@ -262,13 +262,13 @@ BOOL lookup_name(TALLOC_CTX *mem_ctx, /* 11. Ok, windows would end here. Samba has two more options: Unmapped users and unmapped groups */ - if (lookup_unix_user_name(name, &sid)) { + if (!(flags & LOOKUP_NAME_EXPLICIT) && lookup_unix_user_name(name, &sid)) { domain = talloc_strdup(tmp_ctx, unix_users_domain_name()); type = SID_NAME_USER; goto ok; } - if (lookup_unix_group_name(name, &sid)) { + if (!(flags & LOOKUP_NAME_EXPLICIT) && lookup_unix_group_name(name, &sid)) { domain = talloc_strdup(tmp_ctx, unix_groups_domain_name()); type = SID_NAME_DOM_GRP; goto ok; -- cgit