Skip to content
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

client_ip matcher not working correctly on resumed HTTP/3 connections #6664

Closed
TNorthover opened this issue Oct 27, 2024 · 11 comments
Closed
Labels
bug 🐞 Something isn't working
Milestone

Comments

@TNorthover
Copy link

When an HTTP/3 connection is old enough that it needs explicitly resuming, it seems the client_ip matcher information becomes invalid which breaks LAN filtering (fortunately in a fails-closed state for me, but might not be for everyone).

Example Caddyfile:

(lan-only) {
        @not-lan {
                not client_ip 10.0.10.0/24
        }
        respond @not-lan "Accesssss Denied" 403
}

https://sandbox.lan {
	import lan-only
	tls internal
	log
	respond "Wibble"
}

Gives the logs:

Oct 27 08:58:07 sandbox caddy[164427]: {"level":"info","ts":1730019487.2480054,"logger":"http.log.access","msg":"handled request","request":{"remote_ip":"10.0.10.38","remote_port":"63281","client_ip":"10.0.10.38","proto":"HTTP/2.0","method":"GET","host":"sandbox.lan","uri":"/","headers":{"Accept":["text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8"],"Te":["trailers"],"Accept-Language":["en-GB,en;q=0.5"],"Sec-Fetch-Site":["none"],"Dnt":["1"],"Sec-Fetch-Dest":["document"],"Sec-Fetch-Mode":["navigate"],"Priority":["u=0, i"],"Accept-Encoding":["gzip, deflate, br, zstd"],"Sec-Gpc":["1"],"Sec-Fetch-User":["?1"],"Cookie":["REDACTED"],"Upgrade-Insecure-Requests":["1"],"User-Agent":["Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:131.0) Gecko/20100101 Firefox/131.0"]},"tls":{"resumed":false,"version":772,"cipher_suite":4865,"proto":"h2","server_name":"sandbox.lan"}},"bytes_read":0,"user_id":"","duration":0.000031453,"size":6,"status":200,"resp_headers":{"Alt-Svc":["h3=\":443\"; ma=2592000"],"Content-Type":["text/plain; charset=utf-8"],"Server":["Caddy"]}}
Oct 27 08:59:13 sandbox caddy[164427]: {"level":"info","ts":1730019553.11203,"logger":"http.log.access","msg":"handled request","request":{"remote_ip":"10.0.10.38","remote_port":"56262","client_ip":"10.0.10.38","proto":"HTTP/3.0","method":"GET","host":"sandbox.lan","uri":"/","headers":{"Alt-Used":["sandbox.lan"],"Priority":["u=0, i"],"User-Agent":["Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:131.0) Gecko/20100101 Firefox/131.0"],"Sec-Gpc":["1"],"Accept-Encoding":["gzip, deflate, br, zstd"],"Sec-Fetch-Dest":["document"],"Sec-Fetch-Mode":["navigate"],"Accept":["text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8"],"Accept-Language":["en-GB,en;q=0.5"],"Sec-Fetch-Site":["cross-site"],"Dnt":["1"],"Cookie":["REDACTED"],"Upgrade-Insecure-Requests":["1"]},"tls":{"resumed":true,"version":772,"cipher_suite":4865,"proto":"h3","server_name":"sandbox.lan"}},"bytes_read":0,"user_id":"","duration":0.000038118,"size":16,"status":403,"resp_headers":{"Server":["Caddy"],"Content-Type":["text/plain; charset=utf-8"],"Date":["Sun, 27 Oct 2024 08:59:13 GMT"]}}

The client_ip in the log still appears correct, but is not being used by the matcher so the second (resumed) try returns a 403.

Testing is unfortunately a little slow. Only Firefox even seems to attempt HTTP/3 for some reason (Chrome(ish) and Safari don't try, curl is one-shot as far as I know). And it will reuse the connection without an explicit "resumed" handshake (whatever that entails) unless it's left idle for 1 minute between tries.

In the logs above, the first request was HTTP/2 and the followup switched to HTTP/3 but I also have examples where a successful HTTP/3 request is made to start with, which is why I think the "resumed" is important.

@TNorthover
Copy link
Author

This appears to be a regression in the 2.9 beta. 2.8.4 correctly serves the file on a resumed connection.

@steffenbusch
Copy link
Contributor

Same issues here, but have a look at #6596 in particular #6596 (comment)

Temporary workaround for me is:

@early tls early_data
error @early 425

But I hope when #6596 is merged into master and used for the next 2.9 beta that the temporary workaround can be removed.

@TNorthover
Copy link
Author

Ah, thanks. I hadn't seen that PR and the workaround fixes it for me too.

@mattxtaz
Copy link

mattxtaz commented Nov 4, 2024

I raised this issue on the forum https://caddy.community/t/firefox-http-3-0-rtt-issue-with-named-ip-matcher/25764 where we came up with our own workaround which is different to this one. That thread is locked now but I'm slightly confused over what the final fix is going to be for this. Once that PR is committed do we still need either of these workarounds or will that solve it without any workarounds?

@francislavoie
Copy link
Member

francislavoie commented Nov 4, 2024

When #6596 is merged, the client_ip matcher will throw an error instead of returning false, meaning the request will fail with a specific error code (425 Too Early) if its IP address can't be safely determined. Browsers that support that error code should retry the request without 0-RTT and get through, but some browsers may just show an error requiring the user to refresh/retry themselves. (You'll need to yell at the browser vendors to fix that).

If you need the fix right away, you can build from that branch for now with xcaddy

@steffenbusch
Copy link
Contributor

@francislavoie would you mind to merge master into the 6596 branch? I would like to update my 2.9.0-beta2 environment.

@francislavoie
Copy link
Member

francislavoie commented Nov 4, 2024

I just rebased a few minutes ago.

@mattxtaz
Copy link

mattxtaz commented Nov 4, 2024

I've just built that branch with xcaddy and removed my workaround. It appears to do exactly what you said. Looks good to me!

Nov 04 19:56:27 example.com caddy[315175]: 10.0.0.10 - - "GET /unread HTTP/3.0" 425 13 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:132.0) Gecko/20100101 Firefox/132.0"
Nov 04 19:56:27 example.com caddy[315175]: 10.0.0.10 - - "GET /unread HTTP/3.0" 200 2516 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:132.0) Gecko/20100101 Firefox/132.0"

@steffenbusch
Copy link
Contributor

Same experience for me. Works like a charme. Would be awesome, if #6596 could be added to the next 2.9.0-beta (or final) release.

@francislavoie
Copy link
Member

That is the plan @steffenbusch

@mholt
Copy link
Member

mholt commented Nov 4, 2024

Yeah I'm in process of reviewing this

@mholt mholt added this to the v2.9.0-beta.3 milestone Nov 4, 2024
@mholt mholt closed this as completed Nov 5, 2024
@mholt mholt added the bug 🐞 Something isn't working label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants