[389-ds-base] branch 389-ds-base-1.4.3 updated: Issue 51192 - Add option to reject internal unindexed searches

This is an automated email from the git hooks/post-receive script.

mreynolds pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.

The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
     new df99fbd  Issue 51192 - Add option to reject internal unindexed searches
df99fbd is described below

commit df99fbdb37e02d56f20d3b0bf412f89731aff469
Author: Mark Reynolds <mreynolds(a)redhat.com&gt;
AuthorDate: Mon Jun 29 17:43:07 2020 -0400

    Issue 51192 - Add option to reject internal unindexed searches
    
    Bug Description:  Some plugins can perform unindexed searches, and under the
                      right conditions this can cause problems like exhausting DB locks.
                      The setting "nsslapd-require-index" does not apply to
internal
                      searches, so there is no way to prevent these searches from
                      occuring.
    
    Fix Description:  Add a new database setting
"nsslapd-require-internalop-index"
                      that rejects an internal unindexed searches.
    
                      Also found during testing that when the RI plugin fails that
                      it does not set the proper result error code.
    
    relates: https://pagure.io/389-ds-base/issue/51192
    
    Reviewed by: firstyear, spichugi & tbordaz (Thanks!!!)
---
 dirsrvtests/tests/suites/config/config_test.py     | 97 +++++++++++++++++++++-
 ldap/servers/plugins/referint/referint.c           | 24 ++++--
 ldap/servers/slapd/back-ldbm/back-ldbm.h           |  1 +
 ldap/servers/slapd/back-ldbm/ldbm_config.h         |  1 +
 .../servers/slapd/back-ldbm/ldbm_instance_config.c | 27 ++++++
 ldap/servers/slapd/back-ldbm/ldbm_search.c         | 28 +++----
 6 files changed, 153 insertions(+), 25 deletions(-)

diff --git a/dirsrvtests/tests/suites/config/config_test.py
b/dirsrvtests/tests/suites/config/config_test.py
index 567059b..38d1ed9 100644
--- a/dirsrvtests/tests/suites/config/config_test.py
+++ b/dirsrvtests/tests/suites/config/config_test.py
@@ -1,5 +1,5 @@
 # --- BEGIN COPYRIGHT BLOCK ---
-# Copyright (C) 2016 Red Hat, Inc.
+# Copyright (C) 2020 Red Hat, Inc.
 # All rights reserved.
 #
 # License: GPL (version 3 or any later version).
@@ -12,13 +12,15 @@ import pytest
 from lib389.tasks import *
 from lib389.topologies import topology_m2, topology_st as topo
 from lib389.utils import *
-from lib389._constants import DN_CONFIG, DEFAULT_SUFFIX
+from lib389._constants import DN_CONFIG, DEFAULT_SUFFIX, DEFAULT_BENAME
 from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
+from lib389.idm.group import Groups
 from lib389.backend import *
 from lib389.config import LDBMConfig, BDB_LDBMConfig
 from lib389.cos import CosPointerDefinitions, CosTemplates
 from lib389.backend import Backends
 from lib389.monitor import MonitorLDBM
+from lib389.plugins import ReferentialIntegrityPlugin
 
 pytestmark = pytest.mark.tier0
 
