BUG/MEDIUM: h3: handle STOP_SENDING on control stream

Before this patch, STOP_SENDING reception was considered valid even on
H3 control stream. This causes the emission in return of RESET_STREAM
and eventually the closure and freeing of the QCS instance. This then
causes a crash during connection closure as a GOAWAY frame is emitted on
the control stream which is now released.

To fix this crash, STOP_SENDING on the control stream is now properly
rejected as specified by RFC 9114. The new app_ops close callback is
used which in turn will generate a CONNECTION_CLOSE with error
H3_CLOSED_CRITICAL_STREAM.

This bug was detected in github issue #2006. Note that however it is
triggered by an incorrect client behavior. It may be useful to determine
which client behaves like this. If this case is too frequent,
STOP_SENDING should probably be silently ignored.

To reproduce this issue, quiche was patched to emit a STOP_SENDING on
its send() function in quiche/src/lib.rs:
     pub fn send(&mut self, out: &mut [u8]) -> Result<(usize, SendInfo)> {
-        self.send_on_path(out, None, None)
+        let ret = self.send_on_path(out, None, None);
+        self.streams.mark_stopped(3, true, 0);
+        ret
     }

This must be backported up to 2.6 along with the preceeding commit :
  MINOR: mux-quic/h3: define close callback

diff --git a/src/h3.c b/src/h3.c
index e802d299cc8199453909b1e7e8d690f1e983563b..ea512a40432d7dafabaf9dbda7eb7a33d8085b8f 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1615,7 +1615,23 @@ static size_t h3_snd_buf(struct qcs *qcs, struct htx *htx, size_t count)
  */
 static int h3_close(struct qcs *qcs, enum qcc_app_ops_close_side side)
 {
-       /* TODO */
+       struct h3s *h3s = qcs->ctx;
+       struct h3c *h3c = h3s->h3c;;
+
+       /* RFC 9114 6.2.1. Control Streams
+        *
+        * The sender
+        * MUST NOT close the control stream, and the receiver MUST NOT
+        * request that the sender close the control stream.  If either
+        * control stream is closed at any point, this MUST be treated
+        * as a connection error of type H3_CLOSED_CRITICAL_STREAM.
+        */
+       if (qcs == h3c->ctrl_strm) {
+               TRACE_ERROR("closure detected on control stream", H3_EV_H3S_END, qcs->qcc, qcs);
+               qcc_emit_cc_app(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
+               return 1;
+       }
+
        return 0;
 }
 

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 401ccc0c10ff0385cc046338679f35ff23999dcb..cae22df1ffbb51b405bdfebc14ea1145f093d8c7 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1232,6 +1232,13 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err)
 
        qcs_idle_open(qcs);
 
+       if (qcc->app_ops->close) {
+               if (qcc->app_ops->close(qcs, QCC_APP_OPS_CLOSE_SIDE_WR)) {
+                       TRACE_ERROR("closure rejected by app layer", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
+                       goto out;
+               }
+       }
+
        /* RFC 9000 3.5. Solicited State Transitions
         *
         * An endpoint that receives a STOP_SENDING frame