From 4fc1e39371bc8a450300a4ac0068aba243ca7d61 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 4 May 2026 17:42:45 +0200 Subject: [PATCH] BUG/MAJOR: http-ana: fix private session retrieval on NTLM During the architectural review leading to commit 90b2154d93 ("MEDIUM: muxes: always set conn->owner to the session that owns the connection"), we wondered whether srv_conn->owner could ever be NULL or even invalid, or if it ought to be changed to sess, and the projections of various use cases, as well as a number of attempts to fool it led us to conclude that it was always valid since the connection is private. So it was considered safer not to start fiddling with the pointer in case it could still match a previous session after a reuse, which would match the scenario described in the session_add_conn() comment. Actually there was exactly one case where a NULL could be met, and that was covered by the preliminary call to conn_set_owner() that was removed in that patch, precisely related to the one that the next patch tried to address: in http-reuse always, after the second request on a connection releases the connection, the owner can now become NULL, so if an NTLM header is seen at this point, it will crash. Interestingly, after the immediately following commit was merged, d93c53b0df ("MEDIUM: session: always reset the conn->owner on backend when installing mux"), the problem became immediate as the conn's owner is now null during operation if the connection is not private, and now the first response in NTLM is sufficient to crash the process. On the other hand, thanks to the two patches above, we're now certain never to meet a different session, which was the sought goal: either the session is normal and it has no owner, or it's private and it has as owner. Also with HTTP/1 (since the code explicitly checks for H1), there may be a single request at a time on a connection so the owner should either be the session or NULL. So this patch finally implements the original plan, to pass to session_add_conn(). The call is idempotent if the owner is already set, but at least the function performs some preliminary sanity checks which are quite welcome, so better continue to always call it. Note that this is only for 3.4 or any branch that has exactly the two patches above. And if the patches above were to ever be backported (together), this one would be needed as well. Thanks to Omkhar Arasaratnam for reporting this regression. --- src/http_ana.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http_ana.c b/src/http_ana.c index 0201253b1..b49967cbd 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -1695,7 +1695,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) sess->flags |= SESS_FL_PREFER_LAST; conn_set_private(srv_conn); /* If it fail now, the same will be done in mux->detach() callback */ - session_add_conn(srv_conn->owner, srv_conn); + session_add_conn(sess, srv_conn); break; } }