From 7272d9401d447991f4c287178ab79e4f29bf577d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Tue, 5 Sep 2017 10:17:59 +0300 Subject: [PATCH] MXS-1346: Fix the `at_times` rule The rule used the values from the QuerySpeed struct instead of the values in the rule itself. --- .../modules/filter/dbfwfilter/dbfwfilter.hh | 10 +-------- server/modules/filter/dbfwfilter/rules.cc | 22 +++++++++++-------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index fb4be3679..6763e85c6 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -136,25 +136,17 @@ typedef struct timerange_t */ struct QuerySpeed { - QuerySpeed(int period = 0, int cooldown = 0, int limit = 0): + QuerySpeed(): first_query(0), triggered(0), - period(period), - cooldown(cooldown), count(0), - limit(limit), - id(0), active(false) { } time_t first_query; /*< Time when the first query occurred */ time_t triggered; /*< Time when the limit was exceeded */ - int period; /*< Measurement interval in seconds */ - int cooldown; /*< Time the user is denied access for */ int count; /*< Number of queries done */ - int limit; /*< Maximum number of queries */ - long id; /*< Unique id of the rule */ bool active; /*< If the rule has been triggered */ }; diff --git a/server/modules/filter/dbfwfilter/rules.cc b/server/modules/filter/dbfwfilter/rules.cc index f528f954e..de8d4bad8 100644 --- a/server/modules/filter/dbfwfilter/rules.cc +++ b/server/modules/filter/dbfwfilter/rules.cc @@ -288,9 +288,9 @@ bool LimitQueriesRule::matches_query(DbfwSession* session, GWBUF* buffer, char** if (queryspeed->active) { - if (difftime(time_now, queryspeed->triggered) < queryspeed->cooldown) + if (difftime(time_now, queryspeed->triggered) < m_holdoff) { - double blocked_for = queryspeed->cooldown - difftime(time_now, queryspeed->triggered); + double blocked_for = m_holdoff - difftime(time_now, queryspeed->triggered); *msg = create_error("Queries denied for %f seconds", blocked_for); matches = true; @@ -305,28 +305,32 @@ bool LimitQueriesRule::matches_query(DbfwSession* session, GWBUF* buffer, char** } else { - if (queryspeed->count >= queryspeed->limit) + if (queryspeed->count >= m_max) { MXS_INFO("rule '%s': query limit triggered (%d queries in %d seconds), " "denying queries from user for %d seconds.", name().c_str(), - queryspeed->limit, queryspeed->period, queryspeed->cooldown); + m_max, m_timeperiod, m_holdoff); queryspeed->triggered = time_now; queryspeed->active = true; matches = true; - double blocked_for = queryspeed->cooldown - difftime(time_now, queryspeed->triggered); + double blocked_for = m_holdoff - difftime(time_now, queryspeed->triggered); *msg = create_error("Queries denied for %f seconds", blocked_for); } - else if (queryspeed->count > 0 && - difftime(time_now, queryspeed->first_query) <= queryspeed->period) + else if (queryspeed->count == 0) + { + queryspeed->first_query = time_now; + queryspeed->count = 1; + } + else if (difftime(time_now, queryspeed->first_query) <= m_timeperiod) { queryspeed->count++; } else { - queryspeed->first_query = time_now; - queryspeed->count = 1; + /** The time period was exceeded, reset the query count */ + queryspeed->count = 0; } }