[389-commits] [389-ds-base] 18/32: Issue 49192 - Deleting suffix can hang server

This is an automated email from the git hooks/post-receive script. mreynolds pushed a commit to branch rhel-7.4 in repository 389-ds-base. commit 353955ba9b4c487e30315d39d1880b6b784817d2 Author: Mark Reynolds <mreynolds(a)redhat.com&gt; Date: Mon Mar 27 10:59:40 2017 -0400 Issue 49192 - Deleting suffix can hang server Description: If you attempt to bind as an inactive user the backend rwlock is not unlocked. Regression introduced from issue 49051. https://pagure.io/389-ds-base/issue/49192 Reviewed by: nhosoi(Thanks!) --- dirsrvtests/tests/tickets/ticket49192_test.py | 177 ++++++++++++++++++++++++++ ldap/servers/slapd/bind.c | 3 - ldap/servers/slapd/pw_verify.c | 8 +- 3 files changed, 179 insertions(+), 9 deletions(-) diff --git a/dirsrvtests/tests/tickets/ticket49192_test.py b/dirsrvtests/tests/tickets/ticket49192_test.py new file mode 100644 index 0000000..f770ba7 --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket49192_test.py @@ -0,0 +1,177 @@ +import time +import ldap +import logging +import pytest +from lib389 import Entry +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from lib389.utils import * +from lib389.topologies import topology_st as topo + +DEBUGGING = os.getenv("DEBUGGING", default=False) +if DEBUGGING: + logging.getLogger(__name__).setLevel(logging.DEBUG) +else: + logging.getLogger(__name__).setLevel(logging.INFO) +log = logging.getLogger(__name__) + +INDEX_DN = 'cn=index,cn=Second_Backend,cn=ldbm database,cn=plugins,cn=config' +SUFFIX_DN = 'cn=Second_Backend,cn=ldbm database,cn=plugins,cn=config' +MY_SUFFIX = "o=hang.com" +USER_DN = 'uid=user,' + MY_SUFFIX + + +def test_ticket49192(topo): + """Trigger deadlock when removing suffix + """ + + # + # Create a second suffix/backend + # + log.info('Creating second backend...') + topo.standalone.backends.create(None, properties={ + BACKEND_NAME: "Second_Backend", + 'suffix': "o=hang.com", + }) + try: + topo.standalone.add_s(Entry(("o=hang.com", { + 'objectclass': 'top organization'.split(), + 'o': 'hang.com'}))) + except ldap.LDAPError as e: + log.fatal('Failed to create 2nd suffix: error ' + e.message['desc']) + assert False + + # + # Add roles + # + log.info('Adding roles...') + try: + topo.standalone.add_s(Entry(('cn=nsManagedDisabledRole,' + MY_SUFFIX, { + 'objectclass': ['top', 'LdapSubEntry', + 'nsRoleDefinition', + 'nsSimpleRoleDefinition', + 'nsManagedRoleDefinition'], + 'cn': 'nsManagedDisabledRole'}))) + except ldap.LDAPError as e: + log.fatal('Failed to add managed role: error ' + e.message['desc']) + assert False + + try: + topo.standalone.add_s(Entry(('cn=nsDisabledRole,' + MY_SUFFIX, { + 'objectclass': ['top', 'LdapSubEntry', + 'nsRoleDefinition', + 'nsComplexRoleDefinition', + 'nsNestedRoleDefinition'], + 'cn': 'nsDisabledRole', + 'nsRoledn': 'cn=nsManagedDisabledRole,' + MY_SUFFIX}))) + except ldap.LDAPError as e: + log.fatal('Failed to add nested role: error ' + e.message['desc']) + assert False + + try: + topo.standalone.add_s(Entry(('cn=nsAccountInactivationTmp,' + MY_SUFFIX, { + 'objectclass': ['top', 'nsContainer'], + 'cn': 'nsAccountInactivationTmp'}))) + except ldap.LDAPError as e: + log.fatal('Failed to add container: error ' + e.message['desc']) + assert False + + try: + topo.standalone.add_s(Entry(('cn=\"cn=nsDisabledRole,' + MY_SUFFIX + '\",cn=nsAccountInactivationTmp,' + MY_SUFFIX, { + 'objectclass': ['top', 'extensibleObject', 'costemplate', + 'ldapsubentry'], + 'nsAccountLock': 'true'}))) + except ldap.LDAPError as e: + log.fatal('Failed to add cos1: error ' + e.message['desc']) + assert False + + try: + topo.standalone.add_s(Entry(('cn=nsAccountInactivation_cos,' + MY_SUFFIX, { + 'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition', + 'cosClassicDefinition'], + 'cn': 'nsAccountInactivation_cos', + 'cosTemplateDn': 'cn=nsAccountInactivationTmp,' + MY_SUFFIX, + 'cosSpecifier': 'nsRole', + 'cosAttribute': 'nsAccountLock operational'}))) + except ldap.LDAPError as e: + log.fatal('Failed to add cos2 : error ' + e.message['desc']) + assert False + + # + # Add test entry + # + try: + topo.standalone.add_s(Entry((USER_DN, { + 'objectclass': 'top extensibleObject'.split(), + 'uid': 'user', + 'userpassword': 'password', + }))) + except ldap.LDAPError as e: + log.fatal('Failed to add user: error ' + e.message['desc']) + assert False + + # + # Inactivate the user account + # + try: + topo.standalone.modify_s(USER_DN, + [(ldap.MOD_ADD, + 'nsRoleDN', + 'cn=nsManagedDisabledRole,' + MY_SUFFIX)]) + except ldap.LDAPError as e: + log.fatal('Failed to disable user: error ' + e.message['desc']) + assert False + + time.sleep(1) + + # Bind as user (should fail) + try: + topo.standalone.simple_bind_s(USER_DN, 'password') + log.error("Bind incorrectly worked") + assert False + except ldap.UNWILLING_TO_PERFORM: + log.info('Got error 53 as expected') + except ldap.LDAPError as e: + log.fatal('Bind has unexpected error ' + e.message['desc']) + assert False + + # Bind as root DN + try: + topo.standalone.simple_bind_s(DN_DM, PASSWORD) + except ldap.LDAPError as e: + log.fatal('RootDN Bind has unexpected error ' + e.message['desc']) + assert False + + # + # Delete suffix + # + log.info('Delete the suffix and children...') + try: + index_entries = topo.standalone.search_s( + SUFFIX_DN, ldap.SCOPE_SUBTREE, 'objectclass=top') + except ldap.LDAPError as e: + log.error('Failed to search: %s - error %s' % (SUFFIX_DN, str(e))) + + for entry in reversed(index_entries): + try: + log.info("Deleting: " + entry.dn) + if entry.dn != SUFFIX_DN and entry.dn != INDEX_DN: + topo.standalone.search_s(entry.dn, + ldap.SCOPE_ONELEVEL, + 'objectclass=top') + topo.standalone.delete_s(entry.dn) + except ldap.LDAPError as e: + log.fatal('Failed to delete entry: %s - error %s' % + (entry.dn, str(e))) + assert False + + log.info("Test Passed") + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE) + diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c index 5c4fada..f83df7d 100644 --- a/ldap/servers/slapd/bind.c +++ b/ldap/servers/slapd/bind.c @@ -771,9 +771,6 @@ do_bind( Slapi_PBlock *pb ) /* need_new_pw failed; need_new_pw already send_ldap_result in it. */ goto free_and_return; } - if (be) { - slapi_be_Unlock(be); - } } else { /* anonymous */ /* set bind creds here so anonymous limits are set */ bind_credentials_set(pb->pb_conn, authtype, NULL, NULL, NULL, NULL, NULL); diff --git a/ldap/servers/slapd/pw_verify.c b/ldap/servers/slapd/pw_verify.c index a9fd9ec..852b027 100644 --- a/ldap/servers/slapd/pw_verify.c +++ b/ldap/servers/slapd/pw_verify.c @@ -50,8 +50,6 @@ pw_verify_root_dn(const char *dn, const Slapi_Value *cred) * * In the future, this will use the credentials and do mfa. * - * If you get SLAPI_BIND_SUCCESS or SLAPI_BIND_ANONYMOUS you need to unlock - * the backend. * All other results, it's already released. */ int @@ -81,10 +79,8 @@ pw_verify_be_dn(Slapi_PBlock *pb, Slapi_Entry **referral) set_db_default_result_handlers(pb); /* now take the dn, and check it */ rc = (*be->be_bind)(pb); - /* now attempt the bind. */ - if (rc != SLAPI_BIND_SUCCESS && rc != SLAPI_BIND_ANONYMOUS) { - slapi_be_Unlock(be); - } + slapi_be_Unlock(be); + return rc; } -- To stop receiving notification emails like this one, please contact the administrator of this repository.