From: Lukas Tribus <lukas@ltri.eu>
Date: Sat, 27 Oct 2018 18:07:40 +0000 (+0200)
Subject: BUG/MINOR: only auto-prefer last server if lb-alg is non-deterministic
X-Git-Tag: v1.9-dev5~36
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=80512b186fd7f4ef3bc7d9c92b281c549d72aa8a;p=thirdparty%2Fhaproxy.git

BUG/MINOR: only auto-prefer last server if lb-alg is non-deterministic

While "option prefer-last-server" only applies to non-deterministic load
balancing algorithms, 401/407 responses actually caused haproxy to prefer
the last server unconditionally.

As this breaks deterministic load balancing algorithms like uri, this
patch applies the same condition here.

Should be backported to 1.8 (together with "BUG/MINOR: only mark
connections private if NTLM is detected").
---

diff --git a/doc/configuration.txt b/doc/configuration.txt
index b140b60abc..6c22ab2603 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2512,6 +2512,11 @@ balance url_param <param> [check_post]
   algorithm, mode nor option have been set. The algorithm may only be set once
   for each backend.
 
+  With authentication schemes that require the same connection like NTLM, URI
+  based alghoritms must not be used, as they would cause subsequent requests
+  to be routed to different backend servers, breaking the invalid assumptions
+  NTLM relies on.
+
   Examples :
         balance roundrobin
         balance url_param userid
@@ -6537,8 +6542,9 @@ no option prefer-last-server
   close of the connection. This can make sense for static file servers. It does
   not make much sense to use this in combination with hashing algorithms. Note,
   haproxy already automatically tries to stick to a server which sends a 401 or
-  to a proxy which sends a 407 (authentication required). This is mandatory for
-  use with the broken NTLM authentication challenge, and significantly helps in
+  to a proxy which sends a 407 (authentication required), when the load
+  balancing algorithm is not deterministic. This is mandatory for use with the
+  broken NTLM authentication challenge, and significantly helps in
   troubleshooting some faulty applications. Option prefer-last-server might be
   desirable in these environments as well, to avoid redistributing the traffic
   after every other response.
diff --git a/src/backend.c b/src/backend.c
index cbeadff0ef..834caec1f5 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -588,9 +588,9 @@ int assign_server(struct stream *s)
 	if (conn &&
 	    (conn->flags & CO_FL_CONNECTED) &&
 	    objt_server(conn->target) && __objt_server(conn->target)->proxy == s->be &&
+	    (s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_HI &&
 	    ((s->txn && s->txn->flags & TX_PREFER_LAST) ||
 	     ((s->be->options & PR_O_PREF_LAST) &&
-              (s->be->lbprm.algo & BE_LB_KIND) != BE_LB_KIND_HI &&
 	      (!s->be->max_ka_queue ||
 	       server_has_room(__objt_server(conn->target)) ||
 	       (__objt_server(conn->target)->nbpend + 1) < s->be->max_ka_queue))) &&
diff --git a/src/proto_http.c b/src/proto_http.c
index 980525decc..48cb383a0d 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -3712,7 +3712,8 @@ void http_end_txn_clean_session(struct stream *s)
 		 * server over the same connection. This is required by some
 		 * broken protocols such as NTLM, and anyway whenever there is
 		 * an opportunity for sending the challenge to the proper place,
-		 * it's better to do it (at least it helps with debugging).
+		 * it's better to do it (at least it helps with debugging), at
+		 * least for non-deterministic load balancing algorithms.
 		 */
 		s->txn->flags |= TX_PREFER_LAST;
 	}