MEDIUM: http-rules: report access control rules relying on aliasable contents

Rules such as "http-request deny if { path_beg /admin }" are regularly
reported as bypassable using another form of the same request (case,
%-encoding, "//", "/./", "../", backslashes etc). This is expected, since
these aliases depend on what lies behind haproxy, and the only reliable
way to filter requests is to reject what was not explicitly accepted.

Now when an http-request rule using an action marked KWF_ACCESS_CTRL
(auth, deny, reject, silent-drop, tarpit) has a condition which may be
true because of a match on a sample fetch marked SMP_USE__MAY_ALIAS, it
is reported at boot. Only terms making the condition true when they
match are considered (non-negated after "if", negated after "unless"),
so that allow-lists are not reported. Converters are not considered, as
most of them do not handle all aliases.

Some rules are legitimately used as an extra filter (e.g. blocking %2E or
"<script" in the query string as a complement to other protections). For
these ones, the "no-warn" keyword placed right before "if" or "unless"
excludes the rule from the report. It also indicates to future readers
that the rule is known not to be reliable by itself. It is not made a
warning so as not to cause problems to configs running in zero-warning
mode.

The documentation was updated accordingly.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d9c41bf00d463a69e4f74837b03f4536a18e129d..9eafe88fccbdf2552899844ad3384f6ad533340c 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -9628,6 +9628,18 @@ http-request <action> [options...] [ { if | unless } <condition> ]
   supported actions and their respective syntaxes are enumerated in section 4.3
   "Actions" (look for actions which tick "HTTP Req").
 
+  Actions used to block requests ("auth", "deny", "reject", "silent-drop" and
+  "tarpit") are reported when their condition may be true because of a
+  pattern matching the path, the URL, the query string or the parameters they
+  convey (e.g. "deny if { path_beg /admin }"), because such rules are easy to
+  bypass. They are summarized in a notice by default, or reported as warnings
+  depending on the global "on-unreliable-rules" setting. Please read section
+  7.5 "Secure filtering of HTTP requests" to learn how to write reliable rules.
+  When such a rule is only meant as an extra filter, it may be excluded from
+  this report by placing the "no-warn" keyword right before "if" or "unless"
+  (e.g. "http-request deny no-warn if { ... }"). This keyword is only
+  supported by these actions.
+
   This directive is only available from named defaults sections, not anonymous
   ones. Rules defined in the defaults section are evaluated before ones in the
   associated proxy section. To avoid ambiguities, in this case the same
@@ -16247,7 +16259,7 @@ attach-srv <srv> [name <expr>] [ EXPERIMENTAL ]
   Note that a very similar but independent protocol is under development. See
   https://www.ietf.org/archive/id/draft-bt-httpbis-reverse-http-00.html.
 
-auth [realm <realm>]
+auth [realm <realm>] [no-warn]
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
                     - |          -  |   -  |   -  |   -  |          X |  - |  -
 
@@ -16265,6 +16277,13 @@ auth [realm <realm>]
   message must not be altered, consider to use "http-request return" rule
   instead.
 
+  Except in specific cases, this rule shouldn't be used to intercept access to
+  resources that may be aliased (path, query string etc), as it will not catch
+  all possible forms. See 7.5 "Secure filtering of HTTP requests" for details.
+  If this rule is used in an http-request statement conditionned by an ACL
+  involving inputs that may be subject to aliasing, a warning will be emitted
+  unless the "no-warn" option is placed before the "if" or "unless" keyword.
+
   Example:
         acl auth_ok http_auth_group(L1) G1
         http-request auth unless auth_ok
@@ -16394,7 +16413,7 @@ del-map(<map-name>) <key fmt>
 deny [ { status | deny_status } <code> ] [ content-type <type> ]
      [ { default-errorfiles | errorfile <file> | errorfiles <name> |
        file <file> | lf-file <file> | string <str> | lf-string <fmt> } ]
-     [ hdr <name> <fmt> ]*
+     [ hdr <name> <fmt> ]* [no-warn]
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
                     - |          -  |   -  |   -  |   -  |          X |  X |  -
 
@@ -16409,6 +16428,13 @@ deny [ { status | deny_status } <code> ] [ content-type <type> ]
   for the current section. See also the "return" action for the advanced
   syntax.
 
+  Except in specific cases, this rule shouldn't be used to intercept access to
+  resources that may be aliased (path, query string etc), as it will not catch
+  all possible forms. See 7.5 "Secure filtering of HTTP requests" for details.
+  If this rule is used in an http-request statement conditionned by an ACL
+  involving inputs that may be subject to aliasing, a warning will be emitted
+  unless the "no-warn" option is placed before the "if" or "unless" keyword.
+
 
 dgram-drop
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
@@ -16837,7 +16863,7 @@ redirect <rule>
   See the "redirect" keyword for the rule's syntax.
 
 
-reject
+reject [no-warn]
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
                     X |          X  |   X  |   X  |   X  |          X |  - |  -
 
@@ -16866,6 +16892,13 @@ reject
   connection is immediately closed without any SSL handshake processing and the
   client is notified via a CONNECTION_REFUSED error code.
 
