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

net: PreferGo DNS resolver failure #56107

Closed
CaledoniaProject opened this issue Oct 8, 2022 · 6 comments
Closed

net: PreferGo DNS resolver failure #56107

CaledoniaProject opened this issue Oct 8, 2022 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@CaledoniaProject
Copy link

CaledoniaProject commented Oct 8, 2022

What version of Go are you using (go version)?

$ go version
go version go1.19.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/xxxx/Library/Caches/go-build"
GOENV="/Users/xxxx/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/xxxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/xxxx/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/homebrew/Cellar/go/1.19.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/homebrew/Cellar/go/1.19.2/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/x4/qgthmfjj73n0gh9jpdnbd6780000gn/T/go-build2739239647=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I'm trying to resolve a domain with a custom dialer.

package main

import (
	"context"
	"fmt"
	"net"
	"time"
)

func main() {
	domain := "a.------------a.flocktory.com"
	r := &net.Resolver{
		PreferGo: true,
		Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
			d := net.Dialer{
				Timeout: time.Second * time.Duration(5),
			}
			return d.DialContext(ctx, network, "8.8.8.8:53")
		},
	}

	ips, err := r.LookupIP(context.Background(), "ip4", domain)
	fmt.Println(ips)
	fmt.Println(err)

	fmt.Println(net.LookupIP(domain))
}

Output

[]
lookup a.------------a.flocktory.com: no such host
[52.51.157.173 52.48.66.89 108.128.206.57 52.31.238.211] <nil>

What did you expect to see?

I'm expecting go dns to get the same result as the cgo version

What did you see instead?

The go version failed to resolve anything.

@ianlancetaylor ianlancetaylor changed the title affected/package: net/dns critical bug in PreferGo net: PreferGo DNS resolver failure Oct 8, 2022
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 8, 2022
@mateusz834
Copy link
Member

mateusz834 commented Oct 8, 2022

The go resolver is not even sending a dns request. It is rejected here:

if !isDomainName(name) {
// See comment in func lookup above about use of errNoSuchHost.
return nil, dnsmessage.Name{}, &DNSError{Err: errNoSuchHost.Error(), Name: name, IsNotFound: true}
}

a.------------a.flocktory.com is not a valid hostname as specified in RFC 1035 2.3.1.

They must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen.

Seems like the glibc is more permissive here.

@ianlancetaylor ianlancetaylor added this to the Backlog milestone Oct 8, 2022
@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 8, 2022
@CaledoniaProject
Copy link
Author

CaledoniaProject commented Oct 8, 2022

  1. Why is golang returning a errNoSuchHost, it should be something different right? e.g errInvalidRequest. The comment says // See comment in func lookup above about use of errNoSuchHost. but I didn't find it ..
  2. Can you make this isDomainName() method public to us? e.g IsDomainName()
  3. homebrew dig also works
%> dig a.------------a.flocktory.com @8.8.8.8

; <<>> DiG 9.10.6 <<>> a.------------a.flocktory.com @8.8.8.8
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 4203
;; flags: qr rd ra; QUERY: 1, ANSWER: 5, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 512
;; QUESTION SECTION:
;a.------------a.flocktory.com.	IN	A

;; ANSWER SECTION:
a.------------a.flocktory.com. 300 IN	CNAME	rkn.flocktory.com.
rkn.flocktory.com.	60	IN	A	52.51.157.173
rkn.flocktory.com.	60	IN	A	108.128.206.57
rkn.flocktory.com.	60	IN	A	52.48.66.89
rkn.flocktory.com.	60	IN	A	52.31.238.211

;; Query time: 58 msec
;; SERVER: 8.8.8.8#53(8.8.8.8)
;; WHEN: Sun Oct 09 07:54:05 CST 2022
;; MSG SIZE  rcvd: 140

@mateusz834
Copy link
Member

mateusz834 commented Oct 9, 2022

  1. if !isDomainName(name) {
    // We used to use "invalid domain name" as the error,
    // but that is a detail of the specific lookup mechanism.
    // Other lookups might allow broader name syntax
    // (for example Multicast DNS allows UTF-8; see RFC 6762).
    // For consistency with libc resolvers, report no such host.
    return dnsmessage.Parser{}, "", &DNSError{Err: errNoSuchHost.Error(), Name: name, IsNotFound: true}
    }
  2. requires proposal. See https://go.dev/s/proposal.
  3. this is a dns name, not a hostname.

@thediveo
Copy link

thediveo commented Oct 9, 2022

Ah, the ghost of RFC 1035 "hostnames". May I throw in RFC 8499, the Best Current Practise RFC on DNS Terminology? Section 2 is about "Names".

Label:  An ordered list of zero or more octets that makes up a
      portion of a domain name.  Using graph theory, a label identifies
      one node in a portion of the graph of all possible domain names.

Notice, how labels are made up of octets, or bytes (for those disliking the term "octet"). It's not about hostnames.

The basic wire format for names in the global DNS is a list of
 labels ordered by decreasing distance from the root, with the
 root label last.  Each label is preceded by a length octet.
 [RFC1035] also defines a compression scheme that modifies this
 format.

And then comes the "deadly" strike:

Host name:  This term and its equivalent, "hostname", have been
widely used but are not defined in [RFC1034], [RFC1035],
[RFC1123], or [RFC2181].  The DNS was originally deployed into the
Host Tables environment as outlined in [RFC952], and it is likely
that the term followed informally from the definition there.  Over
time, the definition seems to have shifted.  "Host name" is often
meant to be a domain name that follows the rules in Section 3.5 of
[RFC1034], which is also called the "preferred name syntax".  (In
that syntax, every character in each label is a letter, a digit,
or a hyphen).  Note that any label in a domain name can contain
any octet value; hostnames are generally considered to be domain
names where every label follows the rules in the "preferred name
syntax", with the amendment that labels can start with ASCII
digits (this amendment comes from Section 2.1 of [RFC1123].

Bottom line: the DNS wire protocol uses length-encoded octet labels, with the end of the list represented by the empty lable ("root"). The interpretation of the label octets is up to a particular DNS resolver. As already mentioned in the cited code section above, Apple made fun of any definitions by encoding UNICODE into the labels; they are perfectly within the definitions, or the lack of them.

The Go DNS stub resolver ("DNS client") thus should not make any assumption of what octets do constitute a label, as long as the wire protocol isn't violated and that means either an empty label that isn't the list termination, or an overlong label. And finally, all encoded labels must not be more than 255 (254?) octets in total.

Go is often tagged as "opinionated", but its DNS stub resolver shouldn't take that description too serious.

@seankhliao
Copy link
Member

Duplicate of #17659

@seankhliao seankhliao marked this as a duplicate of #17659 Oct 9, 2022
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2022
@mateusz834
Copy link
Member

I don't know the exac reason on why the isDomainName() check was added, but i think that it was added to align with the glibc. I tested the code sample with cgo on debian 9 (so with an older glibc) and the cgo version also returned an no such host error. But on a latest glibc it returns without an error, so something has changed in glibc logic.

@golang golang locked and limited conversation to collaborators Oct 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants