Merge pull request #5428 from rspamd/vstakhov-replies-fake-rcpt

[Rework] Replies: consider all recipients and use smtp ones

7 files changed, 90 insertions, 57 deletions

diff --git a/src/plugins/lua/known_senders.lua b/src/plugins/lua/known_senders.lua
index 5cb2ddcf5..0cbf3cdcf 100644
--- a/src/plugins/lua/known_senders.lua
+++ b/src/plugins/lua/known_senders.lua
@@ -106,21 +106,26 @@ local function configure_scripts(_, _, _)
   -- script checks if given recipients are in the local replies set of the sender
   local redis_zscore_script = [[
     local replies_recipients_addrs = ARGV
-    if replies_recipients_addrs then
+    if replies_recipients_addrs and #replies_recipients_addrs > 0 then
+      local found = false
       for _, rcpt in ipairs(replies_recipients_addrs) do
         local score = redis.call('ZSCORE', KEYS[1], rcpt)
-        -- check if score is nil (for some reason redis script does not see if score is a nil value)
-        if type(score) == 'boolean' then
-          score = nil
-          -- 0 is stand for failure code
-          return 0
+        if score then
+          -- If we found at least one recipient, consider it a match
+          found = true
+          break
         end
       end
-      -- first number in return statement is stands for the success/failure code
-      -- where success code is 1 and failure code is 0
-      return 1
+
+      if found then
+        -- Success code is 1
+        return 1
+      else
+        -- Failure code is 0
+        return 0
+      end
     else
-    -- 0 is a failure code
+      -- No recipients to check, failure code is 0
       return 0
     end
   ]]
@@ -259,7 +264,13 @@ local function verify_local_replies_set(task)
     return nil
   end
 
-  local replies_recipients = task:get_recipients('mime') or E
+  local replies_recipients = task:get_recipients('smtp') or E
+
+  -- If no recipients, don't proceed
+  if #replies_recipients == 0 then
+    lua_util.debugm(N, task, 'No recipients to verify')
+    return nil
+  end
 
   local replies_sender_string = lua_util.maybe_obfuscate_string(tostring(replies_sender), settings,
       settings.sender_prefix)
@@ -268,13 +279,16 @@ local function verify_local_replies_set(task)
   local function redis_zscore_script_cb(err, data)
     if err ~= nil then
       rspamd_logger.errx(task, 'Could not verify %s local replies set %s', replies_sender_key, err)
-    end
-    if data ~= 1 then
-      lua_util.debugm(N, task, 'Recipients were not verified')
       return
     end
-    lua_util.debugm(N, task, 'Recipients were verified')
-    task:insert_result(settings.symbol_check_mail_local, 1.0, replies_sender_key)
+
+    -- We need to ensure we're properly checking the result
+    if data == 1 then
+      lua_util.debugm(N, task, 'Recipients were verified')
+      task:insert_result(settings.symbol_check_mail_local, 1.0, replies_sender_key)
+    else
+      lua_util.debugm(N, task, 'Recipients were not verified, data=%s', data)
+    end
   end
 
   local replies_recipients_addrs = {}
@@ -284,12 +298,24 @@ local function verify_local_replies_set(task)
     table.insert(replies_recipients_addrs, replies_recipients[i].addr)
   end
 
-  lua_util.debugm(N, task, 'Making redis request to local replies set')
-  lua_redis.exec_redis_script(zscore_script_id,
+  -- Only proceed if we have recipients to check
+  if #replies_recipients_addrs == 0 then
+    lua_util.debugm(N, task, 'No recipient addresses to verify')
+    return nil
+  end
+
+  lua_util.debugm(N, task, 'Making redis request to local replies set with key %s and recipients %s',
+      replies_sender_key, table.concat(replies_recipients_addrs, ", "))
+
+  local ret = lua_redis.exec_redis_script(zscore_script_id,
       { task = task, is_write = true },
       redis_zscore_script_cb,
       { replies_sender_key },
       replies_recipients_addrs)
+
+  if not ret then
+    rspamd_logger.errx(task, "redis script request wasn't scheduled")
+  end
 end
 
 local function check_known_incoming_mail_callback(task)
diff --git a/src/plugins/lua/replies.lua b/src/plugins/lua/replies.lua
index 08fb68bc7..2f0153d00 100644
--- a/src/plugins/lua/replies.lua
+++ b/src/plugins/lua/replies.lua
@@ -79,8 +79,8 @@ local function configure_redis_scripts(_, _)
       end
       ]]
   local set_script_zadd_global = lua_util.jinja_template(redis_script_zadd_global,
-          { max_global_size = settings.max_global_size })
-  global_replies_set_script =  lua_redis.add_redis_script(set_script_zadd_global, redis_params)
+      { max_global_size = settings.max_global_size })
+  global_replies_set_script = lua_redis.add_redis_script(set_script_zadd_global, redis_params)
 
   local redis_script_zadd_local = [[
       redis.call('ZREMRANGEBYRANK', KEYS[1], 0, -({= max_local_size =} + 1)) -- keeping size of local replies set
@@ -102,7 +102,7 @@ local function configure_redis_scripts(_, _)
       end
     ]]
   local set_script_zadd_local = lua_util.jinja_template(redis_script_zadd_local,
-          { expire_time = settings.expire, max_local_size = settings.max_local_size })
+      { expire_time = settings.expire, max_local_size = settings.max_local_size })
   local_replies_set_script = lua_redis.add_redis_script(set_script_zadd_local, redis_params)
 end
 
