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.
This commit is contained in:
Amaury Denoyelle 2023-11-28 16:01:38 +01:00
parent 74ba22b1ee
commit 263f4e3d9c

107
src/h3.c
View File

@ -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>. /* 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 * Returns 0 if frame valid, otherwise HTTP/3 error code.
* be reported.
*/ */
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; struct h3s *h3s = qcs->ctx;
int ret = 0;
/* Stream type must be known to ensure frame is valid for this stream. */ /* Stream type must be known to ensure frame is valid for this stream. */
BUG_ON(h3s->type == H3S_T_UNKNOWN); BUG_ON(h3s->type == H3S_T_UNKNOWN);
switch (ftype) { switch (ftype) {
case H3_FT_DATA: case H3_FT_DATA:
return h3s->type != H3S_T_CTRL && (h3s->st_req == H3S_ST_REQ_HEADERS || /* cf H3_FT_HEADERS case. */
h3s->st_req == H3S_ST_REQ_DATA); 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: 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_CANCEL_PUSH:
case H3_FT_GOAWAY: case H3_FT_GOAWAY:
case H3_FT_MAX_PUSH_ID: case H3_FT_MAX_PUSH_ID:
/* Only allowed for control stream. First frame of control /* RFC 9114 7.2.3. CANCEL_PUSH
* stream MUST be SETTINGS. *
* 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: case H3_FT_SETTINGS:
/* draft-ietf-quic-http34 7.2.4. SETTINGS /* RFC 9114 7.2.4. SETTINGS
* *
* If an endpoint receives a second SETTINGS frame on the control * 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.
*
* 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 * stream, the endpoint MUST respond with a connection error of type
* H3_FRAME_UNEXPECTED. * H3_FRAME_UNEXPECTED.
*/ */
return h3s->type == H3S_T_CTRL && if (h3s->type != H3S_T_CTRL || h3c->flags & H3_CF_SETTINGS_RECV)
!(h3c->flags & H3_CF_SETTINGS_RECV); ret = H3_FRAME_UNEXPECTED;
break;
case H3_FT_PUSH_PROMISE: case H3_FT_PUSH_PROMISE:
/* RFC 9114 7.2.5. PUSH_PROMISE /* RFC 9114 7.2.5. PUSH_PROMISE
*
* A client MUST NOT send a PUSH_PROMISE frame. A server MUST treat the * 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 * receipt of a PUSH_PROMISE frame as a connection error of type
* H3_FRAME_UNEXPECTED. * H3_FRAME_UNEXPECTED.
*/ */
/* TODO server-side only. */ /* TODO server-side only. */
return 0; ret = H3_FRAME_UNEXPECTED;
break;
default: default:
/* draft-ietf-quic-http34 9. Extensions to HTTP/3 /* RFC 9114 9. Extensions to HTTP/3
* *
* Implementations MUST discard frames [...] that have unknown * Implementations MUST ignore unknown or unsupported values in all
* or unsupported types. * 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 /* 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; 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); 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; goto err;
} }