quicwg / base-drafts

Internet-Drafts that make up the base QUIC specification
https://quicwg.org
1.63k stars 205 forks source link

Forwarding upstream errors, and the implications #3300

Closed kazuho closed 4 years ago

kazuho commented 4 years ago

In HTTP/2, it has been easy for a proxy to forward upstream errors to client.

When the upstream abruptly closes a H1 connection (i.e., closed before serving the correct number of bytes as suggested by the content-length header, or before trailers when chunked encoding is used), the HTTP/2 terminator could forward all the content that it received from upstream, then send an RST_STREAM frame. Because HTTP/2 uses TCP, which guarantees the transmission order, the client receives the bytes that the origin has sent, before receiving a reset.

In contrast, HTTP/3 does not have such guarantee. When a HTTP/3 proxy resets a request stream mid-response, the QUIC stack is allowed to cease transmitting data. Depending on when the HTTP/3 proxy receives input from upstream, and depending on how the responses are prioritized, there is fair chance that the HTTP/3 proxy would be resetting the request stream with HTTP_REQUEST_CANCELED, without sending any response at all.

Are we fine with this behavior change?

We could argue that the root cause is the upstream failing to send the correct response. But the change of the failure mode might be an issue, because it would worsen the user-experience when the upstream misbehaves. For example, when an upstream server abruptly terminates the transfer of a JPEG image, a user connected through an HTTP/2 proxy would receive a partial image, whereas a user connected through an HTTP/3 proxy is more likely to see nothing (due to the reasons stated above).

Related to: #2426

kazuho commented 4 years ago

PS. Note that HTTP expects intermediaries to forward these errors. If an intermediary erases this error, downstream might cache and reuse the corrupt response.

ianswett commented 4 years ago

If the TCP connection is closed with a reset, there's no guarantee how many bytes the intermediary has read from the kernel buffer before the reset is received, so I don't think the behavior is likely to be that different.

If there's an error at the HTTP layer, then I think there would commonly be a change in what the client sees, but that seems like a buggy backend, so I'm not sure it's worth attempting to deal with more gracefully?

janaiyengar commented 4 years ago

@ianswett's point is right: there's no guarantee what the proxy has read out of a connection that has received a TCP RST. Additionally, there's no guarantee that the H1 sender's pending data has been sent out of its TCP buffers yet either.

@kazuho: Do you think think this is somehow different?

kazuho commented 4 years ago

@ianswett @janaiyengar Thank you for your responses.

This is an HTTP issue, and it is NOT related to TCP resets.

HTTP/1.1 does not distinguish between TCP FIN and RST, but has it's own framing; see RFC 7230 section 3.3.3. HTTP/2 does not (need to) distinguish between the two, it relies on HTTP/2 frames to see if an HTTP message has been transferred completely.

My point is that a HTTP/1.1 or HTTP/2 intermediary acting as a "tunnel" (as defined in RFC 7230 section 2.3), is able to forward unexpected termination of an HTTP message that it received from upstream to downstream. In HTTP/1.1, what a tunnel can do is abruptly terminate a chunked encoding (by closing the connection normally). In HTTP/2, a tunnel sends a RST_STREAM.

In either case, the close signal never overtakes the partial payload.

I am not sure what others do, but at least H2O have been strict about forwarding partial payload and error close as they were received from upstream; see https://github.com/h2o/h2o/pull/1490. The intention is to provide the same response to the user even when a server running behind returns a corrupt response.

But in HTTP/3, because a stream reset can overtake the transmission of application data, it becomes very hard to act as a tunnel in such a way.

The worst case is as follows: consider the case where an origin always returns an HTTP response (containing an image) that terminates abruptly (at the HTTP/1.1 or HTTP/2 layer). Note that the delivery of partial, progressive JPEG image might not have any negative effects on user experience. Or the origin might in fact be returning an entire image, but failing to terminate the chunked encoding correctly.

When the connection between the tunnel and the client is HTTP/1.1 or HTTP/2, the tunnel can (and likely will) forward that partial response. But when the connection between the tunnel and the client is HTTP/3, it is very hard for the tunnel to deliver the partial response before the stream reset. Assuming that the partial response is of low priority, it is likely that the client would see nothing (rather than an image), as the stream reset signal would precede delivery of any stream data.

