Le 07/07/2020 à 15:16, Michael Wimmesberger a écrit :
Hi,
I might have found a potentially critical bug in haproxy. It occurs when
haproxy is retrying to dispatch a request to a server. If haproxy fails
to dispatch a request to a server that is either up or has no health
checks enabled it dispatches the request to a random server on any
backend in any mode (tcp or http) as long as they are in the up state
(via tcp-connect or httpchk health checks). In addition haproxy logs the
correct server although it dispatches the request to a wrong server.
Hi Michael,
Thanks for the reproducer and the detailed description. I'm able to reproduce
the bug thanks to it. I attached a patch to address it. I will see with Willy
tomorrow morning if it is the good way to fix it. But it should do the trick.
Thanks again !
--
Christopher Faulet
>From bbc89e04a252b3719f221cd0afbad49f507c4c46 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Tue, 7 Jul 2020 22:25:19 +0200
Subject: [PATCH] BUG/MAJOR: stream: Mark the server address as unset on new
outgoing connection
In connect_server() function, when a new connection is created, it is important
to mark the server address as explicitly unset, removing the SF_ADDR_SET
stream's flag, to be sure to set it. On the first connect attempt, it is not a
problem because the flag is not set. But on a connection failure, the faulty
endpoint is detached. It is specific to the 2.0. See the commit 7b69c91e7
("BUG/MAJOR: stream-int: always detach a faulty endpoint on connect failure")
for details. As a result of this commit, on a connect retry, a new connection is
created. But this time, the SF_ADDR_SET flag is set (except on a
redispatch). But in reality, because it is a new connection, the server address
is not set.
On the other end, when a connection is released (or when it is created), the
from/to addresses are not cleared. Thus because of the described bug, when a
connection is get from the memory pool, the addresses of the previous connection
is used. leading to undefined and random routing. For a totally new connection,
no addresses are set and an internal error is reported by si_connect().
A reproducer with a detailed description was posted on the ML:
https://www.mail-archive.com/haproxy@formilux.org/msg37850.html
This patch should fix the issue #717. There is no direct mainline commit ID for
this fix, and it must not be backported as it's solely for 2.0.
---
src/backend.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/backend.c b/src/backend.c
index d0d11779a..2d0fd6a43 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1457,6 +1457,7 @@ int connect_server(struct stream *s)
/* no reuse or failed to reuse the connection above, pick a new one */
if (!srv_conn) {
+ s->flags &= ~SF_ADDR_SET;
srv_conn = conn_new();
if (srv_conn)
srv_conn->target = s->target;
--
2.26.2
