From 233b64f26df76aa50f4b37aaf6b3804d208fdc1b Mon Sep 17 00:00:00 2001

From: Ludwig Krispenz <lkrispen@redhat.com>

Date: Mon, 12 Feb 2018 09:24:25 +0100

Subject: [PATCH] Ticket 49551 - v3 - correct handling of numsubordinates for

 cenotaphs and tombstone delete

        Bug: The ticket exposed several problems with tombstone handling.

                - tombstone entries of conflicts were not purged in tombstone purging

                - cenotaphs are tombstone, but the subordinate count was not managed properly

                - direct delete of tombstones failed with err=1

                - delete of entry with only conflict children failed correctly, but gave no hint why

        Fix: update the correct numsobordinates attribut for cenotaphs

             set proper flag in directly deleting a tombstone

             change search filter for tombstone purging to include ldapsubentries

             check for conflict children if a delete is rejected and add a message to the response

        Reviewed by; Thierry, William - thanks

---

 ldap/servers/plugins/replication/repl5_replica.c | 14 +++++++++--

 ldap/servers/plugins/replication/urp.c           |  8 +++---

 ldap/servers/slapd/back-ldbm/ldbm_add.c          |  8 +++---

 ldap/servers/slapd/back-ldbm/ldbm_delete.c       | 14 ++++++++---

 ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c     |  3 ++-

 ldap/servers/slapd/back-ldbm/parents.c           | 12 ++++++---

 ldap/servers/slapd/entry.c                       | 31 ++++++++++++++++++++++++

 ldap/servers/slapd/slapi-plugin.h                |  2 ++

 ldap/servers/slapd/slapi-private.h               |  1 +

 ldap/servers/slapd/task.c                        |  4 +--

 10 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c

index bdb8a5167..628fb9ceb 100644

--- a/ldap/servers/plugins/replication/repl5_replica.c

+++ b/ldap/servers/plugins/replication/repl5_replica.c

@@ -3017,6 +3017,16 @@ process_reap_entry(Slapi_Entry *entry, void *cb_data)

        search in the future, see _replica_reap_tombstones below and add more to the

        attrs array */

     deletion_csn = entry_get_deletion_csn(entry);