@@ -110,7 +110,7 @@ local function replies_check(task)
   local in_reply_to
 
   local function check_recipient(stored_rcpt)
-    local rcpts = task:get_recipients('mime')
+    local rcpts = task:get_recipients('smtp')
     lua_util.debugm(N, task, 'recipients: %s', rcpts)
     if rcpts then
       local filter_predicate = function(input_rcpt)
@@ -119,7 +119,7 @@ local function replies_check(task)
         return real_rcpt_h == stored_rcpt
       end
 
-      if fun.any(filter_predicate, fun.map(function(rcpt)
+      if fun.all(filter_predicate, fun.map(function(rcpt)
         return rcpt.addr or ''
       end, rcpts)) then
         lua_util.debugm(N, task, 'reply to %s validated', in_reply_to)
@@ -155,9 +155,9 @@ local function replies_check(task)
     end
 
     lua_redis.exec_redis_script(global_replies_set_script,
-            { task = task, is_write = true },
-            zadd_global_set_cb,
-            { global_key }, params)
+        { task = task, is_write = true },
+        zadd_global_set_cb,
+        { global_key }, params)
   end
 
   local function add_to_replies_set(recipients)
@@ -173,7 +173,7 @@ local function replies_check(task)
 
     local params = recipients
     lua_util.debugm(N, task,
-    'Adding recipients %s to sender %s local replies set', recipients, sender_key)
+        'Adding recipients %s to sender %s local replies set', recipients, sender_key)
 
     local function zadd_cb(err, _)
       if err ~= nil then
@@ -189,9 +189,9 @@ local function replies_check(task)
 
     table.insert(params, 1, task_time_str)
     lua_redis.exec_redis_script(local_replies_set_script,
-            { task = task, is_write = true },
-            zadd_cb,
-            { sender_key }, params)
+        { task = task, is_write = true },
+        zadd_cb,
+        { sender_key }, params)
   end
 
   local function redis_get_cb(err, data, addr)
