|
|
5d2be4 |
From e78d3bd879b880d679b49f3fa5ebe8009d309063 Mon Sep 17 00:00:00 2001
|
|
|
5d2be4 |
From: tbordaz <tbordaz@redhat.com>
|
|
|
5d2be4 |
Date: Fri, 2 Oct 2020 12:03:12 +0200
|
|
|
5d2be4 |
Subject: [PATCH 1/8] Issue 4297- On ADD replication URP issue internal
|
|
|
5d2be4 |
searches with filter containing unescaped chars (#4355)
|
|
|
5d2be4 |
|
|
|
5d2be4 |
Bug description:
|
|
|
5d2be4 |
In MMR a consumer receiving a ADD has to do some checking based on basedn.
|
|
|
5d2be4 |
It checks if the entry was a tombstone or if the conflicting parent entry was a tombstone.
|
|
|
5d2be4 |
|
|
|
5d2be4 |
To do this checking, URP does internal searches using basedn.
|
|
|
5d2be4 |
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
|
|
|
5d2be4 |
https://tools.ietf.org/html/rfc4515#section-3)
|
|
|
5d2be4 |
|
|
|
5d2be4 |
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).
|
|
|
5d2be4 |
|
|
|
5d2be4 |
Fix description:
|
|
|
5d2be4 |
escape the DN before doing internal search in URP
|
|
|
5d2be4 |
|
|
|
5d2be4 |
Fixes: #4297
|
|
|
5d2be4 |
|
|
|
5d2be4 |
Reviewed by: Mark Reynolds, William Brown, Simon Pichugi (thanks !)
|
|
|
5d2be4 |
|
|
|
5d2be4 |
Platforms tested: F31
|
|
|
5d2be4 |
---
|
|
|
5d2be4 |
.../suites/replication/acceptance_test.py | 63 +++++++++++++++++++
|
|
|
5d2be4 |
ldap/servers/plugins/replication/urp.c | 10 ++-
|
|
|
5d2be4 |
ldap/servers/slapd/filter.c | 21 +++++++
|
|
|
5d2be4 |
ldap/servers/slapd/slapi-plugin.h | 1 +
|
|
|
5d2be4 |
4 files changed, 93 insertions(+), 2 deletions(-)
|
|
|
5d2be4 |
|
|
|
5d2be4 |
diff --git a/dirsrvtests/tests/suites/replication/acceptance_test.py b/dirsrvtests/tests/suites/replication/acceptance_test.py
|
|
|
5d2be4 |
index 5009f4e7c..661dddb11 100644
|
|
|
5d2be4 |
--- a/dirsrvtests/tests/suites/replication/acceptance_test.py
|
|
|
5d2be4 |
+++ b/dirsrvtests/tests/suites/replication/acceptance_test.py
|
|
|
5d2be4 |
@@ -7,6 +7,7 @@
|
|
|
5d2be4 |
# --- END COPYRIGHT BLOCK ---
|
|
|
5d2be4 |
#
|
|
|
5d2be4 |
import pytest
|
|
|
5d2be4 |
+import logging
|
|
|
5d2be4 |
from lib389.replica import Replicas
|
|
|
5d2be4 |
from lib389.tasks import *
|
|
|
5d2be4 |
from lib389.utils import *
|
|
|
5d2be4 |
@@ -556,6 +557,68 @@ def test_csnpurge_large_valueset(topo_m2):
|
|
|
5d2be4 |
for i in range(21,25):
|
|
|
5d2be4 |
test_user.add('description', 'value {}'.format(str(i)))
|
|
|
5d2be4 |
|
|
|
5d2be4 |
+@pytest.mark.ds51244
|
|
|
5d2be4 |
+def test_urp_trigger_substring_search(topo_m2):
|
|
|
5d2be4 |
+ """Test that a ADD of a entry with a '*' in its DN, triggers
|
|
|
5d2be4 |
+ an internal search with a escaped DN
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ :id: 9869bb39-419f-42c3-a44b-c93eb0b77667
|
|
|
5d2be4 |
+ :setup: MMR with 2 masters
|
|
|
5d2be4 |
+ :steps:
|
|
|
5d2be4 |
+ 1. enable internal operation loggging for plugins
|
|
|
5d2be4 |
+ 2. Create on M1 a test_user with a '*' in its DN
|
|
|
5d2be4 |
+ 3. Check the test_user is replicated
|
|
|
5d2be4 |
+ 4. Check in access logs that the internal search does not contain '*'
|
|
|
5d2be4 |
+ :expectedresults:
|
|
|
5d2be4 |
+ 1. Should succeeds
|
|
|
5d2be4 |
+ 2. Should succeeds
|
|
|
5d2be4 |
+ 3. Should succeeds
|
|
|
5d2be4 |
+ 4. Should succeeds
|
|
|
5d2be4 |
+ """
|
|
|
5d2be4 |
+ m1 = topo_m2.ms["master1"]
|
|
|
5d2be4 |
+ m2 = topo_m2.ms["master2"]
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ # Enable loggging of internal operation logging to capture URP intop
|
|
|
5d2be4 |
+ log.info('Set nsslapd-plugin-logging to on')
|
|
|
5d2be4 |
+ for inst in (m1, m2):
|
|
|
5d2be4 |
+ inst.config.loglevel([AccessLog.DEFAULT, AccessLog.INTERNAL], service='access')
|
|
|
5d2be4 |
+ inst.config.set('nsslapd-plugin-logging', 'on')
|
|
|
5d2be4 |
+ inst.restart()
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ # add a user with a DN containing '*'
|
|
|
5d2be4 |
+ test_asterisk_uid = 'asterisk_*_in_value'
|
|
|
5d2be4 |
+ test_asterisk_dn = 'uid={},{}'.format(test_asterisk_uid, DEFAULT_SUFFIX)
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ test_user = UserAccount(m1, test_asterisk_dn)
|
|
|
5d2be4 |
+ if test_user.exists():
|
|
|
5d2be4 |
+ log.info('Deleting entry {}'.format(test_asterisk_dn))
|
|
|
5d2be4 |
+ test_user.delete()
|
|
|
5d2be4 |
+ test_user.create(properties={
|
|
|
5d2be4 |
+ 'uid': test_asterisk_uid,
|
|
|
5d2be4 |
+ 'cn': test_asterisk_uid,
|
|
|
5d2be4 |
+ 'sn': test_asterisk_uid,
|
|
|
5d2be4 |
+ 'userPassword': test_asterisk_uid,
|
|
|
5d2be4 |
+ 'uidNumber' : '1000',
|
|
|
5d2be4 |
+ 'gidNumber' : '2000',
|
|
|
5d2be4 |
+ 'homeDirectory' : '/home/asterisk',
|
|
|
5d2be4 |
+ })
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ # check that the ADD was replicated on M2
|
|
|
5d2be4 |
+ test_user_m2 = UserAccount(m2, test_asterisk_dn)
|
|
|
5d2be4 |
+ for i in range(1,5):
|
|
|
5d2be4 |
+ if test_user_m2.exists():
|
|
|
5d2be4 |
+ break
|
|
|
5d2be4 |
+ else:
|
|
|
5d2be4 |
+ log.info('Entry not yet replicated on M2, wait a bit')
|
|
|
5d2be4 |
+ time.sleep(2)
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ # check that M2 access logs does not "(&(objectclass=nstombstone)(nscpentrydn=uid=asterisk_*_in_value,dc=example,dc=com))"
|
|
|
5d2be4 |
+ log.info('Check that on M2, URP as not triggered such internal search')
|
|
|
5d2be4 |
+ pattern = ".*\(Internal\).*SRCH.*\(&\(objectclass=nstombstone\)\(nscpentrydn=uid=asterisk_\*_in_value,dc=example,dc=com.*"
|
|
|
5d2be4 |
+ found = m2.ds_access_log.match(pattern)
|
|
|
5d2be4 |
+ log.info("found line: %s" % found)
|
|
|
5d2be4 |
+ assert not found
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
|
|
|
5d2be4 |
if __name__ == '__main__':
|
|
|
5d2be4 |
# Run isolated
|
|
|
5d2be4 |
diff --git a/ldap/servers/plugins/replication/urp.c b/ldap/servers/plugins/replication/urp.c
|
|
|
5d2be4 |
index 79a817c90..301e9fa00 100644
|
|
|
5d2be4 |
--- a/ldap/servers/plugins/replication/urp.c
|
|
|
5d2be4 |
+++ b/ldap/servers/plugins/replication/urp.c
|
|
|
5d2be4 |
@@ -1411,9 +1411,12 @@ urp_add_check_tombstone (Slapi_PBlock *pb, char *sessionid, Slapi_Entry *entry,
|
|
|
5d2be4 |
Slapi_Entry **entries = NULL;
|
|
|
5d2be4 |
Slapi_PBlock *newpb;
|
|
|
5d2be4 |
char *basedn = slapi_entry_get_ndn(entry);
|
|
|
5d2be4 |
+ char *escaped_basedn;
|
|
|
5d2be4 |
const Slapi_DN *suffix = slapi_get_suffix_by_dn(slapi_entry_get_sdn (entry));
|
|
|
5d2be4 |
+ escaped_basedn = slapi_filter_escape_filter_value("nscpentrydn", basedn);
|
|
|
5d2be4 |
|
|
|
5d2be4 |
- char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", basedn);
|
|
|
5d2be4 |
+ char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", escaped_basedn);
|
|
|
5d2be4 |
+ slapi_ch_free((void **)&escaped_basedn);
|
|
|
5d2be4 |
newpb = slapi_pblock_new();
|
|
|
5d2be4 |
slapi_search_internal_set_pb(newpb,
|
|
|
5d2be4 |
slapi_sdn_get_dn(suffix), /* Base DN */
|
|
|
5d2be4 |
@@ -1602,12 +1605,15 @@ urp_find_tombstone_for_glue (Slapi_PBlock *pb, char *sessionid, const Slapi_Entr
|
|
|
5d2be4 |
Slapi_Entry **entries = NULL;
|
|
|
5d2be4 |
Slapi_PBlock *newpb;
|
|
|
5d2be4 |
const char *basedn = slapi_sdn_get_dn(parentdn);
|
|
|
5d2be4 |
+ char *escaped_basedn;
|
|
|
5d2be4 |
+ escaped_basedn = slapi_filter_escape_filter_value("nscpentrydn", basedn);
|
|
|
5d2be4 |
|
|
|
5d2be4 |
char *conflict_csnstr = (char*)slapi_entry_attr_get_ref((Slapi_Entry *)entry, "conflictcsn");
|
|
|
5d2be4 |
CSN *conflict_csn = csn_new_by_string(conflict_csnstr);
|
|
|
5d2be4 |
CSN *tombstone_csn = NULL;
|
|
|
5d2be4 |
|
|
|
5d2be4 |
- char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", basedn);
|
|
|
5d2be4 |
+ char *filter = slapi_filter_sprintf("(&(objectclass=nstombstone)(nscpentrydn=%s))", escaped_basedn);
|
|
|
5d2be4 |
+ slapi_ch_free((void **)&escaped_basedn);
|
|
|
5d2be4 |
newpb = slapi_pblock_new();
|
|
|
5d2be4 |
char *parent_dn = slapi_dn_parent (basedn);
|
|
|
5d2be4 |
slapi_search_internal_set_pb(newpb,
|
|
|
5d2be4 |
diff --git a/ldap/servers/slapd/filter.c b/ldap/servers/slapd/filter.c
|
|
|
5d2be4 |
index c818baec3..d671c87ff 100644
|
|
|
5d2be4 |
--- a/ldap/servers/slapd/filter.c
|
|
|
5d2be4 |
+++ b/ldap/servers/slapd/filter.c
|
|
|
5d2be4 |
@@ -130,6 +130,27 @@ filter_escape_filter_value(struct slapi_filter *f, const char *fmt, size_t len _
|
|
|
5d2be4 |
return ptr;
|
|
|
5d2be4 |
}
|
|
|
5d2be4 |
|
|
|
5d2be4 |
+/* Escaped an equality filter value (assertionValue) of a given attribute
|
|
|
5d2be4 |
+ * Caller must free allocated escaped filter value
|
|
|
5d2be4 |
+ */
|
|
|
5d2be4 |
+char *
|
|
|
5d2be4 |
+slapi_filter_escape_filter_value(char* filter_attr, char *filter_value)
|
|
|
5d2be4 |
+{
|
|
|
5d2be4 |
+ char *result;
|
|
|
5d2be4 |
+ struct slapi_filter *f;
|
|
|
5d2be4 |
+
|
|
|
5d2be4 |
+ if ((filter_attr == NULL) || (filter_value == NULL)) {
|
|
|
5d2be4 |
+ return NULL;
|
|
|
5d2be4 |
+ }
|
|
|
5d2be4 |
+ f = (struct slapi_filter *)slapi_ch_calloc(1, sizeof(struct slapi_filter));
|
|
|
5d2be4 |
+ f->f_choice = LDAP_FILTER_EQUALITY;
|
|
|
5d2be4 |
+ f->f_un.f_un_ava.ava_type = filter_attr;
|
|
|
5d2be4 |
+ f->f_un.f_un_ava.ava_value.bv_len = strlen(filter_value);
|
|
|
5d2be4 |
+ f->f_un.f_un_ava.ava_value.bv_val = filter_value;
|
|
|
5d2be4 |
+ result = filter_escape_filter_value(f, FILTER_EQ_FMT, FILTER_EQ_LEN);
|
|
|
5d2be4 |
+ slapi_ch_free((void**) &f);
|
|
|
5d2be4 |
+ return result;
|
|
|
5d2be4 |
+}
|
|
|
5d2be4 |
|
|
|
5d2be4 |
/*
|
|
|
5d2be4 |
* get_filter_internal(): extract an LDAP filter from a BerElement and create
|
|
|
5d2be4 |
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
|
|
|
5d2be4 |
index 8d9c3fa6a..04c02cf7c 100644
|
|
|
5d2be4 |
--- a/ldap/servers/slapd/slapi-plugin.h
|
|
|
5d2be4 |
+++ b/ldap/servers/slapd/slapi-plugin.h
|
|
|
5d2be4 |
@@ -5262,6 +5262,7 @@ int slapi_vattr_filter_test_ext(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Filter *
|
|
|
5d2be4 |
int slapi_filter_compare(struct slapi_filter *f1, struct slapi_filter *f2);
|
|
|
5d2be4 |
Slapi_Filter *slapi_filter_dup(Slapi_Filter *f);
|
|
|
5d2be4 |
int slapi_filter_changetype(Slapi_Filter *f, const char *newtype);
|
|
|
5d2be4 |
+char *slapi_filter_escape_filter_value(char* filter_attr, char *filter_value);
|
|
|
5d2be4 |
|
|
|
5d2be4 |
int slapi_attr_is_last_mod(char *attr);
|
|
|
5d2be4 |
|
|
|
5d2be4 |
--
|
|
|
5d2be4 |
2.26.2
|
|
|
5d2be4 |
|