MEDIUM: h3: enforce GOAWAY by resetting higher unhandled stream

When a GOAWAY has been emitted, an ID is announced to represent handled
streams. H3 RFC suggests that higher streams should be resetted with the
error code H3_REQUEST_CANCELLED. This allows the peer to replay requests
on another connection.

For the moment, the impact of this change is limitted as GOAWAY is only
used on connection shutdown just before the MUX is freed. However, for
soft-stop support, a GOAWAY can be emitted in anticipation while keeping
the MUX to finish the active streams. In this case, new streams opened
by the client are resetted.

As a consequence of this change, app_ops.attach() operation has been
delayed at the very end of qcs_new(). This ensure that all qcs members
are initialized to support RESET_STREAM sending.

This should be backported up to 2.7.

diff --git a/src/h3.c b/src/h3.c
index 1572ac1b6540ada4f53b820dff592d710add81b7..c912c0a15fed29c01e550dad239b815c85494f7e 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -114,6 +114,7 @@ INITCALL1(STG_REGISTER, trace_register_source, TRACE_SOURCE);
 #define H3_CF_UNI_CTRL_SET      0x00000004  /* Remote H3 Control stream opened */
 #define H3_CF_UNI_QPACK_DEC_SET 0x00000008  /* Remote QPACK decoder stream opened */
 #define H3_CF_UNI_QPACK_ENC_SET 0x00000010  /* Remote QPACK encoder stream opened */
+#define H3_CF_GOAWAY_SENT       0x00000020  /* GOAWAY sent on local control stream */
 
 /* Default settings */
 static uint64_t h3_settings_qpack_max_table_capacity = 0;
@@ -1661,13 +1662,38 @@ static int h3_close(struct qcs *qcs, enum qcc_app_ops_close_side side)
 
 static int h3_attach(struct qcs *qcs, void *conn_ctx)
 {
-       struct h3s *h3s;
+       struct h3c *h3c = conn_ctx;
+       struct h3s *h3s = NULL;
 
        TRACE_ENTER(H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
 
+       /* RFC 9114 5.2. Connection Shutdown
+        *
+        * Upon sending
+        * a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+        * Sections 4.1.1 and 7.2.3) any requests or pushes that have
+        * identifiers greater than or equal to the one indicated, in
+        * order to clean up transport state for the affected streams.
+        * The endpoint SHOULD continue to do so as more requests or
+        * pushes arrive.
+        */
+       if (h3c->flags & H3_CF_GOAWAY_SENT && qcs->id >= h3c->id_goaway &&
+           quic_stream_is_bidi(qcs->id)) {
+               /* Reject request and do not allocate a h3s context.
+                * TODO support push uni-stream rejection.
+                */
+               TRACE_STATE("reject stream higher than goaway", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+               qcc_abort_stream_read(qcs);
+               qcc_reset_stream(qcs, H3_REQUEST_REJECTED);
+               goto done;
+       }
+
        h3s = pool_alloc(pool_head_h3s);
-       if (!h3s)
-               return 1;
+       if (!h3s) {
+               TRACE_ERROR("h3s allocation failure", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+               qcc_emit_cc_app(qcs->qcc, H3_INTERNAL_ERROR, 1);
+               goto err;
+       }
 
        qcs->ctx = h3s;
        h3s->h3c = conn_ctx;
@@ -1689,8 +1715,13 @@ static int h3_attach(struct qcs *qcs, void *conn_ctx)
                h3s->type = H3S_T_UNKNOWN;
        }
 
+ done:
        TRACE_LEAVE(H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
        return 0;
+
+ err:
+       TRACE_DEVEL("leaving in error", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+       return 1;
 }
 
 static void h3_detach(struct qcs *qcs)
@@ -1758,10 +1789,15 @@ static int h3_send_goaway(struct h3c *h3c)
        b_force_xfer(res, &pos, b_data(&pos));
        qcc_send_stream(qcs, 1);
 
+       h3c->flags |= H3_CF_GOAWAY_SENT;
        TRACE_LEAVE(H3_EV_H3C_END, h3c->qcc->conn);
        return 0;
 
  err:
+       /* Consider GOAWAY as sent even if not really the case. This will
+        * block future stream opening using H3_REQUEST_REJECTED reset.
+        */
+       h3c->flags |= H3_CF_GOAWAY_SENT;
        TRACE_DEVEL("leaving in error", H3_EV_H3C_END, h3c->qcc->conn);
        return 1;
 }

diff --git a/src/mux_quic.c b/src/mux_quic.c
index d68abbeee3971c775ad4d80534b58c53d05e94ac..8577debd3f96a5c8942944f6c6cac001e5239a31 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -135,13 +135,6 @@ static struct qcs *qcs_new(struct qcc *qcc, uint64_t id, enum qcs_type type)
                }
        }
 
-       if (qcc->app_ops->attach) {
-               if (qcc->app_ops->attach(qcs, qcc->ctx)) {
-                       TRACE_ERROR("app proto failure", QMUX_EV_QCS_NEW, qcc->conn, qcs);
-                       goto err;
-               }
-       }
-
        /* If stream is local, use peer remote-limit, or else the opposite. */
        if (quic_stream_is_bidi(id)) {
                qcs->tx.msd = quic_stream_is_local(qcc, id) ? qcc->rfctl.msd_bidi_r :
@@ -174,6 +167,11 @@ static struct qcs *qcs_new(struct qcc *qcc, uint64_t id, enum qcs_type type)
 
        qcs->err = 0;
 
+       if (qcc->app_ops->attach && qcc->app_ops->attach(qcs, qcc->ctx)) {
+               TRACE_ERROR("app proto failure", QMUX_EV_QCS_NEW, qcc->conn, qcs);
+               goto err;
+       }
+
  out:
        TRACE_LEAVE(QMUX_EV_QCS_NEW, qcc->conn, qcs);
        return qcs;