Issue 6004 - idletimeout may be ignored (#6005)

* Issue 6004 - idletimeout may be ignored

Problem: idletimeout is still not handled when binding as non root (unless there are some activity
on another connection)
Fix:
Add a slapi_eq_repeat_rel handler that walks all active connection every seconds and check if the timeout is expired.
Note about CI test:
Notice that idletimeout is never enforced for connections bound as root (i.e cn=directory manager).

Issue #6004

Reviewed by: @droideck, @tbordaz (Thanks!)

dirsrvtests/tests/suites/config/regression_test.py

@@ -6,20 +6,49 @@
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 #
+import os
 import logging
 import pytest
+import time
 from lib389.utils import *
 from lib389.dseldif import DSEldif
-from lib389.config import BDB_LDBMConfig, LDBMConfig
+from lib389.config import BDB_LDBMConfig, LDBMConfig, Config
 from lib389.backend import Backends
 from lib389.topologies import topology_st as topo
+from lib389.idm.user import UserAccounts, TEST_USER_PROPERTIES
+from lib389._constants import DEFAULT_SUFFIX, PASSWORD, DN_DM
 
 pytestmark = pytest.mark.tier0
 
 logging.getLogger(__name__).setLevel(logging.INFO)
 log = logging.getLogger(__name__)
 
+DEBUGGING = os.getenv("DEBUGGING", default=False)
 CUSTOM_MEM = '9100100100'
+IDLETIMEOUT = 5
+DN_TEST_USER = f'uid={TEST_USER_PROPERTIES["uid"]},ou=People,{DEFAULT_SUFFIX}'
+
+
+@pytest.fixture(scope="module")
+def idletimeout_topo(topo, request):
+    """Create an instance with a test user and set idletimeout"""
+    inst = topo.standalone
+    config = Config(inst)
+
+    users = UserAccounts(inst, DEFAULT_SUFFIX)
+    user = users.create(properties={
+        **TEST_USER_PROPERTIES,
+        'userpassword' : PASSWORD,
+    })
+    config.replace('nsslapd-idletimeout', str(IDLETIMEOUT))
+
+    def fin():
+        if not DEBUGGING:
+            config.reset('nsslapd-idletimeout')
+            user.delete()
+
+    request.addfinalizer(fin)
+    return topo
 
 
 # Function to return value of available memory in kb
@@ -79,7 +108,7 @@ def test_maxbersize_repl(topo):
     nsslapd-errorlog-logmaxdiskspace are set in certain order
 
     :id: 743e912c-2be4-4f5f-9c2a-93dcb18f51a0
-    :setup: MMR with two suppliers
+    :setup: Standalone Instance
     :steps:
         1. Stop the instance
         2. Set nsslapd-errorlog-maxlogsize before/after
@@ -117,7 +146,7 @@ def test_bdb_config(topo):
     """Check that bdb config entry exists
 
     :id: edbc6f54-7c98-11ee-b1c0-482ae39447e5
-    :setup: MMR with two suppliers
+    :setup: standalone
     :steps:
         1. Check that bdb config instance exists.
     :expectedresults:
@@ -126,3 +155,38 @@ def test_bdb_config(topo):
 
     inst = topo.standalone
     assert BDB_LDBMConfig(inst).exists()
+
+
+@pytest.mark.parametrize("dn,expected_result", [(DN_TEST_USER, True), (DN_DM, False)])
+def test_idletimeout(idletimeout_topo, dn, expected_result):
+    """Check that bdb config entry exists
+
+    :id: b20f2826-942a-11ee-827b-482ae39447e5
+    :parametrized: yes
+    :setup: Standalone Instance with test user and idletimeout
+    :steps:
+        1. Open new ldap connection
+        2. Bind with the provided dn
+        3. Wait longer than idletimeout
+        4. Try to bind again the provided dn and check if
+           connection is closed or not.
+        5. Check if result is the expected one.
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+        5. Success
+    """
+
+    inst = idletimeout_topo.standalone
+
+    l = ldap.initialize(f'ldap://localhost:{inst.port}')
+    l.bind_s(dn, PASSWORD)
+    time.sleep(IDLETIMEOUT+1)
+    try:
+        l.bind_s(dn, PASSWORD)
+        result = False
+    except ldap.SERVER_DOWN:
+        result = True
+    assert expected_result == result

ldap/servers/slapd/daemon.c

@@ -64,6 +64,8 @@
 #define SLAPD_ACCEPT_WAKEUP_TIMER 250
 #endif
 
+#define MILLISECONDS_PER_SECOND 1000
+
 int slapd_wakeup_timer = SLAPD_WAKEUP_TIMER; /* time in ms to wakeup */
 int slapd_accept_wakeup_timer = SLAPD_ACCEPT_WAKEUP_TIMER; /* time in ms to wakeup */
 int slapd_ct_thread_wakeup_timer = SLAPD_WAKEUP_TIMER; /* time in ms to wakeup */
@@ -918,6 +920,48 @@ slapd_sockets_ports_free(daemon_ports_t *ports_info)
 #endif
 }
 
+/*
+ * Tells if idle timeout has expired
+ */
+static inline int __attribute__((always_inline))
+has_idletimeout_expired(Connection *c, time_t curtime)
+{
+    return (c->c_state != CONN_STATE_FREE && !c->c_gettingber &&
+            c->c_idletimeout > 0 && NULL == c->c_ops &&
+            curtime - c->c_idlesince >= c->c_idletimeout);
+}
+
+/*
+ * slapi_eq_repeat_rel callback that checks that idletimeout has not expired.
+ */
+void
+check_idletimeout(time_t when __attribute__((unused)), void *arg __attribute__((unused)) )
+{
+    Connection_Table *ct = the_connection_table;
+    time_t curtime = slapi_current_rel_time_t();
+    /* Walk all active connections of all connection listeners */
+    for (int list_num = 0; list_num < ct->list_num; list_num++) {
+        for (Connection *c = connection_table_get_first_active_connection(ct, list_num);
+             c != NULL; c = connection_table_get_next_active_connection(ct, c)) {
+            if (!has_idletimeout_expired(c, curtime)) {
+                continue;
+            }
+            /* Looks like idletimeout has expired, lets acquire the lock
+             * and double check.
+             */
+            if (pthread_mutex_trylock(&(c->c_mutex)) == EBUSY) {
+                continue;
+            }
+            if (has_idletimeout_expired(c, curtime)) {
+                /* idle timeout has expired */
+                disconnect_server_nomutex(c, c->c_connid, -1,
+                                          SLAPD_DISCONNECT_IDLE_TIMEOUT, ETIMEDOUT);
+            }
+            pthread_mutex_unlock(&(c->c_mutex));
+        }
+    }
+}
+
 void
 slapd_daemon(daemon_ports_t *ports)
 {
@@ -1123,6 +1167,9 @@ slapd_daemon(daemon_ports_t *ports)
                   "MAINPID=%lu",
                   (unsigned long)getpid());
 #endif
+    slapi_eq_repeat_rel(check_idletimeout, NULL,
+                        slapi_current_rel_time_t(),
+                        MILLISECONDS_PER_SECOND);
     /* The meat of the operation is in a loop on a call to select */
     while (!g_get_shutdown()) {
 
@@ -1645,9 +1692,7 @@ handle_pr_read_ready(Connection_Table *ct, int list_num, PRIntn num_poll __attri
                         disconnect_server_nomutex(c, c->c_connid, -1,
                                                   SLAPD_DISCONNECT_POLL, EPIPE);
                     }
-                } else if (c->c_idletimeout > 0 &&
-                           (curtime - c->c_idlesince) >= c->c_idletimeout &&
-                           NULL == c->c_ops) {
+                } else if (has_idletimeout_expired(c, curtime)) {
                     /* idle timeout */
                     disconnect_server_nomutex(c, c->c_connid, -1,
                                               SLAPD_DISCONNECT_IDLE_TIMEOUT, ETIMEDOUT);