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 af16656..1d530f4 100644
--- a/include/haproxy/xprt_quic-t.h
+++ b/include/haproxy/xprt_quic-t.h
@@ -623,6 +623,7 @@
#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 678d48e..d78eecf 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -1072,7 +1072,10 @@
/* 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 @@
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 @@
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 @@
/* 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 @@
{
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 @@
* 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;
}