[SCM] Samba Shared Repository - branch master updated

Douglas Bagnall dbagnall at samba.org
Fri Oct 12 05:24:02 UTC 2018


The branch, master has been updated
       via  3efb458 dsdb: Add dsdb_request_has_control() helper function
       via  b2c4b82 netcmd: Change Py3 incompatible long() for tombstone expunge
       via  24669e5 dsdb: Remove redundant variable/check
       via  4092b36 dsdb: Ensure that a DN (now) pointing at a deleted object counts for objectclass-based MUST
       via  dec3eda tests: Add corner-case test: fromServer points to dead server
       via  1139a4a s4/script/samba_upgradeprovision: set global dnNotToRecalculateFound var
       via  69fad8b s4/script/samba_upgradeprovision: remove unused variable
       via  1851c35 s4/script/samba_upgradeprovision: remove duplicate (contradictory) dict key
       via  cb5ad7f s4/script/samba_upgradeprovision: use int not long for Python 3
      from  ff3e2fa vfs_full_audit: ntimes: log a-, m-, c- and creation-time

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 3efb4588abaf55c0b58b035790d9da822f62b09d
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Thu Oct 11 17:50:52 2018 +1300

    dsdb: Add dsdb_request_has_control() helper function
    
    Most of the DSDB modules only want to check the existence of a control,
    rather than access the control itself. Adding a helper function allows
    the code to ask more natural-sounding yes/no questions, and tidies up
    an ugly-looking long-line in extended_dn_out.c.
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(master): Douglas Bagnall <dbagnall at samba.org>
    Autobuild-Date(master): Fri Oct 12 07:23:26 CEST 2018 on sn-devel-144

commit b2c4b829a75001ec939f9ecbc0db0712758ece80
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Thu Oct 4 14:37:44 2018 +1300

    netcmd: Change Py3 incompatible long() for tombstone expunge
    
    The code to expunge tombstones uses long(), which is not Python3
    compatible. Python3 uses int() instead, and works out how big it needs
    to be.
    
    As long as we don't run the samba-tool command on a 32-bit machine
    after the year 2038, then we should avoid any integer overflow on
    Python 2.x.
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 24669e57fcb5eb592e86746c9451977752d37f23
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Fri Sep 28 14:55:14 2018 +1200

    dsdb: Remove redundant variable/check
    
    Previously, this code used to live inside the loop, so the
    checked_reveal_control was needed to save ourselves unnecessary work.
    
    However, now that the code has been moved outside the loop, the
    checked_reveal_control variable is just unnecessary complication.
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 4092b369aeeb7058d78b8d6f41dbbc6d69203ecc
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed Sep 12 14:48:04 2018 -0500

    dsdb: Ensure that a DN (now) pointing at a deleted object counts for objectclass-based MUST
    
    Add the 'reveal_internals' controls when performing objectclass-based
    checks of mandatory attributes. This prevents the extended_dn DSDB
    module from suppressing attributes that point to deleted (i.e.
    non-existent/expunged) objects.
    
    This ensures that, when modifying an object (and often not even
    touching the mandatory attribute) that the fact that an attribute is a
    DN, and the DN target is deleted, that the schema check will still pass.
    
    Otherwise a fromServer pointing at a dead server can cause failures,
    i.e. you can't modify the affected object at all, because the DSDB
    thinks a mandatory attribute is missing.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit dec3eda1f74f5bf7ea91c1be3d5dfd832e9672b9
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Fri Sep 28 12:35:35 2018 +1200

    tests: Add corner-case test: fromServer points to dead server
    
    The fromServer attribute is slightly unique, in that it's a DN (similar
    to a one-way link), but it is also a mandatory attribute.
    
    Currently, if fromServer gets a bad value (i.e. a dead server that has
    been expunged), the DSDB rejects any attempts to modify the associated
    nTDSConnection object (regardless of whether or not you're actually
    changing the fromServer attribute).
    
    This patch adds a test-case that demonstrates how the DB can get into
    such a state.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13621
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 1139a4a6a0fedfbbc7a7d822e845927c6af52905
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Oct 10 17:51:54 2018 +1300

    s4/script/samba_upgradeprovision: set global dnNotToRecalculateFound var
    
    as probably intended. Without this the local variable shadows the
    global one and is never used while the global one is never changed.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Noel Power <noel.power at suse.com>

commit 69fad8bff0be5efc5a3cc4339bff66af8734f38f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Oct 10 17:50:24 2018 +1300

    s4/script/samba_upgradeprovision: remove unused variable
    
    A similarly named variable is always set two lines down, so we don't need this
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Noel Power <noel.power at suse.com>

