[PATCH] Cache SAMR QueryDisplayInfo, EnumDomainGroups and EnumDomainUsers
Gary Lockyer
gary at catalyst.net.nz
Sun Nov 18 23:01:28 UTC 2018
Updated patches attached, After looking at the code using_uint32_t for
the loop and other counters seemed the most appropriate in this case. As
the protocol input and the outputs are uint32_t and max value of these
values is bounded by the number of elements in the database which is <<
MAX_UINT32
Thanks
Gary
On 15/11/18 21:36, Andreas Schneider wrote:
> On Wednesday, 14 November 2018 21:13:51 CET Gary Lockyer via samba-technical
> wrote:
>> Again with a link to the CI results.
>>
>> Updated patch set, incorporating feedback from Matthias, and Andrews via
>> gitlab.
>>
>> CI results: https://gitlab.com/samba-team/devel/samba/pipelines/36499155
>
> Hi Gary,
>
> the size_t type is usually be used for loop counters, array indexing and
> address arithmetic. The size_t type may be bigger than, equal to, or smaller
> than an unsigned int, and your compiler might make assumptions about it for
> optimization!
>
> Please use size_t instead of 'unsigned'!
>
>
> Thanks,
>
>
> Andreas
>
>
-------------- next part --------------
From 908b726bdcfcd3179bd4074e508865275a40dbef Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 9 Oct 2018 11:09:20 +1300
Subject: [PATCH 01/12] tests samr: Extra tests for samr_QueryDisplayInfo
Add extra tests to test the content returned by samr_QueryDisplayInfo,
which is not tested for the ADDC. Also adds tests for the result
caching added in the following commit.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
python/samba/tests/dcerpc/sam.py | 390 ++++++++++++++++++++++++++++++-
selftest/knownfail.d/samr | 10 +
2 files changed, 399 insertions(+), 1 deletion(-)
create mode 100644 selftest/knownfail.d/samr
diff --git a/python/samba/tests/dcerpc/sam.py b/python/samba/tests/dcerpc/sam.py
index 9c432e3a37e..d02827e8acc 100644
--- a/python/samba/tests/dcerpc/sam.py
+++ b/python/samba/tests/dcerpc/sam.py
@@ -19,9 +19,20 @@
"""Tests for samba.dcerpc.sam."""
-from samba.dcerpc import samr, security
+from samba.dcerpc import samr, security, lsa
from samba.tests import RpcInterfaceTestCase
+from samba.tests import env_loadparm, delete_force
+from samba.credentials import Credentials
+from samba.auth import system_session
+from samba.samdb import SamDB
+from samba.dsdb import (
+ ATYPE_NORMAL_ACCOUNT,
+ ATYPE_WORKSTATION_TRUST,
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP)
+from samba import generate_random_password
+import os
# FIXME: Pidl should be doing this for us
def toArray(handle, array, num_entries):
@@ -33,6 +44,32 @@ class SamrTests(RpcInterfaceTestCase):
def setUp(self):
super(SamrTests, self).setUp()
self.conn = samr.samr("ncalrpc:", self.get_loadparm())
+ self.open_samdb()
+ self.open_domain_handle()
+
+ #
+ # Open the samba database
+ #
+ def open_samdb(self):
+ self.lp = env_loadparm()
+ self.domain = os.environ["DOMAIN"]
+ self.creds = Credentials()
+ self.creds.guess(self.lp)
+ self.session = system_session()
+ self.samdb = SamDB(
+ session_info=self.session, credentials=self.creds, lp=self.lp)
+
+ #
+ # Open a SAMR Domain handle
+ def open_domain_handle(self):
+ self.handle = self.conn.Connect2(
+ None, security.SEC_FLAG_MAXIMUM_ALLOWED)
+
+ self.domain_sid = self.conn.LookupDomain(
+ self.handle, lsa.String(self.domain))
+
+ self.domain_handle = self.conn.OpenDomain(
+ self.handle, security.SEC_FLAG_MAXIMUM_ALLOWED, self.domain_sid)
def test_connect5(self):
(level, info, handle) = self.conn.Connect5(None, 0, 1, samr.ConnectInfo1())
@@ -45,3 +82,354 @@ class SamrTests(RpcInterfaceTestCase):
handle = self.conn.Connect2(None, security.SEC_FLAG_MAXIMUM_ALLOWED)
domains = toArray(*self.conn.EnumDomains(handle, 0, 4294967295))
self.conn.Close(handle)
+
+ # Create groups based on the id list supplied, the id is used to
+ # form a unique name and description.
+ #
+ # returns a list of the created dn's, which can be passed to delete_dns
+ # to clean up after the test has run.
+ def create_groups(self, ids):
+ dns = []
+ for i in ids:
+ name = "SAMR_GRP%d" % i
+ dn = "cn=%s,cn=Users,%s" % (name, self.samdb.domain_dn())
+ delete_force(self.samdb, dn)
+
+ self.samdb.newgroup(name)
+ dns.append(dn)
+ return dns
+
+ # Create user accounts based on the id list supplied, the id is used to
+ # form a unique name and description.
+ #
+ # returns a list of the created dn's, which can be passed to delete_dns
+ # to clean up after the test has run.
+ def create_users(self, ids):
+ dns = []
+ for i in ids:
+ name = "SAMR_USER%d" % i
+ dn = "cn=%s,CN=USERS,%s" % (name, self.samdb.domain_dn())
+ delete_force(self.samdb, dn)
+
+ # We only need the user to exist, we don't need a password
+ self.samdb.newuser(
+ name,
+ password=None,
+ setpassword=False,
+ description="Description for " + name,
+ givenname="given%dname" % i,
+ surname="surname%d" % i)
+ dns.append(dn)
+ return dns
+
+ # Create computer accounts based on the id list supplied, the id is used to
+ # form a unique name and description.
+ #
+ # returns a list of the created dn's, which can be passed to delete_dns
+ # to clean up after the test has run.
+ def create_computers(self, ids):
+ dns = []
+ for i in ids:
+ name = "SAMR_CMP%d" % i
+ dn = "cn=%s,cn=COMPUTERS,%s" % (name, self.samdb.domain_dn())
+ delete_force(self.samdb, dn)
+
+ self.samdb.newcomputer(name, description="Description of " + name)
+ dns.append(dn)
+ return dns
+
+ # Delete the specified dn's.
+ #
+ # Used to clean up entries created by individual tests.
+ #
+ def delete_dns(self, dns):
+ for dn in dns:
+ delete_force(self.samdb, dn)
+
+ # Common tests for QueryDisplayInfo
+ #
+ def _test_QueryDisplayInfo(
+ self, level, check_results, select, attributes, add_elements):
+ #
+ # Get the expected results by querying the samdb database directly.
+ # We do this rather than use a list of expected results as this runs
+ # with other tests so we do not have a known fixed list of elements
+ expected = self.samdb.search(expression=select, attrs=attributes)
+ self.assertTrue(len(expected) > 0)
+
+ #
+ # Perform QueryDisplayInfo with max results greater than the expected
+ # number of results.
+ (ts, rs, actual) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 0, 1024, 4294967295)
+
+ self.assertEquals(len(expected), ts)
+ self.assertEquals(len(expected), rs)
+ check_results(expected, actual.entries)
+
+ #
+ # Perform QueryDisplayInfo with max results set to the number of
+ # results returned from the first query, should return the same results
+ (ts1, rs1, actual1) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 0, rs, 4294967295)
+ self.assertEquals(ts, ts1)
+ self.assertEquals(rs, rs1)
+ check_results(expected, actual1.entries)
+
+ #
+ # Perform QueryDisplayInfo and get the last two results.
+ # Note: We are assuming there are at least three entries
+ self.assertTrue(ts > 2)
+ (ts2, rs2, actual2) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, (ts - 2), 2, 4294967295)
+ self.assertEquals(ts, ts2)
+ self.assertEquals(2, rs2)
+ check_results(list(expected)[-2:], actual2.entries)
+
+ #
+ # Perform QueryDisplayInfo and get the first two results.
+ # Note: We are assuming there are at least three entries
+ self.assertTrue(ts > 2)
+ (ts2, rs2, actual2) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 0, 2, 4294967295)
+ self.assertEquals(ts, ts2)
+ self.assertEquals(2, rs2)
+ check_results(list(expected)[:2], actual2.entries)
+
+ #
+ # Perform QueryDisplayInfo and get two results in the middle of the
+ # list i.e. not the first or the last entry.
+ # Note: We are assuming there are at least four entries
+ self.assertTrue(ts > 3)
+ (ts2, rs2, actual2) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 1, 2, 4294967295)
+ self.assertEquals(ts, ts2)
+ self.assertEquals(2, rs2)
+ check_results(list(expected)[1:2], actual2.entries)
+
+ #
+ # To check that cached values are being returned rather than the
+ # results being re-read from disk we add elements, and request all
+ # but the first result.
+ #
+ dns = add_elements([1000, 1002, 1003, 1004])
+
+ #
+ # Perform QueryDisplayInfo and get all but the first result.
+ # We should be using the cached results so the entries we just added
+ # should not be present
+ (ts3, rs3, actual3) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 1, 1024, 4294967295)
+ self.assertEquals(ts, ts3)
+ self.assertEquals(len(expected) - 1, rs3)
+ check_results(list(expected)[1:], actual3.entries)
+
+ #
+ # Perform QueryDisplayInfo and get all the results.
+ # As the start index is zero we should reread the data from disk and
+ # the added entries should be there
+ new = self.samdb.search(expression=select, attrs=attributes)
+ (ts4, rs4, actual4) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 0, 1024, 4294967295)
+ self.assertEquals(len(expected) + len(dns), ts4)
+ self.assertEquals(len(expected) + len(dns), rs4)
+ check_results(new, actual4.entries)
+
+ # Delete the added DN's and query all but the first entry.
+ # This should ensure the cached results are used and that the
+ # missing entry code is triggered.
+ self.delete_dns(dns)
+ (ts5, rs5, actual5) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, 1, 1024, 4294967295)
+ self.assertEquals(len(expected) + len(dns), ts5)
+ # The deleted results will be filtered from the result set so should
+ # be missing from the returned results.
+ # Note: depending on the GUID order, the first result in the cache may
+ # be a deleted entry, in which case the results will contain all
+ # the expected elements, otherwise the first expected result will
+ # be missing.
+ if rs5 == len(expected):
+ check_results(expected, actual5.entries)
+ elif rs5 == (len(expected) - 1):
+ check_results(list(expected)[1:], actual5.entries)
+ else:
+ self.fail("Incorrect number of entries {0}".format(rs5))
+
+ #
+ # Perform QueryDisplayInfo specifying an index past the end of the
+ # available data.
+ # Should return no data.
+ (ts6, rs6, actual6) = self.conn.QueryDisplayInfo(
+ self.domain_handle, level, ts5, 1, 4294967295)
+ self.assertEquals(ts5, ts6)
+ self.assertEquals(0, rs6)
+
+ self.conn.Close(self.handle)
+
+ # Test for QueryDisplayInfo, Level 1
+ # Returns the sAMAccountName, displayName and description for all
+ # the user accounts.
+ #
+ def test_QueryDisplayInfo_level_1(self):
+ def check_results(expected, actual):
+ # Assume the QueryDisplayInfo and ldb.search return their results
+ # in the same order
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.DispEntryGeneral))
+ self.assertEquals(str(e["sAMAccountName"]),
+ str(a.account_name))
+
+ # The displayName and description are optional.
+ # In the expected results they will be missing, in
+ # samr.DispEntryGeneral the corresponding attribute will have a
+ # length of zero.
+ #
+ if a.full_name.length == 0:
+ self.assertFalse("displayName" in e)
+ else:
+ self.assertEquals(str(e["displayName"]), str(a.full_name))
+
+ if a.description.length == 0:
+ self.assertFalse("description" in e)
+ else:
+ self.assertEquals(str(e["description"]),
+ str(a.description))
+ # Create four user accounts
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_users([1, 2, 3, 4])
+
+ select = "(&(objectclass=user)(sAMAccountType={0}))".format(
+ ATYPE_NORMAL_ACCOUNT)
+ attributes = ["sAMAccountName", "displayName", "description"]
+ self._test_QueryDisplayInfo(
+ 1, check_results, select, attributes, self.create_users)
+
+ self.delete_dns(dns)
+
+ # Test for QueryDisplayInfo, Level 2
+ # Returns the sAMAccountName and description for all
+ # the computer accounts.
+ #
+ def test_QueryDisplayInfo_level_2(self):
+ def check_results(expected, actual):
+ # Assume the QueryDisplayInfo and ldb.search return their results
+ # in the same order
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.DispEntryFull))
+ self.assertEquals(str(e["sAMAccountName"]),
+ str(a.account_name))
+
+ # The description is optional.
+ # In the expected results they will be missing, in
+ # samr.DispEntryGeneral the corresponding attribute will have a
+ # length of zero.
+ #
+ if a.description.length == 0:
+ self.assertFalse("description" in e)
+ else:
+ self.assertEquals(str(e["description"]),
+ str(a.description))
+
+ # Create four computer accounts
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_computers([1, 2, 3, 4])
+
+ select = "(&(objectclass=user)(sAMAccountType={0}))".format(
+ ATYPE_WORKSTATION_TRUST)
+ attributes = ["sAMAccountName", "description"]
+ self._test_QueryDisplayInfo(
+ 2, check_results, select, attributes, self.create_computers)
+
+ self.delete_dns(dns)
+
+ # Test for QueryDisplayInfo, Level 3
+ # Returns the sAMAccountName and description for all
+ # the groups.
+ #
+ def test_QueryDisplayInfo_level_3(self):
+ def check_results(expected, actual):
+ # Assume the QueryDisplayInfo and ldb.search return their results
+ # in the same order
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.DispEntryFullGroup))
+ self.assertEquals(str(e["sAMAccountName"]),
+ str(a.account_name))
+
+ # The description is optional.
+ # In the expected results they will be missing, in
+ # samr.DispEntryGeneral the corresponding attribute will have a
+ # length of zero.
+ #
+ if a.description.length == 0:
+ self.assertFalse("description" in e)
+ else:
+ self.assertEquals(str(e["description"]),
+ str(a.description))
+
+ # Create four groups
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_groups([1, 2, 3, 4])
+
+ select = "(&(|(groupType=%d)(groupType=%d))(objectClass=group))" % (
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP)
+ attributes = ["sAMAccountName", "description"]
+ self._test_QueryDisplayInfo(
+ 3, check_results, select, attributes, self.create_groups)
+
+ self.delete_dns(dns)
+
+ # Test for QueryDisplayInfo, Level 4
+ # Returns the sAMAccountName (as an ASCII string)
+ # for all the user accounts.
+ #
+ def test_QueryDisplayInfo_level_4(self):
+ def check_results(expected, actual):
+ # Assume the QueryDisplayInfo and ldb.search return their results
+ # in the same order
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.DispEntryAscii))
+ self.assertTrue(
+ isinstance(a.account_name, lsa.AsciiStringLarge))
+ self.assertEquals(
+ str(e["sAMAccountName"]), str(a.account_name.string))
+
+ # Create four user accounts
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_users([1, 2, 3, 4])
+
+ select = "(&(objectclass=user)(sAMAccountType={0}))".format(
+ ATYPE_NORMAL_ACCOUNT)
+ attributes = ["sAMAccountName", "displayName", "description"]
+ self._test_QueryDisplayInfo(
+ 4, check_results, select, attributes, self.create_users)
+
+ self.delete_dns(dns)
+
+ # Test for QueryDisplayInfo, Level 5
+ # Returns the sAMAccountName (as an ASCII string)
+ # for all the groups.
+ #
+ def test_QueryDisplayInfo_level_5(self):
+ def check_results(expected, actual):
+ # Assume the QueryDisplayInfo and ldb.search return their results
+ # in the same order
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.DispEntryAscii))
+ self.assertTrue(
+ isinstance(a.account_name, lsa.AsciiStringLarge))
+ self.assertEquals(
+ str(e["sAMAccountName"]), str(a.account_name.string))
+
+ # Create four groups
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_groups([1, 2, 3, 4])
+
+ select = "(&(|(groupType=%d)(groupType=%d))(objectClass=group))" % (
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP)
+ attributes = ["sAMAccountName", "description"]
+ self._test_QueryDisplayInfo(
+ 5, check_results, select, attributes, self.create_groups)
+
+ self.delete_dns(dns)
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
new file mode 100644
index 00000000000..d92b89028b6
--- /dev/null
+++ b/selftest/knownfail.d/samr
@@ -0,0 +1,10 @@
+samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_1\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_2\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_3\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_4\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_5\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_1\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_2\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_3\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_4\(ad_dc_ntvfs:local\)
+samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_5\(ad_dc_ntvfs:local\)
--
2.17.1
From 54e5054521d70e9f47cf16441d4f28dd5f81b3d7 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 9 Oct 2018 11:11:12 +1300
Subject: [PATCH 02/12] tests samr: remove PEP8 warnings
Remove PEP8 warnings from the samr tests.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
python/samba/tests/dcerpc/sam.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/python/samba/tests/dcerpc/sam.py b/python/samba/tests/dcerpc/sam.py
index d02827e8acc..f709ba48950 100644
--- a/python/samba/tests/dcerpc/sam.py
+++ b/python/samba/tests/dcerpc/sam.py
@@ -34,6 +34,7 @@ from samba.dsdb import (
from samba import generate_random_password
import os
+
# FIXME: Pidl should be doing this for us
def toArray(handle, array, num_entries):
return [(entry.idx, entry.name) for entry in array.entries[:num_entries]]
@@ -72,7 +73,8 @@ class SamrTests(RpcInterfaceTestCase):
self.handle, security.SEC_FLAG_MAXIMUM_ALLOWED, self.domain_sid)
def test_connect5(self):
- (level, info, handle) = self.conn.Connect5(None, 0, 1, samr.ConnectInfo1())
+ (level, info, handle) =\
+ self.conn.Connect5(None, 0, 1, samr.ConnectInfo1())
def test_connect2(self):
handle = self.conn.Connect2(None, security.SEC_FLAG_MAXIMUM_ALLOWED)
@@ -80,7 +82,7 @@ class SamrTests(RpcInterfaceTestCase):
def test_EnumDomains(self):
handle = self.conn.Connect2(None, security.SEC_FLAG_MAXIMUM_ALLOWED)
- domains = toArray(*self.conn.EnumDomains(handle, 0, 4294967295))
+ toArray(*self.conn.EnumDomains(handle, 0, 4294967295))
self.conn.Close(handle)
# Create groups based on the id list supplied, the id is used to
--
2.17.1
From 514a789d9e704d966ae00c671ac4bc20cf2185c3 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed, 10 Oct 2018 09:20:25 +1300
Subject: [PATCH 03/12] source4 samr: cache samr_QueryDisplayInfo results
Add a cache of GUID's that matched the last samr_QueryDisplayInfo made on a
domain handle. The cache is cleared if the requested start index is
zero, or if the level does not match that in the cache.
The cache is maintained in the guid_caches array of the dcesrv_handle.
Note: that currently this cache exists for the lifetime of the RPC
handle.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
selftest/knownfail.d/samr | 10 -
source4/rpc_server/samr/dcesrv_samr.c | 449 +++++++++++++++++---------
source4/rpc_server/samr/dcesrv_samr.h | 15 +
3 files changed, 319 insertions(+), 155 deletions(-)
delete mode 100644 selftest/knownfail.d/samr
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
deleted file mode 100644
index d92b89028b6..00000000000
--- a/selftest/knownfail.d/samr
+++ /dev/null
@@ -1,10 +0,0 @@
-samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_1\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_2\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_3\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_4\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_5\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_1\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_2\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_3\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_4\(ad_dc_ntvfs:local\)
-samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_QueryDisplayInfo_level_5\(ad_dc_ntvfs:local\)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 3df0c51dfee..846fe0a1498 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -145,7 +145,62 @@ static NTSTATUS dcesrv_interface_samr_bind(struct dcesrv_call_state *dce_call,
} \
} while (0)
+/*
+ * Clear a GUID cache
+ */
+static void clear_guid_cache(struct samr_guid_cache *cache)
+{
+ cache->handle = 0;
+ cache->size = 0;
+ TALLOC_FREE(cache->entries);
+}
+/*
+ * initialize a GUID cache
+ */
+static void initialize_guid_cache(struct samr_guid_cache *cache)
+{
+ cache->handle = 0;
+ cache->size = 0;
+ cache->entries = NULL;
+}
+
+static NTSTATUS load_guid_cache(
+ struct samr_guid_cache *cache,
+ struct samr_domain_state *d_state,
+ unsigned int ldb_cnt,
+ struct ldb_message **res)
+{
+ NTSTATUS status = NT_STATUS_OK;
+ unsigned int i;
+ TALLOC_CTX *frame = talloc_stackframe();
+
+ clear_guid_cache(cache);
+
+ /*
+ * Store the GUID's in the cache.
+ */
+ cache->handle = 0;
+ cache->size = ldb_cnt;
+ cache->entries = talloc_array(d_state, struct GUID, ldb_cnt);
+ if (cache->entries == NULL) {
+ clear_guid_cache(cache);
+ status = NT_STATUS_NO_MEMORY;
+ goto exit;
+ }
+
+ /*
+ * Extract a list of the GUIDs for all the matching objects
+ * we cache just the GUIDS to reduce the memory overhead of
+ * the result cache.
+ */
+ for (i = 0; i < ldb_cnt; i++) {
+ cache->entries[i] = samdb_result_guid(res[i], "objectGUID");
+ }
+exit:
+ TALLOC_FREE(frame);
+ return status;
+}
/*
samr_Connect
@@ -384,6 +439,7 @@ static NTSTATUS dcesrv_samr_OpenDomain(struct dcesrv_call_state *dce_call, TALLO
const char * const dom_attrs[] = { "cn", NULL};
struct ldb_message **dom_msgs;
int ret;
+ unsigned int i;
ZERO_STRUCTP(r->out.domain_handle);
@@ -435,6 +491,10 @@ static NTSTATUS dcesrv_samr_OpenDomain(struct dcesrv_call_state *dce_call, TALLO
d_state->lp_ctx = dce_call->conn->dce_ctx->lp_ctx;
+ for (i = 0; i < SAMR_LAST_CACHE; i++) {
+ initialize_guid_cache(&d_state->guid_caches[i]);
+ }
+
h_domain = dcesrv_handle_new(dce_call->context, SAMR_HANDLE_DOMAIN);
if (!h_domain) {
talloc_free(d_state);
@@ -3707,88 +3767,164 @@ static NTSTATUS dcesrv_samr_GetGroupsForUser(struct dcesrv_call_state *dce_call,
return NT_STATUS_OK;
}
-
/*
- samr_QueryDisplayInfo
-*/
+ * samr_QueryDisplayInfo
+ *
+ * A cache of the GUID's matching the last query is maintained
+ * in the SAMR_QUERY_DISPLAY_INFO_CACHE guid_cache maintained o
+ * n the dcesrv_handle.
+ */
static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
struct samr_QueryDisplayInfo *r)
{
struct dcesrv_handle *h;
struct samr_domain_state *d_state;
struct ldb_result *res;
- unsigned int i;
- uint32_t count;
- const char * const attrs[] = { "objectSid", "sAMAccountName",
- "displayName", "description", "userAccountControl",
- "pwdLastSet", NULL };
+ uint32_t i;
+ uint32_t results = 0;
+ uint32_t count = 0;
+ const char *const cache_attrs[] = {"objectGUID", NULL};
+ const char *const attrs[] = {
+ "objectSID", "sAMAccountName", "displayName", "description", NULL};
struct samr_DispEntryFull *entriesFull = NULL;
struct samr_DispEntryFullGroup *entriesFullGroup = NULL;
struct samr_DispEntryAscii *entriesAscii = NULL;
struct samr_DispEntryGeneral *entriesGeneral = NULL;
const char *filter;
int ret;
+ NTSTATUS status;
+ struct samr_guid_cache *cache = NULL;
DCESRV_PULL_HANDLE(h, r->in.domain_handle, SAMR_HANDLE_DOMAIN);
d_state = h->data;
- switch (r->in.level) {
- case 1:
- case 4:
- filter = talloc_asprintf(mem_ctx, "(&(objectclass=user)"
- "(sAMAccountType=%d))",
- ATYPE_NORMAL_ACCOUNT);
- break;
- case 2:
- filter = talloc_asprintf(mem_ctx, "(&(objectclass=user)"
- "(sAMAccountType=%d))",
- ATYPE_WORKSTATION_TRUST);
- break;
- case 3:
- case 5:
- filter = talloc_asprintf(mem_ctx,
- "(&(|(groupType=%d)(groupType=%d))"
- "(objectClass=group))",
- GTYPE_SECURITY_UNIVERSAL_GROUP,
- GTYPE_SECURITY_GLOBAL_GROUP);
- break;
- default:
- return NT_STATUS_INVALID_INFO_CLASS;
- }
+ cache = &d_state->guid_caches[SAMR_QUERY_DISPLAY_INFO_CACHE];
+ /*
+ * Can the cached results be used?
+ * The cache is discarded if the start index is zero, or the requested
+ * level is different from that in the cache.
+ */
+ if ((r->in.start_idx == 0) || (r->in.level != cache->handle)) {
+ /*
+ * The cached results can not be used, so will need to query
+ * the database.
+ */
- /* search for all requested objects in all domains. This could
- possibly be cached and resumed based on resume_key */
- ret = dsdb_search(d_state->sam_ctx, mem_ctx, &res, ldb_get_default_basedn(d_state->sam_ctx),
- LDB_SCOPE_SUBTREE, attrs, 0, "%s", filter);
- if (ret != LDB_SUCCESS) {
- return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ /*
+ * Get the search filter for the current level
+ */
+ switch (r->in.level) {
+ case 1:
+ case 4:
+ filter = talloc_asprintf(mem_ctx,
+ "(&(objectclass=user)"
+ "(sAMAccountType=%d))",
+ ATYPE_NORMAL_ACCOUNT);
+ break;
+ case 2:
+ filter = talloc_asprintf(mem_ctx,
+ "(&(objectclass=user)"
+ "(sAMAccountType=%d))",
+ ATYPE_WORKSTATION_TRUST);
+ break;
+ case 3:
+ case 5:
+ filter =
+ talloc_asprintf(mem_ctx,
+ "(&(|(groupType=%d)(groupType=%d))"
+ "(objectClass=group))",
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP);
+ break;
+ default:
+ return NT_STATUS_INVALID_INFO_CLASS;
+ }
+ clear_guid_cache(cache);
+
+ /*
+ * search for all requested objects in all domains.
+ */
+ ret = dsdb_search(d_state->sam_ctx,
+ mem_ctx,
+ &res,
+ ldb_get_default_basedn(d_state->sam_ctx),
+ LDB_SCOPE_SUBTREE,
+ guid_attr,
+ 0,
+ "%s",
+ filter);
+ if (ret != LDB_SUCCESS) {
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ }
+ if ((res->count == 0) || (r->in.max_entries == 0)) {
+ return NT_STATUS_OK;
+ }
+
+ status = load_guid_cache(cache, d_state, res->count, res->msgs);
+ TALLOC_FREE(res);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ cache->handle = r->in.level;
}
- if ((res->count == 0) || (r->in.max_entries == 0)) {
+ *r->out.total_size = cache->size;
+
+ /*
+ * if there are no entries or the requested start index is greater
+ * than the number of entries, we return an empty response.
+ */
+ if (r->in.start_idx >= cache->size) {
+ *r->out.returned_size = 0;
+ switch(r->in.level) {
+ case 1:
+ r->out.info->info1.count = *r->out.returned_size;
+ r->out.info->info1.entries = NULL;
+ break;
+ case 2:
+ r->out.info->info2.count = *r->out.returned_size;
+ r->out.info->info2.entries = NULL;
+ break;
+ case 3:
+ r->out.info->info3.count = *r->out.returned_size;
+ r->out.info->info3.entries = NULL;
+ break;
+ case 4:
+ r->out.info->info4.count = *r->out.returned_size;
+ r->out.info->info4.entries = NULL;
+ break;
+ case 5:
+ r->out.info->info5.count = *r->out.returned_size;
+ r->out.info->info5.entries = NULL;
+ break;
+ }
return NT_STATUS_OK;
}
+ /*
+ * Allocate an array of the appropriate result structures for the
+ * current query level.
+ *
+ * r->in.start_idx is always < cache->size due to the check above
+ */
+ results = MIN((cache->size - r->in.start_idx), r->in.max_entries);
switch (r->in.level) {
case 1:
- entriesGeneral = talloc_array(mem_ctx,
- struct samr_DispEntryGeneral,
- res->count);
+ entriesGeneral = talloc_array(
+ mem_ctx, struct samr_DispEntryGeneral, results);
break;
case 2:
- entriesFull = talloc_array(mem_ctx,
- struct samr_DispEntryFull,
- res->count);
+ entriesFull =
+ talloc_array(mem_ctx, struct samr_DispEntryFull, results);
break;
case 3:
- entriesFullGroup = talloc_array(mem_ctx,
- struct samr_DispEntryFullGroup,
- res->count);
+ entriesFullGroup = talloc_array(
+ mem_ctx, struct samr_DispEntryFullGroup, results);
break;
case 4:
case 5:
- entriesAscii = talloc_array(mem_ctx,
- struct samr_DispEntryAscii,
- res->count);
+ entriesAscii =
+ talloc_array(mem_ctx, struct samr_DispEntryAscii, results);
break;
}
@@ -3796,135 +3932,158 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
(entriesAscii == NULL) && (entriesFullGroup == NULL))
return NT_STATUS_NO_MEMORY;
+ /*
+ * Process the list of result GUID's.
+ * Read the details of each object and populate the result structure
+ * for the current level.
+ */
count = 0;
-
- for (i = 0; i < res->count; i++) {
+ for (i = 0; i < results; i++) {
struct dom_sid *objectsid;
+ struct ldb_result *rec;
+ const uint32_t idx = r->in.start_idx + i;
+ uint32_t rid;
- objectsid = samdb_result_dom_sid(mem_ctx, res->msgs[i],
- "objectSid");
- if (objectsid == NULL)
+ /*
+ * Read an object from disk using the GUID as the key
+ *
+ * If the object can not be read, or it does not have a SID
+ * it is ignored. In this case the number of entries returned
+ * will be less than the requested size, there will also be
+ * a gap in the idx numbers in the returned elements e.g. if
+ * there are 3 GUIDs a, b, c in the cache and b is deleted from
+ * disk then details for a, and c will be returned with
+ * idx values of 1 and 3 respectively.
+ *
+ */
+ ret = dsdb_search_by_dn_guid(d_state->sam_ctx,
+ mem_ctx,
+ &rec,
+ &cache->entries[idx],
+ attrs,
+ 0);
+ if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+ struct GUID_txt_buf guid_buf;
+ char *guid_str =
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
+ DBG_WARNING("GUID [%s] not found\n", guid_str);
continue;
+ } else if (ret != LDB_SUCCESS) {
+ clear_guid_cache(cache);
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ }
+ objectsid = samdb_result_dom_sid(mem_ctx,
+ rec->msgs[0],
+ "objectSID");
+ if (objectsid == NULL) {
+ struct GUID_txt_buf guid_buf;
+ char *guid_str =
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
+ DBG_WARNING("objectSID for GUID [%s] not found\n",
+ guid_str);
+ continue;
+ }
+ /*
+ * Populate the result structure for the current object
+ */
switch(r->in.level) {
case 1:
- entriesGeneral[count].idx = count + 1;
+
+ entriesGeneral[count].idx = idx + 1;
entriesGeneral[count].rid =
- objectsid->sub_auths[objectsid->num_auths-1];
+ objectsid->sub_auths[objectsid->num_auths - 1];
entriesGeneral[count].acct_flags =
- samdb_result_acct_flags(res->msgs[i], NULL);
+ samdb_result_acct_flags(rec->msgs[0], NULL);
entriesGeneral[count].account_name.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "sAMAccountName", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
entriesGeneral[count].full_name.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "displayName", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "displayName", "");
entriesGeneral[count].description.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "description", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "description", "");
break;
case 2:
- entriesFull[count].idx = count + 1;
+ entriesFull[count].idx = idx + 1;
entriesFull[count].rid =
- objectsid->sub_auths[objectsid->num_auths-1];
+ objectsid->sub_auths[objectsid->num_auths - 1];
- /* No idea why we need to or in ACB_NORMAL here, but this is what Win2k3 seems to do... */
+ /*
+ * No idea why we need to or in ACB_NORMAL here,
+ * but this is what Win2k3 seems to do...
+ */
entriesFull[count].acct_flags =
- samdb_result_acct_flags(res->msgs[i],
- NULL) | ACB_NORMAL;
+ samdb_result_acct_flags(rec->msgs[0], NULL) |
+ ACB_NORMAL;
entriesFull[count].account_name.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "sAMAccountName", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
entriesFull[count].description.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "description", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "description", "");
break;
case 3:
- entriesFullGroup[count].idx = count + 1;
+ entriesFullGroup[count].idx = idx + 1;
entriesFullGroup[count].rid =
- objectsid->sub_auths[objectsid->num_auths-1];
- /* We get a "7" here for groups */
- entriesFullGroup[count].acct_flags
- = SE_GROUP_MANDATORY | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_ENABLED;
+ objectsid->sub_auths[objectsid->num_auths - 1];
+ /*
+ * We get a "7" here for groups
+ */
+ entriesFullGroup[count].acct_flags =
+ SE_GROUP_MANDATORY | SE_GROUP_ENABLED_BY_DEFAULT |
+ SE_GROUP_ENABLED;
entriesFullGroup[count].account_name.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "sAMAccountName", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
entriesFullGroup[count].description.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "description", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "description", "");
break;
case 4:
case 5:
- entriesAscii[count].idx = count + 1;
+ entriesAscii[count].idx = idx + 1;
entriesAscii[count].account_name.string =
- ldb_msg_find_attr_as_string(res->msgs[i],
- "sAMAccountName", "");
+ ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
break;
}
-
- count += 1;
+ count++;
}
- *r->out.total_size = count;
-
- if (r->in.start_idx >= count) {
- *r->out.returned_size = 0;
- switch(r->in.level) {
- case 1:
- r->out.info->info1.count = *r->out.returned_size;
- r->out.info->info1.entries = NULL;
- break;
- case 2:
- r->out.info->info2.count = *r->out.returned_size;
- r->out.info->info2.entries = NULL;
- break;
- case 3:
- r->out.info->info3.count = *r->out.returned_size;
- r->out.info->info3.entries = NULL;
- break;
- case 4:
- r->out.info->info4.count = *r->out.returned_size;
- r->out.info->info4.entries = NULL;
- break;
- case 5:
- r->out.info->info5.count = *r->out.returned_size;
- r->out.info->info5.entries = NULL;
- break;
- }
- } else {
- *r->out.returned_size = MIN(count - r->in.start_idx,
- r->in.max_entries);
- switch(r->in.level) {
- case 1:
- r->out.info->info1.count = *r->out.returned_size;
- r->out.info->info1.entries =
- &(entriesGeneral[r->in.start_idx]);
- break;
- case 2:
- r->out.info->info2.count = *r->out.returned_size;
- r->out.info->info2.entries =
- &(entriesFull[r->in.start_idx]);
- break;
- case 3:
- r->out.info->info3.count = *r->out.returned_size;
- r->out.info->info3.entries =
- &(entriesFullGroup[r->in.start_idx]);
- break;
- case 4:
- r->out.info->info4.count = *r->out.returned_size;
- r->out.info->info4.entries =
- &(entriesAscii[r->in.start_idx]);
- break;
- case 5:
- r->out.info->info5.count = *r->out.returned_size;
- r->out.info->info5.entries =
- &(entriesAscii[r->in.start_idx]);
- break;
- }
+ /*
+ * Build the response based on the request level.
+ */
+ *r->out.returned_size = count;
+ switch(r->in.level) {
+ case 1:
+ r->out.info->info1.count = count;
+ r->out.info->info1.entries = entriesGeneral;
+ break;
+ case 2:
+ r->out.info->info2.count = count;
+ r->out.info->info2.entries = entriesFull;
+ break;
+ case 3:
+ r->out.info->info3.count = count;
+ r->out.info->info3.entries = entriesFullGroup;
+ break;
+ case 4:
+ r->out.info->info4.count = count;
+ r->out.info->info4.entries = entriesAscii;
+ break;
+ case 5:
+ r->out.info->info5.count = count;
+ r->out.info->info5.entries = entriesAscii;
+ break;
}
- return (*r->out.returned_size < (count - r->in.start_idx)) ?
- STATUS_MORE_ENTRIES : NT_STATUS_OK;
+ return ((r->in.start_idx + results) < cache->size)
+ ? STATUS_MORE_ENTRIES
+ : NT_STATUS_OK;
}
diff --git a/source4/rpc_server/samr/dcesrv_samr.h b/source4/rpc_server/samr/dcesrv_samr.h
index 261bd052efe..f08bac053c8 100644
--- a/source4/rpc_server/samr/dcesrv_samr.h
+++ b/source4/rpc_server/samr/dcesrv_samr.h
@@ -42,6 +42,20 @@ struct samr_connect_state {
uint32_t access_mask;
};
+/*
+ * Cache of object GUIDS
+ */
+struct samr_guid_cache {
+ unsigned handle;
+ unsigned size;
+ struct GUID *entries;
+};
+
+enum samr_guid_cache_id {
+ SAMR_QUERY_DISPLAY_INFO_CACHE,
+ SAMR_LAST_CACHE
+};
+
/*
state associated with a samr_OpenDomain() operation
*/
@@ -55,6 +69,7 @@ struct samr_domain_state {
enum server_role role;
bool builtin;
struct loadparm_context *lp_ctx;
+ struct samr_guid_cache guid_caches[SAMR_LAST_CACHE];
};
/*
--
2.17.1
From 382d4bff7e0539716c10a305f8e1dd7478b0a4c1 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Nov 2018 09:53:25 +1300
Subject: [PATCH 04/12] s4-samr: Use dom_sid_split_rid() to get the RID in
dcesrv_samr_QueryDisplayInfo
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/samr/dcesrv_samr.c | 29 +++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 846fe0a1498..18eb2c4253b 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -3985,6 +3985,24 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
guid_str);
continue;
}
+ status = dom_sid_split_rid(NULL,
+ objectsid,
+ NULL,
+ &rid);
+ if (!NT_STATUS_IS_OK(status)) {
+ struct dom_sid_buf sid_buf;
+ char *sid_str =
+ dom_sid_str_buf(objectsid,
+ &sid_buf);
+ struct GUID_txt_buf guid_buf;
+ char *guid_str =
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
+ DBG_WARNING("objectSID [%s] for GUID [%s] invalid\n",
+ sid_str,
+ guid_str);
+ continue;
+ }
/*
* Populate the result structure for the current object
@@ -3993,8 +4011,8 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
case 1:
entriesGeneral[count].idx = idx + 1;
- entriesGeneral[count].rid =
- objectsid->sub_auths[objectsid->num_auths - 1];
+ entriesGeneral[count].rid = rid;
+
entriesGeneral[count].acct_flags =
samdb_result_acct_flags(rec->msgs[0], NULL);
entriesGeneral[count].account_name.string =
@@ -4009,8 +4027,7 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
break;
case 2:
entriesFull[count].idx = idx + 1;
- entriesFull[count].rid =
- objectsid->sub_auths[objectsid->num_auths - 1];
+ entriesFull[count].rid = rid;
/*
* No idea why we need to or in ACB_NORMAL here,
@@ -4028,8 +4045,8 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
break;
case 3:
entriesFullGroup[count].idx = idx + 1;
- entriesFullGroup[count].rid =
- objectsid->sub_auths[objectsid->num_auths - 1];
+ entriesFullGroup[count].rid = rid;
+
/*
* We get a "7" here for groups
*/
--
2.17.1
From bb507cb1311f187a2ddc56e2e6ad6e61af8a6d54 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 12 Oct 2018 11:21:10 +1300
Subject: [PATCH 05/12] test samr: Extra tests for samr_EnumDomainGroups
Add extra tests to test the content returned by samr_EnumDomainGroups,
and tests for the result caching added in the following commit.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
python/samba/tests/dcerpc/sam.py | 169 +++++++++++++++++++++++++++++++
selftest/knownfail.d/samr | 2 +
2 files changed, 171 insertions(+)
create mode 100644 selftest/knownfail.d/samr
diff --git a/python/samba/tests/dcerpc/sam.py b/python/samba/tests/dcerpc/sam.py
index f709ba48950..90455f04266 100644
--- a/python/samba/tests/dcerpc/sam.py
+++ b/python/samba/tests/dcerpc/sam.py
@@ -32,6 +32,7 @@ from samba.dsdb import (
GTYPE_SECURITY_UNIVERSAL_GROUP,
GTYPE_SECURITY_GLOBAL_GROUP)
from samba import generate_random_password
+from samba.ndr import ndr_unpack
import os
@@ -40,6 +41,24 @@ def toArray(handle, array, num_entries):
return [(entry.idx, entry.name) for entry in array.entries[:num_entries]]
+# Extract the rid from an ldb message, assumes that the message has a
+# objectSID attribute
+#
+def rid(msg):
+ sid = ndr_unpack(security.dom_sid, msg["objectSID"][0])
+ (_, rid) = sid.split()
+ return rid
+
+
+# Calculate the request size for EnumDomainUsers and EnumDomainGroups calls
+# to hold the specified number of entries.
+# We use the w2k3 element size value of 54, code under test
+# rounds this up i.e. (1+(max_size/SAMR_ENUM_USERS_MULTIPLIER))
+#
+def calc_max_size(num_entries):
+ return (num_entries - 1) * 54
+
+
class SamrTests(RpcInterfaceTestCase):
def setUp(self):
@@ -72,6 +91,18 @@ class SamrTests(RpcInterfaceTestCase):
self.domain_handle = self.conn.OpenDomain(
self.handle, security.SEC_FLAG_MAXIMUM_ALLOWED, self.domain_sid)
+ # Filter a list of records, removing those that are not part of the
+ # current domain.
+ #
+ def filter_domain(self, unfiltered):
+ def sid(msg):
+ sid = ndr_unpack(security.dom_sid, msg["objectSID"][0])
+ (x, _) = sid.split()
+ return x
+
+ dom_sid = security.dom_sid(self.samdb.get_domain_sid())
+ return [x for x in unfiltered if sid(x) == dom_sid]
+
def test_connect5(self):
(level, info, handle) =\
self.conn.Connect5(None, 0, 1, samr.ConnectInfo1())
@@ -435,3 +466,141 @@ class SamrTests(RpcInterfaceTestCase):
5, check_results, select, attributes, self.create_groups)
self.delete_dns(dns)
+
+ def test_EnumDomainUsers(self):
+ def check_results(expected, actual):
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.SamEntry))
+ self.assertEquals(
+ str(e["sAMAccountName"]), str(a.name.string))
+
+ # Create four groups
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_groups([1, 2, 3, 4])
+
+ #
+ # Get the expected results by querying the samdb database directly.
+ # We do this rather than use a list of expected results as this runs
+ # with other tests so we do not have a known fixed list of elements
+ select = "(&(|(groupType=%d)(groupType=%d))(objectClass=group))" % (
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP)
+ attributes = ["sAMAccountName", "objectSID"]
+ unfiltered = self.samdb.search(expression=select, attrs=attributes)
+ filtered = self.filter_domain(unfiltered)
+ self.assertTrue(len(filtered) > 4)
+
+ # Sort the expected results by rid
+ expected = sorted(list(filtered), key=rid)
+
+ #
+ # Perform EnumDomainGroups with max size greater than the expected
+ # number of results. Allow for an extra 10 entries
+ #
+ max_size = calc_max_size(len(expected) + 10)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ self.assertEquals(len(expected), num_entries)
+ check_results(expected, actual.entries)
+
+ #
+ # Perform EnumDomainGroups with size set to so that it contains
+ # 4 entries.
+ #
+ max_size = calc_max_size(4)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ self.assertEquals(4, num_entries)
+ check_results(expected[:4], actual.entries)
+
+ #
+ # Try calling with resume_handle greater than number of entries
+ # Should return no results and a resume handle of 0
+ max_size = calc_max_size(1)
+ rh = len(expected)
+ self.conn.Close(self.handle)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, rh, max_size)
+
+ self.assertEquals(0, num_entries)
+ self.assertEquals(0, resume_handle)
+
+ #
+ # Enumerate through the domain groups one element at a time.
+ #
+ max_size = calc_max_size(1)
+ actual = []
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, resume_handle, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ #
+ # Check that the cached results are being returned.
+ # Obtain a new resume_handle and insert new entries into the
+ # into the DB
+ #
+ actual = []
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ extra_dns = self.create_groups([1000, 1002, 1003, 1004])
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, resume_handle, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ self.assertEquals(len(expected), len(actual))
+ check_results(expected, actual)
+
+ #
+ # Perform EnumDomainGroups, we should read the newly added domains
+ #
+ max_size = calc_max_size(len(expected) + len(extra_dns) + 10)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ self.assertEquals(len(expected) + len(extra_dns), num_entries)
+
+ #
+ # Get a new expected result set by querying the database directly
+ unfiltered01 = self.samdb.search(expression=select, attrs=attributes)
+ filtered01 = self.filter_domain(unfiltered01)
+ self.assertTrue(len(filtered01) > len(expected))
+
+ # Sort the expected results by rid
+ expected01 = sorted(list(filtered01), key=rid)
+
+ #
+ # Now check that we read the new entries.
+ #
+ check_results(expected01, actual.entries)
+
+ #
+ # Check that deleted results are handled correctly.
+ # Obtain a new resume_handle and delete entries from the DB.
+ #
+ actual = []
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, 0, max_size)
+ self.delete_dns(extra_dns)
+ while resume_handle and num_entries:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainGroups(
+ self.domain_handle, resume_handle, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ self.assertEquals(len(expected), len(actual))
+ check_results(expected, actual)
+
+ self.delete_dns(dns)
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
new file mode 100644
index 00000000000..1bf809eead0
--- /dev/null
+++ b/selftest/knownfail.d/samr
@@ -0,0 +1,2 @@
+^samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers\(ad_dc_ntvfs:local\)
+^samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers\(ad_dc_ntvfs:local\)
--
2.17.1
From b5c0ea753da6bd4e5f5e9dac6b9000b03cdc21cf Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Thu, 18 Oct 2018 10:16:24 +1300
Subject: [PATCH 06/12] source4 samr: cache samr_EnumDomainGroups results
Add a cache of GUID's that matched the last samr_EnunDomainGroups made on a
domain handle. The cache is cleared if resume_handle is zero, and when the
final results are returned to the caller.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
selftest/knownfail.d/samr | 2 -
source4/rpc_server/samr/dcesrv_samr.c | 263 +++++++++++++++++++++-----
source4/rpc_server/samr/dcesrv_samr.h | 1 +
3 files changed, 212 insertions(+), 54 deletions(-)
delete mode 100644 selftest/knownfail.d/samr
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
deleted file mode 100644
index 1bf809eead0..00000000000
--- a/selftest/knownfail.d/samr
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers\(ad_dc_ntvfs:local\)
-^samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers\(ad_dc_ntvfs:local\)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 18eb2c4253b..0a22c7aad2e 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1125,6 +1125,63 @@ static int compare_SamEntry(struct samr_SamEntry *e1, struct samr_SamEntry *e2)
return e1->idx - e2->idx;
}
+static int compare_msgRid(struct ldb_message **m1, struct ldb_message **m2) {
+ struct dom_sid *sid1 = NULL;
+ struct dom_sid *sid2 = NULL;
+ uint32_t rid1;
+ uint32_t rid2;
+ int res = 0;
+ NTSTATUS status;
+ TALLOC_CTX *frame = talloc_stackframe();
+
+ sid1 = samdb_result_dom_sid(frame, *m1, "objectSid");
+ sid2 = samdb_result_dom_sid(frame, *m2, "objectSid");
+
+ /*
+ * If entries don't have a SID we want to sort them to the end of
+ * the list.
+ */
+ if (sid1 == NULL && sid2 == NULL) {
+ res = 0;
+ goto exit;
+ } else if (sid2 == NULL) {
+ res = 1;
+ goto exit;
+ } else if (sid1 == NULL) {
+ res = -1;
+ goto exit;
+ }
+
+ /*
+ * Get and compare the rids, if we fail to extract a rid treat it as a
+ * missing SID and sort to the end of the list
+ */
+ status = dom_sid_split_rid(NULL, sid1, NULL, &rid1);
+ if (!NT_STATUS_IS_OK(status)) {
+ res = 1;
+ goto exit;
+ }
+
+ status = dom_sid_split_rid(NULL, sid2, NULL, &rid2);
+ if (!NT_STATUS_IS_OK(status)) {
+ res = -1;
+ goto exit;
+ }
+
+ if (rid1 == rid2) {
+ res = 0;
+ }
+ else if (rid1 > rid2) {
+ res = 1;
+ }
+ else {
+ res = -1;
+ }
+exit:
+ TALLOC_FREE(frame);
+ return res;
+}
+
/*
samr_EnumDomainGroups
*/
@@ -1134,11 +1191,17 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
struct dcesrv_handle *h;
struct samr_domain_state *d_state;
struct ldb_message **res;
- int i, ldb_cnt;
- uint32_t first, count;
+ uint32_t i;
+ uint32_t count;
+ uint32_t results;
+ uint32_t max_entries;
+ uint32_t remaining_entries;
+ uint32_t resume_handle;
struct samr_SamEntry *entries;
const char * const attrs[] = { "objectSid", "sAMAccountName", NULL };
+ const char * const cache_attrs[] = { "objectSid", "objectGUID", NULL };
struct samr_SamArray *sam;
+ struct samr_guid_cache *cache = NULL;
*r->out.resume_handle = 0;
*r->out.sam = NULL;
@@ -1147,77 +1210,173 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
DCESRV_PULL_HANDLE(h, r->in.domain_handle, SAMR_HANDLE_DOMAIN);
d_state = h->data;
+ cache = &d_state->guid_caches[SAMR_ENUM_DOMAIN_GROUPS_CACHE];
- /* search for all domain groups in this domain. This could possibly be
- cached and resumed based on resume_key */
- ldb_cnt = samdb_search_domain(d_state->sam_ctx, mem_ctx,
- d_state->domain_dn, &res, attrs,
- d_state->domain_sid,
- "(&(|(groupType=%d)(groupType=%d))(objectClass=group))",
- GTYPE_SECURITY_UNIVERSAL_GROUP,
- GTYPE_SECURITY_GLOBAL_GROUP);
- if (ldb_cnt < 0) {
- return NT_STATUS_INTERNAL_DB_CORRUPTION;
- }
+ /*
+ * If the resume_handle is zero, query the database and cache the
+ * matching GUID's
+ */
+ if (*r->in.resume_handle == 0) {
+ NTSTATUS status;
+ int ldb_cnt;
+ clear_guid_cache(cache);
+ /*
+ * search for all domain groups in this domain.
+ */
+ ldb_cnt = samdb_search_domain(
+ d_state->sam_ctx,
+ mem_ctx,
+ d_state->domain_dn,
+ &res,
+ cache_attrs,
+ d_state->domain_sid,
+ "(&(|(groupType=%d)(groupType=%d))(objectClass=group))",
+ GTYPE_SECURITY_UNIVERSAL_GROUP,
+ GTYPE_SECURITY_GLOBAL_GROUP);
+ if (ldb_cnt < 0) {
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ }
+ /*
+ * Sort the results into RID order, while the spec states there
+ * is no order, Windows appears to sort the results by RID and
+ * so it is possible that there are clients that depend on
+ * this ordering
+ */
+ TYPESAFE_QSORT(res, ldb_cnt, compare_msgRid);
- /* convert to SamEntry format */
- entries = talloc_array(mem_ctx, struct samr_SamEntry, ldb_cnt);
- if (!entries) {
- return NT_STATUS_NO_MEMORY;
+ /*
+ * cache the sorted GUID's
+ */
+ status = load_guid_cache(cache, d_state, ldb_cnt, res);
+ TALLOC_FREE(res);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ cache->handle = 0;
}
- count = 0;
- for (i=0;i<ldb_cnt;i++) {
- struct dom_sid *group_sid;
-
- group_sid = samdb_result_dom_sid(mem_ctx, res[i],
- "objectSid");
- if (group_sid == NULL) {
- return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ /*
+ * If the resume handle is out of range we return an empty response
+ * and invalidate the cache.
+ *
+ * From the specification:
+ * Servers SHOULD validate that EnumerationContext is an expected
+ * value for the server's implementation. Windows does NOT validate
+ * the input, though the result of malformed information merely results
+ * in inconsistent output to the client.
+ */
+ if (*r->in.resume_handle >= cache->size) {
+ clear_guid_cache(cache);
+ sam = talloc(mem_ctx, struct samr_SamArray);
+ if (!sam) {
+ return NT_STATUS_NO_MEMORY;
}
+ sam->entries = NULL;
+ sam->count = 0;
- entries[count].idx =
- group_sid->sub_auths[group_sid->num_auths-1];
- entries[count].name.string =
- ldb_msg_find_attr_as_string(res[i], "sAMAccountName", "");
- count += 1;
+ *r->out.sam = sam;
+ *r->out.resume_handle = 0;
+ return NT_STATUS_OK;
}
- /* sort the results by rid */
- TYPESAFE_QSORT(entries, count, compare_SamEntry);
- /* find the first entry to return */
- for (first=0;
- first<count && entries[first].idx <= *r->in.resume_handle;
- first++) ;
+ /*
+ * Calculate the number of entries to return limit by max_size.
+ * Note that we use the w2k3 element size value of 54
+ */
+ max_entries = 1 + (r->in.max_size/SAMR_ENUM_USERS_MULTIPLIER);
+ remaining_entries = cache->size - *r->in.resume_handle;
+ results = MIN(remaining_entries, max_entries);
- /* return the rest, limit by max_size. Note that we
- use the w2k3 element size value of 54 */
- *r->out.num_entries = count - first;
- *r->out.num_entries = MIN(*r->out.num_entries,
- 1+(r->in.max_size/SAMR_ENUM_USERS_MULTIPLIER));
+ /*
+ * Process the list of result GUID's.
+ * Read the details of each object and populate the Entries
+ * for the current level.
+ */
+ count = 0;
+ resume_handle = *r->in.resume_handle;
+ entries = talloc_array(mem_ctx, struct samr_SamEntry, results);
+ if (entries == NULL) {
+ clear_guid_cache(cache);
+ return NT_STATUS_NO_MEMORY;
+ }
+ for (i = 0; i < results; i++) {
+ struct dom_sid *sid;
+ struct ldb_result *rec;
+ const uint32_t idx = *r->in.resume_handle + i;
+ int ret;
+ const char *name = NULL;
+
+ resume_handle++;
+ /*
+ * Read an object from disk using the GUID as the key
+ *
+ * If the object can not be read, or it does not have a SID
+ * it is ignored.
+ *
+ * As a consequence of this, if all the remaining GUID's
+ * have been deleted an empty result will be returned.
+ * i.e. even if the previous call returned a non zero
+ * resume_handle it is possible for no results to be returned.
+ *
+ */
+ ret = dsdb_search_by_dn_guid(d_state->sam_ctx,
+ mem_ctx,
+ &rec,
+ &cache->entries[idx],
+ attrs,
+ 0);
+ if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+ char *guid_str =
+ GUID_string(mem_ctx, &cache->entries[idx]);
+ DBG_WARNING("GUID [%s] not found\n", guid_str);
+ continue;
+ } else if (ret != LDB_SUCCESS) {
+ clear_guid_cache(cache);
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ }
+ sid = samdb_result_dom_sid(mem_ctx, rec->msgs[0], "objectSID");
+ if (sid == NULL) {
+ char *guid_str =
+ GUID_string(mem_ctx, &cache->entries[idx]);
+ DBG_WARNING("objectSID for GUID [%s] not found\n",
+ guid_str);
+ continue;
+ }
+ entries[count].idx = sid->sub_auths[sid->num_auths - 1];
+ name = ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
+ entries[count].name.string = talloc_strdup(entries, name);
+ count++;
+ }
sam = talloc(mem_ctx, struct samr_SamArray);
if (!sam) {
+ clear_guid_cache(cache);
return NT_STATUS_NO_MEMORY;
}
- sam->entries = entries+first;
- sam->count = *r->out.num_entries;
+ sam->entries = entries;
+ sam->count = count;
*r->out.sam = sam;
+ *r->out.resume_handle = resume_handle;
+ *r->out.num_entries = count;
- if (first == count) {
+ /*
+ * Signal no more results by returning zero resume handle,
+ * the cache is also cleared at this point
+ */
+ if (*r->out.resume_handle >= cache->size) {
+ *r->out.resume_handle = 0;
+ clear_guid_cache(cache);
return NT_STATUS_OK;
}
-
- if (*r->out.num_entries < count - first) {
- *r->out.resume_handle = entries[first+*r->out.num_entries-1].idx;
- return STATUS_MORE_ENTRIES;
- }
-
- return NT_STATUS_OK;
+ /*
+ * There are more results to be returned.
+ */
+ return STATUS_MORE_ENTRIES;
}
@@ -3850,7 +4009,7 @@ static NTSTATUS dcesrv_samr_QueryDisplayInfo(struct dcesrv_call_state *dce_call,
&res,
ldb_get_default_basedn(d_state->sam_ctx),
LDB_SCOPE_SUBTREE,
- guid_attr,
+ cache_attrs,
0,
"%s",
filter);
diff --git a/source4/rpc_server/samr/dcesrv_samr.h b/source4/rpc_server/samr/dcesrv_samr.h
index f08bac053c8..04866aca757 100644
--- a/source4/rpc_server/samr/dcesrv_samr.h
+++ b/source4/rpc_server/samr/dcesrv_samr.h
@@ -53,6 +53,7 @@ struct samr_guid_cache {
enum samr_guid_cache_id {
SAMR_QUERY_DISPLAY_INFO_CACHE,
+ SAMR_ENUM_DOMAIN_GROUPS_CACHE,
SAMR_LAST_CACHE
};
--
2.17.1
From 0321a841db76ef1bee17ab9597b7a8be1d98d336 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Thu, 18 Oct 2018 13:54:31 +1300
Subject: [PATCH 07/12] s4-samr: Use dom_sid_split_rid() to get the RID in
dcesrv_samr_EnumDomainGroups
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/samr/dcesrv_samr.c | 32 ++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 0a22c7aad2e..83600642784 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1302,12 +1302,13 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
return NT_STATUS_NO_MEMORY;
}
for (i = 0; i < results; i++) {
- struct dom_sid *sid;
+ struct dom_sid *objectsid;
+ uint32_t rid;
struct ldb_result *rec;
const uint32_t idx = *r->in.resume_handle + i;
int ret;
+ NTSTATUS status;
const char *name = NULL;
-
resume_handle++;
/*
* Read an object from disk using the GUID as the key
@@ -1336,15 +1337,36 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
clear_guid_cache(cache);
return NT_STATUS_INTERNAL_DB_CORRUPTION;
}
- sid = samdb_result_dom_sid(mem_ctx, rec->msgs[0], "objectSID");
- if (sid == NULL) {
+
+ objectsid = samdb_result_dom_sid(mem_ctx,
+ rec->msgs[0],
+ "objectSID");
+ if (objectsid == NULL) {
char *guid_str =
GUID_string(mem_ctx, &cache->entries[idx]);
DBG_WARNING("objectSID for GUID [%s] not found\n",
guid_str);
continue;
}
- entries[count].idx = sid->sub_auths[sid->num_auths - 1];
+ status = dom_sid_split_rid(NULL,
+ objectsid,
+ NULL,
+ &rid);
+ if (!NT_STATUS_IS_OK(status)) {
+ struct dom_sid_buf sid_buf;
+ char *sid_str =
+ dom_sid_str_buf(objectsid,
+ &sid_buf);
+ struct GUID_txt_buf guid_buf;
+ char *guid_str =
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
+ DBG_WARNING("objectSID [%s] for GUID [%s] invalid\n",
+ sid_str, guid_str);
+ continue;
+ }
+
+ entries[count].idx = rid;
name = ldb_msg_find_attr_as_string(
rec->msgs[0], "sAMAccountName", "");
entries[count].name.string = talloc_strdup(entries, name);
--
2.17.1
From 6b4f082a6d8ef25ca2c17a9de8bc4adcf323bfd2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Nov 2018 13:44:23 +1300
Subject: [PATCH 08/12] s4-samr: Use GUID_buf_string() in
dcesrv_samr_EnumDomainGroups()
This avoids memory allocation
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/samr/dcesrv_samr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 83600642784..b485b5a2b62 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1329,8 +1329,10 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
attrs,
0);
if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+ struct GUID_txt_buf guid_buf;
char *guid_str =
- GUID_string(mem_ctx, &cache->entries[idx]);
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
DBG_WARNING("GUID [%s] not found\n", guid_str);
continue;
} else if (ret != LDB_SUCCESS) {
@@ -1342,8 +1344,10 @@ static NTSTATUS dcesrv_samr_EnumDomainGroups(struct dcesrv_call_state *dce_call,
rec->msgs[0],
"objectSID");
if (objectsid == NULL) {
+ struct GUID_txt_buf guid_buf;
char *guid_str =
- GUID_string(mem_ctx, &cache->entries[idx]);
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
DBG_WARNING("objectSID for GUID [%s] not found\n",
guid_str);
continue;
--
2.17.1
From 73680d412b260cec153a630d0762d15cfd70988c Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Thu, 18 Oct 2018 13:53:55 +1300
Subject: [PATCH 09/12] tests samr: Extra tests for samr_EnumDomainUserss
Add extra tests to test the content returned by samr_EnumDomainUsers,
and tests for the result caching added in the following commit.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
python/samba/tests/dcerpc/sam.py | 146 ++++++++++++++++++++++++++++++-
selftest/knownfail.d/samr | 2 +
2 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 selftest/knownfail.d/samr
diff --git a/python/samba/tests/dcerpc/sam.py b/python/samba/tests/dcerpc/sam.py
index 90455f04266..ab710861383 100644
--- a/python/samba/tests/dcerpc/sam.py
+++ b/python/samba/tests/dcerpc/sam.py
@@ -467,7 +467,7 @@ class SamrTests(RpcInterfaceTestCase):
self.delete_dns(dns)
- def test_EnumDomainUsers(self):
+ def test_EnumDomainGroups(self):
def check_results(expected, actual):
for (e, a) in zip(expected, actual):
self.assertTrue(isinstance(a, samr.SamEntry))
@@ -604,3 +604,147 @@ class SamrTests(RpcInterfaceTestCase):
check_results(expected, actual)
self.delete_dns(dns)
+
+ def test_EnumDomainUsers(self):
+ def check_results(expected, actual):
+ for (e, a) in zip(expected, actual):
+ self.assertTrue(isinstance(a, samr.SamEntry))
+ self.assertEquals(
+ str(e["sAMAccountName"]), str(a.name.string))
+
+ # Create four users
+ # to ensure that we have the minimum needed for the tests.
+ dns = self.create_users([1, 2, 3, 4])
+
+ #
+ # Get the expected results by querying the samdb database directly.
+ # We do this rather than use a list of expected results as this runs
+ # with other tests so we do not have a known fixed list of elements
+ select = "(objectClass=user)"
+ attributes = ["sAMAccountName", "objectSID", "userAccountConrol"]
+ unfiltered = self.samdb.search(expression=select, attrs=attributes)
+ filtered = self.filter_domain(unfiltered)
+ self.assertTrue(len(filtered) > 4)
+
+ # Sort the expected results by rid
+ expected = sorted(list(filtered), key=rid)
+
+ #
+ # Perform EnumDomainUsers with max_size greater than required for the
+ # expected number of results. We should get all the results.
+ #
+ max_size = calc_max_size(len(expected) + 10)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ self.assertEquals(len(expected), num_entries)
+ check_results(expected, actual.entries)
+
+ #
+ # Perform EnumDomainUsers with size set to so that it contains
+ # 4 entries.
+ max_size = calc_max_size(4)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ self.assertEquals(4, num_entries)
+ check_results(expected[:4], actual.entries)
+
+ #
+ # Try calling with resume_handle greater than number of entries
+ # Should return no results and a resume handle of 0
+ rh = len(expected)
+ max_size = calc_max_size(1)
+ self.conn.Close(self.handle)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, rh, 0, max_size)
+
+ self.assertEquals(0, num_entries)
+ self.assertEquals(0, resume_handle)
+
+ #
+ # Enumerate through the domain users one element at a time.
+ # We should get all the results.
+ #
+ actual = []
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, resume_handle, 0, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ self.assertEquals(len(expected), len(actual))
+ check_results(expected, actual)
+
+ #
+ # Check that the cached results are being returned.
+ # Obtain a new resume_handle and insert new entries into the
+ # into the DB. As the entries were added after the results were cached
+ # they should not show up in the returned results.
+ #
+ actual = []
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ extra_dns = self.create_users([1000, 1002, 1003, 1004])
+ while resume_handle:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, resume_handle, 0, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ self.assertEquals(len(expected), len(actual))
+ check_results(expected, actual)
+
+ #
+ # Perform EnumDomainUsers, we should read the newly added groups
+ # As resume_handle is zero, the results will be read from disk.
+ #
+ max_size = calc_max_size(len(expected) + len(extra_dns) + 10)
+ (resume_handle, actual, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ self.assertEquals(len(expected) + len(extra_dns), num_entries)
+
+ #
+ # Get a new expected result set by querying the database directly
+ unfiltered01 = self.samdb.search(expression=select, attrs=attributes)
+ filtered01 = self.filter_domain(unfiltered01)
+ self.assertTrue(len(filtered01) > len(expected))
+
+ # Sort the expected results by rid
+ expected01 = sorted(list(filtered01), key=rid)
+
+ #
+ # Now check that we read the new entries.
+ #
+ self.assertEquals(len(expected01), num_entries)
+ check_results(expected01, actual.entries)
+
+ #
+ # Check that deleted results are handled correctly.
+ # Obtain a new resume_handle and delete entries from the DB.
+ # We will not see the deleted entries in the result set, as details
+ # need to be read from disk. Only the object GUID's are cached.
+ #
+ actual = []
+ max_size = calc_max_size(1)
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, 0, 0, max_size)
+ self.delete_dns(extra_dns)
+ while resume_handle and num_entries:
+ self.assertEquals(1, num_entries)
+ actual.append(a.entries[0])
+ (resume_handle, a, num_entries) = self.conn.EnumDomainUsers(
+ self.domain_handle, resume_handle, 0, max_size)
+ if num_entries:
+ actual.append(a.entries[0])
+
+ self.assertEquals(len(expected), len(actual))
+ check_results(expected, actual)
+
+ self.delete_dns(dns)
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
new file mode 100644
index 00000000000..4e95d3c7922
--- /dev/null
+++ b/selftest/knownfail.d/samr
@@ -0,0 +1,2 @@
+^samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers
+^samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers
--
2.17.1
From 237a37df090c67486118684df29f2ad2162b5a59 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Thu, 18 Oct 2018 13:54:31 +1300
Subject: [PATCH 10/12] source4 samr: cache samr_EnumDomainUsers results
Add a cache of GUID's that matched the last samr_EnunDomainUsers made on a
domain handle. The cache is cleared if resume_handle is zero, and when the
final results are returned to the caller.
The existing code repeated the database query for each chunk requested.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
selftest/knownfail.d/samr | 2 -
source4/rpc_server/samr/dcesrv_samr.c | 203 ++++++++++++++++++++------
source4/rpc_server/samr/dcesrv_samr.h | 1 +
3 files changed, 156 insertions(+), 50 deletions(-)
delete mode 100644 selftest/knownfail.d/samr
diff --git a/selftest/knownfail.d/samr b/selftest/knownfail.d/samr
deleted file mode 100644
index 4e95d3c7922..00000000000
--- a/selftest/knownfail.d/samr
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.tests.dcerpc.sam.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers
-^samba.tests.dcerpc.sam.python3.samba.tests.dcerpc.sam.SamrTests.test_EnumDomainUsers
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index b485b5a2b62..3a5b0146ba7 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1516,12 +1516,18 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
struct dcesrv_handle *h;
struct samr_domain_state *d_state;
struct ldb_message **res;
- int i, ldb_cnt;
- uint32_t first, count;
+ uint32_t i;
+ uint32_t count;
+ uint32_t results;
+ uint32_t max_entries;
+ uint32_t remaining_entries;
+ uint32_t resume_handle;
struct samr_SamEntry *entries;
const char * const attrs[] = { "objectSid", "sAMAccountName",
"userAccountControl", NULL };
+ const char *const cache_attrs[] = {"objectSid", "objectGUID", NULL};
struct samr_SamArray *sam;
+ struct samr_guid_cache *cache = NULL;
*r->out.resume_handle = 0;
*r->out.sam = NULL;
@@ -1530,73 +1536,174 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
DCESRV_PULL_HANDLE(h, r->in.domain_handle, SAMR_HANDLE_DOMAIN);
d_state = h->data;
+ cache = &d_state->guid_caches[SAMR_ENUM_DOMAIN_USERS_CACHE];
- /* search for all domain users in this domain. This could possibly be
- cached and resumed on resume_key */
- ldb_cnt = samdb_search_domain(d_state->sam_ctx, mem_ctx,
- d_state->domain_dn,
- &res, attrs,
- d_state->domain_sid,
- "(objectClass=user)");
- if (ldb_cnt < 0) {
- return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ /*
+ * If the resume_handle is zero, query the database and cache the
+ * matching GUID's
+ */
+ if (*r->in.resume_handle == 0) {
+ NTSTATUS status;
+ int ldb_cnt;
+ clear_guid_cache(cache);
+ /*
+ * search for all domain users in this domain.
+ */
+ ldb_cnt = samdb_search_domain(d_state->sam_ctx,
+ mem_ctx,
+ d_state->domain_dn,
+ &res,
+ cache_attrs,
+ d_state->domain_sid,
+ "(objectClass=user)");
+ if (ldb_cnt < 0) {
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
+ }
+ /*
+ * Sort the results into RID order, while the spec states there
+ * is no order, Windows appears to sort the results by RID and
+ * so it is possible that there are clients that depend on
+ * this ordering
+ */
+ TYPESAFE_QSORT(res, ldb_cnt, compare_msgRid);
+
+ /*
+ * cache the sorted GUID's
+ */
+ status = load_guid_cache(cache, d_state, ldb_cnt, res);
+ TALLOC_FREE(res);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ cache->handle = 0;
}
- /* convert to SamEntry format */
- entries = talloc_array(mem_ctx, struct samr_SamEntry, ldb_cnt);
- if (!entries) {
- return NT_STATUS_NO_MEMORY;
+ /*
+ * If the resume handle is out of range we return an empty response
+ * and invalidate the cache.
+ *
+ * From the specification:
+ * Servers SHOULD validate that EnumerationContext is an expected
+ * value for the server's implementation. Windows does NOT validate
+ * the input, though the result of malformed information merely results
+ * in inconsistent output to the client.
+ */
+ if (*r->in.resume_handle >= cache->size) {
+ clear_guid_cache(cache);
+ sam = talloc(mem_ctx, struct samr_SamArray);
+ if (!sam) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ sam->entries = NULL;
+ sam->count = 0;
+
+ *r->out.sam = sam;
+ *r->out.resume_handle = 0;
+ return NT_STATUS_OK;
}
+ /*
+ * Calculate the number of entries to return limit by max_size.
+ * Note that we use the w2k3 element size value of 54
+ */
+ max_entries = 1 + (r->in.max_size / SAMR_ENUM_USERS_MULTIPLIER);
+ remaining_entries = cache->size - *r->in.resume_handle;
+ results = MIN(remaining_entries, max_entries);
+
+ /*
+ * Process the list of result GUID's.
+ * Read the details of each object and populate the Entries
+ * for the current level.
+ */
count = 0;
+ resume_handle = *r->in.resume_handle;
+ entries = talloc_array(mem_ctx, struct samr_SamEntry, results);
+ if (entries == NULL) {
+ clear_guid_cache(cache);
+ return NT_STATUS_NO_MEMORY;
+ }
+ for (i = 0; i < results; i++) {
+ struct dom_sid *sid;
+ struct ldb_result *rec;
+ const uint32_t idx = *r->in.resume_handle + i;
+ int ret;
+ const char *name = NULL;
- for (i=0;i<ldb_cnt;i++) {
- /* Check if a mask has been requested */
- if (r->in.acct_flags
- && ((samdb_result_acct_flags(res[i], NULL) & r->in.acct_flags) == 0)) {
+ resume_handle++;
+ /*
+ * Read an object from disk using the GUID as the key
+ *
+ * If the object can not be read, or it does not have a SID
+ * it is ignored.
+ *
+ * As a consequence of this, if all the remaining GUID's
+ * have been deleted an empty result will be returned.
+ * i.e. even if the previous call returned a non zero
+ * resume_handle it is possible for no results to be returned.
+ *
+ */
+ ret = dsdb_search_by_dn_guid(d_state->sam_ctx,
+ mem_ctx,
+ &rec,
+ &cache->entries[idx],
+ attrs,
+ 0);
+ if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+ char *guid_str =
+ GUID_string(mem_ctx, &cache->entries[idx]);
+ DBG_WARNING("GUID [%s] not found\n", guid_str);
continue;
+ } else if (ret != LDB_SUCCESS) {
+ clear_guid_cache(cache);
+ return NT_STATUS_INTERNAL_DB_CORRUPTION;
}
- entries[count].idx = samdb_result_rid_from_sid(mem_ctx, res[i],
- "objectSid", 0);
- entries[count].name.string = ldb_msg_find_attr_as_string(res[i],
- "sAMAccountName", "");
- count += 1;
+ sid = samdb_result_dom_sid(mem_ctx, rec->msgs[0], "objectSID");
+ if (sid == NULL) {
+ char *guid_str =
+ GUID_string(mem_ctx, &cache->entries[idx]);
+ DBG_WARNING("objectSID for GUID [%s] not found\n",
+ guid_str);
+ continue;
+ }
+ if (r->in.acct_flags &&
+ ((samdb_result_acct_flags(rec->msgs[0], NULL) &
+ r->in.acct_flags) == 0)) {
+ continue;
+ }
+ entries[count].idx = samdb_result_rid_from_sid(
+ mem_ctx, rec->msgs[0], "objectSid", 0);
+ name = ldb_msg_find_attr_as_string(
+ rec->msgs[0], "sAMAccountName", "");
+ entries[count].name.string = talloc_strdup(entries, name);
+ count++;
}
- /* sort the results by rid */
- TYPESAFE_QSORT(entries, count, compare_SamEntry);
-
- /* find the first entry to return */
- for (first=0;
- first<count && entries[first].idx <= *r->in.resume_handle;
- first++) ;
-
- /* return the rest, limit by max_size. Note that we
- use the w2k3 element size value of 54 */
- *r->out.num_entries = count - first;
- *r->out.num_entries = MIN(*r->out.num_entries,
- 1+(r->in.max_size/SAMR_ENUM_USERS_MULTIPLIER));
-
sam = talloc(mem_ctx, struct samr_SamArray);
if (!sam) {
+ clear_guid_cache(cache);
return NT_STATUS_NO_MEMORY;
}
- sam->entries = entries+first;
- sam->count = *r->out.num_entries;
+ sam->entries = entries;
+ sam->count = count;
*r->out.sam = sam;
+ *r->out.resume_handle = resume_handle;
+ *r->out.num_entries = count;
- if (first == count) {
+ /*
+ * Signal no more results by returning zero resume handle,
+ * the cache is also cleared at this point
+ */
+ if (*r->out.resume_handle >= cache->size) {
+ *r->out.resume_handle = 0;
+ clear_guid_cache(cache);
return NT_STATUS_OK;
}
-
- if (*r->out.num_entries < count - first) {
- *r->out.resume_handle = entries[first+*r->out.num_entries-1].idx;
- return STATUS_MORE_ENTRIES;
- }
-
- return NT_STATUS_OK;
+ /*
+ * There are more results to be returned.
+ */
+ return STATUS_MORE_ENTRIES;
}
diff --git a/source4/rpc_server/samr/dcesrv_samr.h b/source4/rpc_server/samr/dcesrv_samr.h
index 04866aca757..d530fc67bd9 100644
--- a/source4/rpc_server/samr/dcesrv_samr.h
+++ b/source4/rpc_server/samr/dcesrv_samr.h
@@ -54,6 +54,7 @@ struct samr_guid_cache {
enum samr_guid_cache_id {
SAMR_QUERY_DISPLAY_INFO_CACHE,
SAMR_ENUM_DOMAIN_GROUPS_CACHE,
+ SAMR_ENUM_DOMAIN_USERS_CACHE,
SAMR_LAST_CACHE
};
--
2.17.1
From 7f2d7cca51343f958a252d7295e3b2cdcc1a79db Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Nov 2018 12:48:15 +1300
Subject: [PATCH 11/12] s4-samr: Use dom_sid_split_rid() to get the RID in
dcesrv_samr_EnumDomainUsers
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/samr/dcesrv_samr.c | 32 ++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 3a5b0146ba7..53e2347fef6 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1623,10 +1623,12 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
return NT_STATUS_NO_MEMORY;
}
for (i = 0; i < results; i++) {
- struct dom_sid *sid;
+ struct dom_sid *objectsid;
+ uint32_t rid;
struct ldb_result *rec;
const uint32_t idx = *r->in.resume_handle + i;
int ret;
+ NTSTATUS status;
const char *name = NULL;
resume_handle++;
@@ -1657,8 +1659,10 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
clear_guid_cache(cache);
return NT_STATUS_INTERNAL_DB_CORRUPTION;
}
- sid = samdb_result_dom_sid(mem_ctx, rec->msgs[0], "objectSID");
- if (sid == NULL) {
+ objectsid = samdb_result_dom_sid(mem_ctx,
+ rec->msgs[0],
+ "objectSID");
+ if (objectsid == NULL) {
char *guid_str =
GUID_string(mem_ctx, &cache->entries[idx]);
DBG_WARNING("objectSID for GUID [%s] not found\n",
@@ -1670,8 +1674,26 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
r->in.acct_flags) == 0)) {
continue;
}
- entries[count].idx = samdb_result_rid_from_sid(
- mem_ctx, rec->msgs[0], "objectSid", 0);
+ status = dom_sid_split_rid(NULL,
+ objectsid,
+ NULL,
+ &rid);
+ if (!NT_STATUS_IS_OK(status)) {
+ struct dom_sid_buf sid_buf;
+ char *sid_str =
+ dom_sid_str_buf(objectsid,
+ &sid_buf);
+ struct GUID_txt_buf guid_buf;
+ char *guid_str =
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
+ DBG_WARNING("objectSID [%s] for GUID [%s] invalid\n",
+ sid_str,
+ guid_str);
+ continue;
+ }
+
+ entries[count].idx = rid;
name = ldb_msg_find_attr_as_string(
rec->msgs[0], "sAMAccountName", "");
entries[count].name.string = talloc_strdup(entries, name);
--
2.17.1
From 4c50f061beb92bf6eb7b041328dc8842ff543682 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 15 Nov 2018 13:44:29 +1300
Subject: [PATCH 12/12] s4-samr: Use GUID_buf_string() in
dcesrv_samr_EnumDomainUsers()
This avoids memory allocation.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
source4/rpc_server/samr/dcesrv_samr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index 53e2347fef6..8e464b1bf7e 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1651,8 +1651,10 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
attrs,
0);
if (ret == LDB_ERR_NO_SUCH_OBJECT) {
+ struct GUID_txt_buf guid_buf;
char *guid_str =
- GUID_string(mem_ctx, &cache->entries[idx]);
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
DBG_WARNING("GUID [%s] not found\n", guid_str);
continue;
} else if (ret != LDB_SUCCESS) {
@@ -1663,8 +1665,10 @@ static NTSTATUS dcesrv_samr_EnumDomainUsers(struct dcesrv_call_state *dce_call,
rec->msgs[0],
"objectSID");
if (objectsid == NULL) {
+ struct GUID_txt_buf guid_buf;
char *guid_str =
- GUID_string(mem_ctx, &cache->entries[idx]);
+ GUID_buf_string(&cache->entries[idx],
+ &guid_buf);
DBG_WARNING("objectSID for GUID [%s] not found\n",
guid_str);
continue;
--
2.17.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181119/e5ad7bea/signature.sig>
More information about the samba-technical
mailing list