From 2c29d1f524b533a2b9be584f59f014d3bc2323a2 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 2 Jun 2023 14:10:36 +0200 Subject: [PATCH] BUG/MINOR: peers: Improve detection of config errors in peers sections 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 | 21 +++++++++++++++++++++ src/server.c | 4 +++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/cfgparse.c b/src/cfgparse.c index 84c8cfa0c..7409ab42b 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -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 @@ int check_config_validity() 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); diff --git a/src/server.c b/src/server.c index a6e6b39d6..eefb3bccc 100644 --- a/src/server.c +++ b/src/server.c @@ -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;