+  Except in specific cases, this rule shouldn't be used to intercept access to
+  resources that may be aliased (path, query string etc), as it will not catch
+  all possible forms. See 7.5 "Secure filtering of HTTP requests" for details.
+  If this rule is used in an http-request statement conditionned by an ACL
+  involving inputs that may be subject to aliasing, a warning will be emitted
+  unless the "no-warn" option is placed before the "if" or "unless" keyword.
+
 
 replace-header <name> <match-regex> <replace-fmt>
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
@@ -17739,7 +17772,7 @@ set-vars-from-map(<scope>,<map-file>[,<type>])
     http-request set-vars-from-map(req,/etc/haproxy/limits.map,int)
 
 
-silent-drop [ rst-ttl <ttl> ]
+silent-drop [ rst-ttl <ttl> ] [no-warn]
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
                     - |          X  |   X  |   X  |   X  |          X |  X |  -
 
@@ -17765,6 +17798,13 @@ silent-drop [ rst-ttl <ttl> ]
   client's. This must not be used unless the consequences of doing this are
   fully understood.
 
+  Except in specific cases, this rule shouldn't be used to intercept access to
+  resources that may be aliased (path, query string etc), as it will not catch
+  all possible forms. See 7.5 "Secure filtering of HTTP requests" for details.
+  If this rule is used in an http-request statement conditionned by an ACL
+  involving inputs that may be subject to aliasing, a warning will be emitted
+  unless the "no-warn" option is placed before the "if" or "unless" keyword.
+
 
 strict-mode { on | off }
   Usable in:  QUIC Ini|    TCP RqCon| RqSes| RqCnt| RsCnt|    HTTP Req| Res| Aft
@@ -29842,6 +29882,25 @@ a real allow-list or to the application's own access control, for example to
 cheaply block known scanners or well-known attack patterns before they reach
 the servers. They must just never be the only protection.
 
+In order to help spot such rules, HAProxy reports them when the condition
+of an "http-request" rule using one of the "auth", "deny", "reject",
+"silent-drop" or "tarpit" actions may be true because of a match on contents
+which support multiple equivalent forms. These are the path, the URL, the
+query string and the parameters they convey, as returned by the "base",
+"base32", "base32+src", "baseq", "capture.req.uri", "path", "pathq", "query",
+"req.body_param", "url", "url32", "url32+src", "url_param", "urlp" and
+"urlp_val" sample fetch functions and their derived ACL keywords, regardless
+of any converter applied to them. Only a term which makes the condition true
+when it matches is considered, i.e. a non-negated term after "if", or a negated
+one after "unless". This means that the allow-lists above are not reported. A
+notice message will indicate affected rules. When such a rule is only meant as
+an extra filter, it may be excluded from this report by placing the "no-warn"
+keyword right before "if" or "unless". It also serves as a reminder for whoever
+modifies the rule later that it is not reliable by itself :
+
+    http-request deny no-warn if { path_sub -i %2e %2f }
+    http-request deny no-warn if { query -m sub -i <script }
+
 
 8. Logging
 ----------

diff --git a/src/http_rules.c b/src/http_rules.c
index a980fa976e3904e67c805292228065dd800f5cfe..e75bf34841308ea3ff39cd91e6c1a419843d8b50 100644 (file)
--- a/src/http_rules.c
+++ b/src/http_rules.c
@@ -94,6 +94,7 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
 {
        struct act_rule *rule;
        const struct action_kw *custom = NULL;
+       int no_warn = 0;
        int cur_arg;
 
        rule = new_act_rule(ACT_F_HTTP_REQ, file, linenum);
@@ -121,6 +122,7 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
                        }
 
                        if (*args[cur_arg] && strcmp(args[cur_arg - 1], "no-warn") == 0) {
+                               no_warn = 1;
                                for (cur_arg--; *args[cur_arg + 1]; cur_arg++)
                                        args[cur_arg] = args[cur_arg + 1];
                                args[cur_arg] = args[cur_arg + 1];
@@ -172,6 +174,23 @@ struct act_rule *parse_http_req_cond(const char **args, const char *file, int li
                        goto out_err;
                }
                rule->cond = cond;
+
+               if ((custom->flags & KWF_ACCESS_CTRL) && !no_warn) {
+                       const struct acl *acl;
+                       const char *kw;
+
+                       if (acl_cond_find_positive_use(cond, SMP_USE__MAY_ALIAS, &acl, &kw)) {
+                               ha_notice("parsing [%s:%d] : 'http-request %s' applies when '%s'%s%s%s matches a pattern,"
+                                         " which may be bypassed since HTTP allows many equivalent forms of a same request"
+                                         " (case, %%-encoding, '//', '/./', '/../', '\\', etc). Only rejecting what was not"
+                                         " explicitly accepted is reliable (e.g. 'http-request %s unless <accepted>')."
+                                         " If this rule is only an extra filter, add 'no-warn' before '%s' to silence this"
+                                         " warning. See 'Secure filtering of HTTP requests' in the configuration manual.\n",
+                                         file, linenum, args[0], kw,
+                                         *acl->name ? " (from ACL '" : "", acl->name, *acl->name ? "')" : "",
+                                         args[0], args[cur_arg]);
+                       }
+               }
        }
        else if (*args[cur_arg]) {
                ha_alert("parsing [%s:%d]: 'http-request %s' expects"