MINOR: h3: use correct error code for missing SETTINGS

Each received HTTP/3 frame is checked to ensure it is valid given the
type of stream and its current status. This was implemented via
h3_is_frame_valid().

Previously, no distinction was made for error code, so every failure
triggered a CONNECTION_CLOSE_APP with code H3_FRAME_UNEXPECTED. However,
this function also ensures that the first frame received on control
frame is of type SETTINGS. If not, the error code to use is
H3_MISSING_SETTINGS.

To support this, adjust the function prototype. Instead of returning a
boolean, 0 is returned for success, or a HTTP/3 error code. The function
is renamed h3_check_frame_valid() to reflects the return type change.

This is not considered as a bug as previously the connection was
correctly closed on a missing SETTINGS, albeit with a non conform error
code. It's not deemed as sufficient to be backported.

diff --git a/src/h3.c b/src/h3.c
index a51eafb6af8e7be06e1e4650bebe6ed14368c998..c849bde1373f6f7e047c9b510516a830d2848ee3 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -301,61 +301,126 @@ static inline size_t h3_decode_frm_header(uint64_t *ftype, uint64_t *flen,
 
 /* Check if H3 frame of type <ftype> is valid when received on stream <qcs>.
  *
- * Returns a boolean. If false, a connection error H3_FRAME_UNEXPECTED should
- * be reported.
+ * Returns 0 if frame valid, otherwise HTTP/3 error code.
  */
-static int h3_is_frame_valid(struct h3c *h3c, struct qcs *qcs, uint64_t ftype)
+static int h3_check_frame_valid(struct h3c *h3c, struct qcs *qcs, uint64_t ftype)
 {
        struct h3s *h3s = qcs->ctx;
+       int ret = 0;
 
        /* Stream type must be known to ensure frame is valid for this stream. */
        BUG_ON(h3s->type == H3S_T_UNKNOWN);
 
        switch (ftype) {
        case H3_FT_DATA:
-               return h3s->type != H3S_T_CTRL && (h3s->st_req == H3S_ST_REQ_HEADERS ||
-                                                  h3s->st_req == H3S_ST_REQ_DATA);
+               /* cf H3_FT_HEADERS case. */
+               if (h3s->type == H3S_T_CTRL ||
+                   (h3s->st_req != H3S_ST_REQ_HEADERS && h3s->st_req != H3S_ST_REQ_DATA)) {
+                       ret = H3_FRAME_UNEXPECTED;
+               }
+
+               break;
 
        case H3_FT_HEADERS:
-               return h3s->type != H3S_T_CTRL && h3s->st_req != H3S_ST_REQ_TRAILERS;
+               /* RFC 9114 4.1. HTTP Message Framing
+                *
+                *
+                * An HTTP message (request or response) consists of:
+                * 1. the header section, including message control data, sent as a
+                *    single HEADERS frame,
+                * 2. optionally, the content, if present, sent as a series of DATA
+                *    frames, and
+                * 3. optionally, the trailer section, if present, sent as a single
+                *    HEADERS frame.
+                *
+                * [...]
+                *
+                *  Receipt of an invalid sequence of frames MUST be treated as a
+                *  connection error of type H3_FRAME_UNEXPECTED. In particular, a DATA
+                *  frame before any HEADERS frame, or a HEADERS or DATA frame after the
+                *  trailing HEADERS frame, is considered invalid. Other frame types,
+                *  especially unknown frame types, might be permitted subject to their
+                *  own rules; see Section 9.
+                */
+               if (h3s->type == H3S_T_CTRL || h3s->st_req == H3S_ST_REQ_TRAILERS)
+                       ret = H3_FRAME_UNEXPECTED;
+               break;
 
        case H3_FT_CANCEL_PUSH:
        case H3_FT_GOAWAY:
        case H3_FT_MAX_PUSH_ID:
-               /* Only allowed for control stream. First frame of control
-                * stream MUST be SETTINGS.
+               /* RFC 9114 7.2.3. CANCEL_PUSH
+                *
+                * A CANCEL_PUSH frame is sent on the control stream. Receiving a
+                * CANCEL_PUSH frame on a stream other than the control stream MUST be
+                * treated as a connection error of type H3_FRAME_UNEXPECTED.
                 */
-               return h3s->type == H3S_T_CTRL &&
-                      (h3c->flags & H3_CF_SETTINGS_RECV);
+
+               /* RFC 9114 7.2.6. GOAWAY
+                *
+                * A client MUST treat a GOAWAY frame on a stream other than the
+                * control stream as a connection error of type H3_FRAME_UNEXPECTED.
+                */
+
+               /* RFC 9114 7.2.7. MAX_PUSH_ID
+                *
+                * The MAX_PUSH_ID frame is always sent on the control stream. Receipt
+                * of a MAX_PUSH_ID frame on any other stream MUST be treated as a
+                * connection error of type H3_FRAME_UNEXPECTED.
+                */
+
+               if (h3s->type != H3S_T_CTRL)
+                       ret = H3_FRAME_UNEXPECTED;
+               else if (!(h3c->flags & H3_CF_SETTINGS_RECV))
+                       ret = H3_MISSING_SETTINGS;
+               break;
 
        case H3_FT_SETTINGS:
-               /* draft-ietf-quic-http34 7.2.4. SETTINGS
+               /* RFC 9114 7.2.4. SETTINGS
+                *
+                * A SETTINGS frame MUST be sent as the first frame of
+                * each control stream (see Section 6.2.1) by each peer, and it MUST NOT
+                * be sent subsequently. If an endpoint receives a second SETTINGS frame
+                * on the control stream, the endpoint MUST respond with a connection
+                * error of type H3_FRAME_UNEXPECTED.
                 *
-                * If an endpoint receives a second SETTINGS frame on the control
+                * SETTINGS frames MUST NOT be sent on any stream other than the control
+                * stream. If an endpoint receives a SETTINGS frame on a different
                 * stream, the endpoint MUST respond with a connection error of type
                 * H3_FRAME_UNEXPECTED.
                 */
-               return h3s->type == H3S_T_CTRL &&
-                      !(h3c->flags & H3_CF_SETTINGS_RECV);
+               if (h3s->type != H3S_T_CTRL || h3c->flags & H3_CF_SETTINGS_RECV)
+                       ret = H3_FRAME_UNEXPECTED;
+               break;
 
        case H3_FT_PUSH_PROMISE:
                /* RFC 9114 7.2.5. PUSH_PROMISE
+                *
                 * A client MUST NOT send a PUSH_PROMISE frame. A server MUST treat the
                 * receipt of a PUSH_PROMISE frame as a connection error of type
                 * H3_FRAME_UNEXPECTED.
                 */
 
                /* TODO server-side only. */
-               return 0;
+               ret = H3_FRAME_UNEXPECTED;
+               break;
 
        default:
-               /* draft-ietf-quic-http34 9. Extensions to HTTP/3
+               /* RFC 9114 9. Extensions to HTTP/3
                 *
-                * Implementations MUST discard frames [...] that have unknown
-                * or unsupported types.
+                * Implementations MUST ignore unknown or unsupported values in all
+                * extensible protocol elements. [...]
+                * However, where a known frame type is required to be in a
+                * specific location, such as the SETTINGS frame as the first frame of
+                * the control stream (see Section 6.2.1), an unknown frame type does
+                * not satisfy that requirement and SHOULD be treated as an error.
                 */
-               return h3s->type != H3S_T_CTRL || (h3c->flags & H3_CF_SETTINGS_RECV);
+               if (h3s->type == H3S_T_CTRL && !(h3c->flags & H3_CF_SETTINGS_RECV))
+                       ret = H3_MISSING_SETTINGS;
+               break;
        }
+
+       return ret;
 }
 
 /* Check from stream <qcs> that length of all DATA frames does not exceed with
@@ -1235,9 +1300,9 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
                                        break;
                        }
 
-                       if (!h3_is_frame_valid(h3c, qcs, ftype)) {
+                       if ((ret = h3_check_frame_valid(h3c, qcs, ftype))) {
                                TRACE_ERROR("received an invalid frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-                               qcc_set_error(qcs->qcc, H3_FRAME_UNEXPECTED, 1);
+                               qcc_set_error(qcs->qcc, ret, 1);
                                goto err;
                        }