From 6cd4b1c60dbd3d7b74adb19a2434585d50553f39 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz <tbordaz@redhat.com>

Date: Fri, 5 Jun 2020 12:14:51 +0200

Subject: [PATCH] Ticket 49859 - A distinguished value can be missing in an

 entry

Bug description:

	According to RFC 4511 (see ticket), the values of the RDN attributes

        should be present in an entry.

	With a set of replicated operations, it is possible that those values

        would be missing

Fix description:

        MOD and MODRDN update checks that the RDN values are presents.

        If they are missing they are added to the resulting entry. In addition

        the set of modifications to add those values are also indexed.

        The specific case of single-valued attributes, where the final and unique value

        can not be the RDN value, the attribute nsds5ReplConflict is added.

https://pagure.io/389-ds-base/issue/49859

Reviewed by: Mark Reynolds, William Brown

Platforms tested: F31

---

 .../replication/conflict_resolve_test.py      | 174 +++++++++++++++++-

 ldap/servers/slapd/back-ldbm/ldbm_modify.c    | 136 ++++++++++++++

 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c    |  37 +++-

 .../servers/slapd/back-ldbm/proto-back-ldbm.h |   1 +

 4 files changed, 343 insertions(+), 5 deletions(-)

diff --git a/dirsrvtests/tests/suites/replication/conflict_resolve_test.py b/dirsrvtests/tests/suites/replication/conflict_resolve_test.py

index 99a072935..48d0067db 100644

--- a/dirsrvtests/tests/suites/replication/conflict_resolve_test.py

+++ b/dirsrvtests/tests/suites/replication/conflict_resolve_test.py

@@ -10,10 +10,11 @@ import time

 import logging

 import ldap

 import pytest

+import re

 from itertools import permutations

 from lib389._constants import *

 from lib389.idm.nscontainer import nsContainers

-from lib389.idm.user import UserAccounts

+from lib389.idm.user import UserAccounts, UserAccount

 from lib389.idm.group import Groups

 from lib389.idm.organizationalunit import OrganizationalUnits

 from lib389.replica import ReplicationManager

@@ -763,6 +764,177 @@ class TestTwoMasters:

         user_dns_m2 = [user.dn for user in test_users_m2.list()]

         assert set(user_dns_m1) == set(user_dns_m2)

+    def test_conflict_attribute_multi_valued(self, topology_m2, base_m2):

+        """A RDN attribute being multi-valued, checks that after several operations

+           MODRDN and MOD_REPL its RDN values are the same on both servers

+

+        :id: 225b3522-8ed7-4256-96f9-5fab9b7044a5

+        :setup: Two master replication,

+                audit log, error log for replica and access log for internal

+        :steps:

+            1. Create a test entry uid=user_test_1000,...

+            2. Pause all replication agreements

+            3. On M1 rename it into uid=foo1,...

+            4. On M2 rename it into uid=foo2,...

+            5. On M1 MOD_REPL uid:foo1

+            6. Resume all replication agreements

+            7. Check that entry on M1 has uid=foo1, foo2

+            8. Check that entry on M2 has uid=foo1, foo2

+            9. Check that entry on M1 and M2 has the same uid values

+        :expectedresults:

+            1. It should pass

+            2. It should pass

+            3. It should pass

+            4. It should pass

+            5. It should pass

+            6. It should pass

+            7. It should pass

+            8. It should pass

+            9. It should pass

+        """

+

+        M1 = topology_m2.ms["master1"]

+        M2 = topology_m2.ms["master2"]

+

+        # add a test user

+        test_users_m1 = UserAccounts(M1, base_m2.dn, rdn=None)

+        user_1 = test_users_m1.create_test_user(uid=1000)

+        test_users_m2 = UserAccount(M2, user_1.dn)

+        # Waiting fo the user to be replicated

+        for i in range(0,4):

+            time.sleep(1)

+            if test_users_m2.exists():

+                break

+        assert(test_users_m2.exists())

+

+        # Stop replication agreements

+        topology_m2.pause_all_replicas()

+

+        # On M1 rename test entry in uid=foo1

+        original_dn = user_1.dn

+        user_1.rename('uid=foo1')

+        time.sleep(1)

+

+        # On M2 rename test entry in uid=foo2

+        M2.rename_s(original_dn, 'uid=foo2')

+        time.sleep(2)

+

+        # on M1 MOD_REPL uid into foo1

+        user_1.replace('uid', 'foo1')

+

+        # resume replication agreements

+        topology_m2.resume_all_replicas()

