[PATCHES] winbindd: fix sid->xid for SID History SIDs
Uri Simchoni
uri at samba.org
Fri Apr 7 18:23:43 UTC 2017
On 04/07/2017 08:32 PM, Ralph Böhme wrote:
> On Mon, Apr 03, 2017 at 07:50:08AM +0300, Uri Simchoni via samba-technical wrote:
>> Hi,
>>
>> Here's a V2 of the patch set.
>
> just fyi, I have this on my list.
>
> -slow
>
Thanks.
Here's a rebased patch set, with 4/6 adapted to the recent change in
wb_lookupsids_bulk(). Otherwise the same.
Uri.
-------------- next part --------------
From ea9974482d4792d6c01b834bd083c0e79e39b6b6 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 v3 1/6] 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 0709a7a..0e42eb4 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -344,6 +344,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 f0c7d14e9a7c1efc66196907f94e9706979718d8 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Wed, 29 Mar 2017 21:59:40 +0300
Subject: [PATCH v3 2/6] libcli: introduce extend_sid_array()
This function adds the elements of array2 to
the back of array1.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
libcli/security/dom_sid.h | 5 +++++
libcli/security/util_sid.c | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index bdcec94..735806d 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -102,6 +102,11 @@ NTSTATUS add_sid_to_array(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
struct dom_sid **sids, uint32_t *num);
NTSTATUS add_sid_to_array_unique(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
struct dom_sid **sids, uint32_t *num_sids);
+NTSTATUS extend_sid_array(TALLOC_CTX *mem_ctx,
+ struct dom_sid **array1,
+ uint32_t *num_sids,
+ const struct dom_sid *array2,
+ uint32_t num_extend);
void del_sid_from_array(const struct dom_sid *sid, struct dom_sid **sids,
uint32_t *num);
bool add_rid_to_array_unique(TALLOC_CTX *mem_ctx,
diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index 0e42eb4..a6af017 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -384,6 +384,30 @@ NTSTATUS add_sid_to_array_unique(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
}
/********************************************************************
+ Add SIDs of array2 to the back of array1.
+********************************************************************/
+
+NTSTATUS extend_sid_array(TALLOC_CTX *mem_ctx,
+ struct dom_sid **array1,
+ uint32_t *num_sids,
+ const struct dom_sid *array2,
+ uint32_t num_extend)
+{
+ uint32_t i;
+ NTSTATUS status;
+
+ for (i = 0; i < num_extend; ++i) {
+ status =
+ add_sid_to_array(mem_ctx, &array2[i], array1, num_sids);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ }
+
+ return NT_STATUS_OK;
+}
+
+/********************************************************************
Remove SID from an array
********************************************************************/
--
2.9.3
From 9c0b9e177ecd1bc1339150003bef6fd0a144420d 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 v3 3/6] libcli: introduce is_domain[_principa]l_sid()
Add routines to indicate 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 | 3 +++
libcli/security/util_sid.c | 20 ++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index 735806d..ee0f67a 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -113,5 +113,8 @@ 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_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 a6af017..69ee521 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -93,6 +93,9 @@ const struct dom_sid global_sid_Anonymous = /* Anonymous login */
/* S-1-5-9 */
const struct dom_sid global_sid_Enterprise_DCs = /* Enterprise DCs */
{ 1, 1, {0,0,0,0,0,5}, {9,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
+/* S-1-5-21 */
+const struct dom_sid global_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}};
/* S-1-5-32 */
const struct dom_sid global_sid_Builtin = /* Local well-known domain */
{ 1, 1, {0,0,0,0,0,5}, {32,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
@@ -465,3 +468,20 @@ bool is_null_sid(const struct dom_sid *sid)
static const struct dom_sid null_sid = {0};
return dom_sid_equal(sid, &null_sid);
}
+
+/*
+ * The following couple of routines check whether an SID belongs to
+ * a domain or is a domain SID (as opposed to aliases of all sorts)
+ * See [MS-DTYP] 2.4.2.4
+ */
+bool is_domain_sid(const struct dom_sid *sid)
+{
+ return sid_compare_domain(sid, &global_sid_non_unique) == 0 &&
+ sid->num_auths == 4;
+}
+
+bool is_domain_principal_sid(const struct dom_sid *sid)
+{
+ return sid_compare_domain(sid, &global_sid_non_unique) == 0 &&
+ sid->num_auths == 5;
+}
--
2.9.3
From aa8bde910251aced98225ebc883277ffe97403bc Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Sun, 2 Apr 2017 00:04:32 +0300
Subject: [PATCH v3 4/6] winbindd: bulk-resolve domain SIDs
This change changes SID resolving as follows:
1. Limit bulk sid resolution to SIDs belonging to a
domain (according to the comment in the source that
was the original intent)
2. Add domain sids to bulk resolution - DCs seem to be
able to handle this (verified using "rpcclient lookupsids")
Adding the domain SIDs means adding them to the list of SIDs
to be resolved incurs little overhead, which is important for
a change-to-come.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/winbindd/wb_lookupsids.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/source3/winbindd/wb_lookupsids.c b/source3/winbindd/wb_lookupsids.c
index c13bd5b..ff6d07d 100644
--- a/source3/winbindd/wb_lookupsids.c
+++ b/source3/winbindd/wb_lookupsids.c
@@ -250,7 +250,7 @@ static bool wb_lookupsids_next(struct tevent_req *req,
static bool wb_lookupsids_bulk(const struct dom_sid *sid)
{
- if (sid->num_auths != 5) {
+ if (!is_domain_sid(sid) && !is_domain_principal_sid(sid)) {
/*
* Only do "S-1-5-21-x-y-z-rid" domains via bulk
* lookup
@@ -283,22 +283,6 @@ static bool wb_lookupsids_bulk(const struct dom_sid *sid)
return false;
}
- if (sid_check_is_in_unix_groups(sid) ||
- sid_check_is_unix_groups(sid) ||
- sid_check_is_in_unix_users(sid) ||
- sid_check_is_unix_users(sid) ||
- sid_check_is_in_builtin(sid) ||
- sid_check_is_builtin(sid) ||
- sid_check_is_wellknown_domain(sid, NULL) ||
- sid_check_is_in_wellknown_domain(sid))
- {
- /*
- * These are locally done piece by piece anyway, no
- * need for bulk optimizations.
- */
- return false;
- }
-
/*
* All other SIDs are sent to the DC we're connected to as
* member via a single lsa_lookupsids call.
--
2.9.3
From 160913cc1c93e8638f2cb9da9f9d1e55b77ab9c3 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 v3 5/6] winbindd-sids2xids: resolve domain sids of resolved
sids
When resolving a list of SIDs prior to sid->xid translation,
also resolve the domain SID for each SID on the list.
The domain SID 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 | 63 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index 9bb8fa8..f3add28 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_sids_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_sids_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,55 @@ 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 SID of the same domain. That will be
+ * the basis for determining the id-mapping domain.
+ *
+ * Each domain SID is added exactly
+ * once (that is, even if it had been on the list
+ * before calling this routine).
+ */
+static bool wb_add_domain_sids_to_lookup(TALLOC_CTX *ctx,
+ struct dom_sid **sids,
+ uint32_t *num_sids)
+{
+ uint32_t i, n = *num_sids;
+ uint32_t num_dom_sids = 0;
+ struct dom_sid sid;
+ struct dom_sid *dom_sids = NULL;
+ NTSTATUS status;
+
+ for (i = 0; i < n; ++i) {
+ sid_copy(&sid, &(*sids)[i]);
+ if (!is_domain_principal_sid(&sid)) {
+ /* "special" or domain SID */
+ continue;
+ }
+
+ if (sid_check_is_in_our_sam(&sid)) {
+ /* no need to add our domain -
+ * the lookup is always
+ * correct for ID mapping.
+ */
+ continue;
+ }
+
+ sid_split_rid(&sid, NULL);
+ status =
+ add_sid_to_array_unique(ctx, &sid, &dom_sids, &num_dom_sids);
+ if (!NT_STATUS_IS_OK(status)) {
+ TALLOC_FREE(dom_sids);
+ return false;
+ }
+ }
+
+ status = extend_sid_array(ctx, sids, num_sids, dom_sids, num_dom_sids);
+ TALLOC_FREE(dom_sids);
+ return NT_STATUS_IS_OK(status);
+}
+
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 a33559ebda631047bd6fac3120b1ea2d7b36a823 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 v3 6/6] winbindd-sids2xids: reliably determine idmap backend
name
For SIDs which belong to domains, determine the backend name
by the resolved name of corresponding domain SID.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12702
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/winbindd/wb_sids2xids.c | 65 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 62 insertions(+), 3 deletions(-)
diff --git a/source3/winbindd/wb_sids2xids.c b/source3/winbindd/wb_sids2xids.c
index f3add28..771b889 100644
--- a/source3/winbindd/wb_sids2xids.c
+++ b/source3/winbindd/wb_sids2xids.c
@@ -216,6 +216,12 @@ static bool wb_add_domain_sids_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,
+ uint32_t dom_sids_idx);
static struct wbint_TransIDArray *wb_sids2xids_extract_for_domain_index(
TALLOC_CTX *mem_ctx, const struct wbint_TransIDArray *src,
uint32_t domain_index);
@@ -247,7 +253,9 @@ 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, state->num_non_cached);
struct lsa_TranslatedName *n = &names->names[i];
struct wbint_TransID *t = &state->ids.ids[i];
int domain_index;
@@ -255,11 +263,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;
@@ -318,6 +325,58 @@ 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,
+ uint32_t dom_sids_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);
+
+ for (idx = dom_sids_idx; idx < names->count; ++idx) {
+ if (!dom_sid_equal(&sid, &sids[idx])) {
+ continue;
+ }
+
+ /* has the DC been able to resolve
+ * the domain SID?
+ */
+ n = &names->names[idx];
+ if (n->sid_type != SID_NAME_DOMAIN) {
+ DBG_DEBUG("the DC returned non-domain SID type of %d "
+ "for domain %s\n",
+ n->sid_type, sid_string_dbg(&sids[idx]));
+ /* default id-mapping backend */
+ return "";
+ }
+
+ goto ret;
+ }
+
+ /* we're not supposed to just fall off the loop... */
+ smb_panic("Have not found domain 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