@@ -458,6 +460,97 @@ def test_ndn_cache_enabled(topo):
         topo.standalone.config.set('nsslapd-ndn-cache-max-size',
'invalid_value')
 
 
+def test_require_index(topo):
+    """Test nsslapd-ignore-virtual-attrs configuration attribute
+
+    :id: fb6e31f2-acc2-4e75-a195-5c356faeb803
+    :setup: Standalone instance
+    :steps:
+        1. Set "nsslapd-require-index" to "on"
+        2. Test an unindexed search is rejected
+    :expectedresults:
+        1. Success
+        2. Success
+    """
+
+    # Set the config
+    be_insts = Backends(topo.standalone).list()
+    for be in be_insts:
+        if be.get_attr_val_utf8_l('nsslapd-suffix') == DEFAULT_SUFFIX:
+            be.set('nsslapd-require-index', 'on')
+
+    db_cfg = DatabaseConfig(topo.standalone)
+    db_cfg.set([('nsslapd-idlistscanlimit', '100')])
+
+    users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)
+    for i in range(101):
+        users.create_test_user(uid=i)
+
+    # Issue unindexed search,a nd make sure it is rejected
+    raw_objects = DSLdapObjects(topo.standalone, basedn=DEFAULT_SUFFIX)
+    with pytest.raises(ldap.UNWILLING_TO_PERFORM):
+        raw_objects.filter("(description=test*)")
+
+
+
+(a)pytest.mark.skipif(ds_is_older(&#39;1.4.2&#39;), reason="The config setting only
exists in 1.4.2 and higher")
+def test_require_internal_index(topo):
+    """Test nsslapd-ignore-virtual-attrs configuration attribute
+
+    :id: 22b94f30-59e3-4f27-89a1-c4f4be036f7f
+    :setup: Standalone instance
+    :steps:
+        1. Set "nsslapd-require-internalop-index" to "on"
+        2. Enable RI plugin, and configure it to use an attribute that is not indexed
+        3. Create a user and add it a group
+        4. Deleting user should be rejected as the RI plugin issues an
+        unindexed internal search
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+    """
+    # Set the config
+    be_insts = Backends(topo.standalone).list()
+    for be in be_insts:
+        if be.get_attr_val_utf8_l('nsslapd-suffix') == DEFAULT_SUFFIX:
+            be.set('nsslapd-require-index', 'off')
+            be.set('nsslapd-require-internalop-index', 'on')
+
+    # Configure RI plugin
+    rip = ReferentialIntegrityPlugin(topo.standalone)
+    rip.set('referint-membership-attr', 'description')
+    rip.enable()
+
+    # Create a bunch of users
+    db_cfg = DatabaseConfig(topo.standalone)
+    db_cfg.set([('nsslapd-idlistscanlimit', '100')])
+    users = UserAccounts(topo.standalone, DEFAULT_SUFFIX)
+    for i in range(102, 202):
+        users.create_test_user(uid=i)
+
+    # Create user and group
+    user = users.create(properties={
+        'uid': 'indexuser',
+        'cn' : 'indexuser',
+        'sn' : 'user',
+        'uidNumber' : '1010',
+        'gidNumber' : '2010',
+        'homeDirectory' : '/home/indexuser'
+    })
+    groups = Groups(topo.standalone, DEFAULT_SUFFIX)
+    group = groups.create(properties={'cn': 'group',
+                                      'member': user.dn})
+
+    # Restart the server
+    topo.standalone.restart()
+
+    # Deletion of user should be rejected
+    with pytest.raises(ldap.UNWILLING_TO_PERFORM):
+        user.delete()
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/plugins/referint/referint.c
b/ldap/servers/plugins/referint/referint.c
index 7319976..eb4b089 100644
--- a/ldap/servers/plugins/referint/referint.c
+++ b/ldap/servers/plugins/referint/referint.c
@@ -49,7 +49,7 @@ int referint_postop_del(Slapi_PBlock *pb);
 int referint_postop_modrdn(Slapi_PBlock *pb);
 int referint_postop_start(Slapi_PBlock *pb);
 int referint_postop_close(Slapi_PBlock *pb);
-int update_integrity(Slapi_DN *sDN, char *newrDN, Slapi_DN *newsuperior);
+int update_integrity(Slapi_DN *sDN, char *newrDN, Slapi_DN *newsuperior, Slapi_PBlock
*pb);
 int GetNextLine(char *dest, int size_dest, PRFileDesc *stream);
 int my_fgetc(PRFileDesc *stream);
 void referint_thread_func(void *arg);
@@ -611,7 +611,7 @@ referint_postop_del(Slapi_PBlock *pb)
     } else if (delay == 0) { /* no delay */
         /* call function to update references to entry */
         if (referint_sdn_in_entry_scope(sdn)) {
-            rc = update_integrity(sdn, NULL, NULL);
+            rc = update_integrity(sdn, NULL, NULL, pb);
         }
     } else {
         /* write the entry to integrity log */
@@ -663,7 +663,7 @@ referint_postop_modrdn(Slapi_PBlock *pb)
         /* call function to update references to entry */
         if (!plugin_EntryScope && !plugin_ExcludeEntryScope) {
             /* no scope defined, default always process referint */
-            rc = update_integrity(sdn, newrdn, newsuperior);
+            rc = update_integrity(sdn, newrdn, newsuperior, pb);
         } else {
             const char *newsuperiordn = slapi_sdn_get_dn(newsuperior);
             if ((newsuperiordn == NULL && referint_sdn_in_entry_scope(sdn)) ||
@@ -672,10 +672,10 @@ referint_postop_modrdn(Slapi_PBlock *pb)
                  * It is a modrdn inside the scope or into the scope,
                  * process normal modrdn
                  */
-                rc = update_integrity(sdn, newrdn, newsuperior);
+                rc = update_integrity(sdn, newrdn, newsuperior, pb);
             } else if (referint_sdn_in_entry_scope(sdn)) {
                 /* the entry is moved out of scope, treat as delete */
-                rc = update_integrity(sdn, NULL, NULL);
+                rc = update_integrity(sdn, NULL, NULL, pb);
             }
         }
     } else {
@@ -1067,7 +1067,8 @@ bail:
 int
 update_integrity(Slapi_DN *origSDN,
                  char *newrDN,
-                 Slapi_DN *newsuperior)
+                 Slapi_DN *newsuperior,
+                 Slapi_PBlock *pb)
 {
     Slapi_PBlock *search_result_pb = NULL;
     Slapi_PBlock *mod_pb = slapi_pblock_new();
@@ -1181,6 +1182,10 @@ update_integrity(Slapi_DN *origSDN,
                                          * We're using backend transactions,
                                          * so we need to stop on failure.
                                          */
+                                        if (pb) {
+                                            /* Set the error code of the failure */
+                                            slapi_pblock_set(pb, SLAPI_RESULT_CODE,
&rc);
+                                        }
                                         rc = SLAPI_PLUGIN_FAILURE;
                                         goto free_and_return;
                                     } else {
@@ -1196,8 +1201,11 @@ update_integrity(Slapi_DN *origSDN,
                                       "update_integrity - Search (base=%s filter=%s)
returned "
                                       "error %d\n",
                                       search_base, filter, search_result);
-                        rc = SLAPI_PLUGIN_FAILURE;
                         slapi_free_search_results_internal(search_result_pb);
+                        if (pb) {
+                            slapi_pblock_set(pb, SLAPI_RESULT_CODE, &search_result);
+                        }
+                        rc = SLAPI_PLUGIN_FAILURE;
                         goto free_and_return;
                     }
                 }
@@ -1432,7 +1440,7 @@ referint_thread_func(void *arg __attribute__((unused)))
                 }
             }
 
