sbus: terminate ongoing chained requests if backend is restarted

If there is an outgoing request already chained and backend is restarted, a new outgoing request is chained but not processed, it waits for a timeout. This patch makes sure that all outgoing requests are gracefully terminated if backend restarts. Resolves: #7503 Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Justin Stephenson <[email protected]>
SSSD · Oct 9, 2024 · 263cb2e · 263cb2e
1 parent e0ec488
commit 263cb2e
Show file tree

Hide file tree

Showing 8 changed files with 138 additions and 4 deletions.
diff --git a/src/sbus/connection/sbus_connection.c b/src/sbus/connection/sbus_connection.c
@@ -471,3 +471,20 @@ void sbus_connection_free(struct sbus_connection *conn)
               conn);
     }
 }
+
+void
+sbus_connection_terminate_member_requests(struct sbus_connection *conn,
+                                          const char *member)
+{
+    DEBUG(SSSDBG_TRACE_FUNC, "Terminating outgoing chained requests for: %s\n",
+          member);
+
+    sbus_requests_terminate_member(conn->requests->outgoing, member,
+                                   ERR_TERMINATED);
+
+    DEBUG(SSSDBG_TRACE_FUNC, "Terminating incoming chained requests from: %s\n",
+          member);
+
+    sbus_requests_terminate_member(conn->requests->incoming, member,
+                                   ERR_TERMINATED);
+}
diff --git a/src/sbus/connection/sbus_dispatcher.c b/src/sbus/connection/sbus_dispatcher.c
@@ -38,6 +38,7 @@ sbus_dispatch_reconnect(struct sbus_connection *conn)
     /* Terminate all outgoing requests associated with this connection. */
     DEBUG(SSSDBG_TRACE_FUNC, "Connection lost. Terminating active requests.\n");
     sbus_requests_terminate_all(conn->requests->outgoing, ERR_TERMINATED);
+    sbus_requests_terminate_all(conn->requests->incoming, ERR_TERMINATED);
 
     switch (conn->type) {
     case SBUS_CONNECTION_CLIENT:

diff --git a/src/sbus/interface/sbus_std_signals.c b/src/sbus/interface/sbus_std_signals.c
@@ -37,6 +37,11 @@ sbus_name_owner_changed(TALLOC_CTX *mem_ctx,
     /* Delete any existing sender information since it is now obsolete. */
     sbus_senders_delete(conn->senders, name);
 
+    /* Terminate active request if the owner has disconnected. */
+    if (new_owner == NULL || new_owner[0] == '\0') {
+        sbus_connection_terminate_member_requests(sbus_req->conn, old_owner);
+    }
+
     return EOK;
 }
 

diff --git a/src/sbus/request/sbus_request.c b/src/sbus/request/sbus_request.c
@@ -449,6 +449,7 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
     DBusMessageIter *write_iter = NULL;
     struct sbus_sender *sender;
     struct tevent_req *req;
+    const char *member = NULL;
     bool key_exists;
     errno_t ret;
 
@@ -463,6 +464,9 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
     }
 
     state->request->sender = talloc_steal(state->request, sender);
+    if (sender != NULL) {
+        member = sender->name;
+    }
 
     ret = sbus_check_access(state->conn, state->request);
     if (ret != EOK) {
@@ -500,7 +504,7 @@ static void sbus_incoming_request_sender_done(struct tevent_req *subreq)
      * set a tevent callback that is triggered when the method handler is done.
      */
     ret = sbus_requests_add(state->conn->requests->incoming, state->key,
-                            state->conn, req, true, &key_exists);
+                            state->conn, req, member, true, &key_exists);
     if (ret != EOK || key_exists) {
         /* Cancel the sub request. Since there was either an error or the
          * sub request was chained. */
@@ -617,6 +621,7 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
     struct sbus_outgoing_request_state *state;
     struct tevent_req *subreq;
     struct tevent_req *req;
+    const char *destination;
     bool key_exists;
     errno_t ret;
 
@@ -645,6 +650,8 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
         }
     }
 
+    destination = dbus_message_get_destination(msg);
+
     /**
      * We will search table to see if the same request is not already
      * in progress. If it is, we register ourselves for notification
@@ -654,7 +661,7 @@ sbus_outgoing_request_send(TALLOC_CTX *mem_ctx,
      * set a tevent callback that is triggered when the method handler is done.
      */
     ret = sbus_requests_add(conn->requests->outgoing, key,
-                            conn, req, true, &key_exists);
+                            conn, req, destination, true, &key_exists);
     if (ret != EOK) {
         goto done;
     }
@@ -776,7 +783,7 @@ sbus_request_await_send(TALLOC_CTX *mem_ctx,
 
     /* Otherwise attach to this request. */
     ret = sbus_requests_add(conn->requests->outgoing, key, conn,
-                            req, false, NULL);
+                            req, member, false, NULL);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to attach to the request list "
               "[%d]: %s\n", ret, sss_strerror(ret));

