From bf067513a88156f5b3882dd4098a61110ced4caf Mon Sep 17 00:00:00 2001

From: tbordaz <tbordaz@redhat.com>

Date: Fri, 2 Oct 2020 12:03:12 +0200

Subject: [PATCH 4/5] Issue 4297- On ADD replication URP issue internal

 searches with filter containing unescaped chars (#4355)

Bug description:

	In MMR a consumer receiving a ADD has to do some checking based on basedn.

	It checks if the entry was a tombstone or if the conflicting parent entry was a tombstone.

	To do this checking, URP does internal searches using basedn.

	A '*' (ASTERISK) is valid in a RDN and in a DN. But using a DN in an assertionvalue of a filter, the ASTERISK needs to be escaped else the server will interprete the filtertype to be a substring. (see

	https://tools.ietf.org/html/rfc4515#section-3)

	The problem is that if a added entry contains an ASTERISK in the DN, it will not be escaped in internal search and trigger substring search (likely unindexed).

Fix description:

	escape the DN before doing internal search in URP

Fixes: #4297

Reviewed by:  Mark Reynolds, William Brown, Simon Pichugi (thanks !)

Platforms tested: F31

---

 .../suites/replication/acceptance_test.py     | 167 ++++++++++++++++++

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

 ldap/servers/slapd/filter.c                   |  21 +++

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

 4 files changed, 196 insertions(+), 3 deletions(-)

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

index 2897726f5..be432bea4 100644

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

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

@@ -7,6 +7,8 @@

 # --- END COPYRIGHT BLOCK ---

 #

 import pytest

+import logging

+from lib389.replica import Replicas

 from lib389.tasks import *

 from lib389.utils import *

 from lib389.topologies import topology_m4 as topo_m4

@@ -524,6 +526,171 @@ def test_invalid_agmt(topo_m4):

         assert False

+def test_warining_for_invalid_replica(topo_m4):

+    """Testing logs to indicate the inconsistency when configuration is performed.

+

+    :id: dd689d03-69b8-4bf9-a06e-2acd19d5e2c8

+    :setup: MMR with four masters

+    :steps:

+        1. Setup nsds5ReplicaBackoffMin to 20

+        2. Setup nsds5ReplicaBackoffMax to 10

+    :expectedresults:

+        1. nsds5ReplicaBackoffMin should set to 20

+        2. An error should be generated and also logged in the error logs.

+    """

+    replicas = Replicas(topo_m4.ms["master1"])

+    replica = replicas.list()[0]

+    log.info('Set nsds5ReplicaBackoffMin to 20')

+    replica.set('nsds5ReplicaBackoffMin', '20')

+    with pytest.raises(ldap.UNWILLING_TO_PERFORM):

+        log.info('Set nsds5ReplicaBackoffMax to 10')

+        replica.set('nsds5ReplicaBackoffMax', '10')

+    log.info('Resetting configuration: nsds5ReplicaBackoffMin')

+    replica.remove_all('nsds5ReplicaBackoffMin')

+    log.info('Check the error log for the error')

+    assert topo_m4.ms["master1"].ds_error_log.match('.*nsds5ReplicaBackoffMax.*10.*invalid.*')

+

+@pytest.mark.skipif(ds_is_older('1.4.4'), reason="Not implemented")

+def test_csngen_task(topo_m2):

+    """Test csn generator test

+

+    :id: b976849f-dbed-447e-91a7-c877d5d71fd0

+    :setup: MMR with 2 masters

+    :steps:

+        1. Create a csngen_test task

+        2. Check that debug messages "_csngen_gen_tester_main" are in errors logs

+    :expectedresults:

+        1. Should succeeds

+        2. Should succeeds

+    """

+    m1 = topo_m2.ms["master1"]

+    csngen_task = csngenTestTask(m1)

+    csngen_task.create(properties={

+        'ttl': '300'

+    })

+    time.sleep(10)

+    log.info('Check the error log contains strings showing csn generator is tested')

+    assert m1.searchErrorsLog("_csngen_gen_tester_main")

+

+@pytest.mark.ds51082

+def test_csnpurge_large_valueset(topo_m2):

+    """Test csn generator test

+

+    :id: 63e2bdb2-0a8f-4660-9465-7b80a9f72a74

+    :setup: MMR with 2 masters

+    :steps:

+        1. Create a test_user

+        2. add a large set of values (more than 10)

+        3. delete all the values (more than 10)

+        4. configure the replica to purge those values (purgedelay=5s)

+        5. Waiting for 6 second

+        6. do a series of update

+    :expectedresults:

+        1. Should succeeds

+        2. Should succeeds

+        3. Should succeeds

+        4. Should succeeds

+        5. Should succeeds

+        6. Should not crash

+    """

+    m1 = topo_m2.ms["master2"]

+

+    test_user = UserAccount(m1, TEST_ENTRY_DN)

+    if test_user.exists():

+        log.info('Deleting entry {}'.format(TEST_ENTRY_DN))

+        test_user.delete()

+    test_user.create(properties={

+        'uid': TEST_ENTRY_NAME,

+        'cn': TEST_ENTRY_NAME,

+        'sn': TEST_ENTRY_NAME,

+        'userPassword': TEST_ENTRY_NAME,

+        'uidNumber' : '1000',

+        'gidNumber' : '2000',

+        'homeDirectory' : '/home/mmrepl_test',

+    })

+

+    # create a large value set so that it is sorted

+    for i in range(1,20):

+        test_user.add('description', 'value {}'.format(str(i)))

+

+    # delete all values of the valueset

+    for i in range(1,20):

+        test_user.remove('description', 'value {}'.format(str(i)))

+

+    # set purging delay to 5 second and wait more that 5second

+    replicas = Replicas(m1)

+    replica = replicas.list()[0]

+    log.info('nsds5ReplicaPurgeDelay to 5')

+    replica.set('nsds5ReplicaPurgeDelay', '5')

+    time.sleep(6)

+

+    # add some new values to the valueset containing entries that should be purged

+    for i in range(21,25):

+        test_user.add('description', 'value {}'.format(str(i)))

+

+@pytest.mark.ds51244

+def test_urp_trigger_substring_search(topo_m2):

+    """Test that a ADD of a entry with a '*' in its DN, triggers

+    an internal search with a escaped DN

+

+    :id: 9869bb39-419f-42c3-a44b-c93eb0b77667

+    :setup: MMR with 2 masters

+    :steps:

+        1. enable internal operation loggging for plugins

+        2. Create on M1 a test_user with a '*' in its DN

+        3. Check the test_user is replicated

+        4. Check in access logs that the internal search does not contain '*'

+    :expectedresults:

+        1. Should succeeds

+        2. Should succeeds

+        3. Should succeeds

+        4. Should succeeds

+    """

+    m1 = topo_m2.ms["master1"]

+    m2 = topo_m2.ms["master2"]

+

+    # Enable loggging of internal operation logging to capture URP intop

+    log.info('Set nsslapd-plugin-logging to on')

+    for inst in (m1, m2):

+        inst.config.loglevel([AccessLog.DEFAULT, AccessLog.INTERNAL], service='access')

+        inst.config.set('nsslapd-plugin-logging', 'on')

+        inst.restart()

+

+    # add a user with a DN containing '*'

+    test_asterisk_uid = 'asterisk_*_in_value'

+    test_asterisk_dn = 'uid={},{}'.format(test_asterisk_uid, DEFAULT_SUFFIX)

+

+    test_user = UserAccount(m1, test_asterisk_dn)

+    if test_user.exists():

+        log.info('Deleting entry {}'.format(test_asterisk_dn))

+        test_user.delete()

+    test_user.create(properties={

+        'uid': test_asterisk_uid,

+        'cn': test_asterisk_uid,

+        'sn': test_asterisk_uid,

+        'userPassword': test_asterisk_uid,

+        'uidNumber' : '1000',

+        'gidNumber' : '2000',

+        'homeDirectory' : '/home/asterisk',

+    })

+

+    # check that the ADD was replicated on M2

+    test_user_m2 = UserAccount(m2, test_asterisk_dn)

+    for i in range(1,5):

+        if test_user_m2.exists():

+            break

+        else:

+            log.info('Entry not yet replicated on M2, wait a bit')

+            time.sleep(2)

+

+    # check that M2 access logs does not "(&(objectclass=nstombstone)(nscpentrydn=uid=asterisk_*_in_value,dc=example,dc=com))"

+    log.info('Check that on M2, URP as not triggered such internal search')

+    pattern = ".*\(Internal\).*SRCH.*\(&\(objectclass=nstombstone\)\(nscpentrydn=uid=asterisk_\*_in_value,dc=example,dc=com.*"

+    found = m2.ds_access_log.match(pattern)

+    log.info("found line: %s" % found)

+    assert not found

+

+

 if __name__ == '__main__':

     # Run isolated

     # -s for DEBUG mode

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

index 37fe77379..2c2d83c6c 100644

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

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

@@ -1379,9 +1379,12 @@ urp_add_check_tombstone (Slapi_PBlock *pb, char *sessionid, Slapi_Entry *entry,

     Slapi_Entry **entries = NULL;

     Slapi_PBlock *newpb;

     char *basedn = slapi_entry_get_ndn(entry);

+    char *escaped_basedn;

     const Slapi_DN *suffix = slapi_get_suffix_by_dn(slapi_entry_get_sdn (entry));

+    escaped_basedn = slapi_filter_escape_filter_value("nscpentrydn", basedn);

-    char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", basedn);

+    char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", escaped_basedn);

+    slapi_ch_free((void **)&escaped_basedn);

     newpb = slapi_pblock_new();

     slapi_search_internal_set_pb(newpb,

                                  slapi_sdn_get_dn(suffix), /* Base DN */

@@ -1574,11 +1577,12 @@ urp_find_tombstone_for_glue (Slapi_PBlock *pb, char *sessionid, const Slapi_Entr

     Slapi_PBlock *newpb;

     const char *basedn = slapi_sdn_get_dn(parentdn);

     char *conflict_csnstr = slapi_entry_attr_get_charptr(entry, "conflictcsn");

+    char *escaped_basedn = slapi_filter_escape_filter_value("nscpentrydn", basedn);

     CSN *conflict_csn = csn_new_by_string(conflict_csnstr);

     slapi_ch_free_string(&conflict_csnstr);

     CSN *tombstone_csn = NULL;

-

-    char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", basedn);

+    char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", escaped_basedn);

+    slapi_ch_free((void **)&escaped_basedn);

     newpb = slapi_pblock_new();

     char *parent_dn = slapi_dn_parent (basedn);

     slapi_search_internal_set_pb(newpb,

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

index 393a4dcee..8e21b34c3 100644

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

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

@@ -127,6 +127,27 @@ filter_escape_filter_value(struct slapi_filter *f, const char *fmt, size_t len _

     return ptr;

 }

+/* Escaped an equality filter value (assertionValue) of a given attribute

+ * Caller must free allocated escaped filter value

+ */

+char *

+slapi_filter_escape_filter_value(char* filter_attr, char *filter_value)

+{

+    char *result;

+    struct slapi_filter *f;

+

+    if ((filter_attr == NULL) || (filter_value == NULL)) {

+        return NULL;

+    }

+    f = (struct slapi_filter *)slapi_ch_calloc(1, sizeof(struct slapi_filter));

+    f->f_choice = LDAP_FILTER_EQUALITY;

+    f->f_un.f_un_ava.ava_type = filter_attr;

+    f->f_un.f_un_ava.ava_value.bv_len = strlen(filter_value);

+    f->f_un.f_un_ava.ava_value.bv_val = filter_value;

+    result = filter_escape_filter_value(f, FILTER_EQ_FMT, FILTER_EQ_LEN);

+    slapi_ch_free((void**) &f);

+    return result;

+}

 /*

  * get_filter_internal(): extract an LDAP filter from a BerElement and create

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

index ea9bcf87b..a8b563dd1 100644

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

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

@@ -5284,6 +5284,7 @@ int slapi_vattr_filter_test_ext(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Filter *

 int slapi_filter_compare(struct slapi_filter *f1, struct slapi_filter *f2);

 Slapi_Filter *slapi_filter_dup(Slapi_Filter *f);

 int slapi_filter_changetype(Slapi_Filter *f, const char *newtype);

+char *slapi_filter_escape_filter_value(char* filter_attr, char *filter_value);

 int slapi_attr_is_last_mod(char *attr);

-- 

2.26.2