kazuho commented 4 years ago

One possible solution to this issue is to allow a tunnel to intentionally send a malformed response to downstream, rather than resetting the stream. I've created #3303 that specifies such behavior.

martinthomson commented 4 years ago

I don't think that this is a good response to this condition. It creates a situation where an intermediary purposefully violates the protocol. If the connection is broken, then let it be broken. The intermediary can forward precisely what it received, ensure that that data was received, then reset the stream if it truly cares about fidelity.

Note that the intermediary might also have to deal with gaps toward the tail of such a broken stream as well, and then they can't propagate that information with the same accuracy.

We shouldn't permit a condition that the recipient can't distinguish from badness. The result of a change like this is to force clients to accept this junk as something other than a connection-level error. That means we can't catch badness that looks similar to this. For this particular kind of behaviour, I would expect connection errors in most implementations.

I realize that HTTP/1.1 implementations do this all the time and that CDNs often have to patch up these situations, but this isn't patching it up, it's propagating the badness to the rest of the system. You are right the the badness is sometimes constrained. As you say, the trailing part of a progressive image often can be lost with no visible impact. This is an area where browsers might need to be tolerant of resets and retain at least what has already been displayed. You might want to be able to rely on that happening too, so maybe this is an area in which we pursue solutions in other areas (like Fetch).

kazuho commented 4 years ago

@martinthomson

I don't think that this is a good response to this condition. It creates a situation where an intermediary purposefully violates the protocol. If the connection is broken, then let it be broken. The intermediary can forward precisely what it received, ensure that that data was received, then reset the stream if it truly cares about fidelity.

The problem here is that it is complex to ensure that the data was received before transmitting a reset. That's something that the QUIC transport does not have a concept, and I'm also not sure if it can be implemented in the HTTP/3 stack, because it is uncertain if we can except QUIC stacks to expose a signal indicating if data has been acked.

We might argue that in-order delivery of stream resets was a feature of HTTP/2, and that we need it in HTTP/3. As you state, I think we'd better define a way to patch up these situations, otherwise people might suffer from worse user experience when using HTTP/3.

Am I correct in assuming that your distaste is against the solution proposed in #3303 rather than against this issue itself? If so, would you be more open to using a HTTP/3 frame for indicating abrupt termination?

martinthomson commented 4 years ago

Look, I recognize the difficulty here, but I don't think that this problem suggests anything other than a (maybe) a new stream reset error code. I realize that pushes for widening of the HTTP/3-to-transport interface, but I can't see an alternative that does not create horrible externalities for this use case. Important as it might be for CDNs and the web, I don't think that it is a good idea to have the rest of the system bear the cost of dealing with the problem.

I am very much opposed to the proposal in #3303 for previously-stated reasons.

Enabling in-order delivery of stream resets would be a massive disruption to the protocol, even if we only did it at the HTTP/3 layer. I don't concretely know how to do that, but it implies a significant shift (or bifurcation) in the stream model if we did it at the transport layer.

Adding in-order resets at the application layer for HTTP/3 is not trivial either. A frame on the same stream is not going to work because it prevents an intermediary from forwarding without reframing at that layer. Otherwise, they would not be able to guarantee that they can send any frame on the stream. That requires a big shift in how people approach building of intermediaries. Something on a control stream might work, but then you have the potential for ambiguity about placement relative to the target stream unless you effectively replicate the RESET_STREAM design at the application layer. That would make me very uncomfortable.

It seems to me that you can address this without changing the protocol, even if it requires some awkward crossing of protocol layers. That's my preference.

kazuho commented 4 years ago

@martinthomson Thank you for sharing your views. I am not sure if it is correct to frame this issue as specific to CDNs, I might argue that it is a problem for intermediaries in general.

I agree that we shouldn't try to introduce in-order delivery of stream resets at the transport layer. If we are to introduce in-order delivery, that should be done in HTTP/3.

Adding in-order resets at the application layer for HTTP/3 is not trivial either. A frame on the same stream is not going to work because it prevents an intermediary from forwarding without reframing at that layer. Otherwise, they would not be able to guarantee that they can send any frame on the stream. That requires a big shift in how people approach building of intermediaries.

