The Samba-Bugzilla – Attachment 15892 Details for
Bug 14331
CVE-2020-10700 [SECURITY] Use-after-free in AD DC LDAP server when ASQ and paged_results combined
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch for 4.12 (v01)
ASQ-paged_results-v4-12-stable.patch (text/plain), 13.63 KB, created by
Andrew Bartlett
on 2020-04-03 02:50:56 UTC
(
hide
)
Description:
patch for 4.12 (v01)
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2020-04-03 02:50:56 UTC
Size:
13.63 KB
patch
obsolete
>From 6dc7ebfa9f48deba9147e0dd1d091244d609e43a Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 30 Mar 2020 09:44:20 +0000 >Subject: [PATCH 1/3] dsdb: Add test for ASQ and ASQ in combination with > paged_results > >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> >--- > selftest/knownfail.d/asq | 1 + > source4/dsdb/tests/python/asq.py | 171 +++++++++++++++++++++++++++++++ > source4/selftest/tests.py | 1 + > 3 files changed, 173 insertions(+) > create mode 100644 selftest/knownfail.d/asq > create mode 100644 source4/dsdb/tests/python/asq.py > >diff --git a/selftest/knownfail.d/asq b/selftest/knownfail.d/asq >new file mode 100644 >index 00000000000..eb0e3e0aba1 >--- /dev/null >+++ b/selftest/knownfail.d/asq >@@ -0,0 +1 @@ >+samba4.asq.python\(ad_dc_default\).__main__.ASQLDAPTest.test_asq_paged >\ No newline at end of file >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 f570d35dfba..e62a7aa1b3f 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -898,6 +898,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']: >-- >2.17.1 > > >From 6056b1b9a585519e089cd3810374f629d2b621c1 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 11 Mar 2020 16:41:34 +1300 >Subject: [PATCH 2/3] 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> >--- > 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 2573846dd784536c17e983d79e48c5707c8df8a6 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Wed, 11 Mar 2020 16:43:31 +1300 >Subject: [PATCH 3/3] 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 <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> >--- > selftest/knownfail.d/asq | 1 - > source4/dsdb/samdb/ldb_modules/paged_results.c | 18 +++++++++++++----- > 2 files changed, 13 insertions(+), 6 deletions(-) > delete mode 100644 selftest/knownfail.d/asq > >diff --git a/selftest/knownfail.d/asq b/selftest/knownfail.d/asq >deleted file mode 100644 >index eb0e3e0aba1..00000000000 >--- a/selftest/knownfail.d/asq >+++ /dev/null >@@ -1 +0,0 @@ >-samba4.asq.python\(ad_dc_default\).__main__.ASQLDAPTest.test_asq_paged >\ No newline at end of file >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; > } >-- >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
:
ci-passed+
Actions:
View
Attachments on
bug 14331
:
15882
|
15884
|
15885
|
15890
|
15891
|
15892
|
15921
|
15924
|
15925
|
15926
|
15927
|
15929
|
15930
|
15931
|
15934