From 63e1ceac74cdfda7cf432537a18670e9562b58df Mon Sep 17 00:00:00 2001 From: progier389 Date: Mon, 2 May 2022 18:43:25 +0200 Subject: [PATCH] Issue 5126 - Memory leak in slapi_ldap_get_lderrno (#5153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Issue 5126 - Memory leak in slapi_ldap_get_lderrno The problem is that some time ago libldap API replaced ​LDAP_OPT_ERROR_STRING whose data should not be freed by LDAP_OPT_DIAGNOSTIC_MESSAGE whose data must be freed. slapi_ldap_get_lderrno was adapted to use the new option but the callers were not modified to free the value. The Solution: Insure that we also need to free slapi_ldap_get_lderrno value if legacy LDAP_OPT_ERROR_STRING is used (by duping the value) Insure that the callers free the value. Added test case about replication using SASL/Digest-md5 authentication Added test case to check this leak Also updated test case about SASL/GSSAPI to be comapatible with current lib389 framework but marked as skipped because it requires a specific configuration (This path should be tested by IPA tests) Fixed valgrind lib389 function to run on prefixed installation without needing to be root. At last I also improved lib389 mapped object to have a better diagnostic when LDAP operation fails (by adding the request within the exception) issue: 5126 https://github.com/389ds/389-ds-base/issues/5126 Reviewd by: @droideck (cherry picked from commit 4d89e11494233d8297896540bc752cfdbab2cc69) --- .../suites/gssapi_repl/gssapi_repl_test.py | 31 ++- .../tests/suites/replication/sasl_m2_test.py | 185 ++++++++++++++++++ ldap/servers/plugins/chainingdb/cb_search.c | 6 +- ldap/servers/plugins/passthru/ptbind.c | 2 + .../plugins/replication/repl5_connection.c | 4 + .../plugins/replication/windows_connection.c | 3 + ldap/servers/slapd/ldaputil.c | 6 + src/lib389/lib389/_mapped_object.py | 76 ++++--- src/lib389/lib389/utils.py | 40 +++- 9 files changed, 311 insertions(+), 42 deletions(-) create mode 100644 dirsrvtests/tests/suites/replication/sasl_m2_test.py diff --git a/dirsrvtests/tests/suites/gssapi_repl/gssapi_repl_test.py b/dirsrvtests/tests/suites/gssapi_repl/gssapi_repl_test.py index 41f323c06..402684aab 100644 --- a/dirsrvtests/tests/suites/gssapi_repl/gssapi_repl_test.py +++ b/dirsrvtests/tests/suites/gssapi_repl/gssapi_repl_test.py @@ -9,6 +9,7 @@ import pytest from lib389.tasks import * from lib389.utils import * +from lib389.agreement import * from lib389.topologies import topology_m2 pytestmark = pytest.mark.tier2 @@ -65,10 +66,27 @@ def _allow_machine_account(inst, name): # First we need to get the mapping tree dn mt = inst.mappingtree.list(suffix=DEFAULT_SUFFIX)[0] inst.modify_s('cn=replica,%s' % mt.dn, [ - (ldap.MOD_REPLACE, 'nsDS5ReplicaBindDN', "uid=%s,ou=Machines,%s" % (name, DEFAULT_SUFFIX)) + (ldap.MOD_REPLACE, 'nsDS5ReplicaBindDN', f"uid={name},ou=Machines,{DEFAULT_SUFFIX}".encode('utf-8')) ]) - +def _verify_etc_hosts(): + #Check if /etc/hosts is compatible with the test + NEEDED_HOSTS = ( ('ldapkdc.example.com', '127.0.0.1'), + ('ldapkdc1.example.com', '127.0.1.1'), + ('ldapkdc2.example.com', '127.0.2.1')) + found_hosts = {} + with open('/etc/hosts','r') as f: + for l in f: + s = l.split() + if len(s) < 2: + continue + for nh in NEEDED_HOSTS: + if (s[0] == nh[1] and s[1] == nh[0]): + found_hosts[s[1]] = True + return len(found_hosts) == len(NEEDED_HOSTS) + +@pytest.mark.skipif(not _verify_etc_hosts(), reason="/etc/hosts does not contains the needed hosts.") +@pytest.mark.skipif(True, reason="Test disabled because it requires specific kerberos requirement (server principal, keytab, etc ...") def test_gssapi_repl(topology_m2): """Test gssapi authenticated replication agreement of two suppliers using KDC @@ -94,8 +112,6 @@ def test_gssapi_repl(topology_m2): 6. Test User should be created on M1 and M2 both 7. Test User should be created on M1 and M2 both """ - - return supplier1 = topology_m2.ms["supplier1"] supplier2 = topology_m2.ms["supplier2"] @@ -121,6 +137,7 @@ def test_gssapi_repl(topology_m2): properties = {RA_NAME: r'meTo_$host:$port', RA_METHOD: 'SASL/GSSAPI', RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + supplier1.agreement.delete(suffix=SUFFIX, consumer_host=supplier2.host, consumer_port=supplier2.port) m1_m2_agmt = supplier1.agreement.create(suffix=SUFFIX, host=supplier2.host, port=supplier2.port, properties=properties) if not m1_m2_agmt: log.fatal("Fail to create a supplier -> supplier replica agreement") @@ -133,6 +150,7 @@ def test_gssapi_repl(topology_m2): properties = {RA_NAME: r'meTo_$host:$port', RA_METHOD: 'SASL/GSSAPI', RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + supplier2.agreement.delete(suffix=SUFFIX, consumer_host=supplier1.host, consumer_port=supplier1.port) m2_m1_agmt = supplier2.agreement.create(suffix=SUFFIX, host=supplier1.host, port=supplier1.port, properties=properties) if not m2_m1_agmt: log.fatal("Fail to create a supplier -> supplier replica agreement") @@ -145,8 +163,9 @@ def test_gssapi_repl(topology_m2): # # Initialize all the agreements # - supplier1.agreement.init(SUFFIX, HOST_SUPPLIER_2, PORT_SUPPLIER_2) - supplier1.waitForReplInit(m1_m2_agmt) + agmt = Agreement(supplier1, m1_m2_agmt) + agmt.begin_reinit() + agmt.wait_reinit() # Check replication is working... if supplier1.testReplication(DEFAULT_SUFFIX, supplier2): diff --git a/dirsrvtests/tests/suites/replication/sasl_m2_test.py b/dirsrvtests/tests/suites/replication/sasl_m2_test.py new file mode 100644 index 000000000..d7406ac7e --- /dev/null +++ b/dirsrvtests/tests/suites/replication/sasl_m2_test.py @@ -0,0 +1,185 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2022 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import logging +import os +import pytest +import ldap +import uuid +from lib389.utils import ds_is_older, valgrind_enable, valgrind_disable, valgrind_get_results_file, valgrind_check_file + +from lib389.idm.services import ServiceAccounts +from lib389.idm.group import Groups +from lib389.config import CertmapLegacy, Config +from lib389._constants import DEFAULT_SUFFIX +from lib389.agreement import Agreements +from lib389._mapped_object import DSLdapObject +from lib389.replica import ReplicationManager, Replicas, BootstrapReplicationManager +from lib389.topologies import topology_m2 as topo_m2 + +pytestmark = pytest.mark.tier1 + +DEBUGGING = os.getenv("DEBUGGING", default=False) +if DEBUGGING: + logging.getLogger(__name__).setLevel(logging.DEBUG) +else: + logging.getLogger(__name__).setLevel(logging.INFO) +log = logging.getLogger(__name__) + +def set_sasl_md5_client_auth(inst, to): + # Create the certmap before we restart + cm = CertmapLegacy(to) + certmaps = cm.list() + certmaps['default']['nsSaslMapRegexString'] = '^dn:\\(.*\\)' + certmaps['default']['nsSaslMapBaseDNTemplate'] = 'cn=config' + certmaps['default']['nsSaslMapFilterTemplate'] = '(objectclass=*)' + cm.set(certmaps) + + Config(to).replace("passwordStorageScheme", 'CLEAR') + + # Create a repl manager on the replica + replication_manager_pwd = 'secret12' + brm = BootstrapReplicationManager(to) + try: + brm.delete() + except ldap.NO_SUCH_OBJECT: + pass + brm.create(properties={ + 'cn': brm.common_name, + 'userPassword': replication_manager_pwd + }) + replication_manager_dn = brm.dn + + replica = Replicas(inst).get(DEFAULT_SUFFIX) + replica.set('nsDS5ReplicaBindDN', brm.dn) + replica.remove_all('nsDS5ReplicaBindDNgroup') + agmt = replica.get_agreements().list()[0] + agmt.replace_many( + ('nsDS5ReplicaBindMethod', 'SASL/DIGEST-MD5'), + ('nsDS5ReplicaTransportInfo', 'LDAP'), + ('nsDS5ReplicaPort', str(to.port)), + ('nsDS5ReplicaBindDN', replication_manager_dn), + ('nsDS5ReplicaCredentials', replication_manager_pwd), + ) + + +def gen_valgrind_wrapper(dir): + name=f"{dir}/VALGRIND" + with open(name, 'w') as f: + f.write('#!/bin/sh\n') + f.write('export SASL_PATH=foo\n') + f.write(f'valgrind -q --tool=memcheck --leak-check=yes --leak-resolution=high --num-callers=50 --log-file=/var/tmp/slapd.vg.$$ {dir}/ns-slapd.original "$@"\n') + os.chmod(name, 0o755) + return name + +@pytest.fixture +def use_valgrind(topo_m2, request): + """Adds entries to the supplier1""" + + log.info("Enable valgrind") + m1 = topo_m2.ms['supplier1'] + m2 = topo_m2.ms['supplier2'] + if m1.has_asan(): + pytest.skip('Tescase using valgring cannot run on asan enabled build') + return + set_sasl_md5_client_auth(m1, m2) + set_sasl_md5_client_auth(m2, m1) + m1.stop() + m2.stop() + m1.systemd_override = False + m2.systemd_override = False + valgrind_enable(m1.ds_paths.sbin_dir, gen_valgrind_wrapper(m1.ds_paths.sbin_dir)) + + def fin(): + log.info("Disable valgrind") + valgrind_disable(m1.ds_paths.sbin_dir) + + request.addfinalizer(fin) + + +def test_repl_sasl_md5_auth(topo_m2): + """Test replication with SASL digest-md5 authentication + + :id: 922d16f8-662a-4915-a39e-0aecd7c8e6e2 + :setup: Two supplier replication + :steps: + 1. Set sasl digest/md4 on both suppliers + 2. Restart the instance + 3. Check that replication works + :expectedresults: + 1. Success + 2. Success + 3. Replication works + """ + + m1 = topo_m2.ms['supplier1'] + m2 = topo_m2.ms['supplier2'] + + set_sasl_md5_client_auth(m1, m2) + set_sasl_md5_client_auth(m2, m1) + + m1.restart() + m2.restart() + + repl = ReplicationManager(DEFAULT_SUFFIX) + repl.test_replication_topology(topo_m2) + + +@pytest.mark.skipif(not os.path.exists('/usr/bin/valgrind'), reason="valgrind is not installed.") +def test_repl_sasl_leak(topo_m2, use_valgrind): + """Test replication with SASL digest-md5 authentication + + :id: 180e088e-841c-11ec-af4f-482ae39447e5 + :setup: Two supplier replication, valgrind + :steps: + 1. Set sasl digest/md4 on both suppliers + 2. Break sasl by setting invalid PATH + 3. Restart the instances + 4. Perform a change + 5. Poke replication 100 times + 6. Stop server + 7. Check presence of "SASL(-4): no mechanism available: No worthy mechs found" message in error log + 8 Check that there is no leak about slapi_ldap_get_lderrno + :expectedresults: + 1. Success + 2. Success + 2. Success + 4. Success + 5. Success + 6. Success + 7. Success + 8. Success + """ + + m1 = topo_m2.ms['supplier1'] + m2 = topo_m2.ms['supplier2'] + + os.environ["SASL_PATH"] = 'foo' + + m1.start() + m2.start() + + resfile=valgrind_get_results_file(m1) + + # Perform a change + from_groups = Groups(m1, basedn=DEFAULT_SUFFIX, rdn=None) + from_group = from_groups.get('replication_managers') + change = str(uuid.uuid4()) + from_group.replace('description', change) + + # Poke replication to trigger thev leak + replica = Replicas(m1).get(DEFAULT_SUFFIX) + agmt = Agreements(m1, replica.dn).list()[0] + for i in range(0, 100): + agmt.pause() + agmt.resume() + + m1.stop() + assert m1.searchErrorsLog("worthy") + assert not valgrind_check_file(resfile, 'slapi_ldap_get_lderrno'); + diff --git a/ldap/servers/plugins/chainingdb/cb_search.c b/ldap/servers/plugins/chainingdb/cb_search.c index ffc8f56f8..d6f30b357 100644 --- a/ldap/servers/plugins/chainingdb/cb_search.c +++ b/ldap/servers/plugins/chainingdb/cb_search.c @@ -348,10 +348,9 @@ chainingdb_build_candidate_list(Slapi_PBlock *pb) warned_rc = 1; } cb_send_ldap_result(pb, rc, NULL, ENDUSERMSG, 0, NULL); - /* BEWARE: matched_msg and error_msg points */ + /* BEWARE: matched_msg points */ /* to ld fields. */ matched_msg = NULL; - error_msg = NULL; rc = -1; } @@ -695,10 +694,9 @@ chainingdb_next_search_entry(Slapi_PBlock *pb) } cb_send_ldap_result(pb, rc, matched_msg, ENDUSERMSG, 0, NULL); - /* BEWARE: Don't free matched_msg && error_msg */ + /* BEWARE: Don't free matched_msg */ /* Points to the ld fields */ matched_msg = NULL; - error_msg = NULL; retcode = -1; } else { /* Add control response sent by the farm server */ diff --git a/ldap/servers/plugins/passthru/ptbind.c b/ldap/servers/plugins/passthru/ptbind.c index 705ab2c3a..3e79b47f6 100644 --- a/ldap/servers/plugins/passthru/ptbind.c +++ b/ldap/servers/plugins/passthru/ptbind.c @@ -33,6 +33,8 @@ passthru_simple_bind_once_s(PassThruServer *srvr, const char *dn, struct berval * are only interested in recovering silently when the remote server is up * but decided to close our connection, we retry without pausing between * attempts. + * + * Note that errmsgp must be freed by the caller. */ int passthru_simple_bind_s(Slapi_PBlock *pb, PassThruServer *srvr, int tries, const char *dn, struct berval *creds, LDAPControl **reqctrls, int *lderrnop, char **matcheddnp, char **errmsgp, struct berval ***refurlsp, LDAPControl ***resctrlsp) diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c index 2dd74f9e7..b6bc21c46 100644 --- a/ldap/servers/plugins/replication/repl5_connection.c +++ b/ldap/servers/plugins/replication/repl5_connection.c @@ -244,6 +244,7 @@ conn_delete_internal(Repl_Connection *conn) PR_ASSERT(NULL != conn); close_connection_internal(conn); /* slapi_ch_free accepts NULL pointer */ + slapi_ch_free_string(&conn->last_ldap_errmsg); slapi_ch_free((void **)&conn->hostname); slapi_ch_free((void **)&conn->binddn); slapi_ch_free((void **)&conn->plain); @@ -450,6 +451,7 @@ conn_read_result_ex(Repl_Connection *conn, char **retoidp, struct berval **retda char *s = NULL; rc = slapi_ldap_get_lderrno(conn->ld, NULL, &s); + slapi_ch_free_string(&conn->last_ldap_errmsg); conn->last_ldap_errmsg = s; conn->last_ldap_error = rc; /* some errors will require a disconnect and retry the connection @@ -1937,6 +1939,7 @@ bind_and_check_pwp(Repl_Connection *conn, char *binddn, char *password) agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errmsg ? errmsg : ""); + slapi_ch_free_string(&errmsg); } else { char *errmsg = NULL; /* errmsg is a pointer directly into the ld structure - do not free */ @@ -1946,6 +1949,7 @@ bind_and_check_pwp(Repl_Connection *conn, char *binddn, char *password) agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errmsg ? errmsg : ""); + slapi_ch_free_string(&errmsg); } return (CONN_OPERATION_FAILED); diff --git a/ldap/servers/plugins/replication/windows_connection.c b/ldap/servers/plugins/replication/windows_connection.c index 5eca5fad1..d3f6a4e93 100644 --- a/ldap/servers/plugins/replication/windows_connection.c +++ b/ldap/servers/plugins/replication/windows_connection.c @@ -331,6 +331,7 @@ windows_perform_operation(Repl_Connection *conn, int optype, const char *dn, LDA "windows_perform_operation - %s: Received error %d: %s for %s operation\n", agmt_get_long_name(conn->agmt), rc, s ? s : "NULL", op_string); + slapi_ch_free_string(&s); conn->last_ldap_error = rc; /* some errors will require a disconnect and retry the connection later */ @@ -1709,6 +1710,7 @@ bind_and_check_pwp(Repl_Connection *conn, char *binddn, char *password) agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errmsg); + slapi_ch_free_string(&errmsg); } else { char *errmsg = NULL; /* errmsg is a pointer directly into the ld structure - do not free */ @@ -1718,6 +1720,7 @@ bind_and_check_pwp(Repl_Connection *conn, char *binddn, char *password) agmt_get_long_name(conn->agmt), mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errmsg); + slapi_ch_free_string(&errmsg); } slapi_log_err(SLAPI_LOG_TRACE, windows_repl_plugin_name, "<= bind_and_check_pwp - CONN_OPERATION_FAILED\n"); diff --git a/ldap/servers/slapd/ldaputil.c b/ldap/servers/slapd/ldaputil.c index 336ca3912..db3300e30 100644 --- a/ldap/servers/slapd/ldaputil.c +++ b/ldap/servers/slapd/ldaputil.c @@ -375,6 +375,8 @@ slapi_ldap_url_parse(const char *url, LDAPURLDesc **ludpp, int require_dn, int * #include + +/* Warning: caller must free s (if not NULL) */ int slapi_ldap_get_lderrno(LDAP *ld, char **m, char **s) { @@ -389,6 +391,9 @@ slapi_ldap_get_lderrno(LDAP *ld, char **m, char **s) ldap_get_option(ld, LDAP_OPT_DIAGNOSTIC_MESSAGE, s); #else ldap_get_option(ld, LDAP_OPT_ERROR_STRING, s); + if (*s) { + *s = slapi_ch_strdup(*s); + } #endif } return rc; @@ -1517,6 +1522,7 @@ slapd_ldap_sasl_interactive_bind( mech ? mech : "SIMPLE", rc, ldap_err2string(rc), errmsg, errno, slapd_system_strerror(errno)); + slapi_ch_free_string(&errmsg); if (can_retry_bind(ld, mech, bindid, creds, rc, errmsg)) { ; /* pass through to retry one time */ } else { diff --git a/src/lib389/lib389/_mapped_object.py b/src/lib389/lib389/_mapped_object.py index 48d3879a3..1c314322b 100644 --- a/src/lib389/lib389/_mapped_object.py +++ b/src/lib389/lib389/_mapped_object.py @@ -67,6 +67,34 @@ def _gen_filter(attrtypes, values, extra=None): return filt +# Define wrappers around the ldap operation to have a clear diagnostic +def _ldap_op_s(inst, f, fname, *args, **kwargs): + # f.__name__ says 'inner' so the wanted name is provided as argument + try: + return f(*args, **kwargs) + except ldap.LDAPError as e: + new_desc = f"{fname}({args},{kwargs}) on instance {inst.serverid}"; + if len(e.args) >= 1: + e.args[0]['ldap_request'] = new_desc + logging.getLogger().error(f"args={e.args}") + raise + +def _add_ext_s(inst, *args, **kwargs): + return _ldap_op_s(inst, inst.add_ext_s, 'add_ext_s', *args, **kwargs) + +def _modify_ext_s(inst, *args, **kwargs): + return _ldap_op_s(inst, inst.modify_ext_s, 'modify_ext_s', *args, **kwargs) + +def _delete_ext_s(inst, *args, **kwargs): + return _ldap_op_s(inst, inst.delete_ext_s, 'delete_ext_s', *args, **kwargs) + +def _search_ext_s(inst, *args, **kwargs): + return _ldap_op_s(inst, inst.search_ext_s, 'search_ext_s', *args, **kwargs) + +def _search_s(inst, *args, **kwargs): + return _ldap_op_s(inst, inst.search_s, 'search_s', *args, **kwargs) + + class DSLogging(object): """The benefit of this is automatic name detection, and correct application of level and verbosity to the object. @@ -129,7 +157,7 @@ class DSLdapObject(DSLogging, DSLint): :returns: Entry object """ - return self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*"], + return _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] @@ -140,7 +168,7 @@ class DSLdapObject(DSLogging, DSLint): """ try: - self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, + _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') except ldap.NO_SUCH_OBJECT: @@ -156,7 +184,7 @@ class DSLdapObject(DSLogging, DSLint): search_scope = ldap.SCOPE_ONE elif scope == 'subtree': search_scope = ldap.SCOPE_SUBTREE - return self._instance.search_ext_s(self._dn, search_scope, filter, + return _search_ext_s(self._instance,self._dn, search_scope, filter, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') @@ -166,7 +194,7 @@ class DSLdapObject(DSLogging, DSLint): :returns: LDIF formatted string """ - e = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=attrlist, + e = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=attrlist, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] return e.__repr__() @@ -258,7 +286,7 @@ class DSLdapObject(DSLogging, DSLint): raise ValueError("Invalid state. Cannot get presence on instance that is not ONLINE") self._log.debug("%s present(%r) %s" % (self._dn, attr, value)) - self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[attr, ], + _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[attr, ], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] values = self.get_attr_vals_bytes(attr) @@ -313,7 +341,7 @@ class DSLdapObject(DSLogging, DSLint): else: value = [ensure_bytes(arg[1])] mods.append((ldap.MOD_REPLACE, ensure_str(arg[0]), value)) - return self._instance.modify_ext_s(self._dn, mods, serverctrls=self._server_controls, + return _modify_ext_s(self._instance,self._dn, mods, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') # This needs to work on key + val, and key @@ -457,7 +485,7 @@ class DSLdapObject(DSLogging, DSLint): elif value is not None: value = [ensure_bytes(value)] - return self._instance.modify_ext_s(self._dn, [(action, key, value)], + return _modify_ext_s(self._instance,self._dn, [(action, key, value)], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') @@ -497,7 +525,7 @@ class DSLdapObject(DSLogging, DSLint): else: # Error too many items raise ValueError('Too many arguments in the mod op') - return self._instance.modify_ext_s(self._dn, mod_list, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + return _modify_ext_s(self._instance,self._dn, mod_list, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') def _unsafe_compare_attribute(self, other): """Compare two attributes from two objects. This is currently marked unsafe as it's @@ -593,7 +621,7 @@ class DSLdapObject(DSLogging, DSLint): raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE") else: # retrieving real(*) and operational attributes(+) - attrs_entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, + attrs_entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*", "+"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] # getting dict from 'entry' object @@ -613,7 +641,7 @@ class DSLdapObject(DSLogging, DSLint): raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE") else: # retrieving real(*) and operational attributes(+) - attrs_entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, + attrs_entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=["*", "+"], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] # getting dict from 'entry' object @@ -627,7 +655,7 @@ class DSLdapObject(DSLogging, DSLint): if self._instance.state != DIRSRV_STATE_ONLINE: raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE") else: - entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, + entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] return entry.getValuesSet(keys) @@ -636,7 +664,7 @@ class DSLdapObject(DSLogging, DSLint): self._log.debug("%s get_attrs_vals_utf8(%r)" % (self._dn, keys)) if self._instance.state != DIRSRV_STATE_ONLINE: raise ValueError("Invalid state. Cannot get properties on instance that is not ONLINE") - entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys, + entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=keys, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] vset = entry.getValuesSet(keys) @@ -655,7 +683,7 @@ class DSLdapObject(DSLogging, DSLint): else: # It would be good to prevent the entry code intercepting this .... # We have to do this in this method, because else we ignore the scope base. - entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, + entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[key], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] vals = entry.getValues(key) @@ -675,7 +703,7 @@ class DSLdapObject(DSLogging, DSLint): # In the future, I plan to add a mode where if local == true, we # can use get on dse.ldif to get values offline. else: - entry = self._instance.search_ext_s(self._dn, ldap.SCOPE_BASE, self._object_filter, + entry = _search_ext_s(self._instance,self._dn, ldap.SCOPE_BASE, self._object_filter, attrlist=[key], serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')[0] return entry.getValue(key) @@ -831,11 +859,11 @@ class DSLdapObject(DSLogging, DSLint): # Is there a way to mark this as offline and kill it if recursive: filterstr = "(|(objectclass=*)(objectclass=ldapsubentry))" - ents = self._instance.search_s(self._dn, ldap.SCOPE_SUBTREE, filterstr, escapehatch='i am sure') + ents = _search_s(self._instance, self._dn, ldap.SCOPE_SUBTREE, filterstr, escapehatch='i am sure') for ent in sorted(ents, key=lambda e: len(e.dn), reverse=True): - self._instance.delete_ext_s(ent.dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + _delete_ext_s(self._instance, ent.dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') else: - self._instance.delete_ext_s(self._dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + _delete_ext_s(self._instance, self._dn, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') def _validate(self, rdn, properties, basedn): """Used to validate a create request. @@ -933,7 +961,7 @@ class DSLdapObject(DSLogging, DSLint): # If we are running in stateful ensure mode, we need to check if the object exists, and # we can see the state that it is in. try: - self._instance.search_ext_s(dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + _search_ext_s(self._instance,dn, ldap.SCOPE_BASE, self._object_filter, attrsonly=1, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') exists = True except ldap.NO_SUCH_OBJECT: pass @@ -946,7 +974,7 @@ class DSLdapObject(DSLogging, DSLint): mods = [] for k, v in list(valid_props.items()): mods.append((ldap.MOD_REPLACE, k, v)) - self._instance.modify_ext_s(self._dn, mods, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + _modify_ext_s(self._instance,self._dn, mods, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') elif not exists: # This case is reached in two cases. One is we are in ensure mode, and we KNOW the entry # doesn't exist. @@ -957,7 +985,7 @@ class DSLdapObject(DSLogging, DSLint): e.update({'objectclass': ensure_list_bytes(self._create_objectclasses)}) e.update(valid_props) # We rely on exceptions here to indicate failure to the parent. - self._instance.add_ext_s(e, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') + _add_ext_s(self._instance, e, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure') self._log.debug('Created entry %s : %s' % (dn, display_log_data(e.data))) # If it worked, we need to fix our instance dn for the object's self reference. Because # we may not have a self reference yet (just created), it may have changed (someone @@ -1104,7 +1132,7 @@ class DSLdapObjects(DSLogging, DSLints): else: # If not paged try: - results = self._instance.search_ext_s( + results = _search_ext_s(self._instance, base=self._basedn, scope=self._scope, filterstr=filterstr, @@ -1172,7 +1200,7 @@ class DSLdapObjects(DSLogging, DSLints): filterstr = self._get_objectclass_filter() self._log.debug('_gen_dn filter = %s' % filterstr) self._log.debug('_gen_dn dn = %s' % dn) - return self._instance.search_ext_s( + return _search_ext_s(self._instance, base=dn, scope=ldap.SCOPE_BASE, filterstr=filterstr, @@ -1187,7 +1215,7 @@ class DSLdapObjects(DSLogging, DSLints): # This will yield and & filter for objectClass with as many terms as needed. filterstr = self._get_selector_filter(selector) self._log.debug('_gen_selector filter = %s' % filterstr) - return self._instance.search_ext_s( + return _search_ext_s(self._instance, base=self._basedn, scope=self._scope, filterstr=filterstr, @@ -1261,7 +1289,7 @@ class DSLdapObjects(DSLogging, DSLints): self._list_attrlist = attrlist self._log.debug(f'list filter = {search_filter} with scope {scope} and attribute list {attrlist}') try: - results = self._instance.search_ext_s( + results = _search_ext_s(self._instance, base=self._basedn, scope=scope, filterstr=search_filter, diff --git a/src/lib389/lib389/utils.py b/src/lib389/lib389/utils.py index 6eba2d7b9..da966ed97 100644 --- a/src/lib389/lib389/utils.py +++ b/src/lib389/lib389/utils.py @@ -52,7 +52,7 @@ from ldapurl import LDAPUrl from contextlib import closing import lib389 -from lib389.paths import Paths +from lib389.paths import ( Paths, DEFAULTS_PATH ) from lib389.dseldif import DSEldif from lib389._constants import ( DEFAULT_USER, VALGRIND_WRAPPER, DN_CONFIG, CFGSUFFIX, LOCALHOST, @@ -495,8 +495,10 @@ def valgrind_enable(sbin_dir, wrapper=None): :raise EnvironmentError: If script is not run as 'root' ''' - if os.geteuid() != 0: - log.error('This script must be run as root to use valgrind') + if not os.access(sbin_dir, os.W_OK): + # Note: valgrind has no limitation but ns-slapd must be replaced + # This check allows non root user to use custom install prefix + log.error('This script must be run as root to use valgrind (Should at least be able to write in {sbin_dir})') raise EnvironmentError if not wrapper: @@ -542,7 +544,20 @@ def valgrind_enable(sbin_dir, wrapper=None): e.strerror) # Disable selinux - os.system('setenforce 0') + if os.geteuid() == 0: + os.system('setenforce 0') + + # Disable systemd by turning off with_system in .inf file + old_path = Paths()._get_defaults_loc(DEFAULTS_PATH) + new_path = f'{old_path}.orig' + os.rename(old_path, new_path) + with open(new_path, 'rt') as fin: + with open(old_path, 'wt') as fout: + for line in fin: + if line.startswith('with_systemd'): + fout.write('with_systemd = 0\n') + else: + fout.write(line) log.info('Valgrind is now enabled.') @@ -559,8 +574,10 @@ def valgrind_disable(sbin_dir): :raise EnvironmentError: If script is not run as 'root' ''' - if os.geteuid() != 0: - log.error('This script must be run as root to use valgrind') + if not os.access(sbin_dir, os.W_OK): + # Note: valgrind has no limitation but ns-slapd must be replaced + # This check allows non root user to use custom install prefix + log.error('This script must be run as root to use valgrind (Should at least be able to write in {sbin_dir})') raise EnvironmentError nsslapd_orig = '%s/ns-slapd' % sbin_dir @@ -584,7 +601,14 @@ def valgrind_disable(sbin_dir): e.strerror) # Enable selinux - os.system('setenforce 1') + if os.geteuid() == 0: + os.system('setenforce 1') + + # Restore .inf file (for systemd) + new_path = Paths()._get_defaults_loc(DEFAULTS_PATH) + old_path = f'{new_path}.orig' + if os.path.exists(old_path): + os.replace(old_path, new_path) log.info('Valgrind is now disabled.') @@ -610,7 +634,7 @@ def valgrind_get_results_file(dirsrv_inst): # Run the command and grab the output p = os.popen(cmd) - results_file = p.readline() + results_file = p.readline().strip() p.close() return results_file -- 2.31.1