I think this is what I disagree. I think that many if not most of the intermediaries to be doing reframing. In case of HTTP/2, intermediaries should have been doing reframing by itself, otherwise it cannot prioritize the responses, I'd assume that proxies would be doing the same for HTTP/3 (though I might be wrong).

If that assumption is correct, the fix would be as trivial as introducing a HTTP/3 frame indicating the error, which is sent on the request stream. It's ugly to have two ways of signaling error (one at QUIC layer and one at HTTP layer), but it'd help us provide same expectations across different versions of HTTP.

kazuho commented 4 years ago

PS. Or, if we are to consider H3 tunnels that do not do reframing as within the scope of the specification, I think we need to have something like #3303.

Consider the case of a H3-H3 intermediary that parses the request, and binds HTTP requests to different origins, transmitting the response as-is without dealing with frames. Such a tunnel would work as expected, assuming that QPACK dynamic table is not used.

In this type of design, a stream-level error (i.e. a stream closed normally while transmitting an incomplete H3 frame) does not indicate a connection-level error. Therefore, we need to state that.

RyanTheOptimist commented 4 years ago

I think I agree with @martinthomson here. Adding in-order stream reset sounds extremely complicated at either the HTTP/3 or QUIC layers. The particular use case here does not seem compelling enough to warrant this complexity.

kazuho commented 4 years ago

@RyanAtGoogle

Adding in-order stream reset sounds extremely complicated at either the HTTP/3 or QUIC layers. The particular use case here does not seem compelling enough to warrant this complexity.

As stated above, if it ought to be a complicated mechanism at the HTTP/3 layer depends on the how the HTTP/3 tunnel frames the response. As we've discussed previously in #1885, an HTTP/3 intermediaries can and will have different framing strategies. When the strategy employed by an intermediary is to create a new DATA frame every time it emits something, sending a HTTP frame to indicate an error after sending all the HTTP response it has is trivial.

All that said, I think that we might not need a new frame after all.

We already allow a HTTP tunnel to send a malformed response (see the third paragraph of section 4.1.3). That tunnel might be coalescing HTTP messages going to different endpoints. That means that an endpoint should not handle a stream that contains invalid DATA frames as a connection-level issue.

I've changed #3303 to be a clarification of this requirement, which has been implicit until now.

WDYT?

RyanTheOptimist commented 4 years ago

Ah! This sounds much better. No additional complexity in HTTP/3 or QUIC? SGTM.

janaiyengar commented 4 years ago

Thanks, @kazuho -- this re-framing (pun alert!) helps quite a bit. What you now have in the PR makes sense. Do you mind changing the title and description of this issue to reflect that?

kazuho commented 4 years ago

@janaiyengar Changed the title. Though I'm not sure if you like the new one. I think the only difference is that we are now talking about errors in the headers too.

@RyanAtGoogle

No additional complexity in HTTP/3 or QUIC? SGTM.

Yes. That true for what we have now in #3303.

Though, unfortunately, I now realize that we have the following provision in section 7.1: When a stream terminates cleanly, if the last frame on the stream was truncated, this MUST be treated as a connection error of type H3_FRAMEERROR.

If we consider that a tunnel should be allowed to send DATA frames (or HTTP/1 chunks) that it receives from upstream as is, forwarding them frame-by-frame to downstream, and expect things to work, then we need change this provision to so that incomplete (DATA) frames at the end of the stream would be considered as a malformed response instead of a connection-level error.

I would prefer making such a change, though I would anticipate hearing objections. Admittedly, it is an unnecessary complexity for clients, and for server deployments who control the origin. To this end, I wonder what other proxy vendors think.

@MikeBishop @LPardue WDYT?

LPardue commented 4 years ago

I wasn't following this discussion so closely, so IIUC the problem is that the stream cannot end abruptly or else there are no guarantess from QUIC layer that the DATA will arrive at the HTTP client. Closing cleanly would ensure DATA is delivered but triggers h3-24 section 7.1.

If we consider that a tunnel should be allowed to send DATA frames (or HTTP/1 chunks) that it receives from upstream as is,

