The branch, master has been updated
via ea2de21dd8a s4 dsdb util: samdb_client_site_name clean up
via f0e96d21b55 s4 dsdb util: remove samdb_search_count
via 2546f260918 s4 dsdb util: samdb_client_site_name use
dsdb_domain_count
via 7fc379ce867 s4 rpc_server_samr: DomGeneralInformation use
dsdb_domain_count
via 12fcab1181e s4 dsdb util: add dsdb_domain_count
via f3fd2d94579 s2 decrpc samr: Add tests for QueryDomainInfo
from 2346cef9fe8 .gitlab-ci.yml: Make docker image name more explicit
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit ea2de21dd8a2f8dffbb09c635fb34f22d9e344f9
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Thu Feb 14 10:53:37 2019 +1300
s4 dsdb util: samdb_client_site_name clean up
* Initialise pointers to NULL
* replace talloc_free with TALLOC_FREE
* add goto exit to ensure memory deallocated correctly
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Thu Feb 14 06:53:14 CET 2019 on sn-devel-144
commit f0e96d21b55c03f4610f4a2013cf7fdf2a50e29b
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Thu Feb 14 09:19:21 2019 +1300
s4 dsdb util: remove samdb_search_count
All the uses have been replaced with calls to dsdb_domain_count, so it
is no longer needed.
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 2546f2609189ae6075f626de7eb9f63a083d3386
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Thu Feb 14 09:18:20 2019 +1300
s4 dsdb util: samdb_client_site_name use dsdb_domain_count
Replace the call to samdb_search_count with dsdb_domain_count. As this
is the only remaining caller of samdb_search_count, replacing it will
allow the removal of samdb_search_count.
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 7fc379ce867ed18e1d4ac508bea4a6b90c59300f
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Thu Feb 14 09:33:57 2019 +1300
s4 rpc_server_samr: DomGeneralInformation use dsdb_domain_count
Use dsdb_domain_count instead of samdb_search_count to determine the
number of users, groups and aliases. This gives a performance gain of
around 10%, reduces the total memory allocated and fixes the incorrect
count returned for aliases.
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit 12fcab1181efbda3e8b2ac8e471388e9a8d5c009
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Wed Feb 13 14:36:33 2019 +1300
s4 dsdb util: add dsdb_domain_count
This counts the number of objects that are in the domain,
provided a domain SID was supplied (otherwise it just
counts all the objects).
This routine avoids allocating memory for the full
result set by using a callback.
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
commit f3fd2d9457920b3da3e0ce6c1c6ee77be6849c65
Author: Gary Lockyer <g...@catalyst.net.nz>
Date: Wed Feb 13 14:34:06 2019 +1300
s2 decrpc samr: Add tests for QueryDomainInfo
Add tests for the number of domain users, groups and aliases returned by
QueryDomainInfo.
These tests revealed that the existing code was not checking the
returned elements to ensure they were part of the domain.
Signed-off-by: Gary Lockyer <g...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/tests/dcerpc/sam.py | 55 ++++++++
source4/dsdb/common/util.c | 243 ++++++++++++++++++++++++++++------
source4/rpc_server/samr/dcesrv_samr.c | 71 ++++++++--
3 files changed, 318 insertions(+), 51 deletions(-)
Changeset truncated at 500 lines:
diff --git a/python/samba/tests/dcerpc/sam.py b/python/samba/tests/dcerpc/sam.py
index ab710861383..4d787214b1d 100644
--- a/python/samba/tests/dcerpc/sam.py
+++ b/python/samba/tests/dcerpc/sam.py
@@ -20,6 +20,7 @@
"""Tests for samba.dcerpc.sam."""
from samba.dcerpc import samr, security, lsa
+from samba.dcerpc.samr import DomainGeneralInformation
from samba.tests import RpcInterfaceTestCase
from samba.tests import env_loadparm, delete_force
@@ -748,3 +749,57 @@ class SamrTests(RpcInterfaceTestCase):
check_results(expected, actual)
self.delete_dns(dns)
+
+ def test_DomGeneralInformation_num_users(self):
+ info = self.conn.QueryDomainInfo(
+ self.domain_handle, DomainGeneralInformation)
+ #
+ # Enumerate through all the domain users and compare the number
+ # returned against QueryDomainInfo they should be the same
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ count = num_entries
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, resume_handle, 0, max_size)
+ count += num_entries
+
+ self.assertEquals(count, info.num_users)
+
+ def test_DomGeneralInformation_num_groups(self):
+ info = self.conn.QueryDomainInfo(
+ self.domain_handle, DomainGeneralInformation)
+ #
+ # Enumerate through all the domain groups and compare the number
+ # returned against QueryDomainInfo they should be the same
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ count = num_entries
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, resume_handle, max_size)
+ count += num_entries
+
+ self.assertEquals(count, info.num_groups)
+
+ def test_DomGeneralInformation_num_aliases(self):
+ info = self.conn.QueryDomainInfo(
+ self.domain_handle, DomainGeneralInformation)
+ #
+ # Enumerate through all the domain aliases and compare the number
+ # returned against QueryDomainInfo they should be the same
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainAliases(
+ self.domain_handle, 0, max_size)
+ count = num_entries
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainAliases(
+ self.domain_handle, resume_handle, max_size)
+ count += num_entries
+
+ self.assertEquals(count, info.num_aliases)
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 45f0ffc8355..fc7055d800a 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -202,26 +202,6 @@ struct dom_sid *samdb_search_dom_sid(struct ldb_context
*sam_ldb,
return sid;
}
-/*
- return the count of the number of records in the sam matching the query
-*/
-int samdb_search_count(struct ldb_context *sam_ldb,
- TALLOC_CTX *mem_ctx,
- struct ldb_dn *basedn,
- const char *format, ...) _PRINTF_ATTRIBUTE(4,5)
-{
- va_list ap;
- const char *attrs[] = { NULL };
- int ret;
-
- va_start(ap, format);
- ret = gendb_search_v(sam_ldb, mem_ctx, basedn, NULL, attrs, format, ap);
- va_end(ap);
-
- return ret;
-}
-
-
/*
search the sam for a single integer attribute in exactly 1 record
*/
@@ -1893,13 +1873,16 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
bool fallback)
{
const char *attrs[] = { "cn", "siteObject", NULL };
- struct ldb_dn *sites_container_dn, *subnets_dn, *sites_dn;
- struct ldb_result *res;
- const struct ldb_val *val;
- const char *site_name = NULL, *l_subnet_name = NULL;
+ struct ldb_dn *sites_container_dn = NULL;
+ struct ldb_dn *subnets_dn = NULL;
+ struct ldb_dn *sites_dn = NULL;
+ struct ldb_result *res = NULL;
+ const struct ldb_val *val = NULL;
+ const char *site_name = NULL;
+ const char *l_subnet_name = NULL;
const char *allow_list[2] = { NULL, NULL };
unsigned int i, count;
- int cnt, ret;
+ int ret;
/*
* if we don't have a client ip e.g. ncalrpc
@@ -1911,14 +1894,12 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
sites_container_dn = samdb_sites_dn(ldb, mem_ctx);
if (sites_container_dn == NULL) {
- return NULL;
+ goto exit;
}
subnets_dn = ldb_dn_copy(mem_ctx, sites_container_dn);
if ( ! ldb_dn_add_child_fmt(subnets_dn, "CN=Subnets")) {
- talloc_free(sites_container_dn);
- talloc_free(subnets_dn);
- return NULL;
+ goto exit;
}
ret = ldb_search(ldb, mem_ctx, &res, subnets_dn, LDB_SCOPE_ONELEVEL,
@@ -1926,9 +1907,7 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
if (ret == LDB_ERR_NO_SUCH_OBJECT) {
count = 0;
} else if (ret != LDB_SUCCESS) {
- talloc_free(sites_container_dn);
- talloc_free(subnets_dn);
- return NULL;
+ goto exit;
} else {
count = res->count;
}
@@ -1953,7 +1932,7 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
site_name = talloc_strdup(mem_ctx,
(const char *) val->data);
- talloc_free(sites_dn);
+ TALLOC_FREE(sites_dn);
break;
}
@@ -1965,8 +1944,17 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
* is for sure the same as our server site). If more sites do
* exist then we don't know which one to use and set the site
* name to "". */
- cnt = samdb_search_count(ldb, mem_ctx, sites_container_dn,
- "(objectClass=site)");
+ size_t cnt = 0;
+ ret = dsdb_domain_count(
+ ldb,
+ &cnt,
+ sites_container_dn,
+ NULL,
+ LDB_SCOPE_SUBTREE,
+ "(objectClass=site)");
+ if (ret != LDB_SUCCESS) {
+ goto exit;
+ }
if (cnt == 1) {
site_name = samdb_server_site_name(ldb, mem_ctx);
} else {
@@ -1979,9 +1967,10 @@ const char *samdb_client_site_name(struct ldb_context
*ldb, TALLOC_CTX *mem_ctx,
*subnet_name = talloc_strdup(mem_ctx, l_subnet_name);
}
- talloc_free(sites_container_dn);
- talloc_free(subnets_dn);
- talloc_free(res);
+exit:
+ TALLOC_FREE(sites_container_dn);
+ TALLOC_FREE(subnets_dn);
+ TALLOC_FREE(res);
return site_name;
}
@@ -5805,3 +5794,181 @@ bool dsdb_objects_have_same_nc(struct ldb_context *ldb,
return same_nc;
}
+/*
+ * Context for dsdb_count_domain_callback
+ */
+struct dsdb_count_domain_context {
+ /*
+ * Number of matching records
+ */
+ size_t count;
+ /*
+ * sid of the domain that the records must belong to.
+ * if NULL records can belong to any domain.
+ */
+ struct dom_sid *dom_sid;
+};
+
+/*
+ * @brief ldb aysnc callback for dsdb_domain_count.
+ *
+ * count the number of records in the database matching an LDAP query,
+ * optionally filtering for domain membership.
+ *
+ * @param [in,out] req the ldb request being processed
+ * req->context contains:
+ * count The number of matching records
+ * dom_sid The domain sid, if present records must
belong
+ * to the domain to be counted.
+ *@param [in,out] ares The query result.
+ *
+ * @return an LDB error code
+ *
+ */
+static int dsdb_count_domain_callback(
+ struct ldb_request *req,
+ struct ldb_reply *ares)
+{
+
+ if (ares == NULL) {
+ return ldb_request_done(req, LDB_ERR_OPERATIONS_ERROR);
+ }
+ if (ares->error != LDB_SUCCESS) {
+ int error = ares->error;
+ TALLOC_FREE(ares);
+ return ldb_request_done(req, error);
+ }
+
+ switch (ares->type) {
+ case LDB_REPLY_ENTRY:
+ {
+ struct dsdb_count_domain_context *context = NULL;
+ bool ok, in_domain;
+ struct dom_sid sid;
+ const struct ldb_val *v;
+
+ context = req->context;
+ if (context->dom_sid == NULL) {
+ context->count++;
+ break;
+ }
+
+ v = ldb_msg_find_ldb_val(ares->message, "objectSid");
+ if (v == NULL) {
+ break;
+ }
+
+ ok = sid_parse(v->data, v->length, &sid);
+ if (!ok) {
+ break;
+ }
+
+ in_domain = dom_sid_in_domain(context->dom_sid, &sid);
+ if (!in_domain) {
+ break;
+ }
+
+ context->count++;
+ break;
+ }
+ case LDB_REPLY_REFERRAL:
+ break;
+
+ case LDB_REPLY_DONE:
+ TALLOC_FREE(ares);
+ return ldb_request_done(req, LDB_SUCCESS);
+ }
+
+ TALLOC_FREE(ares);
+
+ return LDB_SUCCESS;
+}
+
+/*
+ * @brief Count the number of records matching a query.
+ *
+ * Count the number of entries in the database matching the supplied query,
+ * optionally filtering only those entries belonging to the supplied domain.
+ *
+ * @param ldb [in] Current ldb context
+ * @param count [out] Pointer to the count
+ * @param base [in] The base dn for the quey
+ * @param dom_sid [in] The domain sid, if non NULL records that are not a
member
+ * of the domain are ignored.
+ * @param scope [in] Search scope.
+ * @param exp_fmt [in] format string for the query.
+ *
+ * @return LDB_STATUS code.
+ */
+int dsdb_domain_count(
+ struct ldb_context *ldb,
+ size_t *count,
+ struct ldb_dn *base,
+ struct dom_sid *dom_sid,
+ enum ldb_scope scope,
+ const char *exp_fmt, ...)
+{
+ TALLOC_CTX *tmp_ctx = NULL;
+ struct ldb_request *req = NULL;
+ struct dsdb_count_domain_context *context = NULL;
+ char *expression = NULL;
+ const char *object_sid[] = {"objectSid", NULL};
+ const char *none[] = {NULL};
+ va_list ap;
+ int ret;
+
+ *count = 0;
+ tmp_ctx = talloc_new(ldb);
+
+ context = talloc_zero(tmp_ctx, struct dsdb_count_domain_context);
+ if (context == NULL) {
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ context->dom_sid = dom_sid;
+
+ if (exp_fmt) {
+ va_start(ap, exp_fmt);
+ expression = talloc_vasprintf(tmp_ctx, exp_fmt, ap);
+ va_end(ap);
+
+ if (expression == NULL) {
+ TALLOC_FREE(context);
+ TALLOC_FREE(tmp_ctx);
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+ }
+
+ ret = ldb_build_search_req(
+ &req,
+ ldb,
+ tmp_ctx,
+ base,
+ scope,
+ expression,
+ (dom_sid == NULL) ? none : object_sid,
+ NULL,
+ context,
+ dsdb_count_domain_callback,
+ NULL);
+ ldb_req_set_location(req, "dsdb_domain_count");
+
+ if (ret != LDB_SUCCESS) goto done;
+
+ ret = ldb_request(ldb, req);
+
+ if (ret == LDB_SUCCESS) {
+ ret = ldb_wait(req->handle, LDB_WAIT_ALL);
+ if (ret == LDB_SUCCESS) {
+ *count = context->count;
+ }
+ }
+
+
+done:
+ TALLOC_FREE(expression);
+ TALLOC_FREE(req);
+ TALLOC_FREE(context);
+ TALLOC_FREE(tmp_ctx);
+
+ return ret;
+}
diff --git a/source4/rpc_server/samr/dcesrv_samr.c
b/source4/rpc_server/samr/dcesrv_samr.c
index 84400284f42..7edb0fbe5b2 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -540,6 +540,10 @@ static NTSTATUS
dcesrv_samr_info_DomGeneralInformation(struct samr_domain_state
struct ldb_message
**dom_msgs,
struct
samr_DomGeneralInformation *info)
{
+ size_t count = 0;
+ const enum ldb_scope scope = LDB_SCOPE_SUBTREE;
+ int ret = 0;
+
/* MS-SAMR 2.2.4.1 - ReplicaSourceNodeName: "domainReplica" attribute */
info->primary.string = ldb_msg_find_attr_as_string(dom_msgs[0],
"domainReplica",
@@ -578,21 +582,62 @@ static NTSTATUS
dcesrv_samr_info_DomGeneralInformation(struct samr_domain_state
break;
}
- info->num_users = samdb_search_count(state->sam_ctx, mem_ctx,
- state->domain_dn,
- "(objectClass=user)");
- info->num_groups = samdb_search_count(state->sam_ctx, mem_ctx,
- state->domain_dn,
-
"(&(objectClass=group)(|(groupType=%d)(groupType=%d)))",
- GTYPE_SECURITY_UNIVERSAL_GROUP,
- GTYPE_SECURITY_GLOBAL_GROUP);
- info->num_aliases = samdb_search_count(state->sam_ctx, mem_ctx,
- state->domain_dn,
-
"(&(objectClass=group)(|(groupType=%d)(groupType=%d)))",
-
GTYPE_SECURITY_BUILTIN_LOCAL_GROUP,
-
GTYPE_SECURITY_DOMAIN_LOCAL_GROUP);
+ /*
+ * Users are not meant to be in BUILTIN
+ * so to speed up the query we do not filter on domain_sid
+ */
+ ret = dsdb_domain_count(
+ state->sam_ctx,
+ &count,
+ state->domain_dn,
+ NULL,
+ scope,
+ "(objectClass=user)");
+ if (ret != LDB_SUCCESS || count > UINT32_MAX) {
+ goto error;
+ }
+ info->num_users = count;
+
+ /*
+ * Groups are not meant to be in BUILTIN
+ * so to speed up the query we do not filter on domain_sid
+ */
+ ret = dsdb_domain_count(
+ state->sam_ctx,
+ &count,
+ state->domain_dn,
+ NULL,
+ scope,
+ "(&(objectClass=group)(|(groupType=%d)(groupType=%d)))",
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP);
+ if (ret != LDB_SUCCESS || count > UINT32_MAX) {
+ goto error;
+ }
+ info->num_groups = count;
+
+ ret = dsdb_domain_count(
+ state->sam_ctx,
+ &count,
+ state->domain_dn,
+ state->domain_sid,
+ scope,
+ "(&(objectClass=group)(|(groupType=%d)(groupType=%d)))",
+ GTYPE_SECURITY_BUILTIN_LOCAL_GROUP,
+ GTYPE_SECURITY_DOMAIN_LOCAL_GROUP);
+ if (ret != LDB_SUCCESS || count > UINT32_MAX) {
+ goto error;
+ }
+ info->num_aliases = count;
return NT_STATUS_OK;
+
+error:
+ if (count > UINT32_MAX) {
+ return NT_STATUS_INTEGER_OVERFLOW;
+ }
+ return dsdb_ldb_err_to_ntstatus(ret);
+
}
/*
--
Samba Shared Repository
