From b30295a7b2e3d7cae47348a1df2f32adc5be7397 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Feb 26 2019 14:21:34 +0000 Subject: Ticket 50238 - Failed modrdn can corrupt entry cache Bug Description: Under certain conditions (found under IPA) when a backend transaction plugin fails and causes a modrdn operation to fail the entry cache no longer contains the original/pre entry, but instead it has the post modrdn'ed entry with the original entry's ID Fix Description: Upon failure, if the post entry is in the cache, then swap it out with the original entry. https://pagure.io/389-ds-base/issue/50238 Reviewed by: firstyear, spichugi, & tboardaz (Thanks!!!) --- diff --git a/dirsrvtests/tests/suites/betxns/betxn_test.py b/dirsrvtests/tests/suites/betxns/betxn_test.py index 9a6e835..3b81434 100644 --- a/dirsrvtests/tests/suites/betxns/betxn_test.py +++ b/dirsrvtests/tests/suites/betxns/betxn_test.py @@ -8,6 +8,7 @@ # import pytest import six +import ldap from lib389.tasks import * from lib389.utils import * from lib389.topologies import topology_st @@ -156,10 +157,6 @@ def test_betxn_memberof(topology_st): 5. Add operation should FAIL """ - ENTRY1_DN = 'cn=group1,' + DEFAULT_SUFFIX - ENTRY2_DN = 'cn=group2,' + DEFAULT_SUFFIX - PLUGIN_DN = 'cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config' - memberof = MemberOfPlugin(topology_st.standalone) memberof.enable() memberof.set_autoaddoc('referral') @@ -168,11 +165,11 @@ def test_betxn_memberof(topology_st): groups = Groups(topology_st.standalone, DEFAULT_SUFFIX) group1 = groups.create(properties={ - 'cn' : 'group1', + 'cn': 'group1', }) group2 = groups.create(properties={ - 'cn' : 'group2', + 'cn': 'group2', }) # We may need to mod groups to not have nsMemberOf ... ? @@ -183,11 +180,10 @@ def test_betxn_memberof(topology_st): # Add group2 to group1 - it should fail with objectclass violation try: group1.add_member(group2.dn) - topology_st.standalone.modify_s(ENTRY1_DN, [(ldap.MOD_ADD, 'member', ENTRY2_DN)]) log.fatal('test_betxn_memberof: Group2 was incorrectly allowed to be added to group1') assert False except ldap.LDAPError as e: - log.info('test_betxn_memberof: Group2 was correctly rejected (mod add): error') + log.info('test_betxn_memberof: Group2 was correctly rejected (mod add): error: ' + str(e)) # # Done @@ -195,6 +191,62 @@ def test_betxn_memberof(topology_st): log.info('test_betxn_memberof: PASSED') +def test_betxn_modrdn_memberof(topology_st): + """Test modrdn operartions and memberOf + + :id: 70d0b96e-b693-4bf7-bbf5-102a66ac5994 + + :setup: Standalone instance + + :steps: 1. Enable and configure memberOf plugin + 2. Set memberofgroupattr="member" and memberofAutoAddOC="nsContainer" + 3. Create group and user outside of memberOf plugin scope + 4. Do modrdn to move group into scope + 5. Do modrdn to move group into scope (again) + + :expectedresults: + 1. memberOf plugin plugin should be ON + 2. Set memberofgroupattr="member" and memberofAutoAddOC="nsContainer" should PASS + 3. Creating group and user should PASS + 4. Modrdn should fail with objectclass violation + 5. Second modrdn should also fail with objectclass violation + """ + + peoplebase = 'ou=people,%s' % DEFAULT_SUFFIX + memberof = MemberOfPlugin(topology_st.standalone) + memberof.enable() + memberof.set_autoaddoc('nsContainer') # Bad OC + memberof.set('memberOfEntryScope', peoplebase) + memberof.set('memberOfAllBackends', 'on') + topology_st.standalone.restart() + + groups = Groups(topology_st.standalone, DEFAULT_SUFFIX) + group = groups.create(properties={ + 'cn': 'group', + }) + + # Create user and add it to group + users = UserAccounts(topology_st.standalone, basedn=DEFAULT_SUFFIX) + user = users.create(properties=TEST_USER_PROPERTIES) + if not ds_is_older('1.3.7'): + user.remove('objectClass', 'nsMemberOf') + + group.add_member(user.dn) + + # Attempt modrdn that should fail, but the original entry should stay in the cache + with pytest.raises(ldap.OBJECTCLASS_VIOLATION): + group.rename('cn=group_to_people', newsuperior=peoplebase) + + # Should fail, but not with NO_SUCH_OBJECT as the original entry should still be in the cache + with pytest.raises(ldap.OBJECTCLASS_VIOLATION): + group.rename('cn=group_to_people', newsuperior=peoplebase) + + # + # Done + # + log.info('test_betxn_modrdn_memberof: PASSED') + + if __name__ == '__main__': # Run isolated # -s for DEBUG mode diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 684b040..e4d0337 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1411,14 +1411,20 @@ common_return: "operation failed, the target entry is cleared from dncache (%s)\n", slapi_entry_get_dn(ec->ep_entry)); CACHE_REMOVE(&inst->inst_dncache, bdn); CACHE_RETURN(&inst->inst_dncache, &bdn); + /* + * If the new/invalid entry (ec) is in the cache, that means we need to + * swap it out with the original entry (e) --> to undo the swap that + * modrdn_rename_entry_update_indexes() did. + */ + if (cache_is_in_cache(&inst->inst_cache, ec)) { + if (cache_replace(&inst->inst_cache, ec, e) != 0) { + slapi_log_err(SLAPI_LOG_ALERT, "ldbm_back_modrdn", + "failed to replace cache entry after error\n"); + } + } } - /* remove the new entry from the cache if the op failed - - otherwise, leave it in */ if (ec && inst) { - if (retval && cache_is_in_cache(&inst->inst_cache, ec)) { - CACHE_REMOVE(&inst->inst_cache, ec); - } CACHE_RETURN(&inst->inst_cache, &ec); } ec = NULL;