BUG/MEDIUM: quic: handle ECONNREFUSED on RX side

Unlike the detection performed during sendto() for an unreachable peer,
ECONNREFUSED was not handled when received via recvmsg() as an ICMP
"host unreachable" message.

This patch tracks ECONNREFUSED errors on the receive path.

Note that this detection is entirely dependent on the remote host effectively
sending an ICMP "host unreachable" message and on the absence of any network
filtering (e.g., firewalls) that would drop such ICMP packets. Without
receiving this ICMP signal, the connection state cannot be updated through
this mechanism.

At a higher level, similar to how this error is handled on sendto(),
the connection is now terminated as soon as possible by calling
qc_kill_conn(). This triggers a call to qc_notify_err(). When the mux
does not exist, it attempts to create one via conn_create_mux(). While
the latter systematically fails if the connection is flagged with
CO_FL_ERROR, it has the useful side effect of waking the stconn stream
attached to the connection during a session opening without a mux
(e.g., for H3).

This issue was caught by haload (upcoming tool).

Must be backported as far as 2.6 because it impacts both the QUIC
frontends and backends.

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 26c67c7d418bdec654076b3932419f9d8e552ad1..ea198e84cb16935bc66d6423999c6ee6cd512a17 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -566,8 +566,11 @@ struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned int sta
        TRACE_ENTER(QUIC_EV_CONN_IO_CB, qc);
        TRACE_STATE("connection handshake state", QUIC_EV_CONN_IO_CB, qc, &qc->state);
 
-       if (qc_test_fd(qc))
-               qc_rcv_buf(qc);
+       if (qc_test_fd(qc) && qc_rcv_buf(qc) < 0) {
+               TRACE_ERROR("recvmsg fatal error", QUIC_EV_CONN_SPPKTS, qc);
+               qc_kill_conn(qc);
+               goto no_rx_pkts;
+       }
 
        /* Prepare post-handshake frames
         * - after connection is instantiated (accept is done)
@@ -591,6 +594,7 @@ struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned int sta
                goto out;
        }
 
+ no_rx_pkts:
        if (qc->flags & QUIC_FL_CONN_TO_KILL) {
                TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_IO_CB, qc);
                goto out;
@@ -657,8 +661,10 @@ static struct task *quic_conn_closed_io_cb(struct task *t, void *context, unsign
 
        TRACE_ENTER(QUIC_EV_CONN_IO_CB, qc);
 
-       if (qc_test_fd(qc))
-               qc_rcv_buf(qc);
+       if (qc_test_fd(qc) && qc_rcv_buf(qc) < 0) {
+               TRACE_ERROR("recvmsg fatal error", QUIC_EV_CONN_IO_CB, qc);
+               goto fatal_error;
+       }
 
        /* Do not send too much data if the peer address was not validated. */
        if ((qc->flags & QUIC_FL_CONN_IMMEDIATE_CLOSE) &&
@@ -670,11 +676,7 @@ static struct task *quic_conn_closed_io_cb(struct task *t, void *context, unsign
                     QUIC_MAX_CC_BUFSIZE - headlen, 0, cc_qc->cc_dgram_len);
        if (qc_snd_buf(qc, &buf, buf.data, 0, 0) < 0) {
                TRACE_ERROR("sendto fatal error", QUIC_EV_CONN_IO_CB, qc);
-               quic_release_cc_conn(cc_qc);
-               cc_qc = NULL;
-               qc = NULL;
-               t = NULL;
-               goto leave;
+               goto fatal_error;
        }
 
        qc->flags &= ~QUIC_FL_CONN_IMMEDIATE_CLOSE;
@@ -683,6 +685,13 @@ static struct task *quic_conn_closed_io_cb(struct task *t, void *context, unsign
        TRACE_LEAVE(QUIC_EV_CONN_IO_CB, qc);
 
        return t;
+
+ fatal_error:
+       quic_release_cc_conn(cc_qc);
+       cc_qc = NULL;
+       qc = NULL;
+       t = NULL;
+       goto leave;
 }
 
 /* The task handling the idle timeout of a connection in "connection close" state */
@@ -802,8 +811,16 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
                        goto out;
        }
 
-       if (qc_test_fd(qc))
-               qc_rcv_buf(qc);
+       if (qc_test_fd(qc) && qc_rcv_buf(qc) < 0) {
+               TRACE_ERROR("recvmsg fatal error", QUIC_EV_CONN_SPPKTS, qc);
+               qc_kill_conn(qc);
+               goto out;
+       }
+
+       if (qc->flags & QUIC_FL_CONN_TO_KILL) {
+               TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_PHPKTS, qc);
+               goto out;
+       }
 
        if (!qc_treat_rx_pkts(qc))
                goto out;
@@ -1945,6 +1962,14 @@ void qc_notify_err(struct quic_conn *qc)
                 */
                tasklet_wakeup(qc->qcc->wait_event.tasklet);
        }
+       else if (qc->conn) {
+               qc->conn->flags |= CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH;
+               /* Note: this creation will failed, but the upper layer will be informed
+                * about this connection errors.
+                */
+               if (conn_create_mux(qc->conn, NULL) < 0)
+                       TRACE_ERROR("mux creation failed", QUIC_EV_CONN_IO_CB, qc);
+       }
 
        TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
 }

diff --git a/src/quic_sock.c b/src/quic_sock.c
index 38210b2a851b82a4588cf8db27cf396b4b8580be..b4fe318b55419dd39a3b6ef4feafd3416025f51d 100644 (file)
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -892,9 +892,17 @@ int qc_rcv_buf(struct quic_conn *qc)
                                (struct sockaddr *)&daddr, sizeof(daddr),
                                get_net_port(&qc->local_addr), !!l);
                if (ret <= 0) {
+                       /* Only negative errors are fatal */
+                       ret = 0;
                        /* Subscribe FD for future reception. */
-                       if (errno == EAGAIN || errno == EWOULDBLOCK || errno == ENOTCONN)
+                       if (errno == EAGAIN || errno == EWOULDBLOCK || errno == ENOTCONN) {
                                fd_want_recv(qc->fd);
+                       }
+                       else if (errno == ECONNREFUSED) {
+                               TRACE_PRINTF(TRACE_LEVEL_USER, QUIC_EV_CONN_RCV, qc, 0, 0, 0,
+                                            "UDP recv failure errno=%d (%s)", errno, strerror(errno));
+                               ret = -errno;
+                       }
                        /* TODO handle other error codes as fatal on the connection. */
                        break;
                }