Attachment 15160 Details for Bug 13952 – First iteration patch under test in the merge request

[patch] First iteration patch under test in the merge request

tdb-no-repack-fail.patch (text/plain), 15.31 KB, created by Andrew Bartlett on 2019-05-16 04:35:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2019-05-16 04:35:56 UTC

Size: 15.31 KB

patch

obsolete

>From 2da2177c6fd7f40043e241ac0ba006000a818751 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <abartlet@samba.org>
>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 <abartlet@samba.org>
>---
> 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 <http://www.gnu.org/licenses/>.
>+
>+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] <host>")
>+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
>

Flags:

abartlet: review? (jra)

Actions: View

Attachments on bug 13952: 15160 | 15162