-            update_integrity(sdn, tmprdn, tmpsuperior);
+            update_integrity(sdn, tmprdn, tmpsuperior, NULL);
 
             slapi_sdn_free(&sdn);
             slapi_ch_free_string(&tmprdn);
diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h
b/ldap/servers/slapd/back-ldbm/back-ldbm.h
index 9d78ad7..e325d25 100644
--- a/ldap/servers/slapd/back-ldbm/back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h
@@ -755,6 +755,7 @@ typedef struct ldbm_instance
     char *inst_dataversion;          /* The user data version tag.  Used by replication.
*/
     void *inst_db;                   /* implementation specific instance data */
     int require_index;               /* set to 1 to require an index be used in search
*/
+    int require_internalop_index;    /* set to 1 to require an index be used in an
internal search */
     struct cache inst_dncache;       /* The dn cache for this instance. */
 } ldbm_instance;
 
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_config.h
b/ldap/servers/slapd/back-ldbm/ldbm_config.h
index fdf8f6e..58e6479 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_config.h
+++ b/ldap/servers/slapd/back-ldbm/ldbm_config.h
@@ -135,6 +135,7 @@ struct config_info
 #define CONFIG_INSTANCE_DIR "nsslapd-directory"
 
 #define CONFIG_INSTANCE_REQUIRE_INDEX "nsslapd-require-index"
+#define CONFIG_INSTANCE_REQUIRE_INTERNALOP_INDEX
"nsslapd-require-internalop-index"
 
 #define CONFIG_USE_LEGACY_ERRORCODE "nsslapd-do-not-use-vlv-error"
 
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
index 628ac90..36ec112 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_instance_config.c
@@ -247,6 +247,14 @@ ldbm_instance_config_require_index_get(void *arg)
     return (void *)((uintptr_t)inst->require_index);
 }
 
