From: Frédéric Lécaille Date: Fri, 17 Jun 2022 13:11:32 +0000 (+0200) Subject: BUG/MINOR: quic: Unexpected half open connection counter wrapping X-Git-Tag: v2.7-dev1~26 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2aebaa49b11c44fbbc4f059bb24695ebf13e0537;p=thirdparty%2Fhaproxy.git 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 af16656c4e..1d530f42aa 100644 --- 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 678d48ed35..d78eecf279 100644 --- 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 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 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 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; }