[PATCH] Patch for bug 13052
Ralph Böhme
slow at samba.org
Mon Oct 9 16:24:33 UTC 2017
Howdy!
In the xid to SID mapping function idmap_rid uses the trusted domain list to get
the SID for the mapping domain.
But the idmap child may lack trusted domains in the case when before trusted
domains enumeration finished a winbindd idmapping request came in that triggered
the idmap child fork.
When it forks, the idmap child inherits the trusted domain list of the parent
which is not yet complete. Even after the parent finishes trusted domain
enumeration, xid2sid idmapping requets will continue to fail, so a transient
error becomes a permanent one.
The fix is to pass the domain sid as an additional argument to the idmap xid2sid
mapping functions. To get the sid, we call lsalookupnames on the domain name of
all idmap mapping domains.
Please review&push if happy. The patchset just survived a private autobuild and
was reported by a customer to fix the problem.
Thanks!
-slow
-------------- next part --------------
From a30f7a21e83ea4836927a3343e2c8f0805e71088 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 25 Sep 2017 13:25:57 +0200
Subject: [PATCH 1/4] winbindd: add domain SID to idmap mapping domains
Fetch the domain SID for every domain in the idmap-domain map. This is
in preperation of passing the domain SID as an additional argument to
xid2sid requests to the idmap child.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13052
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/wb_xids2sids.c | 155 ++++++++++++++++++++++++++++++++++++----
1 file changed, 140 insertions(+), 15 deletions(-)
diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c
index 15e94c4203f..16928f57369 100644
--- a/source3/winbindd/wb_xids2sids.c
+++ b/source3/winbindd/wb_xids2sids.c
@@ -23,11 +23,13 @@
#include "idmap_cache.h"
#include "librpc/gen_ndr/ndr_winbind_c.h"
#include "librpc/gen_ndr/ndr_netlogon.h"
+#include "passdb/lookup_sid.h"
struct wb_xids2sids_dom_map {
unsigned low_id;
unsigned high_id;
const char *name;
+ struct dom_sid sid;
};
/*
@@ -93,6 +95,7 @@ static bool wb_xids2sids_add_dom(const char *domname,
dom_maps = tmp;
map = &dom_maps[num_maps];
+ ZERO_STRUCTP(map);
map->name = talloc_move(dom_maps, &name);
}
@@ -102,30 +105,131 @@ static bool wb_xids2sids_add_dom(const char *domname,
return false;
}
-static void wb_xids2sids_init_dom_maps(void)
+struct wb_xids2sids_init_dom_maps_state {
+ struct tevent_context *ev;
+ size_t dom_idx;
+};
+
+static void wb_xids2sids_init_dom_maps_lookupname_done(
+ struct tevent_req *subreq);
+
+static struct tevent_req *wb_xids2sids_init_dom_maps_send(
+ TALLOC_CTX *mem_ctx, struct tevent_context *ev)
{
- if (dom_maps != NULL) {
- return;
+ struct tevent_req *req = NULL;
+ struct tevent_req *subreq = NULL;
+ struct wb_xids2sids_init_dom_maps_state *state = NULL;
+
+ req = tevent_req_create(mem_ctx, &state,
+ struct wb_xids2sids_init_dom_maps_state);
+ if (req == NULL) {
+ return NULL;
}
+ *state = (struct wb_xids2sids_init_dom_maps_state) {
+ .ev = ev,
+ .dom_idx = 0,
+ };
+ if (dom_maps != NULL) {
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
/*
* Put the passdb idmap domain first. We always need to try
* there first.
*/
- dom_maps = talloc_array(NULL, struct wb_xids2sids_dom_map, 1);
- if (dom_maps == NULL) {
- return;
+ dom_maps = talloc_zero_array(NULL, struct wb_xids2sids_dom_map, 1);
+ if (tevent_req_nomem(dom_maps, req)) {
+ return tevent_req_post(req, ev);
}
dom_maps[0].low_id = 0;
dom_maps[0].high_id = UINT_MAX;
dom_maps[0].name = talloc_strdup(dom_maps, get_global_sam_name());
- if (dom_maps[0].name == NULL) {
+ if (tevent_req_nomem(dom_maps[0].name, req)) {
TALLOC_FREE(dom_maps);
- return;
+ return tevent_req_post(req, ev);
}
lp_scan_idmap_domains(wb_xids2sids_add_dom, NULL);
+
+ if (strequal(dom_maps[state->dom_idx].name, "*")) {
+ state->dom_idx++;
+ if (state->dom_idx == talloc_array_length(dom_maps)) {
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+ }
+
+ subreq = wb_lookupname_send(state,
+ state->ev,
+ dom_maps[state->dom_idx].name,
+ "",
+ LOOKUP_NAME_NO_NSS);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq,
+ wb_xids2sids_init_dom_maps_lookupname_done,
+ req);
+ return req;
+}
+
+static void wb_xids2sids_init_dom_maps_lookupname_done(
+ struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct wb_xids2sids_init_dom_maps_state *state = tevent_req_data(
+ req, struct wb_xids2sids_init_dom_maps_state);
+ enum lsa_SidType type;
+ NTSTATUS status;
+
+ /* We don't need the type, so don't store it in dom_maps */
+ status = wb_lookupname_recv(subreq,
+ &dom_maps[state->dom_idx].sid,
+ &type);
+ TALLOC_FREE(subreq);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_WARNING("Lookup domain name '%s' failed '%s'\n",
+ dom_maps[state->dom_idx].name,
+ nt_errstr(status));
+ } else {
+ DBG_DEBUG("Domain '%s' -> SID '%s'\n",
+ dom_maps[state->dom_idx].name,
+ sid_string_dbg(&dom_maps[state->dom_idx].sid));
+ }
+
+ state->dom_idx++;
+ if (state->dom_idx == talloc_array_length(dom_maps)) {
+ tevent_req_done(req);
+ return;
+ }
+
+ if (strequal(dom_maps[state->dom_idx].name, "*")) {
+ state->dom_idx++;
+ if (state->dom_idx == talloc_array_length(dom_maps)) {
+ tevent_req_done(req);
+ return;
+ }
+ }
+
+ subreq = wb_lookupname_send(state,
+ state->ev,
+ dom_maps[state->dom_idx].name,
+ "",
+ LOOKUP_NAME_NO_NSS);
+ if (tevent_req_nomem(subreq, req)) {
+ return;
+ }
+ tevent_req_set_callback(subreq,
+ wb_xids2sids_init_dom_maps_lookupname_done,
+ req);
+}
+
+static NTSTATUS wb_xids2sids_init_dom_maps_recv(struct tevent_req *req)
+{
+ return tevent_req_simple_recv_ntstatus(req);
}
struct wb_xids2sids_dom_state {
@@ -185,7 +289,6 @@ static struct tevent_req *wb_xids2sids_dom_send(
/* already mapped */
continue;
}
-
state->dom_xids[state->num_dom_xids++] = id;
}
@@ -321,6 +424,7 @@ struct wb_xids2sids_state {
};
static void wb_xids2sids_done(struct tevent_req *subreq);
+static void wb_xids2sids_init_dom_maps_done(struct tevent_req *subreq);
struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -329,7 +433,6 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx,
{
struct tevent_req *req, *subreq;
struct wb_xids2sids_state *state;
- size_t num_domains;
req = tevent_req_create(mem_ctx, &state,
struct wb_xids2sids_state);
@@ -371,22 +474,44 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx,
}
}
- wb_xids2sids_init_dom_maps();
- num_domains = talloc_array_length(dom_maps);
+ subreq = wb_xids2sids_init_dom_maps_send(
+ state, state->ev);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, wb_xids2sids_init_dom_maps_done, req);
+ return req;
+}
+static void wb_xids2sids_init_dom_maps_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct wb_xids2sids_state *state = tevent_req_data(
+ req, struct wb_xids2sids_state);
+ size_t num_domains;
+ NTSTATUS status;
+
+ status = wb_xids2sids_init_dom_maps_recv(subreq);
+ TALLOC_FREE(subreq);
+ if (tevent_req_nterror(req, status)) {
+ return;
+ }
+
+ num_domains = talloc_array_length(dom_maps);
if (num_domains == 0) {
tevent_req_done(req);
- return tevent_req_post(req, ev);
+ return;
}
subreq = wb_xids2sids_dom_send(
state, state->ev, &dom_maps[state->dom_idx],
state->xids, state->num_xids, state->sids);
if (tevent_req_nomem(subreq, req)) {
- return tevent_req_post(req, ev);
+ return;
}
tevent_req_set_callback(subreq, wb_xids2sids_done, req);
- return req;
+ return;
}
static void wb_xids2sids_done(struct tevent_req *subreq)
--
2.13.5
From 1221dc4d7fc1182a6da11f43176593fb99b42211 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 25 Sep 2017 15:39:39 +0200
Subject: [PATCH 2/4] winbindd: pass domain SID to wbint_UnixIDs2Sids
This makes the domain SID available to the idmap child for
wbint_UnixIDs2Sids mapping request. It's not used yet anywhere, this
comes in the next commit.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13052
Signed-off-by: Ralph Boehme <slow at samba.org>
---
librpc/idl/winbind.idl | 1 +
source3/include/idmap.h | 5 +++++
source3/winbindd/idmap.c | 4 +++-
source3/winbindd/idmap_proto.h | 3 ++-
source3/winbindd/wb_xids2sids.c | 5 +++--
source3/winbindd/winbindd_dual_srv.c | 3 ++-
6 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/librpc/idl/winbind.idl b/librpc/idl/winbind.idl
index 737d66abe70..f5e3507bff5 100644
--- a/librpc/idl/winbind.idl
+++ b/librpc/idl/winbind.idl
@@ -58,6 +58,7 @@ interface winbind
NTSTATUS wbint_UnixIDs2Sids(
[in,string,charset(UTF8)] char *domain_name,
+ [in] dom_sid domain_sid,
[in] uint32 num_ids,
[in,out] unixid xids[num_ids],
[out] dom_sid sids[num_ids]
diff --git a/source3/include/idmap.h b/source3/include/idmap.h
index 75d2e45b174..c0e88cc0ec1 100644
--- a/source3/include/idmap.h
+++ b/source3/include/idmap.h
@@ -37,6 +37,11 @@ struct wbint_userinfo;
struct idmap_domain {
const char *name;
+ /*
+ * dom_sid is currently only initialized in the unixids_to_sids request, so
+ * don't rely on this being filled out everywhere!
+ */
+ struct dom_sid dom_sid;
struct idmap_methods *methods;
NTSTATUS (*query_user)(struct idmap_domain *domain,
struct wbint_userinfo *info);
diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index 6e70b44c425..bfac7f86432 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -600,7 +600,8 @@ NTSTATUS idmap_allocate_gid(struct unixid *id)
}
NTSTATUS idmap_backend_unixids_to_sids(struct id_map **maps,
- const char *domain_name)
+ const char *domain_name,
+ struct dom_sid domain_sid)
{
struct idmap_domain *dom = NULL;
NTSTATUS status;
@@ -621,6 +622,7 @@ NTSTATUS idmap_backend_unixids_to_sids(struct id_map **maps,
return NT_STATUS_NONE_MAPPED;
}
+ dom->dom_sid = domain_sid;
status = dom->methods->unixids_to_sids(dom, maps);
DBG_DEBUG("unixid_to_sids for domain %s returned %s\n",
diff --git a/source3/winbindd/idmap_proto.h b/source3/winbindd/idmap_proto.h
index f4fc2c22739..a36d6c2f5bb 100644
--- a/source3/winbindd/idmap_proto.h
+++ b/source3/winbindd/idmap_proto.h
@@ -34,7 +34,8 @@ void idmap_close(void);
NTSTATUS idmap_allocate_uid(struct unixid *id);
NTSTATUS idmap_allocate_gid(struct unixid *id);
NTSTATUS idmap_backend_unixids_to_sids(struct id_map **maps,
- const char *domain_name);
+ const char *domain_name,
+ struct dom_sid domain_sid);
struct idmap_domain *idmap_find_domain(const char *domname);
/* The following definitions come from winbindd/idmap_nss.c */
diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c
index 16928f57369..307d2d5ee48 100644
--- a/source3/winbindd/wb_xids2sids.c
+++ b/source3/winbindd/wb_xids2sids.c
@@ -299,7 +299,7 @@ static struct tevent_req *wb_xids2sids_dom_send(
child = idmap_child();
subreq = dcerpc_wbint_UnixIDs2Sids_send(
- state, ev, child->binding_handle, dom_map->name,
+ state, ev, child->binding_handle, dom_map->name, dom_map->sid,
state->num_dom_xids, state->dom_xids, state->dom_sids);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
@@ -402,7 +402,8 @@ static void wb_xids2sids_dom_gotdc(struct tevent_req *subreq)
child = idmap_child();
subreq = dcerpc_wbint_UnixIDs2Sids_send(
state, state->ev, child->binding_handle, state->dom_map->name,
- state->num_dom_xids, state->dom_xids, state->dom_sids);
+ state->dom_map->sid,state->num_dom_xids,
+ state->dom_xids, state->dom_sids);
if (tevent_req_nomem(subreq, req)) {
return;
}
diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
index f79915c7e69..9fb15e9b0ab 100644
--- a/source3/winbindd/winbindd_dual_srv.c
+++ b/source3/winbindd/winbindd_dual_srv.c
@@ -230,7 +230,8 @@ NTSTATUS _wbint_UnixIDs2Sids(struct pipes_struct *p,
maps[i]->xid = r->in.xids[i];
}
- status = idmap_backend_unixids_to_sids(maps, r->in.domain_name);
+ status = idmap_backend_unixids_to_sids(maps, r->in.domain_name,
+ r->in.domain_sid);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(maps);
return status;
--
2.13.5
From e6ced856abf82a6d1dfe552419f22f21da4d2c71 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 25 Sep 2017 15:42:08 +0200
Subject: [PATCH 3/4] winbindd: idmap_rid: don't rely on the static domain list
The domain list in the idmap child is inherited from the parent winbindd
process and may not contain all domains in case enumerating trusted
domains didn't finish before the first winbind request that triggers the
idmap child fork comes along.
The previous commits added the domain SID as an additional argument to
the wbint_UnixIDs2Sids request, storing the domain SID in struct
idmap_domain.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13052
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/idmap_rid.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/source3/winbindd/idmap_rid.c b/source3/winbindd/idmap_rid.c
index 10088b5a27a..6ebb4a191a8 100644
--- a/source3/winbindd/idmap_rid.c
+++ b/source3/winbindd/idmap_rid.c
@@ -54,7 +54,6 @@ static NTSTATUS idmap_rid_initialize(struct idmap_domain *dom)
static NTSTATUS idmap_rid_id_to_sid(struct idmap_domain *dom, struct id_map *map)
{
- struct winbindd_domain *domain;
struct idmap_rid_context *ctx;
ctx = talloc_get_type(dom->private_data, struct idmap_rid_context);
@@ -66,12 +65,13 @@ static NTSTATUS idmap_rid_id_to_sid(struct idmap_domain *dom, struct id_map *map
return NT_STATUS_NONE_MAPPED;
}
- domain = find_domain_from_name_noinit(dom->name);
- if (domain == NULL ) {
+ if (is_null_sid(&dom->dom_sid)) {
+ DBG_INFO("idmap domain '%s' without SID\n", dom->name);
return NT_STATUS_NO_SUCH_DOMAIN;
}
- sid_compose(map->sid, &domain->sid, map->xid.id - dom->low_id + ctx->base_rid);
+ sid_compose(map->sid, &dom->dom_sid,
+ map->xid.id - dom->low_id + ctx->base_rid);
map->status = ID_MAPPED;
map->xid.type = ID_TYPE_BOTH;
--
2.13.5
From e8c03e676372ff097df537fdc71c04a8893b70ed Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 9 Oct 2017 13:29:05 +0200
Subject: [PATCH 4/4] winbindd: idmap_rid: error code for failing id-to-sid
mapping request
NT_STATUS_NO_SUCH_DOMAIN triggers complete request failure in the parent
winbindd. By returning NT_STATUS_NONE_MAPPED winbindd lets the individual
mapping fail but keeps processing any remaining mapping requests.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=13052
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/idmap_rid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_rid.c b/source3/winbindd/idmap_rid.c
index 6ebb4a191a8..b066ba3c50f 100644
--- a/source3/winbindd/idmap_rid.c
+++ b/source3/winbindd/idmap_rid.c
@@ -67,7 +67,7 @@ static NTSTATUS idmap_rid_id_to_sid(struct idmap_domain *dom, struct id_map *map
if (is_null_sid(&dom->dom_sid)) {
DBG_INFO("idmap domain '%s' without SID\n", dom->name);
- return NT_STATUS_NO_SUCH_DOMAIN;
+ return NT_STATUS_NONE_MAPPED;
}
sid_compose(map->sid, &dom->dom_sid,
--
2.13.5
More information about the samba-technical
mailing list