From d339b89990fe2ba79874ed707064e1d3643314ee Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 3 Jul 2018 11:44:21 +0300 Subject: [PATCH] MXS-1774 Make rejection of functions optional It is now possible to prevent the masking filter from rejecting statements using functions in conjunction with fields to be masked. So now it is possible to not use the blanket rejection of the masking filter and replace it with more detailed firewall rules. --- Documentation/Filters/Masking.md | 23 ++++ .../MaxScale-2.3.0-Release-Notes.md | 6 + .../modules/filter/masking/maskingfilter.cc | 5 + .../filter/masking/maskingfilterconfig.cc | 21 +++ .../filter/masking/maskingfilterconfig.hh | 16 +++ .../filter/masking/maskingfiltersession.cc | 127 ++++++++++-------- .../filter/masking/maskingfiltersession.hh | 2 + 7 files changed, 142 insertions(+), 58 deletions(-) diff --git a/Documentation/Filters/Masking.md b/Documentation/Filters/Masking.md index e3d99b599..4b68df3a3 100644 --- a/Documentation/Filters/Masking.md +++ b/Documentation/Filters/Masking.md @@ -133,6 +133,29 @@ resultsets that do not contain such columns. large_payload=ignore ``` +#### `prevent_function_usage` + +This optional parameter specifies how the masking filter should behave +if a column that should be masked, is used in conjunction with some +function. As the masking filter works _only_ on the basis of the +information in the returned result-set, if the name of a column is +not present in the result-set, then the masking filter cannot mask a +value. This means that the masking filter bascially can be bypassed +with a query like: +``` +SELECT CONCAT(masked_column) FROM tbl; +``` +If the value of `prevent_function_usage` is `true`, then all +statements that contain functions referring to masked columns will +be rejected. As that means that also queries using potentially +harmless functions, such as `LENGTH(masked_column)`, are rejected +as well, this feature can be turned off. In that case, the firewall +filter should be setup to allow or reject the use of certain functions. +``` +prevent_function_usage=false +``` +The default value is `true`. + # Rules The masking rules are expressed as a JSON object. diff --git a/Documentation/Release-Notes/MaxScale-2.3.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.3.0-Release-Notes.md index dec626e23..ea71f0518 100644 --- a/Documentation/Release-Notes/MaxScale-2.3.0-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-2.3.0-Release-Notes.md @@ -28,6 +28,12 @@ still retaining the relative order of each command. To keep the old functionality, add `disable_sescmd_history=true` to the service definition. +### Masking Filter + +By default the masking filter rejects statements that use functions on +conjuction with columns that should be masked. Please see the +[Masking Filter](../Filters/Masking.md) documentation for details. + ### Switchover new master autoselection The switchover command in *mariadbmon* can now be called with just the monitor diff --git a/server/modules/filter/masking/maskingfilter.cc b/server/modules/filter/masking/maskingfilter.cc index 9170e62bd..a52bfc299 100644 --- a/server/modules/filter/masking/maskingfilter.cc +++ b/server/modules/filter/masking/maskingfilter.cc @@ -104,6 +104,11 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() MXS_MODULE_PARAM_ENUM, Config::large_payload_default, MXS_MODULE_OPT_NONE, Config::large_payload_values }, + { + Config::prevent_function_usage_name, + MXS_MODULE_PARAM_BOOL, Config::prevent_function_usage_default, + MXS_MODULE_OPT_NONE, + }, { MXS_END_MODULE_PARAMS } } }; diff --git a/server/modules/filter/masking/maskingfilterconfig.cc b/server/modules/filter/masking/maskingfilterconfig.cc index 16a351199..1af77606f 100644 --- a/server/modules/filter/masking/maskingfilterconfig.cc +++ b/server/modules/filter/masking/maskingfilterconfig.cc @@ -26,6 +26,10 @@ const char config_value_ignore[] = "ignore"; const char config_value_never[] = "never"; const char config_value_always[] = "always"; +const char config_name_prevent_function_usage[] = "prevent_function_usage"; + +const char config_value_true[] = "true"; + } /* @@ -71,6 +75,16 @@ const MXS_ENUM_VALUE MaskingFilterConfig::warn_type_mismatch_values[] = //static const char* MaskingFilterConfig::warn_type_mismatch_default = config_value_never; +/* + * PARAM prevent_function_usage + */ + +//static +const char* MaskingFilterConfig::prevent_function_usage_name = config_name_prevent_function_usage; + +//static +const char* MaskingFilterConfig::prevent_function_usage_default = config_value_true; + /* * MaskingFilterConfig */ @@ -96,3 +110,10 @@ MaskingFilterConfig::get_warn_type_mismatch(const MXS_CONFIG_PARAMETER* pParams) int value = config_get_enum(pParams, warn_type_mismatch_name, warn_type_mismatch_values); return static_cast(value); } + +//static +bool MaskingFilterConfig::get_prevent_function_usage(const MXS_CONFIG_PARAMETER* pParams) +{ + return config_get_bool(pParams, prevent_function_usage_name); +} + diff --git a/server/modules/filter/masking/maskingfilterconfig.hh b/server/modules/filter/masking/maskingfilterconfig.hh index 6df52048c..a6988a89d 100644 --- a/server/modules/filter/masking/maskingfilterconfig.hh +++ b/server/modules/filter/masking/maskingfilterconfig.hh @@ -42,11 +42,15 @@ public: static const MXS_ENUM_VALUE warn_type_mismatch_values[]; static const char* warn_type_mismatch_default; + static const char* prevent_function_usage_name; + static const char* prevent_function_usage_default; + MaskingFilterConfig(const char* zName, const MXS_CONFIG_PARAMETER* pParams) : m_name(zName) , m_large_payload(get_large_payload(pParams)) , m_rules(get_rules(pParams)) , m_warn_type_mismatch(get_warn_type_mismatch(pParams)) + , m_prevent_function_usage(get_prevent_function_usage(pParams)) {} ~MaskingFilterConfig() {} @@ -70,6 +74,11 @@ public: return m_warn_type_mismatch; } + bool prevent_function_usage() const + { + return m_prevent_function_usage; + } + void set_large_payload(large_payload_t l) { m_large_payload = l; @@ -84,13 +93,20 @@ public: m_warn_type_mismatch = w; } + void set_prevent_function_usage(bool b) + { + m_prevent_function_usage = b; + } + static large_payload_t get_large_payload(const MXS_CONFIG_PARAMETER* pParams); static std::string get_rules(const MXS_CONFIG_PARAMETER* pParams); static warn_type_mismatch_t get_warn_type_mismatch(const MXS_CONFIG_PARAMETER* pParams); + static bool get_prevent_function_usage(const MXS_CONFIG_PARAMETER* pParams); private: std::string m_name; large_payload_t m_large_payload; std::string m_rules; warn_type_mismatch_t m_warn_type_mismatch; + bool m_prevent_function_usage; }; diff --git a/server/modules/filter/masking/maskingfiltersession.cc b/server/modules/filter/masking/maskingfiltersession.cc index 52d5006fc..2b7c6e3ae 100644 --- a/server/modules/filter/masking/maskingfiltersession.cc +++ b/server/modules/filter/masking/maskingfiltersession.cc @@ -56,65 +56,15 @@ int MaskingFilterSession::routeQuery(GWBUF* pPacket) switch (request.command()) { case MXS_COM_QUERY: + m_res.reset(request.command(), m_filter.rules()); + + if (m_filter.config().prevent_function_usage() && reject_if_function_used(pPacket)) { - m_res.reset(request.command(), m_filter.rules()); - - SMaskingRules sRules = m_filter.rules(); - - const char *zUser = session_get_user(m_pSession); - const char *zHost = session_get_remote(m_pSession); - - if (!zUser) - { - zUser = ""; - } - - if (!zHost) - { - zHost = ""; - } - - auto pred1 = [&sRules, zUser, zHost](const QC_FIELD_INFO& field_info) - { - const MaskingRules::Rule* pRule = sRules->get_rule_for(field_info, zUser, zHost); - - return pRule ? true : false; - }; - - auto pred2 = [&sRules, zUser, zHost, &pred1](const QC_FUNCTION_INFO& function_info) - { - const QC_FIELD_INFO* begin = function_info.fields; - const QC_FIELD_INFO* end = begin + function_info.n_fields; - - auto i = std::find_if(begin, end, pred1); - - return i != end; - }; - - const QC_FUNCTION_INFO* pInfos; - size_t nInfos; - - qc_get_function_info(pPacket, &pInfos, &nInfos); - - const QC_FUNCTION_INFO* begin = pInfos; - const QC_FUNCTION_INFO* end = begin + nInfos; - - auto i = std::find_if(begin, end, pred2); - - if (i == end) - { - m_state = EXPECTING_RESPONSE; - } - else - { - std::stringstream ss; - ss << "The function " << i->name << " is used in conjunction with a field " - << "that should be masked for '" << zUser << "'@'" << zHost << "', access is denied."; - - GWBUF* pResponse = modutil_create_mysql_err_msg(1, 0, 1141, "HY000", ss.str().c_str()); - set_response(pResponse); - m_state = EXPECTING_NOTHING; - } + m_state = EXPECTING_NOTHING; + } + else + { + m_state = EXPECTING_RESPONSE; } break; @@ -418,3 +368,64 @@ void MaskingFilterSession::mask_values(ComPacket& response) ss_dassert(!true); } } + +bool MaskingFilterSession::reject_if_function_used(GWBUF* pPacket) +{ + bool rejected = false; + + SMaskingRules sRules = m_filter.rules(); + + const char *zUser = session_get_user(m_pSession); + const char *zHost = session_get_remote(m_pSession); + + if (!zUser) + { + zUser = ""; + } + + if (!zHost) + { + zHost = ""; + } + + auto pred1 = [&sRules, zUser, zHost](const QC_FIELD_INFO& field_info) + { + const MaskingRules::Rule* pRule = sRules->get_rule_for(field_info, zUser, zHost); + + return pRule ? true : false; + }; + + auto pred2 = [&sRules, zUser, zHost, &pred1](const QC_FUNCTION_INFO& function_info) + { + const QC_FIELD_INFO* begin = function_info.fields; + const QC_FIELD_INFO* end = begin + function_info.n_fields; + + auto i = std::find_if(begin, end, pred1); + + return i != end; + }; + + const QC_FUNCTION_INFO* pInfos; + size_t nInfos; + + qc_get_function_info(pPacket, &pInfos, &nInfos); + + const QC_FUNCTION_INFO* begin = pInfos; + const QC_FUNCTION_INFO* end = begin + nInfos; + + auto i = std::find_if(begin, end, pred2); + + if (i != end) + { + std::stringstream ss; + ss << "The function " << i->name << " is used in conjunction with a field " + << "that should be masked for '" << zUser << "'@'" << zHost << "', access is denied."; + + GWBUF* pResponse = modutil_create_mysql_err_msg(1, 0, 1141, "HY000", ss.str().c_str()); + set_response(pResponse); + + rejected = true; + } + + return rejected; +} diff --git a/server/modules/filter/masking/maskingfiltersession.hh b/server/modules/filter/masking/maskingfiltersession.hh index b805c1cba..1ad6b6aa6 100644 --- a/server/modules/filter/masking/maskingfiltersession.hh +++ b/server/modules/filter/masking/maskingfiltersession.hh @@ -61,6 +61,8 @@ private: void mask_values(ComPacket& response); + bool reject_if_function_used(GWBUF* pPacket); + private: typedef std::tr1::shared_ptr SMaskingRules;