From 279edce16ed9eb712d352323906ab1382004990b Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 3 May 2019 10:06:53 +0300 Subject: [PATCH] MXS-2457 Add treat_string_as_field to firewall Necessary if the firewall should be able to block columns when 'ANSI_QUOTES' as enabled and " instead of backticks are used. Without this, the following > set @@sql_mode='ANSI_QUOTES'; > select "ssn" from person; will not be blocked if the database firewall has been configured to block the column ssn. --- .../Filters/Database-Firewall-Filter.md | 20 ++++++++- .../modules/filter/dbfwfilter/dbfwfilter.cc | 45 ++++++++++++++++++- .../modules/filter/dbfwfilter/dbfwfilter.hh | 11 +++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/Documentation/Filters/Database-Firewall-Filter.md b/Documentation/Filters/Database-Firewall-Filter.md index 34ca1ade8..9d575f35f 100644 --- a/Documentation/Filters/Database-Firewall-Filter.md +++ b/Documentation/Filters/Database-Firewall-Filter.md @@ -146,16 +146,32 @@ query itself is logged. The log messages are logged at the notice level. 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 fields to be masked even if `ANSI_QUOTES` has -been enabled and `"` is used instead of backtick. +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. + ## 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 ab3911e93..a4044f2ca 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.cc +++ b/server/modules/filter/dbfwfilter/dbfwfilter.cc @@ -632,6 +632,11 @@ MXS_MODULE* MXS_CREATE_MODULE() MXS_MODULE_PARAM_BOOL, "true" }, + { + "treat_string_as_field", + MXS_MODULE_PARAM_BOOL, + "true" + }, {MXS_END_MODULE_PARAMS} } }; @@ -1247,6 +1252,7 @@ int global_version = 1; Dbfw::Dbfw(MXS_CONFIG_PARAMETER* params) : m_action((enum fw_actions)config_get_enum(params, "action", action_values)) , m_log_match(0) + , m_treat_string_as_field(config_get_bool(params, "treat_string_as_field")) , m_treat_string_arg_as_field(config_get_bool(params, "treat_string_arg_as_field")) , m_filename(config_get_string(params, "rules")) , m_version(atomic_add(&global_version, 1)) @@ -1276,6 +1282,32 @@ Dbfw* Dbfw::create(const char* zName, MXS_CONFIG_PARAMETER* pParams) if (process_rule_file(file, &rules, &users)) { rval = new(std::nothrow) Dbfw(pParams); + + if (rval) + { + if (rval->treat_string_as_field() || rval->treat_string_arg_as_field()) + { + MXS_NOTICE("The parameter 'treat_string_arg_as_field' and/or " + "'treat_string_as_field' is enabled for %s. " + "As a consequence, the query classifier cache must be disabled.", + zName); + + QC_CACHE_PROPERTIES cache_properties; + qc_get_cache_properties(&cache_properties); + + if (cache_properties.max_size != 0) + { + cache_properties.max_size = 0; + qc_set_cache_properties(&cache_properties); + + MXS_NOTICE("Query classifier cache disabled."); + } + else + { + MXS_NOTICE("The query classifier cache was disabled already."); + } + } + } } return rval; @@ -1517,7 +1549,18 @@ int DbfwSession::routeQuery(GWBUF* buffer) } else { - uint32_t option = m_instance->treat_string_arg_as_field() ? QC_OPTION_STRING_ARG_AS_FIELD : 0; + uint32_t option = 0; + + if (m_instance->treat_string_as_field()) + { + option |= QC_OPTION_STRING_AS_FIELD; + } + + if (m_instance->treat_string_arg_as_field()) + { + option |= QC_OPTION_STRING_ARG_AS_FIELD; + } + EnableOption enable(option); GWBUF* analyzed_queue = buffer; diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.hh b/server/modules/filter/dbfwfilter/dbfwfilter.hh index 3ff2b8d63..6882751f0 100644 --- a/server/modules/filter/dbfwfilter/dbfwfilter.hh +++ b/server/modules/filter/dbfwfilter/dbfwfilter.hh @@ -224,6 +224,16 @@ public: */ fw_actions get_action() const; + /** + * Should strings be treated as fields? + * + * @return True, if they should, false otherwise. + */ + bool treat_string_as_field() const + { + return m_treat_string_as_field; + } + /** * Should string args be treated as fields? * @@ -277,6 +287,7 @@ public: private: fw_actions m_action; /*< Default operation mode, defaults to deny */ int m_log_match; /*< Log matching and/or non-matching queries */ + bool m_treat_string_as_field; bool m_treat_string_arg_as_field; mutable std::mutex m_lock; /*< Instance spinlock */ std::string m_filename; /*< Path to the rule file */