From 6dc7ebfa9f48deba9147e0dd1d091244d609e43a Mon Sep 17 00:00:00 2001 From: Andrew Bartlett 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 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 --- 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 . + +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] ") +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 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 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 --- 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 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 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 --- 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