From da5af75c1ce4e53118a4594931a48d14b3b81c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 3 Jun 2020 08:27:01 +0300 Subject: [PATCH] MXS-3021: Make strictness of dbfwfilter configurable In some cases the dbfwfilter is too strict and SQL that would not match a rule is blocked due to it not being fully parsed. To allow a more lenient mode of operation, the requirement for full parsing must be made configurable. --- .../Filters/Database-Firewall-Filter.md | 15 ++++++++++++ .../modules/filter/dbfwfilter/dbfwfilter.cc | 23 +++++++++++++------ .../modules/filter/dbfwfilter/dbfwfilter.hh | 11 +++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/Documentation/Filters/Database-Firewall-Filter.md b/Documentation/Filters/Database-Firewall-Filter.md index 7126e74be..8192f21d4 100644 --- a/Documentation/Filters/Database-Firewall-Filter.md +++ b/Documentation/Filters/Database-Firewall-Filter.md @@ -113,31 +113,46 @@ Log all queries that do not match a rule. The matched user and the query is logged. The log messages are logged at the notice level. #### `treat_string_as_field` + This optional parameter specifies how the database firewall should treat strings. If true, they will be handled as fields, which will cause column blocking rules to match even if `ANSI_QUOTES` has been enabled and `"` is used instead of backtick. + ``` treat_string_as_field=false ``` + The default value is `true`. Note that this may cause a false positive, if a "true" string contains the name of a column to be blocked. #### `treat_string_arg_as_field` + This optional parameter specifies how the database firewall should treat strings used as arguments to functions. If true, they will be handled as fields, which will cause function column blocking rules to match even even if `ANSI_QUOTES` has been enabled and `"` is used instead of backtick. + ``` treat_string_arg_as_field=false ``` + The default value is `true`. Note that this may cause a false positive, if a "true" string contains the name of a column to be blocked. +#### `strict` + +Whether to treat unsupported SQL or multi-statement SQL as an error. This is a +boolean parameter and the default value is `true`. + +When disabled, SQL that cannot be fully parsed is allowed to pass if the rules +do not cause it to be blocked. This can be used to provide a best-effort mode +where uncertainly about the SQL is allowed. + ## Rule syntax The rules are defined by using the following syntax: diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.cc b/server/modules/filter/dbfwfilter/dbfwfilter.cc index 6637660f7..d0aae2946 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -635,6 +635,11 @@ MXS_MODULE* MXS_CREATE_MODULE() MXS_MODULE_PARAM_BOOL, "true" }, + { + "strict", + MXS_MODULE_PARAM_BOOL, + "true" + }, {MXS_END_MODULE_PARAMS} } }; @@ -1254,6 +1259,7 @@ Dbfw::Dbfw(MXS_CONFIG_PARAMETER* params) , m_treat_string_arg_as_field(params->get_bool("treat_string_arg_as_field")) , m_filename(params->get_string("rules")) , m_version(atomic_add(&global_version, 1)) + , m_strict(params->get_bool("strict")) { if (params->get_bool("log_match")) { @@ -1533,7 +1539,7 @@ int DbfwSession::routeQuery(GWBUF* buffer) type = qc_get_type_mask(buffer); } - if (modutil_is_SQL(buffer) && modutil_count_statements(buffer) > 1) + if (m_instance->strict() && modutil_is_SQL(buffer) && modutil_count_statements(buffer) > 1) { set_error("This filter does not support multi-statements."); rval = send_error(); @@ -1810,13 +1816,16 @@ bool rule_matches(Dbfw* my_instance, { qc_parse_result_t parse_result = qc_parse(queue, QC_COLLECT_ALL); - if (parse_result == QC_QUERY_INVALID) + if (my_instance->strict()) { - msg = create_parse_error(my_instance, "tokenized", query, &matches); - } - else if (parse_result != QC_QUERY_PARSED && rule->need_full_parsing(queue)) - { - msg = create_parse_error(my_instance, "parsed completely", query, &matches); + if (parse_result == QC_QUERY_INVALID) + { + msg = create_parse_error(my_instance, "tokenized", query, &matches); + } + else if (parse_result != QC_QUERY_PARSED && rule->need_full_parsing(queue)) + { + msg = create_parse_error(my_instance, "parsed completely", query, &matches); + } } } diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index 9e24dbfd8..007339fe1 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -244,6 +244,16 @@ public: return m_treat_string_arg_as_field; } + /** + * Whether unsupported SQL is an error + * + * @return True if unsupported SQL is an error + */ + bool strict() const + { + return m_strict; + } + /** * Get logging option bitmask * @@ -292,6 +302,7 @@ private: mutable std::mutex m_lock; /*< Instance spinlock */ std::string m_filename; /*< Path to the rule file */ int m_version; /*< Latest rule file version, incremented on reload */ + bool m_strict; Dbfw(MXS_CONFIG_PARAMETER* param); bool do_reload_rules(std::string filename);