From 674b3887c9356b5ebe6c93782c49ed4fa5f93dc5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= <markus.makela@mariadb.com>
Date: Wed, 28 Jun 2017 21:00:08 +0300
Subject: [PATCH] MXS-1301: `function` matches functionless queries in `accept`
 mode

A function type rule matches a query if the query uses a function defined
in the rule. This is the desired behavior for blacklist mode operation
with `action=block`.

When in whitelist mode, all queries must match a rule to be allowed
through. For function type rules, this fact is problematic as queries that
don't use functions are blocked. The desired behavior is to allow the use
of certain functions while preventing the use of others.

The allowed set of functions should always contain the "empty set" (no
functions are used) when the filter is in whitelist mode.
---
 Documentation/Filters/Database-Firewall-Filter.md      |  4 ++++
 .../Release-Notes/MaxScale-2.2.0-Release-Notes.md      |  6 ++++++
 server/modules/filter/dbfwfilter/dbfwfilter.c          | 10 ++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/Filters/Database-Firewall-Filter.md b/Documentation/Filters/Database-Firewall-Filter.md
index 412fe0c93..cace99ff7 100644
--- a/Documentation/Filters/Database-Firewall-Filter.md
+++ b/Documentation/Filters/Database-Firewall-Filter.md
@@ -165,6 +165,10 @@ matched. The symbolic comparison operators (`<`, `>`, `>=` etc.) are also
 considered functions whereas the text versions (`NOT`, `IS`, `IS NOT` etc.) are
 not considered functions.
 
+When the filter is in whitelist mode (`action=allow`) the function rule
+will match any query that does not use a function. This means that queries
+that do not use functions will be allowed through a function type rule.
+
 ##### Example
 
 Deny SUM and COUNT functions:
diff --git a/Documentation/Release-Notes/MaxScale-2.2.0-Release-Notes.md b/Documentation/Release-Notes/MaxScale-2.2.0-Release-Notes.md
index 9e2ef5826..cfdefa8cc 100644
--- a/Documentation/Release-Notes/MaxScale-2.2.0-Release-Notes.md
+++ b/Documentation/Release-Notes/MaxScale-2.2.0-Release-Notes.md
@@ -54,6 +54,12 @@ The `match` and `exclude` parameters were changed to use PCRE2 syntax for the
 regular expressions. The regular expression should be enclosed by slashes
 e.g. `match=/select.*from.*test/`.
 
+### Dbfwfilter
+
+The `function` type rule will now match a query that does not use a function
+when the filter is in whitelist mode (`action=allow`). This means that queries
+that don't use functions are allowed though in whitelist mode.
+
 ## Dropped Features
 
 ### MaxAdmin
diff --git a/server/modules/filter/dbfwfilter/dbfwfilter.c b/server/modules/filter/dbfwfilter/dbfwfilter.c
index f105a8746..bedfb3780 100644
--- a/server/modules/filter/dbfwfilter/dbfwfilter.c
+++ b/server/modules/filter/dbfwfilter/dbfwfilter.c
@@ -2082,12 +2082,18 @@ void match_column(RULE_BOOK *rulebook, GWBUF *queue, bool *matches, char **msg)
     }
 }
 
-void match_function(RULE_BOOK *rulebook, GWBUF *queue, bool *matches, char **msg)
+void match_function(RULE_BOOK *rulebook, GWBUF *queue, enum fw_actions mode,
+                    bool *matches, char **msg)
 {
     const QC_FUNCTION_INFO* infos;
     size_t n_infos;
     qc_get_function_info(queue, &infos, &n_infos);
 
+    if (n_infos == 0 && mode == FW_ACTION_ALLOW)
+    {
+        *matches = true;
+    }
+
     for (size_t i = 0; i < n_infos; ++i)
     {
         const char* tok = infos[i].name;
@@ -2219,7 +2225,7 @@ bool rule_matches(FW_INSTANCE* my_instance,
         case RT_FUNCTION:
             if (is_sql)
             {
-                match_function(rulebook, queue, &matches, &msg);
+                match_function(rulebook, queue, my_instance->action, &matches, &msg);
             }
             break;