From 2da2177c6fd7f40043e241ac0ba006000a818751 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 16 May 2019 16:14:13 +1200 Subject: [PATCH 1/2] tdb: Do not return errors from tdb_repack() in the tail of tdb_transaction_commit() This causes lock ordering issues in Samba, showing up as: ldb: ltdb: tdb(../private/sam.ldb.d/DC=SAMBA2008R2,DC=EXAMPLE,DC=COM.ldb): tdb_transaction_prepare_commit: failed to upgrade hash locks: Locking error This is because Samba has multiple tdb databases open, and the lock order between them is important. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13952 Signed-off-by: Andrew Bartlett --- lib/tdb/common/transaction.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c index 73d02b684a3..addcf21c93f 100644 --- a/lib/tdb/common/transaction.c +++ b/lib/tdb/common/transaction.c @@ -1209,7 +1209,27 @@ _PUBLIC_ int tdb_transaction_commit(struct tdb_context *tdb) _tdb_transaction_cancel(tdb); if (need_repack) { - return tdb_repack(tdb); + int ret = tdb_repack(tdb); + if (ret != 0) { + TDB_LOG((tdb, TDB_DEBUG_FATAL, + __location__ " Failed to repack database (not fatal)\n")); + } + /* + * Ignore the error. + * + * Why? + * + * We just committed to the DB above, so anything + * written during the transaction is committed, the + * caller needs to know that the long-term state was + * successfully modified. + * + * tdb_repack can fail for reasons like lock ordering, + * as we may not be able to recover the transaction + * lock at this point, having released it avove. + * + * On all failures the transaction is rolled back + */ } return 0; -- 2.11.0 From 43eea77ab93a3a0a5cec82c1693968613198d80b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 13 May 2019 15:32:23 +1200 Subject: [PATCH 2/2] dsdb: Add tests for large LDAP responses This behaviour is Samba-specific, we have not traditionally cut of responses at 1000 or so as Windows does, and we need to change that behaviour carefully. This triggers this bug in TDB: BUG: https://bugzilla.samba.org/show_bug.cgi?id=13952 Signed-off-by: Andrew Bartlett --- selftest/knownfail.d/large-dc | 6 + source4/dsdb/tests/python/large_ldap.py | 256 ++++++++++++++++++++++++++++++++ source4/selftest/tests.py | 16 ++ 3 files changed, 278 insertions(+) create mode 100644 selftest/knownfail.d/large-dc create mode 100644 source4/dsdb/tests/python/large_ldap.py diff --git a/selftest/knownfail.d/large-dc b/selftest/knownfail.d/large-dc new file mode 100644 index 00000000000..9a9e53a193f --- /dev/null +++ b/selftest/knownfail.d/large-dc @@ -0,0 +1,6 @@ +# Current Samba drops the LDAP socket after 256MB of replies, rather +# than giving an nice error message +^samba4.ldap.large_ldap.gssapi.python\(vampire_dc\).__main__.LargeLDAPTest.test_unindexed_iterator_search +^samba4.ldap.large_ldap.ntlmssp.python\(ad_dc_default\).__main__.LargeLDAPTest.test_unindexed_iterator_search +^samba4.ldap.large_ldap.ldaps.python\(ad_dc_ntvfs\).__main__.LargeLDAPTest.test_unindexed_iterator_search +^samba4.ldap.large_ldap.straight_ldap.python\(fl2008r2dc\).__main__.LargeLDAPTest.test_unindexed_iterator_search diff --git a/source4/dsdb/tests/python/large_ldap.py b/source4/dsdb/tests/python/large_ldap.py new file mode 100644 index 00000000000..2fc56e70455 --- /dev/null +++ b/source4/dsdb/tests/python/large_ldap.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +# +# Test large LDAP response behaviour in Samba +# Copyright (C) Andrew Bartlett 2019 +# +# Based on Unit tests for the notification control +# Copyright (C) Stefan Metzmacher 2016 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function +import optparse +import sys +import os +import random + +sys.path.insert(0, "bin/python") +import samba +from samba.tests.subunitrun import SubunitOptions, TestProgram + +import samba.getopt as options + +from samba.auth import system_session +from samba import ldb +from samba.samdb import SamDB +from samba.ndr import ndr_unpack +from samba import gensec +from samba.credentials import Credentials +import samba.tests + +from ldb import SCOPE_SUBTREE, SCOPE_ONELEVEL, SCOPE_BASE, LdbError +from ldb import ERR_TIME_LIMIT_EXCEEDED, ERR_ADMIN_LIMIT_EXCEEDED, ERR_UNWILLING_TO_PERFORM +from ldb import Message + +parser = optparse.OptionParser("large_ldap.py [options] ") +sambaopts = options.SambaOptions(parser) +parser.add_option_group(sambaopts) +parser.add_option_group(options.VersionOptions(parser)) +# use command line creds if available +credopts = options.CredentialsOptions(parser) +parser.add_option_group(credopts) +subunitopts = SubunitOptions(parser) +parser.add_option_group(subunitopts) +opts, args = parser.parse_args() + +if len(args) < 1: + parser.print_usage() + sys.exit(1) + +url = args[0] + +lp = sambaopts.get_loadparm() +creds = credopts.get_credentials(lp) + + +class ManyLDAPTest(samba.tests.TestCase): + + def setUp(self): + super(ManyLDAPTest, self).setUp() + self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp) + self.base_dn = self.ldb.domain_dn() + self.OU_NAME_MANY="many_ou" + self.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME_MANY + "," + str(self.base_dn)) + + samba.tests.delete_force(self.ldb, self.ou_dn, + controls=['tree_delete:1']) + + self.ldb.add({ + "dn": self.ou_dn, + "objectclass": "organizationalUnit", + "ou": self.OU_NAME_MANY}) + + for x in range(2000): + ou_name = self.OU_NAME_MANY + str(x) + self.ldb.add({ + "dn": "ou=" + ou_name + "," + str(self.ou_dn), + "objectclass": "organizationalUnit", + "ou": ou_name}) + + def tearDown(self): + samba.tests.delete_force(self.ldb, self.ou_dn, + controls=['tree_delete:1']) + + def test_unindexed_iterator_search(self): + """Testing a search for all the OUs. + + Needed to test that more that IOV_MAX responses can be returned + """ + if not url.startswith("ldap"): + self.fail(msg="This test is only valid on ldap") + + count = 0 + msg1 = None + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(ou=" + self.OU_NAME_MANY + "*)", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName"]) + + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + count += 1 + res1 = search1.result() + + # Check we got everything + self.assertEqual(count, 2001) + +class LargeLDAPTest(samba.tests.TestCase): + + def setUp(self): + super(LargeLDAPTest, self).setUp() + self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp) + self.base_dn = self.ldb.domain_dn() + self.USER_NAME = "large_user" + format(random.randint(0, 99999), "05") + "-" + self.OU_NAME="large_user_ou" + self.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME + "," + str(self.base_dn)) + + samba.tests.delete_force(self.ldb, self.ou_dn, + controls=['tree_delete:1']) + + self.ldb.add({ + "dn": self.ou_dn, + "objectclass": "organizationalUnit", + "ou": self.OU_NAME}) + + for x in range(200): + user_name = self.USER_NAME + format(x, "03") + self.ldb.add({ + "dn": "cn=" + user_name + "," + str(self.ou_dn), + "objectclass": "user", + "sAMAccountName": user_name, + "jpegPhoto": b'a' * (2 * 1024 * 1024)}) + + def tearDown(self): + # Remake the connection for tear-down (old Samba drops the socket) + self.ldb = SamDB(url, credentials=creds, session_info=system_session(lp), lp=lp) + samba.tests.delete_force(self.ldb, self.ou_dn, + controls=['tree_delete:1']) + + def test_unindexed_iterator_search(self): + """Testing an unindexed search that will break the result size limit""" + if not url.startswith("ldap"): + self.fail(msg="This test is only valid on ldap") + + count = 0 + msg1 = None + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(sAMAccountName=" + self.USER_NAME + "*)", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName"]) + + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + count += 1 + + res1 = search1.result() + + self.assertEqual(count, 200) + + # Now try breaking the 256MB limit + + count_jpeg = 0 + msg1 = None + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(sAMAccountName=" + self.USER_NAME + "*)", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName", "jpegPhoto"]) + try: + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + msg1 = reply + count_jpeg += 1 + except LdbError as e: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_SIZE_LIMIT_EXCEEDED) + + # Assert we don't get all the entries but still the error + self.assertGreater(count, count_jpeg) + + # Now try for just 100MB (server will do some chunking for this) + + count_jpeg2 = 0 + msg1 = None + try: + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(sAMAccountName=" + self.USER_NAME + "1*)", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName", "jpegPhoto"]) + except LdbError as e: + enum = e.args[0] + estr = e.args[1] + self.fail(estr) + + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + msg1 = reply + count_jpeg2 += 1 + + # Assert we got some entries + self.assertEqual(count_jpeg2, 100) + + def test_iterator_search(self): + """Testing an indexed search that will break the result size limit""" + if not url.startswith("ldap"): + self.fail(msg="This test is only valid on ldap") + + count = 0 + msg1 = None + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(&(objectClass=user)(sAMAccountName=" + self.USER_NAME + "*))", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName"]) + + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + count += 1 + res1 = search1.result() + + # Now try breaking the 256MB limit + + count_jpeg = 0 + msg1 = None + search1 = self.ldb.search_iterator(base=self.ou_dn, + expression="(&(objectClass=user)(sAMAccountName=" + self.USER_NAME + "*))", + scope=ldb.SCOPE_SUBTREE, + attrs=["objectGUID", "samAccountName", "jpegPhoto"]) + try: + for reply in search1: + self.assertIsInstance(reply, ldb.Message) + count_jpeg =+ 1 + except LdbError as e: + enum = err.args[0] + self.assertEqual(enum, ldb.ERR_SIZE_LIMIT_EXCEEDED) + + # Assert we don't get all the entries but still the error + self.assertGreater(count, count_jpeg) + + + +if "://" not in url: + if os.path.isfile(url): + url = "tdb://%s" % url + else: + url = "ldap://%s" % url + +TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 9001a98a5b3..aca41f261cd 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -882,6 +882,22 @@ plantestsuite("samba4.ldap.index.python", "none", [python, os.path.join(srcdir() plantestsuite_loadlist("samba4.ldap.notification.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(DSDB_PYTEST_DIR, "notification.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) plantestsuite_loadlist("samba4.ldap.sites.python(ad_dc_default)", "ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, "sites.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) +env = 'vampire_dc' +# Test with LMDB (GSSAPI/SASL bind) +plantestsuite_loadlist("samba4.ldap.large_ldap.gssapi.python(%s)" % env, env, [python, os.path.join(DSDB_PYTEST_DIR, "large_ldap.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--kerberos=yes', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + +env = 'ad_dc_default' +# Test with TDB (NTLMSSP bind) +plantestsuite_loadlist("samba4.ldap.large_ldap.ntlmssp.python(%s)" % env, env, [python, os.path.join(DSDB_PYTEST_DIR, "large_ldap.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--kerberos=no', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + +env = 'ad_dc_ntvfs' +# Test with ldaps:// +plantestsuite_loadlist("samba4.ldap.large_ldap.ldaps.python(%s)" % env, env, [python, os.path.join(DSDB_PYTEST_DIR, "large_ldap.py"), 'ldaps://$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + +env = 'fl2008r2dc' +# Test with straight ldap +plantestsuite_loadlist("samba4.ldap.large_ldap.straight_ldap.python(%s)" % env, env, [python, os.path.join(DSDB_PYTEST_DIR, "large_ldap.py"), 'ldap://$SERVER', '--simple-bind-dn=$USERNAME@$REALM', '--password=$PASSWORD', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) + planoldpythontestsuite("ad_dc_default", "sort", environ={'SERVER' : '$SERVER', 'DATA_DIR' : os.path.join(samba4srcdir, 'dsdb/tests/python/testdata/')}, name="samba4.ldap.sort.python", extra_path=[os.path.join(samba4srcdir, 'dsdb/tests/python')], extra_args=['-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) plantestsuite_loadlist("samba4.ldap.linked_attributes.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(DSDB_PYTEST_DIR, "linked_attributes.py"), '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) -- 2.11.0