From 0355f62a9b8210df6210a97bf085afa69296f4fa Mon Sep 17 00:00:00 2001 From: William Brown Date: May 23 2019 00:21:21 +0000 Subject: [PATCH 1/2] Ticket 49372 - filter optimisation improvements for common queries Bug Description: Due to the way we apply indexes to searches and the presence of the "filter test threshold" there are a number of queries which can be made faster if they understood the internals of our idl_set and index mechanisms. However, instead of expecting application authors to do this, we should provide it. Fix Description: In the server we have some cases we want to achieve, and some to avoid: * If a union has an unindexed candidate, we throw away all work and return an ALLIDS idls. * In an intersection, if we have an idl that is less than filter test threshold, we return immediately that idl rather than accessing all others, and perform a filter test. Knowing these two properties, we can now look at improving filters for queries. In a common case, SSSD will give us a query which is a union of host cn and sudoHost rules. However, the sudoHost rules are substring searchs that are not able to be indexed - thus the whole filter becomes an unindexed search. IE: (|(cn=a)(cn=b)(cn= ....)(sudoHost=[*]*)) So in this case we want to move the substring to the first query so that if it's un-indexed, we fail immediately with ALLIDS rather than opening the cn index. For intersection, we often see: (&(objectClass=account)(objectClass=posixAccount)(uid=william)) The issue here is that the idls for account and posixAccount both may contain 100,000 items. Even with idl lookthrough limits, until we start to read these, we don't know if we will exceed that. A better query is: (&(uid=william)(objectClass=account)(objectClass=posixAccount)) Because the uid=william index will contain a single item, this put's us below filter test threshold, and we will not open the objectClass indexes. In fact, in an intersection, it is almost always better to perform simple equalities first: (&(uid=william)(modifyTimestamp>=...)(sn=br*)(objectClass=posixAccount)) In most other cases, we will not greatly benefit from re-arrangement due to the size of the idls involved we won't hit filter test. IE (&(modifyTimestamp>=...)(sn=br*)(objectClass=posixAccount)) Would not be significantly better despite and possible arrangement without knowing the content of sn. So in summary, our rules for improving queries are: * unions-with-substrings should have substrings *first* * intersection-with-equality should have all non-objectclass equality filters *first*. https://pagure.io/389-ds-base/issue/49372 https://pagure.io/389-ds-base/issue/50073 Author: wibrown Original Review by: lkrispen, mreynolds (Thanks!) Review by: mreynolds (Thanks!) --- diff --git a/Makefile.am b/Makefile.am index 01ac3a0..1471af6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2132,6 +2132,7 @@ TESTS = test_slapd \ test_slapd_SOURCES = test/main.c \ test/libslapd/test.c \ test/libslapd/counters/atomic.c \ + test/libslapd/filter/optimise.c \ test/libslapd/pblock/analytics.c \ test/libslapd/pblock/v3_compat.c \ test/libslapd/operation/v3_compat.c \ diff --git a/dirsrvtests/tests/suites/filter/basic_filter_test.py b/dirsrvtests/tests/suites/filter/basic_filter_test.py index 0fca56f..261677f 100644 --- a/dirsrvtests/tests/suites/filter/basic_filter_test.py +++ b/dirsrvtests/tests/suites/filter/basic_filter_test.py @@ -1,12 +1,15 @@ # --- BEGIN COPYRIGHT BLOCK --- # Copyright (C) 2019 RED Hat, Inc. +# Copyright (C) 2019 William Brown # All rights reserved. # # License: GPL (version 3 or any later version). # See LICENSE for details. # --- END COPYRIGHT BLOCK ---- -import pytest, os +import ldap +import pytest +import os from lib389._constants import DEFAULT_SUFFIX, PW_DM from lib389.topologies import topology_st as topo @@ -28,20 +31,22 @@ def test_search_attr(topo): 1. Entry should be added 2. Operation should succeed """ - user = UserAccounts(topo.standalone, DEFAULT_SUFFIX) + users = UserAccounts(topo.standalone, DEFAULT_SUFFIX) for i in range(1, 5): - user1 = user.create_test_user(uid=i) - user1.set("mail", "AnujBorah{}@ok.com".format(i)) + user = users.create_test_user(uid=i) + user.set("mail", "email{}@example.com".format(i)) - # Testing filter is working for any king of attr + # Testing users filter is working for any kind of attr - user = Accounts(topo.standalone, DEFAULT_SUFFIX) - - assert len(user.filter('(mail=*)')) == 4 - assert len(user.filter('(uid=*)')) == 4 + assert len(users.filter('(mail=*)')) == 4 + assert len(users.filter('(uid=*)')) == 4 # Testing filter is working for other filters - assert len(user.filter("(objectclass=inetOrgPerson)")) == 4 + assert len(users.filter("(objectclass=nsAccount)")) == 4 + + # Test that invalid filter fails (note the missing ()" + with pytest.raises(ldap.FILTER_ERROR): + users.filter("objectclass=nsAccount") if __name__ == "__main__": diff --git a/dirsrvtests/tests/suites/filter/complex_filters_test.py b/dirsrvtests/tests/suites/filter/complex_filters_test.py index 37d7688..5cd5d23 100644 --- a/dirsrvtests/tests/suites/filter/complex_filters_test.py +++ b/dirsrvtests/tests/suites/filter/complex_filters_test.py @@ -1,3 +1,11 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2019 William Brown +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK ---- + import logging import pytest import os @@ -16,6 +24,8 @@ else: log = logging.getLogger(__name__) ALL_FILTERS = [] +OU_PEOPLE = f"ou=People,{DEFAULT_SUFFIX}" + # Parameterized filters to test AND_FILTERS = [("(&(uid=uid1)(sn=last1)(givenname=first1))", 1), @@ -30,7 +40,9 @@ AND_FILTERS = [("(&(uid=uid1)(sn=last1)(givenname=first1))", 1), OR_FILTERS = [("(|(uid=uid1)(sn=last1)(givenname=first1))", 1), ("(|(uid=uid1)(|(sn=last1)(givenname=first1)))", 1), ("(|(uid=uid1)(|(|(sn=last1))(|(givenname=first1))))", 1), - ("(|(objectclass=*)(sn=last1)(|(givenname=first1)))", 14), + # This returns 6, not 5, due to SCOPE_SUBTREE, and oc=* collects + # the ou due to OR condition. + ("(|(objectclass=*)(sn=last1)(|(givenname=first1)))", 6), ("(|(&(objectclass=*)(sn=last1))(|(givenname=first1)))", 1), ("(|(&(objectclass=*)(sn=last))(|(givenname=first1)))", 1)] @@ -87,9 +99,13 @@ ALL_FILTERS += RANGE_FILTERS @pytest.fixture(scope="module") def setup(topo, request): - """Add teset users + """Add test users """ + # First, purge the subtree of any demo accounts we may have created + # as part of the setup process. + topo.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL) + users = UserAccounts(topo.standalone, DEFAULT_SUFFIX) for i in range(1, 6): users.create(properties={ @@ -106,7 +122,8 @@ def setup(topo, request): @pytest.mark.parametrize("myfilter, expected_results", ALL_FILTERS) def test_filters(topo, setup, myfilter, expected_results): """Test various complex search filters and verify they are returning the - expected number of entries + expected number of entries. We limit this to OU_PEOPLE, because we can't + guarantee the version of the DB we have, and what entries also exist. :id: ee9ead27-5f63-4aed-844d-c39b99138c8d :setup: standalone @@ -120,7 +137,7 @@ def test_filters(topo, setup, myfilter, expected_results): log.info("Testing filter \"{}\"...".format(myfilter)) try: - entries = topo.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, myfilter) + entries = topo.standalone.search_s(OU_PEOPLE, ldap.SCOPE_SUBTREE, myfilter) if len(entries) != expected_results: log.fatal("Search filter \"{}\") returned {} entries, but we expected {}".format( myfilter, len(entries), expected_results)) diff --git a/dirsrvtests/tests/suites/filter/filter_logic_test.py b/dirsrvtests/tests/suites/filter/filter_logic_test.py index d600916..6323e26 100644 --- a/dirsrvtests/tests/suites/filter/filter_logic_test.py +++ b/dirsrvtests/tests/suites/filter/filter_logic_test.py @@ -1,5 +1,6 @@ # --- BEGIN COPYRIGHT BLOCK --- # Copyright (C) 2017 Red Hat, Inc. +# Copyright (C) 2019 William Brown # All rights reserved. # # License: GPL (version 3 or any later version). @@ -11,7 +12,7 @@ import pytest import ldap from lib389.topologies import topology_st -from lib389._constants import DEFAULT_SUFFIX +from lib389._constants import DEFAULT_SUFFIX, ErrorLog from lib389.idm.user import UserAccounts @@ -26,6 +27,7 @@ important to note, some tests check greater than 10 elements to assert that k-wa works, where as most of these actually hit the filtertest threshold so they early return. """ +OU_PEOPLE = f"ou=People,{DEFAULT_SUFFIX}" USER0_DN = 'uid=user0,ou=People,%s' % DEFAULT_SUFFIX USER1_DN = 'uid=user1,ou=People,%s' % DEFAULT_SUFFIX USER2_DN = 'uid=user2,ou=People,%s' % DEFAULT_SUFFIX @@ -49,6 +51,10 @@ USER19_DN = 'uid=user19,ou=People,%s' % DEFAULT_SUFFIX @pytest.fixture(scope="module") def topology_st_f(topology_st): + # First, purge the subtree of any demo accounts we may have created + # as part of the setup process. + topology_st.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL) + # Add our users to the topology_st users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) for i in range(0, 20): @@ -58,19 +64,31 @@ def topology_st_f(topology_st): 'sn': '%s' % i, 'uidNumber': '%s' % i, 'gidNumber': '%s' % i, - 'homeDirectory': '/home/user%s' % i + 'homeDirectory': '/home/user%s' % i, + 'description': 'test', }) # return it # print("ATTACH NOW") # import time # time.sleep(30) + + # Setup logging. + # topology_st.standalone.config.loglevel([ErrorLog.DEFAULT, ErrorLog.SEARCH_FILTER]) + return topology_st.standalone -def _check_filter(topology_st_f, filt, expect_len, expect_dns): - # print("checking %s" % filt) - results = topology_st_f.search_s("ou=People,%s" % DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, filt, ['uid',]) +def _check_filter(topology_st_f, filt, expect_len, expect_dns, scope=ldap.SCOPE_ONELEVEL): + print("++++++++++ checking %s" % filt) + + results = topology_st_f.search_s("ou=People,%s" % DEFAULT_SUFFIX, scope, filt, ['uid',]) + # Because LDAP allows case-sensitive in dns, but the ldap compare + # is case-insensitive, we have to normalise our checks. + result_dns = [result.dn.lower() for result in results] + expect_dns = [expect.lower() for expect in expect_dns] + print(sorted(result_dns)) + print(sorted(expect_dns)) + assert len(results) == expect_len - result_dns = [result.dn for result in results] assert set(expect_dns) == set(result_dns) @@ -444,4 +462,83 @@ def test_or_and_eq(topology_st_f): _check_filter(topology_st_f, '(|(&(uid=user0)(sn=1))(uid=user0))', 1, [USER0_DN]) _check_filter(topology_st_f, '(|(&(uid=user1)(sn=1))(uid=user0))', 2, [USER0_DN, USER1_DN]) +def test_filter_shortcut_correct(topology_st): + """ + Check that with an AND condition on shortcut, we correctly apply the filter + test to the candidate set. + """ + # First, purge the subtree of any demo accounts we may have created + # as part of the setup process. + topology_st.standalone.delete_branch_s(OU_PEOPLE, ldap.SCOPE_ONELEVEL) + + # Add our user to the topology_st: It has to be a single user to keey objectClass=posixAccount + # below filter test to emulate the IPA bug. + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) + users.create(properties={ + 'uid': 'user1', + 'cn': 'user1', + 'sn': '1', + 'uidNumber': '1', + 'gidNumber': '1', + 'homeDirectory': '/home/user1', + 'description': 'test', + }) + # return it + # print("ATTACH NOW") + # import time + # time.sleep(30) + + # Setup logging. + # topology_st.standalone.config.loglevel([ErrorLog.DEFAULT, ErrorLog.SEARCH_FILTER]) + + # V-- should become allIDS + # V-- should be less than test threshold + _check_filter(topology_st.standalone, '(&(|(uid=user1)(description=test))(objectClass=posixAccount)(uid=*)(objectClass=group))', 0, [], ldap.SCOPE_SUBTREE) + + +def test_ipa_cert_filter_complex(topology_st_f): + """ + IPA generates a number of interesting filters, that have historically broken filter + optimisation. This is a pair of those for testing. + + working + a = "(& + (| + (usercertificate;binary=0\82\03\AD0\82\02\95\A0\03\02\01\02\02\01\130\0D\06\09\2A\86H\86\F7\0D\01\01\0B\05) + (ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1) + ) + (objectClass=posixAccount) + (uid=*) + (& + (uidNumber=*) + (! + (uidNumber=0) + ) + ) + )" + failing + b = "(& + (| + (usercertificate;binary=0\82\03\AD0\82\02\95\A0\03\02\01\02\02\01\130\0D\06\09\2A\86H\86\F7\0D\01\01\0B\05) + (ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1) + (altsecurityidentities=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1) + ) + (objectClass=posixAccount) + (uid=*) + (& + (uidNumber=*) + (! + (uidNumber=0) + ) + ) + )" + + """ + + results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',]) + results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1)(altsecurityidentities=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',]) + + results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',]) + results = topology_st_f.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, "(&(|(usercertificate;binary=0\\82\\03\\AD0\\82\\02\\95\\A0\\03\\02\\01\\02\\02\\01\\130\\0D\\06\\09\\2A\\86H\\86\\F7\\0D\\01\\01\\0B\\05)(ipaCertMapData=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1)(altsecurityidentities=X509:O=TESTRELM.TEST,CN=Certificate AuthorityO=TESTRELM.TEST,CN=ipauser1))(objectClass=posixAccount)(uid=*)(&(uidNumber=*)(!(uidNumber=0))))", ['uid',]) + diff --git a/dirsrvtests/tests/suites/filter/filter_test.py b/dirsrvtests/tests/suites/filter/filter_test.py index 08521c4..3786603 100644 --- a/dirsrvtests/tests/suites/filter/filter_test.py +++ b/dirsrvtests/tests/suites/filter/filter_test.py @@ -114,7 +114,7 @@ def test_filter_scope_one(topology_st): :id: cf5a6078-bbe6-4d43-ac71-553c45923f91 :setup: Standalone instance :steps: - 1. Search cn=Directory Administrators,dc=example,dc=com using ldapsearch with + 1. Search ou=people,dc=example,dc=com using ldapsearch with scope one using base as dc=example,dc=com 2. Check that search should return only one entry :expectedresults: @@ -122,11 +122,8 @@ def test_filter_scope_one(topology_st): 2. This should pass """ - parent_dn="dn: dc=example,dc=com" - child_dn="dn: cn=Directory Administrators,dc=example,dc=com" - log.info('Search user using ldapsearch with scope one') - results = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'cn=Directory Administrators',['cn'] ) + results = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'ou=People',['ou'] ) log.info(results) log.info('Search should only have one entry') diff --git a/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py b/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py index 355b3f4..8149565 100644 --- a/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py +++ b/dirsrvtests/tests/suites/filter/rfc3673_all_oper_attrs_test.py @@ -1,5 +1,6 @@ # --- BEGIN COPYRIGHT BLOCK --- # Copyright (C) 2016 Red Hat, Inc. +# Copyright (C) 2019 William Brown # All rights reserved. # # License: GPL (version 3 or any later version). @@ -11,30 +12,34 @@ from lib389.tasks import * from lib389.utils import * from lib389.topologies import topology_st from lib389.idm.user import UserAccounts +from lib389.idm.account import Account +from lib389.idm.directorymanager import DirectoryManager +from lib389.idm.domain import Domain from lib389._constants import DN_DM, DEFAULT_SUFFIX, DN_CONFIG, PASSWORD pytestmark = pytest.mark.tier1 DN_PEOPLE = 'ou=people,%s' % DEFAULT_SUFFIX -DN_ROOT = '' +# The RootDSE entry has dn of '' +DN_DSE = '' TEST_USER_NAME = 'all_attrs_test' TEST_USER_DN = 'uid=%s,%s' % (TEST_USER_NAME, DN_PEOPLE) TEST_USER_PWD = 'all_attrs_test' # Suffix for search, Regular user boolean, List of expected attrs -TEST_PARAMS = [(DN_ROOT, False, [ +TEST_PARAMS = [(DN_DSE, False, [ 'aci', 'createTimestamp', 'creatorsName', 'modifiersName', 'modifyTimestamp', 'namingContexts', - 'nsBackendSuffix', 'nsUniqueId', 'subschemaSubentry', + 'nsBackendSuffix', 'subschemaSubentry', 'supportedControl', 'supportedExtension', 'supportedFeatures', 'supportedLDAPVersion', 'supportedSASLMechanisms', 'vendorName', 'vendorVersion' ]), - (DN_ROOT, True, [ + (DN_DSE, True, [ 'createTimestamp', 'creatorsName', 'modifiersName', 'modifyTimestamp', 'namingContexts', - 'nsBackendSuffix', 'nsUniqueId', 'subschemaSubentry', + 'nsBackendSuffix', 'subschemaSubentry', 'supportedControl', 'supportedExtension', 'supportedFeatures', 'supportedLDAPVersion', 'supportedSASLMechanisms', 'vendorName', 'vendorVersion' @@ -44,20 +49,16 @@ TEST_PARAMS = [(DN_ROOT, False, [ 'entryid', 'modifiersName', 'modifyTimestamp', 'nsUniqueId', 'numSubordinates', 'parentid' ]), - (DN_PEOPLE, True, [ - 'createTimestamp', 'creatorsName', 'entrydn', - 'entryid', 'modifyTimestamp', 'nsUniqueId', - 'numSubordinates', 'parentid' - ]), + # In 1.4.0, you can read ou/oc, but due to the design of this test + # which is testing the "+" behaviour, we only get oc in the * test. + # IE we see nothing besid OC in this test (because ou is *, not +) + (DN_PEOPLE, True, []), (TEST_USER_DN, False, [ 'createTimestamp', 'creatorsName', 'entrydn', 'entryid', 'modifiersName', 'modifyTimestamp', 'nsUniqueId', 'parentid' ]), - (TEST_USER_DN, True, [ - 'createTimestamp', 'creatorsName', 'entrydn', - 'entryid', 'modifyTimestamp', 'nsUniqueId', 'parentid' - ]), + (TEST_USER_DN, True, ['nsUniqueId']), (DN_CONFIG, False, [ 'numSubordinates', 'passwordHistory' ]) @@ -85,13 +86,13 @@ def user_aci(topology_st): """Don't allow modifiersName attribute for the test user under whole suffix """ - - ACI_TARGET = '(targetattr= "modifiersName")' - ACI_RULE = ('(version 3.0; acl "Deny modifiersName for user"; deny (read)' - ' userdn = "ldap:///%s";)' % TEST_USER_DN) - ACI_BODY = ensure_bytes(ACI_TARGET + ACI_RULE) - topology_st.standalone.modify_s(DEFAULT_SUFFIX, [(ldap.MOD_ADD, 'aci', ACI_BODY)]) - + d = Domain(topology_st.standalone, DEFAULT_SUFFIX) + planned_aci = f'(targetattr= "modifiersName")(version 3.0; acl "Deny modifiersName for user"; deny (read) userdn = "ldap:///{TEST_USER_DN}";)' + print("++++++++++++++++++++ %s" % planned_aci) + d.add( + 'aci', + planned_aci + ) def test_supported_features(topology_st): """Verify that OID 1.3.6.1.4.1.4203.1.5.1 is published @@ -113,8 +114,8 @@ def test_supported_features(topology_st): assert supported_value == [b'1.3.6.1.4.1.4203.1.5.1'] -@pytest.mark.parametrize('add_attr', ['', '*', 'objectClass']) -@pytest.mark.parametrize('search_suffix,regular_user,oper_attr_list', +@pytest.mark.parametrize('add_attr', [None, '*', 'objectClass']) +@pytest.mark.parametrize('search_suffix, regular_user, oper_attr_list', TEST_PARAMS) def test_search_basic(topology_st, create_user, user_aci, add_attr, search_suffix, regular_user, oper_attr_list): @@ -139,28 +140,43 @@ def test_search_basic(topology_st, create_user, user_aci, add_attr, 3. It should pass """ + conn = None + if regular_user: log.info("bound as: %s", TEST_USER_DN) - topology_st.standalone.simple_bind_s(TEST_USER_DN, ensure_bytes(TEST_USER_PWD)) + conn = Account(topology_st.standalone, dn=TEST_USER_DN).bind(TEST_USER_PWD) else: - log.info("bound as: %s", DN_DM) - topology_st.standalone.simple_bind_s(DN_DM, ensure_bytes(PASSWORD)) + log.info("Binding as: %s", DN_DM) + conn = DirectoryManager(topology_st.standalone).bind() + + assert conn is not None - search_filter = ['+'] + requested_attributes = ['+'] expected_attrs = oper_attr_list - if add_attr: - search_filter.append(add_attr) + if add_attr is not None: + requested_attributes.append(add_attr) expected_attrs += ['objectClass'] - entries = topology_st.standalone.search_s(search_suffix, ldap.SCOPE_BASE, + print('==================') + print(search_suffix) + print(regular_user) + print(requested_attributes) + print(set(expected_attrs)) + + entries = conn.search_s(search_suffix, ldap.SCOPE_BASE, '(objectclass=*)', - search_filter) + requested_attributes) found_attrs = entries[0].data.keys() - if add_attr == '*': - assert set(expected_attrs) - set(found_attrs) == set() - else: - assert set(expected_attrs) == set(found_attrs) + print(set(found_attrs)) + print('==================') + + + assert set(expected_attrs).issubset(set(found_attrs)) + # if add_attr == '*': + # assert set(expected_attrs) - set(found_attrs) == set() + # else: + # assert set(expected_attrs) == set(found_attrs) if __name__ == '__main__': diff --git a/ldap/admin/src/scripts/ns-slapd-gdb.py b/ldap/admin/src/scripts/ns-slapd-gdb.py index 73e25aa..70c1ba9 100644 --- a/ldap/admin/src/scripts/ns-slapd-gdb.py +++ b/ldap/admin/src/scripts/ns-slapd-gdb.py @@ -16,9 +16,22 @@ import itertools import re +from enum import IntEnum + import gdb from gdb.FrameDecorator import FrameDecorator +class LDAPFilter(IntEnum): + PRESENT = 0x87 + APPROX = 0xa8 + LE = 0xa6 + GE = 0xa5 + SUBSTRINGS = 0xa4 + EQUALITY = 0xa3 + NOT = 0xa2 + OR = 0xa1 + AND = 0xa0 + class DSAccessLog (gdb.Command): """Display the Directory Server access log.""" def __init__ (self): @@ -163,9 +176,65 @@ class DSEntryPrint (gdb.Command): entry_attrs = entry_root['e_attrs'] self._display_attrs(entry_attrs) +class DSFilterPrint (gdb.Command): + """Display a filter's contents""" + def __init__ (self): + super (DSFilterPrint, self).__init__ ("ds-filter-print", gdb.COMMAND_DATA) + + def display_filter(self, filter_element, depth=0): + pad = " " * depth + # Extract the choice, that determines what we access next. + f_choice = filter_element['f_choice'] + f_un = filter_element['f_un'] + f_flags = filter_element['f_flags'] + if f_choice == LDAPFilter.PRESENT: + print("%s(%s=*) flags:%s" % (pad, f_un['f_un_type'], f_flags)) + elif f_choice == LDAPFilter.APPROX: + print("%sAPPROX ???" % pad) + elif f_choice == LDAPFilter.LE: + print("%sLE ???" % pad) + elif f_choice == LDAPFilter.GE: + print("%sGE ???" % pad) + elif f_choice == LDAPFilter.SUBSTRINGS: + f_un_sub = f_un['f_un_sub'] + value = f_un_sub['sf_initial'] + print("%s(%s=%s*) flags:%s" % (pad, f_un_sub['sf_type'], value, f_flags)) + elif f_choice == LDAPFilter.EQUALITY: + f_un_ava = f_un['f_un_ava'] + value = f_un_ava['ava_value']['bv_val'] + print("%s(%s=%s) flags:%s" % (pad, f_un_ava['ava_type'], value, f_flags)) + elif f_choice == LDAPFilter.NOT: + print("%sNOT ???" % pad) + elif f_choice == LDAPFilter.OR: + print("%s(| flags:%s" % (pad, f_flags)) + filter_child = f_un['f_un_complex'].dereference() + self.display_filter(filter_child, depth + 4) + print("%s)" % pad) + elif f_choice == LDAPFilter.AND: + # Our child filter is in f_un_complex. + print("%s(& flags:%s" % (pad, f_flags)) + filter_child = f_un['f_un_complex'].dereference() + self.display_filter(filter_child, depth + 4) + print("%s)" % pad) + else: + print("Corrupted filter, no such value %s" % f_choice) + + f_next = filter_element['f_next'] + if f_next != 0: + self.display_filter(f_next.dereference(), depth) + + def invoke (self, arg, from_tty): + # Select our program state + gdb.newest_frame() + cur_frame = gdb.selected_frame() + # We are given the name of a filter, so we need to look up that symbol. + filter_val = cur_frame.read_var(arg) + filter_root = filter_val.dereference() + self.display_filter(filter_root) + DSAccessLog() DSBacktrace() DSIdleFilter() DSEntryPrint() - +DSFilterPrint() diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif index 993fa4a..b6c8fc7 100644 --- a/ldap/schema/01core389.ldif +++ b/ldap/schema/01core389.ldif @@ -312,6 +312,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2341 NAME 'nsslapd-changelogmaxentries' attributeTypes: ( 2.16.840.1.113730.3.1.2344 NAME 'nsslapd-tls-check-crl' DESC 'Check CRL when opening outbound TLS connections. Valid options are none, peer, all.' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2353 NAME 'nsslapd-encryptionalgorithm' DESC 'The encryption algorithm used to encrypt the changelog' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2084 NAME 'nsSymmetricKey' DESC 'A symmetric key - currently used by attribute encryption' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN 'attribute encryption' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2360 NAME 'nsslapd-filter-optimise-enable' DESC 'Enable or disable the query optimiser within the server' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) # # objectclasses # diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index 5039783..8f1af33 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -82,6 +82,12 @@ filter_candidates_ext( } } + if (slapi_is_loglevel_set(SLAPI_LOG_FILTER)) { + char fbuf[4096] = {0}; + slapi_filter_to_string(f, fbuf, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "filter_candidates_ext", "processing filt -> %s\n", fbuf); + } + result = NULL; switch ((ftype = slapi_filter_get_choice(f))) { case LDAP_FILTER_EQUALITY: @@ -144,8 +150,7 @@ filter_candidates_ext( break; } - slapi_log_err(SLAPI_LOG_TRACE, "filter_candidates_ext", "<= %lu\n", - (u_long)IDL_NIDS(result)); + slapi_log_err(SLAPI_LOG_FILTER, "filter_candidates_ext", "<= IDL length(%lu)\n", (u_long)IDL_NIDS(result)); return (result); } @@ -631,7 +636,7 @@ list_candidates( int is_and = 0; IDListSet *idl_set = NULL; - slapi_log_err(SLAPI_LOG_TRACE, "list_candidates", "=> 0x%x\n", ftype); + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "=> 0x%x\n", ftype); /* * Optimize bounded range queries such as (&(cn>=A)(cn<=B)). @@ -814,21 +819,31 @@ list_candidates( idl_set_insert_complement_idl(idl_set, tmp); } - if (ftype == LDAP_FILTER_OR && idl_set_union_shortcut(idl_set) != 0) { - /* - * If we encounter an allids idl, this means that union will return - * and allids - we should not process anymore, and fallback to full - * table scan at this point. - */ - goto apply_set_op; + if (ftype == LDAP_FILTER_OR) { + if (idl_set_union_shortcut(idl_set) != 0) { + /* + * If we encounter an allids idl, this means that union will return + * and allids - we should not process anymore, and fallback to full + * table scan at this point. + */ + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "union shortcut\n"); + goto apply_set_op; + } else { + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "NOT applying union shortcut\n"); + } } - if (ftype == LDAP_FILTER_AND && idl_set_intersection_shortcut(idl_set) != 0) { - /* - * If we encounter a zero length idl, we bail now because this can never - * result in a meaningful result besides zero. - */ - goto apply_set_op; + if (ftype == LDAP_FILTER_AND) { + if (idl_set_intersection_shortcut(idl_set) != 0) { + /* + * If we encounter a zero length idl, we bail now because this can never + * result in a meaningful result besides zero. + */ + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "intersection shortcut (filter test threshold)\n"); + goto apply_set_op; + } else { + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "NOT applying intersection shortcut (filter test threshold)\n"); + } } } @@ -856,8 +871,7 @@ apply_set_op: idl = idl_set_intersect(idl_set, be); } - slapi_log_err(SLAPI_LOG_TRACE, "list_candidates", "<= %lu\n", - (u_long)IDL_NIDS(idl)); + slapi_log_err(SLAPI_LOG_FILTER, "list_candidates", "<= 0x%x IDL length(%lu)\n", ftype, (u_long)IDL_NIDS(idl)); out: idl_set_destroy(idl_set); if (is_and) { diff --git a/ldap/servers/slapd/back-ldbm/idl_set.c b/ldap/servers/slapd/back-ldbm/idl_set.c index 6b65867..c90d4c6 100644 --- a/ldap/servers/slapd/back-ldbm/idl_set.c +++ b/ldap/servers/slapd/back-ldbm/idl_set.c @@ -252,12 +252,16 @@ idl_set_union(IDListSet *idl_set, backend *be) * Check allids first, because allids = 1, may not * have set count > 0. */ - if (idl_set->allids != 0) { + if (idl_set_union_shortcut(idl_set) != 0) { idl_set_free_idls(idl_set); + /* An OR condition was allids or unindexed, do not bypass */ + slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST); return idl_allids(be); } else if (idl_set->count == 0) { + /* Empty union */ return idl_alloc(0); } else if (idl_set->count == 1) { + /* Only one set, must be only this IDL */ return idl_set->head; } else if (idl_set->count == 2) { IDList *result_list = idl_union(be, idl_set->head, idl_set->head->next); @@ -350,7 +354,11 @@ idl_set_intersect(IDListSet *idl_set, backend *be) IDList *result_list = NULL; if (idl_set->allids) { - /* if any component was allids we have to apply the filtertest */ + /* + * if any component was allids we have to apply the filtertest + * However, we can still continue as we DO reduce the cand set + * in this process. + */ slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST); } @@ -369,7 +377,11 @@ idl_set_intersect(IDListSet *idl_set, backend *be) * If allids, when we intersect head, we get head, so just skip that. */ result_list = idl_set->head; - } else if (idl_set->minimum->b_nids <= FILTER_TEST_THRESHOLD) { + } else if (idl_set_intersection_shortcut(idl_set)) { + /* + * We return the small IDL here, because we then only need to inspect the + * smallest possible matching candidate set. + */ result_list = idl_set->minimum; slapi_be_set_flag(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 65610d6..d2d543e 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -2099,8 +2099,13 @@ moddn_get_children(back_txn *ptxn, * being moved */ strcpy(filterstr, "objectclass=*"); filter = slapi_str2filter(filterstr); + /* + * We used to set managedSAIT here, but because the subtree create + * referral step is now in build_candidate_list, we can trust the filter + * we provide here is exactly as we provide it IE no referrals involved. + */ candidates = subtree_candidates(pb, be, slapi_sdn_get_ndn(dn_parentdn), - parententry, filter, 1 /* ManageDSAIT */, + parententry, filter, NULL /* allids_before_scopingp */, &err); slapi_filter_free(filter, 1); } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index cabc8bd..da053e2 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -31,10 +31,10 @@ /* prototypes */ static int build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const char *base, int scope, int *lookup_returned_allidsp, IDList **candidates); static IDList *base_candidates(Slapi_PBlock *pb, struct backentry *e); -static IDList *onelevel_candidates(Slapi_PBlock *pb, backend *be, const char *base, struct backentry *e, Slapi_Filter *filter, int managedsait, int *lookup_returned_allidsp, int *err); +static IDList *onelevel_candidates(Slapi_PBlock *pb, backend *be, const char *base, Slapi_Filter *filter, int *lookup_returned_allidsp, int *err); static back_search_result_set *new_search_result_set(IDList *idl, int vlv, int lookthroughlimit); static void delete_search_result_set(Slapi_PBlock *pb, back_search_result_set **sr); -static int can_skip_filter_test(Slapi_PBlock *pb, struct slapi_filter *f, int scope, IDList *idl); +static int can_skip_filter_test(Slapi_PBlock *pb, backend *be, struct slapi_filter *f, int scope, IDList *idl); /* This is for performance testing, allows us to disable ACL checking altogether */ #if defined(DISABLE_ACL_CHECK) @@ -881,7 +881,7 @@ ldbm_back_search(Slapi_PBlock *pb) tmp_desc = "Filter is not set"; goto bail; } - if (can_skip_filter_test(pb, filter, scope, candidates)) { + if (can_skip_filter_test(pb, be, filter, scope, candidates)) { sr->sr_flags |= SR_FLAG_CAN_SKIP_FILTER_TEST; } } @@ -942,6 +942,7 @@ build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const c Slapi_Filter *filter = NULL; int err = 0; int r = 0; + int32_t filter_optimise = 0; slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter); if (NULL == filter) { @@ -952,19 +953,33 @@ build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const c slapi_pblock_get(pb, SLAPI_MANAGEDSAIT, &managedsait); + filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); + switch (scope) { case LDAP_SCOPE_BASE: *candidates = base_candidates(pb, e); break; case LDAP_SCOPE_ONELEVEL: - *candidates = onelevel_candidates(pb, be, base, e, filter, managedsait, - lookup_returned_allidsp, &err); + /* modify the filter to be: (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */ + filter = create_onelevel_filter(filter, e, managedsait); + /* Now optimise the filter for use */ + slapi_filter_optimise(filter, filter_optimise); + + *candidates = onelevel_candidates(pb, be, base, filter, lookup_returned_allidsp, &err); + /* Give the optimised filter back to search filter for free */ + slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); break; case LDAP_SCOPE_SUBTREE: - *candidates = subtree_candidates(pb, be, base, e, filter, managedsait, - lookup_returned_allidsp, &err); + /* make (|(originalfilter)(objectclass=referral)) */ + filter = create_subtree_filter(filter, managedsait); + /* Now optimise the filter for use */ + slapi_filter_optimise(filter, filter_optimise); + + *candidates = subtree_candidates(pb, be, base, e, filter, lookup_returned_allidsp, &err); + /* Give the optimised filter back to search filter for free */ + slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); break; default: @@ -1018,20 +1033,17 @@ base_candidates(Slapi_PBlock *pb __attribute__((unused)), struct backentry *e) * Modify the filter to include entries of the referral objectclass * * make (|(originalfilter)(objectclass=referral)) - * - * "focref, forr" are temporary filters which the caller must free - * non-recursively when done with the returned filter. */ static Slapi_Filter * -create_referral_filter(Slapi_Filter *filter, Slapi_Filter **focref, Slapi_Filter **forr) +create_referral_filter(Slapi_Filter *filter) { - char *buf = slapi_ch_strdup("objectclass=referral"); + char *buf = slapi_ch_strdup("(objectclass=referral)"); - *focref = slapi_str2filter(buf); - *forr = slapi_filter_join(LDAP_FILTER_OR, filter, *focref); + Slapi_Filter *focref = slapi_str2filter(buf); + Slapi_Filter *forr = slapi_filter_join(LDAP_FILTER_OR, focref, filter); slapi_ch_free((void **)&buf); - return *forr; + return forr; } /* @@ -1039,26 +1051,23 @@ create_referral_filter(Slapi_Filter *filter, Slapi_Filter **focref, Slapi_Filter * * (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) * - * "fid2kids, focref, fand, forr" are temporary filters which the - * caller must free'd non-recursively when done with the returned filter. - * * This function is exported for the VLV code to use. */ Slapi_Filter * -create_onelevel_filter(Slapi_Filter *filter, const struct backentry *baseEntry, int managedsait, Slapi_Filter **fid2kids, Slapi_Filter **focref, Slapi_Filter **fand, Slapi_Filter **forr) +create_onelevel_filter(Slapi_Filter *filter, const struct backentry *baseEntry, int managedsait) { Slapi_Filter *ftop = filter; char buf[40]; if (!managedsait) { - ftop = create_referral_filter(filter, focref, forr); + ftop = create_referral_filter(filter); } - sprintf(buf, "parentid=%lu", (u_long)(baseEntry != NULL ? baseEntry->ep_id : 0)); - *fid2kids = slapi_str2filter(buf); - *fand = slapi_filter_join(LDAP_FILTER_AND, ftop, *fid2kids); + sprintf(buf, "(parentid=%lu)", (u_long)(baseEntry != NULL ? baseEntry->ep_id : 0)); + Slapi_Filter *fid2kids = slapi_str2filter(buf); + Slapi_Filter *fand = slapi_filter_join(LDAP_FILTER_AND, fid2kids, ftop); - return *fand; + return fand; } /* @@ -1069,38 +1078,16 @@ onelevel_candidates( Slapi_PBlock *pb, backend *be, const char *base, - struct backentry *e, Slapi_Filter *filter, - int managedsait, int *lookup_returned_allidsp, int *err) { - Slapi_Filter *fid2kids = NULL; - Slapi_Filter *focref = NULL; - Slapi_Filter *fand = NULL; - Slapi_Filter *forr = NULL; - Slapi_Filter *ftop = NULL; IDList *candidates; - /* - * modify the filter to be something like this: - * - * (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) - */ - - ftop = create_onelevel_filter(filter, e, managedsait, &fid2kids, &focref, &fand, &forr); - - /* from here, it's just like subtree_candidates */ - candidates = filter_candidates(pb, be, base, ftop, NULL, 0, err); + candidates = filter_candidates(pb, be, base, filter, NULL, 0, err); *lookup_returned_allidsp = slapi_be_is_flag_set(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST); - /* free up just the filter stuff we allocated above */ - slapi_filter_free(fid2kids, 0); - slapi_filter_free(fand, 0); - slapi_filter_free(forr, 0); - slapi_filter_free(focref, 0); - return (candidates); } @@ -1115,12 +1102,12 @@ onelevel_candidates( * This function is exported for the VLV code to use. */ Slapi_Filter * -create_subtree_filter(Slapi_Filter *filter, int managedsait, Slapi_Filter **focref, Slapi_Filter **forr) +create_subtree_filter(Slapi_Filter *filter, int managedsait) { Slapi_Filter *ftop = filter; if (!managedsait) { - ftop = create_referral_filter(filter, focref, forr); + ftop = create_referral_filter(filter); } return ftop; @@ -1137,13 +1124,9 @@ subtree_candidates( const char *base, const struct backentry *e, Slapi_Filter *filter, - int managedsait, int *allids_before_scopingp, int *err) { - Slapi_Filter *focref = NULL; - Slapi_Filter *forr = NULL; - Slapi_Filter *ftop = NULL; IDList *candidates; PRBool has_tombstone_filter; int isroot = 0; @@ -1152,17 +1135,22 @@ subtree_candidates( Operation *op = NULL; PRBool is_bulk_import = PR_FALSE; - /* make (|(originalfilter)(objectclass=referral)) */ - ftop = create_subtree_filter(filter, managedsait, &focref, &forr); - /* Fetch a candidate list for the original filter */ - candidates = filter_candidates_ext(pb, be, base, ftop, NULL, 0, err, allidslimit); - slapi_filter_free(forr, 0); - slapi_filter_free(focref, 0); + candidates = filter_candidates_ext(pb, be, base, filter, NULL, 0, err, allidslimit); /* set 'allids before scoping' flag */ if (NULL != allids_before_scopingp) { - *allids_before_scopingp = (NULL != candidates && ALLIDS(candidates)); + /* + * If we have candidates, AND they are allids OR we requested the + * don't bypass flag: we say that there is all ids etc. So by returning + * 1 here, this affects the "can skip filter test", but doing !allids. IE + * return 1 here, to FORCE the filter test to be taken! + */ + if (NULL != candidates && (ALLIDS(candidates) || slapi_be_is_flag_set(be, SLAPI_BE_FLAG_DONT_BYPASS_FILTERTEST))) { + *allids_before_scopingp = 1; + } else { + *allids_before_scopingp = 0; + } } has_tombstone_filter = (filter->f_flags & SLAPI_FILTER_TOMBSTONE); @@ -1312,10 +1300,12 @@ grok_filter(struct slapi_filter *f) static int can_skip_filter_test( Slapi_PBlock *pb __attribute__((unused)), + backend *be, struct slapi_filter *f, int scope, IDList *idl) { + /* If 0, we CAN NOT skip the test, if 1 we can! */ int rc = 0; /* Is the ID list ALLIDS ? */ @@ -1335,9 +1325,16 @@ can_skip_filter_test( } /* Grok the filter and tell me if it has only equality components in it */ + /* + * Now that we respect the SLAPI_BE_FLAG in most callers, we may not need this test as it + * (correctly) is pretty zealous about testing in most situations. + * + * Arguably, this removal would just make large set's that are fully indexed faster + * to return to the client. But on the flip side, it could let through other issues + * so we could consider that we should always filter test. + */ rc = grok_filter(f); - return rc; } @@ -1656,8 +1653,6 @@ ldbm_back_next_search_entry_ext(Slapi_PBlock *pb, int use_extension) /* it's a regular entry, check if it matches the filter, and passes the ACL check */ if (0 != (sr->sr_flags & SR_FLAG_CAN_SKIP_FILTER_TEST)) { /* Since we do access control checking in the filter test (?Why?) we need to check access now */ - slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext", - "Bypassing filter test\n"); if (ACL_CHECK_FLAG) { filter_test = slapi_vattr_filter_test_ext(pb, e->ep_entry, filter, ACL_CHECK_FLAG, 1 /* Only perform access checking, thank you */); } else { @@ -1675,8 +1670,13 @@ ldbm_back_next_search_entry_ext(Slapi_PBlock *pb, int use_extension) "Filter bypass ERROR on entry %s\n", backentry_get_ndn(e)); filter_test = ft_rc; /* Fix the error */ } + } else { + slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext", + "Bypassing filter test\n"); } } else { + slapi_log_err(SLAPI_LOG_FILTER, "ldbm_back_next_search_entry_ext", + "Applying filter test\n"); /* Old-style case---we need to do a filter test */ filter_test = slapi_vattr_filter_test(pb, e->ep_entry, filter, ACL_CHECK_FLAG); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 00d4aea..100b0bc 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -571,9 +571,9 @@ int bedse_add_index_entry(int argc, char **argv); /* * ldbm_search.c */ -Slapi_Filter *create_onelevel_filter(Slapi_Filter *filter, const struct backentry *e, int managedsait, Slapi_Filter **fid2kids, Slapi_Filter **focref, Slapi_Filter **fand, Slapi_Filter **forr); -Slapi_Filter *create_subtree_filter(Slapi_Filter *filter, int managedsait, Slapi_Filter **focref, Slapi_Filter **forr); -IDList *subtree_candidates(Slapi_PBlock *pb, backend *be, const char *base, const struct backentry *e, Slapi_Filter *filter, int managedsait, int *allids_before_scopingp, int *err); +Slapi_Filter *create_onelevel_filter(Slapi_Filter *filter, const struct backentry *e, int managedsait); +Slapi_Filter *create_subtree_filter(Slapi_Filter *filter, int managedsait); +IDList *subtree_candidates(Slapi_PBlock *pb, backend *be, const char *base, const struct backentry *e, Slapi_Filter *filter, int *allids_before_scopingp, int *err); void search_set_tune(struct ldbminfo *li, int val); int search_get_tune(struct ldbminfo *li); int compute_lookthrough_limit(Slapi_PBlock *pb, struct ldbminfo *li); diff --git a/ldap/servers/slapd/back-ldbm/vlv_srch.c b/ldap/servers/slapd/back-ldbm/vlv_srch.c index 3684174..fc9bb04 100644 --- a/ldap/servers/slapd/back-ldbm/vlv_srch.c +++ b/ldap/servers/slapd/back-ldbm/vlv_srch.c @@ -93,13 +93,13 @@ vlvSearch_reinit(struct vlvSearch *p, const struct backentry *base) p->vlv_slapifilter = slapi_str2filter(p->vlv_filter); filter_normalize(p->vlv_slapifilter); /* make (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */ - { - Slapi_Filter *fid2kids = NULL; - Slapi_Filter *focref = NULL; - Slapi_Filter *fand = NULL; - Slapi_Filter *forr = NULL; - p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, base, 0 /* managedsait */, &fid2kids, &focref, &fand, &forr); - } + p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, base, 0 /* managedsait */); + /* + * We lack the information to know if we can optimise - however, given this is + * a re-init, we may already be optimised. For now, we be safe, and do not alter + * in this context without the ability to check the flags required. + */ + /* slapi_filter_optimise(p->vlv_slapifilter); */ } /* @@ -108,6 +108,7 @@ vlvSearch_reinit(struct vlvSearch *p, const struct backentry *base) void vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm_instance *inst) { + int32_t filter_optimise = 0; /* VLV specification */ /* Need to copy the entry here because this one is in the cache, * not forever ! */ @@ -130,6 +131,8 @@ vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm filter_normalize(p->vlv_slapifilter); } + filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); + /* JCM: Really should convert the slapifilter into a string and use that. */ /* Convert the filter based on the scope of the search */ @@ -173,22 +176,16 @@ vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm /* make (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */ { - Slapi_Filter *fid2kids = NULL; - Slapi_Filter *focref = NULL; - Slapi_Filter *fand = NULL; - Slapi_Filter *forr = NULL; - p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, e, 0 /* managedsait */, &fid2kids, &focref, &fand, &forr); - /* jcm: fid2kids, focref, fand, and forr get freed when we free p->vlv_slapifilter */ + p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, e, 0 /* managedsait */); + slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); CACHE_RETURN(&inst->inst_cache, &e); } } break; case LDAP_SCOPE_SUBTREE: { /* make (|(originalfilter)(objectclass=referral))) */ /* No need for scope-filter since we apply a scope test before the filter test */ - Slapi_Filter *focref = NULL; - Slapi_Filter *forr = NULL; - p->vlv_slapifilter = create_subtree_filter(p->vlv_slapifilter, 0 /* managedsait */, &focref, &forr); - /* jcm: focref and forr get freed when we free p->vlv_slapifilter */ + p->vlv_slapifilter = create_subtree_filter(p->vlv_slapifilter, 0 /* managedsait */); + slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); } break; } } diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c index 1f1f516..9a9b3ab 100644 --- a/ldap/servers/slapd/dse.c +++ b/ldap/servers/slapd/dse.c @@ -1618,6 +1618,7 @@ dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi char returntext[SLAPI_DSE_RETURNTEXT_SIZE] = ""; Slapi_DN *basesdn = NULL; int estimate = 0; /* estimated search result set size */ + int32_t filter_optimise = 0; /* * Get private information created in the init routine. @@ -1639,6 +1640,14 @@ dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi */ isrootdse = slapi_sdn_isempty(basesdn); + /* + * Now optimise the filter for use: note that unlike ldbm_search, + * because we don't change the outer filter container, we don't need + * to set back into pb. + */ + filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); + slapi_filter_optimise(filter, filter_optimise); + switch (scope) { case LDAP_SCOPE_BASE: { Slapi_Entry *baseentry = NULL; diff --git a/ldap/servers/slapd/filter.c b/ldap/servers/slapd/filter.c index 393a4dc..4a452d6 100644 --- a/ldap/servers/slapd/filter.c +++ b/ldap/servers/slapd/filter.c @@ -29,7 +29,6 @@ static int get_extensible_filter(BerElement *ber, mr_filter_t *); static int get_filter_internal(Connection *conn, BerElement *ber, struct slapi_filter **filt, char **fstr, int maxdepth, int curdepth, int *subentry_dont_rewrite, int *has_tombstone_filter, int *has_ruv_filter); static int tombstone_check_filter(Slapi_Filter *f); static int ruv_check_filter(Slapi_Filter *f); -static void filter_optimize(Slapi_Filter *f); /* @@ -75,7 +74,12 @@ get_filter(Connection *conn, BerElement *ber, int scope, struct slapi_filter **f slapi_filter_to_string(*filt, logbuf, logbufsize)); } - filter_optimize(*filt); + /* + * Filter optimise has been moved to the onelevel/subtree candidate dispatch. + * this is because they inject referrals or other business, that we can optimise + * and improve. + */ + /* filter_optimize(*filt); */ if (NULL != logbuf) { slapi_log_err(SLAPI_LOG_DEBUG, "get_filter", " after optimize: %s\n", @@ -858,19 +862,22 @@ slapi_filter_join_ex(int ftype, struct slapi_filter *f1, struct slapi_filter *f2 if (add_to->f_list->f_choice == LDAP_FILTER_NOT) { add_this->f_next = add_to->f_list; add_to->f_list = add_this; - filter_compute_hash(add_to); - return_this = add_to; } else { /* find end of list, add the filter */ for (fjoin = add_to->f_list; fjoin != NULL; fjoin = fjoin->f_next) { if (fjoin->f_next == NULL) { fjoin->f_next = add_this; - filter_compute_hash(add_to); - return_this = add_to; break; } } } + /* + * Make sure we sync the filter flags. The origin filters may have flags + * we still need on the outer layer! + */ + add_to->f_flags |= add_this->f_flags; + filter_compute_hash(add_to); + return_this = add_to; } else { fjoin = (struct slapi_filter *)slapi_ch_calloc(1, sizeof(struct slapi_filter)); fjoin->f_choice = ftype; @@ -883,6 +890,8 @@ slapi_filter_join_ex(int ftype, struct slapi_filter *f1, struct slapi_filter *f2 fjoin->f_list = f1; f1->f_next = f2; } + /* Make sure any flags that were set move to the outer parent */ + fjoin->f_flags |= f1->f_flags | f2->f_flags; filter_compute_hash(fjoin); return_this = fjoin; } @@ -1527,49 +1536,207 @@ ruv_check_filter(Slapi_Filter *f) return 0; /* Not a RUV filter */ } +/* + * To help filter optimise we break out the list manipulation + * code. + */ -/* filter_optimize +static void +filter_prioritise_element(Slapi_Filter **list, Slapi_Filter **head, Slapi_Filter **tail, Slapi_Filter **f_prev, Slapi_Filter **f_cur) { + if (*f_prev != NULL) { + (*f_prev)->f_next = (*f_cur)->f_next; + } else if (*list == *f_cur) { + *list = (*f_cur)->f_next; + } + + if (*head == NULL) { + *head = *f_cur; + *tail = *f_cur; + (*f_cur)->f_next = NULL; + } else { + (*f_cur)->f_next = *head; + *head = *f_cur; + } +} + +static void +filter_merge_subfilter(Slapi_Filter **list, Slapi_Filter **f_prev, Slapi_Filter **f_cur, Slapi_Filter **f_next) { + + /* First, graft in the new item between f_cur and f_cur -> f_next */ + Slapi_Filter *remainder = (*f_cur)->f_next; + (*f_cur)->f_next = (*f_cur)->f_list; + /* Go to the end of the newly grafted list, and put in our remainder. */ + Slapi_Filter *f_cur_tail = *f_cur; + while (f_cur_tail->f_next != NULL) { + f_cur_tail = f_cur_tail->f_next; + } + f_cur_tail->f_next = remainder; + + /* Now indicate to the caller what the next element is. */ + *f_next = (*f_cur)->f_next; + + /* Now that we have grafted our list in, cut out f_cur */ + if (*f_prev != NULL) { + (*f_prev)->f_next = *f_next; + } else if (*list == *f_cur) { + *list = *f_next; + } + + /* Finally free the f_cur (and/or) */ + slapi_filter_free(*f_cur, 0); +} + +/* slapi_filter_optimise * --------------- - * takes a filter and optimizes it for fast evaluation - * currently this merely ensures that any AND or OR - * does not start with a NOT sub-filter if possible + * takes a filter and optimises it for fast evaluation + * + * Optimisations are: + * * In OR conditions move substrings early to promote fail-fast of unindexed types + * * In AND conditions move eq types (that are not objectClass) early to promote triggering threshold shortcut + * * In OR conditions, merge all direct child OR conditions into the list. (|(|(a)(b))) == (|(a)(b)) + * * in AND conditions, merge all direct child AND conditions into the list. (&(&(a)(b))) == (&(a)(b)) + * + * In the case of the OR and AND merges, we remove the inner filter because the outer one may have flags set. + * + * In the future this could be backend dependent. */ -static void -filter_optimize(Slapi_Filter *f) +void +slapi_filter_optimise(Slapi_Filter *f, int32_t enable) { - if (!f) + if (enable == 0) { + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_optimise", "NOT processing filter\n"); return; + } + + /* + * Today tombstone searches RELY on filter ordering + * and a filter test threshold quirk. We need to avoid + * touching these cases!!! + */ + + if (slapi_is_loglevel_set(SLAPI_LOG_FILTER)) { + char fbuf[4096] = {0}; + // char fbuf_res[4096] = {0}; + slapi_filter_to_string(f, fbuf, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_optimise", "processing filt -> %s\n", fbuf); + } + + if (f == NULL || (f->f_flags & SLAPI_FILTER_TOMBSTONE) != 0) { + if (slapi_is_loglevel_set(SLAPI_LOG_FILTER)) { + char fbuf[4096] = {0}; + slapi_filter_to_string(f, fbuf, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_optimise", "Not optimising filt -> %s\n", fbuf); + } + return; + } switch (f->f_choice) { case LDAP_FILTER_AND: - case LDAP_FILTER_OR: { - /* first optimize children */ - filter_optimize(f->f_list); - - /* optimize this */ - if (f->f_list->f_choice == LDAP_FILTER_NOT) { - Slapi_Filter *f_prev = 0; - Slapi_Filter *f_child = 0; - - /* grab a non not filter to place at start */ - for (f_child = f->f_list; f_child != 0; f_child = f_child->f_next) { - if (f_child->f_choice != LDAP_FILTER_NOT) { - /* we have a winner, do swap */ - if (f_prev) - f_prev->f_next = f_child->f_next; - f_child->f_next = f->f_list; - f->f_list = f_child; + /* Move all equality searches to the head. */ + /* Merge any direct descendant AND queries into us */ + { + Slapi_Filter *f_prev = NULL; + Slapi_Filter *f_cur = NULL; + Slapi_Filter *f_next = NULL; + + Slapi_Filter *f_op_head = NULL; + Slapi_Filter *f_op_tail = NULL; + + f_cur = f->f_list; + while(f_cur != NULL) { + + switch(f_cur->f_choice) { + case LDAP_FILTER_AND: + filter_merge_subfilter(&(f->f_list), &f_prev, &f_cur, &f_next); + f_cur = f_next; + break; + case LDAP_FILTER_EQUALITY: + if (strcasecmp(f_cur->f_avtype, "objectclass") != 0) { + f_next = f_cur->f_next; + /* Cut it out */ + filter_prioritise_element(&(f->f_list), &f_op_head, &f_op_tail, &f_prev, &f_cur); + /* Don't change previous, because we remove this f_cur */ + f_cur = f_next; + break; + } else { + /* Move along */ + f_prev = f_cur; + f_cur = f_cur->f_next; + } + break; + default: + /* Move along */ + f_prev = f_cur; + f_cur = f_cur->f_next; break; } + } - f_prev = f_child; + if (f_op_head != NULL) { + f_op_tail->f_next = f->f_list; + f->f_list = f_op_head; } } - } + /* finally optimize children */ + slapi_filter_optimise(f->f_list, 1); + + break; + + case LDAP_FILTER_OR: + /* Move all substring searches to the head. */ + { + Slapi_Filter *f_prev = NULL; + Slapi_Filter *f_cur = NULL; + Slapi_Filter *f_next = NULL; + + Slapi_Filter *f_op_head = NULL; + Slapi_Filter *f_op_tail = NULL; + + f_cur = f->f_list; + while(f_cur != NULL) { + + switch(f_cur->f_choice) { + case LDAP_FILTER_OR: + filter_merge_subfilter(&(f->f_list), &f_prev, &f_cur, &f_next); + f_cur = f_next; + break; + case LDAP_FILTER_APPROX: + case LDAP_FILTER_GE: + case LDAP_FILTER_LE: + case LDAP_FILTER_SUBSTRINGS: + f_next = f_cur->f_next; + /* Cut it out */ + filter_prioritise_element(&(f->f_list), &f_op_head, &f_op_tail, &f_prev, &f_cur); + /* Don't change previous, because we remove this f_cur */ + f_cur = f_next; + break; + default: + /* Move along */ + f_prev = f_cur; + f_cur = f_cur->f_next; + break; + } + } + if (f_op_head != NULL) { + f_op_tail->f_next = f->f_list; + f->f_list = f_op_head; + } + } + /* finally optimize children */ + slapi_filter_optimise(f->f_list, 1); + + break; + default: - filter_optimize(f->f_next); + slapi_filter_optimise(f->f_next, 1); break; } + + if (slapi_is_loglevel_set(SLAPI_LOG_FILTER)) { + char fbuf_res[4096] = {0}; + slapi_filter_to_string(f, fbuf_res, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_optimise", "processed to %s\n", fbuf_res); + } } diff --git a/ldap/servers/slapd/filterentry.c b/ldap/servers/slapd/filterentry.c index 82eb196..b1b9054 100644 --- a/ldap/servers/slapd/filterentry.c +++ b/ldap/servers/slapd/filterentry.c @@ -98,7 +98,10 @@ slapi_filter_test_ext_internal( { int rc; - slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_test_ext_internal", "=>\n"); + char fbuf[4096] = {0}; + char *ndn = slapi_entry_get_dn(e); + slapi_filter_to_string(f, fbuf, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_test_ext_internal", "=> processing filt -> %s on %s\n", fbuf, ndn); /* * RJP: Not sure if this is semantically right, but we have to @@ -188,7 +191,8 @@ slapi_filter_test_ext_internal( rc = -1; } - slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_test_ext_internal", "<= %d\n", rc); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_test_ext_internal", "<= filt %d -> %s on %s\n", rc, fbuf, ndn); + // slapi_log_err(SLAPI_LOG_FILTER, "slapi_filter_test_ext_internal", "<= %d\n", rc); return (rc); } @@ -784,7 +788,11 @@ slapi_vattr_filter_test_ext_internal( { int rc = LDAP_SUCCESS; - slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "=>\n"); + char fbuf[4096] = {0}; + char *ndn = slapi_entry_get_dn(e); + slapi_filter_to_string(f, fbuf, 4095); + slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "=> processing filt -> %s on %s\n", fbuf, ndn); + // slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "=>\n"); /* * RJP: Not sure if this is semantically right, but we have to @@ -792,10 +800,10 @@ slapi_vattr_filter_test_ext_internal( * then we say that it did match and return 0. */ if (f == NULL) { + slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "<=\n"); return (0); } - slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "<=\n"); switch (f->f_choice) { case LDAP_FILTER_EQUALITY: @@ -806,7 +814,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; @@ -818,7 +826,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_SUBSTRING, f->f_sub_type); break; @@ -831,7 +839,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; @@ -844,7 +852,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; @@ -856,7 +864,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_PRES, f->f_type); break; @@ -869,7 +877,7 @@ slapi_vattr_filter_test_ext_internal( *access_check_done = 1; } if (only_check_access || rc != LDAP_SUCCESS) { - return (rc); + goto out; } rc = vattr_test_filter(pb, e, f, FILTER_TYPE_AVA, f->f_ava.ava_type); break; @@ -934,8 +942,10 @@ slapi_vattr_filter_test_ext_internal( rc = -1; } +out: + slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "<= filt %d -> %s on %s\n", rc, fbuf, ndn); + // slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "<= %d\n", rc); - slapi_log_err(SLAPI_LOG_FILTER, "slapi_vattr_filter_test_ext_internal", "<= %d\n", rc); return (rc); } diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c index 8169863..f355b82 100644 --- a/ldap/servers/slapd/libglobs.c +++ b/ldap/servers/slapd/libglobs.c @@ -242,6 +242,7 @@ slapi_onoff_t init_dynamic_plugins; slapi_onoff_t init_cn_uses_dn_syntax_in_dns; slapi_onoff_t init_global_backend_local; slapi_onoff_t init_enable_nunc_stans; +slapi_onoff_t init_filter_optimise_enable; #if defined(LINUX) slapi_int_t init_malloc_mxfast; slapi_int_t init_malloc_trim_threshold; @@ -1148,6 +1149,10 @@ static struct config_get_and_set NULL, 0, (void **)&global_slapdFrontendConfig.enable_nunc_stans, CONFIG_ON_OFF, (ConfigGetFunc)config_get_enable_nunc_stans, &init_enable_nunc_stans}, + {CONFIG_FILTER_OPTIMISE_ENABLE, config_set_filter_optimise_enable, + NULL, 0, + (void **)&global_slapdFrontendConfig.filter_optimise_enable, + CONFIG_ON_OFF, (ConfigGetFunc)config_get_filter_optimise_enable, &init_filter_optimise_enable}, /* Audit fail log configuration */ {CONFIG_AUDITFAILLOG_MODE_ATTRIBUTE, NULL, log_set_mode, SLAPD_AUDITFAIL_LOG, @@ -1744,6 +1749,7 @@ FrontendConfig_init(void) cfg->logging_backend = slapi_ch_strdup(SLAPD_INIT_LOGGING_BACKEND_INTERNAL); cfg->rootdn = slapi_ch_strdup(SLAPD_DEFAULT_DIRECTORY_MANAGER); init_enable_nunc_stans = cfg->enable_nunc_stans = LDAP_OFF; + init_filter_optimise_enable = cfg->filter_optimise_enable = LDAP_OFF; #if defined(LINUX) init_malloc_mxfast = cfg->malloc_mxfast = DEFAULT_MALLOC_UNSET; init_malloc_trim_threshold = cfg->malloc_trim_threshold = DEFAULT_MALLOC_UNSET; @@ -7589,6 +7595,30 @@ config_set_enable_nunc_stans(const char *attrname, char *value, char *errorbuf, return retVal; } +int32_t +config_get_filter_optimise_enable() +{ + int retVal; + slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); + CFG_LOCK_READ(slapdFrontendConfig); + retVal = slapdFrontendConfig->filter_optimise_enable; + CFG_UNLOCK_READ(slapdFrontendConfig); + + return retVal; +} + +int32_t +config_set_filter_optimise_enable(const char *attrname, char *value, char *errorbuf, int apply) +{ + int32_t retVal = LDAP_SUCCESS; + slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); + + retVal = config_set_onoff(attrname, value, + &(slapdFrontendConfig->filter_optimise_enable), + errorbuf, apply); + return retVal; +} + static char * config_initvalue_to_onoff(struct config_get_and_set *cgas, char *initvalbuf, size_t initvalbufsize) { diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c index bc18a7b..030e6ea 100644 --- a/ldap/servers/slapd/pblock.c +++ b/ldap/servers/slapd/pblock.c @@ -4331,6 +4331,18 @@ slapi_pblock_set_op_stack_elem(Slapi_PBlock *pb, void *stack_elem) pb->pb_intop->op_stack_elem = stack_elem; } +int32_t +slapi_pblock_get_filter_optimise_enable(Slapi_PBlock *pb) +{ + return pb->pb_config.filter_optimise_enable; +} + +void +slapi_pblock_set_filter_optimise_enable(Slapi_PBlock *pb, int32_t enable) +{ + pb->pb_config.filter_optimise_enable = enable; +} + /* * Clear and then set the bind DN and related credentials for the * connection `conn'. diff --git a/ldap/servers/slapd/pblock_v3.h b/ldap/servers/slapd/pblock_v3.h index 7ec2f37..bb7ea4c 100644 --- a/ldap/servers/slapd/pblock_v3.h +++ b/ldap/servers/slapd/pblock_v3.h @@ -198,6 +198,11 @@ typedef struct _slapi_pblock_deprecated int pb_config_lineno; } slapi_pblock_deprecated; +typedef struct _slapi_pblock_config +{ + int32_t filter_optimise_enable; +} slapi_pblock_config; + typedef struct slapi_pblock { /* common */ @@ -215,6 +220,15 @@ typedef struct slapi_pblock struct _slapi_pblock_intplugin *pb_intplugin; struct _slapi_pblock_deprecated *pb_deprecated; + /* + * copy-on-read config items - this is not as effective as COW + * on libglobs.c, but for the moment, it helps to avoid smashing atomics + * on operations, and we can at least, duplicate the config to the child pb. + * + * note that unlike the members above, this is NESTED on the pblock. + */ + struct _slapi_pblock_config pb_config; + #ifdef PBLOCK_ANALYTICS uint32_t analytics_init; PLHashTable *analytics; diff --git a/ldap/servers/slapd/plugin_internal_op.c b/ldap/servers/slapd/plugin_internal_op.c index 9da266b..61fd4a7 100644 --- a/ldap/servers/slapd/plugin_internal_op.c +++ b/ldap/servers/slapd/plugin_internal_op.c @@ -707,6 +707,9 @@ search_internal_callback_pb(Slapi_PBlock *pb, void *callback_data, plugin_result slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); slapi_pblock_set(pb, SLAPI_REQCONTROLS, controls); + /* Finally add our config values to the PB if they exist */ + slapi_pblock_set_filter_optimise_enable(pb, config_get_filter_optimise_enable()); + /* set actions taken to process the operation */ set_config_params(pb); diff --git a/ldap/servers/slapd/plugin_syntax.c b/ldap/servers/slapd/plugin_syntax.c index e208442..8963b5f 100644 --- a/ldap/servers/slapd/plugin_syntax.c +++ b/ldap/servers/slapd/plugin_syntax.c @@ -558,7 +558,7 @@ slapi_call_syntax_values2keys_sv( Slapi_PBlock *pipb = slapi_pblock_new(); struct slapdplugin *pi = vpi; - slapi_log_err(SLAPI_LOG_FILTER, "slapi_call_syntax_values2keys_sv", "=>\n"); + slapi_log_err(SLAPI_LOG_TRACE, "slapi_call_syntax_values2keys_sv", "=>\n"); slapi_pblock_set(pipb, SLAPI_PLUGIN, vpi); @@ -570,7 +570,7 @@ slapi_call_syntax_values2keys_sv( slapi_pblock_destroy(pipb); - slapi_log_err(SLAPI_LOG_FILTER, + slapi_log_err(SLAPI_LOG_TRACE, "slapi_call_syntax_values2keys_sv", "<= %d\n", rc); return (rc); } @@ -587,7 +587,7 @@ slapi_attr_values2keys_sv_pb( struct slapdplugin *pi = NULL; IFP v2k_fn = NULL; - slapi_log_err(SLAPI_LOG_FILTER, "slapi_attr_values2keys_sv_pb", "=>\n"); + slapi_log_err(SLAPI_LOG_TRACE, "slapi_attr_values2keys_sv_pb", "=>\n"); if ((sattr->a_plugin == NULL)) { /* could be lazy plugin initialization, get it now */ slapi_attr_init_syntax((Slapi_Attr *)sattr); @@ -629,7 +629,7 @@ slapi_attr_values2keys_sv_pb( } done: - slapi_log_err(SLAPI_LOG_FILTER, + slapi_log_err(SLAPI_LOG_TRACE, "slapi_attr_values2keys_sv_pb", "<= %d\n", rc); return (rc); } @@ -681,7 +681,7 @@ slapi_call_syntax_values2keys_sv_pb( int rc; struct slapdplugin *pi = vpi; - slapi_log_err(SLAPI_LOG_FILTER, "slapi_call_syntax_values2keys_sv_pb", "=>\n"); + slapi_log_err(SLAPI_LOG_TRACE, "slapi_call_syntax_values2keys_sv_pb", "=>\n"); slapi_pblock_set(pb, SLAPI_PLUGIN, vpi); @@ -691,7 +691,7 @@ slapi_call_syntax_values2keys_sv_pb( rc = pi->plg_syntax_values2keys(pb, vals, ivals, ftype); } - slapi_log_err(SLAPI_LOG_FILTER, + slapi_log_err(SLAPI_LOG_TRACE, "slapi_call_syntax_values2keys_sv_pb", "<= %d\n", rc); return (rc); } diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index b3167b8..33af89a 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -572,6 +572,8 @@ int config_set_cn_uses_dn_syntax_in_dns(const char *attrname, char *value, char int config_get_cn_uses_dn_syntax_in_dns(void); int config_get_enable_nunc_stans(void); int config_set_enable_nunc_stans(const char *attrname, char *value, char *errorbuf, int apply); +int32_t config_get_filter_optimise_enable(void); +int32_t config_set_filter_optimise_enable(const char *attrname, char *value, char *errorbuf, int apply); int config_set_extract_pem(const char *attrname, char *value, char *errorbuf, int apply); PLHashNumber hashNocaseString(const void *key); diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index a03ca43..f7f882e 100644 --- a/ldap/servers/slapd/result.c +++ b/ldap/servers/slapd/result.c @@ -2036,8 +2036,21 @@ log_result(Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentries notes_str, csn_str, pr_idx, pr_cookie); } } else if (!internal_op) { + /* + * Display result of a search. To aid debugging we print the filter we actually evaluated + * into the result. Basically this is to find bugs in filter optimisation not doing + * the right thing ... :( + */ char *pbtxt = NULL; char *ext_str = NULL; + char fbuf[4096] = {0}; + + Slapi_Filter *filter = NULL; + slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter); + if (filter != NULL) { + slapi_filter_to_string(filter, fbuf, 4095); + } + slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &pbtxt); if (pbtxt) { ext_str = slapi_ch_smprintf(" - %s", pbtxt); @@ -2046,12 +2059,13 @@ log_result(Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentries } slapi_log_access(LDAP_DEBUG_STATS, "conn=%" PRIu64 " op=%d RESULT err=%d" - " tag=%" BERTAG_T " nentries=%d etime=%s%s%s%s\n", + " tag=%" BERTAG_T " nentries=%d etime=%s%s%s%s filter=\"%s\"\n", op->o_connid, op->o_opid, err, tag, nentries, etime, - notes_str, csn_str, ext_str); + notes_str, csn_str, ext_str, + fbuf); if (pbtxt) { /* if !pbtxt ==> ext_str == "". Don't free ext_str. */ slapi_ch_free_string(&ext_str); diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c index 7e253f5..ad27d4c 100644 --- a/ldap/servers/slapd/search.c +++ b/ldap/servers/slapd/search.c @@ -348,6 +348,9 @@ do_search(Slapi_PBlock *pb) slapi_pblock_set(pb, SLAPI_SEARCH_SIZELIMIT, &sizelimit); slapi_pblock_set(pb, SLAPI_SEARCH_TIMELIMIT, &timelimit); + /* Finally add our config values to the PB if they exist */ + slapi_pblock_set_filter_optimise_enable(pb, config_get_filter_optimise_enable()); + op_shared_search(pb, psearch ? 0 : 1 /* send result */); slapi_pblock_get(pb, SLAPI_PLUGIN_OPRETURN, &rc); diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index b3ede6f..a96ab84 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -2189,6 +2189,7 @@ typedef struct _slapdEntryPoints #define CONFIG_MODDN_ACI_ATTRIBUTE "nsslapd-moddn-aci" #define CONFIG_GLOBAL_BACKEND_LOCK "nsslapd-global-backend-lock" #define CONFIG_ENABLE_NUNC_STANS "nsslapd-enable-nunc-stans" +#define CONFIG_FILTER_OPTIMISE_ENABLE "nsslapd-filter-optimise-enable" #define CONFIG_CONFIG_ATTRIBUTE "nsslapd-config" #define CONFIG_INSTDIR_ATTRIBUTE "nsslapd-instancedir" #define CONFIG_SCHEMADIR_ATTRIBUTE "nsslapd-schemadir" @@ -2516,6 +2517,7 @@ typedef struct _slapdFrontendConfig slapi_onoff_t global_backend_lock; slapi_int_t maxsimplepaged_per_conn; /* max simple paged results reqs handled per connection */ slapi_onoff_t enable_nunc_stans; + slapi_onoff_t filter_optimise_enable; #if defined(LINUX) int malloc_mxfast; /* mallopt M_MXFAST */ int malloc_trim_threshold; /* mallopt M_TRIM_THRESHOLD */ diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 4161002..9e9b7be 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -383,6 +383,7 @@ void ndn_cache_get_stats(uint64_t *hits, uint64_t *tries, uint64_t *size, uint64 int filter_flag_is_set(const Slapi_Filter *f, unsigned char flag); char *slapi_filter_to_string(const Slapi_Filter *f, char *buffer, size_t bufsize); char *slapi_filter_to_string_internal(const struct slapi_filter *f, char *buf, size_t *bufsize); +void slapi_filter_optimise(Slapi_Filter *f, int32_t enable); /* operation.c */ @@ -1442,6 +1443,11 @@ struct slapi_entry *slapi_pblock_get_pw_entry(Slapi_PBlock *pb); void slapi_pblock_set_pw_entry(Slapi_PBlock *pb, struct slapi_entry *entry); +int32_t slapi_pblock_get_filter_optimise_enable(Slapi_PBlock *pb); +void slapi_pblock_set_filter_optimise_enable(Slapi_PBlock *pb, int32_t enable); + + + #ifdef __cplusplus } #endif diff --git a/test/libslapd/filter/optimise.c b/test/libslapd/filter/optimise.c new file mode 100644 index 0000000..61b31c7 --- /dev/null +++ b/test/libslapd/filter/optimise.c @@ -0,0 +1,83 @@ +/** BEGIN COPYRIGHT BLOCK + * Copyright (C) 2017 Red Hat, Inc. + * All rights reserved. + * + * License: GPL (version 3 or any later version). + * See LICENSE for details. + * END COPYRIGHT BLOCK **/ + +#include "../../test_slapd.h" + +/* To access filter optimise */ +#include + +void +test_libslapd_filter_optimise(void **state __attribute__((unused))) +{ + char *test_filters[] = { + "(&(uid=uid1)(sn=last1)(givenname=first1))", + "(&(uid=uid1)(&(sn=last1)(givenname=first1)))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=first1))))", + "(&(uid=*)(sn=last3)(givenname=*))", + "(&(uid=*)(&(sn=last3)(givenname=*)))", + "(&(uid=uid5)(&(&(sn=*))(&(givenname=*))))", + "(&(objectclass=*)(uid=*)(sn=last*))", + "(&(objectclass=*)(uid=*)(sn=last1))", + + "(|(uid=uid1)(sn=last1)(givenname=first1))", + "(|(uid=uid1)(|(sn=last1)(givenname=first1)))", + "(|(uid=uid1)(|(|(sn=last1))(|(givenname=first1))))", + "(|(objectclass=*)(sn=last1)(|(givenname=first1)))", + "(|(&(objectclass=*)(sn=last1))(|(givenname=first1)))", + "(|(&(objectclass=*)(sn=last))(|(givenname=first1)))", + + "(&(uid=uid1)(!(cn=NULL)))", + "(&(!(cn=NULL))(uid=uid1))", + "(&(uid=*)(&(!(uid=1))(!(givenname=first1))))", + + "(&(|(uid=uid1)(uid=NULL))(sn=last1))", + "(&(|(uid=uid1)(uid=NULL))(!(sn=NULL)))", + "(&(|(uid=uid1)(sn=last2))(givenname=first1))", + "(|(&(uid=uid1)(!(uid=NULL)))(sn=last2))", + "(|(&(uid=uid1)(uid=NULL))(sn=last2))", + "(&(uid=uid5)(sn=*)(cn=*)(givenname=*)(uid=u*)(sn=la*)(cn=full*)(givenname=f*)(uid>=u)(!(givenname=NULL)))", + "(|(&(objectclass=*)(sn=last))(&(givenname=first1)))", + + "(&(uid=uid1)(sn=last1)(givenname=NULL))", + "(&(uid=uid1)(&(sn=last1)(givenname=NULL)))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL))))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL)(sn=*)))(|(sn=NULL)))", + "(&(uid=uid1)(&(&(sn=last*))(&(givenname=first*)))(&(sn=NULL)))", + + "(|(uid=NULL)(sn=NULL)(givenname=NULL))", + "(|(uid=NULL)(|(sn=NULL)(givenname=NULL)))", + "(|(uid=NULL)(|(|(sn=NULL))(|(givenname=NULL))))", + + "(uid>=uid3)", + "(&(uid=*)(uid>=uid3))", + "(|(uid>=uid3)(uid<=uid5))", + "(&(uid>=uid3)(uid<=uid5))", + "(|(&(uid>=uid3)(uid<=uid5))(uid=*))", + + "(|(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*))", + NULL + }; + + for (size_t i = 0; test_filters[i] != NULL; i++) { + char *filter_str = slapi_ch_strdup(test_filters[i]); + + struct slapi_filter *filter = slapi_str2filter(filter_str); + slapi_filter_optimise(filter, 1); + slapi_filter_free(filter, 1); + slapi_ch_free_string(&filter_str); + } +} + diff --git a/test/libslapd/test.c b/test/libslapd/test.c index ffa650d..02c6c03 100644 --- a/test/libslapd/test.c +++ b/test/libslapd/test.c @@ -28,6 +28,7 @@ run_libslapd_tests(void) cmocka_unit_test(test_libslapd_operation_v3c_target_spec), cmocka_unit_test(test_libslapd_counters_atomic_usage), cmocka_unit_test(test_libslapd_counters_atomic_overflow), + cmocka_unit_test(test_libslapd_filter_optimise), cmocka_unit_test(test_libslapd_pal_meminfo), cmocka_unit_test(test_libslapd_util_cachesane), }; diff --git a/test/test_slapd.h b/test/test_slapd.h index ad4f73f..efccaea 100644 --- a/test/test_slapd.h +++ b/test/test_slapd.h @@ -26,6 +26,9 @@ int run_plugin_tests(void); /* libslapd */ void test_libslapd_hello(void **state); +/* libslapd-filter-optimise */ +void test_libslapd_filter_optimise(void **state); + /* libslapd-pblock-analytics */ void test_libslapd_pblock_analytics(void **state); From fdebd1f075563ce585ab296f4b44ed605f6655e2 Mon Sep 17 00:00:00 2001 From: William Brown Date: Jun 11 2019 14:29:46 +0000 Subject: [PATCH 2/2] Fix up handling of filter with regard to aci --- diff --git a/dirsrvtests/tests/suites/filter/filter_test.py b/dirsrvtests/tests/suites/filter/filter_test.py index 3786603..634000c 100644 --- a/dirsrvtests/tests/suites/filter/filter_test.py +++ b/dirsrvtests/tests/suites/filter/filter_test.py @@ -123,7 +123,7 @@ def test_filter_scope_one(topology_st): """ log.info('Search user using ldapsearch with scope one') - results = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'ou=People',['ou'] ) + results = topology_st.standalone.search_s('ou=People,%s' % DEFAULT_SUFFIX, ldap.SCOPE_ONELEVEL,'ou=People',['ou']) log.info(results) log.info('Search should only have one entry') diff --git a/dirsrvtests/tests/suites/filter/ticket48275_test.py b/dirsrvtests/tests/suites/filter/ticket48275_test.py new file mode 100644 index 0000000..78f4231 --- /dev/null +++ b/dirsrvtests/tests/suites/filter/ticket48275_test.py @@ -0,0 +1,272 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2015 Red Hat, Inc. +# Copyright (C) 2019 William Brown +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import os +import sys +import time +import ldap +import logging +import pytest +from lib389 import DirSrv, Entry, tools +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from lib389.idm.domain import Domain + +from lib389.topologies import topology_st as topology + +log = logging.getLogger(__name__) + +installation_prefix = None + +USER_NUM = 10 +TEST_USER = 'user' + +OTHER_NAME = 'other_entry' +MAX_OTHERS = 10 + +BIND_NAME = 'bind_entry' +BIND_DN = 'cn=%s, %s' % (BIND_NAME, SUFFIX) +BIND_PW = 'password' + +ENTRY_NAME = 'test_entry' +ENTRY_DN = 'cn=%s, %s' % (ENTRY_NAME, SUFFIX) +ENTRY_OC = "top person" + +# subtrees used in test +SUBTREE_BASE = "ou=testbase,%s" %SUFFIX +SUBTREE_GREEN_1 = "ou=green_one,%s" % SUBTREE_BASE +SUBTREE_GREEN_2 = "ou=green_two,%s" % SUBTREE_BASE +SUBTREE_RED = "ou=red,%s" % SUBTREE_BASE +SUBTREES = (SUBTREE_GREEN_1, SUBTREE_GREEN_2, SUBTREE_RED) + +@pytest.fixture(scope="module") +def ticket48275_init(topology): + """ + It creates identical entries in 3 subtrees + It creates aci which allow access to a set of attrs + in two of these subtrees for bound users + It creates a user to be used for test + + """ + + + topology.standalone.log.info("Add subtree base: %s" % SUBTREE_BASE) + topology.standalone.add_s(Entry((SUBTREE_BASE, { + 'objectclass': "top organizationalunit".split(), + 'ou': "testbase"}))) + topology.standalone.log.info("Add subtree: %s" % SUBTREE_GREEN_1) + topology.standalone.add_s(Entry((SUBTREE_GREEN_1, { + 'objectclass': "top organizationalunit".split(), + 'ou': "green_one"}))) + topology.standalone.log.info("Add subtree: %s" % SUBTREE_GREEN_2) + topology.standalone.add_s(Entry((SUBTREE_GREEN_2, { + 'objectclass': "top organizationalunit".split(), + 'ou': "green_two"}))) + topology.standalone.log.info("Add subtree: %s" % SUBTREE_RED) + topology.standalone.add_s(Entry((SUBTREE_RED, { + 'objectclass': "top organizationalunit".split(), + 'ou': "red"}))) + + # entry used to bind with + topology.standalone.log.info("Add %s" % BIND_DN) + topology.standalone.add_s(Entry((BIND_DN, { + 'objectclass': "top person".split(), + 'sn': BIND_NAME, + 'cn': BIND_NAME, + 'userpassword': BIND_PW}))) + + # enable acl error logging + # mod = [(ldap.MOD_REPLACE, 'nsslapd-errorlog-level', '128')] + # topology.standalone.modify_s(DN_CONFIG, mod) + + # get rid of default ACIs + mod = [(ldap.MOD_DELETE, 'aci', None)] + topology.standalone.modify_s(SUFFIX, mod) + + # Ok Now add the proper ACIs + ACI_TARGET = "(target = \"ldap:///%s\")" % SUBTREE_GREEN_1 + ACI_TARGETATTR = "(targetattr = \"objectclass || cn || sn || uid || givenname\")" + ACI_ALLOW = "(version 3.0; acl \"Allow search-read to green attrs\"; allow (read, search, compare)" + ACI_SUBJECT = " userdn = \"ldap:///all\";)" + ACI_BODY = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT + # mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)] + # topology.standalone.modify_s(SUFFIX, mod) + domain = Domain(topology.standalone, DEFAULT_SUFFIX) + domain.add('aci', ACI_BODY) + + ACI_TARGET = "(target = \"ldap:///%s\")" % SUBTREE_GREEN_2 + ACI_TARGETATTR = "(targetattr != \"employeenumber || mobile || telephonenumber || mail\")" + ACI_BODY = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT + # mod = [(ldap.MOD_ADD, 'aci', ACI_BODY)] + # topology.standalone.modify_s(SUFFIX, mod) + domain.add('aci', ACI_BODY) + + log.info("Adding %d test entries..." % USER_NUM) + for id in range(USER_NUM): + name = "%s.%d" % (TEST_USER, id) + mail = "%s@example.com" % name + for subtree in SUBTREES: + topology.standalone.add_s(Entry(("cn=%s,%s" % (name, subtree), { + 'objectclass': "top person organizationalPerson inetOrgPerson".split(), + 'sn': name, + 'cn': name, + 'uid': name, + 'givenname': 'test', + 'mail': mail, + 'description': 'description', + 'employeenumber': "%d" % id, + 'telephonenumber': "%d%d%d" % (id,id,id), + 'mobile': "%d%d%d" % (id,id,id), + 'l': 'MV', + 'title': 'Engineer'}))) + + +def gen_filtertests_basic(dm): + tests = [] + # 1. Simple filter - green attr - not/matching + tests.append(("(uid=xxxx)",0)) + tests.append(("(uid=user.1)",(3 if dm else 2))) + + # 2. Simple filter -red attr - not/matching + tests.append(("(mail=user.x@example.com)",0)) + tests.append(("(mail=user.1@example.com)",(3 if dm else 0))) + + # 3. NOT filter -green attr - not/matching + tests.append(("(!(objectclass=inetorgperson))",(4 if dm else 2))) + tests.append(("(!(objectclass=dummy))",(34 if dm else 22))) + + # 4. NOT filter - red attr - not/matching + tests.append(("(!(mail=user.1@example.com))",(31 if dm else 0))) + tests.append(("(!(mail=user.x@example.com))",(34 if dm else 0))) + + # 5. AND filter - green/green attr - not/matching + tests.append(("(&(objectclass=inetorgperson)(uid=user.1))",(3 if dm else 2))) + tests.append(("(&(objectclass=inetorgperson)(uid=user.x))",0)) + + # 6. AND filter - green/red attr - not/matching + tests.append(("(&(objectclass=inetorgperson)(mail=user.1@example.com))",(3 if dm else 0))) + tests.append(("(&(objectclass=inetorgperson)(mail=user.x@example.com))",0)) + + # 7. OR filter - green/green - not/matching/order + tests.append(("(|(uid=user.1)(sn=user.3))",(6 if dm else 4))) + tests.append(("(|(uid=user.x)(sn=user.3))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(sn=user.x))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(mail=user.1@example.com))", (3 if dm else 2))) + + return tests + +def gen_filtertests_ext(dm): + ''' + the extended tests add OR components to the search filter using attributes + without access, either matching or not. + The returned entries have to be the same as without these OR parts + ''' + + tests = [] + + # 1. Simple filter - green attr - not/matching + tests.append(("(|(uid=xxxx)(mail=user.xxxx1@example.com))",0)) + tests.append(("(|(uid=xxxx)(mail=user.1@example.com))",(3 if dm else 0))) + tests.append(("(|(uid=user.1)(mail=user.xxxx1@example.com))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(mail=user.3@example.com))",(6 if dm else 2))) + + # 2. Simple filter - red attr - not/matching + tests.append(("(|(mail=user.x@example.com)(employeenumber=3))",(3 if dm else 0))) + tests.append(("(|(mail=user.x@example.com)(employeenumber=100))",0)) + tests.append(("(|(mail=user.1@example.com)(employeenumber=3))",(6 if dm else 0))) + tests.append(("(|(mail=user.1@example.com)(employeenumber=100))",(3 if dm else 0))) + + # 3. NOT filter - green attr - not/matching + tests.append(("(!(|(objectclass=inetorgperson)(employeenumber=3)))",(4 if dm else 2))) + tests.append(("(!(|(objectclass=inetorgperson)(employeenumber=100)))",(4 if dm else 2))) + tests.append(("(|(!(objectclass=inetorgperson))(employeenumber=3))",(7 if dm else 2))) + tests.append(("(|(!(objectclass=inetorgperson))(employeenumber=100))",(4 if dm else 2))) + + # 4. NOT filter - red attr - not/matching + tests.append(("(!(|(mail=user.1@example.com)(employeenumber=3)))",(28 if dm else 0))) + tests.append(("(!(|(mail=user.1@example.com)(employeenumber=100)))",(31 if dm else 0))) + tests.append(("(!(|(mail=user.x@example.com)(employeenumber=3)))",(31 if dm else 0))) + tests.append(("(!(|(mail=user.x@example.com)(employeenumber=100)))",(34 if dm else 0))) + + # 5. AND filter - green/green attr - not/matching + tests.append(("(|(&(objectclass=inetorgperson)(uid=user.1))(employeenumber=3))",(6 if dm else 2))) + tests.append(("(|(&(objectclass=inetorgperson)(uid=user.1))(employeenumber=100))",(3 if dm else 2))) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(uid=user.1)))",(6 if dm else 2))) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(uid=user.1)))",(3 if dm else 2))) + tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(uid=user.1))",(3 if dm else 2))) + tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(uid=user.1))",(3 if dm else 2))) + tests.append(("(|(&(objectclass=inetorgperson)(uid=user.x))(employeenumber=3))",(3 if dm else 0))) + tests.append(("(|(&(objectclass=inetorgperson)(uid=user.x))(employeenumber=100))", 0)) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(uid=user.x)))",(3 if dm else 0))) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(uid=user.x)))",0)) + tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(uid=user.x))",0)) + tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(uid=user.x))",0)) + + # 6. AND filter - green/red attr - not/matching + tests.append(("(&(|(employeenumber=3)(objectclass=inetorgperson))(mail=user.1@example.com))",(3 if dm else 0))) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=3)(mail=user.1@example.com)))",(6 if dm else 0))) + tests.append(("(|(employeenumber=3)(&(objectclass=inetorgperson)(mail=user.1@example.com)))",(6 if dm else 0))) + tests.append(("(&(|(employeenumber=100)(objectclass=inetorgperson))(mail=user.1@example.com))",(3 if dm else 0))) + tests.append(("(&(objectclass=inetorgperson)(|(employeenumber=100)(mail=user.1@example.com)))",(3 if dm else 0))) + tests.append(("(|(employeenumber=100)(&(objectclass=inetorgperson)(mail=user.1@example.com)))",(3 if dm else 0))) + tests.append(("(&(objectclass=inetorgperson)(mail=user.x@example.com))",0)) + + # 7. OR filter - green/green - not/matching/order + tests.append(("(|(uid=user.1)(employeenumber=3)(sn=user.3))",(6 if dm else 4))) + tests.append(("(|(uid=user.1)(sn=user.3)(employeenumber=3))",(6 if dm else 4))) + tests.append(("(|(uid=user.x)(employeenumber=3)(sn=user.3))",(3 if dm else 2))) + tests.append(("(|(uid=user.x)(sn=user.3)(employeenumber=3))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(employeenumber=3)(sn=user.x))",(6 if dm else 2))) + tests.append(("(|(uid=user.1)(sn=user.x)(employeenumber=3))",(6 if dm else 2))) + tests.append(("(|(uid=user.1)(unknown=3)(sn=user.3))",(6 if dm else 4))) + tests.append(("(|(uid=user.1)(sn=user.3)(unknown=3))",(6 if dm else 4))) + tests.append(("(|(uid=user.x)(unknown=3)(sn=user.3))",(3 if dm else 2))) + tests.append(("(|(uid=user.x)(sn=user.3)(unknown=3))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(unknown=3)(sn=user.x))",(3 if dm else 2))) + tests.append(("(|(uid=user.1)(sn=user.x)(unknown=3))",(3 if dm else 2))) + + return tests + +def run_ticket48275_search(topology, dm, basic): + + if basic: + tests = gen_filtertests_basic(dm) + else: + tests = gen_filtertests_ext(dm) + + topology.standalone.log.info("\n\n######################### SEARCH ######################\n") + + for (fstr, exp) in tests: + ents = topology.standalone.search_s(SUBTREE_BASE, ldap.SCOPE_SUBTREE, fstr) + rec = len(ents) + topology.standalone.log.info("Test: filter \"%s\", expected: %d, received: %d\n" % (fstr, exp, rec)) + assert(exp == rec) + +def test_ticket48275_search_basic_dm(topology, ticket48275_init): + + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + run_ticket48275_search(topology, dm=True, basic=True) + +def test_ticket48275_search_basic_user(topology, ticket48275_init): + + topology.standalone.simple_bind_s(BIND_DN, BIND_PW) + run_ticket48275_search(topology, dm=False, basic=True) + +def test_ticket48275_search_ext_dm(topology, ticket48275_init): + + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + run_ticket48275_search(topology, dm=True, basic=False) + +def test_ticket48275_search_ext_user(topology, ticket48275_init): + + topology.standalone.simple_bind_s(BIND_DN, BIND_PW) + run_ticket48275_search(topology, dm=False, basic=False) + + diff --git a/dirsrvtests/tests/suites/filter/ticket49617_test.py b/dirsrvtests/tests/suites/filter/ticket49617_test.py new file mode 100644 index 0000000..c7b8787 --- /dev/null +++ b/dirsrvtests/tests/suites/filter/ticket49617_test.py @@ -0,0 +1,63 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2015 Red Hat, Inc. +# Copyright (C) 2019 William Brown +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# + +import logging +import pytest +import os +import ldap +import time +from lib389 import Entry +from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES +from lib389._constants import * +from lib389.topologies import topology_st as topo + +TARGETATTR = "(targetattr = \"cn || createtimestamp || description || displayname || entryusn || gecos || gidnumber || givenname || homedirectory || initials || loginshell || manager || modifytimestamp || objectclass || sn || title || uid \")" +TARGETFILTER = "(targetfilter = \"(objectclass=posixaccount)\")" +RIGHTS = "(version 3.0;acl \"Read User Standard Attributes\";allow (compare,read,search) userdn = \"ldap:///anyone\";)" +ACI = TARGETATTR + TARGETFILTER + RIGHTS + +PASSWD = 'password' +def test_ticket49617(topo): + """Specify a test case purpose or name here + + :id: 0 + :setup: Standalone Instance + :steps: + 1. Fill in test case steps here + 2. And indent them like this (RST format requirement) + :expectedresults: + 1. Fill in the result that is expected + 2. For each test step + """ + + S = topo.standalone + + PEOPLE = "ou=People,%s" % SUFFIX + # Purge all system default aci + S.modify_s(SUFFIX, [(ldap.MOD_DELETE, 'aci', None)]) + # Add the test ACI + S.modify_s(PEOPLE, [(ldap.MOD_REPLACE, 'aci', ACI.encode())]) + + uid = b'my_user' + users = UserAccounts(S, PEOPLE, rdn=None) + user_props = TEST_USER_PROPERTIES.copy() + user_props.update({'uid': uid, 'cn': uid, 'sn': '_%s' % uid, 'userpassword': PASSWD.encode(), 'description': b'value creation'}) + test_user = users.create(properties=user_props) + + Filter = "(uid=*)" + + S.bind(test_user.dn, PASSWD) + ents = S.search_s(PEOPLE, ldap.SCOPE_SUBTREE, Filter) + # This is 2 because there is a demo user too now! + assert len(ents) == 2 + + ents = S.search_s(PEOPLE, ldap.SCOPE_ONELEVEL, Filter) + assert len(ents) == 2 + diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif index b6c8fc7..bb12b0a 100644 --- a/ldap/schema/01core389.ldif +++ b/ldap/schema/01core389.ldif @@ -312,7 +312,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2341 NAME 'nsslapd-changelogmaxentries' attributeTypes: ( 2.16.840.1.113730.3.1.2344 NAME 'nsslapd-tls-check-crl' DESC 'Check CRL when opening outbound TLS connections. Valid options are none, peer, all.' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2353 NAME 'nsslapd-encryptionalgorithm' DESC 'The encryption algorithm used to encrypt the changelog' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2084 NAME 'nsSymmetricKey' DESC 'A symmetric key - currently used by attribute encryption' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN 'attribute encryption' ) -attributeTypes: ( 2.16.840.1.113730.3.1.2360 NAME 'nsslapd-filter-optimise-enable' DESC 'Enable or disable the query optimiser within the server' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2364 NAME 'nsslapd-filter-optimise-enable' DESC 'Enable or disable the query optimiser within the server' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN '389 Directory Server' ) # # objectclasses # diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index da053e2..efa19cd 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -940,27 +940,30 @@ build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const c struct ldbminfo *li = (struct ldbminfo *)be->be_database->plg_private; int managedsait = 0; Slapi_Filter *filter = NULL; + Slapi_Filter *filter_original = NULL; int err = 0; int r = 0; int32_t filter_optimise = 0; - slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter); - if (NULL == filter) { + slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter_original); + + if (NULL == filter_original) { slapi_send_ldap_result(pb, LDAP_PROTOCOL_ERROR, NULL, "No filter", 0, NULL); r = SLAPI_FAIL_GENERAL; goto bail; } - slapi_pblock_get(pb, SLAPI_MANAGEDSAIT, &managedsait); - filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); - switch (scope) { case LDAP_SCOPE_BASE: *candidates = base_candidates(pb, e); break; case LDAP_SCOPE_ONELEVEL: + /* Duplicate so we don't tamper with the original. */ + filter = slapi_filter_dup(filter_original); + filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); + /* modify the filter to be: (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */ filter = create_onelevel_filter(filter, e, managedsait); /* Now optimise the filter for use */ @@ -968,10 +971,13 @@ build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const c *candidates = onelevel_candidates(pb, be, base, filter, lookup_returned_allidsp, &err); /* Give the optimised filter back to search filter for free */ - slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); + slapi_pblock_set_filter_executed(pb, filter); break; case LDAP_SCOPE_SUBTREE: + /* Duplicate so we don't tamper with the original. */ + filter = slapi_filter_dup(filter_original); + filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); /* make (|(originalfilter)(objectclass=referral)) */ filter = create_subtree_filter(filter, managedsait); /* Now optimise the filter for use */ @@ -979,7 +985,7 @@ build_candidate_list(Slapi_PBlock *pb, backend *be, struct backentry *e, const c *candidates = subtree_candidates(pb, be, base, e, filter, lookup_returned_allidsp, &err); /* Give the optimised filter back to search filter for free */ - slapi_pblock_set(pb, SLAPI_SEARCH_FILTER, filter); + slapi_pblock_set_filter_executed(pb, filter); break; default: diff --git a/ldap/servers/slapd/back-ldbm/vlv_srch.c b/ldap/servers/slapd/back-ldbm/vlv_srch.c index fc9bb04..31e81e3 100644 --- a/ldap/servers/slapd/back-ldbm/vlv_srch.c +++ b/ldap/servers/slapd/back-ldbm/vlv_srch.c @@ -177,7 +177,7 @@ vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm /* make (&(parentid=idofbase)(|(originalfilter)(objectclass=referral))) */ { p->vlv_slapifilter = create_onelevel_filter(p->vlv_slapifilter, e, 0 /* managedsait */); - slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); + /* slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); */ CACHE_RETURN(&inst->inst_cache, &e); } } break; @@ -185,7 +185,7 @@ vlvSearch_init(struct vlvSearch *p, Slapi_PBlock *pb, const Slapi_Entry *e, ldbm /* make (|(originalfilter)(objectclass=referral))) */ /* No need for scope-filter since we apply a scope test before the filter test */ p->vlv_slapifilter = create_subtree_filter(p->vlv_slapifilter, 0 /* managedsait */); - slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); + /* slapi_filter_optimise(p->vlv_slapifilter, filter_optimise); */ } break; } } diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c index 9a9b3ab..a959e48 100644 --- a/ldap/servers/slapd/dse.c +++ b/ldap/servers/slapd/dse.c @@ -1608,7 +1608,8 @@ int dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from this function! */ { int scope; /*Scope of the search*/ - Slapi_Filter *filter; /*The filter*/ + Slapi_Filter *filter = NULL; /*The filter as we execute it*/ + Slapi_Filter *filter_original = NULL; /*The original filter*/ char **attrs; /*Attributes*/ int attrsonly; /*Should we just return the attributes found?*/ /*int nentries= 0; Number of entries found thus far*/ @@ -1628,7 +1629,7 @@ dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi if (slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &pdse) < 0 || slapi_pblock_get(pb, SLAPI_SEARCH_TARGET_SDN, &basesdn) < 0 || slapi_pblock_get(pb, SLAPI_SEARCH_SCOPE, &scope) < 0 || - slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter) < 0 || + slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter_original) < 0 || slapi_pblock_get(pb, SLAPI_SEARCH_ATTRS, &attrs) < 0 || slapi_pblock_get(pb, SLAPI_SEARCH_ATTRSONLY, &attrsonly) < 0) { slapi_send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL); @@ -1641,12 +1642,13 @@ dse_search(Slapi_PBlock *pb) /* JCM There should only be one exit point from thi isrootdse = slapi_sdn_isempty(basesdn); /* - * Now optimise the filter for use: note that unlike ldbm_search, - * because we don't change the outer filter container, we don't need - * to set back into pb. + * Now optimise the filter for use: we then set it back to the pb + * for later free operations. */ filter_optimise = slapi_pblock_get_filter_optimise_enable(pb); + filter = slapi_filter_dup(filter_original); slapi_filter_optimise(filter, filter_optimise); + slapi_pblock_set_filter_executed(pb, filter); switch (scope) { case LDAP_SCOPE_BASE: { diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c index 030e6ea..0e2b3d4 100644 --- a/ldap/servers/slapd/pblock.c +++ b/ldap/servers/slapd/pblock.c @@ -214,6 +214,8 @@ pblock_done(Slapi_PBlock *pb) slapi_ch_free((void **)&(pb->pb_deprecated)); slapi_ch_free((void **)&(pb->pb_misc)); if (pb->pb_intop != NULL) { + /* Free extra parts ... */ + slapi_filter_free(pb->pb_intop->filter_executed, 1); delete_passwdPolicy(&pb->pb_intop->pwdpolicy); slapi_ch_free((void **)&(pb->pb_intop->pb_result_text)); } @@ -4343,6 +4345,22 @@ slapi_pblock_set_filter_optimise_enable(Slapi_PBlock *pb, int32_t enable) pb->pb_config.filter_optimise_enable = enable; } +void +slapi_pblock_set_filter_executed(Slapi_PBlock *pb, Slapi_Filter *filter) +{ + _pblock_assert_pb_intop(pb); + pb->pb_intop->filter_executed = filter; +} + +Slapi_Filter * +slapi_pblock_get_filter_executed(Slapi_PBlock *pb) +{ + if (pb->pb_intop != NULL) { + return pb->pb_intop->filter_executed; + } + return NULL; +} + /* * Clear and then set the bind DN and related credentials for the * connection `conn'. diff --git a/ldap/servers/slapd/pblock_v3.h b/ldap/servers/slapd/pblock_v3.h index bb7ea4c..3a1a2a7 100644 --- a/ldap/servers/slapd/pblock_v3.h +++ b/ldap/servers/slapd/pblock_v3.h @@ -161,6 +161,13 @@ typedef struct _slapi_pblock_intop int pb_paged_results_index; /* stash SLAPI_PAGED_RESULTS_INDEX */ int pb_paged_results_cookie; /* stash SLAPI_PAGED_RESULTS_COOKIE */ + /* + * The pb_op contains the filter as "requested" but after an operation + * we should store and track the filter as we executed it for auditing + * and analysis. It's stored here. + */ + Slapi_Filter *filter_executed; + } slapi_pblock_intop; /* Stuff that is rarely used, but still present */ @@ -209,6 +216,7 @@ typedef struct slapi_pblock Slapi_Backend *pb_backend; Connection *pb_conn; Operation *pb_op; + struct slapdplugin *pb_plugin; /* plugin being called */ /* Private tree of fields for our operations, grouped by usage patterns */ diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index f7f882e..201e815 100644 --- a/ldap/servers/slapd/result.c +++ b/ldap/servers/slapd/result.c @@ -2045,10 +2045,11 @@ log_result(Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentries char *ext_str = NULL; char fbuf[4096] = {0}; - Slapi_Filter *filter = NULL; - slapi_pblock_get(pb, SLAPI_SEARCH_FILTER, &filter); - if (filter != NULL) { - slapi_filter_to_string(filter, fbuf, 4095); + if (op->o_tag == LDAP_REQ_SEARCH) { + Slapi_Filter *filter = slapi_pblock_get_filter_executed(pb); + if (filter != NULL) { + slapi_filter_to_string(filter, fbuf, 4095); + } } slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &pbtxt); diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 9e9b7be..229758b 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -1446,6 +1446,9 @@ void slapi_pblock_set_pw_entry(Slapi_PBlock *pb, struct slapi_entry *entry); int32_t slapi_pblock_get_filter_optimise_enable(Slapi_PBlock *pb); void slapi_pblock_set_filter_optimise_enable(Slapi_PBlock *pb, int32_t enable); +void slapi_pblock_set_filter_executed(Slapi_PBlock *pb, Slapi_Filter *filter); +Slapi_Filter * slapi_pblock_get_filter_executed(Slapi_PBlock *pb); + #ifdef __cplusplus diff --git a/src/lib389/lib389/instance/setup.py b/src/lib389/lib389/instance/setup.py index 347eba9..e877e9d 100644 --- a/src/lib389/lib389/instance/setup.py +++ b/src/lib389/lib389/instance/setup.py @@ -40,6 +40,8 @@ from lib389.utils import ( ds_paths = Paths() +DEBUGSETUP = os.getenv('DEBUGSETUP', default=False) + def get_port(port, default_port, secure=False): # Get the port number for the interactive installer and validate it @@ -860,6 +862,10 @@ class SetupDs(object): # Start the server # Make changes using the temp root ds_instance.start(timeout=60) + if DEBUGSETUP: + import time + self.log.info("Waiting 30 seconds to allow GDB attach ...") + time.sleep(30) ds_instance.open() # In some cases we may want to change log settings