golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.06k stars 17.68k forks source link

x/net/quic: Endpoint.Dial prefers IPv4 to IPv6 #70223

Open jfgiorgi opened 1 week ago

jfgiorgi commented 1 week ago

Go version

go version go1.23.2 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/user/.cache/go-build'
GOENV='/home/user/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/user/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/user/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go1.23.2.linux-amd64'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go1.23.2.linux-amd64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/user/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/user/dev/nspeed-client/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1098587137=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Create a quic.Endpoint then call Dial method with a name (not a literal ip) in the address parameter.

here is a small POC :

package main

import (
    "context"
    "crypto/tls"
    "fmt"
    "net"

    "golang.org/x/net/quic"
)

func main() {
    // a http/3 enabled url like google.com or cloudflare.com
    address := "nspeed.app"

    quicConf := &quic.Config{
        TLSConfig: &tls.Config{
            MinVersion: tls.VersionTLS13,
            NextProtos: []string{"h3"},
            ServerName: address,
        },
    }

    // local endpoint
    endp, err := quic.Listen("udp", ":0", nil)

    if err != nil {
        panic(err)
    }

    // connect to address on port 443
    qconn, err := endp.Dial(context.Background(), "", address+":443", quicConf)
    if err != nil {
        panic(err)
    }

    fmt.Println("connected") // currently there is no way to display the remote ip , use strace or a network capture

    // unless you have https://github.com/golang/net/pull/225
    // fmt.Println("connected to", qconn.RemoteAddr())

    qconn.Close()
}

What did you see happen?

The connection happens with IPv4

What did you expect to see?

The connection should use IPv6 is it's available at the OS level.

This is a direct consequence of net.ResolveUDPAddr not preferring IPv6 (net.ResolveTCPAddr and net.ResolveIPAddr also have this issue)

The culprit is in src/net/ipsock.go#L82 (forResolve function)

see also #28666

A workaround is to resolve the address parameter before calling Endpoint.Dial net.Dial (tcp or udp) doesn't have this issue.

gabyhelp commented 1 week ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

dr2chase commented 1 week ago

@neild @ianlancetaylor

neild commented 1 week ago

Is this an x/net/quic issue, or #28666 happening to apply to x/net/quic as well as anything else that uses net.Resolve*Addr?

I think net.Dial avoids the problem since it attempts to connect to all the resolved addresses for a host, using a Happy Eyeballs style racing IPv4/IPv6 connect.

Possibly x/net/quic should do the same. Or possibly that's something that should be handled at a higher level.

If we wanted to be very fancy in x/net/quic, then the cost of a connection attempt doesn't need to be more than a single datagram sent--we could just send a datagram to every possible address and pick the first one to respond. But that'd be a bit tricky to integrate with the retry logic; if nothing comes back, do we resend to every address on PTO? Have a separate PTO timer per address? Simpler, but more expensive, to treat each potential remote address as a separate connection.

jfgiorgi commented 1 week ago

imho Happy Eyeballs should be higher level. HTTP/3 level for instance, along side the new HTTPS/SVCB DNS records (RFC 9460) .

I'm not a fan of Happy Eyeballs, it's only a transition mechanism , it's not the future and we ran into numerous issues with it, notably when people switch to IPv6 and forget to update or remove the IPv4 DNS record (which still point, for instance, to the older version of a service).

In 2024 "assuming that IPv6 is misconfigured" should not be handled by std lib code but should lead to have network/system people fix their stuff. Much like no one assumes IPv4 is misconfigured and try with IPv6 as a fallback.

net.Resolve*Addr should be deprecated (outside internal usage to keep compatibility). Or at minimum update their documentation to warn users about IPv6.

x/net/quic should be basic and respect IPv6 priority defined by the OS (getaddrinfo & gai.conf on Linux, prefixpolicies on Windows, etc). This shouldn't be in x/net/quic but in the resolver used by x/net/quic which should be may be an optional 'resolver' parameter of Endpoint.Dial ?

mateusz834 commented 6 days ago

net.Resolve*Addr should be deprecated (outside internal usage to keep compatibility). Or at minimum update their documentation to warn users about IPv6.

That is actually a good call. We probably should deprecate even more IP resolution functions, we have like 6-10 of them.