+        time.sleep(5)

+

+        # check that on M1, the entry 'uid' has two values 'foo1' and 'foo2'

+        final_dn = re.sub('^.*1000,', 'uid=foo2,', original_dn)

+        final_user_m1 = UserAccount(M1, final_dn)

+        for val in final_user_m1.get_attr_vals_utf8('uid'):

+            log.info("Check %s is on M1" % val)

+            assert(val in ['foo1', 'foo2'])

+

+        # check that on M2, the entry 'uid' has two values 'foo1' and 'foo2'

+        final_user_m2 = UserAccount(M2, final_dn)

+        for val in final_user_m2.get_attr_vals_utf8('uid'):

+            log.info("Check %s is on M1" % val)

+            assert(val in ['foo1', 'foo2'])

+

+        # check that the entry have the same uid values

+        for val in final_user_m1.get_attr_vals_utf8('uid'):

+            log.info("Check M1.uid %s is also on M2" % val)

+            assert(val in final_user_m2.get_attr_vals_utf8('uid'))

+

+        for val in final_user_m2.get_attr_vals_utf8('uid'):

+            log.info("Check M2.uid %s is also on M1" % val)

+            assert(val in final_user_m1.get_attr_vals_utf8('uid'))

+

+    def test_conflict_attribute_single_valued(self, topology_m2, base_m2):

+        """A RDN attribute being signle-valued, checks that after several operations

+           MODRDN and MOD_REPL its RDN values are the same on both servers

+

+        :id: c38ae613-5d1e-47cf-b051-c7284e64b817

+        :setup: Two master replication, test container for entries, enable plugin logging,

+                audit log, error log for replica and access log for internal

+        :steps:

+            1. Create a test entry uid=user_test_1000,...

+            2. Pause all replication agreements

+            3. On M1 rename it into employeenumber=foo1,...

+            4. On M2 rename it into employeenumber=foo2,...

+            5. On M1 MOD_REPL employeenumber:foo1

+            6. Resume all replication agreements

+            7. Check that entry on M1 has employeenumber=foo1

+            8. Check that entry on M2 has employeenumber=foo1

+            9. Check that entry on M1 and M2 has the same employeenumber values

+        :expectedresults:

+            1. It should pass

+            2. It should pass

+            3. It should pass

+            4. It should pass

+            5. It should pass

+            6. It should pass

+            7. It should pass

+            8. It should pass

+            9. It should pass

+        """

+

+        M1 = topology_m2.ms["master1"]

+        M2 = topology_m2.ms["master2"]

+

+        # add a test user with a dummy 'uid' extra value because modrdn removes

+        # uid that conflict with 'account' objectclass

+        test_users_m1 = UserAccounts(M1, base_m2.dn, rdn=None)

+        user_1 = test_users_m1.create_test_user(uid=1000)

+        user_1.add('objectclass', 'extensibleobject')

+        user_1.add('uid', 'dummy')

+        test_users_m2 = UserAccount(M2, user_1.dn)

+

+        # Waiting fo the user to be replicated

+        for i in range(0,4):

+            time.sleep(1)

+            if test_users_m2.exists():

+                break

+        assert(test_users_m2.exists())

+

+        # Stop replication agreements

+        topology_m2.pause_all_replicas()

+

+        # On M1 rename test entry in employeenumber=foo1

+        original_dn = user_1.dn

+        user_1.rename('employeenumber=foo1')

+        time.sleep(1)

+

+        # On M2 rename test entry in employeenumber=foo2

+        M2.rename_s(original_dn, 'employeenumber=foo2')

+        time.sleep(2)

+

+        # on M1 MOD_REPL uid into foo1

+        user_1.replace('employeenumber', 'foo1')

+

+        # resume replication agreements

+        topology_m2.resume_all_replicas()

+        time.sleep(5)

+

+        # check that on M1, the entry 'employeenumber' has value 'foo1'

+        final_dn = re.sub('^.*1000,', 'employeenumber=foo2,', original_dn)

+        final_user_m1 = UserAccount(M1, final_dn)

+        for val in final_user_m1.get_attr_vals_utf8('employeenumber'):

+            log.info("Check %s is on M1" % val)

+            assert(val in ['foo1'])

+

+        # check that on M2, the entry 'employeenumber' has values 'foo1'

+        final_user_m2 = UserAccount(M2, final_dn)

+        for val in final_user_m2.get_attr_vals_utf8('employeenumber'):

+            log.info("Check %s is on M2" % val)

+            assert(val in ['foo1'])

+

+        # check that the entry have the same uid values

