]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] Fuzzy TCP: fix printf formats, buffer overflow, and timeout handling
authorVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 7 Oct 2025 13:53:16 +0000 (14:53 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Tue, 7 Oct 2025 13:53:16 +0000 (14:53 +0100)
- Fix printf format strings: use %ud instead of %u (rspamd printf requirement)
- Fix TCP write handler buffer overflow when writing frame header
  (ASAN error: remaining was calculated incorrectly)
- Fix timeout handling: mark commands as replied and check session completion
  to prevent task hanging on connection/request timeouts
- Fix Robot Framework test variables: add RSPAMD_ prefix for proper export
- Remove debug config dump

src/plugins/fuzzy_check.c
test/functional/cases/120_fuzzy/lib.robot

index 42bac5c49adb1ef900aa48b37836cd71b63a7d48..506f323443a739ff42fd6315d7c8969161090ca2 100644 (file)
@@ -717,6 +717,7 @@ fuzzy_tcp_connection_cleanup(struct fuzzy_tcp_connection *conn)
        gpointer key, value;
        struct fuzzy_tcp_pending_command *pending;
        GPtrArray *to_remove;
+       GHashTable *sessions_to_check;
        unsigned int i;
        struct rspamd_task *task = NULL;
 
@@ -724,14 +725,25 @@ fuzzy_tcp_connection_cleanup(struct fuzzy_tcp_connection *conn)
                return;
        }
 
-       /* Collect tags to remove (can't modify hash table during iteration) */
+       /* Collect commands to remove and sessions to check */
        to_remove = g_ptr_array_new();
+       sessions_to_check = g_hash_table_new(g_direct_hash, g_direct_equal);
 
        g_hash_table_iter_init(&iter, conn->rule->pending_requests);
        while (g_hash_table_iter_next(&iter, &key, &value)) {
                pending = (struct fuzzy_tcp_pending_command *) value;
 
                if (pending->connection == conn) {
+                       /* Mark command as replied (failed) */
+                       if (!(pending->io->flags & FUZZY_CMD_FLAG_REPLIED)) {
+                               pending->io->flags |= FUZZY_CMD_FLAG_REPLIED;
+                       }
+
+                       /* Collect session for completion check */
+                       if (pending->session) {
+                               g_hash_table_add(sessions_to_check, pending->session);
+                       }
+
                        g_ptr_array_add(to_remove, key);
 
                        /* Get task for logging */
@@ -741,7 +753,7 @@ fuzzy_tcp_connection_cleanup(struct fuzzy_tcp_connection *conn)
                }
        }
 
-       /* Remove pending commands */
+       /* Remove pending commands from hash table */
        for (i = 0; i < to_remove->len; i++) {
                g_hash_table_remove(conn->rule->pending_requests,
                                                        g_ptr_array_index(to_remove, i));
@@ -752,7 +764,16 @@ fuzzy_tcp_connection_cleanup(struct fuzzy_tcp_connection *conn)
                                          (int) to_remove->len, rspamd_upstream_name(conn->server));
        }
 
+       /* Check session completion for all affected sessions */
+       GHashTableIter session_iter;
+       struct fuzzy_client_session *session;
+       g_hash_table_iter_init(&session_iter, sessions_to_check);
+       while (g_hash_table_iter_next(&session_iter, (gpointer *) &session, NULL)) {
+               fuzzy_check_session_is_completed(session);
+       }
+
        g_ptr_array_free(to_remove, TRUE);
