MINOR: quic: reduce half open counters scope

Accounting is implemented for half open connections which represent QUIC
connections waiting for handshake completion. When reaching a certain
limit, Retry mechanism is automatically activated prior to instantiate
new connections.

The issue with this behavior is that two notions are mixed : QUIC
connection handshake phase and Retry which is mechanism against
amplification attacks. As such, only peer address validation should be
taken into account to activate Retry protection.

This patch chooses to reduce the scope of half_open_conn. Now only
connection waiting to validate the peer address are now accounted for.
Most notably, connections instantiated with a validated Retry token
check are not accounted.

One impact of this patch is that it should prevent to activate Retry
mechanism too early, in particular in case if multiple handshakes are
too slow. Another limitation should be implemented to protect against
this scenario.

diff --git a/include/haproxy/quic_stats-t.h b/include/haproxy/quic_stats-t.h
index 59032756194623d5521c29817910d4cb3d63bbeb..1ee626590b4fc16a856212a6fbd1c5b22fec7640 100644 (file)
--- a/include/haproxy/quic_stats-t.h
+++ b/include/haproxy/quic_stats-t.h
@@ -69,7 +69,7 @@ struct quic_counters {
        long long retry_sent;        /* total number of Retry sent */
        long long retry_validated;   /* total number of validated Retry tokens */
        long long retry_error;       /* total number of Retry token errors */
-       long long half_open_conn;    /* total number of half open connections */
+       long long half_open_conn;    /* current number of connections waiting for address validation */
        long long hdshk_fail;        /* total number of handshake failures */
        long long stateless_reset_sent; /* total number of handshake failures */
        /* Special events of interest */

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 03d8076df2900417d7c40e2e059c9a0bacf152f4..b21e6eb1eea5dd1619ae68a5d29c2d89590650d2 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -257,11 +257,6 @@ void quic_set_tls_alert(struct quic_conn *qc, int alert)
 {
        TRACE_ENTER(QUIC_EV_CONN_SSLALERT, qc);
 
-       if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
-               qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
-               TRACE_DEVEL("dec half open counter", QUIC_EV_CONN_SSLALERT, qc);
-               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
-       }
        quic_set_connection_close(qc, quic_err_tls(alert));
        qc->flags |= QUIC_FL_CONN_TLS_ALERT;
        TRACE_STATE("Alert set", QUIC_EV_CONN_SSLALERT, qc);
@@ -1387,6 +1382,9 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
                            QUIC_EV_CONN_INIT, qc);
                qc->flags |= QUIC_FL_CONN_PEER_VALIDATED_ADDR;
        }
+       else {
+               HA_ATOMIC_INC(&qc->prx_counters->half_open_conn);
+       }
 
        TRACE_LEAVE(QUIC_EV_CONN_INIT, qc);
 
@@ -1532,6 +1530,13 @@ void quic_conn_release(struct quic_conn *qc)
        quic_conn_prx_cntrs_update(qc);
        pool_free(pool_head_quic_conn_rxbuf, qc->rx.buf.area);
        qc->rx.buf.area = NULL;
+
+       /* Connection released before peer address validated. */
+       if (unlikely(!(qc->flags & QUIC_FL_CONN_PEER_VALIDATED_ADDR))) {
+               BUG_ON(!qc->prx_counters->half_open_conn);
+               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
+       }
+
        pool_free(pool_head_quic_conn, qc);
        qc = NULL;
 
@@ -1657,8 +1662,6 @@ void qc_idle_timer_rearm(struct quic_conn *qc, int read, int arm_ack)
 struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state)
 {
        struct quic_conn *qc = ctx;
-       struct quic_counters *prx_counters = qc->prx_counters;
-       unsigned int qc_flags = qc->flags;
 
        TRACE_ENTER(QUIC_EV_CONN_IDLE_TIMER, qc);
 
@@ -1706,12 +1709,6 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state)
         * least clean some parts of it such as the tasklet.
         */
 
-       if (!(qc_flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
-               qc_flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
-               TRACE_DEVEL("dec half open counter", QUIC_EV_CONN_IDLE_TIMER, qc);
-               HA_ATOMIC_DEC(&prx_counters->half_open_conn);
-       }
-
  requeue:
        TRACE_LEAVE(QUIC_EV_CONN_IDLE_TIMER, qc);
        return t;

diff --git a/src/quic_rx.c b/src/quic_rx.c
index 3ec02f22c39991d6b452019bd50d313070a0e5c6..c652c9465db6c2886fe72855905a6048bec4747d 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1123,10 +1123,6 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                        /* Increment the error counters */
                        qc_cc_err_count_inc(qc, &frm);
                        if (!(qc->flags & QUIC_FL_CONN_DRAINING)) {
-                               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_STATE("Entering draining state", QUIC_EV_CONN_PRSHPKT, qc);
                                /* RFC 9000 10.2. Immediate Close:
                                 * The closing and draining connection states exist to ensure
@@ -1385,10 +1381,13 @@ int qc_treat_rx_pkts(struct quic_conn *qc)
                                         * Handshake keys confirms that the peer successfully processed an
                                         * Initial packet.
                                         */
-                                       if (qel == qc->hel) {
+                                       if (qel == qc->hel &&
+                                           !(qc->flags & QUIC_FL_CONN_PEER_VALIDATED_ADDR)) {
                                                TRACE_STATE("validate peer address on handshake packet",
                                                            QUIC_EV_CONN_RXPKT, qc, pkt);
                                                qc->flags |= QUIC_FL_CONN_PEER_VALIDATED_ADDR;
+                                               BUG_ON(!qc->prx_counters->half_open_conn);
+                                               HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
                                        }
 
                                        /* Update the list of ranges to acknowledge. */
@@ -2035,8 +2034,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 
                        if (*new_tid != -1)
                                goto out;
-
-                       HA_ATOMIC_INC(&prx_counters->half_open_conn);
                }
        }
        else if (!qc) {

diff --git a/src/quic_ssl.c b/src/quic_ssl.c
index 085f4b8b325da3c04405fd467999a4ea2de07a80..a5b0900cb6fdd1364494e71aca4e8351b2d3e63f 100644 (file)
--- a/src/quic_ssl.c
+++ b/src/quic_ssl.c
@@ -549,13 +549,8 @@ int qc_ssl_provide_quic_data(struct ncbuf *ncbuf,
                                goto out;
                        }
 
-                       /* 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_ERROR("SSL handshake error", QUIC_EV_CONN_IO_CB, qc, &state, &ssl_err);
+                       HA_ATOMIC_INC(&qc->prx_counters->hdshk_fail);
                        qc_ssl_dump_errors(ctx->conn);
                        ERR_clear_error();
                        goto leave;
@@ -570,11 +565,6 @@ int qc_ssl_provide_quic_data(struct ncbuf *ncbuf,
                        goto leave;
                }
 
-               if (!(qc->flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) {
-                       TRACE_DEVEL("dec half open counter", QUIC_EV_CONN_IO_CB, qc, &state);
-                       qc->flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED;
-                       HA_ATOMIC_DEC(&qc->prx_counters->half_open_conn);
-               }
                /* I/O callback switch */
                qc->wait_event.tasklet->process = quic_conn_app_io_cb;
                if (qc_is_listener(ctx->qc)) {