BUG/MEDIUM: servers: Store the connection hash with the parameter cache

When we store the negociated server parameters, such as the ALPN, also
store the calculated hash with the connection. If it is different, as
can happen because the IP address is different because set-dst was used,
we certainly do not want to reuse the information in the cache,
otherwise we could end up using the wrong ALPN and mux.
That means we already have to calculate the hash in connect_server()
now, while before we would not do it for Websockets, if we could not do
connection reuse, as that's all the hash was used for.

This should fix Github issue #3386

This should be backported as far as 3.2.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index a3dbc606a3812bf44583bfbc2d366f08cbccd98a..0d557ab7b9cb95d6dd996d9bc6e870a27d6e2b70 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -329,6 +329,7 @@ enum renegotiate_mode {
 struct path_parameters {
        __decl_thread(HA_RWLOCK_T param_lock);
        char nego_alpn[MAX_ALPN_SIZE];
+       int64_t srv_hash;
 #ifdef USE_QUIC
        struct quic_early_transport_params tps;
 #endif

diff --git a/src/backend.c b/src/backend.c
index 97085be2586560dee3444b9c9f7d14e3c923ad74..45bdb38990814e4de7459364a083030f49882370 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1820,6 +1820,8 @@ int connect_server(struct stream *s)
        struct connection *srv_conn = NULL;
        const struct mux_proto_list *mux_proto;
        struct server *srv;
+       struct ist name = IST_NULL;
+       struct sample *name_smp;
        int reuse_mode;
        int reuse __maybe_unused = 0;
        int may_use_early_data __maybe_unused = 1; // are we allowed to use early data ?
@@ -1841,6 +1843,17 @@ int connect_server(struct stream *s)
        if (err != SRV_STATUS_OK)
                return SF_ERR_INTERNAL;
 
+       if (srv && srv->pool_conn_name_expr) {
+               name_smp = sample_fetch_as_type(s->be, s->sess, s,
+                               SMP_OPT_DIR_REQ | SMP_OPT_FINAL,
+                               srv->pool_conn_name_expr, SMP_T_STR);
+               if (name_smp) {
+                       name = ist2(name_smp->data.u.str.area,
+                                       name_smp->data.u.str.data);
+               }
+       }
+       hash = be_calculate_conn_hash(srv, s, s->sess, bind_addr, s->scb->dst, name);
+
        if (!be_supports_conn_reuse(s->be))
                goto skip_reuse;
 
@@ -1852,20 +1865,7 @@ int connect_server(struct stream *s)
        }
        else {
                const int not_first_req = s->txn.http && s->txn.http->flags & TX_NOT_FIRST;
-               struct ist name = IST_NULL;
-               struct sample *name_smp;
-
-               if (srv && srv->pool_conn_name_expr) {
-                       name_smp = sample_fetch_as_type(s->be, s->sess, s,
-                                                       SMP_OPT_DIR_REQ | SMP_OPT_FINAL,
-                                                       srv->pool_conn_name_expr, SMP_T_STR);
-                       if (name_smp) {
-                               name = ist2(name_smp->data.u.str.area,
-                                           name_smp->data.u.str.data);
-                       }
-               }
 
-               hash = be_calculate_conn_hash(srv, s, s->sess, bind_addr, s->scb->dst, name);
                err = be_reuse_connection(hash, s->sess, s->be, srv, s->scb,
                                          s->target, not_first_req);
                if (err == SF_ERR_INTERNAL)
@@ -2087,7 +2087,7 @@ int connect_server(struct stream *s)
                        if (IS_HTX_STRM(s) && srv->use_ssl &&
                            (srv->ssl_ctx.alpn_str || srv->ssl_ctx.npn_str)) {
                                HA_RWLOCK_RDLOCK(SERVER_LOCK, &srv->path_params.param_lock);
-                               if (srv->path_params.nego_alpn[0] == 0)
+                               if (srv->path_params.srv_hash != hash || srv->path_params.nego_alpn[0] == 0)
                                        may_start_mux_now = 0;
                                HA_RWLOCK_RDUNLOCK(SERVER_LOCK, &srv->path_params.param_lock);
                        }

diff --git a/src/connection.c b/src/connection.c
index 23d2a7447233e7d2164dcc3706b670998162bebe..c56049ed29cf29351fa511ef32be92f4055abe66 100644 (file)
--- a/src/connection.c
+++ b/src/connection.c
@@ -470,7 +470,7 @@ int conn_install_mux_be(struct connection *conn, void *ctx, struct session *sess
                int mode = conn_pr_mode_to_proto_mode(prx->mode);
 
                if (!conn_get_alpn(conn, &alpn_str, &alpn_len)) {
-                       if (srv && srv->path_params.nego_alpn[0]) {
+                       if (srv && srv->path_params.srv_hash == conn->hash_node.key && srv->path_params.nego_alpn[0]) {
                                alpn_str = srv->path_params.nego_alpn;
                                alpn_len = strlen(alpn_str);
                        }

diff --git a/src/server.c b/src/server.c
index 9ebce0c8c2f8704f43d0f40fb2b2c1983d5ef869..ae35cc72f558dac943a57cceab61d21ee3d0af41 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -145,6 +145,7 @@ static void srv_reset_path_parameters(struct server *s)
 {
        HA_RWLOCK_WRLOCK(SERVER_LOCK, &s->path_params.param_lock);
        s->path_params.nego_alpn[0] = 0;
+       s->path_params.srv_hash = 0;
        HA_RWLOCK_WRUNLOCK(SERVER_LOCK, &s->path_params.param_lock);
 }
 

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 38f8ab505081c6ba2333c1e152da75bafbdd958e..dcc41124d522c35667c7baf2977168ff66111f63 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4287,9 +4287,10 @@ static int ssl_sess_new_srv_cb(SSL *ssl, SSL_SESSION *sess)
                        if (ssl_sock_get_alpn(conn, qc->xprt_ctx, &alpn, &len)) {
                                struct quic_early_transport_params *etps = &s->path_params.tps;
 
-                               if (len < sizeof(s->path_params.nego_alpn) &&
-                                   (len != strlen(s->path_params.nego_alpn) ||
-                                    memcmp(&s->path_params.nego_alpn, alpn, len) != 0)) {
+                               if (s->path_params.srv_hash != conn->hash_node.key ||
+                                   (len < sizeof(s->path_params.nego_alpn) &&
+                                    (len != strlen(s->path_params.nego_alpn) ||
+                                    memcmp(&s->path_params.nego_alpn, alpn, len) != 0))) {
                                        HA_RWLOCK_WRLOCK(SERVER_LOCK, &s->path_params.param_lock);
                                        memcpy(&s->path_params.nego_alpn, alpn, len);
                                        s->path_params.nego_alpn[len] = 0;
@@ -6920,9 +6921,10 @@ struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned int state)
 
                                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)) {
+                                       if (srv->path_params.srv_hash != conn->hash_node.key ||
+                                           (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;