+    if (deletion_csn == NULL) {

+        /* this might be a tombstone which was directly added, eg a cenotaph

+         * check if a tombstonecsn exist and use it

+         */

+        char *tombstonecsn = slapi_entry_attr_get_charptr(entry, SLAPI_ATTR_TOMBSTONE_CSN);

+        if (tombstonecsn) {

+            deletion_csn = csn_new_by_string(tombstonecsn);

+            slapi_ch_free_string(&tombstonecsn);

+        }

+    }

     if ((NULL == deletion_csn || csn_compare(deletion_csn, purge_csn) < 0) &&

         (!is_ruv_tombstone_entry(entry))) {

@@ -3116,11 +3126,11 @@ _replica_reap_tombstones(void *arg)

              */

             csn_as_string(purge_csn, PR_FALSE, deletion_csn_str);

             PR_snprintf(tombstone_filter, 128,

-                        "(&(%s<=%s)(objectclass=nsTombstone))", SLAPI_ATTR_TOMBSTONE_CSN,

+                        "(&(%s<=%s)(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))", SLAPI_ATTR_TOMBSTONE_CSN,

                         csn_as_string(purge_csn, PR_FALSE, deletion_csn_str));

         } else {

             /* Use the old inefficient filter */

-            PR_snprintf(tombstone_filter, 128, "(objectclass=nsTombstone)");

+            PR_snprintf(tombstone_filter, 128, "(&(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))");

         }

         /* we just need the objectclass - for the deletion csn

diff --git a/ldap/servers/plugins/replication/urp.c b/ldap/servers/plugins/replication/urp.c

index d4556d7fd..11c5da7cf 100644

--- a/ldap/servers/plugins/replication/urp.c

+++ b/ldap/servers/plugins/replication/urp.c

@@ -911,7 +911,7 @@ urp_fixup_add_cenotaph (Slapi_PBlock *pb, char *sessionid, CSN *opcsn)

                                     cenotaph,

                                     NULL,

                                     repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION),

-                                    OP_FLAG_REPL_FIXUP|OP_FLAG_NOOP);

+                                    OP_FLAG_REPL_FIXUP|OP_FLAG_NOOP|OP_FLAG_CENOTAPH_ENTRY);

     slapi_add_internal_pb(add_pb);

     slapi_pblock_get(add_pb, SLAPI_PLUGIN_INTOP_RESULT, &ret;;

@@ -1922,7 +1922,7 @@ done:

     newpb = NULL;

     slapi_log_err(SLAPI_LOG_REPL, sessionid,

-                  "urp_get_min_naming_conflict_entry - Found %d entries\n", i);

+                  "urp_get_min_naming_conflict_entry - Found %d entries\n", min_csn?1:0);

     return min_naming_conflict_entry;

 }

@@ -2172,8 +2172,8 @@ mod_objectclass_attr(const char *uniqueid, const Slapi_DN *entrysdn, const Slapi

     char csnstr[CSN_STRSIZE+1] = {0};

     slapi_mods_init(&smods, 3);

-    slapi_mods_add(&smods, LDAP_MOD_ADD, "objectclass", strlen("ldapsubentry"),"ldapsubentry");

-    slapi_mods_add(&smods, LDAP_MOD_REPLACE, "conflictcsn", CSN_STRSIZE, csn_as_string(opcsn, PR_FALSE, csnstr));

+    slapi_mods_add_string(&smods, LDAP_MOD_ADD, "objectclass", "ldapsubentry");

+    slapi_mods_add_string(&smods, LDAP_MOD_REPLACE, "conflictcsn", csn_as_string(opcsn, PR_FALSE, csnstr));

     op_result = urp_fixup_modify_entry(uniqueid, entrysdn, opcsn, &smods, 0);

     slapi_mods_done(&smods);

     if (op_result == LDAP_TYPE_OR_VALUE_EXISTS) {

diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c

index c93d44a65..f0a3262ec 100644

--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c

+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c

@@ -81,6 +81,7 @@ ldbm_back_add(Slapi_PBlock *pb)

     Slapi_Operation *operation;

     int is_replicated_operation = 0;

     int is_resurect_operation = 0;

+    int is_cenotaph_operation = 0;

     int is_tombstone_operation = 0;

     int is_fixup_operation = 0;

     int is_remove_from_cache = 0;

@@ -116,6 +117,7 @@ ldbm_back_add(Slapi_PBlock *pb)

     }

     is_resurect_operation = operation_is_flag_set(operation, OP_FLAG_RESURECT_ENTRY);

+    is_cenotaph_operation = operation_is_flag_set(operation, OP_FLAG_CENOTAPH_ENTRY);

     is_tombstone_operation = operation_is_flag_set(operation, OP_FLAG_TOMBSTONE_ENTRY);

     is_fixup_operation = operation_is_flag_set(operation, OP_FLAG_REPL_FIXUP);

     is_ruv = operation_is_flag_set(operation, OP_FLAG_REPL_RUV);

@@ -846,9 +848,9 @@ ldbm_back_add(Slapi_PBlock *pb)

                the in-memory state of the parent to reflect the new child (update

                subordinate count specifically */

             if (parententry) {

-                retval = parent_update_on_childchange(&parent_modify_c,

-                                                      is_resurect_operation ? PARENTUPDATE_RESURECT : PARENTUPDATE_ADD,

-                                                      NULL);

+                int op = is_resurect_operation ? PARENTUPDATE_RESURECT : PARENTUPDATE_ADD;

+                if (is_cenotaph_operation ) op |= PARENTUPDATE_CREATE_TOMBSTONE;

+                retval = parent_update_on_childchange(&parent_modify_c, op, NULL);

                 slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_add",

                               "conn=%lu op=%d parent_update_on_childchange: old_entry=0x%p, new_entry=0x%p, rc=%d\n",

                               conn_id, op_id, parent_modify_c.old_entry, parent_modify_c.new_entry, retval);

diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c

index be0db1bd0..bc0a3654e 100644

--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c

+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c

@@ -291,9 +291,16 @@ replace_entry:

             retval = slapi_entry_has_children(e->ep_entry);

             if (retval && !is_replicated_operation) {

                 ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF;

-                slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete", 

-                              "conn=%lu op=%d Deleting entry %s has %d children.\n", 

-                               conn_id, op_id, slapi_entry_get_dn(e->ep_entry), retval);

+                if (slapi_entry_has_conflict_children(e->ep_entry, (void *)li->li_identity) > 0) {

+                    ldap_result_message = "Entry has replication conflicts as children";

+                    slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_delete",

+                                  "conn=%lu op=%d Deleting entry %s has replication conflicts as children.\n",

+                                   conn_id, op_id, slapi_entry_get_dn(e->ep_entry));

+                } else {

+                    slapi_log_err(SLAPI_LOG_BACKLDBM, "ldbm_back_delete",

+                                  "conn=%lu op=%d Deleting entry %s has %d children.\n",

+                                   conn_id, op_id, slapi_entry_get_dn(e->ep_entry), retval);

+                }

                 retval = -1;

                 goto error_return;

             }

@@ -431,6 +438,7 @@ replace_entry:

                     slapi_log_err(SLAPI_LOG_WARNING, "ldbm_back_delete",

                             "Attempt to Tombstone again a tombstone entry %s\n", dn);

                     delete_tombstone_entry = 1;

+                    operation_set_flag(operation, OP_FLAG_TOMBSTONE_ENTRY);

                 }

             }

diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c

index b41a2d241..5797dd779 100644

--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c

+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c

@@ -2824,7 +2824,8 @@ _entryrdn_delete_key(backend *be,

                     break;

                 }

                 childelem = (rdn_elem *)dataret.data;

