From 7dc14e639ad5d088270d85eb52e5d0112ac62525 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 25 Feb 2019 14:38:50 +0100 Subject: [PATCH 01/11] lib: Make idmap_cache return negative mappings Without this we'd query non-existent mappings over and over again. Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit d9303e8eb90d48f09f2e2e8bdf01f4a7c3c21d11) --- source3/lib/idmap_cache.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source3/lib/idmap_cache.c b/source3/lib/idmap_cache.c index 1e8a1ebc607..0ec23df6a05 100644 --- a/source3/lib/idmap_cache.c +++ b/source3/lib/idmap_cache.c @@ -213,7 +213,12 @@ static void idmap_cache_xid2sid_parser(time_t timeout, DATA_BLOB blob, value = (char *)blob.data; - if (value[0] != '-') { + if ((value[0] == '-') && (value[1] == '\0')) { + /* + * Return NULL SID, see comment to uid2sid + */ + state->ret = true; + } else { state->ret = string_to_sid(state->sid, value); } if (state->ret) { -- 2.11.0 From e744d3e97554b58e3f62636ea8f04b861b402b08 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 12:46:39 +0100 Subject: [PATCH 02/11] idmap_cache: Only touch "sid" on success in find_xid_to_sid Why? This makes the negative mapping condition (is_null_sid) more explicit in the code. The callers in lookup_sid initialized "psid" anyway before, and the ones in wb_xids2sids now do as well. This is more in line with other APIs we have: Only touch output parameters if you have something to say. Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit 4faf3e9f6da7515fc263d79f77226d105c2f8524) --- source3/lib/idmap_cache.c | 5 ++--- source3/winbindd/wb_xids2sids.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/lib/idmap_cache.c b/source3/lib/idmap_cache.c index 0ec23df6a05..cf63a229da5 100644 --- a/source3/lib/idmap_cache.c +++ b/source3/lib/idmap_cache.c @@ -201,13 +201,11 @@ static void idmap_cache_xid2sid_parser(time_t timeout, DATA_BLOB blob, (struct idmap_cache_xid2sid_state *)private_data; char *value; - ZERO_STRUCTP(state->sid); - state->ret = false; - if ((blob.length == 0) || (blob.data[blob.length-1] != 0)) { /* * Not a string, can't be a valid mapping */ + state->ret = false; return; } @@ -217,6 +215,7 @@ static void idmap_cache_xid2sid_parser(time_t timeout, DATA_BLOB blob, /* * Return NULL SID, see comment to uid2sid */ + *state->sid = (struct dom_sid) {0}; state->ret = true; } else { state->ret = string_to_sid(state->sid, value); diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 1e251d8cbeb..766092b2664 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -462,7 +462,7 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, uint32_t i; for (i=0; i Date: Tue, 26 Feb 2019 12:52:28 +0100 Subject: [PATCH 03/11] winbind: Initialize "expired" parameter to idmap_cache_xid2sid The code in idmap_cache only touches its output parameters upon success Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit 8c28c12702c0935a852c7fed6565987623f09fee) --- source3/winbindd/wb_xids2sids.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 766092b2664..5be55d59b75 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -463,7 +463,7 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, for (i=0; i Date: Mon, 25 Feb 2019 14:55:00 +0100 Subject: [PATCH 04/11] winbind: Now we explicitly track if we got ids from cache This now properly makes us use negative cache entries Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit 95d33ca79cc315f1a2e41cd60859ef01d6548c77) --- source3/winbindd/wb_xids2sids.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 5be55d59b75..55c24822925 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -243,6 +243,7 @@ static NTSTATUS wb_xids2sids_init_dom_maps_recv(struct tevent_req *req) struct wb_xids2sids_dom_state { struct tevent_context *ev; struct unixid *all_xids; + const bool *cached; size_t num_all_xids; struct dom_sid *all_sids; struct wb_xids2sids_dom_map *dom_map; @@ -259,7 +260,10 @@ static void wb_xids2sids_dom_gotdc(struct tevent_req *subreq); static struct tevent_req *wb_xids2sids_dom_send( TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct wb_xids2sids_dom_map *dom_map, - struct unixid *xids, size_t num_xids, struct dom_sid *sids) + struct unixid *xids, + const bool *cached, + size_t num_xids, + struct dom_sid *sids) { struct tevent_req *req, *subreq; struct wb_xids2sids_dom_state *state; @@ -273,6 +277,7 @@ static struct tevent_req *wb_xids2sids_dom_send( } state->ev = ev; state->all_xids = xids; + state->cached = cached; state->num_all_xids = num_xids; state->all_sids = sids; state->dom_map = dom_map; @@ -293,7 +298,7 @@ static struct tevent_req *wb_xids2sids_dom_send( /* out of range */ continue; } - if (!is_null_sid(&state->all_sids[i])) { + if (state->cached[i]) { /* already mapped */ continue; } @@ -360,7 +365,7 @@ static void wb_xids2sids_dom_done(struct tevent_req *subreq) /* out of range */ continue; } - if (!is_null_sid(&state->all_sids[i])) { + if (state->cached[i]) { /* already mapped */ continue; } @@ -517,7 +522,7 @@ static void wb_xids2sids_init_dom_maps_done(struct tevent_req *subreq) subreq = wb_xids2sids_dom_send( state, state->ev, &dom_maps[state->dom_idx], - state->xids, state->num_xids, state->sids); + state->xids, state->cached, state->num_xids, state->sids); if (tevent_req_nomem(subreq, req)) { return; } @@ -548,6 +553,7 @@ static void wb_xids2sids_done(struct tevent_req *subreq) state->ev, &dom_maps[state->dom_idx], state->xids, + state->cached, state->num_xids, state->sids); if (tevent_req_nomem(subreq, req)) { -- 2.11.0 From b6a5c7e1971a0db5cf075ef2712954d8e0267907 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 14:32:52 +0100 Subject: [PATCH 05/11] idmap_cache: Introduce idmap_cache_find_xid2sid Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit bb8122dd8c53bb307819a79b7888cc0940a7c13b) --- source3/lib/idmap_cache.c | 36 ++++++++++++++++++++++++++++++++++++ source3/lib/idmap_cache.h | 2 ++ 2 files changed, 38 insertions(+) diff --git a/source3/lib/idmap_cache.c b/source3/lib/idmap_cache.c index cf63a229da5..9fc32215001 100644 --- a/source3/lib/idmap_cache.c +++ b/source3/lib/idmap_cache.c @@ -276,6 +276,42 @@ bool idmap_cache_find_gid2sid(gid_t gid, struct dom_sid *sid, bool *expired) } /** + * Find a xid2sid mapping + * @param[in] id the unix id to map + * @param[out] sid where to put the result + * @param[out] expired is the cache entry expired? + * @retval Was anything in the cache at all? + * + * If "is_null_sid(sid)", this was a negative mapping. + */ +bool idmap_cache_find_xid2sid( + const struct unixid *id, struct dom_sid *sid, bool *expired) +{ + struct idmap_cache_xid2sid_state state = { + .sid = sid, .expired = expired + }; + fstring key; + char c; + + switch (id->type) { + case ID_TYPE_UID: + c = 'U'; + break; + case ID_TYPE_GID: + c = 'G'; + break; + default: + return false; + } + + fstr_sprintf(key, "IDMAP/%cID2SID/%d", c, (int)id->id); + + gencache_parse(key, idmap_cache_xid2sid_parser, &state); + return state.ret; +} + + +/** * Store a mapping in the idmap cache * @param[in] sid the sid to map * @param[in] unix_id the unix_id to map diff --git a/source3/lib/idmap_cache.h b/source3/lib/idmap_cache.h index dc497022e3b..d5afa170e1a 100644 --- a/source3/lib/idmap_cache.h +++ b/source3/lib/idmap_cache.h @@ -31,6 +31,8 @@ bool idmap_cache_find_sid2gid(const struct dom_sid *sid, gid_t *pgid, bool *expired); bool idmap_cache_find_uid2sid(uid_t uid, struct dom_sid *sid, bool *expired); bool idmap_cache_find_gid2sid(gid_t gid, struct dom_sid *sid, bool *expired); +bool idmap_cache_find_xid2sid( + const struct unixid *id, struct dom_sid *sid, bool *expired); void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid *unix_id); bool idmap_cache_del_uid(uid_t uid); -- 2.11.0 From 6eee08a60b24e5aed0d45f3d7ecc0903efcf5c72 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 27 Feb 2019 14:54:12 +0100 Subject: [PATCH 06/11] torture: Add tests for idmap cache Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit e5a903bab6eda8f7ff2a7c8149d51022d9d8aede) --- source3/selftest/tests.py | 1 + source3/torture/proto.h | 1 + source3/torture/test_idmap_cache.c | 122 +++++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 1 + source3/wscript_build | 1 + 5 files changed, 126 insertions(+) create mode 100644 source3/torture/test_idmap_cache.c diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 96524d1748f..2d6aa2c51bb 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -164,6 +164,7 @@ local_tests = [ "LOCAL-G-LOCK5", "LOCAL-G-LOCK6", "LOCAL-NAMEMAP-CACHE1", + "LOCAL-IDMAP-CACHE1", "LOCAL-hex_encode_buf", "LOCAL-remove_duplicate_addrs2"] diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 1634da49315..eb98aba49dd 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -137,5 +137,6 @@ bool run_g_lock5(int dummy); bool run_g_lock6(int dummy); bool run_g_lock_ping_pong(int dummy); bool run_local_namemap_cache1(int dummy); +bool run_local_idmap_cache1(int dummy); #endif /* __TORTURE_H__ */ diff --git a/source3/torture/test_idmap_cache.c b/source3/torture/test_idmap_cache.c new file mode 100644 index 00000000000..b9cba3b4a53 --- /dev/null +++ b/source3/torture/test_idmap_cache.c @@ -0,0 +1,122 @@ +/* + * Unix SMB/CIFS implementation. + * Test dbwrap_watch API + * Copyright (C) Volker Lendecke 2017 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "includes.h" +#include "torture/proto.h" +#include "lib/idmap_cache.h" +#include "librpc/gen_ndr/idmap.h" +#include "libcli/security/dom_sid.h" + +bool run_local_idmap_cache1(int dummy) +{ + struct dom_sid sid, found_sid; + struct unixid xid, found_xid; + bool ret = false; + bool expired = false; + + xid = (struct unixid) { .id = 1234, .type = ID_TYPE_UID }; + dom_sid_parse("S-1-5-21-2864185242-3846410404-2398417794-1235", &sid); + idmap_cache_set_sid2unixid(&sid, &xid); + + ret = idmap_cache_find_sid2unixid(&sid, &found_xid, &expired); + if (!ret) { + fprintf(stderr, "idmap_cache_find_sid2unixid failed\n"); + goto done; + } + if (expired) { + fprintf(stderr, + "idmap_cache_find_sid2unixid returned an expired " + "value\n"); + goto done; + } + if ((xid.type != found_xid.type) || (xid.id != found_xid.id)) { + fprintf(stderr, + "idmap_cache_find_sid2unixid returned wrong " + "values\n"); + goto done; + } + + ret = idmap_cache_find_xid2sid(&xid, &found_sid, &expired); + if (!ret) { + fprintf(stderr, "idmap_cache_find_xid2sid failed\n"); + goto done; + } + if (expired) { + fprintf(stderr, + "idmap_cache_find_xid2sid returned an expired " + "value\n"); + goto done; + } + if (!dom_sid_equal(&sid, &found_sid)) { + fprintf(stderr, + "idmap_cache_find_xid2sid returned wrong sid\n"); + goto done; + } + + xid.type = ID_TYPE_GID; + + ret = idmap_cache_find_xid2sid(&xid, &found_sid, &expired); + if (ret) { + fprintf(stderr, + "idmap_cache_find_xid2sid found a GID where it " + "should not\n"); + goto done; + } + + idmap_cache_del_sid(&sid); + + xid.type = ID_TYPE_UID; + ret = idmap_cache_find_xid2sid(&xid, &found_sid, &expired); + if (ret) { + fprintf(stderr, + "idmap_cache_find_xid2sid found a UID where it " + "should not\n"); + goto done; + } + + /* + * Test that negative mappings can also be cached + */ + sid = (struct dom_sid) {0}; + xid = (struct unixid) { .id = 1234, .type = ID_TYPE_UID }; + idmap_cache_set_sid2unixid(&sid, &xid); + + ret = idmap_cache_find_xid2sid(&xid, &found_sid, &expired); + if (!ret) { + fprintf(stderr, + "idmap_cache_find_xid2sid failed to find " + "negative mapping\n"); + goto done; + } + if (expired) { + fprintf(stderr, + "idmap_cache_find_xid2sid returned an expired " + "value\n"); + goto done; + } + if (!dom_sid_equal(&sid, &found_sid)) { + fprintf(stderr, + "idmap_cache_find_xid2sid returned wrong sid\n"); + goto done; + } + + ret = true; +done: + return ret; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 412e9b413fa..8ca2ef2975a 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -11718,6 +11718,7 @@ static struct { { "LOCAL-G-LOCK-PING-PONG", run_g_lock_ping_pong, 0 }, { "LOCAL-CANONICALIZE-PATH", run_local_canonicalize_path, 0 }, { "LOCAL-NAMEMAP-CACHE1", run_local_namemap_cache1, 0 }, + { "LOCAL-IDMAP-CACHE1", run_local_idmap_cache1, 0 }, { "qpathinfo-bufsize", run_qpathinfo_bufsize, 0 }, {NULL, NULL, 0}}; diff --git a/source3/wscript_build b/source3/wscript_build index 40d93cbcae5..3108de5dc35 100644 --- a/source3/wscript_build +++ b/source3/wscript_build @@ -1201,6 +1201,7 @@ bld.SAMBA3_BINARY('smbtorture' + bld.env.suffix3, torture/wbc_async.c torture/test_g_lock.c torture/test_namemap_cache.c + torture/test_idmap_cache.c ''', deps=''' talloc -- 2.11.0 From c0900d1e5bfcc241a79058df914abdb02b514db2 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 14:34:56 +0100 Subject: [PATCH 07/11] winbind: Use idmap_cache_find_xid2sid Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit bc9824bd42d9370279819ea0d927e236f6041324) --- source3/winbindd/wb_xids2sids.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/source3/winbindd/wb_xids2sids.c b/source3/winbindd/wb_xids2sids.c index 55c24822925..03690278856 100644 --- a/source3/winbindd/wb_xids2sids.c +++ b/source3/winbindd/wb_xids2sids.c @@ -470,19 +470,8 @@ struct tevent_req *wb_xids2sids_send(TALLOC_CTX *mem_ctx, struct dom_sid sid = {0}; bool ok, expired = true; - switch (xids[i].type) { - case ID_TYPE_UID: - ok = idmap_cache_find_uid2sid( - xids[i].id, &sid, &expired); - break; - case ID_TYPE_GID: - ok = idmap_cache_find_gid2sid( - xids[i].id, &sid, &expired); - break; - default: - ok = false; - } - + ok = idmap_cache_find_xid2sid( + &xids[i], &sid, &expired); if (ok && !expired) { sid_copy(&state->sids[i], &sid); state->cached[i] = true; -- 2.11.0 From 5d97473d6d44f1e55488fb77e6a417c117e203c8 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 14:45:32 +0100 Subject: [PATCH 08/11] lib: Introduce winbind_xid_to_sid This does not merge a winbind communication error into "global_sid_NULL" (S-1-0-0), which by the way non-intuitively does not go along with is_null_sid(). Instead, this just touches the output sid when winbind returned success. This success might well be a negative mapping indicated by S-0-0, which *is* is_null_sid()... Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit ef706a3e63b3e25edd27e0f99c3e2d8ff7209cb6) --- source3/lib/winbind_util.c | 30 ++++++++++++++++++++++++++++++ source3/lib/winbind_util.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/source3/lib/winbind_util.c b/source3/lib/winbind_util.c index 427831f04c8..5f10cb68725 100644 --- a/source3/lib/winbind_util.c +++ b/source3/lib/winbind_util.c @@ -197,6 +197,36 @@ bool winbind_gid_to_sid(struct dom_sid *sid, gid_t gid) return (result == WBC_ERR_SUCCESS); } +bool winbind_xid_to_sid(struct dom_sid *sid, const struct unixid *xid) +{ + struct wbcUnixId wbc_xid; + struct wbcDomainSid dom_sid; + wbcErr result; + + switch (xid->type) { + case ID_TYPE_UID: + wbc_xid = (struct wbcUnixId) { + .type = WBC_ID_TYPE_UID, .id.uid = xid->id + }; + break; + case ID_TYPE_GID: + wbc_xid = (struct wbcUnixId) { + .type = WBC_ID_TYPE_GID, .id.gid = xid->id + }; + break; + default: + return false; + } + + result = wbcUnixIdsToSids(&wbc_xid, 1, &dom_sid); + if (result != WBC_ERR_SUCCESS) { + return false; + } + + memcpy(sid, &dom_sid, sizeof(struct dom_sid)); + return true; +} + /* Check for a trusted domain */ wbcErr wb_is_trusted_domain(const char *domain) diff --git a/source3/lib/winbind_util.h b/source3/lib/winbind_util.h index c2bf0e02d76..5ecda5a7b09 100644 --- a/source3/lib/winbind_util.h +++ b/source3/lib/winbind_util.h @@ -22,6 +22,7 @@ #define __LIB__WINBIND_UTIL_H__ #include "../librpc/gen_ndr/lsa.h" +#include "librpc/gen_ndr/idmap.h" /* needed for wbcErr below */ #include "nsswitch/libwbclient/wbclient.h" @@ -38,6 +39,7 @@ bool winbind_sid_to_uid(uid_t *puid, const struct dom_sid *sid); bool winbind_uid_to_sid(struct dom_sid *sid, uid_t uid); bool winbind_sid_to_gid(gid_t *pgid, const struct dom_sid *sid); bool winbind_gid_to_sid(struct dom_sid *sid, gid_t gid); +bool winbind_xid_to_sid(struct dom_sid *sid, const struct unixid *xid); struct passwd * winbind_getpwnam(const char * sname); struct passwd * winbind_getpwsid(const struct dom_sid *sid); wbcErr wb_is_trusted_domain(const char *domain); -- 2.11.0 From c6bc7880fe70c3a78ac3a4291f5fa24840d1a58b Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 18 Oct 2018 05:46:37 +0200 Subject: [PATCH 09/11] lib: Add dom_sid_str_buf This is modeled after server_id_str_buf, which as an API to me is easier to use: I can rely on the compiler to get the buffer size right. It is designed to violate README.Coding's "Make use of helper variables", but as this API is simple enough and the output should never be a surprise at all, I think that's worth it. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Nov 2 20:11:11 CET 2018 on sn-devel-144 (cherry picked from commit 8b9d36221930a487ca5c51bf2e38ed04de9d50f7) --- libcli/security/dom_sid.c | 10 ++++++++++ libcli/security/dom_sid.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c index 17ac0560d83..9d9f466e080 100644 --- a/libcli/security/dom_sid.c +++ b/libcli/security/dom_sid.c @@ -488,3 +488,13 @@ char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid) talloc_set_name_const(result, result); return result; } + +char *dom_sid_str_buf(const struct dom_sid *sid, struct dom_sid_buf *dst) +{ + int ret; + ret = dom_sid_string_buf(sid, dst->buf, sizeof(dst->buf)); + if ((ret < 0) || (ret >= sizeof(dst->buf))) { + strlcpy(dst->buf, "(INVALID SID)", sizeof(dst->buf)); + } + return dst->buf; +} diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h index 491fc0699f4..0010fd2c3af 100644 --- a/libcli/security/dom_sid.h +++ b/libcli/security/dom_sid.h @@ -103,6 +103,8 @@ bool dom_sid_is_valid_account_domain(const struct dom_sid *sid); int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen); char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid); +struct dom_sid_buf { char buf[DOM_SID_STR_BUFLEN]; }; +char *dom_sid_str_buf(const struct dom_sid *sid, struct dom_sid_buf *dst); const char *sid_type_lookup(uint32_t sid_type); const struct security_token *get_system_token(void); -- 2.11.0 From 0039373a16f84fd3afa7229468436a2f7ae162a2 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 15:10:21 +0100 Subject: [PATCH 10/11] passdb: Introduce xid_to_sid This explicitly avoids the legacy_[ug]id_to_sid calls, which create long-term cache entries to S-1-22-x-y if anthing fails. We can't do this, because this will turn temporary winbind communication failures into long-term problems: A short hickup in winbind_uid_to_sid will create a mapping to S-1-22-1-uid for a week. It should be up to the lower layers to do the caching. Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit 92f27ebb14c0c18b1d0fd49544ad851aeb14781c) --- source3/passdb/lookup_sid.c | 74 +++++++++++++++++++++++++++++++++++++++++++++ source3/passdb/lookup_sid.h | 1 + 2 files changed, 75 insertions(+) diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c index eeaf2b720a7..1cf3a7ddb87 100644 --- a/source3/passdb/lookup_sid.c +++ b/source3/passdb/lookup_sid.c @@ -1337,6 +1337,80 @@ void gid_to_sid(struct dom_sid *psid, gid_t gid) return; } +void xid_to_sid(struct dom_sid *psid, const struct unixid *xid) +{ + bool expired = true; + bool ret; + struct dom_sid_buf buf; + + SMB_ASSERT(xid->type == ID_TYPE_UID || xid->type == ID_TYPE_GID); + + *psid = (struct dom_sid) {0}; + + ret = idmap_cache_find_xid2sid(xid, psid, &expired); + if (ret && !expired) { + DBG_DEBUG("%cID %"PRIu32" -> %s from cache\n", + xid->type == ID_TYPE_UID ? 'U' : 'G', + xid->id, + dom_sid_str_buf(psid, &buf)); + goto done; + } + + ret = winbind_xid_to_sid(psid, xid); + if (ret) { + /* + * winbind can return an explicit negative mapping + * here. It's up to winbind to prime the cache either + * positively or negatively, don't mess with the cache + * here. + */ + DBG_DEBUG("%cID %"PRIu32" -> %s from cache\n", + xid->type == ID_TYPE_UID ? 'U' : 'G', + xid->id, + dom_sid_str_buf(psid, &buf)); + goto done; + } + + { + /* + * Make a copy, pdb_id_to_sid might want to turn + * xid->type into ID_TYPE_BOTH, which we ignore here. + */ + struct unixid rw_xid = *xid; + + become_root(); + ret = pdb_id_to_sid(&rw_xid, psid); + unbecome_root(); + } + + if (ret) { + DBG_DEBUG("%cID %"PRIu32" -> %s from passdb\n", + xid->type == ID_TYPE_UID ? 'U' : 'G', + xid->id, + dom_sid_str_buf(psid, &buf)); + goto done; + } + +done: + if (is_null_sid(psid)) { + /* + * Nobody found anything: Return S-1-22-xx-yy. Don't + * store that in caches, this is up to the layers + * beneath us. + */ + if (xid->type == ID_TYPE_UID) { + uid_to_unix_users_sid(xid->id, psid); + } else { + gid_to_unix_groups_sid(xid->id, psid); + } + + DBG_DEBUG("%cID %"PRIu32" -> %s fallback\n", + xid->type == ID_TYPE_UID ? 'U' : 'G', + xid->id, + dom_sid_str_buf(psid, &buf)); + } +} + bool sids_to_unixids(const struct dom_sid *sids, uint32_t num_sids, struct unixid *ids) { diff --git a/source3/passdb/lookup_sid.h b/source3/passdb/lookup_sid.h index 8b5edf6bcb8..8a21cca2a4d 100644 --- a/source3/passdb/lookup_sid.h +++ b/source3/passdb/lookup_sid.h @@ -83,6 +83,7 @@ bool lookup_sid(TALLOC_CTX *mem_ctx, const struct dom_sid *sid, enum lsa_SidType *ret_type); void uid_to_sid(struct dom_sid *psid, uid_t uid); void gid_to_sid(struct dom_sid *psid, gid_t gid); +void xid_to_sid(struct dom_sid *psid, const struct unixid *xid); bool sid_to_uid(const struct dom_sid *psid, uid_t *puid); bool sid_to_gid(const struct dom_sid *psid, gid_t *pgid); bool sids_to_unixids(const struct dom_sid *sids, uint32_t num_sids, -- 2.11.0 From 8b24e68eb3afeb704c6802bc9550d3362c448094 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 26 Feb 2019 15:17:36 +0100 Subject: [PATCH 11/11] passdb: Make [ug]id_to_sid use xid_to_sid Signed-off-by: Volker Lendecke Reviewed-by: Christof Schmitt Bug: https://bugzilla.samba.org/show_bug.cgi?id=13813 (cherry picked from commit 40de67f1fcc46b7a64a7364c91dcedb474826d51) --- source3/passdb/lookup_sid.c | 201 +++----------------------------------------- 1 file changed, 12 insertions(+), 189 deletions(-) diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c index 1cf3a7ddb87..caa3442c6f1 100644 --- a/source3/passdb/lookup_sid.c +++ b/source3/passdb/lookup_sid.c @@ -1102,97 +1102,6 @@ bool lookup_sid(TALLOC_CTX *mem_ctx, const struct dom_sid *sid, } /***************************************************************** - Id mapping cache. This is to avoid Winbind mappings already - seen by smbd to be queried too frequently, keeping winbindd - busy, and blocking smbd while winbindd is busy with other - stuff. Written by Michael Steffens , - modified to use linked lists by jra. -*****************************************************************/ - - -/***************************************************************** - *THE LEGACY* convert uid_t to SID function. -*****************************************************************/ - -static void legacy_uid_to_sid(struct dom_sid *psid, uid_t uid) -{ - bool ret; - struct unixid id; - - ZERO_STRUCTP(psid); - - id.id = uid; - id.type = ID_TYPE_UID; - - become_root(); - ret = pdb_id_to_sid(&id, psid); - unbecome_root(); - - if (ret) { - /* This is a mapped user */ - goto done; - } - - /* This is an unmapped user */ - - uid_to_unix_users_sid(uid, psid); - - { - struct unixid xid = { - .id = uid, .type = ID_TYPE_UID - }; - idmap_cache_set_sid2unixid(psid, &xid); - } - - done: - DEBUG(10,("LEGACY: uid %u -> sid %s\n", (unsigned int)uid, - sid_string_dbg(psid))); - - return; -} - -/***************************************************************** - *THE LEGACY* convert gid_t to SID function. -*****************************************************************/ - -static void legacy_gid_to_sid(struct dom_sid *psid, gid_t gid) -{ - bool ret; - struct unixid id; - - ZERO_STRUCTP(psid); - - id.id = gid; - id.type = ID_TYPE_GID; - - become_root(); - ret = pdb_id_to_sid(&id, psid); - unbecome_root(); - - if (ret) { - /* This is a mapped group */ - goto done; - } - - /* This is an unmapped group */ - - gid_to_unix_groups_sid(gid, psid); - - { - struct unixid xid = { - .id = gid, .type = ID_TYPE_GID - }; - idmap_cache_set_sid2unixid(psid, &xid); - } - - done: - DEBUG(10,("LEGACY: gid %u -> sid %s\n", (unsigned int)gid, - sid_string_dbg(psid))); - - return; -} - -/***************************************************************** *THE LEGACY* convert SID to id function. *****************************************************************/ @@ -1239,104 +1148,6 @@ static bool legacy_sid_to_uid(const struct dom_sid *psid, uid_t *puid) return false; } -/***************************************************************** - *THE CANONICAL* convert uid_t to SID function. -*****************************************************************/ - -void uid_to_sid(struct dom_sid *psid, uid_t uid) -{ - bool expired = true; - bool ret; - ZERO_STRUCTP(psid); - - /* Check the winbindd cache directly. */ - ret = idmap_cache_find_uid2sid(uid, psid, &expired); - - if (ret && !expired && is_null_sid(psid)) { - /* - * Negative cache entry, we already asked. - * do legacy. - */ - legacy_uid_to_sid(psid, uid); - return; - } - - if (!ret || expired) { - /* Not in cache. Ask winbindd. */ - if (!winbind_uid_to_sid(psid, uid)) { - /* - * We shouldn't return the NULL SID - * here if winbind was running and - * couldn't map, as winbind will have - * added a negative entry that will - * cause us to go though the - * legacy_uid_to_sid() - * function anyway in the case above - * the next time we ask. - */ - DEBUG(5, ("uid_to_sid: winbind failed to find a sid " - "for uid %u\n", (unsigned int)uid)); - - legacy_uid_to_sid(psid, uid); - return; - } - } - - DEBUG(10,("uid %u -> sid %s\n", (unsigned int)uid, - sid_string_dbg(psid))); - - return; -} - -/***************************************************************** - *THE CANONICAL* convert gid_t to SID function. -*****************************************************************/ - -void gid_to_sid(struct dom_sid *psid, gid_t gid) -{ - bool expired = true; - bool ret; - ZERO_STRUCTP(psid); - - /* Check the winbindd cache directly. */ - ret = idmap_cache_find_gid2sid(gid, psid, &expired); - - if (ret && !expired && is_null_sid(psid)) { - /* - * Negative cache entry, we already asked. - * do legacy. - */ - legacy_gid_to_sid(psid, gid); - return; - } - - if (!ret || expired) { - /* Not in cache. Ask winbindd. */ - if (!winbind_gid_to_sid(psid, gid)) { - /* - * We shouldn't return the NULL SID - * here if winbind was running and - * couldn't map, as winbind will have - * added a negative entry that will - * cause us to go though the - * legacy_gid_to_sid() - * function anyway in the case above - * the next time we ask. - */ - DEBUG(5, ("gid_to_sid: winbind failed to find a sid " - "for gid %u\n", (unsigned int)gid)); - - legacy_gid_to_sid(psid, gid); - return; - } - } - - DEBUG(10,("gid %u -> sid %s\n", (unsigned int)gid, - sid_string_dbg(psid))); - - return; -} - void xid_to_sid(struct dom_sid *psid, const struct unixid *xid) { bool expired = true; @@ -1411,6 +1222,18 @@ done: } } +void uid_to_sid(struct dom_sid *psid, uid_t uid) +{ + struct unixid xid = { .type = ID_TYPE_UID, .id = uid}; + xid_to_sid(psid, &xid); +} + +void gid_to_sid(struct dom_sid *psid, gid_t gid) +{ + struct unixid xid = { .type = ID_TYPE_GID, .id = gid}; + xid_to_sid(psid, &xid); +} + bool sids_to_unixids(const struct dom_sid *sids, uint32_t num_sids, struct unixid *ids) { -- 2.11.0