+       g_hash_table_unref(sessions_to_check);
 }
 
 /**
@@ -766,6 +787,7 @@ fuzzy_tcp_check_pending_timeouts(struct fuzzy_rule *rule, ev_tstamp now)
        gpointer key, value;
        struct fuzzy_tcp_pending_command *pending;
        GPtrArray *to_remove;
+       GHashTable *sessions_to_check;
        unsigned int i;
        ev_tstamp timeout;
 
@@ -775,6 +797,7 @@ fuzzy_tcp_check_pending_timeouts(struct fuzzy_rule *rule, ev_tstamp now)
 
        timeout = rule->io_timeout;
        to_remove = g_ptr_array_new();
+       sessions_to_check = g_hash_table_new(g_direct_hash, g_direct_equal);
 
        g_hash_table_iter_init(&iter, rule->pending_requests);
        while (g_hash_table_iter_next(&iter, &key, &value)) {
@@ -782,11 +805,21 @@ fuzzy_tcp_check_pending_timeouts(struct fuzzy_rule *rule, ev_tstamp now)
 
                /* Check if request timed out */
                if ((now - pending->send_time) > timeout) {
+                       /* Mark command as replied (timed out) */
+                       if (!(pending->io->flags & FUZZY_CMD_FLAG_REPLIED)) {
+                               pending->io->flags |= FUZZY_CMD_FLAG_REPLIED;
+                       }
+
+                       /* Collect session for completion check */
+                       if (pending->session) {
+                               g_hash_table_add(sessions_to_check, pending->session);
+                       }
+
                        g_ptr_array_add(to_remove, key);
 
                        if (pending->task) {
                                struct rspamd_task *task = pending->task;
-                               msg_info_task("fuzzy_tcp: request timeout after %.2fs for tag %u to %s",
+                               msg_info_task("fuzzy_tcp: request timeout after %.2fs for tag %ud to %s",
                                                          now - pending->send_time,
                                                          pending->io->tag,
                                                          rspamd_upstream_name(pending->connection->server));
@@ -800,7 +833,16 @@ fuzzy_tcp_check_pending_timeouts(struct fuzzy_rule *rule, ev_tstamp now)
                                                        g_ptr_array_index(to_remove, i));
        }
 
+       /* Check session completion for all affected sessions */
+       GHashTableIter session_iter;
+       struct fuzzy_client_session *session;
+       g_hash_table_iter_init(&session_iter, sessions_to_check);
+       while (g_hash_table_iter_next(&session_iter, (gpointer *) &session, NULL)) {
+               fuzzy_check_session_is_completed(session);
+       }
+
        g_ptr_array_free(to_remove, TRUE);
+       g_hash_table_unref(sessions_to_check);
 }
 
 /**
@@ -905,17 +947,19 @@ fuzzy_tcp_write_handler(struct fuzzy_tcp_connection *conn)
 
        while ((buf = g_queue_peek_head(conn->write_queue)) != NULL) {
                /* Write remaining data */
-               gsize remaining = buf->total_len - buf->bytes_written;
+               gsize remaining;
                unsigned char *write_ptr;
 
                /* Determine what to write: size_hdr or data */
                if (buf->bytes_written < sizeof(buf->size_hdr)) {
                        /* Still writing size header */
                        write_ptr = (unsigned char *) &buf->size_hdr + buf->bytes_written;
+                       remaining = sizeof(buf->size_hdr) - buf->bytes_written;
                }
                else {
                        /* Writing data */
                        write_ptr = buf->data + (buf->bytes_written - sizeof(buf->size_hdr));
+                       remaining = buf->total_len - buf->bytes_written;
                }
 
                r = write(conn->fd, write_ptr, remaining);
