Re: Can servers generate responses to malformed requests in h2?

Agree with Cory and Lucas here.

- Sending a HTTP response is preferable, since this gives much better error reporting on the client side.
- Violations of HTTP/2 framing layer should error on the h2 layer. Basically, the server no longer trusts the client.
- Sending a RESPONSE with subsequent RST is a good pattern

Apache httpd prefers to send a response on error. And if it receives an client frames on that stream after that, it sends a RST.

Cheers,
Stefan

> Am 25.09.2023 um 09:33 schrieb Cory Benfield <cory@lukasa.co.uk>:
> 
> For what it's worth, there is a reading of the specification that
> holds all of these statements as true, and not in conflict.
> 
> Again, quoting from Martin's summary at the top:
> 
>> Intermediaries that process HTTP requests or responses (i.e., any intermediary not acting as a tunnel) MUST NOT forward a malformed request or response. Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.
>> 
>> For malformed requests, a server MAY send an HTTP response prior to closing or resetting the stream. Clients MUST NOT accept a malformed response.
> 
> For servers, this allows the following series of actions on a malformed request:
> 
> 1. Send HEADERS containing 400 Bad Request with END_STREAM set.
> 2. Follow the frame immediately with RST_STREAM with PROTOCOL_ERROR.
> 
> The protocol action of this is clear: you are terminating your end of
> the stream with END_STREAM, and then sending RST_STREAM to prevent any
> further frame being received.
> 
> I should also note that ยง 5.4.2 (Stream Error Handling) doesn't
> actually contain normative language around the RST_STREAM frame. It
> reads:
> 
>> An endpoint that detects a stream error sends a RST_STREAM frame (Section 6.4) that contains the stream identifier of the stream where the error occurred.
> 
> Note that is not "MUST send", it's just "sends". If the RST_STREAM
> frame is not necessary, one can argue that the spec allows you to omit
> it. But in either case, you can choose to both send a response _and_
> send RST_STREAM. Sending RST_STREAM is not forbidden on closed
> streams.
> 
> Otherwise I agree with Martin's analysis here.
> 
> On Mon, 25 Sept 2023 at 06:59, Martin Thomson <mt@lowentropy.net> wrote:
>> 
>> On Mon, Sep 25, 2023, at 13:33, Glenn Strauss wrote:
>>> My understanding is that I could treat all HTTP request header errors
>>> received as HTTP/2 requests as h2 stream error PROTOCOL_ERROR.  If that
>>> is what is desired by the RFC authors, please issue an update or errata
>>> and I'll change that code in lighttpd.
>> 
>> That was never really the intent.  The goal of HTTP/2 was to limit stream errors to those cases where errors are found in HTTP/2-specific stuff (frame format and ordering mainly).
>> 
>> This encroaches into "headers" in only two ways: pseudo-fields and Content-Length.  Control data and length delimiting are functions in HTTP/1.1 that are not so cleanly separate from the rest of the protocol.  Those are considered part of HTTP/2 proper and not HTTP in general.
>> 
>> For Content-Length in particular, we originally decided that this was necessary because of the way in which the length of requests and responses was a function that HTTP/2 provided and we wanted to ensure that there was no way for a Content-Length field to create an alternative narrative.  The believe was that that could be used for request-smuggling in some cases.
>> 
>> So our response was to make these stream errors.  Perhaps we should have allowed either type of error in the case of Content-Length, but that was the decision at the time.
>> 
>>> However, I currently am unable to understand why h2 PROTOCOL_ERROR
>>> (or H3_GENERAL_PROTOCOL_ERROR) is somehow better than 400 Bad Request.
>> 
>> I think that your "another reason" below might point to why this is preferable.  A client that makes a request via a proxy that upgrades it to HTTP/2 or HTTP/3 is not responsible for errors that arise in translation to the target request format, so it is in no position to correct that error.  The error is something that the proxy caused and so the proxy needs to handle it.
>> 
>> Maybe that's no real help to a client who is dealing with a busted proxy and who is presented with a request that times out or whatever the proxy does in that case.  But it's about ensuring that the error is appropriately targeted.  If the proxy isn't implementing the protocol properly, then the developers of that proxy really do need to pay attention.  Protocol errors like this should not occur during normal operation.
>> 
>> Yes, PROTOCOL_ERROR isn't exactly the nicest error (which is partly why closing the connection might be preferable, because you get to include a reason phrase), but that only suggests that we might want to explore ways of making errors clearer or more actionable.
>> 
>>> I can see where h2 specification around pseudo-headers is specific to
>>> the h2 protocol.  However, an implementation "could" HPACK-decode the
>>> entire HEADERS frame into a single string that looks almost identical
>>> to HTTP/1.1 request headers with the exception of the addition of
>>> pseudo-headers.  It could then be loosely parsed as HTTP/1.x request
>>> headers.
>> 
>> If you are rewriting HTTP/2 into HTTP/1.x, you need all the pseudo-fields in order to make sense of the message.  If one is missing, then your converter can generate an error (which you can send as a stream error).  Same if a pseudo-field arrives late.
>> 
>>> An implementation might need to fully parse the HEADERS frame
>>> before being able to determine that a required pseudo-header is missing.
>>> If it has already gotten this far parsing the HTTP request, why should
>>> the RFC disallow the implementation from returning an HTTP error code?
>> 
>> The specification requires that pseudo-fields precede real fields, so this is not really an option.
>> 
>

Received on Monday, 25 September 2023 09:24:32 UTC