BUG/MEDIUM: ssl: Always check the ALPN after handshake

Move the code that is responsible for checking the ALPN, and updating
the one stored in the server's path_param, from after we created the
mux, to after we did an handshake. Once we did it once, the mux will not
be created by the ssl code anymore, as when we know which mux to use
thanks to the ALPN, it will be done earlier in connect_server(), so in
the unlikely event it changes, we would not detect it anymore, and we'd
keep on creating the wrong mux.
This can be reproduced by doing a first request, and then changing the
ALPN of the server without haproxy noticing (ie without haproxy noticing
that the server went down).

This should be backported to 3.3.

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 601c976130ceea8c332c067d579d342ca403c13e..1bc35d3ff1e40e8598fb0d312221a0e5cca298a4 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6860,6 +6860,32 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
                                ssl_sock_setup_ktls(ctx);
 #endif
 #endif
+                       /*
+                        * For backend connections, attempt to
+                        * retrieve the ALPN, and store it into
+                        * the server's path_params, so that for
+                        * next connections, we'll know the ALPN
+                        * already, and immediately know which mux
+                        * to use, in case we want to use 0RTT.
+                        */
+                       if (!(conn->flags & CO_FL_ERROR) && conn_is_back(conn)) {
+                               struct server *srv;
+                               const char *alpn;
+                               int len;
+
+                               srv = objt_server(conn->target);
+                               if (srv && ssl_sock_get_alpn(conn, ctx, &alpn, &len)) {
+                                       if (len < sizeof(srv->path_params.nego_alpn) &&
+                                           (len != strlen(srv->path_params.nego_alpn) ||
+                                            memcmp(&srv->path_params.nego_alpn, alpn, len) != 0)) {
+                                               HA_RWLOCK_WRLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+                                               memcpy(&srv->path_params.nego_alpn, alpn, len);
+                                               srv->path_params.nego_alpn[len] = 0;
+                                               HA_RWLOCK_WRUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
+                                       }
+                               }
+
+                       }
                }
        }
        /* If we had an error, or the handshake is done and I/O is available,
@@ -6893,35 +6919,8 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
                if (ctx->conn->xprt_ctx == ctx) {
                        int closed_connection = 0;
 
-                       if (!ctx->conn->mux) {
+                       if (!ctx->conn->mux)
                                ret = conn_create_mux(ctx->conn, &closed_connection);
-                               /*
-                                * For backend connections, attempt to
-                                * retrieve the ALPN, and store it into
-                                * the server's path_params, so that for
-                                * next connections, we'll know the ALPN
-                                * already, and immediately know which mux
-                                * to use, in case we want to use 0RTT.
-                                */
-                               if (ret >= 0 && conn_is_back(conn)) {
-                                       struct server *srv;
-                                       const char *alpn;
-                                       int len;
-
-                                       srv = objt_server(conn->target);
-                                       if (srv && ssl_sock_get_alpn(conn, ctx, &alpn, &len)) {
-                                               if (len < sizeof(srv->path_params.nego_alpn) &&
-                                                   (len != strlen(srv->path_params.nego_alpn) ||
-                                                    memcmp(&srv->path_params.nego_alpn, alpn, len) != 0)) {
-                                                       HA_RWLOCK_WRLOCK(SERVER_LOCK, &srv->path_params.param_lock);
-                                                       memcpy(&srv->path_params.nego_alpn, alpn, len);
-                                                       srv->path_params.nego_alpn[len] = 0;
-                                                       HA_RWLOCK_WRUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
-                                               }
-                                       }
-                               }
-
-                       }
                        if (ret >= 0 && !woke && ctx->conn->mux && ctx->conn->mux->wake) {
                                ret = ctx->conn->mux->wake(ctx->conn);
                                if (ret < 0)