diff --git a/src/sbus/request/sbus_request_hash.c b/src/sbus/request/sbus_request_hash.c
@@ -118,6 +118,7 @@ sbus_requests_add(hash_table_t *table,
                   const char *key,
                   struct sbus_connection *conn,
                   struct tevent_req *req,
+                  const char *member,
                   bool is_dbus,
                   bool *_key_exists)
 {
@@ -150,6 +151,11 @@ sbus_requests_add(hash_table_t *table,
     item->req = req;
     item->conn = conn;
     item->is_dbus = is_dbus;
+    item->member = talloc_strdup(item, member);
+    if (member != NULL && item->member == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
 
     ret = sbus_requests_attach_spies(item);
     if (ret != EOK) {
@@ -323,3 +329,37 @@ sbus_requests_terminate_all(hash_table_t *table,
 
     talloc_free(values);
 }
+
+void
+sbus_requests_terminate_member(hash_table_t *table,
+                               const char *member,
+                               errno_t error)
+{
+    struct sbus_request_list *list;
+    struct sbus_request_list *item;
+    hash_value_t *values;
+    unsigned long int num;
+    unsigned long int i;
+    int hret;
+
+    hret = hash_values(table, &num, &values);
+    if (hret != HASH_SUCCESS) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get list of active requests "
+              "[%d]: %s\n", hret, hash_error_string(hret));
+        return;
+    }
+
+    for (i = 0; i < num; i++) {
+        list = sss_ptr_get_value(&values[i], struct sbus_request_list);
+        if ((member == NULL && list->member == NULL)
+            || (member != NULL && list->member != NULL && strcmp(member, list->member) == 0)) {
+            DLIST_FOR_EACH(item, list) {
+                sbus_requests_finish(item, error);
+            }
+        }
+
+        sbus_requests_delete(list);
+    }
+
+    talloc_free(values);
+}
diff --git a/src/sbus/sbus.h b/src/sbus/sbus.h
@@ -336,6 +336,16 @@ errno_t
 sbus_connection_add_path_map(struct sbus_connection *conn,
                              struct sbus_path *map);
 
+/**
+ * Terminate all outgoing requests for given member.
+ *
+ * @param conn      An sbus connection.
+ * @param member D-Bus member name (destination)
+ */
+void
+sbus_connection_terminate_member_requests(struct sbus_connection *conn,
+                                          const char *member);
+
 /**
  * Add new signal listener to the router.
  *

diff --git a/src/sbus/sbus_private.h b/src/sbus/sbus_private.h
@@ -431,6 +431,9 @@ struct sbus_request_list {
     struct tevent_req *req;
     struct sbus_connection *conn;
 
+    /* Member part of the key. Destination for outgoing, sender for incoming.*/
+    const char *member;
+
     bool is_invalid;
     bool is_dbus;
 
@@ -462,6 +465,7 @@ sbus_requests_add(hash_table_t *table,
                   const char *key,
                   struct sbus_connection *conn,
                   struct tevent_req *req,
+                  const char *member,
                   bool is_dbus,
                   bool *_key_exists);
 
@@ -484,6 +488,12 @@ void
 sbus_requests_terminate_all(hash_table_t *table,
                             errno_t error);
 
+/* Terminate requests associated with given member. */
+void
+sbus_requests_terminate_member(hash_table_t *table,
+                               const char *member,
+                               errno_t error);
+
 /* Create new sbus request. */
 struct sbus_request *
 sbus_request_create(TALLOC_CTX *mem_ctx,

diff --git a/src/tests/system/tests/test_identity.py b/src/tests/system/tests/test_identity.py
@@ -12,7 +12,8 @@
 from sssd_test_framework.roles.client import Client
 from sssd_test_framework.roles.generic import GenericADProvider, GenericProvider
 from sssd_test_framework.roles.ipa import IPA
-from sssd_test_framework.topology import KnownTopologyGroup
+from sssd_test_framework.roles.ldap import LDAP
+from sssd_test_framework.topology import KnownTopology, KnownTopologyGroup
 
 
 @pytest.mark.importance("critical")
@@ -681,3 +682,46 @@ def test_identity__lookup_when_auto_private_groups_is_set_to_hybrid(client: Clie
     assert result is not None, "User 'user_group_gid' not found!"
     assert result.gid == 555555, "gid does not match expected value!"
     assert client.tools.getent.group(555555) is not None, "auto private group not found!"
+
+
+@pytest.mark.importance("low")
+@pytest.mark.topology(KnownTopology.LDAP)
+def test_identity__lookup_when_backend_restarts(client: Client, ldap: LDAP):
+    """
+    :title: Look up user when backend is restarted with previous lookup unfinished
+    :description:
+        If there is an active lookup for a user and the backend is restarted
+        before this lookup is finished, the next lookup of the same user after
+        the restart must not timeout.
+    :setup:
+        1. Add a user "tuser"
+        2. Start SSSD
+    :steps:
+        1. Add 10s network traffic delay to the LDAP host
+        2. Lookup "tuser" asynchronously
+        3. Kill sssd_be with SIGKILL so it is restarted
+        4. Remove the network traffic delay
+        5. Lookup of "tuser" must yield the user and not timeout
+    :expectedresults:
+        1. Network traffic is delayed
+        2. Lookup hangs, does not finish and waits for a timeout
+        3. The backend process is restarted
+        4. Network traffic is no longer delayed
+        5. User lookup returns the user immediately
+    :customerscenario: False
+    """
+    ldap.user("tuser").add()
+
+    client.sssd.start()
+
+    # Add a delay so the next lookup will hang
+    client.tc.add_delay(ldap, "10s")
+    client.host.conn.async_run("getent passwd tuser")
+
+    # Kill backend and remove the delay
+    client.host.conn.run("kill -KILL $(pidof sssd_be)")
+    client.tc.remove_delay(ldap)
+
+    # The next lookup should not timeout
+    result = client.tools.wait_for_condition("getent passwd tuser", timeout=5)
+    assert "tuser" in result.stdout, "tuser was not found"