Blob Blame Raw
From 353955ba9b4c487e30315d39d1880b6b784817d2 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 27 Mar 2017 10:59:40 -0400
Subject: [PATCH] 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(-)
 create mode 100644 dirsrvtests/tests/tickets/ticket49192_test.py

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;
 }
 
-- 
2.9.3