]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: peers: Improve detection of config errors in peers sections
authorChristopher Faulet <cfaulet@haproxy.com>
Fri, 2 Jun 2023 12:10:36 +0000 (14:10 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 5 Jun 2023 06:24:34 +0000 (08:24 +0200)
There are several misuses in peers sections that are not detected during the
configuration parsing and that could lead to undefined behaviors or crashes.

First, only one listener is expected for a peers section. If several bind
lines or local peer definitions are used, an error is triggered. However, if
multiple addresses are set on the same bind line, there is no error while
only the last listener is properly configured. On the 2.8, there is no crash
but side effects are hardly predictable. On older version, HAProxy crashes
if an unconfigured listener is used.

Then, there is no check on remote peers name. It is unexpected to have same
name for several remote peers. There is now a test, performed during the
post-parsing, to verify all remote peer names are unique.

Finally, server parsing options for the peers sections are changed to be
sure a port is always defined, and not a port range or a port offset.

This patch fixes the issue #2066. It could be backported to all stable
versions.

src/cfgparse.c
src/server.c

index 84c8cfa0cc5aebdc23ace930709603fcf46b8513..7409ab42b1d8aef122419a059d97f76937e9bd0a 100644 (file)
@@ -744,6 +744,14 @@ int cfg_parse_peers(const char *file, int linenum, char **args, int kwm)
                                goto out;
                        }
 
+                       /* Only one listener supported. Compare first listener
+                        * against the last one. It must be the same one.
+                        */
+                       if (bind_conf->listeners.n != bind_conf->listeners.p) {
+                               ha_alert("parsing [%s:%d] : Only one listener per \"peers\" section is authorized. Multiple listening addresses or port range are not supported.\n", file, linenum);
+                               err_code |= ERR_ALERT | ERR_FATAL;
+                               goto out;
+                       }
                        /*
                         * Newly allocated listener is at the end of the list
                         */
@@ -4371,6 +4379,19 @@ init_proxies_list_stage2:
                                curpeers->peers_fe->flags |= PR_FL_READY;
                                p = curpeers->remote;
                                while (p) {
+                                       struct peer *other_peer;
+
+                                       for (other_peer = curpeers->remote; other_peer && other_peer != p; other_peer = other_peer->next) {
+                                               if (strcmp(other_peer->id, p->id) == 0) {
+                                                       ha_alert("Peer section '%s' [%s:%d]: another peer named '%s' was already defined at line %s:%d, please use distinct names.\n",
+                                                                curpeers->peers_fe->id,
+                                                                p->conf.file, p->conf.line,
+                                                                other_peer->id, other_peer->conf.file, other_peer->conf.line);
+                                                       cfgerr++;
+                                                       break;
+                                               }
+                                       }
+
                                        if (p->srv) {
                                                if (p->srv->use_ssl == 1 && xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->prepare_srv)
                                                        cfgerr += xprt_get(XPRT_SSL)->prepare_srv(p->srv);
index a6e6b39d69e211103b3c8d7fc3621a1396d0b7be..eefb3bccced6e4dd1722cbf9748c0fa37f346400 100644 (file)
@@ -2771,7 +2771,9 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
 
                sk = str2sa_range(args[*cur_arg], &port, &port1, &port2, NULL, NULL,
                                  &errmsg, NULL, &fqdn,
-                                 (parse_flags & SRV_PARSE_INITIAL_RESOLVE ? PA_O_RESOLVE : 0) | PA_O_PORT_OK | PA_O_PORT_OFS | PA_O_STREAM | PA_O_XPRT | PA_O_CONNECT);
+                                 (parse_flags & SRV_PARSE_INITIAL_RESOLVE ? PA_O_RESOLVE : 0) | PA_O_PORT_OK |
+                                 (parse_flags & SRV_PARSE_IN_PEER_SECTION ? PA_O_PORT_MAND : PA_O_PORT_OFS) |
+                                 PA_O_STREAM | PA_O_XPRT | PA_O_CONNECT);
                if (!sk) {
                        ha_alert("%s\n", errmsg);
                        err_code |= ERR_ALERT | ERR_FATAL;