+        for val in final_user_m1.get_attr_vals_utf8('employeenumber'):

+            log.info("Check M1.uid %s is also on M2" % val)

+            assert(val in final_user_m2.get_attr_vals_utf8('employeenumber'))

+

+        for val in final_user_m2.get_attr_vals_utf8('employeenumber'):

+            log.info("Check M2.uid %s is also on M1" % val)

+            assert(val in final_user_m1.get_attr_vals_utf8('employeenumber'))

 class TestThreeMasters:

     def test_nested_entries(self, topology_m3, base_m3):

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

index e9d7e87e3..a507f3c31 100644

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

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

@@ -213,6 +213,112 @@ error:

     return retval;

 }

+int32_t

+entry_get_rdn_mods(Slapi_PBlock *pb, Slapi_Entry *entry, CSN *csn, int repl_op, Slapi_Mods **smods_ret)

+{

+    unsigned long op_type = SLAPI_OPERATION_NONE;

+    char *new_rdn = NULL;

+    char **dns = NULL;

+    char **rdns = NULL;

+    Slapi_Mods *smods = NULL;

+    char *type = NULL;

+    struct berval *bvp[2] = {0};

+    struct berval bv;

+    Slapi_Attr *attr = NULL;

+    const char *entry_dn = NULL;

+

+    *smods_ret = NULL;

+    entry_dn = slapi_entry_get_dn_const(entry);

+    /* Do not bother to check that RDN is present, no one rename RUV or change its nsuniqueid */

+    if (strcasestr(entry_dn, RUV_STORAGE_ENTRY_UNIQUEID)) {

+        return 0;

+    }

+

+    /* First get the RDNs of the operation */

+    slapi_pblock_get(pb, SLAPI_OPERATION_TYPE, &op_type);

+    switch (op_type) {

+        case SLAPI_OPERATION_MODIFY:

+            dns = slapi_ldap_explode_dn(entry_dn, 0);

+            if (dns == NULL) {

+                slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",

+                      "Fails to split DN \"%s\" into components\n", entry_dn);

+                return -1;

+            }

+            rdns = slapi_ldap_explode_rdn(dns[0], 0);

+            slapi_ldap_value_free(dns);

+

+            break;

+        case SLAPI_OPERATION_MODRDN:

+            slapi_pblock_get(pb, SLAPI_MODRDN_NEWRDN, &new_rdn);

+            rdns = slapi_ldap_explode_rdn(new_rdn, 0);

+            break;

+        default:

+            break;

+    }

+    if (rdns == NULL || rdns[0] == NULL) {

+        slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",

+                      "Fails to split RDN \"%s\" into components\n", slapi_entry_get_dn_const(entry));

+        return -1;

+    }

+

+    /* Update the entry to add RDNs values if they are missing */

+    smods = slapi_mods_new();

+

+    bvp[0] = &bv;

+    bvp[1] = NULL;

+    for (size_t rdns_count = 0; rdns[rdns_count]; rdns_count++) {

+        Slapi_Value *value;

+        attr = NULL;

+        slapi_rdn2typeval(rdns[rdns_count], &type, &bv;;

+

+        /* Check if the RDN value exists */

+        if ((slapi_entry_attr_find(entry, type, &attr) != 0) ||

+            (slapi_attr_value_find(attr, &bv))) {

+            const CSN *csn_rdn_add;

+            const CSN *adcsn = attr_get_deletion_csn(attr);

+

+            /* It is missing => adds it */

+            if (slapi_attr_flag_is_set(attr, SLAPI_ATTR_FLAG_SINGLE)) {

+                if (csn_compare(adcsn, csn) >= 0) {

+                    /* this is a single valued attribute and the current value

+                     * (that is different from RDN value) is more recent than

+                     * the RDN value we want to apply.

+                     * Keep the current value and add a conflict flag

+                     */

+

+                    type = ATTR_NSDS5_REPLCONFLICT;

+                    bv.bv_val = "RDN value may be missing because it is single-valued";

+                    bv.bv_len = strlen(bv.bv_val);

+                    slapi_entry_add_string(entry, type, bv.bv_val);

+                    slapi_mods_add_modbvps(smods, LDAP_MOD_ADD, type, bvp);

+                    continue;

+                }

+            }

+            /* if a RDN value needs to be forced, make sure it csn is ahead */

+            slapi_mods_add_modbvps(smods, LDAP_MOD_ADD, type, bvp);

+            csn_rdn_add = csn_max(adcsn, csn);

+

+            if (entry_apply_mods_wsi(entry, smods, csn_rdn_add, repl_op)) {

+                slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",

+                              "Fails to set \"%s\" in  \"%s\"\n", type, slapi_entry_get_dn_const(entry));

+                slapi_ldap_value_free(rdns);

+                slapi_mods_free(&smods);

+                return -1;

+            }

+            /* Make the RDN value a distinguished value */

+            attr_value_find_wsi(attr, &bv, &value);

+            value_update_csn(value, CSN_TYPE_VALUE_DISTINGUISHED, csn_rdn_add);

+        }

+    }