To invoke the problem, an intermediary would have needed to committed a DATA frame of length N, to stream offset O, then attempt to fin the stream at offset F, which is less than O+N. I can see several scenarios that this might occur in an intermediary and it would be quite unfortunate to cause a connection close.

Since request streams are the only core streams that can be cleanly closed without blowing up the connection I think the middle ground might be to state something like: an endpoint that receives truncated leading HEADERS or PUSH_PROMISE should treat it as a connection error. Truncation of DATA and non-leading HEADERS is ok. Extension frames or streams need to consider truncation and specify the behaviour.

MikeBishop commented 4 years ago

In either case, the close signal never overtakes the partial payload.

This is not true, or at least not universal. It's entirely legitimate for a TCP stack to dump anything that's in buffers (unread by the application) and immediately surface the TCP RST. The reset can and does overtake partial payload in places.

We already allow a HTTP tunnel to send a malformed response (see the third paragraph of section 4.1.3). That tunnel might be coalescing HTTP messages going to different endpoints. That means that an endpoint should not handle a stream that contains invalid DATA frames as a connection-level issue.

That's not my understanding of that paragraph. It says that if an intermediary receives a malformed response, it MUST NOT forward it and MUST treat receipt of the malformed response as a stream error. This is unclear in two ways we need to correct:

I would propose that we need YAEC to signal an aborted response upstream.

The suggestion that we send as much as we can, then cleanly terminate the stream disturbs me. I'm inclined to take a hard line here and keep the text that intermediaries shouldn't be forwarding known broken responses. That way lies consistency bugs. You forward data as long as it's valid; if it becomes invalid, then you forward a RESET_STREAM that indicates the upstream is no longer talking sense.

MikeBishop commented 4 years ago

Ah, I see -- you're keying off the language that says an intermediary might not process the message and therefore might not notice that the message is malformed. I agree that an intermediary that deliberately doesn't check for message validity wouldn't trigger this requirement for things like missing pseudo-headers.

I'm less sanguine about saying the tunnel could silently pass through things like truncated frames, unknown frame types, etc.

kazuho commented 4 years ago

@LPardue @MikeBishop Thank you for sharing your thoughts.

I can see the hesitation against requiring endpoints (clients) to consider partial frame at the end of stream as stream-level errors (rather than connection-level problem).

Maybe what we want to discuss first is:

Q1. Do intermediary developers (especially of those that talk to servers owned by different organizational entities) want to deliver the partial response that they've received from upstream to downstream, before tearing down the stream?

Q2. Do we need a mechanism for signaling such behavior? That mechanism can be a partial frame at the end of the stream, or could be a new HTTP/3 frame. Or if the answer to this question is no, then, people wanting to do Q1 need to send the partial response body, wait for all the ACKs, then send a stream reset.

My answer to Q1 is a strong yes, and Q2 is a weaker yes. I'd prefer having a simple signaling mechanism (partly because I think everybody should do Q1), but for myself, I can resort to the complex way of delaying the emission of stream reset.

janaiyengar commented 4 years ago

I'm less sanguine about saying the tunnel could silently pass through things like truncated frames, unknown frame types, etc.

It seems to me that we don't need to specify what tunnels ought to do but recognize what tunnels might do. The way I read it, Kazuho's proposed recommendation makes sense when a tunnel passes a partial response through, as H2O does and others might as well.

LPardue commented 4 years ago

I am a yes for Q1. For Q2, if we wanted to pursue another frame type I don't (yet?) understand how it would work, won't the usefulness of the frame be subject to the size of DATA frames that people chose to use?

ianswett commented 4 years ago

With TCP, typically the reset will arrive after the payload, so the typically the partial payload can be processed.

How about using a similar recommendation for QUIC/H3: You CAN/SHOULD send all the data before transmitting the reset, but there's no need to guarantee it all arrives before sending the reset? If an implementation really wants to guarantee the payload arrives before the stream is reset, it can, but this seems like best effort.

I don't want to add a new mechanism for this, but I agree it'd be nice to avoid the H3 connection being torn down in this case. I do think we should scope this case a bit tighter and be more specific than the current PR, since it currently says any incorrectly formatted response MUST NOT cause a connection close and I'm realizing how wide that might be.

