BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure

QUIC connections are accounted inside global sslconns. As with QUIC
actconn, it suffered from a similar issue if an intermediary allocation
failed inside qc_new_conn().

Fix this similarly by moving increment operation inside qc_new_conn().
Increment and error path are now centralized and much easier to
validate.

The consequences are similar to the actconn fix : on memory allocation
global sslconns may wrap, this time blocking any future QUIC or SSL
connections on the process.

This must be backported up to 2.6.

diff --git a/include/haproxy/quic_ssl.h b/include/haproxy/quic_ssl.h
index f2518ba8a0fba2bcc47a56f0a449e1164751b2d7..f31cafc3d83f3c4dc89a80962f4bb80fc34e72de 100644 (file)
--- a/include/haproxy/quic_ssl.h
+++ b/include/haproxy/quic_ssl.h
@@ -43,8 +43,6 @@ static inline void qc_free_ssl_sock_ctx(struct ssl_sock_ctx **ctx)
        SSL_free((*ctx)->ssl);
        pool_free(pool_head_quic_ssl_sock_ctx, *ctx);
        *ctx = NULL;
-
-       _HA_ATOMIC_DEC(&global.sslconns);
 }
 
 #endif /* _HAPROXY_QUIC_SSL_H */

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 7f300c9d038eda63b0be4cdfd9915707cccb7784..f25bf4c9ff07456bd7331c98e8200f41248f5036 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1162,7 +1162,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
        struct quic_conn *qc = NULL;
        struct listener *l = NULL;
        struct quic_cc_algo *cc_algo = NULL;
-       unsigned int next_actconn = 0;
+       unsigned int next_actconn = 0, next_sslconn = 0;
 
        TRACE_ENTER(QUIC_EV_CONN_INIT);
 
@@ -1173,6 +1173,12 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                goto err;
        }
 
+       next_sslconn = increment_sslconn();
+       if (!next_sslconn) {
+               TRACE_STATE("sslconn reached", QUIC_EV_CONN_INIT);
+               goto err;
+       }
+
        qc = pool_alloc(pool_head_quic_conn);
        if (!qc) {
                TRACE_ERROR("Could not allocate a new connection", QUIC_EV_CONN_INIT);
@@ -1182,7 +1188,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
        /* Now that quic_conn instance is allocated, quic_conn_release() will
         * ensure global accounting is decremented.
         */
-       next_actconn = 0;
+       next_sslconn = next_actconn = 0;
 
        /* Initialize in priority qc members required for a safe dealloc. */
        qc->nictx = NULL;
@@ -1382,6 +1388,8 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
         */
        if (next_actconn)
                _HA_ATOMIC_DEC(&actconn);
+       if (next_sslconn)
+               _HA_ATOMIC_DEC(&global.sslconns);
 
        TRACE_LEAVE(QUIC_EV_CONN_INIT);
        return NULL;
@@ -1519,6 +1527,7 @@ void quic_conn_release(struct quic_conn *qc)
         * time with limited ressources.
         */
        _HA_ATOMIC_DEC(&actconn);
+       _HA_ATOMIC_DEC(&global.sslconns);
 
        TRACE_PROTO("QUIC conn. freed", QUIC_EV_CONN_FREED, qc);
  leave:

diff --git a/src/quic_rx.c b/src/quic_rx.c
index da98403a445392fe7251964ed0431ee39c345c7d..c921c975130553e17abd2bd20d7d9a0ccfda434b 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1902,7 +1902,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
        struct quic_conn *qc = NULL;
        struct proxy *prx;
        struct quic_counters *prx_counters;
-       unsigned int next_sslconn = 0;
 
        TRACE_ENTER(QUIC_EV_CONN_LPKT);
 
@@ -1960,13 +1959,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
                        pkt->saddr = dgram->saddr;
                        ipv4 = dgram->saddr.ss_family == AF_INET;
 
-                       next_sslconn = increment_sslconn();
-                       if (!next_sslconn) {
-                               TRACE_STATE("drop packet on sslconn reached",
-                                           QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
-                               goto err;
-                       }
-
                        /* Generate the first connection CID. This is derived from the client
                         * ODCID and address. This allows to retrieve the connection from the
                         * ODCID without storing it in the CID tree. This is an interesting
@@ -1985,7 +1977,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
                                goto err;
                        }
 
-                       next_sslconn = 0;
                        /* Compute and store into the quic_conn the hash used to compute extra CIDs */
                        if (quic_hash64_from_cid)
                                qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len,
@@ -2036,9 +2027,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
        else
                HA_ATOMIC_INC(&prx_counters->dropped_pkt);
 
-       if (next_sslconn)
-               _HA_ATOMIC_DEC(&global.sslconns);
-
        TRACE_LEAVE(QUIC_EV_CONN_LPKT);
        return NULL;
 }