The Samba-Bugzilla – Attachment 16008 Details for
Bug 14364
CVE-2020-10730 [SECURITY] NULL de-reference in AD DC LDAP server when ASQ and VLV combined
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for V4.5 (v2)
CVE-2020-10730-V4-5-v2.patch (text/plain), 26.11 KB, created by
Gary Lockyer
on 2020-05-26 20:54:33 UTC
(
hide
)
Description:
Patch for V4.5 (v2)
Filename:
MIME Type:
Creator:
Gary Lockyer
Created:
2020-05-26 20:54:33 UTC
Size:
26.11 KB
patch
obsolete
>From efcb4276b5f618d0b02aa43d2be8743398c97158 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 11 Mar 2020 16:41:34 +1300 >Subject: [PATCH 1/8] 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 <andrei.popa@next-gen.ro> 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 <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >(cherry picked from c309e6b2a704472ab2870e226bdaa172b4bf0fb8) >--- > lib/ldb/modules/asq.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > >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) >-- >2.17.1 > > >From 2ee81936582bda56d90430f6ee81290029664930 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Wed, 7 Jun 2017 17:44:25 +1200 >Subject: [PATCH 2/8] CVE-2020-10730: python/test: delete_force() passes on > command line args > >This allows you to use e.g.: > > delete_force(self.ldb, ou, controls=['tree_delete:1']) > >Only in tests of course. > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit 142d8617fe6b0dc4587384c373119d1d6289637c) >--- > python/samba/tests/__init__.py | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > >diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py >index 6960c0ca3ae..9801496e406 100644 >--- a/python/samba/tests/__init__.py >+++ b/python/samba/tests/__init__.py >@@ -901,8 +901,9 @@ def connect_samdb_env(env_url, env_username, env_password, lp=None): > return connect_samdb(samdb_url, credentials=creds, lp=lp) > > >-def delete_force(samdb, dn): >+def delete_force(samdb, dn, **kwargs): > try: >- samdb.delete(dn) >- except ldb.LdbError, (num, errstr): >+ samdb.delete(dn, **kwargs) >+ except ldb.LdbError as error: >+ (num, errstr) = error.args > assert num == ldb.ERR_NO_SUCH_OBJECT, "ldb.delete() failed: %s" % errstr >-- >2.17.1 > > >From 74e0533ed4870ee80cbc21fdd39bb62abee3fc69 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 5 May 2020 12:54:59 +1200 >Subject: [PATCH 3/8] CVE-2020-10730: vlv: Use strcmp(), not strncmp() checking > the NULL terminated control OIDs > >The end result is the same, as sizeof() includes the trailing NUL, but this >avoids having to think about that. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >index bd8df7def53..fe48e6729ba 100644 >--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >@@ -682,8 +682,8 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls) > if (control->oid == NULL) { > break; > } >- if (strncmp(control->oid, LDB_CONTROL_VLV_REQ_OID, sizeof(LDB_CONTROL_VLV_REQ_OID)) == 0 || >- strncmp(control->oid, LDB_CONTROL_SERVER_SORT_OID, sizeof(LDB_CONTROL_SERVER_SORT_OID)) == 0) { >+ if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 || >+ strcmp(control->oid, LDB_CONTROL_SERVER_SORT_OID) == 0) { > continue; > } > new_controls[j] = talloc_steal(new_controls, control); >-- >2.17.1 > > >From 5736e031841cee3fed0473d88f8bfb0a078ae068 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 5 May 2020 12:55:57 +1200 >Subject: [PATCH 4/8] CVE-2020-10730: vlv: Do not re-ASQ search the results of > an ASQ search with VLV > >This is a silly combination, but at least try and keep the results sensible >and avoid a double-dereference. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > source4/dsdb/samdb/ldb_modules/vlv_pagination.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > >diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >index fe48e6729ba..ae5842afc81 100644 >--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >@@ -682,10 +682,21 @@ vlv_copy_down_controls(TALLOC_CTX *mem_ctx, struct ldb_control **controls) > if (control->oid == NULL) { > break; > } >+ /* >+ * Do not re-use VLV, nor the server-sort, both are >+ * already handled here. >+ */ > if (strcmp(control->oid, LDB_CONTROL_VLV_REQ_OID) == 0 || > strcmp(control->oid, LDB_CONTROL_SERVER_SORT_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); > j++; > } >-- >2.17.1 > > >From 5b234f693a083369ff1106c9736e20160ed34bfd Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Tue, 5 May 2020 16:34:11 +1200 >Subject: [PATCH 5/8] CVE-2020-10730: vlv: Another workaround for mixing ASQ > and VLV > >This is essentially an alternative patch, but without the correct >behaviour. Instead this just avoids a segfault. > >Included in case we have something simialr again in >another module. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > .../dsdb/samdb/ldb_modules/vlv_pagination.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >index ae5842afc81..d6bad4bbd4a 100644 >--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >@@ -442,10 +442,21 @@ static int vlv_results(struct vlv_context *ac) > ret = vlv_search_by_dn_guid(ac->module, ac, &result, guid, > ac->req->op.search.attrs); > >- if (ret == LDAP_NO_SUCH_OBJECT) { >- /* The thing isn't there, which we quietly >- ignore and go on to send an extra one >- instead. */ >+ if (ret == LDAP_NO_SUCH_OBJECT >+ || result->count != 1) { >+ /* >+ * The thing isn't there, which we quietly >+ * ignore and go on to send an extra one >+ * instead. >+ * >+ * result->count == 0 or > 1 can only >+ * happen if ASQ (which breaks all the >+ * rules) is somehow invoked (as this >+ * is a BASE search). >+ * >+ * (We skip the ASQ cookie for the >+ * GUID searches) >+ */ > if (last_i < ac->store->num_entries - 1) { > last_i++; > } >-- >2.17.1 > > >From 5c6c6d4f62dbf85ce08bc8b9abe95171a86bcf57 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Mon, 18 May 2020 12:37:39 +1200 >Subject: [PATCH 6/8] CVE-2020-10730: s4 dsdb vlv_pagination: Prevent repeat > call of ldb_module_done > >Check the return code from vlv_results, if it is not LDB_SUCCESS >ldb_module_done has already been called, and SHOULD NOT be called again. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 > >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > .../dsdb/samdb/ldb_modules/vlv_pagination.c | 61 +++++++++++++++---- > 1 file changed, 49 insertions(+), 12 deletions(-) > >diff --git a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >index d6bad4bbd4a..99cff4ac642 100644 >--- a/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >+++ b/source4/dsdb/samdb/ldb_modules/vlv_pagination.c >@@ -387,7 +387,7 @@ static int vlv_calc_real_offset(int offset, int denominator, int n_entries) > has been prepared earlier and saved -- or by vlv_search_callback() when a > search has just been completed. */ > >-static int vlv_results(struct vlv_context *ac) >+static int vlv_results(struct vlv_context *ac, struct ldb_reply *ares) > { > struct ldb_vlv_resp_control *vlv; > unsigned int num_ctrls; >@@ -397,7 +397,9 @@ static int vlv_results(struct vlv_context *ac) > int target = 0; > > if (ac->store == NULL) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, ac->controls, ares->response, ret); > } > > if (ac->store->first_ref) { >@@ -406,6 +408,10 @@ static int vlv_results(struct vlv_context *ac) > */ > ret = send_referrals(ac->store, ac->req); > if (ret != LDB_SUCCESS) { >+ /* >+ * send_referrals will have called ldb_module_done >+ * if there was an error. >+ */ > return ret; > } > } >@@ -419,14 +425,23 @@ static int vlv_results(struct vlv_context *ac) > vlv_details, > sort_details, &ret); > if (ret != LDB_SUCCESS) { >- return ret; >+ return ldb_module_done( >+ ac->req, >+ ac->controls, >+ ares->response, >+ ret); > } > } else { > target = vlv_calc_real_offset(vlv_details->match.byOffset.offset, > vlv_details->match.byOffset.contentCount, > ac->store->num_entries); > if (target == -1) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, >+ ac->controls, >+ ares->response, >+ ret); > } > } > >@@ -462,12 +477,20 @@ static int vlv_results(struct vlv_context *ac) > } > continue; > } else if (ret != LDB_SUCCESS) { >- return ret; >+ return ldb_module_done( >+ ac->req, >+ ac->controls, >+ ares->response, >+ ret); > } > > ret = ldb_module_send_entry(ac->req, result->msgs[0], > NULL); > if (ret != LDB_SUCCESS) { >+ /* >+ * ldb_module_send_entry will have called >+ * ldb_module_done if there was an error >+ */ > return ret; > } > } >@@ -488,7 +511,9 @@ static int vlv_results(struct vlv_context *ac) > > ac->controls = talloc_array(ac, struct ldb_control *, num_ctrls + 1); > if (ac->controls == NULL) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, ac->controls, ares->response, ret); > } > ac->controls[num_ctrls] = NULL; > >@@ -498,20 +523,26 @@ static int vlv_results(struct vlv_context *ac) > > ac->controls[i] = talloc(ac->controls, struct ldb_control); > if (ac->controls[i] == NULL) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, ac->controls, ares->response, ret); > } > > ac->controls[i]->oid = talloc_strdup(ac->controls[i], > LDB_CONTROL_VLV_RESP_OID); > if (ac->controls[i]->oid == NULL) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, ac->controls, ares->response, ret); > } > > ac->controls[i]->critical = 0; > > vlv = talloc(ac->controls[i], struct ldb_vlv_resp_control); > if (vlv == NULL) { >- return LDB_ERR_OPERATIONS_ERROR; >+ ret = LDB_ERR_OPERATIONS_ERROR; >+ return ldb_module_done( >+ ac->req, ac->controls, ares->response, ret); > } > ac->controls[i]->data = vlv; > >@@ -600,7 +631,13 @@ static int vlv_search_callback(struct ldb_request *req, struct ldb_reply *ares) > store->result_array_size = store->num_entries; > > ac->store->controls = talloc_move(ac->store, &ares->controls); >- ret = vlv_results(ac); >+ ret = vlv_results(ac, ares); >+ if (ret != LDB_SUCCESS) { >+ /* vlv_results will have called ldb_module_done >+ * if there was an error. >+ */ >+ return ret; >+ } > return ldb_module_done(ac->req, ac->controls, > ares->response, ret); > } >@@ -845,9 +882,9 @@ static int vlv_search(struct ldb_module *module, struct ldb_request *req) > return ret; > } > >- ret = vlv_results(ac); >+ ret = vlv_results(ac, NULL); > if (ret != LDB_SUCCESS) { >- return ldb_module_done(req, NULL, NULL, ret); >+ return ret; > } > return ldb_module_done(req, ac->controls, NULL, > LDB_SUCCESS); >-- >2.17.1 > > >From 3ad0e1582bad7d2196343ff4600ce4b4788c0e17 Mon Sep 17 00:00:00 2001 >From: Gary Lockyer <gary@catalyst.net.nz> >Date: Mon, 25 May 2020 10:53:27 +1200 >Subject: [PATCH 7/8] CVE-2020-10730: selftest: Back port ASQ tests > >Backport the ASQ tests added for CVE-2020-10700. The paged_result tests >were removed as that functionality is not in Samba 4.5 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 >Signed-off-by: Gary Lockyer <gary@catalyst.net.nz> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/dsdb/tests/python/asq.py | 177 +++++++++++++++++++++++++++++++ > source4/selftest/tests.py | 9 ++ > 2 files changed, 186 insertions(+) > create mode 100644 source4/dsdb/tests/python/asq.py > >diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py >new file mode 100644 >index 00000000000..3049533f532 >--- /dev/null >+++ b/source4/dsdb/tests/python/asq.py >@@ -0,0 +1,177 @@ >+#!/usr/bin/env python >+# >+# 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("asq.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 + "_41" >+ 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_vlv(self): >+ """Testing ASQ behaviour with VLV 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! >+ >+ """ >+ >+ sort_control = "server_sort:1:0:cn" >+ >+ msgs = self.ldb.search(base=self.top_dn, >+ scope=ldb.SCOPE_BASE, >+ attrs=["objectGUID", "cn", "member"], >+ controls=["asq:1:member", >+ sort_control, >+ "vlv:1:20:20:11:0"]) >+ >+ 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 966fb0397fc..ee76c561b0e 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -581,6 +581,15 @@ planoldpythontestsuite("ad_dc", "samba.tests.dcerpc.raw_protocol", extra_args=[' > plantestsuite_loadlist("samba4.ldap.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/ldap.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) > plantestsuite_loadlist("samba4.tokengroups.python(ad_dc_ntvfs)", "ad_dc_ntvfs:local", [python, os.path.join(samba4srcdir, "dsdb/tests/python/token_group.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) > plantestsuite("samba4.sam.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/sam.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) >+plantestsuite( >+ "samba4.asq.python(ad_dc_ntvfs)", >+ "ad_dc_ntvfs", >+ [ >+ python, >+ os.path.join(samba4srcdir, "dsdb/tests/python/asq.py"), >+ '$SERVER', >+ '-U"$USERNAME%$PASSWORD"', >+ '--workgroup=$DOMAIN']) > plantestsuite("samba4.user_account_control.python(ad_dc_ntvfs)", "ad_dc_ntvfs", [python, os.path.join(samba4srcdir, "dsdb/tests/python/user_account_control.py"), '$SERVER', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN']) > planoldpythontestsuite("ad_dc_ntvfs", "dsdb_schema_info", > extra_path=[os.path.join(samba4srcdir, 'dsdb/tests/python')], >-- >2.17.1 > > >From 2f41fc0c7038f1e451316a56f6a89bb6aaf402c1 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 6 May 2020 16:19:01 +1200 >Subject: [PATCH 8/8] CVE-2020-10730: selftest: Add test to show that VLV and > paged_results are incompatible > >As tested against Windows Server 1709 > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364 > >Note for the back port to 4.5 these tests are tagged as known failures. >As source4/dsdb/samdb/ldb_modules/paged_results.c, which enforces the >constraint added after version 4.5. However the tests do prove that >the combination will not crash the LDAP server. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Gary Lockyer <gary@catalyst.net.nz> >--- > selftest/knownfail | 3 +++ > source4/dsdb/tests/python/asq.py | 26 ++++++++++++++++++++++++++ > source4/dsdb/tests/python/vlv.py | 23 +++++++++++++++++++++++ > 3 files changed, 52 insertions(+) > >diff --git a/selftest/knownfail b/selftest/knownfail >index 17667cd29d0..4cd6a068c5e 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -290,3 +290,6 @@ > ^samba4.drs.ridalloc_exop.python.*ridalloc_exop.DrsReplicaSyncTestCase.test_join_time_ridalloc > ^samba4.drs.ridalloc_exop.python.*ridalloc_exop.DrsReplicaSyncTestCase.test_rid_set_dbcheck_after_seize > ^samba4.drs.ridalloc_exop.python.*ridalloc_exop.DrsReplicaSyncTestCase.test_rid_set_dbcheck >+ >+^samba4.ldap.vlv.python.*test_vlv_paged >+^samba4.asq.python.*asq_vlv_paged >diff --git a/source4/dsdb/tests/python/asq.py b/source4/dsdb/tests/python/asq.py >index 3049533f532..56f749b3580 100644 >--- a/source4/dsdb/tests/python/asq.py >+++ b/source4/dsdb/tests/python/asq.py >@@ -167,6 +167,32 @@ class ASQLDAPTest(samba.tests.TestCase): > self.assertIn(ldb.Dn(self.ldb, str(group)), > self.members) > >+ def test_asq_vlv_paged(self): >+ """Testing ASQ behaviour with VLV and 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! >+ >+ Thankfully combining both of these gives >+ unavailable-critical-extension against Windows 1709 >+ >+ """ >+ >+ sort_control = "server_sort:1:0:cn" >+ >+ try: >+ msgs = self.ldb.search(base=self.top_dn, >+ scope=ldb.SCOPE_BASE, >+ attrs=["objectGUID", "cn", "member"], >+ controls=["asq:1:member", >+ sort_control, >+ "vlv:1:20:20:11:0", >+ "paged_results:1:1024"]) >+ self.fail("should have failed with LDAP_UNAVAILABLE_CRITICAL_EXTENSION") >+ except ldb.LdbError as e: >+ (enum, estr) = e.args >+ self.assertEqual(enum, ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION) > > if "://" not in url: > if os.path.isfile(url): >diff --git a/source4/dsdb/tests/python/vlv.py b/source4/dsdb/tests/python/vlv.py >index 069731920e0..7792845120a 100644 >--- a/source4/dsdb/tests/python/vlv.py >+++ b/source4/dsdb/tests/python/vlv.py >@@ -1075,6 +1075,29 @@ class VLVTests(samba.tests.TestCase): > controls=[sort_control, > "vlv:0:1:1:1:0:%s" % vlv_cookies[-1]]) > >+ def test_vlv_paged(self): >+ """Testing behaviour with VLV and paged_results set. >+ >+ A strange combination, certainly >+ >+ Thankfully combining both of these gives >+ unavailable-critical-extension against Windows 1709 >+ >+ """ >+ sort_control = "server_sort:1:0:cn" >+ >+ try: >+ msgs = self.ldb.search(base=self.base_dn, >+ scope=ldb.SCOPE_SUBTREE, >+ attrs=["objectGUID", "cn", "member"], >+ controls=["vlv:1:20:20:11:0", >+ sort_control, >+ "paged_results:1:1024"]) >+ self.fail("should have failed with LDAP_UNAVAILABLE_CRITICAL_EXTENSION") >+ except ldb.LdbError as e: >+ (enum, estr) = e.args >+ self.assertEqual(enum, ldb.ERR_UNSUPPORTED_CRITICAL_EXTENSION) >+ > > if "://" not in host: > if os.path.isfile(host): >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
abartlet
:
review+
gary
:
ci-passed+
Actions:
View
Attachments on
bug 14364
:
15951
|
15952
|
15983
|
15987
|
15998
|
16002
|
16003
|
16004
|
16005
|
16007
| 16008 |
16009
|
16056
|
16067
|
16089