RE: Connection-level rejection of oversized DATA frames?

You're right, the server should be able to send that GOAWAY frame -- and already can, with the current draft text.  Note that this text specifies in which cases the FRAME_SIZE_ERROR MUST be a connection error, but does *not* say that other cases MUST NOT be connection errors or MUST be stream errors.  In the other circumstances, implementations are free to choose whether to kill just the stream or the entire connection in response to a violation.

See 5.4.1:
> An endpoint can end a connection at any time. In particular, an endpoint MAY choose to treat a stream error as a connection error. Endpoints SHOULD send a GOAWAY frame when ending a connection, providing that circumstances permit it.

-----Original Message-----
From: Jonathan Thackray [mailto:jthackray@gmail.com] On Behalf Of Jonathan Thackray
Sent: Monday, January 26, 2015 2:23 PM
To: ietf-http-wg@w3.org
Subject: Connection-level rejection of oversized DATA frames?

Hi all,

At the moment, section 4.2 of draft-ietf-httpbis-http2-16 says:

   An endpoint MUST send a FRAME_SIZE_ERROR error if a frame exceeds the
   size defined in SETTINGS_MAX_FRAME_SIZE, any limit defined for the
   frame type, or it is too small to contain mandatory frame data.  A
   frame size error in a frame that could alter the state of the entire
   connection MUST be treated as a connection error (Section 5.4.1);
   this includes any frame carrying a header block (Section 4.3) (that
   is, HEADERS, PUSH_PROMISE, and CONTINUATION), SETTINGS, and any frame
   with a stream identifier of 0.

I read this to mean that if the server sets the maximum frame size it will accept to 256KB, and a client sends a DATA frame with a length of 16MB (defined in the header) followed by 16MB of client data, the server has no choice but to read the data, as the error is at the stream-level (i.e. RST_STREAM), as the second sentence in that paragraph doesn’t apply.

The draft text doesn’t allow rejecting this frame early without reading _ALL_ the data, which is a waste of bandwidth. I think the server should be able to send a GOAWAY connection error of type FRAME_SIZE_ERROR, followed by closing the TCP/TLS socket. With the current draft spec, the only way to handle this now is to send a RST_STREAM of type FRAME_SIZE_ERROR and then send a GOAWAY( NO_ERROR ) to avoid reading the large amount of client data following, but this is more ambiguous.

Is there any reason why we couldn't add the following text to the bottom
of section 6.1 which says:    (could be either MAY or MUST)

   Receipt of a DATA frame with a length greater than the receiver's
   SETTINGS_MAX_FRAME_SIZE setting MAY be treated as a connection error
   (Section 5.4.1) of type FRAME_SIZE_ERROR.

Or alternatively, we could define all frame size errors to be connection errors, which would simplify the spec and implementation logic. Given that this is a new protocol, this would help avoid poor implementations. Frame size errors should only happen due to poor programming or fuzzing/cracking attempts.

We could then remove this entire paragraph from section 4.2:

   A frame size error in a frame that could alter the state of the entire
   connection MUST be treated as a connection error (Section 5.4.1);
   this includes any frame carrying a header block (Section 4.3) (that
   is, HEADERS, PUSH_PROMISE, and CONTINUATION), SETTINGS, and any frame
   with a stream identifier of 0.

and replace it with:

   A frame size error in a frame MUST be treated as a connection error
   (Section 5.4.1).

The only stream-level FRAME_SIZE_ERROR defined in the specification appears to be:

   A PRIORITY frame with a length other than 5 octets MUST be treated as
   a stream error (Section 5.4.2) of type FRAME_SIZE_ERROR.

What do others think? I appreciate this is "last-minute", but I think we should sort out this nit -- the smaller change of just adding the extra text for the DATA frame would be sufficient for me, but I’d prefer it if we could tidy up and make the text and specification clearer. Happy to prepare a PR of revised text for this.

Thanks,
Jonathan.

Received on Monday, 26 January 2015 22:34:39 UTC