389-ds-base

Clone
Source Code
GIT

#50021 Ticket 50020 - during MODRDN referential integrity can fail erronously while updating large groups
Closed 3 years ago by spichugi. Opened 5 years ago by tbordaz.
tbordaz/389-ds-base ticket_50020  into  master

file added
+103 
@@ -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)
 

+ 

+

file modified
+10 -8 
@@ -824,20 +824,21 @@ 
 

           */
 

          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 @@ 
 

          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) {

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: ?

Platforms tested: F27

Flag Day: no

LGTM and tested

ack too

rebased onto 5284dce
5 years ago

Pull-Request has been merged by tbordaz
5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3080

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 2
+103
file added
dirsrvtests/tests/suites/plugins/referint_test.py
+10 -8
file changed
ldap/servers/plugins/referint/referint.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.