-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
caddyhttp: Reject 0-RTT early data in IP matchers and set Early-Data header when proxying #6427
Conversation
…header when proxying See RFC 8470: https://httpwg.org/specs/rfc8470.html Thanks to Michael Wedl (@MWedl) at the University of Applied Sciences St. Poelten for reporting this.
/cc @marten-seemann in case you want to take a look at this too |
Oh, and @mohammed90 I also finally removed those pesky "experimental" comments on the listener APIs, they'd been sitting in my working copy for a while. |
When rejecting matching IPs when the TLS handshake has not completed, clients are not notified why the request fails. Caddy should return HTTP status code 425 Too Early to notify clients to try again later when the handshake is completed. Without sending status code 425, clients do not know that the request failure is only temporary. |
// easily be spoofed. It is conventional to respond with HTTP 425 | ||
// Too Early if the request cannot risk being processed in this | ||
// state. | ||
HandshakeComplete *bool `json:"handshake_complete,omitempty"` |
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.
Not familiar with the architecture here. Does this make 0-RTT request rejection configurable? Is there a compelling use case for that?
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.
It makes it matchable. Server handlers are configured like "if-then" blocks, if the condition is true, then chain in the middleware handler. The matchers are like "if" statements. So this makes 0-RTT/Early Data "matchable" or "detectable". It allows users to do what @MWedl and the RFCs suggest, that is to reply with HTTP 425. Once I add Caddyfile support, it would look like this:
@early tls handshake_incomplete
respond @early 425
I'm trying to decide if I should improve the syntax a bit. Maybe I could rename handshake_incomplete
to early_data
, as that may be more canonical?
To clarify, the IP matchers don't even try to match when the handshake is incomplete. Matchers can (and should) only return true or false; they don't respond to the request. Users who wish to have robust clients retry risky requests can easily configure the server to match early data and respond with 425. (memo to self: add Caddyfile support) |
Maybe this change is causing an issue I recently observed during tests with I have a path protected with an IP matcher, for example at https://example.com/protected, and the server is using HTTP/3. The configuration looks like this: handle /protected* {
@access_denied {
not remote_ip 4.8.15.16
}
error @access_denied 404
reverse_proxy {
to http://127.0.0.1:8703
}
} In general, it works fine when I access the protected path because my IP (4.8.15.16) is allowed. However, I noticed that after a certain period of inactivity (maybe an hour), I receive a 404 error from the My guess is that the F5 refresh that causes the 404 (by not letting my IP through) might be related to 0-RTT. I understand that IP addresses can't be trusted in 0-RTT because they could be spoofed, but this behavior is problematic since I have other rules that trigger redirects to different URLs when the IP doesn’t match. @mholt, would disabling 0-RTT in Caddy be a possible solution in this case? |
@steffenbusch See #6596, we're fixing it there to make IP matchers throw an error when the TLS handshake is incomplete. |
If I understand the description of #6596 correct, the IP matcher will then throw an error in case of incomplete TLS handshake and that error will "cancel the HTTP request pipeline" and that would make the 0-RTT "request" fail and the browser would send a regular reuqest? |
I don't actually know what the browser will do at this stage, but yes that's the idea, the matcher should throw an error instead of returning false. |
See RFC 8470: https://httpwg.org/specs/rfc8470.html
tls
matcher, which can match on properties of the underlying TLS connection. For now the only property is whether the TLS handshake has completed; we can add more later if needed. This could be useful for configuring HTTP 425 responses for unwanted Early Data.Early-Data: 1
header in the reverse proxy if the handshake has not completed. We don't enforce the other rules regarding this header since the user would have to explicitly configure violating behavior, which they may have good reason for or may not actually be in violation, so my current answer to that would be "don't do that".{http.request.remote[.host]}
placeholders now return an empty/nil value when it's early data, since the IP can't be trusted.Thanks to Michael Wedl (@MWedl) at the University of Applied Sciences St. Poelten for reporting this.