+    slapi_ldap_value_free(rdns);

+    if (smods->num_mods == 0) {

+        /* smods_ret already NULL, just free the useless smods */

+        slapi_mods_free(&smods);

+    } else {

+        *smods_ret = smods;

+    }

+    return 0;

+}

 /**

    Apply the mods to the ec entry.  Check for syntax, schema problems.

    Check for abandon.

@@ -269,6 +375,8 @@ modify_apply_check_expand(

         goto done;

     }

+

+

     /*

      * If the objectClass attribute type was modified in any way, expand

      * the objectClass values to reflect the inheritance hierarchy.

@@ -414,6 +522,7 @@ ldbm_back_modify(Slapi_PBlock *pb)

     int result_sent = 0;

     int32_t parent_op = 0;

     struct timespec parent_time;

+    Slapi_Mods *smods_add_rdn = NULL;

     slapi_pblock_get(pb, SLAPI_BACKEND, &be);

     slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li;;

@@ -731,6 +840,15 @@ ldbm_back_modify(Slapi_PBlock *pb)

             }

         } /* else if new_mod_count == mod_count then betxnpremod plugin did nothing */

+        /* time to check if applying a replicated operation removed

+         * the RDN value from the entry. Assuming that only replicated update

+         * can lead to that bad result

+         */

+        if (entry_get_rdn_mods(pb, ec->ep_entry, opcsn, repl_op, &smods_add_rdn)) {

+            goto error_return;

+        }

+

+

         /*

          * Update the ID to Entry index.

          * Note that id2entry_add replaces the entry, so the Entry ID

@@ -764,6 +882,23 @@ ldbm_back_modify(Slapi_PBlock *pb)

             MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);

             goto error_return;

         }

+

+        if (smods_add_rdn && slapi_mods_get_num_mods(smods_add_rdn) > 0) {

+            retval = index_add_mods(be, (LDAPMod **) slapi_mods_get_ldapmods_byref(smods_add_rdn), e, ec, &txn);

+            if (DB_LOCK_DEADLOCK == retval) {

+                /* Abort and re-try */

+                slapi_mods_free(&smods_add_rdn);

+                continue;

+            }

+            if (retval != 0) {

+                slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modify",

+                        "index_add_mods (rdn) failed, err=%d %s\n",

+                        retval, (msg = dblayer_strerror(retval)) ? msg : "");

+                MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);

+                slapi_mods_free(&smods_add_rdn);

+                goto error_return;

+            }

+        }

         /*

          * Remove the old entry from the Virtual List View indexes.

          * Add the new entry to the Virtual List View indexes.

@@ -978,6 +1113,7 @@ error_return:

 common_return:

     slapi_mods_done(&smods);

+    slapi_mods_free(&smods_add_rdn);

     if (inst) {

         if (ec_locked || cache_is_in_cache(&inst->inst_cache, ec)) {

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

index fde83c99f..e97b7a5f6 100644

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

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

@@ -21,7 +21,7 @@ static void moddn_unlock_and_return_entry(backend *be, struct backentry **target

 static int moddn_newrdn_mods(Slapi_PBlock *pb, const char *olddn, struct backentry *ec, Slapi_Mods *smods_wsi, int is_repl_op);

 static IDList *moddn_get_children(back_txn *ptxn, Slapi_PBlock *pb, backend *be, struct backentry *parententry, Slapi_DN *parentdn, struct backentry ***child_entries, struct backdn ***child_dns, int is_resurect_operation);

 static int moddn_rename_children(back_txn *ptxn, Slapi_PBlock *pb, backend *be, IDList *children, Slapi_DN *dn_parentdn, Slapi_DN *dn_newsuperiordn, struct backentry *child_entries[]);

-static int modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3);

+static int modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li, struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3, Slapi_Mods *smods4);

 static void mods_remove_nsuniqueid(Slapi_Mods *smods);

 #define MOD_SET_ERROR(rc, error, count)                                            \

@@ -100,6 +100,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb)

     Connection *pb_conn = NULL;

     int32_t parent_op = 0;

     struct timespec parent_time;

+    Slapi_Mods *smods_add_rdn = NULL;

     if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) {

         conn_id = 0; /* connection is NULL */

