From 6c50e815c5a3b8d50fde09a1f662c6535a9ca0d7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 18:40:04 +0200 Subject: [PATCH 1/3] Revert "s3-winbind: Fix paranoia checks in winbindd_samr.c." This reverts commit 207a84d725b905c2b119d2ef0f4f4d4eb391140d. This is the wrong fix for the problem, see bug #8215. metze (cherry picked from commit 283f8a7fb5089a7126f07e26315fd06ab59997d8) --- source3/winbindd/winbindd_samr.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c index 98997b6..ee5ff08 100644 --- a/source3/winbindd/winbindd_samr.c +++ b/source3/winbindd/winbindd_samr.c @@ -762,8 +762,8 @@ static NTSTATUS sam_rids_to_names(struct winbindd_domain *domain, ZERO_STRUCT(lsa_policy); /* Paranoia check */ - if (!sid_check_is_in_builtin(domain_sid) && - !sid_check_is_in_our_domain(domain_sid) && + if (!sid_check_is_builtin(domain_sid) && + !sid_check_is_domain(domain_sid) && !sid_check_is_unix_users(domain_sid) && !sid_check_is_unix_groups(domain_sid) && !sid_check_is_in_wellknown_domain(domain_sid)) { -- 1.7.4.1 From 96b988cf6ea5c66cbb8c945f5ac103adb7d8da7b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 18:16:15 +0200 Subject: [PATCH 2/3] s3:wb_lookupsids: don't ignore 'result' and check if we got useable values The wrong fix for bug #8215 discovered this bug, as it caused sam_rids_to_names() to always return NT_STATUS_NONE_MAPPED. metze (cherry picked from commit 85809ccbe3a79f307af1fdd227f33b899d8db1b4) --- source3/winbindd/wb_lookupsids.c | 52 +++++++++++++++++++++++++++++++++++--- 1 files changed, 48 insertions(+), 4 deletions(-) diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c index 05601ad..9998b1b 100644 --- a/source3/winbindd/wb_lookupsids.c +++ b/source3/winbindd/wb_lookupsids.c @@ -428,6 +428,7 @@ static void wb_lookupsids_done(struct tevent_req *subreq) req, struct wb_lookupsids_state); struct wb_lookupsids_domain *d; uint32_t i; + bool fallback = false; NTSTATUS status, result; @@ -437,13 +438,31 @@ static void wb_lookupsids_done(struct tevent_req *subreq) return; } + d = &state->domains[state->domains_done]; + + if (NT_STATUS_IS_ERR(result)) { + fallback = true; + } else if (state->tmp_names.count != d->sids.num_sids) { + fallback = true; + } + + if (fallback) { + for (i=0; i < d->sids.num_sids; i++) { + uint32_t res_sid_index = d->sid_indexes[i]; + + state->single_sids[state->num_single_sids] = + res_sid_index; + state->num_single_sids += 1; + } + state->domains_done += 1; + wb_lookupsids_next(req, state); + return; + } + /* - * Ignore "result" here. We depend on the individual states in - * the translated names. + * Look at the individual states in the translated names. */ - d = &state->domains[state->domains_done]; - for (i=0; itmp_names.count; i++) { uint32_t res_sid_index = d->sid_indexes[i]; @@ -544,6 +563,7 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req *subreq) NTSTATUS status, result; struct wb_lookupsids_domain *d; uint32_t i; + bool fallback = false; status = dcerpc_wbint_LookupRids_recv(subreq, state, &result); TALLOC_FREE(subreq); @@ -552,6 +572,30 @@ static void wb_lookupsids_lookuprids_done(struct tevent_req *subreq) } d = &state->domains[state->domains_done]; + + if (NT_STATUS_IS_ERR(result)) { + fallback = true; + } else if (state->rid_names.num_principals != d->sids.num_sids) { + fallback = true; + } + + if (fallback) { + for (i=0; i < d->sids.num_sids; i++) { + uint32_t res_sid_index = d->sid_indexes[i]; + + state->single_sids[state->num_single_sids] = + res_sid_index; + state->num_single_sids += 1; + } + state->domains_done += 1; + wb_lookupsids_next(req, state); + return; + } + + /* + * Look at the individual states in the translated names. + */ + sid_copy(&src_domain_sid, get_global_sam_sid()); src_domain.name.string = get_global_sam_name(); src_domain.sid = &src_domain_sid; -- 1.7.4.1 From 39e9995ae9ac5c608603d1219093577757fc2469 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 18:25:15 +0200 Subject: [PATCH 3/3] s3:wb_lookupsids: add some paranoia checks to wb_lookupsids_recv() This hopefully catches future bugs. metze Autobuild-User: Stefan Metzmacher Autobuild-Date: Thu Jun 16 19:50:16 CEST 2011 on sn-devel-104 (cherry picked from commit 5961852d9c0e5cf64cea988586d610af9d63d487) --- source3/winbindd/wb_lookupsids.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c index 9998b1b..bf2ddb3 100644 --- a/source3/winbindd/wb_lookupsids.c +++ b/source3/winbindd/wb_lookupsids.c @@ -639,6 +639,24 @@ NTSTATUS wb_lookupsids_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, if (tevent_req_is_nterror(req, &status)) { return status; } + + /* + * The returned names need to match the given sids, + * if not we have a bug in the code! + * + */ + SMB_ASSERT(state->res_names->count == state->num_sids); + + /* + * Not strictly needed, but it might make debugging in the callers + * easier in future, if the talloc_array_length() returns the + * expected result... + */ + state->res_domains->domains = talloc_realloc(state->res_domains, + state->res_domains->domains, + struct lsa_DomainInfo, + state->res_domains->count); + *domains = talloc_move(mem_ctx, &state->res_domains); *names = talloc_move(mem_ctx, &state->res_names); return NT_STATUS_OK; -- 1.7.4.1