BUG/MINOR: backend: check delay MUX before conn_prepare()

In connect_server(), when a new connection must be instantiated, MUX
initialization is delayed if an ALPN setting is present on the server
line configuration, as negotiation must be performed to select the
correct MUX. However, this is not the case if the ALPN can already be
retrieved on the server cache.

This check is performed too late however and may cause issue with the
QUIC stack. The problem can happen when the server ALPN is not yet set.
In the normal case, quic_conn layer is instantiated and MUX init is
delayed until the handshake completion. When the MUX is finally
instantiated, it reused without any issue app_ops from its quic_conn,
which is derived from the negotiated ALPN.

However, there is a race condition if another QUIC connection populates
the server ALPN cache. If this happens after the first quic_conn init
but prior to the MUX delay check, the MUX will thus immediately start in
connect_server(). When app_ops is retrieved from its quic_conn, a crash
occurs in qcc_install_app_ops() as the QUIC handshake is not yet
finalized :

  #0  0x000055e242a66df4 in qcc_install_app_ops (qcc=0x7f127c39da90, app_ops=0x0) at src/mux_quic.c:1697
  1697		if (app_ops->init && !app_ops->init(qcc)) {
  [Current thread is 1 (Thread 0x7f12810f06c0 (LWP 25758))]

To fix this, MUX delay check is moved up in connect_server(). It is now
performed prior conn_prepare() which is responsible for the quic_conn
layer instantiation. Thus, it ensures consistency for the QUIC stack :
MUX init is always delayed if the quic_conn does not reuses itself the
SSL session and ALPN server cache (no quic_reuse_srv_params()).

This must be backported up to 3.3.

diff --git a/src/backend.c b/src/backend.c
index 49fe787ea9554c8345099b31547a56d7c5567f51..573bb9edef900724ec467ed3fcc620ef8b1ec837 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -2059,6 +2059,25 @@ int connect_server(struct stream *s)
                                        srv_conn->sni_hash = ssl_sock_sni_hash(sni);
                                }
                        }
+
+#if defined(TLSEXT_TYPE_application_layer_protocol_negotiation)
+                       /* Delay mux initialization if SSL and ALPN/NPN is set
+                        * and server cache is not yet populated. Note that in
+                        * TCP mode this check is ignored as only mux-pt is
+                        * available.
+                        *
+                        * This check must be performed before conn_prepare()
+                        * to ensure consistency accross the whole stack, in
+                        * particular for QUIC between quic-conn and mux layer.
+                        */
+                       HA_RWLOCK_RDLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+                       if (IS_HTX_STRM(s) && srv->use_ssl &&
+                           (srv->ssl_ctx.alpn_str || srv->ssl_ctx.npn_str) &&
+                           srv->path_params.nego_alpn[0] == 0)
+                               may_start_mux_now = 0;
+                       HA_RWLOCK_RDUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+#endif /* TLSEXT_TYPE_application_layer_protocol_negotiation */
+
 #endif /* USE_OPENSSL */
 
                        if (conn_prepare(srv_conn, proto, srv->xprt)) {
@@ -2091,21 +2110,6 @@ int connect_server(struct stream *s)
                }
                srv_conn->ctx = s->scb;
 
-#if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation)
-               /* Delay mux initialization if SSL and ALPN/NPN is set. Note
-                * that this is skipped in TCP mode as we only want mux-pt
-                * anyway.
-                */
-               if (srv) {
-                       HA_RWLOCK_RDLOCK(SERVER_LOCK, &srv->path_params.param_lock);
-                       if (IS_HTX_STRM(s) && srv->use_ssl &&
-                           (srv->ssl_ctx.alpn_str || srv->ssl_ctx.npn_str) &&
-                           srv->path_params.nego_alpn[0] == 0)
-                               may_start_mux_now = 0;
-                       HA_RWLOCK_RDUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
-               }
-#endif
-
                /* process the case where the server requires the PROXY protocol to be sent */
                srv_conn->send_proxy_ofs = 0;