From ab4af68ef49fcdc5f2f6d0c1f5c7b9a5333b1bee Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Oct 25 2018 09:29:10 +0000 Subject: Ticket 49967 - entry cache corruption after failed MODRDN Bug Description: During a MODRDN the DN cache is updated to replace source DN with the target DN (modrdn_rename_entry_update_indexes) If later a failure occurs (for example if BETXN_POSTOP fails) and the txn is aborted, the target DN (for the specific entryID) remains in the DN cache. If the entry is returned in a search, to build the DN there is a lookup of the DN cache with the entryID. It retrieves the target DN rather than the source DN Fix Description: In case of failure of the operation, the entry (from the entryID) need to be cleared from the DN cache https://pagure.io/389-ds-base/issue/49967 Reviewed by: Mark Reynolds Platforms tested: F27 Flag Day: no Doc impact: no --- diff --git a/dirsrvtests/tests/suites/memberof_plugin/regression_test.py b/dirsrvtests/tests/suites/memberof_plugin/regression_test.py index 5a97492..fb2b4da 100644 --- a/dirsrvtests/tests/suites/memberof_plugin/regression_test.py +++ b/dirsrvtests/tests/suites/memberof_plugin/regression_test.py @@ -508,6 +508,166 @@ def test_memberof_group(topology_st): _find_memberof(inst, dn1, g2n, True) _find_memberof(inst, dn2, g2n, True) +def _config_memberof_entrycache_on_modrdn_failure(server): + + server.plugins.enable(name=PLUGIN_MEMBER_OF) + peoplebase = 'ou=people,%s' % SUFFIX + MEMBEROF_PLUGIN_DN = ('cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config') + server.modify_s(MEMBEROF_PLUGIN_DN, [(ldap.MOD_REPLACE, 'memberOfAllBackends', b'on'), + (ldap.MOD_REPLACE, 'memberOfEntryScope', peoplebase.encode())]) + +def _disable_auto_oc_memberof(server): + MEMBEROF_PLUGIN_DN = ('cn=' + PLUGIN_MEMBER_OF + ',cn=plugins,cn=config') + server.modify_s(MEMBEROF_PLUGIN_DN, + [(ldap.MOD_REPLACE, 'memberOfAutoAddOC', b'nsContainer')]) + +@pytest.mark.ds49967 +def test_entrycache_on_modrdn_failure(topology_st): + """Specify a test case purpose or name here + + :id: a4d8ac0b-2448-406a-9dc2-5a72851e30b6 + :setup: Standalone Instance + :steps: + 1. configure memberof to only scope ou=people,SUFFIX + 2. Creates 10 users + 3. Create groups0 (in peoplebase) that contain user0 and user1 + 4. Check user0 and user1 have memberof=group0.dn + 5. Create group1 (OUT peoplebase) that contain user0 and user1 + 6. Check user0 and user1 have NOT memberof=group1.dn + 7. Move group1 IN peoplebase and check users0 and user1 HAVE memberof=group1.dn + 8. Create group2 (OUT peoplebase) that contain user2 and user3. Group2 contains a specific description value + 9. Check user2 and user3 have NOT memberof=group2.dn + 10. configure memberof so that added objectclass does not allow 'memberof' attribute + 11. Move group2 IN peoplebase and check move failed OPERATIONS_ERROR (because memberof failed) + 12. Search all groups and check that the group, having the specific description value, + has the original DN of group2.dn + :expectedresults: + 1. should succeed + 2. should succeed + 3. should succeed + 4. should succeed + 5. should succeed + 6. should succeed + 7. should succeed + 8. should succeed + 9. should succeed + 10. should succeed + 11. should fail OPERATION_ERROR because memberof plugin fails to add 'memberof' to members. + 12. should succeed + + """ + + # only scopes peoplebase + _config_memberof_entrycache_on_modrdn_failure(topology_st.standalone) + topology_st.standalone.restart(timeout=10) + + # create 10 users + peoplebase = 'ou=people,%s' % SUFFIX + for i in range(10): + cn = 'user%d' % i + dn = 'cn=%s,%s' % (cn, peoplebase) + log.fatal('Adding user (%s): ' % dn) + topology_st.standalone.add_s(Entry((dn, {'objectclass': ['top', 'person'], + 'sn': 'user_%s' % cn, + 'description': 'add on standalone'}))) + + # Check that members of group0 (in the scope) have 'memberof + group0_dn = 'cn=group_in0,%s' % peoplebase + topology_st.standalone.add_s(Entry((group0_dn, {'objectclass': ['top', 'groupofnames'], + 'member': [ + 'cn=user0,%s' % peoplebase, + 'cn=user1,%s' % peoplebase, + ], + 'description': 'mygroup'}))) + + # Check the those entries have memberof with group0 + for i in range(2): + user_dn = 'cn=user%d,%s' % (i, peoplebase) + ent = topology_st.standalone.getEntry(user_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + assert ent.hasAttr('memberof') + found = False + for val in ent.getValues('memberof'): + topology_st.standalone.log.info("!!!!!!! %s: memberof->%s (vs %s)" % (user_dn, val, group0_dn.encode().lower())) + if val.lower() == group0_dn.encode().lower(): + found = True + break + assert found + + # Create a group1 out of the scope + group1_dn = 'cn=group_out1,%s' % SUFFIX + topology_st.standalone.add_s(Entry((group1_dn, {'objectclass': ['top', 'groupofnames'], + 'member': [ + 'cn=user0,%s' % peoplebase, + 'cn=user1,%s' % peoplebase, + ], + 'description': 'mygroup'}))) + + # Check the those entries have not memberof with group1 + for i in range(2): + user_dn = 'cn=user%d,%s' % (i, peoplebase) + ent = topology_st.standalone.getEntry(user_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + assert ent.hasAttr('memberof') + found = False + for val in ent.getValues('memberof'): + topology_st.standalone.log.info("!!!!!!! %s: memberof->%s (vs %s)" % (user_dn, val, group1_dn.encode().lower())) + if val.lower() == group1_dn.encode().lower(): + found = True + break + assert not found + + # move group1 into the scope and check user0 and user1 are memberof group1 + topology_st.standalone.rename_s(group1_dn, 'cn=group_in1', newsuperior=peoplebase, delold=0) + new_group1_dn = 'cn=group_in1,%s' % peoplebase + for i in range(2): + user_dn = 'cn=user%d,%s' % (i, peoplebase) + ent = topology_st.standalone.getEntry(user_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + assert ent.hasAttr('memberof') + found = False + for val in ent.getValues('memberof'): + topology_st.standalone.log.info("!!!!!!! %s: memberof->%s (vs %s)" % (user_dn, val, new_group1_dn.encode().lower())) + if val.lower() == new_group1_dn.encode().lower(): + found = True + break + assert found + + # Create a group2 out of the scope with a SPECIFIC description value + entry_description = "this is to check that the entry having this description has the appropriate DN" + group2_dn = 'cn=group_out2,%s' % SUFFIX + topology_st.standalone.add_s(Entry((group2_dn, {'objectclass': ['top', 'groupofnames'], + 'member': [ + 'cn=user2,%s' % peoplebase, + 'cn=user3,%s' % peoplebase, + ], + 'description': entry_description}))) + + # Check the those entries have not memberof with group2 + for i in (2, 3): + user_dn = 'cn=user%d,%s' % (i, peoplebase) + ent = topology_st.standalone.getEntry(user_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + assert not ent.hasAttr('memberof') + + # memberof will not add the missing objectclass + _disable_auto_oc_memberof(topology_st.standalone) + topology_st.standalone.restart(timeout=10) + + # move group2 into the scope and check it fails + try: + topology_st.standalone.rename_s(group2_dn, 'cn=group_in2', newsuperior=peoplebase, delold=0) + topology_st.standalone.log.info("This is unexpected, modrdn should fail as the member entry have not the appropriate objectclass") + assert False + except ldap.OPERATIONS_ERROR: + pass + + # retrieve the entry having the specific description value + # check that the entry DN is the original group2 DN + ents = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, '(cn=gr*)') + found = False + for ent in ents: + topology_st.standalone.log.info("retrieve: %s with desc=%s" % (ent.dn, ent.getValue('description'))) + if ent.getValue('description') == entry_description.encode(): + found = True + assert ent.dn == group2_dn + assert found if __name__ == '__main__': # Run isolated diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index b9a092c..684b040 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -1400,6 +1400,19 @@ common_return: } } } + + if (ec && retval) { + /* if the operation failed, the destination entry does not exist + * but it has been added in dncache during cache_add_tentative + * we need to remove it. Else a retrieval from ep_id can give the wrong DN + */ + struct backdn *bdn = dncache_find_id(&inst->inst_dncache, ec->ep_id); + slapi_log_err(SLAPI_LOG_CACHE, "ldbm_back_modrdn", + "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); + } + /* remove the new entry from the cache if the op failed - otherwise, leave it in */ if (ec && inst) {