From 63e1ceac74cdfda7cf432537a18670e9562b58df Mon Sep 17 00:00:00 2001

From: progier389 <progier@redhat.com>

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 <sasl/sasl.h>

+

+/* 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)