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