[PATCHES] winbindd: fix sid->xid for SID History SIDs
Uri Simchoni
uri at samba.org
Mon Mar 20 04:49:54 UTC 2017
Hi,
We few weeks ago we've discussed SID history and id-mapping -
https://lists.samba.org/archive/samba-technical/2017-February/118771.html.
Attached is a proposed initial fix for the issue, which focuses on
avoiding wrong results.
The fix finds the domain of the SID by resolving a SID with same domain
component and an RID of 513 (domain users), which hopefully never gets
migrated.
We've discussed other means such as smb.conf stuff or netsamlogon - I
think those methods can come on top of this method, because if they
don't work we should always fall back to something. The added resolving
doesn't cost much because it's in the same round-trip.
The key thing about this fix is that doesn't try to translate sid->xid
in any possible case (such as when old domain is gone and forgotten), it
just avoids getting the *wrong* result. As such, it's a good minimal fix
that can be applied to stable versions. For master, we can add the
smb.conf-based stuff, that will support more cases.
Review appreciated.
Thanks,
Uri.
-------------- next part --------------
From cf77907f4cfdb8154406dec7c4f3170fdb90d5fb Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Fri, 17 Mar 2017 00:18:08 +0200
Subject: [PATCH 1/4] add_sid_to_array() - support pre-allocated arrays
Only grow array if needed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
libcli/security/util_sid.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index ac44876..f88244e 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -343,6 +343,13 @@ NTSTATUS add_sid_to_array(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
return NT_STATUS_INTEGER_OVERFLOW;
}
+ if (*sids != NULL && talloc_array_length(*sids) >= (*num) + 1) {
+ sid_copy(&((*sids)[*num]), sid);
+ *num += 1;
+
+ return NT_STATUS_OK;
+ }
+
tmp = talloc_realloc(mem_ctx, *sids, struct dom_sid, (*num)+1);
if (tmp == NULL) {
*num = 0;
--
2.9.3
From d480d3738e9c41071100a3217a7d9524190a9f51 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Fri, 17 Mar 2017 23:45:31 +0200
Subject: [PATCH 2/4] introduce is_domain_principal_sid()
This routine indicates whether an SID belongs to a
domain, as opposed to "special", alias SIDs.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
libcli/security/dom_sid.h | 1 +
libcli/security/util_sid.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index bdcec94..df8724e 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -108,5 +108,6 @@ bool add_rid_to_array_unique(TALLOC_CTX *mem_ctx,
uint32_t rid, uint32_t **pp_rids, size_t *p_num);
bool is_null_sid(const struct dom_sid *sid);
+bool is_domain_principal_sid(const struct dom_sid *sid);
#endif /*_DOM_SID_H_*/
diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index f88244e..ba6c5c0 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -440,3 +440,17 @@ bool is_null_sid(const struct dom_sid *sid)
static const struct dom_sid null_sid = {0};
return dom_sid_equal(sid, &null_sid);
}
+
+/*
+ * This routine checks whether an SID belongs to
+ * a domain (as opposed to aliases of all sorts)
+ * See [MS-DTYP] 2.4.2.4
+ */
+bool is_domain_principal_sid(const struct dom_sid *sid)
+{
+ /* S-1-5-21 */
+ static const struct dom_sid non_unique =
+ { 1, 1, {0,0,0,0,0,5}, {21,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
+
+ return sid_compare_domain(sid, &non_unique) == 0;
+}
--
2.9.3
From 09b3140401a2cf2bba7e785ae2f5e06c9becbf90 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Fri, 17 Mar 2017 23:53:00 +0200
Subject: [PATCH 3/4] winbindd-sids2xids: resolve "domain users" of resolved
sids
When resolving a list of SIDs prior to sid->xid translation,
also resolve the "domain users" SID for each SID on the list.
The "domain users" shall be a more reliable source of domain
name than the SID itself, because the SID to be translated
might belong to an account that moved into another domain.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/winbindd/wb_sids2xids.c | 54 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index 9bb8fa8..9da08d2 100644
--- a/source3/winbindd/wb_sids2xids.c
+++ b/source3/winbindd/wb_sids2xids.c
@@ -25,6 +25,7 @@
#include "librpc/gen_ndr/ndr_winbind_c.h"
#include "librpc/gen_ndr/ndr_netlogon.h"
#include "lsa.h"
+#include "passdb/machine_sid.h"
struct wb_sids2xids_state {
struct tevent_context *ev;
@@ -36,6 +37,7 @@ struct wb_sids2xids_state {
struct dom_sid *non_cached;
uint32_t num_non_cached;
+ uint32_t num_to_resolve;
/*
* Domain array to use for the idmap call. The output from
@@ -60,6 +62,9 @@ struct wb_sids2xids_state {
static bool wb_sids2xids_in_cache(struct dom_sid *sid, struct id_map *map);
+static bool wb_add_domain_users_to_lookup(TALLOC_CTX *ctx,
+ struct dom_sid **sids,
+ uint32_t *num_sids);
static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq);
static void wb_sids2xids_done(struct tevent_req *subreq);
static void wb_sids2xids_gotdc(struct tevent_req *subreq);
@@ -125,8 +130,15 @@ struct tevent_req *wb_sids2xids_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
+ state->num_to_resolve = state->num_non_cached;
+ if (!wb_add_domain_users_to_lookup(state, &state->non_cached,
+ &state->num_to_resolve)) {
+ tevent_req_oom(req);
+ return tevent_req_post(req, ev);
+ }
+
subreq = wb_lookupsids_send(state, ev, state->non_cached,
- state->num_non_cached);
+ state->num_to_resolve);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
}
@@ -154,6 +166,46 @@ static bool wb_sids2xids_in_cache(struct dom_sid *sid, struct id_map *map)
return false;
}
+/*
+ * For each SID in the list (except for "special" ones),
+ * make sure the list also contains the corresponding
+ * "domain users" SID of the same domain. That will be
+ * the basis for determining the id-mapping domain.
+ */
+static bool wb_add_domain_users_to_lookup(TALLOC_CTX *ctx,
+ struct dom_sid **sids,
+ uint32_t *num_sids)
+{
+ uint32_t i, n = *num_sids;
+ struct dom_sid sid;
+ NTSTATUS status;
+
+ for (i = 0; i < n; ++i) {
+ sid_copy(&sid, &(*sids)[i]);
+ if (!is_domain_principal_sid(&sid)) {
+ /* "special" SID */
+ continue;
+ }
+
+ if (sid_check_is_in_our_sam(&sid)) {
+ /* no need to add "domain users"
+ * in our domain - the lookup is always
+ * correct for ID mapping.
+ */
+ continue;
+ }
+
+ sid_split_rid(&sid, NULL);
+ sid_append_rid(&sid, DOMAIN_RID_USERS);
+ status = add_sid_to_array_unique(ctx, &sid, sids, num_sids);
+ if (!NT_STATUS_IS_OK(status)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
static enum id_type lsa_SidType_to_id_type(const enum lsa_SidType sid_type);
static struct wbint_TransIDArray *wb_sids2xids_extract_for_domain_index(
TALLOC_CTX *mem_ctx, const struct wbint_TransIDArray *src,
--
2.9.3
From 3fcf7130a086cb3c6bc3dc444dd1010d8078ae29 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Fri, 17 Mar 2017 23:58:35 +0200
Subject: [PATCH 4/4] winbindd-sids2xids: reliably determine idmap backend name
For SIDs which belong to domains, determine the backend name
by the domain name of "domain users" SID from the same domain.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/winbindd/wb_sids2xids.c | 61 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index 9da08d2..ea0d9c7 100644
--- a/source3/winbindd/wb_sids2xids.c
+++ b/source3/winbindd/wb_sids2xids.c
@@ -207,6 +207,11 @@ static bool wb_add_domain_users_to_lookup(TALLOC_CTX *ctx,
}
static enum id_type lsa_SidType_to_id_type(const enum lsa_SidType sid_type);
+static const char *
+wb_get_idmap_backend_name(const struct lsa_RefDomainList *domains,
+ const struct lsa_TransNameArray *names,
+ const struct dom_sid *sids,
+ uint32_t idx);
static struct wbint_TransIDArray *wb_sids2xids_extract_for_domain_index(
TALLOC_CTX *mem_ctx, const struct wbint_TransIDArray *src,
uint32_t domain_index);
@@ -238,7 +243,8 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq)
for (i=0; i<state->num_non_cached; i++) {
struct dom_sid dom_sid;
- struct lsa_DomainInfo *info;
+ const char *dom_name = wb_get_idmap_backend_name(
+ domains, names, state->non_cached, i);
struct lsa_TranslatedName *n = &names->names[i];
struct wbint_TransID *t = &state->ids.ids[i];
int domain_index;
@@ -246,11 +252,10 @@ static void wb_sids2xids_lookupsids_done(struct tevent_req *subreq)
sid_copy(&dom_sid, &state->non_cached[i]);
sid_split_rid(&dom_sid, &t->rid);
- info = &domains->domains[n->sid_index];
t->type = lsa_SidType_to_id_type(n->sid_type);
domain_index = init_lsa_ref_domain_list(
- state, &state->idmap_doms, info->name.string, &dom_sid);
+ state, &state->idmap_doms, dom_name, &dom_sid);
if (domain_index == -1) {
tevent_req_oom(req);
return;
@@ -309,6 +314,56 @@ static enum id_type lsa_SidType_to_id_type(const enum lsa_SidType sid_type)
return type;
}
+static const char *
+wb_get_idmap_backend_name(const struct lsa_RefDomainList *domains,
+ const struct lsa_TransNameArray *names,
+ const struct dom_sid *sids,
+ uint32_t idx)
+{
+ struct dom_sid sid = sids[idx];
+ struct lsa_TranslatedName *n = &names->names[idx];
+ struct lsa_DomainInfo *info = NULL;
+ if (!is_domain_principal_sid(&sid)) {
+ /* "special" SID */
+ goto ret;
+ }
+
+ if (sid_check_is_in_our_sam(&sid)) {
+ /* our domain - the lookup is always
+ * correct for ID mapping.
+ */
+ goto ret;
+ }
+
+ sid_split_rid(&sid, NULL);
+ sid_append_rid(&sid, DOMAIN_RID_USERS);
+
+ for (idx = 0; idx < names->count; ++idx) {
+ if (!dom_sid_equal(&sid, &sids[idx])) {
+ continue;
+ }
+
+ /* has the DC been able to resolve
+ * "domain users" for same domain?
+ */
+ n = &names->names[idx];
+ if (lsa_SidType_to_id_type(n->sid_type) ==
+ ID_TYPE_NOT_SPECIFIED) {
+ /* default id-mapping backend */
+ return "";
+ }
+
+ goto ret;
+ }
+
+ /* we're not supposed to just fall off the loop... */
+ smb_panic("Have not found 'domain users' SID");
+
+ret:
+ info = &domains->domains[n->sid_index];
+ return info->name.string;
+}
+
static void wb_sids2xids_done(struct tevent_req *subreq)
{
struct tevent_req *req = tevent_req_callback_data(
--
2.9.3
More information about the samba-technical
mailing list