The branch, master has been updated
via 75cad1d79f6 tdb: Release tdb 1.4.1
via eafef2bbfd4 dsdb: Add tests for large LDAP responses
via 444b594fd12 tdb: Do not return errors from tdb_repack() in the tail
of tdb_transaction_commit()
from ab648a4c635 smbd: Do oplock break messages in ndr
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 75cad1d79f68a8a81e24c74b42fadaaa13812e7a
Author: Andrew Bartlett <[email protected]>
Date: Fri May 17 14:16:43 2019 +1200
tdb: Release tdb 1.4.1
* Do not propogate any errors from tdb_repack() to to
tdb_transaction_commit()
The repack may fail due to lock ordering or memory limits, but the
original data has
been written so the call must succeed. (bug 13952)
Signed-off-by: Andrew Bartlett <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
Autobuild-User(master): Jeremy Allison <[email protected]>
Autobuild-Date(master): Fri May 17 08:21:52 UTC 2019 on sn-devel-184
commit eafef2bbfd4be71962520b7bb14726741af66a2c
Author: Andrew Bartlett <[email protected]>
Date: Mon May 13 15:32:23 2019 +1200
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 <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
commit 444b594fd12dbf910bea55cdc2055152cfee2449
Author: Andrew Bartlett <[email protected]>
Date: Thu May 16 16:14:13 2019 +1200
tdb: Do not return errors from tdb_repack() in the tail of
tdb_transaction_commit()
The call to tdb_repack() inside tdb_transaction_commit()
is an optimization, not part of the transaction itself,
so failing due to lock or other errors isn't a fatal error
that should cause the caller to think the transaction was
a failure by returning -1.
The tdb transaction itself has finished and been committed
onto stable storage via fsync and all locks released at the
point tdb_repack() is called.
tdb_repack() is only called here as it's a convenient point
to attempt to reduce tdb fragmentation without having to add
a timer call to repack in all users of tdb.
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 <[email protected]>
Reviewed-by: Jeremy Allison <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
lib/tdb/ABI/{tdb-1.3.17.sigs => tdb-1.4.1.sigs} | 0
lib/tdb/common/transaction.c | 24 ++-
lib/tdb/wscript | 2 +-
selftest/knownfail.d/large-dc | 6 +
source4/dsdb/tests/python/large_ldap.py | 256 ++++++++++++++++++++++++
source4/selftest/tests.py | 16 ++
6 files changed, 302 insertions(+), 2 deletions(-)
copy lib/tdb/ABI/{tdb-1.3.17.sigs => tdb-1.4.1.sigs} (100%)
create mode 100644 selftest/knownfail.d/large-dc
create mode 100644 source4/dsdb/tests/python/large_ldap.py
Changeset truncated at 500 lines:
diff --git a/lib/tdb/ABI/tdb-1.3.17.sigs b/lib/tdb/ABI/tdb-1.4.1.sigs
similarity index 100%
copy from lib/tdb/ABI/tdb-1.3.17.sigs
copy to lib/tdb/ABI/tdb-1.4.1.sigs
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index 73d02b684a3..b67f84f215d 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -1209,7 +1209,29 @@ _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 is an optimization that can fail for
+ * reasons like lock ordering and we cannot recover
+ * the transaction lock at this point, having released
+ * it above.
+ *
+ * If we return a failure the caller thinks the
+ * transaction was rolled back.
+ */
}
return 0;
diff --git a/lib/tdb/wscript b/lib/tdb/wscript
index 919004de737..ece44f82e33 100644
--- a/lib/tdb/wscript
+++ b/lib/tdb/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'tdb'
-VERSION = '1.4.0'
+VERSION = '1.4.1'
import sys, os
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'])
--
Samba Shared Repository
