From 7b7217538908ae58df864ef5cd82e1d3303c189f Mon Sep 17 00:00:00 2001 From: Mark Reynolds 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