commit 1851c35e99b84c057859d736dd3d1cda6a428281
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Oct 10 17:40:25 2018 +1300

    s4/script/samba_upgradeprovision: remove duplicate (contradictory) dict key
    
    The second, winning, entry says '"defaultSecurityDescriptor": replace + add'
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Noel Power <noel.power at suse.com>

commit cb5ad7fefedb85b21e19475851819b0021e06276
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Oct 10 17:36:50 2018 +1300

    s4/script/samba_upgradeprovision: use int not long for Python 3
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Noel Power <noel.power at suse.com>

-----------------------------------------------------------------------

Summary of changes:
 python/samba/netcmd/domain.py                      |   4 +-
 source4/dsdb/common/util.c                         |   8 ++
 source4/dsdb/samdb/ldb_modules/extended_dn_out.c   |   9 +-
 source4/dsdb/samdb/ldb_modules/objectclass_attrs.c |  11 ++
 source4/dsdb/tests/python/attr_from_server.py      | 150 +++++++++++++++++++++
 source4/scripting/bin/samba_upgradeprovision       |   7 +-
 source4/selftest/tests.py                          |   5 +
 7 files changed, 182 insertions(+), 12 deletions(-)
 create mode 100644 source4/dsdb/tests/python/attr_from_server.py


Changeset truncated at 500 lines:

diff --git a/python/samba/netcmd/domain.py b/python/samba/netcmd/domain.py
index 7d0af53..f022e37 100644
--- a/python/samba/netcmd/domain.py
+++ b/python/samba/netcmd/domain.py
@@ -3891,10 +3891,10 @@ This command expunges tombstones from the database."""
 
         if current_time_string is not None:
             current_time_obj = time.strptime(current_time_string, "%Y-%m-%d")
-            current_time = long(time.mktime(current_time_obj))
+            current_time = int(time.mktime(current_time_obj))
 
         else:
-            current_time = long(time.time())
+            current_time = int(time.time())
 
         if len(ncs) == 0:
             res = samdb.search(expression="", base="", scope=ldb.SCOPE_BASE,
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 193fa2a..e7b860d 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -4439,6 +4439,14 @@ int dsdb_request_add_controls(struct ldb_request *req, uint32_t dsdb_flags)
 }
 
 /*
+   returns true if a control with the specified "oid" exists
+*/
+bool dsdb_request_has_control(struct ldb_request *req, const char *oid)
+{
+	return (ldb_request_get_control(req, oid) != NULL);
+}
+
+/*
   an add with a set of controls
 */
 int dsdb_add(struct ldb_context *ldb, const struct ldb_message *message,
diff --git a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
index 6a869d0..014ae0a 100644
--- a/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
+++ b/source4/dsdb/samdb/ldb_modules/extended_dn_out.c
@@ -402,7 +402,7 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
 	struct ldb_message *msg;
 	struct extended_dn_out_private *p;
 	struct ldb_context *ldb;
-	bool have_reveal_control=false, checked_reveal_control=false;
+	bool have_reveal_control=false;
 
 	ac = talloc_get_type(req->context, struct extended_search_context);
 	p = talloc_get_type(ldb_module_get_private(ac->module), struct extended_dn_out_private);
@@ -473,11 +473,8 @@ static int extended_callback(struct ldb_request *req, struct ldb_reply *ares,
 		}
 	}
 
-	if (!checked_reveal_control) {
-		have_reveal_control =
-			ldb_request_get_control(req, LDB_CONTROL_REVEAL_INTERNALS) != NULL;
-		checked_reveal_control = true;
-	}
+	have_reveal_control =
+		dsdb_request_has_control(req, LDB_CONTROL_REVEAL_INTERNALS);
 
 	/* 
 	 * Shortcut for repl_meta_data.  We asked for the data
diff --git a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
index cfacaf5..67c93ca 100644
--- a/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
+++ b/source4/dsdb/samdb/ldb_modules/objectclass_attrs.c
@@ -617,6 +617,17 @@ static int oc_op_callback(struct ldb_request *req, struct ldb_reply *ares)
 		return ldb_module_done(ac->req, NULL, NULL, ret);
 	}
 
+	/*
+	 * This ensures we see if there was a DN, that pointed at an
+	 * object that is now deleted, that we still consider the
+	 * schema check to have passed
+	 */
+	ret = ldb_request_add_control(search_req, LDB_CONTROL_REVEAL_INTERNALS,
+				      false, NULL);
+	if (ret != LDB_SUCCESS) {
+		return ldb_module_done(ac->req, NULL, NULL, ret);
+	}
+
 	ret = ldb_next_request(ac->module, search_req);
 	if (ret != LDB_SUCCESS) {
 		return ldb_module_done(ac->req, NULL, NULL, ret);
diff --git a/source4/dsdb/tests/python/attr_from_server.py b/source4/dsdb/tests/python/attr_from_server.py
new file mode 100644
index 0000000..1a0112d
--- /dev/null
+++ b/source4/dsdb/tests/python/attr_from_server.py
@@ -0,0 +1,150 @@
+# -*- coding: utf-8 -*-
+#
+# Tests a corner-case involving the fromServer attribute, which is slightly
+# unique: it's an Object(DS-DN) (like a one-way link), but it is also a
+# mandatory attribute (for nTDSConnection). The corner-case is that the
+# fromServer can potentially end up pointing to a non-existent object.
+# This can happen with other one-way links, but these other one-way links
+# are not mandatory attributes.
+#
+# Copyright (C) Andrew Bartlett 2018
+#
+# 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
+sys.path.insert(0, "bin/python")
+import samba
+import os
+import time
+import ldb
+import samba.tests
+from samba.tests.subunitrun import TestProgram, SubunitOptions
+from samba.dcerpc import misc
+from samba.provision import DEFAULTSITE
+
+# note we must connect to the local ldb file on disk, in order to
+# add system-only nTDSDSA objects
+parser = optparse.OptionParser("attr_from_server.py <LDB-filepath>")
+subunitopts = SubunitOptions(parser)
+parser.add_option_group(subunitopts)
+opts, args = parser.parse_args()
+
+if len(args) < 1:
+    parser.print_usage()
+    sys.exit(1)
+
+ldb_path = args[0]
+
+
+class FromServerAttrTest(samba.tests.TestCase):
+    def setUp(self):
+        super(FromServerAttrTest, self).setUp()
+        self.ldb = samba.tests.connect_samdb(ldb_path)
+
+    def tearDown(self):
+        super(FromServerAttrTest, self).tearDown()
+
+    def set_attribute(self, dn, attr, value, operation=ldb.FLAG_MOD_ADD):
+        """Modifies an attribute for an object"""
+        m = ldb.Message()
+        m.dn = ldb.Dn(self.ldb, dn)
+        m[attr] = ldb.MessageElement(value, operation, attr)
+        self.ldb.modify(m)
+
+    def get_object_guid(self, dn):
+        res = self.ldb.search(base=dn, attrs=["objectGUID"],
+                              scope=ldb.SCOPE_BASE)
+        self.assertTrue(len(res) == 1)
+        return str(misc.GUID(res[0]['objectGUID'][0]))
+
+    def test_dangling_server_attr(self):
+        """
+        Tests a scenario where an object has a fromServer attribute that points
+        to an object that no longer exists.
+        """
+
+        # add a temporary server and its associated NTDS Settings object
+        config_dn = self.ldb.get_config_basedn()
+        sites_dn = "CN=Sites,{0}".format(config_dn)
+        servers_dn = "CN=Servers,CN={0},{1}".format(DEFAULTSITE, sites_dn)
+        tmp_server = "CN=TMPSERVER,{0}".format(servers_dn)
+        self.ldb.add({"dn": tmp_server, "objectclass": "server"})
+        server_guid = self.get_object_guid(tmp_server)
+        tmp_ntds_settings = "CN=NTDS Settings,{0}".format(tmp_server)
+        self.ldb.add({"dn": tmp_ntds_settings, "objectClass": "nTDSDSA"},
+                     ["relax:0"])
+
+        # add an NTDS connection under the testenv DC that points to the tmp DC
+        testenv_dc = "CN={0},{1}".format(os.environ["SERVER"], servers_dn)
+        ntds_conn = "CN=Test-NTDS-Conn,CN=NTDS Settings,{0}".format(testenv_dc)
+        ldif = """
+dn: {dn}
+objectClass: nTDSConnection
+fromServer: CN=NTDS Settings,{fromServer}
+options: 1
+enabledConnection: TRUE
+""".format(dn=ntds_conn, fromServer=tmp_server)
+        self.ldb.add_ldif(ldif)
+        self.addCleanup(self.ldb.delete, ntds_conn)
+
+        # sanity-check we can modify the NTDS Connection object
+        self.set_attribute(ntds_conn, 'description', 'Test-value')
+
+        # sanity-check we can't modify the fromServer to point to a bad DN
+        try:
+            bad_dn = "CN=NTDS Settings,CN=BAD-DC,{0}".format(servers_dn)
+            self.set_attribute(ntds_conn, 'fromServer', bad_dn,
+                               operation=ldb.FLAG_MOD_REPLACE)
+            self.fail("Successfully set fromServer to bad DN")
+        except ldb.LdbError as err:
+            enum = err.args[0]
+            self.assertEqual(enum, ldb.ERR_CONSTRAINT_VIOLATION)
+
+        # delete the tmp server, i.e. pretend we demoted it
+        self.ldb.delete(tmp_server, ["tree_delete:1"])
+
+        # check we can still see the deleted server object
+        search_expr = '(objectGUID={0})'.format(server_guid)
+        res = self.ldb.search(config_dn, scope=ldb.SCOPE_SUBTREE,
+                              expression=search_expr,
+                              controls=["show_deleted:1"])
+        self.assertTrue(len(res) == 1, "Could not find deleted server entry")
+
+        # now pretend some time has passed and the deleted server object
+        # has been tombstone-expunged from the DB
+        time.sleep(1)
+        current_time = int(time.time())
+        self.ldb.garbage_collect_tombstones([str(config_dn)], current_time,
+                                            tombstone_lifetime=0)
+
+        # repeat the search to sanity-check the deleted object is really gone
+        res = self.ldb.search(config_dn, scope=ldb.SCOPE_SUBTREE,
+                              expression=search_expr,
+                              controls=["show_deleted:1"])
+        self.assertTrue(len(res) == 0, "Did not expunge deleted server")
+
+        # the nTDSConnection now has a (mandatory) fromServer attribute that
+        # points to an object that no longer exists. Now try to modify an
+        # unrelated attribute on the nTDSConnection
+        try:
+            self.set_attribute(ntds_conn, 'description', 'Test-value-2',
+                               operation=ldb.FLAG_MOD_REPLACE)
+        except ldb.LdbError as err:
+            print(err)
+            self.fail("Could not modify NTDS connection")
+
+
+TestProgram(module=__name__, opts=subunitopts)
diff --git a/source4/scripting/bin/samba_upgradeprovision b/source4/scripting/bin/samba_upgradeprovision
index 5d040d2..df7a8e0 100755
--- a/source4/scripting/bin/samba_upgradeprovision
+++ b/source4/scripting/bin/samba_upgradeprovision
@@ -124,7 +124,6 @@ hashOverwrittenAtt = {  "prefixMap": replace, "systemMayContain": replace,
                         "description":replace, "operatingSystemVersion":replace,
                         "adminPropertyPages":replace, "groupType":replace,
                         "wellKnownObjects":replace, "privilege":never,
-                        "defaultSecurityDescriptor": replace,
                         "rIDAvailablePool": never,
                         "versionNumber" : add,
                         "rIDNextRID": add, "rIDUsedPool": never,
@@ -377,7 +376,7 @@ def handle_special_case(att, delta, new, old, useReplMetadata, basedn, aldb):
                             (int(str(old[0][att])), int(str(new[0][att]))))
             return False
         if (att == "minPwdAge" and flag == FLAG_MOD_REPLACE):
-            if (long(str(old[0][att])) == 0):
+            if (int(str(old[0][att])) == 0):
                 delta[att] = MessageElement(new[0][att], FLAG_MOD_REPLACE, att)
             return True
 
@@ -638,7 +637,6 @@ def add_missing_object(ref_samdb, samdb, dn, names, basedn, hash, index):
             delta.remove(att)
         for att in backlinked:
             delta.remove(att)
-        depend_on_yettobecreated = None
         for att in dn_syntax_att:
             depend_on_yet_tobecreated = check_dn_nottobecreated(hash, index,
                                                                 delta.get(str(att)))
@@ -906,6 +904,7 @@ def checkKeepAttributeWithMetadata(delta, att, message, reference, current,
                     message(CHANGESD, "But the SD has been changed by someonelse "
                                     "so it's impossible to know if the difference"
                                     " cames from the modification or from a previous bug")
+                    global dnNotToRecalculateFound
                     dnNotToRecalculateFound = True
                 else:
                     dnToRecalculate.append(dn)
@@ -1808,7 +1807,7 @@ if __name__ == '__main__':
         # as we are assured that on this DNs we will have differences !
         # Also the check must be done in a clever way as for the moment we just
         # compare SDDL
-        if dnNotToRecalculateFound is False and (opts.debugchangesd or opts.debugall):
+        if dnNotToRecalculateFound == False and (opts.debugchangesd or opts.debugall):
             message(CHANGESD, "Checking recalculated SDs")
             check_updated_sd(new_ldbs.sam, ldbs.sam, names)
 
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 101418a..3242795 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -147,6 +147,11 @@ if os.path.exists(os.path.join(samba4bindir, "ldbtest")):
 else:
     skiptestsuite("ldb.base", "Using system LDB, ldbtest not available")
 
+plantestsuite_loadlist("samba4.tests.attr_from_server.python(ad_dc_ntvfs)",
+                       "ad_dc_ntvfs:local",
+                       [python, os.path.join(samba4srcdir, "dsdb/tests/python/attr_from_server.py"),
+                        '$PREFIX_ABS/ad_dc_ntvfs/private/sam.ldb', '$LOADLIST', '$LISTOPT'])
+
 # Tests for RPC
 
 # add tests to this list as they start passing, so we test


-- 
Samba Shared Repository



More information about the samba-cvs mailing list