The branch, master has been updated
       via  906aa7ddb8f CVE-2020-10700: dsdb: Do not permit the ASQ control for 
the GUID search in paged_results
       via  c309e6b2a70 CVE-2020-10700: ldb: Always use ldb_next_request() in 
ASQ module
       via  5603d26770a CVE-2020-10700: dsdb: Add test for ASQ and ASQ in 
combination with paged_results
      from  bac809348a7 CVE-2020-10704 libcli ldap: Check search request 
lengths.

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


- Log -----------------------------------------------------------------
commit 906aa7ddb8fadb88581adaa158ea39dd50bf7e4e
Author: Andrew Bartlett <[email protected]>
Date:   Wed Mar 11 16:43:31 2020 +1300

    CVE-2020-10700: dsdb: Do not permit the ASQ control for the GUID search in 
paged_results
    
    ASQ is a very strange control and a BASE search can return multiple results
    that are NOT the requested DN, but the DNs pointed to by it!
    
    Thanks to Andrei Popa <[email protected]> for finding,
    reporting and working with us to diagnose this issue!
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14331
    
    Signed-off-by: Andrew Bartlett <[email protected]>
    Reviewed-by: Gary Lockyer <[email protected]>
    
    Autobuild-User(master): Karolin Seeger <[email protected]>
    Autobuild-Date(master): Mon May  4 10:14:28 UTC 2020 on sn-devel-184

commit c309e6b2a704472ab2870e226bdaa172b4bf0fb8
Author: Andrew Bartlett <[email protected]>
Date:   Wed Mar 11 16:41:34 2020 +1300

    CVE-2020-10700: ldb: Always use ldb_next_request() in ASQ module
    
    We want to keep going down the module stack, and not start from the top 
again.
    
    ASQ is above the ACL modules, but below paged_results and we do not wish to
    re-trigger that work.
    
    Thanks to Andrei Popa <[email protected]> for finding,
    reporting and working with us to diagnose this issue!
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14331
    
    Signed-off-by: Andrew Bartlett <[email protected]>
    Reviewed-by: Gary Lockyer <[email protected]>

commit 5603d26770a41e81c035cf8c3c5931b143e1d330
Author: Andrew Bartlett <[email protected]>
Date:   Mon Mar 30 09:44:20 2020 +0000

    CVE-2020-10700: dsdb: Add test for ASQ and ASQ in combination with 
paged_results
    
    Thanks to Andrei Popa <[email protected]> for finding,
    reporting and working with us to diagnose this issue!
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14331
    
    Signed-off-by: Andrew Bartlett <[email protected]>
    Reviewed-by: Gary Lockyer <[email protected]>

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

Summary of changes:
 lib/ldb/modules/asq.c                          |  12 +-
 source4/dsdb/samdb/ldb_modules/paged_results.c |  18 ++-
 source4/dsdb/tests/python/asq.py               | 171 +++++++++++++++++++++++++
 source4/selftest/tests.py                      |   1 +
 4 files changed, 188 insertions(+), 14 deletions(-)
 create mode 100644 source4/dsdb/tests/python/asq.py


Changeset truncated at 500 lines:

diff --git a/lib/ldb/modules/asq.c b/lib/ldb/modules/asq.c
index 7482de826f0..4eba941ae0b 100644
--- a/lib/ldb/modules/asq.c
+++ b/lib/ldb/modules/asq.c
@@ -311,12 +311,9 @@ static int asq_build_multiple_requests(struct asq_context 
*ac, bool *terminated)
 
 static int asq_search_continue(struct asq_context *ac)
 {
-       struct ldb_context *ldb;
        bool terminated = false;
        int ret;
 
-       ldb = ldb_module_get_ctx(ac->module);
-
        switch (ac->step) {
        case ASQ_SEARCH_BASE:
 
@@ -328,7 +325,7 @@ static int asq_search_continue(struct asq_context *ac)
 
                ac->step = ASQ_SEARCH_MULTI;
 
-               return ldb_request(ldb, ac->reqs[ac->cur_req]);
+               return ldb_next_request(ac->module, ac->reqs[ac->cur_req]);
 
        case ASQ_SEARCH_MULTI:
 
@@ -339,7 +336,7 @@ static int asq_search_continue(struct asq_context *ac)
                        return asq_search_terminate(ac);
                }
 
-               return ldb_request(ldb, ac->reqs[ac->cur_req]);
+               return ldb_next_request(ac->module, ac->reqs[ac->cur_req]);
        }
 
        return LDB_ERR_OPERATIONS_ERROR;