@@ -387,7 +387,7 @@ if opts then
       end
 
       lua_redis.register_prefix(settings.sender_prefix, N,
-              'Prefix to identify replies sets')
+          'Prefix to identify replies sets')
 
       local id = rspamd_config:register_symbol({
         name = 'REPLIES_CHECK',
diff --git a/test/functional/cases/400_known_senders.robot b/test/functional/cases/400_known_senders.robot
index d827acc0e..a7cde59cb 100644
--- a/test/functional/cases/400_known_senders.robot
+++ b/test/functional/cases/400_known_senders.robot
@@ -43,33 +43,37 @@ INCOMING MAIL SENDER IS UNKNOWN
   ...  Settings={symbols_enabled [${SYMBOL_GLOBAL}, ${SYMBOL_LOCAL}]}
   Do Not Expect Symbol  ${SYMBOL_GLOBAL}
   Do Not Expect Symbol  ${SYMBOL_LOCAL}
-  
+
 INCOMING MAIL SENDER IS KNOWN RECIPIENTS ARE UNKNOWN
   Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  User=xxx@abrakadabra.com
+  ...  From=xxx@abrakadabra.com
   ...  Settings=${SETTINGS_REPLIES}
   Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  Settings=${SETTINGS_REPLIES}
+  ...  Rcpt=xxx@abrakadabra.com
   ...  Settings=${SETTINGS_REPLIES}
+  ...  From=user@emailbl.com
   Scan File  ${RSPAMD_TESTDIR}/messages/inc_mail_known_sender.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
   ...  Settings={symbols_enabled [${SYMBOL_GLOBAL}, ${SYMBOL_LOCAL}]}
   Expect Symbol  ${SYMBOL_GLOBAL}
   Do Not Expect Symbol   ${SYMBOL_LOCAL}
 
 INCOMING MAIL SENDER IS KNOWN RECIPIENTS ARE KNOWN
   Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8  User=user@emailbl.com  From=user@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
   Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8  User=user@emailbl.com  Rcpt=user@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
   Scan File  ${RSPAMD_TESTDIR}/messages/inc_mail_known_sender.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8  User=user@emailbl.com  Rcpt=user@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
   Scan File  ${RSPAMD_TESTDIR}/messages/inc_mail_known_sender.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8  User=user@emailbl.com  Rcpt=user@emailbl.com
   ...  Settings={symbols_enabled [${SYMBOL_GLOBAL}, ${SYMBOL_LOCAL}]}
   Expect Symbol  ${SYMBOL_GLOBAL}
   Expect Symbol  ${SYMBOL_LOCAL}
-
diff --git a/test/functional/cases/410_replies.robot b/test/functional/cases/410_replies.robot
index 23ad9df35..b6710149c 100644
--- a/test/functional/cases/410_replies.robot
+++ b/test/functional/cases/410_replies.robot
@@ -15,33 +15,36 @@ ${RSPAMD_SCOPE}        Suite
 *** Test Cases ***
 Reply to 1 sender 1 recipients
   Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  User=xxx@abrakadabra.com
+  ...  From=xxx@abrakadabra.com
   ...  Settings=${SETTINGS_REPLIES}
+  ...  Rcpt=user@emailbl.com
   Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_1.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  Rcpt=xxx@abrakadabra.com
   ...  Settings=${SETTINGS_REPLIES}
+  ...  From=user@emailbl.com
   Expect Symbol  ${SYMBOL}
 
-Reply to 1 sender 2 recipients first is set second is not
+Reply to 1 sender 2 recipients but SMTP recipient matches
   Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_1_2_first.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  User=xxxx@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
   Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_2.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  IP=8.8.8.8
+  ...  Rcpt=xxxx@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
   Expect Symbol  ${SYMBOL}
 
-Reply to 1 sender 2 recipients 1 rcpt is same
-  Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_2_s.eml
-  ...  IP=8.8.8.8  User=user@emailbl.com
+Reply to 1 sender 2 recipients but SMTP recipient NOT matches
+  Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_1_2_first.eml
+  ...  IP=8.8.8.8
+  ...  User=user@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
-  Expect Symbol  ${SYMBOL}
-
-Reply to another sender 2 recipients
-  Scan File  ${RSPAMD_TESTDIR}/messages/set_replyto_2_2.eml
-  ...  IP=8.8.8.8  User=another@emailbl.com
+  Scan File  ${RSPAMD_TESTDIR}/messages/replyto_1_2.eml
+  ...  IP=8.8.8.8  User=user@emailbl.com
+  ...  Rcpt=another@emailbl.com
   ...  Settings=${SETTINGS_REPLIES}
-  Scan File  ${RSPAMD_TESTDIR}/messages/replyto_2_2.eml
-  ...  IP=8.8.8.8  User=another@emailbl.com
-   ...  Settings=${SETTINGS_REPLIES}
-  Expect Symbol  ${SYMBOL}
+  Do Not Expect Symbol  ${SYMBOL}
diff --git a/test/functional/cases/410_logging/000_console/000_systemd_logger.robot b/test/functional/cases/411_logging/000_console/000_systemd_logger.robot
index 88178461b..88178461b 100644
--- a/test/functional/cases/410_logging/000_console/000_systemd_logger.robot
+++ b/test/functional/cases/411_logging/000_console/000_systemd_logger.robot
diff --git a/test/functional/cases/410_logging/000_console/001_timestamps.robot b/test/functional/cases/411_logging/000_console/001_timestamps.robot
index bd8e2c349..bd8e2c349 100644
--- a/test/functional/cases/410_logging/000_console/001_timestamps.robot
+++ b/test/functional/cases/411_logging/000_console/001_timestamps.robot
diff --git a/test/functional/cases/410_logging/001_file/000_json.robot b/test/functional/cases/411_logging/001_file/000_json.robot
index a2f04e85c..a2f04e85c 100644
--- a/test/functional/cases/410_logging/001_file/000_json.robot
+++ b/test/functional/cases/411_logging/001_file/000_json.robot