-
Notifications
You must be signed in to change notification settings - Fork 151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[connect-tcp] Remove bare TCP payload mode #2943
Conversation
This leaves only the Capsule Protocol mode Fixes #2855
draft-ietf-httpbis-connect-tcp.md
Outdated
|
||
When using "connect-tcp-capsule", TCP payload data is sent in the payload of a new Capsule Type named DATA ({{data-capsule}}). The ordered concatenation of DATA capsule payloads represents the main payload data stream in any protocol where this is well-defined. Intermediaries MAY split or merge DATA capsules. Endpoints MAY indicate a TCP connection error by sending an incomplete DATA capsule, as an alternative to using TCP, TLS, or HTTP stream errors. | ||
* In HTTP/1.1, endpoints SHOULD send an incomplete DATA capsule to indicate receipt of a TCP connection error (e.g., a TCP RST or timeout). When a connection is terminated with any incomplete capsule, or with an error in the underlying connection (e.g. a TLS "internal_error" alert or TCP RST), the endpoint SHOULD send a TCP RST if the underlying TCP implementation permits it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's "incomplete DATA capsule"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A DATA capsule is incomplete if it is truncated by the end of the underlying data stream. This concept is defined in RFC 9297, so I've added a reference here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if there is no data in-flight (i.e. an idle connection)? Should intermediaries inject malformed DATA
frame?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the recommendation is to inject a truncated DATA capsule by sending a DATA capsule header indicating nonzero length, then close the stream without sending any payload.
An alternative would be to define a new capsule type that serves as an explicit error indication.
An explicit error indication is much more powerful, may support easier debugging of connection failures, and may be easier to implement on the sender side. However, it is more work to implement on the receiver side, and opens up a major design bikeshed. The parsimony of having fewer ways to fail also might increase the odds of those failures being handled correctly.
I'm open to either approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're now always using capsules, I think there's value in making this an explicit signal. How about adding a DATA_END capsule of length_zero and requiring to send that? If the stream closes without receiving that, then it means it was aborted.
Alternate encoding: is to have two capsule types for DATA and a FIN bit, like we do for STREAM frames in QUIC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a bunch of different ways a TCP connection can end. At a minimum, there's the distinction between FIN and RST, but there's also timeout (dead air), interface going down, ICMP Unreachable, proxy-injected errors like "quota exceeded", etc. We have to decide how much detail to encode, and how to encode it.
We have to be able to emit those back to TCP endpoints, so the amount of details and bike-sheding that we can do here is limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DID SOMEONE SAY BIKESHED??? I'M ENTHUSIASTIC
I never liked the fact that reading 0 on a BSD socket means "end of stream" for TCP. It's odd, and inconsistent with other uses (e.g., for UDP it means "I received a UDP packet with no payload").
We should definitely separate between "clean termination" (TCP FIN) and "abortive termination" (TCP RST, timeout, etc.) but I don't think we need to separate RST from timeout. That would match what classic CONNECT does.
So, +1 for DATA capsule with a FIN bit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A FIN bit adds overhead to every DATA capsule, which seems unappealing to me. A zero-length DATA capsule seems fine to me (AFAIK we don't have any other use for length=0), but otherwise I would define a new capsule type ... probably a type meaning "clean shutdown", so that underlying transport failures naturally are treated as "unclean".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I meant putting the FIN bit in the capsule type. That can be described as two separate capsules (DATA = 0xTBD, DATA_END=0xTBD+1) or as a bit (DATA = 0xTBD..0xTBD+1, lowest bit of capsule type means FIN) but they're the same thing on the wire
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've written this in as a new FINAL_DATA capsule type.
draft-ietf-httpbis-connect-tcp.md
Outdated
|
||
When using "connect-tcp", TCP payload data is sent in the payload of a new Capsule Type named DATA (see registration in {{data-capsule}}). The ordered concatenation of DATA capsule payloads represents the TCP payload data. | ||
When using "connect-tcp", TCP payload data is sent in the payload of new Capsule Types named DATA and FINAL_DATA (see registrations in {{data-capsule}}). The ordered concatenation of these capsule payloads represents the TCP payload data. A FINAL_DATA capsule indicates that sender has closed this stream, semantically equivalent to TCP FIN. After sending a FINAL_DATA capsule, an endpoint MUST NOT send any more DATA or FINAL_DATA capsules on this data stream. (See {{closing-connections}} for related requirements.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mention that FINAL_DATA can contain payload? It's straightforward from the name but it's worth spelling it out to be safe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's mentioned several times here. I've made one small change to make that clearer: "A FINAL_DATA capsule additionally indicates...".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
draft-ietf-httpbis-connect-tcp.md
Outdated
A template-driven TCP transport proxy for HTTP is identified by a URI Template {{!RFC6570}} containing variables named "target_host" and "target_port". This URI Template and its variable values MUST meet all the same requirements as for UDP proxying ({{!RFC9298, Section 2}}), and are subject to the same validation rules. The client MUST substitute the destination host and port number into this template to produce the request URI. | ||
A template-driven TCP transport proxy for HTTP is identified by a URI Template {{!RFC6570}} containing variables named "target_host" and "target_port". This URI Template and its variable values MUST meet all the same requirements as for UDP proxying ({{!RFC9298, Section 2}}), and are subject to the same validation rules. The client MUST substitute the destination host and port number into this template to produce the request URI. The derived URI serves as the destination of a Capsule Protocol connection using the Upgrade Token "connect-tcp" (see registration in {{new-upgrade-token}}). | ||
|
||
When using "connect-tcp", TCP payload data is sent in the payload of new Capsule Types named DATA and FINAL_DATA (see registrations in {{data-capsule}}). The ordered concatenation of these capsule payloads represents the TCP payload data. A FINAL_DATA capsule additionally indicates that sender has closed this stream, semantically equivalent to TCP FIN. After sending a FINAL_DATA capsule, an endpoint MUST NOT send any more DATA or FINAL_DATA capsules on this data stream. (See {{closing-connections}} for related requirements.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to be late, but is there a strong reason to add FINAL_DATA when in H2 and H3 we have clear distinction between streams being reset (RST_STREAM and RESET_STREAM) vs. clean shutdown (FIN bit)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strongest argument I see for FINAL_DATA is that it guarantees* correct shutdown handling through semi-compliant HTTP/1.1 intermediaries. In particular, unclean shutdown signalling is often difficult to implement in HTTP/1.1, where (absent a signal in the Capsule Protocol) it requires injecting a TLS alert (or TCP RST), but TLS and TCP APIs don't always offer this ability. Previous versions of this draft suggested signalling by sending a truncated capsule, but this requires piercing an abstraction barrier that might reasonably be exposed by a Capsule Protocol implementation.
Even if the client and server are both speaking HTTP/3, there could be an HTTP/1.1 "hop" in between that "bleaches" the stream error.
*Nothing is ever guaranteed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I do not think people have complained that as a problem with the traditional CONNECT. TCP shutdown is murky, so applications tend to have their own ways of marking the end.
Therefore, the only case where having FINAL_DATA helps is when HTTP/1.1 is used for connect-tcp and when an application protocol that does not have its own way of marking the end but is vulnerable to truncation is being used.
Is that something we need to care?
I would not mind seeing this PR getting merged as-is, but I think FINAL_DATA needs its own issue and that we need to discuss at the WG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I have moved FINAL_DATA out of this PR and into a separate one (#2949).
I can say that I have personally fought with (1) applications that silently lost data due to truncation and (2) broken Happy Eyeballs with proxies that reported successful connection because the implementation had no way to report failure, so I do believe these issues are real, though maybe not the most common.
I do think we should add FINAL_DATA, but I'm fine with merging this first and then discussing that in #2949 |
This leaves only the Capsule Protocol mode
Fixes #2855