@@ -842,6 +843,15 @@ ldbm_back_modrdn(Slapi_PBlock *pb)

                     goto error_return;

                 }

             }

+

+            /* time to check if applying a replicated operation removed

+             * the RDN value from the entry. Assuming that only replicated update

+             * can lead to that bad result

+             */

+            if (entry_get_rdn_mods(pb, ec->ep_entry, opcsn, is_replicated_operation, &smods_add_rdn)) {

+                goto error_return;

+            }

+

             /* check that the entry still obeys the schema */

             if (slapi_entry_schema_check(pb, ec->ep_entry) != 0) {

                 ldap_result_code = LDAP_OBJECT_CLASS_VIOLATION;

@@ -1003,7 +1013,7 @@ ldbm_back_modrdn(Slapi_PBlock *pb)

         /*

          * Update the indexes for the entry.

          */

-        retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi);

+        retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi, smods_add_rdn);

         if (DB_LOCK_DEADLOCK == retval) {

             /* Retry txn */

             continue;

@@ -1497,6 +1507,7 @@ common_return:

     slapi_mods_done(&smods_operation_wsi);

     slapi_mods_done(&smods_generated);

     slapi_mods_done(&smods_generated_wsi);

+    slapi_mods_free(&smods_add_rdn);

     slapi_ch_free((void **)&child_entries);

     slapi_ch_free((void **)&child_dns);

     if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL"))

@@ -1778,7 +1789,7 @@ mods_remove_nsuniqueid(Slapi_Mods *smods)

  * mods contains the list of attribute change made.

  */

 static int

-modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li __attribute__((unused)), struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3)

+modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbminfo *li __attribute__((unused)), struct backentry *e, struct backentry **ec, Slapi_Mods *smods1, Slapi_Mods *smods2, Slapi_Mods *smods3, Slapi_Mods *smods4)

 {

     backend *be;

     ldbm_instance *inst;

@@ -1874,6 +1885,24 @@ modrdn_rename_entry_update_indexes(back_txn *ptxn, Slapi_PBlock *pb, struct ldbm

             goto error_return;

         }

     }

+    if (smods4 != NULL && slapi_mods_get_num_mods(smods4) > 0) {

+        /*

+         * update the indexes: lastmod, rdn, etc.

+         */

+        retval = index_add_mods(be, slapi_mods_get_ldapmods_byref(smods4), e, *ec, ptxn);

+        if (DB_LOCK_DEADLOCK == retval) {

+            /* Retry txn */

+            slapi_log_err(SLAPI_LOG_BACKLDBM, "modrdn_rename_entry_update_indexes",

+                          "index_add_mods4 deadlock\n");

+            goto error_return;

+        }

+        if (retval != 0) {

+            slapi_log_err(SLAPI_LOG_TRACE, "modrdn_rename_entry_update_indexes",

+                          "index_add_mods 4 failed, err=%d %s\n",

+                          retval, (msg = dblayer_strerror(retval)) ? msg : "");

+            goto error_return;

+        }

+    }

     /*

      * Remove the old entry from the Virtual List View indexes.

      * Add the new entry to the Virtual List View indexes.

@@ -1991,7 +2020,7 @@ moddn_rename_child_entry(

          * Update all the indexes.

          */

         retval = modrdn_rename_entry_update_indexes(ptxn, pb, li, e, ec,

-                                                    smodsp, NULL, NULL);

+                                                    smodsp, NULL, NULL, NULL);

         /* JCMREPL - Should the children get updated modifiersname and lastmodifiedtime? */

         slapi_mods_done(&smods);

     }

diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

index 4d2524fd9..e2f1100ed 100644

--- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

+++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -324,6 +324,7 @@ int get_parent_rdn(DB *db, ID parentid, Slapi_RDN *srdn);

 /*

  * modify.c

  */

+int32_t entry_get_rdn_mods(Slapi_PBlock *pb, Slapi_Entry *entry, CSN *csn, int repl_op, Slapi_Mods **smods_ret);

 int modify_update_all(backend *be, Slapi_PBlock *pb, modify_context *mc, back_txn *txn);

 void modify_init(modify_context *mc, struct backentry *old_entry);

 int modify_apply_mods(modify_context *mc, Slapi_Mods *smods);

-- 

2.26.2