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