From ef3171c5eb4bc22b79690afb36bbb1e681473ea0 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Mon, 2 Oct 2023 16:24:26 +0000 Subject: [PATCH] net: handle the network parameter properly in LookupPort The cgo version (unix) is populating the GetAddrInfo hints based on the network parameter, but windows not quite. This change populates the hints the same way as the cgo unix version does now. This bug was spotted by Bryan in CL 530415. https://go-review.googlesource.com/c/go/+/530415/comment/76640dc7_ed0409ca/ Change-Id: I6fc29b1e4cdc879123ab0f5a624b6f37c68c00ba GitHub-Last-Rev: eaa616378b3fa9a5a72192f3d501c591804f45d8 GitHub-Pull-Request: golang/go#63284 Reviewed-on: https://go-review.googlesource.com/c/go/+/531635 Reviewed-by: Ian Lance Taylor Run-TryBot: Mateusz Poliwczak Auto-Submit: Ian Lance Taylor TryBot-Result: Gopher Robot Reviewed-by: David du Colombier <0intro@gmail.com> Reviewed-by: Bryan Mills --- src/net/cgo_unix.go | 2 +- src/net/lookup.go | 51 ++++++++++++++++++------------ src/net/lookup_plan9.go | 28 +++++++++++------ src/net/lookup_test.go | 66 ++++++++++++++++++++++++++++++++++++--- src/net/lookup_windows.go | 28 +++++++++++------ 5 files changed, 131 insertions(+), 44 deletions(-) diff --git a/src/net/cgo_unix.go b/src/net/cgo_unix.go index 2a7d1ec3fa..0a783d08a9 100644 --- a/src/net/cgo_unix.go +++ b/src/net/cgo_unix.go @@ -80,7 +80,7 @@ func cgoLookupHost(ctx context.Context, name string) (hosts []string, err error) func cgoLookupPort(ctx context.Context, network, service string) (port int, err error) { var hints _C_struct_addrinfo switch network { - case "": // no hints + case "ip": // no hints case "tcp", "tcp4", "tcp6": *_C_ai_socktype(&hints) = _C_SOCK_STREAM *_C_ai_protocol(&hints) = _C_IPPROTO_TCP diff --git a/src/net/lookup.go b/src/net/lookup.go index 28532075d4..15165970b6 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -41,19 +41,20 @@ var services = map[string]map[string]int{ "domain": 53, }, "tcp": { - "ftp": 21, - "ftps": 990, - "gopher": 70, // ʕ◔ϖ◔ʔ - "http": 80, - "https": 443, - "imap2": 143, - "imap3": 220, - "imaps": 993, - "pop3": 110, - "pop3s": 995, - "smtp": 25, - "ssh": 22, - "telnet": 23, + "ftp": 21, + "ftps": 990, + "gopher": 70, // ʕ◔ϖ◔ʔ + "http": 80, + "https": 443, + "imap2": 143, + "imap3": 220, + "imaps": 993, + "pop3": 110, + "pop3s": 995, + "smtp": 25, + "submissions": 465, + "ssh": 22, + "telnet": 23, }, } @@ -83,12 +84,20 @@ const maxPortBufSize = len("mobility-header") + 10 func lookupPortMap(network, service string) (port int, error error) { switch network { - case "tcp4", "tcp6": - network = "tcp" - case "udp4", "udp6": - network = "udp" + case "ip": // no hints + if p, err := lookupPortMapWithNetwork("tcp", "ip", service); err == nil { + return p, nil + } + return lookupPortMapWithNetwork("udp", "ip", service) + case "tcp", "tcp4", "tcp6": + return lookupPortMapWithNetwork("tcp", "tcp", service) + case "udp", "udp4", "udp6": + return lookupPortMapWithNetwork("udp", "udp", service) } + return 0, &DNSError{Err: "unknown network", Name: network + "/" + service} +} +func lookupPortMapWithNetwork(network, errNetwork, service string) (port int, error error) { if m, ok := services[network]; ok { var lowerService [maxPortBufSize]byte n := copy(lowerService[:], service) @@ -96,9 +105,9 @@ func lookupPortMap(network, service string) (port int, error error) { if port, ok := m[string(lowerService[:n])]; ok && n == len(service) { return port, nil } - return 0, &DNSError{Err: "unknown port", Name: network + "/" + service, IsNotFound: true} + return 0, &DNSError{Err: "unknown port", Name: errNetwork + "/" + service, IsNotFound: true} } - return 0, &DNSError{Err: "unknown network", Name: network + "/" + service} + return 0, &DNSError{Err: "unknown network", Name: errNetwork + "/" + service} } // ipVersion returns the provided network's IP version: '4', '6' or 0 @@ -415,11 +424,13 @@ func LookupPort(network, service string) (port int, err error) { } // LookupPort looks up the port for the given network and service. +// +// The network must be one of "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6" or "ip". func (r *Resolver) LookupPort(ctx context.Context, network, service string) (port int, err error) { port, needsLookup := parsePort(service) if needsLookup { switch network { - case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": + case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6", "ip": case "": // a hint wildcard for Go 1.0 undocumented behavior network = "ip" default: diff --git a/src/net/lookup_plan9.go b/src/net/lookup_plan9.go index c9b4da951c..9d2c4cda5b 100644 --- a/src/net/lookup_plan9.go +++ b/src/net/lookup_plan9.go @@ -203,26 +203,36 @@ func (r *Resolver) lookupIP(ctx context.Context, network, host string) (addrs [] return } -func (*Resolver) lookupPort(ctx context.Context, network, service string) (port int, err error) { +func (r *Resolver) lookupPort(ctx context.Context, network, service string) (port int, err error) { switch network { - case "tcp4", "tcp6": - network = "tcp" - case "udp4", "udp6": - network = "udp" + case "ip": // no hints + if p, err := r.lookupPortWithNetwork(ctx, "tcp", "ip", service); err == nil { + return p, nil + } + return r.lookupPortWithNetwork(ctx, "udp", "ip", service) + case "tcp", "tcp4", "tcp6": + return r.lookupPortWithNetwork(ctx, "tcp", "tcp", service) + case "udp", "udp4", "udp6": + return r.lookupPortWithNetwork(ctx, "udp", "udp", service) + default: + return 0, &DNSError{Err: "unknown network", Name: network + "/" + service} } +} + +func (*Resolver) lookupPortWithNetwork(ctx context.Context, network, errNetwork, service string) (port int, err error) { lines, err := queryCS(ctx, network, "127.0.0.1", toLower(service)) if err != nil { if stringsHasSuffix(err.Error(), "can't translate service") { - return 0, &DNSError{Err: "unknown port", Name: network + "/" + service, IsNotFound: true} + return 0, &DNSError{Err: "unknown port", Name: errNetwork + "/" + service, IsNotFound: true} } return } if len(lines) == 0 { - return 0, &DNSError{Err: "unknown port", Name: network + "/" + service, IsNotFound: true} + return 0, &DNSError{Err: "unknown port", Name: errNetwork + "/" + service, IsNotFound: true} } f := getFields(lines[0]) if len(f) < 2 { - return 0, &DNSError{Err: "unknown port", Name: network + "/" + service, IsNotFound: true} + return 0, &DNSError{Err: "unknown port", Name: errNetwork + "/" + service, IsNotFound: true} } s := f[1] if i := bytealg.IndexByteString(s, '!'); i >= 0 { @@ -231,7 +241,7 @@ func (*Resolver) lookupPort(ctx context.Context, network, service string) (port if n, _, ok := dtoi(s); ok { return n, nil } - return 0, &DNSError{Err: "unknown port", Name: network + "/" + service, IsNotFound: true} + return 0, &DNSError{Err: "unknown port", Name: errNetwork + "/" + service, IsNotFound: true} } func (r *Resolver) lookupCNAME(ctx context.Context, name string) (cname string, err error) { diff --git a/src/net/lookup_test.go b/src/net/lookup_test.go index ab68d75836..1e222763bd 100644 --- a/src/net/lookup_test.go +++ b/src/net/lookup_test.go @@ -1462,9 +1462,65 @@ func testLookupNoData(t *testing.T, prefix string) { } func TestLookupPortNotFound(t *testing.T) { - _, err := LookupPort("udp", "_-unknown-service-") - var dnsErr *DNSError - if !errors.As(err, &dnsErr) || !dnsErr.IsNotFound { - t.Fatalf("unexpected error: %v", err) - } + allResolvers(t, func(t *testing.T) { + _, err := LookupPort("udp", "_-unknown-service-") + var dnsErr *DNSError + if !errors.As(err, &dnsErr) || !dnsErr.IsNotFound { + t.Fatalf("unexpected error: %v", err) + } + }) +} + +// submissions service is only available through a tcp network, see: +// https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=submissions +var tcpOnlyService = func() string { + // plan9 does not have submissions service defined in the service database. + if runtime.GOOS == "plan9" { + return "https" + } + return "submissions" +}() + +func TestLookupPortDifferentNetwork(t *testing.T) { + allResolvers(t, func(t *testing.T) { + _, err := LookupPort("udp", tcpOnlyService) + var dnsErr *DNSError + if !errors.As(err, &dnsErr) || !dnsErr.IsNotFound { + t.Fatalf("unexpected error: %v", err) + } + }) +} + +func TestLookupPortEmptyNetworkString(t *testing.T) { + allResolvers(t, func(t *testing.T) { + _, err := LookupPort("", tcpOnlyService) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) +} + +func TestLookupPortIPNetworkString(t *testing.T) { + allResolvers(t, func(t *testing.T) { + _, err := LookupPort("ip", tcpOnlyService) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) +} + +func allResolvers(t *testing.T, f func(t *testing.T)) { + t.Run("default resolver", f) + t.Run("forced go resolver", func(t *testing.T) { + if fixup := forceGoDNS(); fixup != nil { + defer fixup() + f(t) + } + }) + t.Run("forced cgo resolver", func(t *testing.T) { + if fixup := forceCgoDNS(); fixup != nil { + defer fixup() + f(t) + } + }) } diff --git a/src/net/lookup_windows.go b/src/net/lookup_windows.go index ce0df7ddd8..b6ef6da716 100644 --- a/src/net/lookup_windows.go +++ b/src/net/lookup_windows.go @@ -200,18 +200,28 @@ func (r *Resolver) lookupPort(ctx context.Context, network, service string) (int // TODO(bradfitz): finish ctx plumbing. Nothing currently depends on this. acquireThread() defer releaseThread() - var stype int32 + + var hints syscall.AddrinfoW + switch network { - case "tcp4", "tcp6": - stype = syscall.SOCK_STREAM - case "udp4", "udp6": - stype = syscall.SOCK_DGRAM + case "ip": // no hints + case "tcp", "tcp4", "tcp6": + hints.Socktype = syscall.SOCK_STREAM + hints.Protocol = syscall.IPPROTO_TCP + case "udp", "udp4", "udp6": + hints.Socktype = syscall.SOCK_DGRAM + hints.Protocol = syscall.IPPROTO_UDP + default: + return 0, &DNSError{Err: "unknown network", Name: network + "/" + service} } - hints := syscall.AddrinfoW{ - Family: syscall.AF_UNSPEC, - Socktype: stype, - Protocol: syscall.IPPROTO_IP, + + switch ipVersion(network) { + case '4': + hints.Family = syscall.AF_INET + case '6': + hints.Family = syscall.AF_INET6 } + var result *syscall.AddrinfoW e := syscall.GetAddrInfoW(nil, syscall.StringToUTF16Ptr(service), &hints, &result) if e != nil {