From: Christopher Faulet <cfaulet@haproxy.com>
Date: Fri, 2 Jun 2023 12:10:36 +0000 (+0200)
Subject: BUG/MINOR: peers: Improve detection of config errors in peers sections
X-Git-Tag: v2.9-dev1~71
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2c29d1f524b533a2b9be584f59f014d3bc2323a2;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 84c8cfa0cc..7409ab42b1 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 @@ 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);
diff --git a/src/server.c b/src/server.c
index a6e6b39d69..eefb3bccce 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;