BUG/MINOR: resolvers: do not index resolvers names in the proxies

Resolvers are self-sustaining sections that have their own list, but for
the purpose of supporting TCP connections, a backend proxy is created
with them, and it holds the same name as the section. Since we started
to create a "default" resolvers section, a similarly named backend in
TCP mode automatically appeared, resulting in issues such as the one
described in GH #3445:

   defaults
       mode http

   frontend public
       bind :8001
       default_backend default

It yells: "... tries to use incompatible tcp proxy 'default' (<internal>:0)
in a 'use_backend' rule (see 'mode')" because it in fact finds the hidden
resolvers proxy. Depending on versions, adding such a default backend makes
the issue go away or not.

Worse, the following config:

   frontend public
       bind :8001
       default_backend default

actually routes the traffic to the local resolvers :-(

Resolvers do not need to appear in proxies index tree, as they're looked
up by find_resolvers_by_id() which iterates over all resolvers sections
(i.e. tree not used), so we can safely avoid indexing them.

Some other proxies are now protected against this via the use of PR_CAP_INT
which also prevents indexing, but that flag changed multiple times along
versions and is extremely sensitive in older ones, to the point of not
being suitable for a backport.

Instead, commit 116983ad94 ("MEDIUM: cfgparse: do not store unnamed
defaults in name tree") added the following test to refrain from
indexing an unnamed proxy in setup_new_proxy():

  @@ -1718,7 +1718,8 @@ int setup_new_proxy(struct proxy *px, const char *name, unsigned int cap, char *
          px->cap = cap;
          px->last_change = ns_to_sec(now_ns);

  -       if (name && !(cap & PR_CAP_INT))
  +       /* Internal proxies or with empty name are not stored in the named tree. */
  +       if (name && name[0] != '\0' && !(cap & PR_CAP_INT))
                  proxy_store_name(px);

          if (!(cap & PR_CAP_DEF))

This part is totally suitable for backporting and permits to simply
change the resolvers' proxy creation to pass NULL or an empty string
instead of the resolvers section's name so that we don't index that
proxy. That's what this patch does. For versions before 3.4, the
commented patch above must be explicitly backported. This patch must
be backported to 3.0. Thanks to Lukasz Nowak for reporting this bug.

It's worth noting that this area is really falling apart and urgently
needs attention. The internal API is currently too limited to permit
reliable lookups, which is the reason why so many proxies are hidden
that way. In an ideal world, all names would be indexed and caps and
modes properly set to unambiguous values, so that lookup functions
only look up specific caps and modes, and support being passed a mask
to avoid matching undesired caps. At the very least a mask should be
given in lookups, and a few extra caps should permit to distinguish
regular proxies (those declared with the "frontend" and "backend"
keyword, supporting use_backend) from other ones (rings, resolvers,
log-forward etc).

diff --git a/src/resolvers.c b/src/resolvers.c
index f646e6aa7e2f13eebf11e83f37ca83152ac921cc..89dc104a03927d7e8a7742e2ac39314780eaf6b1 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -3613,7 +3613,7 @@ static int resolvers_new(struct resolvers **resolvers, const char *id, const cha
        }
 
        /* allocate new proxy to tcp servers */
-       p = alloc_new_proxy(id, PR_CAP_FE | PR_CAP_BE, &errmsg);
+       p = alloc_new_proxy(NULL, PR_CAP_FE | PR_CAP_BE, &errmsg);
        if (!p) {
                ha_free(&errmsg); // ignored
                err_code |= ERR_ALERT | ERR_FATAL;