@@ -1010,7 +1054,7 @@ fuzzy_tcp_send_command(struct fuzzy_tcp_connection *conn,
                /* Mark as sent */
                io->flags |= FUZZY_CMD_FLAG_SENT;
 
-               msg_debug_fuzzy_check("fuzzy_tcp: queued command with tag %u to %s",
+               msg_debug_fuzzy_check("fuzzy_tcp: queued command with tag %ud to %s",
                                                          io->tag, rspamd_upstream_name(conn->server));
        }
 
@@ -1081,7 +1125,7 @@ fuzzy_tcp_process_reply(struct fuzzy_tcp_connection *conn,
        pending = g_hash_table_lookup(rule->pending_requests, GINT_TO_POINTER(tag));
 
        if (!pending) {
-               msg_debug("fuzzy_tcp: unexpected tag %u from %s",
+               msg_debug("fuzzy_tcp: unexpected tag %ud from %s",
                                  tag, rspamd_upstream_name(conn->server));
                return;
        }
@@ -1155,7 +1199,7 @@ fuzzy_tcp_process_reply(struct fuzzy_tcp_connection *conn,
                pending->io->flags |= FUZZY_CMD_FLAG_REPLIED;
        }
 
-       msg_debug_fuzzy_check("fuzzy_tcp: processed reply with tag %u from %s (prob=%.2f)",
+       msg_debug_fuzzy_check("fuzzy_tcp: processed reply with tag %ud from %s (prob=%.2f)",
                                                  tag, rspamd_upstream_name(conn->server), (double) rep->v1.prob);
 
        /* Remove from pending requests */
@@ -1309,6 +1353,8 @@ fuzzy_parse_rule(struct rspamd_config *cfg, const ucl_object_t *obj,
        rule->alg = RSPAMD_SHINGLES_OLD;
        rule->skip_map = NULL;
 
+       msg_debug_config("parsing fuzzy rule '%s'", name ? name : "default");
+
        if ((value = ucl_object_lookup(obj, "skip_hashes")) != NULL) {
                rspamd_map_add_from_ucl(cfg, value,
                                                                "Fuzzy hashes whitelist",
@@ -1419,6 +1465,84 @@ fuzzy_parse_rule(struct rspamd_config *cfg, const ucl_object_t *obj,
                rule->no_subject = ucl_obj_toboolean(value);
        }
 
+       /* TCP configuration */
+       if ((value = ucl_object_lookup(obj, "tcp")) != NULL) {
+               const char *tcp_mode = NULL;
+
+               msg_debug_config("rule %s: found 'tcp' option, type=%d",
+                                                name ? name : "default", ucl_object_type(value));
+
+               if (ucl_object_type(value) == UCL_BOOLEAN) {
+                       if (ucl_object_toboolean(value)) {
+                               rule->tcp_enabled = TRUE;
+                               rule->tcp_auto = FALSE;
+                       }
+               }
+               else if (ucl_object_type(value) == UCL_STRING) {
+                       tcp_mode = ucl_object_tostring(value);
+
+                       if (g_ascii_strcasecmp(tcp_mode, "yes") == 0 ||
+                               g_ascii_strcasecmp(tcp_mode, "true") == 0) {
+                               rule->tcp_enabled = TRUE;
+                               rule->tcp_auto = FALSE;
+                       }
+                       else if (g_ascii_strcasecmp(tcp_mode, "auto") == 0) {
+                               rule->tcp_auto = TRUE;
+                               rule->tcp_enabled = FALSE;
+                       }
+                       else if (g_ascii_strcasecmp(tcp_mode, "no") == 0 ||
+                                        g_ascii_strcasecmp(tcp_mode, "false") == 0) {
+                               rule->tcp_enabled = FALSE;
+                               rule->tcp_auto = FALSE;
+                       }
+                       else {
+                               msg_warn_config("unknown tcp mode: %s, TCP disabled", tcp_mode);
+                       }
+               }
+       }
+       else {
+               msg_debug_config("rule %s: 'tcp' option not found in configuration",
+                                                name ? name : "default");
+       }
+
+       if ((value = ucl_object_lookup(obj, "tcp_threshold")) != NULL) {
+               msg_debug_config("rule %s: found tcp_threshold=%.2f",
+                                                name ? name : "default", ucl_obj_todouble(value));
+               rule->tcp_threshold = ucl_obj_todouble(value);
+       }
+       else {
+               rule->tcp_threshold = 1.0; /* Default: 1 req/sec */
+       }
+
+       if ((value = ucl_object_lookup(obj, "tcp_window")) != NULL) {
+               rule->tcp_window = ucl_obj_todouble(value);
+       }
+       else {
+               rule->tcp_window = 1.0; /* Default: 1 second window */
+       }
+
+       if ((value = ucl_object_lookup(obj, "tcp_timeout")) != NULL) {
+               rule->tcp_timeout = ucl_obj_todouble(value);
+       }
+       else {
+               rule->tcp_timeout = 5.0; /* Default: 5 seconds */
+       }
+
+       /* Log TCP configuration */
+       if (rule->tcp_enabled) {
+               msg_info_config("rule %s: TCP explicitly enabled (timeout=%.1fs)",
+                                               rule->name ? rule->name : "default", rule->tcp_timeout);
+       }
+       else if (rule->tcp_auto) {
+               msg_info_config("rule %s: TCP auto-switch enabled (threshold=%.2f req/s, window=%.1fs)",
+                                               rule->name ? rule->name : "default",
+                                               rule->tcp_threshold, rule->tcp_window);
+       }
+       else {
+               msg_info_config("rule %s: TCP disabled (using UDP only)",
+                                               rule->name ? rule->name : "default");
+       }
+
        if ((value = ucl_object_lookup(obj, "algorithm")) != NULL) {
                rule->algorithm_str = ucl_object_tostring(value);
 
@@ -2488,6 +2612,11 @@ fuzzy_tcp_session_cleanup(struct fuzzy_client_session *session)
                pending = (struct fuzzy_tcp_pending_command *) value;
 
                if (pending->session == session) {
+                       /* Mark command as replied (session finished) */
+                       if (!(pending->io->flags & FUZZY_CMD_FLAG_REPLIED)) {
+                               pending->io->flags |= FUZZY_CMD_FLAG_REPLIED;
+                       }
+
                        g_ptr_array_add(to_remove, key);
                }
        }
index 73611403b9139cdbbc8b134a9f89c2bb4b1838ab..9b3bca000568287aa8ce8393efea8ad09444250d 100644 (file)
@@ -19,8 +19,8 @@ ${RSPAMD_FUZZY_KEY}             null
 ${RSPAMD_FUZZY_SERVER_MODE}     servers
 ${RSPAMD_FUZZY_SHINGLES_KEY}    null
 ${RSPAMD_SCOPE}                 Suite
-${SETTINGS_FUZZY_CHECK}         ${EMPTY}
-${SETTINGS_FUZZY_WORKER}        ${EMPTY}
+${RSPAMD_SETTINGS_FUZZY_CHECK}         ${EMPTY}
+${RSPAMD_SETTINGS_FUZZY_WORKER}        ${EMPTY}
 @{MESSAGES_SKIP}                ${RSPAMD_TESTDIR}/messages/priority.eml
 @{MESSAGES}                     ${RSPAMD_TESTDIR}/messages/spam_message.eml  ${RSPAMD_TESTDIR}/messages/zip.eml
 @{RANDOM_MESSAGES}              ${RSPAMD_TESTDIR}/messages/bad_message.eml  ${RSPAMD_TESTDIR}/messages/zip-doublebad.eml
@@ -145,7 +145,7 @@ Fuzzy Setup Plain
   [Arguments]  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  servers
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}";
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}";
   Rspamd Redis Setup
 
 Fuzzy Setup Keyed
@@ -227,26 +227,26 @@ Fuzzy Multimessage Overwrite Test
 Fuzzy Setup Split Servers
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  siphash
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  split
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  read_servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}"; write_servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}";
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  read_servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}"; write_servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}";
   Rspamd Redis Setup
 
 Fuzzy Setup Read Only
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  siphash
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  read_only
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  read_only = true;
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  read_only = true;
   Rspamd Redis Setup
 
 Fuzzy Setup Write Only
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  siphash
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  write_only
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  mode = "write_only";
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  mode = "write_only";
   Rspamd Redis Setup
 
 Fuzzy Setup TCP
   [Arguments]  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  servers
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}"; tcp = "auto"; tcp_threshold = 5;
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  tcp = "auto"; tcp_threshold = 5;
   Rspamd Redis Setup
 
 Fuzzy Setup TCP Siphash
@@ -260,8 +260,8 @@ Fuzzy Setup TCP Encrypted
   Set Suite Variable  ${RSPAMD_FUZZY_CLIENT_ENCRYPTION_KEY}  ${RSPAMD_KEY_PUB1}
   Set Suite Variable  ${RSPAMD_FUZZY_INCLUDE}  ${RSPAMD_TESTDIR}/configs/fuzzy-encryption-key.conf
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  servers
-  Set Suite Variable  ${SETTINGS_FUZZY_WORKER}  tcp = true;
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  tcp = "auto"; tcp_threshold = 5;
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_WORKER}  tcp = true;
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  tcp = "auto"; tcp_threshold = 5;
   Rspamd Redis Setup
 
 Fuzzy Setup TCP Encrypted Siphash
@@ -271,8 +271,8 @@ Fuzzy Setup TCP Explicit
   [Arguments]  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_ALGORITHM}  ${algorithm}
   Set Suite Variable  ${RSPAMD_FUZZY_SERVER_MODE}  servers
-  Set Suite Variable  ${SETTINGS_FUZZY_WORKER}  tcp = true;
-  Set Suite Variable  ${SETTINGS_FUZZY_CHECK}  servers = "${RSPAMD_LOCAL_ADDR}:${RSPAMD_PORT_FUZZY}"; tcp = "yes";
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_WORKER}  tcp = true;
+  Set Suite Variable  ${RSPAMD_SETTINGS_FUZZY_CHECK}  tcp = "yes";
   Rspamd Redis Setup
 
 Fuzzy Setup TCP Explicit Siphash