[PATCH] Fix idmap_autorid for cross-forest trusts
Volker Lendecke
vl at samba.org
Wed Dec 14 14:26:11 UTC 2016
Hi!
idmap_autorid right now refused to map valid users behind a cross-forest
trust or a one-way trust because it depends on the trust to be listed
in the winbind tdc cache. Cross-forest trusts don't necessarily end up
there. This patchset stores all domain sids we've seen via SMB logins
in netsamlogon_cache and idmap_autorid looks there.
This could also be used as sample code to get rid of the tdc cache and
the trustdom list dependency overall.
It also contains the usual drive-by cleanups.
Review appreciated!
Thanks, Volker
-------------- next part --------------
>From 6a4be0e6285b6dd86c54eeb4c036271c771f84be Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 1 Dec 2016 20:45:35 +0000
Subject: [PATCH 01/19] samlogon_cache: Simplify netsamlogon_cache_have
We're interested in existence only, we should be able to trust the data
format consistency for this type of query.
netsamlogon_cache_get calls netsamlogon_cache_init for us, now we have
to do it directly.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libsmb/samlogon_cache.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/source3/libsmb/samlogon_cache.c b/source3/libsmb/samlogon_cache.c
index 8131b2b..9a0f65d 100644
--- a/source3/libsmb/samlogon_cache.c
+++ b/source3/libsmb/samlogon_cache.c
@@ -259,19 +259,16 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
bool netsamlogon_cache_have(const struct dom_sid *user_sid)
{
- TALLOC_CTX *mem_ctx = talloc_init("netsamlogon_cache_have");
- struct netr_SamInfo3 *info3 = NULL;
- bool result;
+ char keystr[DOM_SID_STR_BUFLEN];
+ bool ok;
- if (mem_ctx == NULL) {
+ if (!netsamlogon_cache_init()) {
+ DBG_WARNING("Cannot open %s\n", NETSAMLOGON_TDB);
return false;
}
- info3 = netsamlogon_cache_get(mem_ctx, user_sid);
-
- result = (info3 != NULL);
-
- TALLOC_FREE(mem_ctx);
+ dom_sid_string_buf(user_sid, keystr, sizeof(keystr));
- return result;
+ ok = tdb_exists(netsamlogon_tdb, string_term_tdb_data(keystr));
+ return ok;
}
--
2.1.4
>From 80cf9e85b265196ef9170f59d553fcb028ca4f86 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 1 Dec 2016 20:46:47 +0000
Subject: [PATCH 02/19] samlogon_cache: Add the user's domain sid into the
samlogon_cache
This will be used by autorid and possibly others instead of the tdc
cache. The only reliable way to find a domain to be trusted is via a
successful login. We indicate successful login via a netsamlogon_cache.tdb
entry. This patch also adds the user's domain sid with an entry, so we
can check for that existence without traversing the cache.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libsmb/samlogon_cache.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/source3/libsmb/samlogon_cache.c b/source3/libsmb/samlogon_cache.c
index 9a0f65d..4815cfb 100644
--- a/source3/libsmb/samlogon_cache.c
+++ b/source3/libsmb/samlogon_cache.c
@@ -122,7 +122,8 @@ void netsamlogon_clear_cached_user(const struct dom_sid *user_sid)
bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
{
- TDB_DATA data;
+ uint8_t dummy = 0;
+ TDB_DATA data = { .dptr = &dummy, .dsize = sizeof(dummy) };
char keystr[DOM_SID_STR_BUFLEN];
bool result = false;
struct dom_sid user_sid;
@@ -130,6 +131,7 @@ bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
DATA_BLOB blob;
enum ndr_err_code ndr_err;
struct netsamlogoncache_entry r;
+ int ret;
if (!info3) {
return false;
@@ -141,6 +143,23 @@ bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
return false;
}
+ /*
+ * First write a record with just the domain sid for
+ * netsamlogon_cache_domain_known. Use TDB_INSERT to avoid
+ * overwriting potentially other data. We're just interested
+ * in the existence of that record.
+ */
+ dom_sid_string_buf(info3->base.domain_sid, keystr, sizeof(keystr));
+
+ ret = tdb_store_bystring(netsamlogon_tdb, keystr, data, TDB_INSERT);
+
+ if ((ret == -1) && (tdb_error(netsamlogon_tdb) != TDB_ERR_EXISTS)) {
+ DBG_WARNING("Could not store domain marker for %s: %s\n",
+ keystr, tdb_errorstr(netsamlogon_tdb));
+ TALLOC_FREE(tmp_ctx);
+ return false;
+ }
+
sid_compose(&user_sid, info3->base.domain_sid, info3->base.rid);
/* Prepare key as DOMAIN-SID/USER-RID string */
--
2.1.4
>From dab1adfef8c57d458171d702f080276961747efb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 5 Dec 2016 14:38:14 +0000
Subject: [PATCH 03/19] samlogon_cache: Rename "user_sid" to "sid"
This is no longer just a user, we can also check for domains
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libsmb/samlogon_cache.c | 4 ++--
source3/libsmb/samlogon_cache.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/source3/libsmb/samlogon_cache.c b/source3/libsmb/samlogon_cache.c
index 4815cfb..5a16686 100644
--- a/source3/libsmb/samlogon_cache.c
+++ b/source3/libsmb/samlogon_cache.c
@@ -276,7 +276,7 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
return info3;
}
-bool netsamlogon_cache_have(const struct dom_sid *user_sid)
+bool netsamlogon_cache_have(const struct dom_sid *sid)
{
char keystr[DOM_SID_STR_BUFLEN];
bool ok;
@@ -286,7 +286,7 @@ bool netsamlogon_cache_have(const struct dom_sid *user_sid)
return false;
}
- dom_sid_string_buf(user_sid, keystr, sizeof(keystr));
+ dom_sid_string_buf(sid, keystr, sizeof(keystr));
ok = tdb_exists(netsamlogon_tdb, string_term_tdb_data(keystr));
return ok;
diff --git a/source3/libsmb/samlogon_cache.h b/source3/libsmb/samlogon_cache.h
index 0a2fd14..221e67b 100644
--- a/source3/libsmb/samlogon_cache.h
+++ b/source3/libsmb/samlogon_cache.h
@@ -36,6 +36,6 @@ bool netsamlogon_cache_store(const char *username,
struct netr_SamInfo3 *info3);
struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx,
const struct dom_sid *user_sid);
-bool netsamlogon_cache_have(const struct dom_sid *user_sid);
+bool netsamlogon_cache_have(const struct dom_sid *sid);
#endif
--
2.1.4
>From 1444b1517f7e30a61392a5e6f3c762bcc66a01f0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Nov 2016 18:35:55 +0100
Subject: [PATCH 04/19] idmap_autorid: Slightly simplify
idmap_autorid_unixids_to_sids
Avoid an else branch where it's not necessary
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 1fd6a76..3da8966 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -340,7 +340,8 @@ static NTSTATUS idmap_autorid_unixids_to_sids(struct idmap_domain *dom,
if (num_tomap == num_mapped) {
return NT_STATUS_OK;
- } else if (num_mapped == 0) {
+ }
+ if (num_mapped == 0) {
return NT_STATUS_NONE_MAPPED;
}
--
2.1.4
>From 3afae891805a3ccad30950b7d370f5f73c1c8b13 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 30 Nov 2016 18:43:44 +0100
Subject: [PATCH 05/19] idmap_tdb: Harden idmap_tdb_common_unixid_to_sid
A non-null terminated record would make string_to_sid read beyond the
end of allocated data.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_tdb_common.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/source3/winbindd/idmap_tdb_common.c b/source3/winbindd/idmap_tdb_common.c
index ebf1bb9..0d7e734 100644
--- a/source3/winbindd/idmap_tdb_common.c
+++ b/source3/winbindd/idmap_tdb_common.c
@@ -430,6 +430,12 @@ NTSTATUS idmap_tdb_common_unixid_to_sid(struct idmap_domain * dom,
goto done;
}
+ if ((data.dsize == 0) || (data.dptr[data.dsize-1] != '\0')) {
+ DBG_DEBUG("Invalid record length %zu\n", data.dsize);
+ ret = NT_STATUS_INTERNAL_DB_ERROR;
+ goto done;
+ }
+
if (!string_to_sid(map->sid, (const char *)data.dptr)) {
DEBUG(10, ("INVALID SID (%s) in record %s\n",
(const char *)data.dptr, keystr));
--
2.1.4
>From bdeb319df2f40001f4537f51e4077f8841567307 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 1 Dec 2016 13:28:01 +0000
Subject: [PATCH 06/19] idmap_autorid: Protect against dsize==0
Not sure it can happen, but you never know...
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 3da8966..8067238 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -217,7 +217,7 @@ static NTSTATUS idmap_autorid_id_to_sid(struct autorid_global_config *cfg,
return NT_STATUS_OK;
}
- if (data.dptr[data.dsize-1] != '\0') {
+ if ((data.dsize == 0) || (data.dptr[data.dsize-1] != '\0')) {
DBG_WARNING("Invalid range %"PRIu32"\n", range_number);
TALLOC_FREE(data.dptr);
map->status = ID_UNKNOWN;
--
2.1.4
>From 8b1d9dfedbaff6e7d50721116bd0e3a8dddcf272 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 1 Dec 2016 13:29:29 +0000
Subject: [PATCH 07/19] idmap_autorid: Fix a comment
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 8067238..08fd230 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -142,7 +142,7 @@ static NTSTATUS idmap_autorid_allocate_id(struct idmap_domain *dom,
}
/*
- * map a SID to xid using the idmap_tdb like pool
+ * map a xid to SID using the idmap_tdb like pool
*/
static NTSTATUS idmap_autorid_id_to_sid_alloc(struct idmap_domain *dom,
struct id_map *map)
--
2.1.4
>From e1c80afaf45533ea67c0fdea8e32e714da2db53a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 1 Dec 2016 16:24:51 +0000
Subject: [PATCH 08/19] idmap_autorid: Tighten idmap_autorid_id_to_sid a bit
We should only allow '#' as a sid/range-number separator in autorid.tdb.
The logic might be a bit clumsy. But the switch statement with failure
fall thru was the clearest I could come up with.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 08fd230..f90aea8 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -169,7 +169,7 @@ static NTSTATUS idmap_autorid_id_to_sid(struct autorid_global_config *cfg,
struct id_map *map)
{
uint32_t range_number;
- uint32_t domain_range_index = 0;
+ uint32_t domain_range_index;
uint32_t normalized_id;
uint32_t reduced_rid;
uint32_t rid;
@@ -243,14 +243,29 @@ static NTSTATUS idmap_autorid_id_to_sid(struct autorid_global_config *cfg,
map->status = ID_UNKNOWN;
return NT_STATUS_OK;
}
- if (*q != '\0') {
- if (sscanf(q+1, "%"SCNu32, &domain_range_index) != 1) {
- DEBUG(10, ("Domain range index not found, "
- "ignoring mapping request\n"));
- TALLOC_FREE(data.dptr);
- map->status = ID_UNKNOWN;
- return NT_STATUS_OK;
- }
+
+ /*
+ * Allow for sid#range_index, just sid is range index 0
+ */
+
+ switch (*q) {
+ case '\0':
+ domain_range_index = 0;
+ break;
+ case '#':
+ if (sscanf(q+1, "%"SCNu32, &domain_range_index) == 1) {
+ break;
+ }
+ /*
+ * fall through to failure, something weird is in
+ * the record
+ */
+ default:
+ DBG_DEBUG("SID/domain range: %s\n",
+ (const char *)data.dptr);
+ TALLOC_FREE(data.dptr);
+ map->status = ID_UNKNOWN;
+ return NT_STATUS_OK;
}
TALLOC_FREE(data.dptr);
--
2.1.4
>From 75a270f51213ab2d0ef85af20c88d9afe85de766 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 10:58:35 +0000
Subject: [PATCH 09/19] idmap_autorid: idmap_autorid_sid_to_id_rid only uses
low_id from "range"
Simplification -- from the callers perspective looks like a complex
routine which it is not
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index f90aea8..3c85dd6 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -291,7 +291,7 @@ static NTSTATUS idmap_autorid_id_to_sid(struct autorid_global_config *cfg,
static NTSTATUS idmap_autorid_sid_to_id_rid(
struct autorid_global_config *global,
- struct autorid_range_config *range,
+ uint32_t low_id,
struct id_map *map)
{
uint32_t rid;
@@ -301,7 +301,7 @@ static NTSTATUS idmap_autorid_sid_to_id_rid(
reduced_rid = rid % global->rangesize;
- map->xid.id = reduced_rid + range->low_id;
+ map->xid.id = reduced_rid + low_id;
map->xid.type = ID_TYPE_BOTH;
map->status = ID_MAPPED;
@@ -605,7 +605,7 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
return ret;
}
- return idmap_autorid_sid_to_id_rid(global, &range, map);
+ return idmap_autorid_sid_to_id_rid(global, range.low_id, map);
}
/**********************************
--
2.1.4
>From 209c2ecadff1c88d47db4a28720ad209cfaee633 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 10:58:35 +0000
Subject: [PATCH 10/19] idmap_autorid: idmap_autorid_sid_to_id_rid only uses
rangesize from "global"
Simplification -- from the callers perspective looks like a complex
routine which it is not
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 3c85dd6..03f6184 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -290,7 +290,7 @@ static NTSTATUS idmap_autorid_id_to_sid(struct autorid_global_config *cfg,
**********************************/
static NTSTATUS idmap_autorid_sid_to_id_rid(
- struct autorid_global_config *global,
+ uint32_t rangesize,
uint32_t low_id,
struct id_map *map)
{
@@ -299,7 +299,7 @@ static NTSTATUS idmap_autorid_sid_to_id_rid(
sid_peek_rid(map->sid, &rid);
- reduced_rid = rid % global->rangesize;
+ reduced_rid = rid % rangesize;
map->xid.id = reduced_rid + low_id;
map->xid.type = ID_TYPE_BOTH;
@@ -605,7 +605,8 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
return ret;
}
- return idmap_autorid_sid_to_id_rid(global, range.low_id, map);
+ return idmap_autorid_sid_to_id_rid(global->rangesize, range.low_id,
+ map);
}
/**********************************
--
2.1.4
>From e6276daff255185e31ef1a62a003770cbeaca96d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 12:14:17 +0000
Subject: [PATCH 11/19] idmap_autorid: Do a readonly attempt before looking at
the tdc cache
If autorid.tdb already has a mapping for a domain range, we can just
return that. Even if the volatile tdc cache at this point does not have
the domain, we should return a correct mapping.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 03f6184..4faad5f 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -574,6 +574,18 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
return NT_STATUS_NONE_MAPPED;
}
+ sid_to_fstring(range.domsid, &domainsid);
+
+ range.domain_range_index = rid / (global->rangesize);
+
+ ret = idmap_autorid_getrange(autorid_db, range.domsid,
+ range.domain_range_index,
+ &range.rangenum, &range.low_id);
+ if (NT_STATUS_IS_OK(ret)) {
+ return idmap_autorid_sid_to_id_rid(
+ global->rangesize, range.low_id, map);
+ }
+
/*
* Check if the domain is around
*/
@@ -587,10 +599,6 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
}
TALLOC_FREE(domain);
- sid_to_fstring(range.domsid, &domainsid);
-
- range.domain_range_index = rid / (global->rangesize);
-
ret = idmap_autorid_get_domainrange(autorid_db, &range, dom->read_only);
if (NT_STATUS_EQUAL(ret, NT_STATUS_NOT_FOUND) && dom->read_only) {
DEBUG(10, ("read-only is enabled, did not allocate "
--
2.1.4
>From abe2552ad9983de174d346dc2b2a5831e66f806c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 12:18:06 +0000
Subject: [PATCH 12/19] idmap_autorid: Only look at the tdc cache when
allocating ranges
---
source3/winbindd/idmap_autorid.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index 4faad5f..ff17518 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -586,6 +586,13 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
global->rangesize, range.low_id, map);
}
+ if (dom->read_only) {
+ DBG_DEBUG("read-only is enabled, did not allocate "
+ "new range for domain %s\n", range.domsid);
+ map->status = ID_UNMAPPED;
+ return NT_STATUS_NONE_MAPPED;
+ }
+
/*
* Check if the domain is around
*/
--
2.1.4
>From a01a994257dbfc8c09ba995fd0b36c709ae3a6f1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 14:10:00 +0000
Subject: [PATCH 13/19] idmap_autorid: Add ntstatus to a debug message
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index ff17518..bca8259 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -615,8 +615,9 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
return NT_STATUS_NONE_MAPPED;
}
if (!NT_STATUS_IS_OK(ret)) {
- DEBUG(3, ("Could not determine range for domain, "
- "check previous messages for reason\n"));
+ DBG_NOTICE("Could not determine range for domain: %s, "
+ "check previous messages for reason\n",
+ nt_errstr(ret));
return ret;
}
--
2.1.4
>From 704f2cb55127377c2c775ed04a879a2d8ece965f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 14:10:34 +0000
Subject: [PATCH 14/19] idmap_autorid: Fix checks for valid domains to allocate
ranges for
The tdc cache is not reliable. The main dynamic check is
netsamlogon_cache_have: The only reliable way to see a domain as valid
for allocating a range for is a successful login. With a recent addition
to netsamlogon_cache_store, we can now reliably tell from there whether
a domain is trusted.
This also adds a few heuristic checks, such as allocation for the local
domains and additional ranges where we already have a mapping for range
index 0 for.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 78 +++++++++++++++++++++++++++++++---------
1 file changed, 61 insertions(+), 17 deletions(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index bca8259..a10fc57 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -78,6 +78,8 @@
#include "idmap.h"
#include "idmap_rw.h"
#include "../libcli/security/dom_sid.h"
+#include "libsmb/samlogon_cache.h"
+#include "passdb/machine_sid.h"
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_IDMAP
@@ -541,7 +543,6 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
struct autorid_global_config *global =
talloc_get_type_abort(common->private_data,
struct autorid_global_config);
- struct winbindd_tdc_domain *domain;
struct autorid_range_config range;
uint32_t rid;
struct dom_sid domainsid;
@@ -594,26 +595,69 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
}
/*
- * Check if the domain is around
+ * Check if we should allocate a domain range. We need to
+ * protect against unknown domains to not fill our ranges
+ * needlessly.
*/
- domain = wcache_tdc_fetch_domainbysid(talloc_tos(),
- &domainsid);
- if (domain == NULL) {
- DEBUG(10, ("Ignoring unknown domain sid %s\n",
- sid_string_dbg(&domainsid)));
- map->status = ID_UNMAPPED;
- return NT_STATUS_NONE_MAPPED;
+
+ if (sid_check_is_builtin(&domainsid) ||
+ sid_check_is_our_sam(&domainsid)) {
+ goto allocate;
}
- TALLOC_FREE(domain);
- ret = idmap_autorid_get_domainrange(autorid_db, &range, dom->read_only);
- if (NT_STATUS_EQUAL(ret, NT_STATUS_NOT_FOUND) && dom->read_only) {
- DEBUG(10, ("read-only is enabled, did not allocate "
- "new range for domain %s\n",
- sid_string_dbg(&domainsid)));
- map->status = ID_UNMAPPED;
- return NT_STATUS_NONE_MAPPED;
+ {
+ struct winbindd_domain *domain;
+
+ /*
+ * Deterministic check for domain members: We can be
+ * sure that the domain we are member of is worth to
+ * add a mapping for.
+ */
+
+ domain = find_our_domain();
+ if ((domain != NULL) &&
+ dom_sid_equal(&domain->sid, &domainsid)) {
+ goto allocate;
+ }
+ }
+
+ /*
+ * If we have already allocated range index 0, this domain is
+ * worth allocating for in higher ranges.
+ */
+ if (range.domain_range_index != 0) {
+ uint32_t zero_rangenum, zero_low_id;
+
+ ret = idmap_autorid_getrange(autorid_db, range.domsid, 0,
+ &zero_rangenum, &zero_low_id);
+ if (NT_STATUS_IS_OK(ret)) {
+ goto allocate;
+ }
}
+
+ /*
+ * Check of last resort: A domain is valid if a user from that
+ * domain has recently logged in. The samlogon_cache these
+ * days also stores the domain sid.
+ *
+ * We used to check the list of trusted domains we received
+ * from "our" dc, but this is not reliable enough.
+ */
+ if (netsamlogon_cache_have(&domainsid)) {
+ goto allocate;
+ }
+
+ /*
+ * Nobody knows this domain, so refuse to allocate a fresh
+ * range.
+ */
+
+ DBG_NOTICE("Allocating range for domain %s refused\n", range.domsid);
+ map->status = ID_UNMAPPED;
+ return NT_STATUS_NONE_MAPPED;
+
+allocate:
+ ret = idmap_autorid_get_domainrange(autorid_db, &range, false);
if (!NT_STATUS_IS_OK(ret)) {
DBG_NOTICE("Could not determine range for domain: %s, "
"check previous messages for reason\n",
--
2.1.4
>From d55c3d2780b4416d9f4d307feb120ed9264170bd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 15:11:24 +0000
Subject: [PATCH 15/19] idmap_autorid: Make idmap_autorid_acquire_range public
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/idmap_autorid_tdb.h | 3 +++
source3/winbindd/idmap_autorid_tdb.c | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/source3/include/idmap_autorid_tdb.h b/source3/include/idmap_autorid_tdb.h
index 52bee56..fb3a4c4 100644
--- a/source3/include/idmap_autorid_tdb.h
+++ b/source3/include/idmap_autorid_tdb.h
@@ -82,6 +82,9 @@ NTSTATUS idmap_autorid_setrange(struct db_context *db,
uint32_t domain_range_index,
uint32_t rangenum);
+NTSTATUS idmap_autorid_acquire_range(struct db_context *db,
+ struct autorid_range_config *range);
+
/**
* Delete a domain#index <-> range maping from the database.
* The mapping is specified by the sid and index.
diff --git a/source3/winbindd/idmap_autorid_tdb.c b/source3/winbindd/idmap_autorid_tdb.c
index 2f09f6c..3386380 100644
--- a/source3/winbindd/idmap_autorid_tdb.c
+++ b/source3/winbindd/idmap_autorid_tdb.c
@@ -298,8 +298,8 @@ NTSTATUS idmap_autorid_setrange(struct db_context *db,
return status;
}
-static NTSTATUS idmap_autorid_acquire_range(struct db_context *db,
- struct autorid_range_config *range)
+NTSTATUS idmap_autorid_acquire_range(struct db_context *db,
+ struct autorid_range_config *range)
{
return idmap_autorid_addrange(db, range, true);
}
--
2.1.4
>From 67c148ac3231e92707f029186edfe92abdb83ffc Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 15:25:10 +0000
Subject: [PATCH 16/19] idmap_autorid: Use acquire_range directly
idmap_autorid_get_domainrange is reading again for an existing mapping. We
know we need to allocate here, so avoid passing down that r/o boolean :-)
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
index a10fc57..21a5220 100644
--- a/source3/winbindd/idmap_autorid.c
+++ b/source3/winbindd/idmap_autorid.c
@@ -657,7 +657,7 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
return NT_STATUS_NONE_MAPPED;
allocate:
- ret = idmap_autorid_get_domainrange(autorid_db, &range, false);
+ ret = idmap_autorid_acquire_range(autorid_db, &range);
if (!NT_STATUS_IS_OK(ret)) {
DBG_NOTICE("Could not determine range for domain: %s, "
"check previous messages for reason\n",
--
2.1.4
>From c52c2a7c8191f84089184a2fd2df6f5525cf4b6e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Dec 2016 15:37:49 +0000
Subject: [PATCH 17/19] idmap_autorid: Fix a race condition when acquiring
ranges
Here we are in a transaction to create a range, but we already found
one to exist. We need to return the information about this range to the
caller, just as we do when actually allocating the range. This does not
hit us with current code, as we just have one idmap child. However, if
we parallelize that, two children might have found a domain to not exist
and call idmap_autorid_acquire_range simultaneously. One will create
the range, the other one will find it to already exist. The second child
will also have to pass the info up.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid_tdb.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/source3/winbindd/idmap_autorid_tdb.c b/source3/winbindd/idmap_autorid_tdb.c
index 3386380..702a2f6 100644
--- a/source3/winbindd/idmap_autorid_tdb.c
+++ b/source3/winbindd/idmap_autorid_tdb.c
@@ -127,6 +127,26 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
if (acquire) {
DEBUG(10, ("domain range already allocated - "
"Not adding!\n"));
+
+ mem_ctx = talloc_stackframe();
+
+ ret = idmap_autorid_loadconfig(db, mem_ctx,
+ &globalcfg);
+ if (!NT_STATUS_IS_OK(ret)) {
+ DEBUG(1, ("Fatal error while fetching "
+ "configuration: %s\n",
+ nt_errstr(ret)));
+ goto error;
+ }
+
+ range->rangenum = stored_rangenum;
+ range->low_id = globalcfg->minvalue
+ + range->rangenum * globalcfg->rangesize;
+ range->high_id =
+ range->low_id + globalcfg->rangesize - 1;
+
+ TALLOC_FREE(mem_ctx);
+
return NT_STATUS_OK;
}
--
2.1.4
>From 3e1872a98b0e95852add493f8a7654775ea0c92c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 5 Dec 2016 15:29:06 +0000
Subject: [PATCH 18/19] idmap_autorid: Fix a small memleak
Not long-term, all callers free our "mem_ctx" immediately
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/winbindd/idmap_autorid_tdb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/winbindd/idmap_autorid_tdb.c b/source3/winbindd/idmap_autorid_tdb.c
index 702a2f6..5de4725 100644
--- a/source3/winbindd/idmap_autorid_tdb.c
+++ b/source3/winbindd/idmap_autorid_tdb.c
@@ -887,6 +887,7 @@ NTSTATUS idmap_autorid_loadconfig(struct db_context *db,
}
ok = idmap_autorid_parse_configstr(configstr, cfg);
+ TALLOC_FREE(configstr);
if (!ok) {
talloc_free(cfg);
return NT_STATUS_INVALID_PARAMETER;
--
2.1.4
>From 93edc4fc7e1f312cbe76c7747d8b922ed0715a94 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 5 Dec 2016 15:31:56 +0000
Subject: [PATCH 19/19] idmap_autorid: Simplify idmap_autorid_loadconfig
autorid_global_config is a fixed small structure that can be stack-allocated.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/include/idmap_autorid_tdb.h | 3 +-
source3/winbindd/idmap_autorid_tdb.c | 66 ++++++++++++++----------------------
2 files changed, 27 insertions(+), 42 deletions(-)
diff --git a/source3/include/idmap_autorid_tdb.h b/source3/include/idmap_autorid_tdb.h
index fb3a4c4..36a595f 100644
--- a/source3/include/idmap_autorid_tdb.h
+++ b/source3/include/idmap_autorid_tdb.h
@@ -138,8 +138,7 @@ NTSTATUS idmap_autorid_db_init(const char *path,
* Load the configuration stored in the autorid database.
*/
NTSTATUS idmap_autorid_loadconfig(struct db_context *db,
- TALLOC_CTX *ctx,
- struct autorid_global_config **result);
+ struct autorid_global_config *result);
/**
* Save the global autorid configuration into the autorid database.
diff --git a/source3/winbindd/idmap_autorid_tdb.c b/source3/winbindd/idmap_autorid_tdb.c
index 5de4725..a95702e 100644
--- a/source3/winbindd/idmap_autorid_tdb.c
+++ b/source3/winbindd/idmap_autorid_tdb.c
@@ -88,7 +88,7 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
NTSTATUS ret;
uint32_t hwm;
char *numstr;
- struct autorid_global_config *globalcfg;
+ struct autorid_global_config globalcfg = {0};
fstring keystr;
uint32_t increment;
TALLOC_CTX *mem_ctx = NULL;
@@ -128,10 +128,7 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
DEBUG(10, ("domain range already allocated - "
"Not adding!\n"));
- mem_ctx = talloc_stackframe();
-
- ret = idmap_autorid_loadconfig(db, mem_ctx,
- &globalcfg);
+ ret = idmap_autorid_loadconfig(db, &globalcfg);
if (!NT_STATUS_IS_OK(ret)) {
DEBUG(1, ("Fatal error while fetching "
"configuration: %s\n",
@@ -140,12 +137,10 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
}
range->rangenum = stored_rangenum;
- range->low_id = globalcfg->minvalue
- + range->rangenum * globalcfg->rangesize;
+ range->low_id = globalcfg.minvalue
+ + range->rangenum * globalcfg.rangesize;
range->high_id =
- range->low_id + globalcfg->rangesize - 1;
-
- TALLOC_FREE(mem_ctx);
+ range->low_id + globalcfg.rangesize - 1;
return NT_STATUS_OK;
}
@@ -172,7 +167,7 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
mem_ctx = talloc_stackframe();
- ret = idmap_autorid_loadconfig(db, mem_ctx, &globalcfg);
+ ret = idmap_autorid_loadconfig(db, &globalcfg);
if (!NT_STATUS_IS_OK(ret)) {
DEBUG(1, ("Fatal error while fetching configuration: %s\n",
nt_errstr(ret)));
@@ -186,11 +181,11 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
requested_rangenum = hwm;
}
- if (requested_rangenum >= globalcfg->maxranges) {
+ if (requested_rangenum >= globalcfg.maxranges) {
DEBUG(1, ("Not enough ranges available: New range %u must be "
"smaller than configured maximum number of ranges "
"(%u).\n",
- requested_rangenum, globalcfg->maxranges));
+ requested_rangenum, globalcfg.maxranges));
ret = NT_STATUS_NO_MEMORY;
goto error;
}
@@ -276,9 +271,9 @@ static NTSTATUS idmap_autorid_addrange_action(struct db_context *db,
range->rangenum = requested_rangenum;
- range->low_id = globalcfg->minvalue
- + range->rangenum * globalcfg->rangesize;
- range->high_id = range->low_id + globalcfg->rangesize - 1;
+ range->low_id = globalcfg.minvalue
+ + range->rangenum * globalcfg.rangesize;
+ range->high_id = range->low_id + globalcfg.rangesize - 1;
ret = NT_STATUS_OK;
@@ -328,7 +323,7 @@ static NTSTATUS idmap_autorid_getrange_int(struct db_context *db,
struct autorid_range_config *range)
{
NTSTATUS status = NT_STATUS_INVALID_PARAMETER;
- struct autorid_global_config *globalcfg = NULL;
+ struct autorid_global_config globalcfg = {0};
fstring keystr;
if (db == NULL || range == NULL) {
@@ -353,16 +348,14 @@ static NTSTATUS idmap_autorid_getrange_int(struct db_context *db,
goto done;
}
- status = idmap_autorid_loadconfig(db, talloc_tos(), &globalcfg);
+ status = idmap_autorid_loadconfig(db, &globalcfg);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("Failed to read global configuration"));
goto done;
}
- range->low_id = globalcfg->minvalue
- + range->rangenum * globalcfg->rangesize;
- range->high_id = range->low_id + globalcfg->rangesize - 1;
-
- TALLOC_FREE(globalcfg);
+ range->low_id = globalcfg.minvalue
+ + range->rangenum * globalcfg.rangesize;
+ range->high_id = range->low_id + globalcfg.rangesize - 1;
done:
return status;
}
@@ -864,10 +857,9 @@ bool idmap_autorid_parse_configstr(const char *configstr,
}
NTSTATUS idmap_autorid_loadconfig(struct db_context *db,
- TALLOC_CTX *mem_ctx,
- struct autorid_global_config **result)
+ struct autorid_global_config *result)
{
- struct autorid_global_config *cfg;
+ struct autorid_global_config cfg = {0};
NTSTATUS status;
bool ok;
char *configstr = NULL;
@@ -876,26 +868,20 @@ NTSTATUS idmap_autorid_loadconfig(struct db_context *db,
return NT_STATUS_INVALID_PARAMETER;
}
- status = idmap_autorid_getconfigstr(db, mem_ctx, &configstr);
+ status = idmap_autorid_getconfigstr(db, db, &configstr);
if (!NT_STATUS_IS_OK(status)) {
return status;
}
- cfg = talloc_zero(mem_ctx, struct autorid_global_config);
- if (cfg == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
-
- ok = idmap_autorid_parse_configstr(configstr, cfg);
+ ok = idmap_autorid_parse_configstr(configstr, &cfg);
TALLOC_FREE(configstr);
if (!ok) {
- talloc_free(cfg);
return NT_STATUS_INVALID_PARAMETER;
}
DEBUG(10, ("Loaded previously stored configuration "
"minvalue:%d rangesize:%d\n",
- cfg->minvalue, cfg->rangesize));
+ cfg.minvalue, cfg.rangesize));
*result = cfg;
@@ -906,7 +892,7 @@ NTSTATUS idmap_autorid_saveconfig(struct db_context *db,
struct autorid_global_config *cfg)
{
- struct autorid_global_config *storedconfig = NULL;
+ struct autorid_global_config storedconfig = {0};
NTSTATUS status = NT_STATUS_INVALID_PARAMETER;
TDB_DATA data;
char *cfgstr;
@@ -928,10 +914,11 @@ NTSTATUS idmap_autorid_saveconfig(struct db_context *db,
goto done;
}
- status = idmap_autorid_loadconfig(db, frame, &storedconfig);
+ status = idmap_autorid_loadconfig(db, &storedconfig);
if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
DEBUG(5, ("No configuration found. Storing initial "
"configuration.\n"));
+ storedconfig = *cfg;
} else if (!NT_STATUS_IS_OK(status)) {
DEBUG(1, ("Error loading configuration: %s\n",
nt_errstr(status)));
@@ -939,9 +926,8 @@ NTSTATUS idmap_autorid_saveconfig(struct db_context *db,
}
/* did the minimum value or rangesize change? */
- if (storedconfig &&
- ((storedconfig->minvalue != cfg->minvalue) ||
- (storedconfig->rangesize != cfg->rangesize)))
+ if ((storedconfig.minvalue != cfg->minvalue) ||
+ (storedconfig.rangesize != cfg->rangesize))
{
DEBUG(1, ("New configuration values for rangesize or "
"minimum uid value conflict with previously "
--
2.1.4
More information about the samba-technical
mailing list