Skip to content

Draft: Add support for coordinating actions across filters

Baptiste Careil requested to merge bcareil/reaction:rust into rust

I saw the rust branch and went with this one because I am more comfortable with rust than go. Though I'm not fluent in rust either.

Anyway, this is a draft that partially addresses #68 but actually solves a different kind of problem.

Given a config with two filters with overlapping matching rules (e.g. one filter bans after 15 404 HTTP errors, another ban on an explicit access to /.env which does not exists and returns a 404). I want to make sure the IP is banned for the appropriate amount of time and that an unban action is not executed earlier than expected (e.g. the first filter bans for 15 minutes and the other for 24 hours, the unban of the first will be executed before the unban of the second which can lead to unexpected consequences, albeit not with iptables AFAIK).

Assuming a simple config which has at most one delayed action (an action with the after property set) per filter, the main goals are to ensure the correct sequencing of actions (a "ban" action must be followed by an "unban" before another "ban" can be issued) and ideally (and this is not implemented yet) to extend the ban duration based on the longest duration between the pending "unban" and the new "unban".

In terms of config, the feature adds a "correlator" field to the filter definition. This is used as a key to aggregate the action of the same name but on different filters. If a "unban" action for that correlator is still pending when a new one arrives, the old one is cancelled (ideally it should depend on whether the new "unban" would be executed after the old one) and the new one takes its place. Whether the two execute the same command is not checked.

In terms of flow:

  • On a match, the filter now passes the list of actions to the execs_manager instead of passing them one by one
  • This allows the execs_manager to look up the correlator (if present) and check if actions are pending for it
    • If yes, the CancelToken is used to cancel the remaining pending actions
  • In either cases, or if no correlators are present, delayed actions are queued in a new class MyTimedMessageQueue (the name can be changed 😄 ) in exchange for a CancelToken (vaguely equivalent to the Guard of the MessageTimer class).
  • On exit, the Stop command now flushes the MyTimedMessageQueue instead of relying on a separate ExecsMap data structure
  • The stop completes when the MyTimedMessageQueue is empty as it drops its end of the channel to the execs_manager

This implementation has several limitations that may not be fixed:

  • If a filter has several delayed actions, and regardless of whether they should be executed after the same delay, it is possible that a new ban action arrives in-between the two delayed actions. In which case:
    • The ban action will not be re-issued
    • one of the unban action will have executed and will not be re-executed
    • the remaining unban action will be further delayed
      This is an acceptable compromise in my opinion, especially since such a situation can be detected when parsing the configuration and we can either issue warnings or errors for it.
  • If two filters share the same correlator but have different names for their actions, we will have the following behavior:
    • The first filter to trigger will execute all non delayed action and schedule all delayed action
    • The second filter to trigger will execute all non delayed action, schedule all delayed actions and will cancel all the delayed actions from the first filter
      Which can be a surprising behavior. But again, I think it's acceptable and we can also detect it when parsing the configuration.

Merge request reports