BUG/MINOR: quic: Unexpected half open connection counter wrapping

This counter must be incremented only one time by connection and decremented
as soon as the handshake has failed or succeeded. This is a gauge. Under certain
conditions this counter could be decremented twice. For instance
after having received a TLS alert, then upon SSL_do_handshake() failure.
To stop having to deal to all the current combinations which can lead to such a
situation (and the next to come), add a connection flag to denote if this counter
has been already decremented for a connection. So, this counter must be decremented
only if this flag has not been already set.

Must be backported up to 2.6.

diff --git a/include/haproxy/xprt_quic-t.h b/include/haproxy/xprt_quic-t.h
index af16656c4ed563ee1ec9cda2bc0b27e09e5048c5..1d530f42aaab70504300a55b63887063d815a6d5 100644 (file)
--- a/include/haproxy/xprt_quic-t.h
+++ b/include/haproxy/xprt_quic-t.h
@@ -623,6 +623,7 @@ enum qc_mux_state {
 #define QUIC_FL_CONN_RETRANS_OLD_DATA            (1U << 8)
 #define QUIC_FL_CONN_TLS_ALERT                   (1U << 9)
 #define QUIC_FL_CONN_APP_ALERT                   (1U << 10) /* A connection error of type CONNECTION_CLOSE_APP must be emitted. */
+#define QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED   (1U << 11) /* The half-open connection counter was decremented */
 #define QUIC_FL_CONN_NOTIFY_CLOSE                (1U << 27) /* MUX notified about quic-conn imminent closure (idle-timeout or CONNECTION_CLOSE emission/reception) */
 #define QUIC_FL_CONN_EXP_TIMER                   (1U << 28) /* timer has expired, quic-conn can be freed */
 #define QUIC_FL_CONN_CLOSING                     (1U << 29)

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 678d48ed35fb4e58a64a58ef6ec4df0c67407918..d78eecf279b3cacab9a739cbcd8a21d29f02fd83 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1072,7 +1072,10 @@ void quic_set_connection_close(struct quic_conn *qc, int err, int app)
 /* Set <alert> TLS alert as QUIC CRYPTO_ERROR error */
 void quic_set_tls_alert(struct quic_conn *qc, int alert)
 {
-       HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+       if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
+               qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
+               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+       }
        quic_set_connection_close(qc, QC_ERR_CRYPTO_ERROR | alert, 0);
        qc->flags |= QUIC_FL_CONN_TLS_ALERT;
        TRACE_PROTO("Alert set", QUIC_EV_CONN_SSLDATA, qc);
@@ -2052,8 +2055,12 @@ static inline int qc_provide_cdata(struct quic_enc_level *el,
                                goto out;
                        }
 
-                       HA_ATOMIC_INC(&qc->prx_counters->hdshk_fail);
-                       HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+                       /* TODO: Should close the connection asap */
+                       if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
+                               qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
+                               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+                               HA_ATOMIC_INC(&qc->prx_counters->hdshk_fail);
+                       }
                        TRACE_DEVEL("SSL handshake error",
                                    QUIC_EV_CONN_IO_CB, qc, &state, &ssl_err);
                        qc_ssl_dump_errors(ctx->conn);
@@ -2070,7 +2077,10 @@ static inline int qc_provide_cdata(struct quic_enc_level *el,
                        goto err;
                }
 
-               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+               if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
+                       qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
+                       HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+               }
                /* I/O callback switch */
                ctx->wait_event.tasklet->process = quic_conn_app_io_cb;
                if (qc_is_listener(ctx->qc)) {
@@ -2541,13 +2551,10 @@ static int qc_parse_pkt_frms(struct quic_rx_packet *pkt, struct ssl_sock_ctx *ct
                        /* Increment the error counters */
                        qc_cc_err_count_inc(qc, &frm);
                        if (!(qc->flags & QUIC_FL_CONN_DRAINING)) {
-                               /* If the connection did not reached the handshake complete state,
-                                * the <half_open_conn> counter was not decremented. Note that if
-                                * a TLS alert was received from the TLS stack, this counter
-                                * has already been decremented.
-                                */
-                               if (qc->state < QUIC_HS_ST_COMPLETE && !(qc->flags & QUIC_FL_CONN_TLS_ALERT))
+                               if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
+                                       qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
                                        HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+                               }
                                TRACE_PROTO("Entering draining state", QUIC_EV_CONN_PRSHPKT, qc);
                                /* RFC 9000 10.2. Immediate Close:
                                 * The closing and draining connection states exist to ensure
@@ -4427,7 +4434,6 @@ static struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int s
 {
        struct quic_conn *qc = ctx;
        struct quic_counters *prx_counters = qc->prx_counters;
-       int qc_state = qc->state;
        unsigned int qc_flags = qc->flags;
 
        /* Notify the MUX before settings QUIC_FL_CONN_EXP_TIMER or the MUX
@@ -4446,13 +4452,10 @@ static struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int s
         * least clean some parts of it such as the tasklet.
         */
 
-       /* If the connection did not reached the handshake complete state,
-        * the <half_open_conn> counter was not decremented. Note that if
-        * a TLS alert was received from the TLS stack, this counter
-        * has already been decremented.
-        */
-       if (qc_state < QUIC_HS_ST_COMPLETE && !(qc_flags & QUIC_FL_CONN_TLS_ALERT))
+       if (!(qc_flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
+               qc_flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
                HA_ATOMIC_DEC(&prx_counters->half_open_conn);
+       }
 
        return NULL;
 }