From dfd1d1e9795ba9055fe895ed90528da9c28bef53 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 28 Mar 2019 14:42:20 +0200 Subject: [PATCH 01/10] Update 2.3.5 release notes --- .../MaxScale-2.3.5-Release-Notes.md | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md diff --git a/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md new file mode 100644 index 000000000..90fec6725 --- /dev/null +++ b/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md @@ -0,0 +1,57 @@ +# MariaDB MaxScale 2.3.5 Release Notes + +Release 2.3.5 is a GA release. + +This document describes the changes in release 2.3.5, when compared to the +previous release in the same series. + +For any problems you encounter, please consider submitting a bug +report on [our Jira](https://jira.mariadb.org/projects/MXS). + +## Bug fixes + +* [MXS-2410](https://jira.mariadb.org/browse/MXS-2410) Hangup delivered to wrong DCB +* [MXS-2403](https://jira.mariadb.org/browse/MXS-2403) Masking filter should check subqueries +* [MXS-2402](https://jira.mariadb.org/browse/MXS-2402) Masking filter should check unions +* [MXS-2398](https://jira.mariadb.org/browse/MXS-2398) Recognize MariaDB specific executable comments +* [MXS-2396](https://jira.mariadb.org/browse/MXS-2396) Masking filter should examine user variables +* [MXS-2394](https://jira.mariadb.org/browse/MXS-2394) global setting "substitute_variables" is rejected as unknown global parameter +* [MXS-2393](https://jira.mariadb.org/browse/MXS-2393) Masking filter should check parse result of query classification. +* [MXS-2392](https://jira.mariadb.org/browse/MXS-2392) Masking filter should examine statement being prepared +* [MXS-2390](https://jira.mariadb.org/browse/MXS-2390) Masking and DBFW filters should reject statement prepared from variable +* [MXS-2389](https://jira.mariadb.org/browse/MXS-2389) Fix executable comment handling +* [MXS-2379](https://jira.mariadb.org/browse/MXS-2379) JSON Interface not work with Maxscale 2.3 +* [MXS-2374](https://jira.mariadb.org/browse/MXS-2374) Binlogfilter can break replication if last event is ignored +* [MXS-2373](https://jira.mariadb.org/browse/MXS-2373) Generated configs for filters does not include module +* [MXS-2370](https://jira.mariadb.org/browse/MXS-2370) Query timeout warning message does not print reason of timeout +* [MXS-2368](https://jira.mariadb.org/browse/MXS-2368) maxctrl requires password on command line and cannot change user password +* [MXS-2365](https://jira.mariadb.org/browse/MXS-2365) Wrong classification of queued queries in readwritesplit +* [MXS-2357](https://jira.mariadb.org/browse/MXS-2357) maxctrl documentation for alter service, include use_sql_variables_in +* [MXS-2355](https://jira.mariadb.org/browse/MXS-2355) MaxScale does not let mysql client 8.0.15 to connect with password +* [MXS-2342](https://jira.mariadb.org/browse/MXS-2342) maxadmin commands hang when master pod deleted after failover occurs +* [MXS-2337](https://jira.mariadb.org/browse/MXS-2337) schemarouter in 2.3.4 doesn't show all tables from all backends +* [MXS-2326](https://jira.mariadb.org/browse/MXS-2326) Routing hints are ignored when reconnection is required +* [MXS-2325](https://jira.mariadb.org/browse/MXS-2325) Disabled events are enabled on promoted slave upon failover +* [MXS-2323](https://jira.mariadb.org/browse/MXS-2323) Connections to servers in maintenance aren't closed +* [MXS-2292](https://jira.mariadb.org/browse/MXS-2292) Allow PAM user and group mapping to work with more specific host than '%' +* [MXS-1991](https://jira.mariadb.org/browse/MXS-1991) Why MariaDBMon complain about replication_user and replication_password? + +## Known Issues and Limitations + +There are some limitations and known issues within this version of MaxScale. +For more information, please refer to the [Limitations](../About/Limitations.md) document. + +## Packaging + +RPM and Debian packages are provided for supported the Linux distributions. + +Packages can be downloaded [here](https://mariadb.com/downloads/mariadb-tx/maxscale). + +## Source Code + +The source code of MaxScale is tagged at GitHub with a tag, which is identical +with the version of MaxScale. For instance, the tag of version X.Y.Z of MaxScale +is `maxscale-X.Y.Z`. Further, the default branch is always the latest GA version +of MaxScale. + +The source code is available [here](https://github.com/mariadb-corporation/MaxScale). From 24c763d701534fb038f7e8ffdbd803599103a997 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Thu, 28 Mar 2019 14:47:31 +0200 Subject: [PATCH 02/10] Update 2.3.5 release notes --- Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md index 90fec6725..e6e7eb34c 100644 --- a/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md +++ b/Documentation/Release-Notes/MaxScale-2.3.5-Release-Notes.md @@ -11,6 +11,7 @@ report on [our Jira](https://jira.mariadb.org/projects/MXS). ## Bug fixes * [MXS-2410](https://jira.mariadb.org/browse/MXS-2410) Hangup delivered to wrong DCB +* [MXS-2409](https://jira.mariadb.org/browse/MXS-2409) Schemarouter crashes if PREPARE or EXECUTE is malformed * [MXS-2403](https://jira.mariadb.org/browse/MXS-2403) Masking filter should check subqueries * [MXS-2402](https://jira.mariadb.org/browse/MXS-2402) Masking filter should check unions * [MXS-2398](https://jira.mariadb.org/browse/MXS-2398) Recognize MariaDB specific executable comments @@ -26,6 +27,7 @@ report on [our Jira](https://jira.mariadb.org/projects/MXS). * [MXS-2370](https://jira.mariadb.org/browse/MXS-2370) Query timeout warning message does not print reason of timeout * [MXS-2368](https://jira.mariadb.org/browse/MXS-2368) maxctrl requires password on command line and cannot change user password * [MXS-2365](https://jira.mariadb.org/browse/MXS-2365) Wrong classification of queued queries in readwritesplit +* [MXS-2359](https://jira.mariadb.org/browse/MXS-2359) LIKE clause in SHOW TABLES is ignored by schemarouter * [MXS-2357](https://jira.mariadb.org/browse/MXS-2357) maxctrl documentation for alter service, include use_sql_variables_in * [MXS-2355](https://jira.mariadb.org/browse/MXS-2355) MaxScale does not let mysql client 8.0.15 to connect with password * [MXS-2342](https://jira.mariadb.org/browse/MXS-2342) maxadmin commands hang when master pod deleted after failover occurs From 514dd963011143b4ecb30ac6f97c6dc34771572a Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 29 Mar 2019 10:09:38 +0200 Subject: [PATCH 03/10] MXS-2413 Parse 'DROP DATABASE [IF EXISTS] db' completely --- query_classifier/qc_sqlite/qc_sqlite.cc | 40 ++++++++++++++----- .../qc_sqlite/sqlite-src-3110100/src/parse.y | 8 +++- .../sqlite-src-3110100/src/sqliteInt.h | 1 + .../sqlite-src-3110100/tool/mkkeywordhash.c | 4 +- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index d57cbc4e3..bfc5c524a 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -2239,24 +2239,44 @@ public: m_type_mask = (QUERY_TYPE_WRITE | QUERY_TYPE_COMMIT); m_operation = QUERY_OP_DROP; - if (what == MXS_DROP_SEQUENCE) + switch (what) { - const char* zDatabase = NULL; - char database[pDatabase ? pDatabase->n + 1 : 1]; - - if (pDatabase) + case MXS_DROP_DATABASE: { +#ifdef TODO_SPECIFIC_OP_FOR_DROP_DATABASE_ADDED + // TODO: As there is only QUERY_OP_DROP, you can't be fully + // TODO: certain what a returned database actually refers to + // TODO: so better not to provide a name until there is a + // TODO: specific op. + char database[pDatabase->n + 1]; strncpy(database, pDatabase->z, pDatabase->n); database[pDatabase->n] = 0; - zDatabase = database; + update_database_names(database); +#endif } + break; - char table[pName->n + 1]; - strncpy(table, pName->z, pName->n); - table[pName->n] = 0; + case MXS_DROP_SEQUENCE: + { + const char* zDatabase = NULL; + char database[pDatabase ? pDatabase->n + 1 : 1]; - update_names(zDatabase, table, NULL, NULL); + if (pDatabase) + { + strncpy(database, pDatabase->z, pDatabase->n); + database[pDatabase->n] = 0; + + zDatabase = database; + } + + char table[pName->n + 1]; + strncpy(table, pName->z, pName->n); + table[pName->n] = 0; + + update_names(zDatabase, table, NULL, NULL); + } + break; } } diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y index e23ef97d6..67088e537 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/parse.y @@ -615,7 +615,7 @@ columnid(A) ::= nm(X). { // TODO: However, if not here then rules such as CAST need to be modified. BINARY /*CASCADE*/ CAST CLOSE COLUMNKW COLUMNS COMMENT CONCURRENT /*CONFLICT*/ - DATA /*DATABASE*/ DEALLOCATE DEFERRED /*DESC*/ /*DETACH*/ DUMPFILE + DATA DATABASE DEALLOCATE DEFERRED /*DESC*/ /*DETACH*/ DUMPFILE /*EACH*/ END ENGINE ENUM EXCLUSIVE /*EXPLAIN*/ FIRST FLUSH /*FOR*/ FORMAT GLOBAL @@ -2861,6 +2861,12 @@ eq_opt ::= EQ. default_opt ::= . default_opt ::= DEFAULT. +////////////////////////// DROP DATABASE statement ///////////////////////////////////// +// +cmd ::= DROP DATABASE ifexists id(X). { + maxscaleDrop(pParse, MXS_DROP_DATABASE, &X, NULL); +} + //////////////////////// CALL statement //////////////////////////////////// // cmd ::= call. diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/src/sqliteInt.h b/query_classifier/qc_sqlite/sqlite-src-3110100/src/sqliteInt.h index 2a19b2f77..fdc3217a6 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/src/sqliteInt.h +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/src/sqliteInt.h @@ -4094,6 +4094,7 @@ int sqlite3DbstatRegister(sqlite3*); typedef enum mxs_drop { + MXS_DROP_DATABASE, MXS_DROP_FUNCTION, MXS_DROP_SEQUENCE, } mxs_drop_t; diff --git a/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c b/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c index 18017915c..675c543c4 100644 --- a/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c +++ b/query_classifier/qc_sqlite/sqlite-src-3110100/tool/mkkeywordhash.c @@ -216,8 +216,10 @@ static Keyword aKeywordTable[] = { { "CURRENT_TIMESTAMP","TK_CTIME_KW", ALWAYS }, #ifdef MAXSCALE { "DATA", "TK_DATA", ALWAYS }, -#endif + { "DATABASE", "TK_DATABASE", ALWAYS }, +#else { "DATABASE", "TK_DATABASE", ATTACH }, +#endif #ifdef MAXSCALE { "DATABASES", "TK_DATABASES_KW", ALWAYS }, { "DEALLOCATE", "TK_DEALLOCATE", ALWAYS }, From 4f8d6d185395ae9def101cb6543e382169eaf481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 28 Mar 2019 16:28:43 +0200 Subject: [PATCH 04/10] Fix transaction_replay documentation The documentation stated that MVCC reads weren't safe with transaction_replay when in reality they are not safe in general and transaction_replay has no effect on it. --- Documentation/Routers/ReadWriteSplit.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Documentation/Routers/ReadWriteSplit.md b/Documentation/Routers/ReadWriteSplit.md index 3eef90e3f..cd108bb94 100644 --- a/Documentation/Routers/ReadWriteSplit.md +++ b/Documentation/Routers/ReadWriteSplit.md @@ -438,11 +438,8 @@ failure of a master node without any visible effects to the client. If no replacement node becomes available before the timeout controlled by `delayed_retry_timeout` is exceeded, the client connection is closed. -Not all transactions can be safely replayed. Only when the following criteria -are met, the transaction can be safely replayed. - -* Transaction contains only data modification (`INSERT`, `UPDATE`, `DELETE` - etc.) or `SELECT ... FOR UPDATE` statements. +Only when the following criteria are met, the transaction can be safely +replayed. * The replacement server where the transaction is applied returns results identical to the original partial transaction. @@ -451,10 +448,11 @@ If the results from the replacement server are not identical when the transactio replayed, the client connection is closed. This means that any transaction with a server specific result (e.g. `NOW()`, `@@server_id`) cannot be replayed successfully. -Performing MVCC reads (`SELECT` queries without `FOR UPDATE` or `LOCK IN SHARE MODE`) -with transaction replay is discouraged. If such statements are executed -but the results of each reply are identical, the transaction is replayed but the results -are not guaranteed to be consistent on the database level. +It is recommended that the transaction contains only data modification +(`INSERT`, `UPDATE`, `DELETE` etc.) or `SELECT ... FOR UPDATE` +statements. Performing MVCC reads (`SELECT` queries without `FOR UPDATE` or +`LOCK IN SHARE MODE`) is discouraged as it doesn't guarantee strict ordering of +events. ### `transaction_replay_max_size` From 48791c38770dafed037ef51d7138e2b19293cea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 29 Mar 2019 07:22:11 +0200 Subject: [PATCH 05/10] Fix duplication of the router parameter The service parameter list had two router entries in it due to the assumption that the parameter list never contained the router itself in it. --- server/core/service.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/core/service.cc b/server/core/service.cc index b7ba9947f..ad8556e80 100644 --- a/server/core/service.cc +++ b/server/core/service.cc @@ -106,12 +106,12 @@ Service* service_alloc(const char* name, const char* router, MXS_CONFIG_PARAMETE dcb_enable_session_timeouts(); } - // Store router, used when service is serialized - service_add_parameter(service, CN_ROUTER, router); - // Store parameters in the service service_add_parameters(service, params); + // Store router, used when service is serialized + service_replace_parameter(service, CN_ROUTER, router); + service->router_instance = router_api->createInstance(service, params); if (service->router_instance == NULL) From 42e658d91fb022a4146ff68d439139e882d1a2ab Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Fri, 29 Mar 2019 10:47:56 +0200 Subject: [PATCH 06/10] MXS-2403 Fix test program ERROR 1248 (42000): Every derived table must have its own alias --- maxscale-system-test/masking_auto_firewall.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/maxscale-system-test/masking_auto_firewall.cpp b/maxscale-system-test/masking_auto_firewall.cpp index 554d1addb..fd7e7a64d 100644 --- a/maxscale-system-test/masking_auto_firewall.cpp +++ b/maxscale-system-test/masking_auto_firewall.cpp @@ -121,13 +121,13 @@ void run(TestConnections& test) test_one(test, "select 1 UNION select * FROM masking_auto_firewall", Expect::FAILURE); // This SHOULD succeed as a masked column is not used in the statment. - test_one(test, "select * FROM (select b from masking_auto_firewall)", Expect::SUCCESS); + test_one(test, "select * FROM (select b from masking_auto_firewall) tbl", Expect::SUCCESS); // This SHOULD succeed as a masked column is not used in the statment. - test_one(test, "select * FROM (select a as b from masking_auto_firewall)", Expect::FAILURE); + test_one(test, "select * FROM (select a as b from masking_auto_firewall) tbl", Expect::FAILURE); // This SHOULD succeed as '*' is used in the statment. - test_one(test, "select * FROM (select * from masking_auto_firewall)", Expect::FAILURE); + test_one(test, "select * FROM (select * from masking_auto_firewall) tbl", Expect::FAILURE); } } From 3977207d12c25d1ce5e02f4625d7d738782ee2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 29 Mar 2019 13:56:53 +0200 Subject: [PATCH 07/10] Update transaction_replay documentation Removed the confusing paragraph. --- Documentation/Routers/ReadWriteSplit.md | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/Documentation/Routers/ReadWriteSplit.md b/Documentation/Routers/ReadWriteSplit.md index cd108bb94..8858ee4ef 100644 --- a/Documentation/Routers/ReadWriteSplit.md +++ b/Documentation/Routers/ReadWriteSplit.md @@ -438,21 +438,10 @@ failure of a master node without any visible effects to the client. If no replacement node becomes available before the timeout controlled by `delayed_retry_timeout` is exceeded, the client connection is closed. -Only when the following criteria are met, the transaction can be safely -replayed. - -* The replacement server where the transaction is applied returns results - identical to the original partial transaction. - -If the results from the replacement server are not identical when the transaction is -replayed, the client connection is closed. This means that any transaction with a server -specific result (e.g. `NOW()`, `@@server_id`) cannot be replayed successfully. - -It is recommended that the transaction contains only data modification -(`INSERT`, `UPDATE`, `DELETE` etc.) or `SELECT ... FOR UPDATE` -statements. Performing MVCC reads (`SELECT` queries without `FOR UPDATE` or -`LOCK IN SHARE MODE`) is discouraged as it doesn't guarantee strict ordering of -events. +If the results from the replacement server are not identical when the +transaction is replayed, the client connection is closed. This means that any +transaction with a server specific result (e.g. `NOW()`, `@@server_id`) cannot +be replayed successfully but it will still be attempted. ### `transaction_replay_max_size` From 5346b24fa40fe49d6e27067604dc5f594d1c6eda Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 1 Apr 2019 09:57:40 +0300 Subject: [PATCH 08/10] MXS-2393 Add parameter 'require_fully_parsed' If set to true and if any of the other blocking related parameters is true, then a statement that cannot be fully parsed will be blocked. Default is true. --- Documentation/Filters/Masking.md | 29 +++++++++ .../modules/filter/masking/maskingfilter.cc | 6 ++ .../filter/masking/maskingfilterconfig.cc | 63 +++++++++---------- .../filter/masking/maskingfilterconfig.hh | 16 +++++ .../filter/masking/maskingfiltersession.cc | 6 +- 5 files changed, 85 insertions(+), 35 deletions(-) diff --git a/Documentation/Filters/Masking.md b/Documentation/Filters/Masking.md index c055b696a..05a763015 100644 --- a/Documentation/Filters/Masking.md +++ b/Documentation/Filters/Masking.md @@ -87,6 +87,15 @@ SELECT revealed_ssn FROM cheat; ``` to get access to the cleartext version of a masked field `ssn`. +From MaxScale 2.3.5 onwards, the masking filter will, if any of the +`prevent_function_usage`, `check_user_variables`, `check_unions` or +`check_subqueries` parameters is set to true, block statements that +cannot be fully parsed. + +Please see the configuration parameter +[require_fully_parsed](#require_fully_parsed) +for how to change the default behaviour. + ## Limitations The masking filter can _only_ be used for masking columns of the following @@ -186,6 +195,26 @@ prevent_function_usage=false ``` The default value is `true`. +#### `require_fully_parsed` + +This optional parameter specifies how the masking filter should +behave in case any of `prevent_function_usage`, `check_user_variables`, +`check_unions` or `check_subqueries` is true and it encounters a +statement that cannot be fully parsed, + +If true, then statements that cannot be fully parsed (due to a +parser limitation) will be blocked. +``` +require_fully_parsed=false +``` + +The default value is `true`. + +Note that if this parameter is set to false, then `prevent_function_usage`, +`check_user_variables`, `check_unions` and `check_subqueries` are rendered +less effective, as it with a statement that can not be fully parsed may be +possible to bypass the protection that they are intended to provide. + #### `check_user_variables` This optional parameter specifies how the masking filter should diff --git a/server/modules/filter/masking/maskingfilter.cc b/server/modules/filter/masking/maskingfilter.cc index 5a3eb5be2..e33d55ebc 100644 --- a/server/modules/filter/masking/maskingfilter.cc +++ b/server/modules/filter/masking/maskingfilter.cc @@ -139,6 +139,12 @@ extern "C" MXS_MODULE* MXS_CREATE_MODULE() Config::check_subqueries_default, MXS_MODULE_OPT_NONE, }, + { + Config::require_fully_parsed_name, + MXS_MODULE_PARAM_BOOL, + Config::require_fully_parsed_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 3f6f2b81c..eab0ed540 100644 --- a/server/modules/filter/masking/maskingfilterconfig.cc +++ b/server/modules/filter/masking/maskingfilterconfig.cc @@ -17,21 +17,23 @@ namespace { -const char config_name_large_payload[] = "large_payload"; -const char config_name_rules[] = "rules"; -const char config_name_warn_type_mismatch[] = "warn_type_mismatch"; - -const char config_value_abort[] = "abort"; -const char config_value_ignore[] = "ignore"; -const char config_value_never[] = "never"; -const char config_value_always[] = "always"; - +const char config_name_check_subqueries[] = "check_subqueries"; +const char config_name_check_unions[] = "check_unions"; +const char config_name_check_user_variables[] = "check_user_variables"; +const char config_name_large_payload[] = "large_payload"; const char config_name_prevent_function_usage[] = "prevent_function_usage"; -const char config_check_user_variables[] = "check_user_variables"; -const char config_check_unions[] = "check_unions"; -const char config_check_subqueries[] = "check_subqueries"; +const char config_name_require_fully_parsed[] = "require_fully_parsed"; +const char config_name_rules[] = "rules"; +const char config_name_warn_type_mismatch[] = "warn_type_mismatch"; + + +const char config_value_abort[] = "abort"; +const char config_value_always[] = "always"; +const char config_value_ignore[] = "ignore"; +const char config_value_never[] = "never"; const char config_value_true[] = "true"; + } /* @@ -63,10 +65,9 @@ const char* MaskingFilterConfig::rules_name = config_name_rules; * PARAM warn_type_mismatch */ -// static const char* MaskingFilterConfig::warn_type_mismatch_name = config_name_warn_type_mismatch; +const char* MaskingFilterConfig::warn_type_mismatch_default = config_value_never; -// static const MXS_ENUM_VALUE MaskingFilterConfig::warn_type_mismatch_values[] = { {config_value_never, MaskingFilterConfig::WARN_NEVER }, @@ -74,46 +75,36 @@ const MXS_ENUM_VALUE MaskingFilterConfig::warn_type_mismatch_values[] = {NULL} }; -// 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; /* * PARAM check_user_variables */ -// static -const char* MaskingFilterConfig::check_user_variables_name = config_check_user_variables; - -// static +const char* MaskingFilterConfig::check_user_variables_name = config_name_check_user_variables; const char* MaskingFilterConfig::check_user_variables_default = config_value_true; /* * PARAM check_unions */ -// static -const char* MaskingFilterConfig::check_unions_name = config_check_unions; - -// static +const char* MaskingFilterConfig::check_unions_name = config_name_check_unions; const char* MaskingFilterConfig::check_unions_default = config_value_true; /* * PARAM check_subqueries */ -// static -const char* MaskingFilterConfig::check_subqueries_name = config_check_subqueries; - -// static +const char* MaskingFilterConfig::check_subqueries_name = config_name_check_subqueries; const char* MaskingFilterConfig::check_subqueries_default = config_value_true; +/* + * PARAM require_fully_parsed + */ +const char* MaskingFilterConfig::require_fully_parsed_name = config_name_require_fully_parsed; +const char* MaskingFilterConfig::require_fully_parsed_default = config_name_require_fully_parsed; + /* * MaskingFilterConfig @@ -164,3 +155,9 @@ bool MaskingFilterConfig::get_check_subqueries(const MXS_CONFIG_PARAMETER* pPara { return config_get_bool(pParams, check_subqueries_name); } + +// static +bool MaskingFilterConfig::get_require_fully_parsed(const MXS_CONFIG_PARAMETER* pParams) +{ + return config_get_bool(pParams, require_fully_parsed_name); +} diff --git a/server/modules/filter/masking/maskingfilterconfig.hh b/server/modules/filter/masking/maskingfilterconfig.hh index 20c3bb3a7..9b4cb6f37 100644 --- a/server/modules/filter/masking/maskingfilterconfig.hh +++ b/server/modules/filter/masking/maskingfilterconfig.hh @@ -54,6 +54,9 @@ public: static const char* check_subqueries_name; static const char* check_subqueries_default; + static const char* require_fully_parsed_name; + static const char* require_fully_parsed_default; + MaskingFilterConfig(const char* zName, const MXS_CONFIG_PARAMETER* pParams) : m_name(zName) , m_large_payload(get_large_payload(pParams)) @@ -63,6 +66,7 @@ public: , m_check_user_variables(get_check_user_variables(pParams)) , m_check_unions(get_check_unions(pParams)) , m_check_subqueries(get_check_subqueries(pParams)) + , m_require_fully_parsed(get_require_fully_parsed(pParams)) { } @@ -110,6 +114,11 @@ public: return m_check_subqueries; } + bool require_fully_parsed() const + { + return m_require_fully_parsed; + } + void set_large_payload(large_payload_t l) { m_large_payload = l; @@ -144,6 +153,11 @@ public: m_check_subqueries = b; } + void set_require_fully_parsed(bool b) + { + m_require_fully_parsed = b; + } + bool is_parsing_needed() const { return prevent_function_usage() || check_user_variables() || check_unions() || check_subqueries(); @@ -156,6 +170,7 @@ public: static bool get_check_user_variables(const MXS_CONFIG_PARAMETER* pParams); static bool get_check_unions(const MXS_CONFIG_PARAMETER* pParams); static bool get_check_subqueries(const MXS_CONFIG_PARAMETER* pParams); + static bool get_require_fully_parsed(const MXS_CONFIG_PARAMETER* pParams); private: std::string m_name; @@ -166,4 +181,5 @@ private: bool m_check_user_variables; bool m_check_unions; bool m_check_subqueries; + bool m_require_fully_parsed; }; diff --git a/server/modules/filter/masking/maskingfiltersession.cc b/server/modules/filter/masking/maskingfiltersession.cc index db29f53e8..b9614f046 100644 --- a/server/modules/filter/masking/maskingfiltersession.cc +++ b/server/modules/filter/masking/maskingfiltersession.cc @@ -127,7 +127,8 @@ bool MaskingFilterSession::check_textual_query(GWBUF* pPacket) { bool rv = false; - if (qc_parse(pPacket, QC_COLLECT_FIELDS | QC_COLLECT_FUNCTIONS) == QC_QUERY_PARSED) + if (qc_parse(pPacket, QC_COLLECT_FIELDS | QC_COLLECT_FUNCTIONS) == QC_QUERY_PARSED + || !m_filter.config().require_fully_parsed()) { if (qc_query_is_type(qc_get_type_mask(pPacket), QUERY_TYPE_PREPARE_NAMED_STMT)) { @@ -165,7 +166,8 @@ bool MaskingFilterSession::check_binary_query(GWBUF* pPacket) { bool rv = false; - if (qc_parse(pPacket, QC_COLLECT_FIELDS | QC_COLLECT_FUNCTIONS) == QC_QUERY_PARSED) + if (qc_parse(pPacket, QC_COLLECT_FIELDS | QC_COLLECT_FUNCTIONS) == QC_QUERY_PARSED + || !m_filter.config().require_fully_parsed()) { rv = check_query(pPacket); } From 06c01439fbf0de9cd60cf619429ec76b147a67ea Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 1 Apr 2019 10:04:53 +0300 Subject: [PATCH 09/10] MXS-2393 Fix masking test The masking_user test creates a database over a masked connection. As 'CREATE DATABASE DB' is not fully parsed the test will fail since it creates a database. To allow the test to pass, we turn off the strict requirement that all statements must be fully parsed. --- maxscale-system-test/cnf/maxscale.cnf.template.masking_mysqltest | 1 + 1 file changed, 1 insertion(+) diff --git a/maxscale-system-test/cnf/maxscale.cnf.template.masking_mysqltest b/maxscale-system-test/cnf/maxscale.cnf.template.masking_mysqltest index 132f04be1..75a1c8acf 100644 --- a/maxscale-system-test/cnf/maxscale.cnf.template.masking_mysqltest +++ b/maxscale-system-test/cnf/maxscale.cnf.template.masking_mysqltest @@ -71,6 +71,7 @@ socket=default type=filter module=masking rules=/###access_homedir###/masking_rules.json +require_fully_parsed=false [server1] type=server From 738ae9178b16c332300c810396bf362429f38d38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 1 Apr 2019 11:25:09 +0300 Subject: [PATCH 10/10] Fix binlogfilter matching The matching always checked the default database when it should only check it if there are no tables in the statement. --- server/modules/filter/binlogfilter/binlogfiltersession.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/modules/filter/binlogfilter/binlogfiltersession.cc b/server/modules/filter/binlogfilter/binlogfiltersession.cc index 24c5faad8..e556a90c3 100644 --- a/server/modules/filter/binlogfilter/binlogfiltersession.cc +++ b/server/modules/filter/binlogfilter/binlogfiltersession.cc @@ -405,10 +405,11 @@ static bool should_skip_query(const BinlogConfig& config, const std::string& sql qc_free_table_names(names, n); } - // Also check for the default database in case the query has no tables in it - if (!rval && should_skip(config, db)) + // Also check for the default database in case the query has no tables in it. The dot at the end is + // required to distinct database names from table names. + if (n == 0) { - rval = true; + rval = should_skip(config, db + '.'); } gwbuf_free(buf);