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