caddyserver / caddy

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
https://caddyserver.com
Apache License 2.0
57.53k stars 4.01k forks source link

reverse proxy to tcp4 address still connects on IPv6 #6447

Open hmoffatt opened 3 months ago

hmoffatt commented 3 months ago

As discussed at https://caddy.community/t/reverse-proxy-to-dynamic-ipv4-only/24773/5, I have the following reverse_proxy directive in my Caddyfile, where somehost has both A and AAAA records:

:9001 {
    reverse_proxy tcp4/somehost:8123
}

Despite tcp4, caddy still connects on IPv6. When I access it, the server logs

2024/07/08 00:30:00.097 DEBUG   http.handlers.reverse_proxy selected upstream   {"dial": "somehost:8123", "total_upstreams": 1}
2024/07/08 00:30:00.104 DEBUG   http.handlers.reverse_proxy upstream roundtrip  {"upstream": "tcp4/somehost:8123", "duration": 0.006397517, "request": {"remote_ip": "xxxx:yyyy:8c5e:42:96b6:f2c1:3fb8:84d3", "remote_port": "54408", "client_ip": "xxxx:yyyy:8c5e:42:96b6:f2c1:3fb8:84d3", "proto": "HTTP/1.1", "method": "GET", "host": "caddy-vm:9001", "uri": "/", "headers": {"X-Forwarded-Proto": ["http"], "X-Forwarded-Host": ["caddy-vm:9001"], "User-Agent": ["curl/7.88.1"], "Accept": ["*/*"], "X-Forwarded-For": ["xxxx:yyyy:8c5e:42:96b6:f2c1:3fb8:84d3"]}}, "headers": {"Content-Type": ["text/plain; charset=utf-8"], "Content-Length": ["16"], "Date": ["Mon, 08 Jul 2024 00:30:00 GMT"], "Server": ["Python/3.12 aiohttp/3.9.5"]}, "status": 400}
francislavoie commented 3 months ago

I don't think it's connecting over IPv6 to your upstream, you're seeing the remote_ip which is the address coming into Caddy. We don't actually log the final proxy dial address because that's handled inside Go's stdlib.

hmoffatt commented 3 months ago

The server at the other end (somehost:8123) is reporting that it got the connection on IPv6, and tcpdump confirms it. I see what you mean about the log though.

hmoffatt commented 3 months ago

These run and ignore the tcp4/tcp6 specifier, despite being nonsense:

:9002 {
    reverse_proxy tcp4/[fd5d:61f5:a84a:42::18]:8123
}

:9003 {
    reverse_proxy tcp6/192.168.42.18:8123
}
francislavoie commented 3 months ago

Since you're able to reproduce the issue, could you try to add some logging around https://github.com/caddyserver/caddy/blob/9338741ca79a74247ced86bc26e4994138470852/modules/caddyhttp/reverseproxy/httptransport.go#L222 to see what you get? Is the network as expected? What address does the conn report as having connected to? If you're not sure how, I can follow up with a branch in the next few days that you could try to build from to start.

hmoffatt commented 3 months ago

to see what you get? Is the network as expected? What address does the conn report as having connected to? If you're not sure how, I can follow up with a branch in the next few days that you could try to build from to start.

network is tcp here, not tcp4. But dialInfo.Network from the context says tcp4 as expected. I'll keep digging.

hmoffatt commented 3 months ago

I think the problem is the same as the one described in the comments for unix socket upstrams at https://github.com/caddyserver/caddy/blob/9338741ca79a74247ced86bc26e4994138470852/modules/caddyhttp/reverseproxy/httptransport.go#L208 - the standard library HTTP transport has passed the network through as "tcp", not the original "tcp4".

I extended the check for "unix" to cover "tcp" as well and got the expected behaviour, including errors about the nonsense addresses tcp4/[fd5d:61f5:a84a:42::18]:8123 and tcp6/192.168.42.18:8123.

2024/07/08 09:13:16.724 DEBUG   http.handlers.reverse_proxy upstream roundtrip  {"upstream": "tcp6/192.168.42.18:8123", "duration": 0.000054247, "request": {"remote_ip": "127.0.0.1", "remote_port": "44122", "client_ip": "127.0.0.1", "proto": "HTTP/1.1", "method": "GET", "host": "localhost:9003", "uri": "/", "headers": {"X-Forwarded-Proto": ["http"], "X-Forwarded-Host": ["localhost:9003"], "Accept": ["*/*"], "User-Agent": ["curl/7.88.1"], "X-Forwarded-For": ["127.0.0.1"]}}, "error": "dial tcp6: address 192.168.42.18: no suitable address found"}
mohammed90 commented 3 months ago

Thanks for the hint. I found this in stdlib code: https://github.com/golang/go/blob/6d89b38ed86e0bfa0ddaba08dc4071e6bb300eea/src/net/http/transport.go#L1727-L1728

It appears that Go stdlib overwrites the value here. Tracking and fixing this might be tricky.

hmoffatt commented 3 months ago

It appears that Go stdlib overwrites the value here. Tracking and fixing this might be tricky.

That's where I traced it to as well. https://github.com/caddyserver/caddy/blob/9338741ca79a74247ced86bc26e4994138470852/modules/caddyhttp/reverseproxy/httptransport.go#L208 already seems to be working around this for the unix case; does it make sense to extend it to other protocols?

I tested it with tcp and it worked as expected, but I don't know if it's actually correct. Nor do I know what should happen in the ip and udp cases that are documented as supported (https://caddyserver.com/docs/conventions#network-addresses).

diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go
index d4245368..427cf21a 100644
--- a/modules/caddyhttp/reverseproxy/httptransport.go
+++ b/modules/caddyhttp/reverseproxy/httptransport.go
@@ -213,7 +213,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
        // when not necessary, because http.ProxyFromEnvironment may have
        // modified the address according to the user's env proxy config.
        if dialInfo, ok := GetDialInfo(ctx); ok {
-           if strings.HasPrefix(dialInfo.Network, "unix") {
+           if strings.HasPrefix(dialInfo.Network, "unix") || strings.HasPrefix(dialInfo.Network, "tcp") {
                network = dialInfo.Network
                address = dialInfo.Address
            }
mholt commented 2 months ago

We could try it. If it doesn't break any tests that's probably a good sign as well.

I also wonder if this should be reported as a bug to the Go std lib.