-                if (!slapi_is_special_rdn(childelem->rdn_elem_nrdn_rdn, RDN_IS_TOMBSTONE)) {

+                if (!slapi_is_special_rdn(childelem->rdn_elem_nrdn_rdn, RDN_IS_TOMBSTONE) &&

+                    !strcasestr(childelem->rdn_elem_nrdn_rdn, "cenotaphid")) {

                     /* there's at least one live child */

                     slapi_log_err(SLAPI_LOG_ERR, "_entryrdn_delete_key",

                                   "Failed to remove %s; has a child %s\n", nrdn,

diff --git a/ldap/servers/slapd/back-ldbm/parents.c b/ldap/servers/slapd/back-ldbm/parents.c

index 79e66451e..1afc795c0 100644

--- a/ldap/servers/slapd/back-ldbm/parents.c

+++ b/ldap/servers/slapd/back-ldbm/parents.c

@@ -89,7 +89,11 @@ parent_update_on_childchange(modify_context *mc, int op, size_t *new_sub_count)

         }

     }

-    if (PARENTUPDATE_DELETE_TOMBSTONE != repl_op) {

+    if ((PARENTUPDATE_ADD == op) && (PARENTUPDATE_CREATE_TOMBSTONE == repl_op)) {

+        /* we are directly adding a tombstone entry, only need to

+         * update the tombstone subordinates

+         */

+    } else if (PARENTUPDATE_DELETE_TOMBSTONE != repl_op) {

         /* are we adding ? */

         if (((PARENTUPDATE_ADD == op) || (PARENTUPDATE_RESURECT == op)) && !already_present) {

             /* If so, and the parent entry does not already have a subcount

@@ -136,10 +140,10 @@ parent_update_on_childchange(modify_context *mc, int op, size_t *new_sub_count)

         }

     }

-    /* tombstoneNumSubordinates is needed only when this is repl op

-     * and a child is being deleted */

+    /* tombstoneNumSubordinates has to be updated if a tombstone child has been

+     * deleted or a tombstone has been directly added (cenotaph) */

     current_sub_count = LDAP_MAXINT;

-    if ((repl_op && (PARENTUPDATE_DEL == op)) || (PARENTUPDATE_RESURECT == op)) {

+    if (repl_op) {

         ret = slapi_entry_attr_find(mc->old_entry->ep_entry,

                                     tombstone_numsubordinates, &read_attr);

         if (0 == ret) {

diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c

index 32828b4e2..b85e9f5b0 100644

--- a/ldap/servers/slapd/entry.c

+++ b/ldap/servers/slapd/entry.c

@@ -3238,6 +3238,37 @@ slapi_entry_has_children(const Slapi_Entry *entry)

     return slapi_entry_has_children_ext(entry, 0);

 }

+int

+slapi_entry_has_conflict_children(const Slapi_Entry *entry, void *plg_id)

+{

+    Slapi_PBlock *search_pb = NULL;

+    Slapi_Entry **entries;

+    int rc = 0;

+

+    search_pb = slapi_pblock_new();

+    slapi_search_internal_set_pb(search_pb, slapi_entry_get_dn_const(entry),

+                                 LDAP_SCOPE_ONELEVEL,

+                                 "(&(objectclass=ldapsubentry)(nsds5ReplConflict=namingConflict*))",

+                                 NULL, 0, NULL, NULL, plg_id, 0);

+    slapi_search_internal_pb(search_pb);

+    slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc);

+    if (rc) {

+        rc = -1;

+    } else {

+        slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES, &entries);

+        if (entries && entries[0]) {

+            /* we found at least one conflict entry */

+            rc = 1;

+        } else {

+            rc = 0;

+        }

+        slapi_free_search_results_internal(search_pb);

+    }

+    slapi_pblock_destroy(search_pb);

+

+    return rc;

+}

+

 /*

  * Renames an entry to simulate a MODRDN operation

  */

diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h

index 95cdcc0da..6978e258f 100644

--- a/ldap/servers/slapd/slapi-plugin.h

+++ b/ldap/servers/slapd/slapi-plugin.h

@@ -2000,6 +2000,8 @@ int slapi_entry_has_children(const Slapi_Entry *e);

  */

 int slapi_entry_has_children_ext(const Slapi_Entry *e, int include_tombstone);

+int slapi_entry_has_conflict_children(const Slapi_Entry *e, void *plg_id);

+

 /**

  * This function determines if an entry is the root DSE.

  *

diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h

index 548d5cabb..b08c0d7ce 100644

--- a/ldap/servers/slapd/slapi-private.h

+++ b/ldap/servers/slapd/slapi-private.h

@@ -403,6 +403,7 @@ char *slapi_filter_to_string_internal(const struct slapi_filter *f, char *buf, s

 #define OP_FLAG_NEVER_CHAIN SLAPI_OP_FLAG_NEVER_CHAIN         /* 0x000800 */

 #define OP_FLAG_TOMBSTONE_ENTRY SLAPI_OP_FLAG_TOMBSTONE_ENTRY /* 0x001000 */

 #define OP_FLAG_RESURECT_ENTRY 0x002000

+#define OP_FLAG_CENOTAPH_ENTRY 0x004000

 #define OP_FLAG_ACTION_NOLOG 0x008000            /* Do not log the entry in \

                                                   * audit log or change log  \

                                                   */

diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c

index 4bd8895ff..3f9d5d995 100644

--- a/ldap/servers/slapd/task.c

+++ b/ldap/servers/slapd/task.c

@@ -2352,10 +2352,10 @@ task_fixup_tombstone_thread(void *arg)

     if (task_data->stripcsn) {

         /* find tombstones with nsTombstoneCSN */

-        filter = "(&(nstombstonecsn=*)(objectclass=nsTombstone))";

+        filter = "(&(nstombstonecsn=*)(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))";

     } else {

         /* find tombstones missing nsTombstoneCSN */

-        filter = "(&(!(nstombstonecsn=*))(objectclass=nsTombstone))";

+        filter = "(&(!(nstombstonecsn=*))(objectclass=nsTombstone)(|(objectclass=*)(objectclass=ldapsubentry)))";

     }

     /* Okay check the specified backends only */

-- 

2.13.6