]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: tcp-checks: always attach the transport before installing the mux
authorWilly Tarreau <w@1wt.eu>
Fri, 31 Jul 2020 06:49:31 +0000 (08:49 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 31 Jul 2020 06:49:31 +0000 (08:49 +0200)
Similarly to the issue described in commit "BUG/MEDIUM: backend: always
attach the transport before installing the mux", in tcpcheck_eval_connect()
we can install a handshake transport layer underneath the mux and replace
its subscriptions, causing a crash if the mux had already subscribed for
whatever reason.

A simple reproducer consists in adding fd_cant_recv() after fd_cant_send()
in tcp_connect_server() and running it on this config, as discussed in issue

  listen foo
    bind :8181
    mode http
    option httpchk
    server srv1 127.0.0.1:8888 send-proxy-v2 check inter 1000

The mux may only be installed *after* xprt_handshake is set up, so that
it registers against it and not against raw_sock or ssl_sock. This needs
to be backported to 2.2 which is the first version using muxes for checks.

src/tcpcheck.c

index fc64a10bb0841115e5f0cc87fa3fae3a098954be..65972396a763b172d6a986bd9b5a4016fbf44421 100644 (file)
@@ -1091,31 +1091,6 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec
        conn_set_private(conn);
        conn->ctx = cs;
 
-       /* The mux may be initialized now if there isn't server attached to the
-        * check (email alerts) or if there is a mux proto specified or if there
-        * is no alpn.
-        */
-       if (!s || ((connect->options & TCPCHK_OPT_DEFAULT_CONNECT) && check->mux_proto) ||
-           connect->mux_proto || (!connect->alpn && !check->alpn_str)) {
-               const struct mux_ops *mux_ops;
-
-               if (connect->mux_proto)
-                       mux_ops = connect->mux_proto->mux;
-               else if ((connect->options & TCPCHK_OPT_DEFAULT_CONNECT) && check->mux_proto)
-                       mux_ops = check->mux_proto->mux;
-               else {
-                       int mode = ((check->tcpcheck_rules->flags & TCPCHK_RULES_PROTO_CHK) == TCPCHK_RULES_HTTP_CHK
-                                   ? PROTO_MODE_HTTP
-                                   : PROTO_MODE_TCP);
-
-                       mux_ops = conn_get_best_mux(conn, IST_NULL, PROTO_SIDE_BE, mode);
-               }
-               if (mux_ops && conn_install_mux(conn, mux_ops, cs, proxy, check->sess) < 0) {
-                       status = SF_ERR_INTERNAL;
-                       goto fail_check;
-               }
-       }
-
 #ifdef USE_OPENSSL
        if (connect->sni)
                ssl_sock_set_servername(conn, connect->sni);
@@ -1155,6 +1130,31 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec
                        status = SF_ERR_RESOURCE;
        }
 
+       /* The mux may be initialized now if there isn't server attached to the
+        * check (email alerts) or if there is a mux proto specified or if there
+        * is no alpn.
+        */
+       if (!s || ((connect->options & TCPCHK_OPT_DEFAULT_CONNECT) && check->mux_proto) ||
+           connect->mux_proto || (!connect->alpn && !check->alpn_str)) {
+               const struct mux_ops *mux_ops;
+
+               if (connect->mux_proto)
+                       mux_ops = connect->mux_proto->mux;
+               else if ((connect->options & TCPCHK_OPT_DEFAULT_CONNECT) && check->mux_proto)
+                       mux_ops = check->mux_proto->mux;
+               else {
+                       int mode = ((check->tcpcheck_rules->flags & TCPCHK_RULES_PROTO_CHK) == TCPCHK_RULES_HTTP_CHK
+                                   ? PROTO_MODE_HTTP
+                                   : PROTO_MODE_TCP);
+
+                       mux_ops = conn_get_best_mux(conn, IST_NULL, PROTO_SIDE_BE, mode);
+               }
+               if (mux_ops && conn_install_mux(conn, mux_ops, cs, proxy, check->sess) < 0) {
+                       status = SF_ERR_INTERNAL;
+                       goto fail_check;
+               }
+       }
+
   fail_check:
        /* It can return one of :
         *  - SF_ERR_NONE if everything's OK