andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone
Blob Blame History Raw
From 7b7217538908ae58df864ef5cd82e1d3303c189f Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Mon, 7 Jun 2021 12:58:42 -0400
Subject: [PATCH] Issue 4447 - Crash when the Referential Integrity log is
 manually edited

Bug Description:  If the referint log is manually edited with a string
                  that is not a DN the server will crash when processing
                  the log.

Fix Description:  Check for NULL pointers when strtoking the file line.

relates: https://github.com/389ds/389-ds-base/issues/4447

Reviewed by: firstyear(Thanks!)
---
 .../tests/suites/plugins/referint_test.py     | 72 +++++++++++++++----
 ldap/servers/plugins/referint/referint.c      |  7 ++
 src/lib389/lib389/plugins.py                  | 15 ++++
 3 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/dirsrvtests/tests/suites/plugins/referint_test.py b/dirsrvtests/tests/suites/plugins/referint_test.py
index 02b985767..fda602545 100644
--- a/dirsrvtests/tests/suites/plugins/referint_test.py
+++ b/dirsrvtests/tests/suites/plugins/referint_test.py
@@ -1,5 +1,5 @@
 # --- BEGIN COPYRIGHT BLOCK ---
-# Copyright (C) 2016 Red Hat, Inc.
+# Copyright (C) 2021 Red Hat, Inc.
 # All rights reserved.
 #
 # License: GPL (version 3 or any later version).
@@ -12,13 +12,11 @@ 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.plugins import ReferentialIntegrityPlugin
+from lib389._constants import DEFAULT_SUFFIX
+from lib389.idm.user import UserAccounts
 from lib389.idm.group import Groups
 from lib389.topologies import topology_st as topo
 
@@ -29,21 +27,27 @@ 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)
+    dn = 'uid=%s,%s' % (uid, DEFAULT_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]})))
+    users = UserAccounts(server, DEFAULT_SUFFIX, None)
+    user_properties = {
+        'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson', 'posixAccount'],
+        'uid': uid,
+        'cn' : uid,
+        'sn' : uid,
+        'uidNumber' : '1000',
+        'gidNumber' : '2000',
+        'homeDirectory' : '/home/testuser',
+    }
+    users.create(properties=user_properties)
     return dn
 
-@pytest.mark.ds50020
 def test_referential_false_failure(topo):
-    """On MODRDN referential integrity can erronously fail
+    """On MODRDN referential integrity can erroneously fail
 
     :id: f77aeb80-c4c4-471b-8c1b-4733b714778b
     :setup: Standalone Instance
@@ -100,6 +104,46 @@ def test_referential_false_failure(topo):
     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)
+    user1.rename('uid=new_test_user_1001', newsuperior=DEFAULT_SUFFIX, deloldrdn=False)
 
 
+def test_invalid_referint_log(topo):
+    """If there is an invalid log line in the referint log, make sure the server
+    does not crash at startup
+
+    :id: 34807b5a-ab17-4281-ae48-4e3513e19145
+    :setup: Standalone Instance
+    :steps:
+        1. Set the referint log delay
+        2. Create invalid log
+        3. Start the server (no crash)
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+    """
+
+    inst = topo.standalone
+
+    # Set delay - required for log parsing at server startup
+    plugin = ReferentialIntegrityPlugin(inst)
+    plugin.enable()
+    plugin.set_update_delay('2')
+    logfile = plugin.get_log_file()
+    inst.restart()
+
+    # Create invalid log
+    inst.stop()
+    with open(logfile, 'w') as log_fh:
+        log_fh.write("CRASH\n")
+
+    # Start the instance
+    inst.start()
+    assert inst.status()
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)
diff --git a/ldap/servers/plugins/referint/referint.c b/ldap/servers/plugins/referint/referint.c
index fd5356d72..28240c1f6 100644
--- a/ldap/servers/plugins/referint/referint.c
+++ b/ldap/servers/plugins/referint/referint.c
@@ -1447,6 +1447,13 @@ referint_thread_func(void *arg __attribute__((unused)))
             sdn = slapi_sdn_new_normdn_byref(ptoken);
             ptoken = ldap_utf8strtok_r(NULL, delimiter, &iter);
 
+            if (ptoken == NULL) {
+                /* Invalid line in referint log, skip it */
+                slapi_log_err(SLAPI_LOG_ERR, REFERINT_PLUGIN_SUBSYSTEM,
+                        "Skipping invalid referint log line: (%s)\n", thisline);
+                slapi_sdn_free(&sdn);
+                continue;
+            }
             if (!strcasecmp(ptoken, "NULL")) {
                 tmprdn = NULL;
             } else {
diff --git a/src/lib389/lib389/plugins.py b/src/lib389/lib389/plugins.py
index 2d88e60bd..b07e80022 100644
--- a/src/lib389/lib389/plugins.py
+++ b/src/lib389/lib389/plugins.py
@@ -518,6 +518,21 @@ class ReferentialIntegrityPlugin(Plugin):
 
         self.set('referint-update-delay', str(value))
 
+    def get_log_file(self):
+        """Get referint log file"""
+
+        return self.get_attr_val_utf8('referint-logfile')
+
+    def get_log_file_formatted(self):
+        """Get referint log file"""
+
+        return self.display_attr('referint-logfile')
+
+    def set_log_file(self, value):
+        """Set referint log file"""
+
+        self.set('referint-logfile', value)
+
     def get_membership_attr(self, formatted=False):
         """Get referint-membership-attr attribute"""
 
-- 
2.31.1