+static void *
+ldbm_instance_config_require_internalop_index_get(void *arg)
+{
+    ldbm_instance *inst = (ldbm_instance *)arg;
+
+    return (void *)((uintptr_t)inst->require_internalop_index);
+}
+
 static int
 ldbm_instance_config_readonly_set(void *arg,
                                   void *value,
@@ -299,6 +307,24 @@ ldbm_instance_config_require_index_set(void *arg,
 }
 
 
+static int
+ldbm_instance_config_require_internalop_index_set(void *arg,
+                                                  void *value,
+                                                  char *errorbuf
__attribute__((unused)),
+                                                  int phase __attribute__((unused)),
+                                                  int apply)
+{
+    ldbm_instance *inst = (ldbm_instance *)arg;
+
+    if (!apply) {
+        return LDAP_SUCCESS;
+    }
+
+    inst->require_internalop_index = (int)((uintptr_t)value);
+
+    return LDAP_SUCCESS;
+}
+
 /*------------------------------------------------------------------------
  * ldbm instance configuration array
  *----------------------------------------------------------------------*/
@@ -307,6 +333,7 @@ static config_info ldbm_instance_config[] = {
     {CONFIG_INSTANCE_CACHEMEMSIZE, CONFIG_TYPE_UINT64, DEFAULT_CACHE_SIZE_STR,
&ldbm_instance_config_cachememsize_get, &ldbm_instance_config_cachememsize_set,
CONFIG_FLAG_ALWAYS_SHOW | CONFIG_FLAG_ALLOW_RUNNING_CHANGE},
     {CONFIG_INSTANCE_READONLY, CONFIG_TYPE_ONOFF, "off",
&ldbm_instance_config_readonly_get, &ldbm_instance_config_readonly_set,
CONFIG_FLAG_ALWAYS_SHOW | CONFIG_FLAG_ALLOW_RUNNING_CHANGE},
     {CONFIG_INSTANCE_REQUIRE_INDEX, CONFIG_TYPE_ONOFF, "off",
&ldbm_instance_config_require_index_get, &ldbm_instance_config_require_index_set,
CONFIG_FLAG_ALWAYS_SHOW | CONFIG_FLAG_ALLOW_RUNNING_CHANGE},
+	{CONFIG_INSTANCE_REQUIRE_INTERNALOP_INDEX, CONFIG_TYPE_ONOFF, "off",
&ldbm_instance_config_require_internalop_index_get,
&ldbm_instance_config_require_internalop_index_set, CONFIG_FLAG_ALWAYS_SHOW |
CONFIG_FLAG_ALLOW_RUNNING_CHANGE},
     {CONFIG_INSTANCE_DNCACHEMEMSIZE, CONFIG_TYPE_UINT64, DEFAULT_DNCACHE_SIZE_STR,
&ldbm_instance_config_dncachememsize_get,
&ldbm_instance_config_dncachememsize_set, CONFIG_FLAG_ALWAYS_SHOW |
CONFIG_FLAG_ALLOW_RUNNING_CHANGE},
     {NULL, 0, NULL, NULL, NULL, 0}};
 
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c
b/ldap/servers/slapd/back-ldbm/ldbm_search.c
index 7235714..1a7b510 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_search.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c
@@ -834,6 +834,7 @@ ldbm_back_search(Slapi_PBlock *pb)
     if (NULL != candidates && ALLIDS(candidates)) {
         unsigned int opnote;
         int ri = 0;
+        int rii = 0;
         int pr_idx = -1;
         Connection *pb_conn = NULL;
         Operation *pb_op = NULL;
@@ -849,23 +850,20 @@ ldbm_back_search(Slapi_PBlock *pb)
         int32_t op_nested_count;
 
         /*
-         * Return error if nsslapd-require-index is set and
-         * this is not an internal operation.
-         * We hope the plugins know what they are doing!
+         * Return error if require index is set
          */
-        if (!internal_op) {
-
-            PR_Lock(inst->inst_config_mutex);
-            ri = inst->require_index;
-            PR_Unlock(inst->inst_config_mutex);
-
-            if (ri) {
-                idl_free(&candidates);
-                candidates = idl_alloc(0);
-                tmp_err = LDAP_UNWILLING_TO_PERFORM;
-                tmp_desc = "Search is not indexed";
-            }
+        PR_Lock(inst->inst_config_mutex);
+        ri = inst->require_index;
+        rii = inst->require_internalop_index;
+        PR_Unlock(inst->inst_config_mutex);
+
+        if ((internal_op && rii) || (!internal_op && ri)) {
+            idl_free(&candidates);
+            candidates = idl_alloc(0);
+            tmp_err = LDAP_UNWILLING_TO_PERFORM;
+            tmp_desc = "Search is not indexed";
         }
+
         /*
          * When an search is fully unindexed we need to log the
          * details as these kinds of searches can cause issues with bdb db

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.