From cc9b0db5e91bfea105c33433d8bca8435eb37934 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 24 Mar 2015 19:32:17 +0200 Subject: [PATCH 01/27] Added logging about authentication errors. --- server/modules/protocol/mysql_common.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index f3665cd81..3cb350b1b 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1530,14 +1530,20 @@ int gw_find_mysql_user_password_sha1(char *username, uint8_t *gateway_password, * user@% not found. */ - LOGIF(LD, - (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [MySQL Client Auth], user [%s@%s] not existent", - pthread_self(), - key.user, - dcb->remote))); - break; + LOGIF(LD, + (skygw_log_write_flush( + LOGFILE_DEBUG, + "%lu [MySQL Client Auth], user [%s@%s] not existent", + pthread_self(), + key.user, + dcb->remote))); + + skygw_log_write_flush( + LOGFILE_ERROR, + "Authentication Failed: user [%s@%s] not found.", + key.user, + dcb->remote); + break; } break; From 5eb223b92eb8f0c94e8c3d891ebb53ebedb8f5e9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 24 Mar 2015 20:00:45 +0200 Subject: [PATCH 02/27] Changed /dev/shm folder names to maxscale. instead of only . --- log_manager/log_manager.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index b18e10e26..2c6f46348 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -2507,6 +2507,7 @@ static bool logfile_init( char* c; pid_t pid = getpid(); int len = strlen(shm_pathname_prefix)+ + + strlen("maxscale.") + get_decimal_len((size_t)pid) + 1; c = (char *)calloc(len, sizeof(char)); @@ -2516,7 +2517,7 @@ static bool logfile_init( succp = false; goto return_with_succp; } - sprintf(c, "%s%d", shm_pathname_prefix, pid); + sprintf(c, "%smaxscale.%d", shm_pathname_prefix, pid); logfile->lf_filepath = c; if (mkdir(c, S_IRWXU | S_IRWXG) != 0 && From 5c68782050f961185fe96487069db9cb0300a366 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 24 Mar 2015 21:28:03 +0200 Subject: [PATCH 03/27] Added more trace logging to readcounnroute, schemarouter and tee. --- server/modules/filter/tee.c | 7 ++++ server/modules/routing/readconnroute.c | 32 +++++++++++++++++-- .../routing/schemarouter/schemarouter.c | 8 +++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index 8ea2dcc00..e337df726 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -1063,6 +1063,7 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) if(!my_session->active) { + skygw_log_write(LOGFILE_TRACE,"Tee: Failed to return reply, session is closed"); gwbuf_free(reply); rc = 0; if(my_session->waiting[PARENT]) @@ -1104,6 +1105,7 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) if(my_session->replies[branch] == 0) { + skygw_log_write(LOGFILE_TRACE,"Tee: First reply to a query for [%s].",branch == PARENT ? "PARENT":"CHILD"); /* Reply is in a single packet if it is an OK, ERR or LOCAL_INFILE packet. * Otherwise the reply is a result set and the amount of packets is unknown. */ @@ -1116,6 +1118,11 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) { flags = get_response_flags(ptr,true); more_results = (flags & 0x08) && my_session->client_multistatement; + if(more_results) + { + skygw_log_write(LOGFILE_TRACE, + "Tee: [%s] waiting for more results.",branch == PARENT ? "PARENT":"CHILD"); + } } } #ifdef SS_DEBUG diff --git a/server/modules/routing/readconnroute.c b/server/modules/routing/readconnroute.c index f9d12c512..16aa637ca 100644 --- a/server/modules/routing/readconnroute.c +++ b/server/modules/routing/readconnroute.c @@ -89,6 +89,8 @@ #include +#include "modutil.h" + /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; @@ -535,6 +537,7 @@ BACKEND *master_host = NULL; pthread_self(), candidate->server->port, candidate->current_connection_count))); + /* * Open a backend connection, putting the DCB for this * connection in the client_rses->backend_dcb @@ -564,7 +567,13 @@ BACKEND *master_host = NULL; spinlock_release(&inst->lock); CHK_CLIENT_RSES(client_rses); - + + skygw_log_write( + LOGFILE_TRACE, + "Readconnroute: New session for server %s. " + "Connections : %d", + candidate->server->unique_name, + candidate->current_connection_count); return (void *)client_rses; } @@ -718,10 +727,19 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) "Error : Failed to route MySQL command %d to backend " "server.", mysql_command))); + skygw_log_write( + LOGFILE_ERROR, + "Error : Failed to route MySQL command %d to backend " + "server %s.", + mysql_command, + router_cli_ses->backend->server->unique_name); rc = 0; goto return_rc; + } + char* trc = NULL; + switch(mysql_command) { case MYSQL_COM_CHANGE_USER: rc = backend_dcb->func.auth( @@ -730,7 +748,8 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) backend_dcb->session, queue); break; - + case MYSQL_COM_QUERY: + trc = modutil_get_SQL(queue); default: rc = backend_dcb->func.write(backend_dcb, queue); break; @@ -745,6 +764,15 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) mysql_command, backend_dcb, rc))); + + skygw_log_write( + LOGFILE_TRACE, + "Routed command [%#x] to '%s'%s%s", + mysql_command, + backend_dcb->server->unique_name, + trc?": ":".", + trc?trc:""); + free(trc); return_rc: return rc; } diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index a386cebe4..9587f3bc4 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -514,6 +514,10 @@ char* get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, */ rval = (char*) hashtable_fetch(ht, client->rses_mysql_session->db); + if(rval) + { + skygw_log_write(LOGFILE_TRACE,"schemarouter: Using active database '%s'",client->rses_mysql_session->db); + } } return rval; @@ -1932,6 +1936,7 @@ static int routeQuery( */ route_target = TARGET_ANY; + skygw_log_write(LOGFILE_TRACE,"schemarouter: Routing query to first available backend."); } else @@ -2008,6 +2013,9 @@ static int routeQuery( /**No valid backends alive*/ skygw_log_write(LOGFILE_TRACE,"schemarouter: No backends are running"); + skygw_log_write(LOGFILE_ERROR, + "Error: Schemarouter: Failed to route query, " + "no backends are available."); rses_end_locked_router_action(router_cli_ses); ret = 0; goto retblock; From 298bbfbc58e1b5546c5d8f4ea9d321451ad5dfea Mon Sep 17 00:00:00 2001 From: Kolbe Kegel Date: Tue, 24 Mar 2015 20:09:24 -0700 Subject: [PATCH 04/27] ... --- Documentation/filters/Database-Firewall-Filter.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/filters/Database-Firewall-Filter.md b/Documentation/filters/Database-Firewall-Filter.md index 9dc415a8a..f1596143f 100644 --- a/Documentation/filters/Database-Firewall-Filter.md +++ b/Documentation/filters/Database-Firewall-Filter.md @@ -1,11 +1,11 @@ #Database Firewall filter ## Overview -The database firewall filter is used to block queries that match a set of rules. It can be used to prevent harmful queries into the database or to limit the access to the database based on a more defined set of rules compared to the traditional GRANT-based rights management. +The database firewall filter is used to block queries that match a set of rules. It can be used to prevent harmful queries from reaching the backend database instances or to limit access to the database based on a more flexible set of rules compared to the traditional GRANT-based privilege system. ## Configuration -The database firewall filter only requires a minimal set of configurations in the MaxScale.cnf file. The actual rules of the database firewall filter are located in a separate text file. The following is an example of a database firewall filter configuration in the MaxScale.cnf file. +The database firewall filter only requires minimal configuration in the MaxScale.cnf file. The actual rules of the database firewall filter are located in a separate text file. The following is an example of a database firewall filter configuration in MaxScale.cnf. ``` [Database Firewall] @@ -24,7 +24,7 @@ The database firewall filter has one mandatory parameter that defines the locati ## Rule syntax -The rules are defined by using the following syntax. +The rules are defined by using the following syntax: ``` rule NAME deny [wildcard | columns VALUE ... | @@ -58,7 +58,7 @@ The limit_queries rule expects three parameters. The first parameter is the numb #### No_where_clause -This rule inspects the query and blocks it if it has no where clause. This way you can't do a DELETE FROM ... query without having the where clause. This does not prevent wrongful usage of the where clause e.g. DELETE FROM ... WHERE 1=1. +This rule inspects the query and blocks it if it has no WHERE clause. For example, this would disallow a DELETE FROM ... query without a WHERE clause. This does not prevent wrongful usage of the WHERE clause e.g. DELETE FROM ... WHERE 1=1. ### Optional rule parameters @@ -66,7 +66,7 @@ Each mandatory rule accepts one or more optional parameters. These are to be def #### At_times -This rule expects a list of time ranges that define the times when the rule in question is active. The time formats are expected to be ISO-8601 compliant and to be separated by a single dash (the - character). For example defining the active period of a rule to be 17:00 to 19:00 you would add `at times 17:00:00-19:00:00` to the end of the rule. +This rule expects a list of time ranges that define the times when the rule in question is active. The time formats are expected to be ISO-8601 compliant and to be separated by a single dash (the - character). For example, to define the active period of a rule to be 5pm to 7pm, you would use `at times 17:00:00-19:00:00` to the end of the rule. #### On_queries @@ -78,7 +78,7 @@ To apply the defined rules to users use the following syntax. `users NAME ... match [any|all|strict_all] rules RULE ...` -The first keyword is users which identifies this line as a user definition line. After this a list of user names and network addresses in the format `user@0.0.0.0` is expected. The first part is the user name and the second part is the network address. You can use the `%` character as the wildcard to enable user name matching from any address or network matching for all users. After the list of users and networks the keyword match is expected. +The first keyword is `users`, which identifies this line as a user definition line. After this, a list of user names and network addresses in the format `user@0.0.0.0` is expected. The first part is the user name and the second part is the network address. You can use the `%` character as the wildcard to enable user name matching from any address or network matching for all users. After the list of users and networks the keyword match is expected. After this either the keyword `any` `all` or `strict_all` is expected. This defined how the rules are matched. If `any` is used when the first rule is matched the query is considered blocked and the rest of the rules are skipped. If instead the `all` keyword is used all rules must match for the query to be blocked. The `strict_all` is the same as `all` but it checks the rules from left to right in the order they were listed. If one of these does not match, the rest of the rules are not checked. This could be usedful in situations where you would for example combine `limit_queries` and `regex` rules. By using `strict_all` you can have the `regex` rule first and the `limit_queries` rule second. This way the rule only matches if the `regex` rule matches enough times for the `limit_queries` rule to match. From 2a3c570cf56ea7a491724dc639d73eb03bd9f2df Mon Sep 17 00:00:00 2001 From: Kolbe Kegel Date: Tue, 24 Mar 2015 20:26:12 -0700 Subject: [PATCH 05/27] ... --- .../filters/Database-Firewall-Filter.md | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/Documentation/filters/Database-Firewall-Filter.md b/Documentation/filters/Database-Firewall-Filter.md index f1596143f..b08025857 100644 --- a/Documentation/filters/Database-Firewall-Filter.md +++ b/Documentation/filters/Database-Firewall-Filter.md @@ -29,7 +29,7 @@ The rules are defined by using the following syntax: ``` rule NAME deny [wildcard | columns VALUE ... | regex REGEX | limit_queries COUNT TIMEPERIOD HOLDOFF | - no_where_clause] [at_times VALUE...] [on_queries [select|update|insert|delete]]` + no_where_clause] [at_times VALUE...] [on_queries [select|update|insert|delete]] ``` Rules always define a blocking action so the basic mode for the database firewall filter is to allow all queries that do not match a given set of rules. Rules are identified by their name and have a mandatory part and optional parts. @@ -40,45 +40,47 @@ The first step of defining a rule is to start with the keyword `rule` which iden The database firewall filter's rules expect a single mandatory parameter for a rule. You can define multiple rules to cover situations where you would like to apply multiple mandatory rules to a query. -#### Wildcard +#### wildcard This rule blocks all queries that use the wildcard character *. -#### Columns +#### columns This rule expects a list of values after the `columns` keyword. These values are interpreted as column names and if a query targets any of these, it is blocked. -#### Regex +#### regex This rule blocks all queries matching a regex enclosed in single or double quotes. -#### Limit_queries +#### limit_queries The limit_queries rule expects three parameters. The first parameter is the number of allowed queries during the time period. The second is the time period in seconds and the third is the amount of time for which the rule is considered active and blocking. -#### No_where_clause +#### no_where_clause -This rule inspects the query and blocks it if it has no WHERE clause. For example, this would disallow a DELETE FROM ... query without a WHERE clause. This does not prevent wrongful usage of the WHERE clause e.g. DELETE FROM ... WHERE 1=1. +This rule inspects the query and blocks it if it has no WHERE clause. For example, this would disallow a `DELETE FROM ...` query without a `WHERE` clause. This does not prevent wrongful usage of the `WHERE` clause e.g. `DELETE FROM ... WHERE 1=1`. ### Optional rule parameters Each mandatory rule accepts one or more optional parameters. These are to be defined after the mandatory part of the rule. -#### At_times +#### at_times -This rule expects a list of time ranges that define the times when the rule in question is active. The time formats are expected to be ISO-8601 compliant and to be separated by a single dash (the - character). For example, to define the active period of a rule to be 5pm to 7pm, you would use `at times 17:00:00-19:00:00` to the end of the rule. +This rule expects a list of time ranges that define the times when the rule in question is active. The time formats are expected to be ISO-8601 compliant and to be separated by a single dash (the - character). For example, to define the active period of a rule to be 5pm to 7pm, you would include `at times 17:00:00-19:00:00` in the rule definition. -#### On_queries +#### on_queries This limits the rule to be active only on certain types of queries. ### Applying rules to users -To apply the defined rules to users use the following syntax. +The `users` directive defines the users to which the rule should be applied. -`users NAME ... match [any|all|strict_all] rules RULE ...` +`users NAME ... match [any|all|strict_all] rules RULE [,...]` -The first keyword is `users`, which identifies this line as a user definition line. After this, a list of user names and network addresses in the format `user@0.0.0.0` is expected. The first part is the user name and the second part is the network address. You can use the `%` character as the wildcard to enable user name matching from any address or network matching for all users. After the list of users and networks the keyword match is expected. +The first keyword is `users`, which identifies this line as a user definition line. + +The second component is a list of user names and network addresses in the format *`user`*`@`*`0.0.0.0`*. The first part is the user name and the second part is the network address. You can use the `%` character as the wildcard to enable user name matching from any address or network matching for all users. After the list of users and networks the keyword match is expected. After this either the keyword `any` `all` or `strict_all` is expected. This defined how the rules are matched. If `any` is used when the first rule is matched the query is considered blocked and the rest of the rules are skipped. If instead the `all` keyword is used all rules must match for the query to be blocked. The `strict_all` is the same as `all` but it checks the rules from left to right in the order they were listed. If one of these does not match, the rest of the rules are not checked. This could be usedful in situations where you would for example combine `limit_queries` and `regex` rules. By using `strict_all` you can have the `regex` rule first and the `limit_queries` rule second. This way the rule only matches if the `regex` rule matches enough times for the `limit_queries` rule to match. From 8c36a45c69524b17094e25a562b0f890c983a6e9 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 25 Mar 2015 11:18:46 +0200 Subject: [PATCH 06/27] Updated some logging to be only done if the logfiles are enabled. --- server/modules/routing/readconnroute.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/modules/routing/readconnroute.c b/server/modules/routing/readconnroute.c index 16aa637ca..783fb5d8b 100644 --- a/server/modules/routing/readconnroute.c +++ b/server/modules/routing/readconnroute.c @@ -749,7 +749,7 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) queue); break; case MYSQL_COM_QUERY: - trc = modutil_get_SQL(queue); + LOGIF(LOGFILE_TRACE,(trc = modutil_get_SQL(queue))); default: rc = backend_dcb->func.write(backend_dcb, queue); break; @@ -765,13 +765,13 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) backend_dcb, rc))); - skygw_log_write( + LOGIF(LOGFILE_TRACE,skygw_log_write( LOGFILE_TRACE, "Routed command [%#x] to '%s'%s%s", mysql_command, backend_dcb->server->unique_name, trc?": ":".", - trc?trc:""); + trc?trc:"")); free(trc); return_rc: return rc; From f870d3bf148b00e5ba6b75efebf063aa93450265 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 25 Mar 2015 11:23:51 +0200 Subject: [PATCH 07/27] Added more alternative logging. --- server/modules/protocol/mysql_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 3cb350b1b..2365ebbec 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1538,11 +1538,11 @@ int gw_find_mysql_user_password_sha1(char *username, uint8_t *gateway_password, key.user, dcb->remote))); - skygw_log_write_flush( + LOGIF(LT,skygw_log_write_flush( LOGFILE_ERROR, "Authentication Failed: user [%s@%s] not found.", key.user, - dcb->remote); + dcb->remote)); break; } From c70f4a65a59cb63c22094878f298aabbf6fb9d2d Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Wed, 25 Mar 2015 16:44:22 +0100 Subject: [PATCH 08/27] Nagios tPlugins tutorial added to doc index Nagios tPlugins tutorial added to doc index --- Documentation/Documentation-Contents.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index 56c8f30f2..159b55229 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -37,6 +37,7 @@ - [MySQL Cluster Setup](Tutorials/MySQL-Cluster-Setup.md) - [Replication Proxy with the Binlog Router Tutorial](Tutorials/Replication-Proxy-Binlog-Router-Tutorial.md) - [RabbitMQ Setup and MaxScale Integration Tutorial](Tutorials/RabbitMQ-Setup-And-MaxScale-Integration.md) + - [Nagios Plugins for MaxScale Tutorial](Tutorials/Nagios-Plugins.md) ## Filters From 39fc889dda30ff06175a1d9f094122aff4d94111 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 26 Mar 2015 05:42:43 +0200 Subject: [PATCH 09/27] Removed unnecessary headers from dbfwfilter. --- server/modules/filter/dbfwfilter.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index 63d07b5a9..5961d97f1 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -61,11 +61,9 @@ * users NAME ... match [any|all|strict_all] rules RULE ... *@endcode */ + #include -#include -#include #include -#include #include #include #include @@ -74,13 +72,11 @@ #include #include #include -#include -#include #include -#include #include #include #include + MODULE_INFO info = { MODULE_API_FILTER, MODULE_ALPHA_RELEASE, From 7307af09a5cc50b3aa6f2437256a188a7800c259 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 26 Mar 2015 06:02:32 +0200 Subject: [PATCH 10/27] Fixed typo in Building-MaxScale-from-Source-Code.md --- .../Getting-Started/Building-MaxScale-from-Source-Code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md index b5f43a658..cd0ed989f 100644 --- a/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md +++ b/Documentation/Getting-Started/Building-MaxScale-from-Source-Code.md @@ -225,7 +225,7 @@ Other useful targets for Make are `documentation`, which generates the Doxygen d MaxScale has a core test suite for internal components and an extended suite of test for modules. To run the core tests, run `make testcore`. This will test the core maxscale executable. -To run `make testall`, the full test suite, you need to have four mysqld servers running on localhost. It assumes a master-slave replication setup with one slave and three slaves. +To run `make testall`, the full test suite, you need to have four mysqld servers running on localhost. It assumes a master-slave replication setup with one master and three slaves. The ports to which these servers are listening and the credentials to use for testing can be specified in the `macros.cmake` file found in the root source folder. From e527508405dd1c06c9f3d3b00da8c7996482cc7f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 26 Mar 2015 11:54:19 +0200 Subject: [PATCH 11/27] Added missing header guards to query classifier.h --- query_classifier/query_classifier.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/query_classifier/query_classifier.h b/query_classifier/query_classifier.h index 4d6c021f9..614637ea2 100644 --- a/query_classifier/query_classifier.h +++ b/query_classifier/query_classifier.h @@ -1,3 +1,5 @@ +#ifndef QUERY_CLASSIFIER_HG +#define QUERY_CLASSIFIER_HG /* This file is distributed as part of the MariaDB Corporation MaxScale. It is free software: you can redistribute it and/or modify it under the terms of the @@ -116,3 +118,4 @@ char** skygw_get_database_names(GWBUF* querybuf,int* size); EXTERN_C_BLOCK_END +#endif From b658a98a34a30db08b5d208a965425e8ab87c782 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 26 Mar 2015 17:04:34 +0100 Subject: [PATCH 12/27] Documentation Update Documentation Update --- Documentation/About/Limitations.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/About/Limitations.md b/Documentation/About/Limitations.md index bcc5549cd..4bfac725b 100644 --- a/Documentation/About/Limitations.md +++ b/Documentation/About/Limitations.md @@ -14,8 +14,6 @@ This section describes the limitations that are common to all configuration of p Both capabilities are not included in MySQL server handshake -* LOAD DATA LOCAL INFILE currently not supported - ## Limitations with MySQL Master/Slave Replication monitoring ## Limitations with Galera Cluster Monitoring From 1f10614da9eb1857bfc9849a0e6ebad57b83a53d Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 26 Mar 2015 17:15:45 +0100 Subject: [PATCH 13/27] Documentation Update Documentation Update --- Documentation/Documentation-Contents.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index 159b55229..77337caa3 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -10,7 +10,6 @@ - [Limitations](About/Limitations.md) - [COPYRIGHT](About/COPYRIGHT.md) - [LICENSE](About/LICENSE.md) - - [SETUP](About/SETUP.md) ## Getting Started @@ -72,3 +71,6 @@ - [MaxScale 1.0.1 Release Notes](Release-Notes/MaxScale-1.0.1-Release-Notes.md) - [MaxScale 1.0.3 Release Notes](Release-Notes/MaxScale-1.0.3-Release-Notes.md) +## Upgrading MaxScale + +- [Upgrading-To-MaxScale-1.1.0](Upgrading-To-MaxScale-1.1.0.md) From f774f27e4e6298221fc67d95b450e2fd4eb5915b Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 26 Mar 2015 17:21:23 +0100 Subject: [PATCH 14/27] Documentation Update --- Documentation/Documentation-Contents.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index 77337caa3..cbdd3426d 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -17,6 +17,10 @@ - [Building MaxScale from Source Code](Getting-Started/Building-MaxScale-from-Source-Code.md) - [Configuration Guide](Getting-Started/Configuration-Guide.md) +## Upgrading MaxScale + +- [Upgrading-To-MaxScale-1.1.0](Upgrading-To-MaxScale-1.1.0.md) + ## Reference - [MaxAdmin](Reference/MaxAdmin.md) @@ -71,6 +75,3 @@ - [MaxScale 1.0.1 Release Notes](Release-Notes/MaxScale-1.0.1-Release-Notes.md) - [MaxScale 1.0.3 Release Notes](Release-Notes/MaxScale-1.0.3-Release-Notes.md) -## Upgrading MaxScale - -- [Upgrading-To-MaxScale-1.1.0](Upgrading-To-MaxScale-1.1.0.md) From ac48edb0378aea96bd1485dfa85131cd8b12a583 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Thu, 26 Mar 2015 17:25:28 +0100 Subject: [PATCH 15/27] Documentation Update Documentation Update --- Documentation/Documentation-Contents.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Documentation-Contents.md b/Documentation/Documentation-Contents.md index cbdd3426d..c255d6f3d 100644 --- a/Documentation/Documentation-Contents.md +++ b/Documentation/Documentation-Contents.md @@ -19,7 +19,7 @@ ## Upgrading MaxScale -- [Upgrading-To-MaxScale-1.1.0](Upgrading-To-MaxScale-1.1.0.md) +- [Upgrading MaxScale to 1.1.0](Upgrading-To-MaxScale-1.1.0.md) ## Reference From 6ae929c5d4aa7f02c069adf2ce3167ce80b3e06d Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 27 Mar 2015 19:40:18 +0200 Subject: [PATCH 16/27] Fix to bug MXS-65: https://mariadb.atlassian.net/browse/MXS-65 Added more checks for incorrect rule syntax. --- server/modules/filter/dbfwfilter.c | 117 +++++++++++++++++------------ 1 file changed, 71 insertions(+), 46 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index 5961d97f1..bdb4ed7f2 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -17,7 +17,7 @@ */ /** - * @file fwfilter.c + * @file dbfwfilter.c * @author Markus Mäkelä * @date 13.2.2015 * @version 1.0.0 @@ -193,7 +193,7 @@ typedef struct rulelist_t{ typedef struct user_t{ char* name;/*< Name of the user */ - SPINLOCK* lock;/*< User spinlock */ + SPINLOCK lock;/*< User spinlock */ QUERYSPEED* qs_limit;/*< The query speed structure unique to this user */ RULELIST* rules_or;/*< If any of these rules match the action is triggered */ RULELIST* rules_and;/*< All of these rules must match for the action to trigger */ @@ -482,7 +482,7 @@ TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) tr = (TIMERANGE*)calloc(1,sizeof(TIMERANGE)); if(tr == NULL){ - skygw_log_write(LOGFILE_ERROR, "fwfilter: malloc returned NULL."); + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: malloc returned NULL."); return NULL; } @@ -649,7 +649,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) if((userptr == NULL || ruleptr == NULL || modeptr == NULL)|| (userptr > modeptr || userptr > ruleptr || modeptr > ruleptr)) { - skygw_log_write(LOGFILE_ERROR, "fwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",rule); + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",rule); return; } @@ -657,8 +657,19 @@ void link_rules(char* rule, FW_INSTANCE* instance) *ruleptr++ = '\0'; tok = strtok_r(modeptr," ",&saveptr); - if(tok && strcmp(tok,"match") == 0){ + + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",rule); + return; + } + if(strcmp(tok,"match") == 0){ tok = strtok_r(NULL," ",&saveptr); + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, missing keyword after 'match': %s",rule); + return; + } if(strcmp(tok,"any") == 0){ match_any = true; }else if(strcmp(tok,"all") == 0){ @@ -667,28 +678,38 @@ void link_rules(char* rule, FW_INSTANCE* instance) match_any = false; strict = true; }else{ - skygw_log_write(LOGFILE_ERROR, "fwfilter: Rule syntax incorrect, 'match' was not followed by 'any' or 'all': %s",rule); + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, 'match' was not followed by correct keyword: %s",rule); return; } } tok = strtok_r(ruleptr," ",&saveptr); tok = strtok_r(NULL," ",&saveptr); - - while(tok) - { - RULE* rule_found = NULL; - - if((rule_found = find_rule(tok,instance)) != NULL) - { - RULELIST* tmp_rl = (RULELIST*)calloc(1,sizeof(RULELIST)); - tmp_rl->rule = rule_found; - tmp_rl->next = rulelist; - rulelist = tmp_rl; - } - tok = strtok_r(NULL," ",&saveptr); - } + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no rules given: %s",rule); + return; + } + + while(tok) + { + RULE* rule_found = NULL; + + if((rule_found = find_rule(tok,instance)) != NULL) + { + RULELIST* tmp_rl = (RULELIST*)calloc(1,sizeof(RULELIST)); + tmp_rl->rule = rule_found; + tmp_rl->next = rulelist; + rulelist = tmp_rl; + + } + else + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, could not find rule '%s'.",tok); + } + tok = strtok_r(NULL," ",&saveptr); + } /** * Apply this list of rules to all the listed users @@ -698,8 +719,14 @@ void link_rules(char* rule, FW_INSTANCE* instance) userptr = strtok_r(rule," ",&saveptr); userptr = strtok_r(NULL," ",&saveptr); + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no users given: %s",rule); + goto parse_err; + } + while(userptr) - { + { USER* user; RULELIST *tl = NULL,*tail = NULL; @@ -709,17 +736,11 @@ void link_rules(char* rule, FW_INSTANCE* instance) user = (USER*)calloc(1,sizeof(USER)); if(user == NULL){ - free(rulelist); - return; - } - - if((user->lock = (SPINLOCK*)malloc(sizeof(SPINLOCK))) == NULL){ - free(user); - free(rulelist); - return; + skygw_log_write(LOGFILE_ERROR,"Error: dbfwfilter: failed to allocate memory when parsing rules."); + goto parse_err; } - spinlock_init(user->lock); + spinlock_init(&user->lock); } user->name = (char*)strdup(userptr); @@ -749,9 +770,9 @@ void link_rules(char* rule, FW_INSTANCE* instance) (void *)user); userptr = strtok_r(NULL," ",&saveptr); - } - + parse_err: + while(rulelist) { RULELIST *tmp = rulelist; @@ -782,7 +803,11 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tok = strtok_r(NULL," ,",&saveptr); - if(tok == NULL) goto retblock; + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, incomplete rule: %s",rule); + goto retblock; + } RULELIST* rlist = NULL; @@ -790,7 +815,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) if(ruledef == NULL) { - skygw_log_write(LOGFILE_ERROR,"Error : Memory allocation failed."); + skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); goto retblock; } @@ -799,7 +824,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) if(rlist == NULL) { free(ruledef); - skygw_log_write(LOGFILE_ERROR,"Error : Memory allocation failed."); + skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); goto retblock; } ruledef->name = strdup(tok); @@ -916,7 +941,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) memcpy(str, start, (tok-start)); if(regcomp(re, str,REG_NOSUB)){ - skygw_log_write(LOGFILE_ERROR, "fwfilter: Invalid regular expression '%s'.", str); + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Invalid regular expression '%s'.", str); free(re); } else @@ -968,7 +993,7 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tok = strtok_r(NULL," ",&saveptr); if(!parse_querytypes(tok,ruledef)){ skygw_log_write(LOGFILE_ERROR, - "fwfilter: Invalid query type" + "dbfwfilter: Invalid query type" "requirements on where/having clauses: %s." ,tok); } @@ -1336,7 +1361,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue if(!rulelist->rule->allow){ msg = strdup("Permission denied, query matched regular expression."); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': regex matched on query",rulelist->rule->name); + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': regex matched on query",rulelist->rule->name); goto queryresolved; }else{ break; @@ -1349,7 +1374,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue if(!rulelist->rule->allow){ matches = true; msg = strdup("Permission denied at this time."); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': query denied at: %s",rulelist->rule->name,asctime(tm_now)); + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': query denied at: %s",rulelist->rule->name,asctime(tm_now)); goto queryresolved; }else{ break; @@ -1372,7 +1397,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue if(!rulelist->rule->allow){ sprintf(emsg,"Permission denied to column '%s'.",strln->value); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': query targets forbidden column: %s",rulelist->rule->name,strln->value); + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': query targets forbidden column: %s",rulelist->rule->name,strln->value); msg = strdup(emsg); goto queryresolved; }else{ @@ -1402,7 +1427,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue matches = true; msg = strdup("Usage of wildcard denied."); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': query contains a wildcard.",rulelist->rule->name); + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': query contains a wildcard.",rulelist->rule->name); goto queryresolved; } } @@ -1420,9 +1445,9 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue rule_qs = (QUERYSPEED*)rulelist->rule->data; spinlock_release(my_instance->lock); - spinlock_acquire(user->lock); + spinlock_acquire(&user->lock); queryspeed = user->qs_limit; - spinlock_release(user->lock); + spinlock_release(&user->lock); while(queryspeed){ if(queryspeed->id == rule_qs->id){ @@ -1453,7 +1478,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue double blocked_for = queryspeed->cooldown - difftime(time_now,queryspeed->triggered); sprintf(emsg,"Queries denied for %f seconds",blocked_for); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': user denied for %f seconds",rulelist->rule->name,blocked_for); + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': user denied for %f seconds",rulelist->rule->name,blocked_for); msg = strdup(emsg); matches = true; @@ -1474,7 +1499,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue queryspeed->active = true; skygw_log_write(LOGFILE_TRACE, - "fwfilter: rule '%s': query limit triggered (%d queries in %f seconds), denying queries from user for %f seconds.", + "dbfwfilter: rule '%s': query limit triggered (%d queries in %f seconds), denying queries from user for %f seconds.", rulelist->rule->name, queryspeed->limit, queryspeed->period, @@ -1504,7 +1529,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue { matches = true; msg = strdup("Required WHERE/HAVING clause is missing."); - skygw_log_write(LOGFILE_TRACE, "fwfilter: rule '%s': query has no where/having clause, query is denied.", + skygw_log_write(LOGFILE_TRACE, "dbfwfilter: rule '%s': query has no where/having clause, query is denied.", rulelist->rule->name); } break; From 7c794b1d21622ed5c14acf3c1a2270a5d00e23cb Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Fri, 27 Mar 2015 20:28:04 +0200 Subject: [PATCH 17/27] Fixed a memory leak in mysql_client --- server/modules/protocol/mysql_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index a696037b1..5a9591c8c 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -900,7 +900,7 @@ int gw_read_client_event( dcb, ERRACT_NEW_CONNECTION, &succp); - free(errbuf); + gwbuf_free(errbuf); /** * If there are not enough backends close * session From 28ee7f18e076ac6bae4aadabbe39adbd12efea27 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 28 Mar 2015 05:38:18 +0200 Subject: [PATCH 18/27] Fixed wrong pointer being checked for NULL value. --- server/modules/filter/dbfwfilter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index bdb4ed7f2..454363452 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -719,7 +719,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) userptr = strtok_r(rule," ",&saveptr); userptr = strtok_r(NULL," ",&saveptr); - if(tok == NULL) + if(userptr == NULL) { skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no users given: %s",rule); goto parse_err; From e0319067c9b2b748fba63a8df3846478a30afda7 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 28 Mar 2015 13:11:57 +0200 Subject: [PATCH 19/27] Fix to MXS-68: https://mariadb.atlassian.net/browse/MXS-68 Filter creation fails if the rule file is not valid. --- server/modules/filter/dbfwfilter.c | 96 ++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 30 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index 454363452..a3bf3bb79 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -294,24 +294,27 @@ void* rlistdup(void* fval) static void* hrulefree(void* fval) { - USER* user = (USER*)fval; - RULELIST *ptr = user->rules_or,*tmp; + RULELIST *ptr = (RULELIST*)fval; while(ptr){ - tmp = ptr; + RULELIST *tmp = ptr; ptr = ptr->next; free(tmp); } - ptr = user->rules_and; - while(ptr){ - tmp = ptr; - ptr = ptr->next; - free(tmp); - } - free(user->name); - free(user); return NULL; } +static void* huserfree(void* fval) +{ + USER* value = (USER*)fval; + + hrulefree(value->rules_and); + hrulefree(value->rules_or); + hrulefree(value->rules_strict_and); + free(value->qs_limit); + free(value->name); + free(value); + return NULL; +} /** * Strips the single or double quotes from a string. @@ -632,13 +635,14 @@ void add_users(char* rule, FW_INSTANCE* instance) * @param rule Rule string to parse * @param instance The FW_FILTER instance */ -void link_rules(char* rule, FW_INSTANCE* instance) +bool link_rules(char* orig, FW_INSTANCE* instance) { - assert(rule != NULL && instance != NULL); /**Apply rules to users*/ bool match_any = true; + bool rval = true; + char *rule = strdup(orig); char *tok, *ruleptr, *userptr, *modeptr; char *saveptr = NULL; RULELIST* rulelist = NULL; @@ -649,8 +653,9 @@ void link_rules(char* rule, FW_INSTANCE* instance) if((userptr == NULL || ruleptr == NULL || modeptr == NULL)|| (userptr > modeptr || userptr > ruleptr || modeptr > ruleptr)) { - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",rule); - return; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",orig); + rval = false; + goto parse_err; } *modeptr++ = '\0'; @@ -660,15 +665,17 @@ void link_rules(char* rule, FW_INSTANCE* instance) if(tok == NULL) { - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",rule); - return; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, right keywords not found in the correct order: %s",orig); + rval = false; + goto parse_err; } if(strcmp(tok,"match") == 0){ tok = strtok_r(NULL," ",&saveptr); if(tok == NULL) { - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, missing keyword after 'match': %s",rule); - return; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, missing keyword after 'match': %s",orig); + rval = false; + goto parse_err; } if(strcmp(tok,"any") == 0){ match_any = true; @@ -678,8 +685,9 @@ void link_rules(char* rule, FW_INSTANCE* instance) match_any = false; strict = true; }else{ - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, 'match' was not followed by correct keyword: %s",rule); - return; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, 'match' was not followed by correct keyword: %s",orig); + rval = false; + goto parse_err; } } @@ -688,8 +696,9 @@ void link_rules(char* rule, FW_INSTANCE* instance) if(tok == NULL) { - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no rules given: %s",rule); - return; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no rules given: %s",orig); + rval = false; + goto parse_err; } while(tok) @@ -721,7 +730,15 @@ void link_rules(char* rule, FW_INSTANCE* instance) if(userptr == NULL) { - skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no users given: %s",rule); + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no users given: %s",orig); + rval = false; + goto parse_err; + } + + if(rulelist == NULL) + { + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Rule syntax incorrect, no rules found: %s",orig); + rval = false; goto parse_err; } @@ -737,6 +754,7 @@ void link_rules(char* rule, FW_INSTANCE* instance) if(user == NULL){ skygw_log_write(LOGFILE_ERROR,"Error: dbfwfilter: failed to allocate memory when parsing rules."); + rval = false; goto parse_err; } @@ -773,12 +791,15 @@ void link_rules(char* rule, FW_INSTANCE* instance) } parse_err: + free(rule); + while(rulelist) { RULELIST *tmp = rulelist; rulelist = rulelist->next; free(tmp); } + return rval; } @@ -1042,7 +1063,7 @@ createInstance(char **options, FILTER_PARAMETER **params) return NULL; } - hashtable_memory_fns(ht,(HASHMEMORYFN)strdup,NULL,(HASHMEMORYFN)free,hrulefree); + hashtable_memory_fns(ht,(HASHMEMORYFN)strdup,NULL,(HASHMEMORYFN)free,huserfree); my_instance->htable = ht; my_instance->def_op = true; @@ -1104,13 +1125,28 @@ createInstance(char **options, FILTER_PARAMETER **params) fclose(file); /**Apply the rules to users*/ + bool err = false; ptr = my_instance->userstrings; + while(ptr){ - link_rules(ptr->value,my_instance); - tmp = ptr; - ptr = ptr->next; - free(tmp->value); - free(tmp); + + if(!link_rules(ptr->value,my_instance)) + { + skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Failed to parse rule: %s",ptr->value); + err = true; + } + tmp = ptr; + ptr = ptr->next; + free(tmp->value); + free(tmp); + } + + if(err) + { + hrulefree(my_instance->rules); + hashtable_free(my_instance->htable); + free(my_instance); + my_instance = NULL; } return (FILTER *)my_instance; From 0b7e1d5a5102fba7a8aec882ecec384ad19ff8d4 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Sat, 28 Mar 2015 16:03:44 +0200 Subject: [PATCH 20/27] Fix to MXS-71 and MXS-72. at_times now fails if the parameter is not a properly formatted time. on_queries was falsely identified as on_operations, the behavior is now in line with the documentation. --- server/modules/filter/dbfwfilter.c | 275 +++++++++++++++++++---------- 1 file changed, 179 insertions(+), 96 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index a3bf3bb79..75bf81273 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -444,7 +444,7 @@ bool check_time(char* str) char* ptr = str; int colons = 0,numbers = 0,dashes = 0; - while(*ptr){ + while(*ptr && ptr - str < 18){ if(isdigit(*ptr)){numbers++;} else if(*ptr == ':'){colons++;} else if(*ptr == '-'){dashes++;} @@ -498,7 +498,7 @@ TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) while(ptr - str < 19){ if(isdigit(*ptr)){ *sdest = *ptr; - }else if(*ptr == ':' ||*ptr == '-' || *ptr == '\0'){ + }else if(*ptr == ':' ||*ptr == '-' || *ptr == '\0' || *ptr == ' '){ *sdest = '\0'; *idest++ = atoi(strbuffer); sdest = strbuffer; @@ -511,7 +511,7 @@ TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) CHK_TIMES(tmptr); - if(*ptr == '\0'){ + if(*ptr == '\0' || *ptr == ' '){ return tr; } @@ -525,8 +525,8 @@ TIMERANGE* parse_time(char* str, FW_INSTANCE* instance) sdest++; } - - return tr; + free(tr); + return NULL; } @@ -802,89 +802,105 @@ bool link_rules(char* orig, FW_INSTANCE* instance) return rval; } - /** * Parse the configuration value either as a new rule or a list of users. * @param rule The string to parse * @param instance The FW_FILTER instance */ -void parse_rule(char* rule, FW_INSTANCE* instance) +bool parse_rule(char* rule, FW_INSTANCE* instance) { - ss_dassert(rule != NULL && instance != NULL); + ss_dassert(rule != NULL && instance != NULL); - char *rulecpy = strdup(rule); - char *saveptr = NULL; - char *tok = strtok_r(rulecpy," ,",&saveptr); - bool allow,deny,mode; - RULE* ruledef = NULL; - - if(tok == NULL) goto retblock; + char *rulecpy = strdup(rule); + char *saveptr = NULL; + char *tok = strtok_r(rulecpy," ,",&saveptr); + bool allow,deny,mode; + RULE* ruledef = NULL; + bool rval = true; - if(strcmp("rule",tok) == 0){ /**Define a new rule*/ + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, no rule rule: %s",rule); + rval = false; + goto retblock; + } - tok = strtok_r(NULL," ,",&saveptr); - - if(tok == NULL) - { - skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, incomplete rule: %s",rule); - goto retblock; - } - - RULELIST* rlist = NULL; - ruledef = (RULE*)calloc(1,sizeof(RULE)); - - if(ruledef == NULL) - { - skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); - goto retblock; - } - - rlist = (RULELIST*)calloc(1,sizeof(RULELIST)); - - if(rlist == NULL) - { - free(ruledef); - skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); - goto retblock; - } - ruledef->name = strdup(tok); - ruledef->type = RT_UNDEFINED; - ruledef->on_queries = QUERY_OP_UNDEFINED; - rlist->rule = ruledef; - rlist->next = instance->rules; - instance->rules = rlist; + if(strcmp("rule",tok) == 0) + { + /**Define a new rule*/ - }else if(strcmp("users",tok) == 0){ + tok = strtok_r(NULL," ,",&saveptr); - /**Apply rules to users*/ - add_users(rule, instance); - goto retblock; + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, incomplete rule: %s",rule); + rval = false; + goto retblock; } - else - { - skygw_log_write(LOGFILE_ERROR,"Error : Unknown token in rule file: %s",tok); - goto retblock; - } + RULELIST* rlist = NULL; + + ruledef = (RULE*)calloc(1,sizeof(RULE)); + + if(ruledef == NULL) + { + skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); + rval = false; + goto retblock; + } + + rlist = (RULELIST*)calloc(1,sizeof(RULELIST)); + + if(rlist == NULL) + { + free(ruledef); + skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); + rval = false; + goto retblock; + } + + ruledef->name = strdup(tok); + ruledef->type = RT_UNDEFINED; + ruledef->on_queries = QUERY_OP_UNDEFINED; + rlist->rule = ruledef; + rlist->next = instance->rules; + instance->rules = rlist; + + }else if(strcmp("users",tok) == 0) + { + + /**Apply rules to users*/ + add_users(rule, instance); + goto retblock; + } + else + { + skygw_log_write(LOGFILE_ERROR,"Error : Unknown token in rule file: %s",tok); + rval = false; + goto retblock; + } + + tok = strtok_r(NULL, " ,",&saveptr); + + + if((allow = (strcmp(tok,"allow") == 0)) || + (deny = (strcmp(tok,"deny") == 0))) + { + + mode = allow ? true:false; + ruledef->allow = mode; + ruledef->type = RT_PERMISSION; tok = strtok_r(NULL, " ,",&saveptr); - if((allow = (strcmp(tok,"allow") == 0)) || - (deny = (strcmp(tok,"deny") == 0))){ - - mode = allow ? true:false; - ruledef->allow = mode; - ruledef->type = RT_PERMISSION; - tok = strtok_r(NULL, " ,",&saveptr); - - - while(tok){ - if(strcmp(tok,"wildcard") == 0) + while(tok) + { + if(strcmp(tok,"wildcard") == 0) { ruledef->type = RT_WILDCARD; } - else if(strcmp(tok,"columns") == 0) + else if(strcmp(tok,"columns") == 0) { STRLINK *tail = NULL,*current; ruledef->type = RT_COLUMN; @@ -896,20 +912,36 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tail = current; tok = strtok_r(NULL, " ,",&saveptr); } - + ruledef->data = (void*)tail; continue; } - else if(strcmp(tok,"at_times") == 0) + else if(strcmp(tok,"at_times") == 0) { tok = strtok_r(NULL, " ,",&saveptr); TIMERANGE *tr = NULL; + bool not_valid = false; while(tok){ + + if(!check_time(tok)) + { + not_valid = true; + break; + } + TIMERANGE *tmp = parse_time(tok,instance); - - if(IS_RVRS_TIME(tmp)){ + + if(tmp == NULL) + { + skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, unexpected characters after time definition."); + rval = false; + goto retblock; + } + + if(IS_RVRS_TIME(tmp)) + { tmp = split_reverse_time(tmp); } tmp->next = tr; @@ -917,44 +949,68 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tok = strtok_r(NULL, " ,",&saveptr); } ruledef->active = tr; + + if(not_valid) + { + continue; + } + } - else if(strcmp(tok,"regex") == 0) + else if(strcmp(tok,"regex") == 0) { bool escaped = false; regex_t *re; char* start, *str; tok = strtok_r(NULL," ",&saveptr); - char delim = '\''; - while(*tok == '\'' || *tok == '"'){ + char delim = '\''; + int n_char = 0; + + while(*tok == '\'' || *tok == '"') + { delim = *tok; tok++; } start = tok; - - while(isspace(*tok) || *tok == delim){ + + while(isspace(*tok) || *tok == delim) + { tok++; } - - while(true){ - if((*tok == delim) && !escaped){ + while(n_char < 2048) + { + + /** Hard-coded regex length cap */ + + if((*tok == delim) && !escaped) + { break; } escaped = (*tok == '\\'); tok++; + n_char++; } + if(n_char >= 2048) + { + skygw_log_write_flush(LOGFILE_ERROR, "dbfwfilter: Failed to parse rule, regular expression length is over 2048 characters."); + rval = false; + goto retblock; + } + str = calloc(((tok - start) + 1),sizeof(char)); if(str == NULL) { skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); + rval = false; goto retblock; } re = (regex_t*)malloc(sizeof(regex_t)); if(re == NULL){ - skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); + skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); + rval = false; free(str); goto retblock; } @@ -963,7 +1019,9 @@ void parse_rule(char* rule, FW_INSTANCE* instance) if(regcomp(re, str,REG_NOSUB)){ skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Invalid regular expression '%s'.", str); + rval = false; free(re); + goto retblock; } else { @@ -973,9 +1031,9 @@ void parse_rule(char* rule, FW_INSTANCE* instance) free(str); } - else if(strcmp(tok,"limit_queries") == 0) + else if(strcmp(tok,"limit_queries") == 0) { - + QUERYSPEED* qs = (QUERYSPEED*)calloc(1,sizeof(QUERYSPEED)); spinlock_acquire(instance->lock); @@ -985,6 +1043,8 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); + rval = false; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Missing parameter in limit_queries: '%s'.", rule); goto retblock; } qs->limit = atoi(tok); @@ -992,41 +1052,48 @@ void parse_rule(char* rule, FW_INSTANCE* instance) tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); + rval = false; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Missing parameter in limit_queries: '%s'.", rule); goto retblock; } qs->period = atof(tok); tok = strtok_r(NULL," ",&saveptr); if(tok == NULL){ free(qs); + rval = false; + skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Missing parameter in limit_queries: '%s'.", rule); goto retblock; } qs->cooldown = atof(tok); ruledef->type = RT_THROTTLE; ruledef->data = (void*)qs; } - else if(strcmp(tok,"no_where_clause") == 0) + else if(strcmp(tok,"no_where_clause") == 0) { ruledef->type = RT_CLAUSE; ruledef->data = (void*)mode; } - else if(strcmp(tok,"on_operations") == 0) + else if(strcmp(tok,"on_queries") == 0) { tok = strtok_r(NULL," ",&saveptr); if(!parse_querytypes(tok,ruledef)){ skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Invalid query type" - "requirements on where/having clauses: %s." - ,tok); + "requirements on where/having clauses: %s." + ,tok); + rval = false; + goto retblock; } } - tok = strtok_r(NULL," ,",&saveptr); - } - - goto retblock; + tok = strtok_r(NULL," ,",&saveptr); } + goto retblock; + } + retblock: - free(rulecpy); + free(rulecpy); + return rval; } @@ -1048,7 +1115,8 @@ createInstance(char **options, FILTER_PARAMETER **params) char *filename = NULL, *nl; char buffer[2048]; FILE* file; - + bool err = false; + if ((my_instance = calloc(1, sizeof(FW_INSTANCE))) == NULL || (my_instance->lock = (SPINLOCK*)malloc(sizeof(SPINLOCK))) == NULL){ skygw_log_write(LOGFILE_ERROR, "Memory allocation for firewall filter failed."); @@ -1119,13 +1187,18 @@ createInstance(char **options, FILTER_PARAMETER **params) *nl = '\0'; } - parse_rule(buffer,my_instance); + if(!parse_rule(buffer,my_instance)) + { + fclose(file); + err = true; + goto retblock; + } } fclose(file); /**Apply the rules to users*/ - bool err = false; + ptr = my_instance->userstrings; while(ptr){ @@ -1141,6 +1214,8 @@ createInstance(char **options, FILTER_PARAMETER **params) free(tmp); } + retblock: + if(err) { hrulefree(my_instance->rules); @@ -1657,6 +1732,7 @@ bool check_match_any(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu bool check_match_all(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue, USER* user,bool strict_all) { bool is_sql, rval = true; + bool have_active_rule = false; int qlen; unsigned char* memptr = (unsigned char*)queue->start; char *fullquery = NULL,*ptr; @@ -1697,7 +1773,8 @@ bool check_match_all(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu rulelist = rulelist->next; continue; } - + + have_active_rule = true; if(!rule_matches(my_instance,my_session,queue,user,rulelist,fullquery)){ rval = false; @@ -1707,6 +1784,12 @@ bool check_match_all(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu rulelist = rulelist->next; } + if(!have_active_rule) + { + /** No active rules */ + rval = false; + } + retblock: free(fullquery); From 875e49c074e0654cc6bdaf6c878e9f85e3fec806 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 30 Mar 2015 07:33:24 +0300 Subject: [PATCH 21/27] Fixed a possible memory leak in dbfwfilter. --- server/modules/filter/dbfwfilter.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index 75bf81273..c8efd7e36 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -802,6 +802,23 @@ bool link_rules(char* orig, FW_INSTANCE* instance) return rval; } +/** + * Free a TIMERANGE struct + * @param tr pointer to a TIMERANGE struct + */ +void tr_free(TIMERANGE* tr) +{ + TIMERANGE *node,*tmp; + + node = tr; + + while(node) + { + tmp = node; + node = node->next; + free(tmp); + } +} /** * Parse the configuration value either as a new rule or a list of users. * @param rule The string to parse @@ -937,6 +954,7 @@ bool parse_rule(char* rule, FW_INSTANCE* instance) { skygw_log_write(LOGFILE_ERROR,"dbfwfilter: Rule parsing failed, unexpected characters after time definition."); rval = false; + tr_free(tr); goto retblock; } From 5f422a96fc0fca57ea094093197da2787f821e03 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 30 Mar 2015 14:24:17 +0300 Subject: [PATCH 22/27] Fixed a memory leak in schemarouter. --- .../modules/routing/schemarouter/schemarouter.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index 9587f3bc4..c3605998e 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -402,7 +402,7 @@ int gen_databaselist(ROUTER_INSTANCE* inst, ROUTER_CLIENT_SES* session) rval); } } - + gwbuf_free(buffer); return !rval; } @@ -2223,6 +2223,7 @@ static void clientReply ( */ if (!rses_begin_locked_router_action(router_cli_ses)) { + while((writebuf = gwbuf_consume(writebuf,gwbuf_length(writebuf)))); goto lock_failed; } /** Holding lock ensures that router session remains open */ @@ -2252,6 +2253,7 @@ static void clientReply ( if (!rses_begin_locked_router_action(router_cli_ses)) { /** Log to debug that router was closed */ + while((writebuf = gwbuf_consume(writebuf,gwbuf_length(writebuf)))); goto lock_failed; } bref = get_bref_from_dcb(router_cli_ses, backend_dcb); @@ -2260,6 +2262,7 @@ static void clientReply ( { /** Unlock router session */ rses_end_locked_router_action(router_cli_ses); + while((writebuf = gwbuf_consume(writebuf,gwbuf_length(writebuf)))); goto lock_failed; } @@ -2307,7 +2310,7 @@ static void clientReply ( } } - gwbuf_free(writebuf); + while((writebuf = gwbuf_consume(writebuf,gwbuf_length(writebuf)))); if(mapped) { @@ -2330,7 +2333,10 @@ static void clientReply ( router_cli_ses->connect_db); router_cli_ses->rses_closed = true; if(router_cli_ses->queue) - gwbuf_free(router_cli_ses->queue); + { + while((router_cli_ses->queue = gwbuf_consume( + router_cli_ses->queue,gwbuf_length(router_cli_ses->queue)))); + } rses_end_locked_router_action(router_cli_ses); return; } @@ -2384,7 +2390,7 @@ static void clientReply ( { GWBUF* tmp = router_cli_ses->queue; router_cli_ses->queue = router_cli_ses->queue->next; - tmp->next = NULL; + tmp->next = NULL; char* querystr = modutil_get_SQL(tmp); skygw_log_write(LOGFILE_DEBUG,"schemarouter: Sending queued buffer for session %p: %s", router_cli_ses->rses_client_dcb->session, @@ -2424,6 +2430,8 @@ static void clientReply ( strcpy(router_cli_ses->rses_mysql_session->db,router_cli_ses->connect_db); ss_dassert(router_cli_ses->init == INIT_READY); rses_end_locked_router_action(router_cli_ses); + if(writebuf) + while((writebuf = gwbuf_consume(writebuf,gwbuf_length(writebuf)))); return; } From 3a0807251c47cf5524adbd8c7cd02a7135acaf33 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 30 Mar 2015 15:43:51 +0300 Subject: [PATCH 23/27] Fix to MXS-54: https://mariadb.atlassian.net/browse/MXS-54 Added log messages for failed authentication attempts. --- server/modules/protocol/mysql_client.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 5a9591c8c..6325016d6 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -530,6 +530,12 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { if (auth_ret == 0) { dcb->user = strdup(client_data->user); } + else + { + skygw_log_write(LOGFILE_ERROR, + "%s: login attempt for user '%s', authentication failed.", + dcb->service->name, username); + } /* let's free the auth_token now */ if (auth_token) { From 97a6bd1b4bee1d33e1d26ce6e7439a0c350f2ad0 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 30 Mar 2015 20:42:18 +0300 Subject: [PATCH 24/27] Fix to MXS-63: https://mariadb.atlassian.net/browse/MXS-63 Maxkeys and maxpasswd now use MAXSCALE_HOME or the default installation directory if MAXSCALE_HOME is not set. If all else fails, /tmp/ is used for logging. --- server/core/gateway.c | 1 + server/core/maxkeys.c | 45 +++++++++++++++++++++++++++++++++++++++-- server/core/maxpasswd.c | 39 ++++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/server/core/gateway.c b/server/core/gateway.c index bcb5a3cfa..7fda1017d 100644 --- a/server/core/gateway.c +++ b/server/core/gateway.c @@ -826,6 +826,7 @@ static bool file_is_readable( absolute_pathname, eno, strerror(eno)))); + LOGIF(LE,(skygw_log_sync_all())); succp = false; } return succp; diff --git a/server/core/maxkeys.c b/server/core/maxkeys.c index 747e575ec..0d5c938ff 100644 --- a/server/core/maxkeys.c +++ b/server/core/maxkeys.c @@ -29,19 +29,60 @@ */ #include #include - +#include +#include int main(int argc, char **argv) { + int arg_count = 3; + char *home; + char** arg_vector; + + if (argc != 2) { fprintf(stderr, "Usage: %s \n", argv[0]); exit(1); } + + arg_vector = malloc(sizeof(char*)*4); + + if(arg_vector == NULL) + { + fprintf(stderr,"Error: Memory allocation failed.\n"); + return 1; + } + + arg_vector[0] = strdup("logmanager"); + arg_vector[1] = strdup("-j"); + + if ((home = getenv("MAXSCALE_HOME")) != NULL) + { + arg_vector[2] = (char*)malloc((strlen(home) + strlen("/log"))*sizeof(char)); + sprintf(arg_vector[2],"%s/log",home); + } + else + { + arg_vector[2] = strdup("/usr/local/mariadb-maxscale/log"); + } + + arg_vector[3] = NULL; + skygw_logmanager_init(arg_count,arg_vector); + skygw_log_enable(LOGFILE_TRACE); + skygw_log_enable(LOGFILE_DEBUG); + free(arg_vector[0]); + free(arg_vector[1]); + free(arg_vector[2]); + free(arg_vector); + if (secrets_writeKeys(argv[1])) { fprintf(stderr, "Failed to encode the password\n"); exit(1); } - exit(0); + + skygw_log_sync_all(); + skygw_logmanager_done(); + + return 0; } diff --git a/server/core/maxpasswd.c b/server/core/maxpasswd.c index 0c728d869..4da1dbde1 100644 --- a/server/core/maxpasswd.c +++ b/server/core/maxpasswd.c @@ -29,7 +29,8 @@ */ #include #include - +#include +#include /** * Encrypt a password for storing in the MaxScale.cnf file * @@ -40,12 +41,46 @@ int main(int argc, char **argv) { char *enc, *pw; + int arg_count = 3; + char *home; + char** arg_vector; + if (argc != 2) { fprintf(stderr, "Usage: %s \n", argv[0]); exit(1); } + + arg_vector = malloc(sizeof(char*)*4); + + if(arg_vector == NULL) + { + fprintf(stderr,"Error: Memory allocation failed.\n"); + return 1; + } + + arg_vector[0] = strdup("logmanager"); + arg_vector[1] = strdup("-j"); + + if ((home = getenv("MAXSCALE_HOME")) != NULL) + { + arg_vector[2] = (char*)malloc((strlen(home) + strlen("/log"))*sizeof(char)); + sprintf(arg_vector[2],"%s/log",home); + } + else + { + arg_vector[2] = strdup("/usr/local/mariadb-maxscale/log"); + } + + arg_vector[3] = NULL; + skygw_logmanager_init(arg_count,arg_vector); + skygw_log_enable(LOGFILE_TRACE); + skygw_log_enable(LOGFILE_DEBUG); + free(arg_vector[0]); + free(arg_vector[1]); + free(arg_vector[2]); + free(arg_vector); pw = calloc(81,sizeof(char)); @@ -63,5 +98,7 @@ main(int argc, char **argv) } free(pw); + skygw_log_sync_all(); + skygw_logmanager_done(); return 0; } From e42f17156449feb7cea591ffb703483e7961bc71 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Mon, 30 Mar 2015 20:45:05 +0300 Subject: [PATCH 25/27] Added support for direct connections to shardrouter. --- server/modules/include/shardrouter.h | 21 +- .../routing/schemarouter/shardrouter.c | 310 +++++++++++++----- 2 files changed, 237 insertions(+), 94 deletions(-) diff --git a/server/modules/include/shardrouter.h b/server/modules/include/shardrouter.h index f47e074a4..dfd83d520 100644 --- a/server/modules/include/shardrouter.h +++ b/server/modules/include/shardrouter.h @@ -159,7 +159,20 @@ typedef struct subservice_t{ int state; int n_res_waiting; bool mapped; -}SUBSERVICE; +}SUBSERVICE; + +/** + * Bitmask values for the router session's initialization. These values are used + * to prevent responses from internal commands being forwarded to the client. + */ +typedef enum shard_init_mask +{ + INIT_READY = 0x0, + INIT_MAPPING = 0x1, + INIT_USE_DB = 0x02, + INIT_UNINT = 0x04 + +} shard_init_mask_t; /** * The client session structure used within this router. @@ -172,8 +185,8 @@ struct router_client_session { int rses_versno; /*< even = no active update, else odd. not used 4/14 */ bool rses_closed; /*< true when closeSession is called */ DCB* rses_client_dcb; - DCB* dummy_dcb; /* DCB used to send the client write messages from the router itself */ - DCB* queue_dcb; /* DCB used to send queued queries to the router */ + DCB* replydcb; /* DCB used to send the client write messages from the router itself */ + DCB* routedcb; /* DCB used to send queued queries to the router */ MYSQL_session* rses_mysql_session; /** Properties listed by their type */ rses_property_t* rses_properties[RSES_PROP_TYPE_COUNT]; @@ -190,6 +203,8 @@ struct router_client_session { bool hash_init; SESSION* session; GWBUF* queue; + char connect_db[MYSQL_DATABASE_MAXLEN+1]; /*< Database the user was trying to connect to */ + shard_init_mask_t init; /*< Initialization state bitmask */ #if defined(SS_DEBUG) skygw_chk_t rses_chk_tail; #endif diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index c4a9e88ed..5d76ca9ad 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -119,7 +119,6 @@ static route_target_t get_shard_route_target( static uint8_t getCapabilities(ROUTER* inst, void* router_session); -//bool parse_db_ignore_list(ROUTER_INSTANCE* router, char* param); static void subsvc_clear_state(SUBSERVICE* svc,subsvc_state_t state); static void subsvc_set_state(SUBSERVICE* svc,subsvc_state_t state); static bool get_shard_subsvc(SUBSERVICE** subsvc,ROUTER_CLIENT_SES* session,char* target); @@ -414,8 +413,14 @@ bool subsvc_is_valid(SUBSERVICE* sub) return false; } +/** + * Map the databases of all subservices. + * @param inst router instance + * @param session router session + * @return 0 on success, 1 on error + */ int -gen_tablelist(ROUTER_INSTANCE* inst, ROUTER_CLIENT_SES* session) +gen_subsvc_dblist(ROUTER_INSTANCE* inst, ROUTER_CLIENT_SES* session) { const char* query = "SHOW DATABASES;"; GWBUF *buffer, *clone; @@ -475,7 +480,6 @@ get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, GWBUF* if(sz > 0) { - has_dbs = true; for(i = 0; i < sz; i++) { @@ -489,9 +493,8 @@ get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, GWBUF* else { skygw_log_write(LOGFILE_TRACE,"shardrouter: Query targets database '%s' on server '%s",dbnms[i],rval); + has_dbs = true; } - for(j = i; j < sz; j++) free(dbnms[j]); - break; } free(dbnms[i]); } @@ -554,6 +557,10 @@ get_shard_target_name(ROUTER_INSTANCE* router, ROUTER_CLIENT_SES* client, GWBUF* */ rval = (char*) hashtable_fetch(ht, client->rses_mysql_session->db); + if(rval) + { + skygw_log_write(LOGFILE_TRACE,"shardrouter: Using active database '%s'",client->rses_mysql_session->db); + } } return rval; @@ -612,64 +619,160 @@ filterReply(FILTER* instance, void *session, GWBUF *reply) ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES*) instance; SUBSERVICE* subsvc; int i, rv = 1; - bool mapped = true; sescmd_cursor_t* scur; GWBUF* tmp = NULL; - skygw_log_write_flush(LOGFILE_TRACE,"shardrouter: filterReply mapped: %s",rses->hash_init ? "true" : "false"); - if(!rses_begin_locked_router_action(rses)) { + tmp = reply; + while((tmp = gwbuf_consume(tmp,gwbuf_length(tmp)))); return 0; } subsvc = get_subsvc_from_ses(rses, session); - if(SUBSVC_IS_WAITING(subsvc)) + if(rses->init & INIT_MAPPING) { - subsvc_clear_state(subsvc, SUBSVC_WAITING_RESULT); + bool mapped = true, logged = false; + int i; + + for(i = 0; i < rses->n_subservice; i++) + { + + if(subsvc->session == rses->subservice[i]->session && + !SUBSVC_IS_MAPPED(rses->subservice[i])) + { + rses->subservice[i]->state |= SUBSVC_MAPPED; + parse_mapping_response(rses, + rses->subservice[i]->service->name, + reply); + + } + + if(SUBSVC_IS_OK(rses->subservice[i]) && + !SUBSVC_IS_MAPPED(rses->subservice[i])) + { + mapped = false; + if(!logged) + { +/* + skygw_log_write(LOGFILE_DEBUG,"schemarouter: Still waiting for reply to SHOW DATABASES from %s for session %p", + bkrf[i].bref_backend->backend_server->unique_name, + rses->rses_client_dcb->session); +*/ + logged = true; + } + } + } + + if(mapped) + { + /* + * Check if the session is reconnecting with a database name + * that is not in the hashtable. If the database is not found + * then close the session. + */ + + rses->init &= ~INIT_MAPPING; + + if(rses->init & INIT_USE_DB) + { + char* target; + + if((target = hashtable_fetch(rses->dbhash, + rses->connect_db)) == NULL) + { + skygw_log_write_flush(LOGFILE_TRACE,"schemarouter: Connecting to a non-existent database '%s'", + rses->connect_db); + rses->rses_closed = true; + if(rses->queue) + { + while((rses->queue = gwbuf_consume( + rses->queue,gwbuf_length(rses->queue)))); + } + rses_end_locked_router_action(rses); + goto retblock; + } + + /* Send a COM_INIT_DB packet to the server with the right database + * and set it as the client's active database */ + + unsigned int qlen; + GWBUF* buffer; + + qlen = strlen(rses->connect_db); + buffer = gwbuf_alloc(qlen + 5); + if(buffer == NULL) + { + skygw_log_write_flush(LOGFILE_ERROR,"Error : Buffer allocation failed."); + rses->rses_closed = true; + if(rses->queue) + gwbuf_free(rses->queue); + goto retblock; + } + + gw_mysql_set_byte3((unsigned char*)buffer->start,qlen+1); + gwbuf_set_type(buffer,GWBUF_TYPE_MYSQL); + *((unsigned char*)buffer->start + 3) = 0x0; + *((unsigned char*)buffer->start + 4) = 0x2; + memcpy(buffer->start+5,rses->connect_db,qlen); + DCB* dcb = NULL; + + SESSION_ROUTE_QUERY(subsvc->session,buffer); + + goto retblock; + } + + if(rses->queue) + { + GWBUF* tmp = rses->queue; + rses->queue = rses->queue->next; + tmp->next = NULL; + char* querystr = modutil_get_SQL(tmp); + skygw_log_write(LOGFILE_DEBUG,"schemarouter: Sending queued buffer for session %p: %s", + rses->rses_client_dcb->session, + querystr); + poll_add_epollin_event_to_dcb(rses->routedcb,tmp); + free(querystr); + + } + skygw_log_write_flush(LOGFILE_DEBUG,"session [%p] database map finished.", + rses); + } + + goto retblock; } - subsvc_clear_state(subsvc, SUBSVC_QUERY_ACTIVE); - - if(!rses->hash_init) - { - subsvc_set_state(subsvc, SUBSVC_MAPPED); - parse_mapping_response(rses, subsvc->service->name, reply); - - for(i = 0; i < rses->n_subservice; i++) - { - if(SUBSVC_IS_OK(rses->subservice[i]) && !SUBSVC_IS_MAPPED(rses->subservice[i])) - { - mapped = false; - break; - } - - } - - gwbuf_free(reply); - - if(mapped) - { - rses->hash_init = true; - if(rses->queue) - { - tmp = rses->queue; - rses->queue = rses->queue->next; - } - } - - goto retblock; - } - - - if(rses->queue) { - tmp = rses->queue; - rses->queue = rses->queue->next; + GWBUF* tmp = rses->queue; + rses->queue = rses->queue->next; + tmp->next = NULL; + char* querystr = modutil_get_SQL(tmp); + skygw_log_write(LOGFILE_DEBUG,"schemarouter: Sending queued buffer for session %p: %s", + rses->rses_client_dcb->session, + querystr); + poll_add_epollin_event_to_dcb(rses->routedcb,tmp); + free(querystr); + tmp = NULL; } + if(rses->init & INIT_USE_DB) + { + skygw_log_write(LOGFILE_DEBUG,"schemarouter: Reply to USE '%s' received for session %p", + rses->connect_db, + rses->rses_client_dcb->session); + rses->init &= ~INIT_USE_DB; + strcpy(rses->rses_mysql_session->db,rses->connect_db); + ss_dassert(rses->init == INIT_READY); + if(reply) + { + tmp = reply; + while((tmp = gwbuf_consume(tmp,gwbuf_length(tmp)))); + tmp = NULL; + } + goto retblock; + } scur = subsvc->scur; @@ -688,12 +791,8 @@ filterReply(FILTER* instance, void *session, GWBUF *reply) rv = SESSION_ROUTE_REPLY(rses->session, reply); -retblock: - rses_end_locked_router_action(rses); - if(tmp) - { - poll_add_epollin_event_to_dcb(rses->queue_dcb,tmp); - } + retblock: + rses_end_locked_router_action(rses); return rv; } @@ -876,6 +975,14 @@ createInstance(SERVICE *service, char **options) tok = strtok(services, ","); */ + if(options == NULL) + { + free(router); + skygw_log_write(LOGFILE_ERROR, "Error : No 'subservice' router option found. Shardrouter requires at least %d " + "configured services listed in the 'subservices' router option to work.", min_nsvc); + return NULL; + } + while(options[i]) { if(sz <= i) @@ -896,6 +1003,13 @@ createInstance(SERVICE *service, char **options) } res_svc[i] = service_find(options[i]); + if(res_svc[i] == NULL) + { + free(res_svc); + free(router); + skygw_log_write(LOGFILE_ERROR, "Error : No service named '%s' found.", options[i]); + return NULL; + } i++; } /* @@ -907,7 +1021,7 @@ createInstance(SERVICE *service, char **options) if(i < min_nsvc) { - skygw_log_write(LOGFILE_ERROR, "Error : Not enough services. Shardrouter requires at least %d " + skygw_log_write(LOGFILE_ERROR, "Error : Not enough parameters for 'subservice' router option. Shardrouter requires at least %d " "configured services to work.", min_nsvc); free(router->services); free(router); @@ -980,15 +1094,15 @@ newSession( client_rses->rses_autocommit_enabled = true; client_rses->rses_transaction_active = false; client_rses->session = session; - client_rses->dummy_dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); - client_rses->dummy_dcb->func.read = fakeReply; - client_rses->dummy_dcb->state = DCB_STATE_POLLING; - client_rses->dummy_dcb->session = session; + client_rses->replydcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); + client_rses->replydcb->func.read = fakeReply; + client_rses->replydcb->state = DCB_STATE_POLLING; + client_rses->replydcb->session = session; - client_rses->queue_dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); - client_rses->queue_dcb->func.read = fakeQuery; - client_rses->queue_dcb->state = DCB_STATE_POLLING; - client_rses->queue_dcb->session = session; + client_rses->routedcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); + client_rses->routedcb->func.read = fakeQuery; + client_rses->routedcb->state = DCB_STATE_POLLING; + client_rses->routedcb->session = session; spinlock_init(&client_rses->rses_lock); @@ -1168,10 +1282,10 @@ closeSession( } router_cli_ses->subservice[i]->state = SUBSVC_CLOSED; } - router_cli_ses->dummy_dcb->session = NULL; - router_cli_ses->queue_dcb->session = NULL; - dcb_close(router_cli_ses->dummy_dcb); - dcb_close(router_cli_ses->queue_dcb); + router_cli_ses->replydcb->session = NULL; + router_cli_ses->routedcb->session = NULL; + dcb_close(router_cli_ses->replydcb); + dcb_close(router_cli_ses->routedcb); /** Unlock */ rses_end_locked_router_action(router_cli_ses); @@ -1480,32 +1594,46 @@ routeQuery(ROUTER* instance, ret = 0; goto retblock; } - - if(!router_cli_ses->hash_init) - { - gen_tablelist(inst, router_cli_ses); - - skygw_log_write(LOGFILE_TRACE,"shardrouter: got a query while mapping databases."); - GWBUF* tmp = router_cli_ses->queue; - - while(tmp && tmp->next) + if(!(rses_is_closed = router_cli_ses->rses_closed)) { - tmp = tmp->next; + if(router_cli_ses->init & INIT_UNINT) + { + /* Generate database list */ + gen_subsvc_dblist(inst,router_cli_ses); + + } + + if(router_cli_ses->init & INIT_MAPPING) + { + + char* querystr = modutil_get_SQL(querybuf); + skygw_log_write(LOGFILE_DEBUG,"shardrouter: Storing query for session %p: %s", + router_cli_ses->rses_client_dcb->session, + querystr); + free(querystr); + gwbuf_make_contiguous(querybuf); + GWBUF* ptr = router_cli_ses->queue; + + while(ptr && ptr->next) + { + ptr = ptr->next; + } + + if(ptr == NULL) + { + router_cli_ses->queue = querybuf; + } + else + { + ptr->next = querybuf; + + } + rses_end_locked_router_action(router_cli_ses); + return 1; + } + } - - if(tmp == NULL) - { - router_cli_ses->queue = querybuf; - } - else - { - tmp->next = querybuf; - } - - rses_end_locked_router_action(router_cli_ses); - return 1; - } - + rses_end_locked_router_action(router_cli_ses); packet = GWBUF_DATA(querybuf); @@ -1608,7 +1736,7 @@ routeQuery(ROUTER* instance, */ GWBUF* dbres = gen_show_dbs_response(inst,router_cli_ses); - poll_add_epollin_event_to_dcb(router_cli_ses->dummy_dcb,dbres); + poll_add_epollin_event_to_dcb(router_cli_ses->replydcb,dbres); ret = 1; goto retblock; } @@ -2982,7 +3110,7 @@ reply_error: * Create an incoming event for randomly selected backend DCB which * will then be notified and replied 'back' to the client. */ - poll_add_epollin_event_to_dcb(rses->dummy_dcb, + poll_add_epollin_event_to_dcb(rses->replydcb, gwbuf_clone(errbuf)); gwbuf_free(errbuf); } From 7c89f49f8242c4754c290967b59cb62912217e99 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 31 Mar 2015 04:21:43 +0300 Subject: [PATCH 26/27] Fix to MXS-74: https://mariadb.atlassian.net/browse/MXS-74 Added missing check for NULL pointer. --- server/modules/filter/dbfwfilter.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/server/modules/filter/dbfwfilter.c b/server/modules/filter/dbfwfilter.c index c8efd7e36..9927c1fd2 100644 --- a/server/modules/filter/dbfwfilter.c +++ b/server/modules/filter/dbfwfilter.c @@ -922,7 +922,8 @@ bool parse_rule(char* rule, FW_INSTANCE* instance) STRLINK *tail = NULL,*current; ruledef->type = RT_COLUMN; tok = strtok_r(NULL, " ,",&saveptr); - while(tok && strcmp(tok,"at_times") != 0){ + while(tok && strcmp(tok,"at_times") != 0 && + strcmp(tok,"on_queries") != 0){ current = malloc(sizeof(STRLINK)); current->value = strdup(tok); current->next = tail; @@ -1094,10 +1095,19 @@ bool parse_rule(char* rule, FW_INSTANCE* instance) else if(strcmp(tok,"on_queries") == 0) { tok = strtok_r(NULL," ",&saveptr); + + if(tok == NULL) + { + skygw_log_write(LOGFILE_ERROR, + "dbfwfilter: Missing parameter for 'on_queries'."); + rval = false; + goto retblock; + } + if(!parse_querytypes(tok,ruledef)){ skygw_log_write(LOGFILE_ERROR, "dbfwfilter: Invalid query type" - "requirements on where/having clauses: %s." + "requirements: %s." ,tok); rval = false; goto retblock; From be968cfad2befc1324579d205bb6044d1438d33f Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Tue, 31 Mar 2015 12:54:52 +0300 Subject: [PATCH 27/27] Fixed missing subservices parameter in config. --- server/core/config.c | 11 +++++++ .../routing/schemarouter/shardrouter.c | 30 +++++++++---------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/server/core/config.c b/server/core/config.c index cc50e1b99..58f5d92b3 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -312,6 +312,7 @@ int error_count = 0; char *strip_db_esc; char *weightby; char *version_string; + char *subservices; bool is_rwsplit = false; bool is_schemarouter = false; char *allow_localhost_match_wildcard_host; @@ -319,6 +320,7 @@ int error_count = 0; obj->element = service_alloc(obj->object, router); user = config_get_value(obj->parameters, "user"); auth = config_get_value(obj->parameters, "passwd"); + subservices = config_get_value(obj->parameters, "subservices"); enable_root_user = config_get_value( obj->parameters, "enable_root_user"); @@ -346,6 +348,15 @@ int error_count = 0; version_string = config_get_value(obj->parameters, "version_string"); + + if(subservices) + { + service_set_param_value(obj->element, + obj->parameters, + subservices, + 1,STRING_TYPE); + } + /** flag for rwsplit-specific parameters */ if (strncmp(router, "readwritesplit", strlen("readwritesplit")+1) == 0) { diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index 5d76ca9ad..03aa02a57 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -942,7 +942,7 @@ static ROUTER * createInstance(SERVICE *service, char **options) { ROUTER_INSTANCE* router; - char *services, *tok; + char *services, *tok, *saveptr; SERVICE **res_svc, **temp; CONFIG_PARAMETER* conf; int i = 0, sz; @@ -957,7 +957,6 @@ createInstance(SERVICE *service, char **options) spinlock_init(&router->lock); conf = config_get_param(service->svc_config_param, "subservices"); -/* if(conf == NULL) { @@ -968,22 +967,20 @@ createInstance(SERVICE *service, char **options) } services = strdup(conf->value); -*/ sz = 2; - res_svc = calloc(sz, sizeof(SERVICE*)); -/* - tok = strtok(services, ","); -*/ - if(options == NULL) + if((res_svc = calloc(sz, sizeof(SERVICE*))) == NULL) { free(router); - skygw_log_write(LOGFILE_ERROR, "Error : No 'subservice' router option found. Shardrouter requires at least %d " - "configured services listed in the 'subservices' router option to work.", min_nsvc); + skygw_log_write(LOGFILE_ERROR,"Error: Memory allocation failed."); return NULL; } - while(options[i]) + tok = strtok_r(services, ",",&saveptr); + + + + while(tok) { if(sz <= i) { @@ -991,9 +988,9 @@ createInstance(SERVICE *service, char **options) if(temp == NULL) { skygw_log_write(LOGFILE_ERROR, "Error : Memory reallocation failed."); - skygw_log_write(LOGFILE_DEBUG, "shardrouter.c: realloc returned NULL. " + LOGIF(LD,(skygw_log_write(LOGFILE_DEBUG, "shardrouter.c: realloc returned NULL. " "service count[%d] buffer size [%u] tried to allocate [%u]", - sz, sizeof(SERVICE*)*(sz), sizeof(SERVICE*)*(sz * 2)); + sz, sizeof(SERVICE*)*(sz), sizeof(SERVICE*)*(sz * 2)))); free(res_svc); free(router); return NULL; @@ -1002,7 +999,7 @@ createInstance(SERVICE *service, char **options) res_svc = temp; } - res_svc[i] = service_find(options[i]); + res_svc[i] = service_find(tok); if(res_svc[i] == NULL) { free(res_svc); @@ -1011,10 +1008,11 @@ createInstance(SERVICE *service, char **options) return NULL; } i++; + tok = strtok_r(NULL,",",&saveptr); } -/* + free(services); -*/ + router->services = res_svc; router->n_services = i;