MikeBishop commented 4 years ago

I think there's a larger idea here: We took a series of PRs to make any protocol misbehavior connection-fatal. However, when there's an intermediary, it's not necessarily clear whether the misbehavior is on the part of the intermediary or the origin. And it's not necessarily clear to the client whether there's an intermediary, so it generally will have to assume one or the other all the time.

The takeaway for me is that we overreached -- if the trigger for an error could be attributed to the origin rather than the intermediary, then the error should not be connection-fatal on the client-intermediary connection. If there are things in this class that we believe need to be connection-fatal anyway, the corollary is that the intermediary MUST check for and sanitize these things.

kazuho commented 4 years ago

@MikeBishop I agree to your comments, with the only exception that I do not think we have "overreached."

In my view, HTTP/2 is underspecified in sense that it allows any stream error to be promoted to a connection error, even though as we agree, a stream error that might have originated from an origin cannot be promoted to a connection error.

I think it is an improvement in HTTP/3, to be more explicit about the distinction between the two types of errors. Therefore, even though I am not sure if I like the complexity of defining the type of error for every possible scenario, I'd prefer trying to do that until we fail now that we've crossed the bridge.

martinthomson commented 4 years ago

HTTP/2 is underspecified in sense that it allows any stream error to be promoted to a connection error

I think that we might disagree on this point. Even if the spec didn't expressly include this permission, you can always reduce this to a question of local policy. That is, my policy could be to cease communicating with a peer that fails to follow the spec.

The effect of such a policy has been beneficial in that it provides some incentive for new implementations to follow the spec. The same policy has also been bad in those cases where this was overzealously implemented: if we can't used new frame types without explicit negotiation, that's not a great state to be in.

even though as we agree, a stream error that might have originated from an origin cannot be promoted to a connection error

I don't think "cannot" is agreed. We might agree on the outcome of propagating the error, but I personally don't think that we should externalize these effects even in this way. We might highlight the possibility of problems, but if an intermediary is forwarding streams without looking at their contents, it is still ultimately responsible for the effect of their contents. If clients decide to hold the intermediary responsible for a failing at an origin server that is invisible to them, that's still the intermediary's problem.

I'm going to suggest that we acknowledge that this is tricky and then move on. I don't want to create another application of the Robustness Principle before we've even shipped the protocol.

kazuho commented 4 years ago

@martinthomson

even though as we agree, a stream error that might have originated from an origin cannot be promoted to a connection error

I don't think "cannot" is agreed. We might agree on the outcome of propagating the error, but I personally don't think that we should externalize these effects even in this way. We might highlight the possibility of problems, but if an intermediary is forwarding streams without looking at their contents, it is still ultimately responsible for the effect of their contents. If clients decide to hold the intermediary responsible for a failing at an origin server that is invisible to them, that's still the intermediary's problem.

I'm going to suggest that we acknowledge that this is tricky and then move on. I don't want to create another application of the Robustness Principle before we've even shipped the protocol.

I might argue that we have had that principle. RFC 7540 by default allows a client to coalesce requests going to multiple origins (assuming that the DNS lookup and the provided certificate allows that), then let the server opt-out (by sending 421). RFC 8336 and draft-ietf-httpbis-http2-secondary-certs talk about coalescing requests going to multiple origins. It is my understanding that we have collectively thought that coalescing connections when possible is beneficial for privacy and performance reasons (prevents leak of SNI, better network utilization by not having multiple congestion controllers between the same endpoints).

Saying that a response from one origin might disrupt responses from other origins that are coalesced on one HTTP connection will be a wind against these previous attempts.

I am not necessarily against creating a new connection per each origin (or set of origins owned by single entity). The privacy benefit will be covered by ESNI, and the performance reason can be covered by something like draft-kazuho-quic-address-bound-token.

However, as I have stated, I think we need to take a principled approach on when coalescing should (or should not) happen. This includes reconsidering if it is a good idea to allow the client to coalesce by default, as sending 421 requires a round-trip.

ianswett commented 4 years ago

I think coalescing is still a good thing. I don't expect this issue to have much impact on that in practice.

MikeBishop commented 4 years ago

