From 70fd6e1fa6667734f39146cef53de6e3ff22d765 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
Date: Fri, 9 Nov 2018 17:07:11 +0100
Subject: [PATCH] Ticket 50020 - during MODRDN referential integrity can fail
erronously while updating large groups
Bug Description:
During a MODRDN of a group member, referential integrity will update the groups containing this member.
Under specific conditions, the MODRDN can fail (err=1).
on MODRDN Referential integrity checks if the original DN of the target MODRDN entry is
member of a given group. If it is then it updates the group.
The returned code of the group update is using the variable 'rc'.
It does a normalized DN comparison to compare original DN with members DN, to determine if
a group needs to be updated.
If the group does not need to be updated, 'rc' is not set.
The bug is that it uses 'rc' to normalize the DN and if the group is not updated
the returned code reflects the normalization returned code rather that the group update.
The bug is hit in specific conditions
One of the evaluated group contains more than 128 members
the last member (last value) of the group is not the moved entry
the last member (last value) of the group is a DN value that contains escaped chars
Fix Description:
Use a local variable to check the result of the DN normalization
https://pagure.io/389-ds-base/issue/50020
Reviewed by: Simon Pichugin, Mark Reynolds (thanks)
Platforms tested: F27
Flag Day: no
---
.../tests/suites/plugins/referint_test.py | 103 ++++++++++++++++++
ldap/servers/plugins/referint/referint.c | 18 +--
2 files changed, 113 insertions(+), 8 deletions(-)
create mode 100644 dirsrvtests/tests/suites/plugins/referint_test.py
diff --git a/dirsrvtests/tests/suites/plugins/referint_test.py b/dirsrvtests/tests/suites/plugins/referint_test.py
new file mode 100644
index 000000000..67a11de9e
--- /dev/null
+++ b/dirsrvtests/tests/suites/plugins/referint_test.py
@@ -0,0 +1,103 @@
+# --- BEGIN COPYRIGHT BLOCK ---
+# Copyright (C) 2016 Red Hat, Inc.
+# All rights reserved.
+#
+# License: GPL (version 3 or any later version).
+# See LICENSE for details.
+# --- END COPYRIGHT BLOCK ---
+#
+'''
+Created on Dec 12, 2019
+
+@author: tbordaz
+'''
+import logging
+import subprocess
+import pytest
+from lib389 import Entry
+from lib389.utils import *
+from lib389.plugins import *
+from lib389._constants import *
+from lib389.idm.user import UserAccounts, UserAccount
+from lib389.idm.group import Groups
+from lib389.topologies import topology_st as topo
+
+log = logging.getLogger(__name__)
+
+ESCAPED_RDN_BASE = "foo\,oo"
+def _user_get_dn(no):
+ uid = '%s%d' % (ESCAPED_RDN_BASE, no)
+ dn = 'uid=%s,%s' % (uid, SUFFIX)
+ return (uid, dn)
+
+def add_escaped_user(server, no):
+ (uid, dn) = _user_get_dn(no)
+ log.fatal('Adding user (%s): ' % dn)
+ server.add_s(Entry((dn, {'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson'],
+ 'uid': [uid],
+ 'sn' : [uid],
+ 'cn' : [uid]})))
+ return dn
+
+@pytest.mark.ds50020
+def test_referential_false_failure(topo):
+ """On MODRDN referential integrity can erronously fail
+
+ :id: f77aeb80-c4c4-471b-8c1b-4733b714778b
+ :setup: Standalone Instance
+ :steps:
+ 1. Configure the plugin
+ 2. Create a group
+ - 1rst member the one that will be move
+ - more than 128 members
+ - last member is a DN containing escaped char
+ 3. Rename the 1rst member
+ :expectedresults:
+ 1. should succeed
+ 2. should succeed
+ 3. should succeed
+ """
+
+ inst = topo[0]
+
+ # stop the plugin, and start it
+ plugin = ReferentialIntegrityPlugin(inst)
+ plugin.disable()
+ plugin.enable()
+
+ ############################################################################
+ # Configure plugin
+ ############################################################################
+ GROUP_CONTAINER = "ou=groups,%s" % DEFAULT_SUFFIX
+ plugin.replace('referint-membership-attr', 'member')
+ plugin.replace('nsslapd-plugincontainerscope', GROUP_CONTAINER)
+
+ ############################################################################
+ # Creates a group with members having escaped DN
+ ############################################################################
+ # Add some users and a group
+ users = UserAccounts(inst, DEFAULT_SUFFIX, None)
+ user1 = users.create_test_user(uid=1001)
+ user2 = users.create_test_user(uid=1002)
+
+ groups = Groups(inst, GROUP_CONTAINER, None)
+ group = groups.create(properties={'cn': 'group'})
+ group.add('member', user2.dn)
+ group.add('member', user1.dn)
+
+ # Add more than 128 members so that referint follows the buggy path
+ for i in range(130):
+ escaped_user = add_escaped_user(inst, i)
+ group.add('member', escaped_user)
+
+ ############################################################################
+ # Check that the MODRDN succeeds
+ ###########################################################################
+ # Here we need to restart so that member values are taken in the right order
+ # the last value is the escaped one
+ inst.restart()
+
+ # Here if the bug is fixed, referential is able to update the member value
+ inst.rename_s(user1.dn, 'uid=new_test_user_1001', newsuperior=SUFFIX, delold=0)
+
+
diff --git a/ldap/servers/plugins/referint/referint.c b/ldap/servers/plugins/referint/referint.c
index f6d1c27a2..9e4e680d3 100644
--- a/ldap/servers/plugins/referint/referint.c
+++ b/ldap/servers/plugins/referint/referint.c
@@ -824,20 +824,21 @@ _update_one_per_mod(Slapi_DN *entrySDN, /* DN of the searched entry */
*/
for (nval = slapi_attr_first_value(attr, &v); nval != -1;
nval = slapi_attr_next_value(attr, nval, &v)) {
+ int normalize_rc;
p = NULL;
dnlen = 0;
/* DN syntax, which should be a string */
sval = slapi_ch_strdup(slapi_value_get_string(v));
- rc = slapi_dn_normalize_case_ext(sval, 0, &p, &dnlen);
- if (rc == 0) { /* sval is passed in; not terminated */
+ normalize_rc = slapi_dn_normalize_case_ext(sval, 0, &p, &dnlen);
+ if (normalize_rc == 0) { /* sval is passed in; not terminated */
*(p + dnlen) = '\0';
sval = p;
- } else if (rc > 0) {
+ } else if (normalize_rc > 0) {
slapi_ch_free_string(&sval);
sval = p;
}
- /* else: (rc < 0) Ignore the DN normalization error for now. */
+ /* else: (normalize_rc < 0) Ignore the DN normalization error for now. */
p = PL_strstr(sval, slapi_sdn_get_ndn(origDN));
if (p == sval) {
@@ -1013,20 +1014,21 @@ _update_all_per_mod(Slapi_DN *entrySDN, /* DN of the searched entry */
for (nval = slapi_attr_first_value(attr, &v);
nval != -1;
nval = slapi_attr_next_value(attr, nval, &v)) {
+ int normalize_rc;
p = NULL;
dnlen = 0;
/* DN syntax, which should be a string */
sval = slapi_ch_strdup(slapi_value_get_string(v));
- rc = slapi_dn_normalize_case_ext(sval, 0, &p, &dnlen);
- if (rc == 0) { /* sval is passed in; not terminated */
+ normalize_rc = slapi_dn_normalize_case_ext(sval, 0, &p, &dnlen);
+ if (normalize_rc == 0) { /* sval is passed in; not terminated */
*(p + dnlen) = '\0';
sval = p;
- } else if (rc > 0) {
+ } else if (normalize_rc > 0) {
slapi_ch_free_string(&sval);
sval = p;
}
- /* else: (rc < 0) Ignore the DN normalization error for now. */
+ /* else: normalize_rc < 0) Ignore the DN normalization error for now. */
p = PL_strstr(sval, slapi_sdn_get_ndn(origDN));
if (p == sval) {
--
2.17.2