MEDIUM: tcp-rules: Warn if a track-sc* content rule doesn't depend on content

The warning is only emitted for HTTP frontend. Idea is to encourage the usage of
"tcp-request session" rules to track counters that does not depend on the
request content. The documentation has been updated accordingly.

The warning is important because since the multiplexers were added in the
processing chain, the HTTP parsing is performed at a lower level. Thus parsing
errors are detected in the multiplexers, before the stream creation. In HTTP/2,
the error is reported by the multiplexer itself and the stream is never
created. This difference has a certain number of consequences, one of which is
that HTTP request counting in stick tables only works for valid H2 request, and
HTTP error tracking in stick tables never considers invalid H2 requests but only
invalid H1 ones. And the aim is to do the same with the mux-h1. This change will
not be done for the 2.3, but the 2.4. At the end, H1 and H2 parsing errors will
be caught by the multiplexers, at the session level. Thus, tracking counters at
the content level should be reserved for rules using a key based on the request
content or those using ACLs based on the request content.

To be clear, a warning will be emitted for the following rules :

  tcp-request content track-sc0 src
  tcp-request content track-sc0 src if ! { src 10.0.0.0/24 }
  tcp-request content track-sc0 src if { ssl_fc }

But not for the following ones :

  tcp-request content track-sc0 req.hdr(host)
  tcp-request content track-sc0 src if { req.hdr(host) -m found }

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 9442c24915a8797c221a687b1d33c2a3ba53a85c..3ecca8593810378357ded1df81be6ec573dfad5f 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11304,6 +11304,13 @@ tcp-request content <action> [{if | unless} <condition>]
   the action, it is simply performed unconditionally. That can be useful for
   "track-sc*" actions as well as for changing the default action to a reject.
 
+  Note also that it is recommended to use a "tcp-request session" rule to track
+  information that does *not* depend on Layer 7 contents, especially for HTTP
+  frontends. Some HTTP processing are performed at the session level and may
+  lead to an early rejection of the requests. Thus, the tracking at the content
+  level may be disturbed in such case. A warning is emitted during startup to
+  prevent, as far as possible, such unreliable usage.
+
   It is perfectly possible to match layer 7 contents with "tcp-request content"
   rules from a TCP proxy, since HTTP-specific ACL matches are able to
   preliminarily parse the contents of a buffer before extracting the required

diff --git a/src/action.c b/src/action.c
index 6c4aa62a82535f24a03d1453e18cb8dcba1fde83..870a8393ae7e6d6a65f2c797f96e3a1f4831065d 100644 (file)
--- a/src/action.c
+++ b/src/action.c
@@ -68,13 +68,30 @@ int check_trk_action(struct act_rule *rule, struct proxy *px, char **err)
                 */
        }
 
-       if (rule->from == ACT_F_TCP_REQ_CNT && (px->cap & PR_CAP_FE) && !px->tcp_req.inspect_delay &&
-           !(rule->arg.trk_ctr.expr->fetch->val & SMP_VAL_FE_SES_ACC)) {
-               ha_warning("config : %s '%s' : a 'tcp-request content track-sc*' rule explicitly depending on request"
-                          " contents without any 'tcp-request inspect-delay' setting."
-                          " This means that this rule will randomly find its contents. This can be fixed by"
-                          " setting the tcp-request inspect-delay.\n",
-                          proxy_type_str(px), px->id);
+       if (rule->from == ACT_F_TCP_REQ_CNT && (px->cap & PR_CAP_FE)) {
+               if (!px->tcp_req.inspect_delay && !(rule->arg.trk_ctr.expr->fetch->val & SMP_VAL_FE_SES_ACC)) {
+                       ha_warning("config : %s '%s' : a 'tcp-request content track-sc*' rule explicitly depending on request"
+                                  " contents without any 'tcp-request inspect-delay' setting."
+                                  " This means that this rule will randomly find its contents. This can be fixed by"
+                                  " setting the tcp-request inspect-delay.\n",
+                                  proxy_type_str(px), px->id);
+               }
+
+               /* The following warning is emitted because HTTP multiplexers are able to catch errors
+                * or timeouts at the session level, before instantiating any stream.
+                * Thus the tcp-request content ruleset will not be evaluated in such case. It means,
+                * http_req and http_err counters will not be incremented as expected, even if the tracked
+                * counter does not use the request content. To track invalid requests it should be
+                * performed at the session level using a tcp-request session rule.
+                */
+               if (px->mode == PR_MODE_HTTP &&
+                   !(rule->arg.trk_ctr.expr->fetch->use & (SMP_USE_L6REQ|SMP_USE_HRQHV|SMP_USE_HRQHP|SMP_USE_HRQBO)) &&
+                   (!rule->cond || !(rule->cond->use & (SMP_USE_L6REQ|SMP_USE_HRQHV|SMP_USE_HRQHP|SMP_USE_HRQBO)))) {
+                       ha_warning("config : %s '%s' : a 'tcp-request content track-sc*' rule not depending on request"
+                                  " contents for an HTTP frontend should be executed at the session level, using a"
+                                  " 'tcp-request session' rule (mandatory to track invalid HTTP requests).\n",
+                                  proxy_type_str(px), px->id);
+               }
        }
 
        return 1;