Taking a look at the text, I still think the right principle here is that errors which might have been tunneled from an origin shouldn't be required to be stream errors. Looking at the text, here are the things that might fall into this category:

Does this principle hold for the opposite direction? Should servers consider garbage on streams as stream errors, because the "client" might actually be a proxy for several clients? It's probably the simplest to keep things symmetrical.

I've put #3336 as an alternative solution to this.

kazuho commented 4 years ago

I think that this issue discusses two orthogonal problems:

It might be a good idea to split this issue in two; both #3303 and #3336 are attempts to address the second problem, but not the first problem.

Regarding the second problem, there are three paths:

As stated in my previous comment, my preference goes to #3303, as it maximizes the chance of requests being coalesced. Unless we forbid the promotion of stream-level errors to connection level (or without stating that such promotion might have negative effects), CDNs cannot allow coalescing of HTTP requests that go to different origins, as one misbehaving origin might obstruct the exchanges between the client and other origins that share the same connection. While this is not an issue for all the server deployments, it would be a backlash against our previous efforts (e.g., ORIGIN frame, secondary certificates) to maximize the chance of requests going to different origins getting coalesced, as there would be less interest in implementing these extensions from CDNs.

MikeBishop commented 4 years ago

I agree that these are separate problems; for the first problem, I'm strongly inclined toward the status quo. If you want to ensure delivery of partial content, there is a workaround; for the default case, "play stupid games, win stupid prizes" seems like a reasonable point in the design space.

I'm willing to add cautionary text around the promotion permission, but I think an outright prohibition is both unwise and unenforceable. Implementers will do what they will do, and when there's garbage coming from the peer, it's a totally sensible design to close aggressively.

Frankly, I'd say CDNs shouldn't be shuffling raw frames anyway; they should be shuttling content, at the very least, and mostly have to anyway for HPACK/QPACK to work (unless everything is literal). At that point, we're really only talking about malformed headers.

RyanTheOptimist commented 4 years ago

I agree with @MikeBishop completely.

kazuho commented 4 years ago

@MikeBishop

I'm willing to add cautionary text around the promotion permission, but I think an outright prohibition is both unwise and unenforceable. Implementers will do what they will do, and when there's garbage coming from the peer, it's a totally sensible design to close aggressively.

Frankly, I'd say CDNs shouldn't be shuffling raw frames anyway; they should be shuttling content, at the very least, and mostly have to anyway for HPACK/QPACK to work (unless everything is literal). At that point, we're really only talking about malformed headers.

It is true that implementations would do whatever they do. To that end, I think it is correct to argue that there is no difference between a MUST NOT and a cautionary text with a clarification on what might happen. We'd have the same design as that in HTTP/2, and having clarification on the side-effects would be an improvement.

I also agree that CDNs shouldn't be shuffling raw frames anyway. That's not something on table. The original issue, for which I am seeing complexity, is about transmitting an error at the same time forwarding all the bytes up to where the error was detected. As previously stated, doing that has become much more complicated in HTTP/3 compared to HTTP/2. That said, it's not impossible, and if others implementing proxies are happy with living with such an workaround (or do not care about the problem), I can concede with that workaround.

Re malformed headers, I am starting to wonder the intent. Which of the following two are we suggesting?

If it's the latter, it would be impossible for an HTTP/3 intermediary to detect all the malformed headers (consider the case of a new HTTP extension defining a authentication header - a type of header field that is forbidden from being sent as part of a trailer). Maybe it's a good idea to clarify that it is the former, assuming that that is the intent.

MikeBishop commented 4 years ago

"Malformed" is defined by Section 4.1.3. If a specific header is not used properly, that's an error at the semantic layer; the server might send you a 400, but that's a successful request/response as far as HTTP/3 is concerned.

3334 adds the advice without any change to what is a stream/connection error, meaning that malformed messages are stream errors and the rest remain connection errors.

kazuho commented 4 years ago

@MikeBishop Yeah my point is that "prohibited header fields" is ambiguous. I've opened #3345.

MikeBishop commented 4 years ago

Note: editorial fix; issue hadn't been triaged as Design. However, @LPardue / @mnot / @larseggert, if you'd like to run the process on not handling the case of reliable delivery prior to a RESET_STREAM, please re-open.