@@ -347,14 +344,11 @@ static int asq_search_continue(struct asq_context *ac)
 
 static int asq_search(struct ldb_module *module, struct ldb_request *req)
 {
-       struct ldb_context *ldb;
        struct ldb_request *base_req;
        struct ldb_control *control;
        struct asq_context *ac;
        int ret;
 
-       ldb = ldb_module_get_ctx(module);
-
        /* check if there's an ASQ control */
        control = ldb_request_get_control(req, LDB_CONTROL_ASQ_OID);
        if (control == NULL) {
@@ -385,7 +379,7 @@ static int asq_search(struct ldb_module *module, struct 
ldb_request *req)
 
        ac->step = ASQ_SEARCH_BASE;
 
-       return ldb_request(ldb, base_req);
+       return ldb_next_request(ac->module, base_req);
 }
 
 static int asq_init(struct ldb_module *module)
diff --git a/source4/dsdb/samdb/ldb_modules/paged_results.c 
b/source4/dsdb/samdb/ldb_modules/paged_results.c
index 940d2254fb0..dc211dd18ce 100644
--- a/source4/dsdb/samdb/ldb_modules/paged_results.c
+++ b/source4/dsdb/samdb/ldb_modules/paged_results.c
@@ -483,8 +483,14 @@ paged_results_copy_down_controls(TALLOC_CTX *mem_ctx,
                if (control->oid == NULL) {
                        continue;
                }
-               if (strncmp(control->oid, LDB_CONTROL_PAGED_RESULTS_OID,
-                   sizeof(LDB_CONTROL_PAGED_RESULTS_OID)) == 0) {
+               if (strcmp(control->oid, LDB_CONTROL_PAGED_RESULTS_OID) == 0) {
+                       continue;
+               }
+               /*
+                * ASQ changes everything, do not copy it down for the
+                * per-GUID search
+                */
+               if (strcmp(control->oid, LDB_CONTROL_ASQ_OID) == 0) {
                        continue;
                }
                new_controls[j] = talloc_steal(new_controls, control);
@@ -534,21 +540,23 @@ static bool paged_controls_same(struct ldb_request *req,
 
        num_non_null_req_controls = 0;
        for (i=0; req->controls[i] != NULL; i++) {
-               if (req->controls[i]->oid != NULL) {
+               if (req->controls[i]->oid != NULL &&
+                   strcmp(req->controls[i]->oid,
+                          LDB_CONTROL_ASQ_OID) != 0) {
                        num_non_null_req_controls++;
                }
        }
 
        /* At this point we have the number of non-null entries for both
         * control lists and we know that:
-        * 1. down_controls does not contain the paged control
+        * 1. down_controls does not contain the paged control or ASQ
         *      (because paged_results_copy_down_controls excludes it)
         * 2. req->controls does contain the paged control
         *      (because this function is only called if this is true)
         * 3. down_controls is a subset of non-null controls in req->controls
         *      (checked above)
         * So to confirm that the two lists are identical except for the paged
-        * control, all we need to check is: */
+        * control and possibly ASQ, all we need to check is: */
        if (num_non_null_req_controls == num_down_controls + 1) {
                return true;
        }
diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py
new file mode 100644
index 00000000000..a32c9f40cd3
--- /dev/null
+++ b/source4/dsdb/tests/python/asq.py
@@ -0,0 +1,171 @@
+#!/usr/bin/env python3
+#
+# Test ASQ LDAP control behaviour in Samba
+# Copyright (C) Andrew Bartlett 2019-2020
+#
+# 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/>.
+
+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 ASQLDAPTest(samba.tests.TestCase):
+
+    def setUp(self):
+        super(ASQLDAPTest, self).setUp()
+        self.ldb = samba.Ldb(url, credentials=creds, 
session_info=system_session(lp), lp=lp)
+        self.base_dn = self.ldb.get_default_basedn()
+        self.NAME_ASQ="asq_" + format(random.randint(0, 99999), "05")
+        self.OU_NAME_ASQ= self.NAME_ASQ + "_ou"
+        self.ou_dn = ldb.Dn(self.ldb, "ou=" + self.OU_NAME_ASQ + "," + 
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_ASQ})
+
+        self.members = []
+        self.members2 = []
+
+        for x in range(20):
+            name = self.NAME_ASQ + "_" + str(x)
+            dn = ldb.Dn(self.ldb,
+                        "cn=" + name + "," + str(self.ou_dn))
+            self.members.append(dn)
+            self.ldb.add({
+                "dn": dn,
+                "objectclass": "group"})
+
+        for x in range(20):
+            name = self.NAME_ASQ + "_" + str(x + 20)
+            dn = ldb.Dn(self.ldb,
+                        "cn=" + name + "," + str(self.ou_dn))
+            self.members2.append(dn)
+            self.ldb.add({
+                "dn": dn,
+                "objectclass": "group",
+                "member": [str(x) for x in self.members]})
+
+        name = self.NAME_ASQ + "_" + str(x + 40)
+        self.top_dn = ldb.Dn(self.ldb,
+                             "cn=" + name + "," + str(self.ou_dn))
+        self.ldb.add({
+            "dn": self.top_dn,
+            "objectclass": "group",
+            "member": [str(x) for x in self.members2]})
+
+    def tearDown(self):
+        samba.tests.delete_force(self.ldb, self.ou_dn,
+                                 controls=['tree_delete:1'])
+
+    def test_asq(self):
+        """Testing ASQ behaviour.
+
+        ASQ is very strange, it turns a BASE search into a search for
+        all the objects pointed to by the specified attribute,
+        returning multiple entries!
+
+        """
+
+        msgs = self.ldb.search(base=self.top_dn,
+                               scope=ldb.SCOPE_BASE,
+                               attrs=["objectGUID", "cn", "member"],
+                               controls=["asq:1:member"])
+
+        self.assertEqual(len(msgs), 20)
+
+        for msg in msgs:
+            self.assertNotEqual(msg.dn, self.top_dn)
+            self.assertIn(msg.dn, self.members2)
+            for group in msg["member"]:
+                self.assertIn(ldb.Dn(self.ldb, str(group)),
+                              self.members)
+
+    def test_asq_paged(self):
+        """Testing ASQ behaviour with paged_results set.
+
+        ASQ is very strange, it turns a BASE search into a search for
+        all the objects pointed to by the specified attribute,
+        returning multiple entries!
+
+        """
+
+        msgs = self.ldb.search(base=self.top_dn,
+                               scope=ldb.SCOPE_BASE,
+                               attrs=["objectGUID", "cn", "member"],
+                               controls=["asq:1:member",
+                                         "paged_results:1:1024"])
+
+        self.assertEqual(len(msgs), 20)
+
+        for msg in msgs:
+            self.assertNotEqual(msg.dn, self.top_dn)
+            self.assertIn(msg.dn, self.members2)
+            for group in msg["member"]:
+                self.assertIn(ldb.Dn(self.ldb, str(group)),
+                              self.members)
+
+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 9e848cf873c..a3831b5d67c 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -941,6 +941,7 @@ 
plantestsuite_loadlist("samba4.tokengroups.krb5.python(ad_dc_default)", "ad_dc_d
 plantestsuite_loadlist("samba4.tokengroups.ntlm.python(ad_dc_default)", 
"ad_dc_default:local", [python, os.path.join(DSDB_PYTEST_DIR, 
"token_group.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', 
'-k', 'no', '$LOADLIST', '$LISTOPT'])
 plantestsuite("samba4.sam.python(fl2008r2dc)", "fl2008r2dc", [python, 
os.path.join(DSDB_PYTEST_DIR, "sam.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', 
'--workgroup=$DOMAIN'])
 plantestsuite("samba4.sam.python(ad_dc_default)", "ad_dc_default", [python, 
os.path.join(DSDB_PYTEST_DIR, "sam.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', 
'--workgroup=$DOMAIN'])
+plantestsuite("samba4.asq.python(ad_dc_default)", "ad_dc_default", [python, 
os.path.join(DSDB_PYTEST_DIR, "asq.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', 
'--workgroup=$DOMAIN'])
 plantestsuite("samba4.user_account_control.python(ad_dc_default)", 
"ad_dc_default", [python, os.path.join(DSDB_PYTEST_DIR, 
"user_account_control.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', 
'--workgroup=$DOMAIN'])
 
 for env in ['ad_dc_default:local', 'schema_dc:local']:


-- 
Samba Shared Repository

Reply via email to