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: LookupCNAME only works for hostnames with CNAME records #18172

Closed
rhansen opened this issue Dec 2, 2016 · 9 comments

Comments

Projects
None yet
7 participants
@rhansen
Copy link

commented Dec 2, 2016

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

go version go1.7.3 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/usr/lib/google-golang"
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build668040631=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

package main

import (
	"fmt"
	"net"
)

func main() {
	if cname, err := net.LookupCNAME("www.google.com"); err != nil {
		fmt.Printf("lookup failed: %q\n", err)
	} else {
		fmt.Printf("lookup success: %s\n", cname)
	}
}

What did you expect to see?

The documentation says "LookupCNAME returns the canonical DNS host for the given name." To me, this means that it returns the canonical name as provided by the POSIX getaddrinfo() function when given the AI_CANONNAME flag. So I expected the following:

lookup success: www.google.com

What did you see instead?

lookup failed: "lookup www.google.com on 127.0.1.1:53: no such host"

It turns out that LookupCNAME simply resolves CNAME records, it does NOT return the canonical DNS host for the given name as documented.

Proposed change

Ideally LookupCNAME would return the canonical hostname as provided by the POSIX getaddrinfo() function.

If the intention of the LookupCNAME function is to only resolve CNAME records, then replace the first sentence of the documentation with:

LookupCNAME resolves the CNAME DNS record for the given name. It is an error if the given hostname does not have a CNAME record.

@mikioh mikioh changed the title net.LookupCNAME only works for hostnames with CNAME records net: LookupCNAME only works for hostnames with CNAME records Dec 2, 2016

@mikioh mikioh added the Documentation label Dec 2, 2016

@mikioh mikioh added this to the Go1.8Maybe milestone Dec 2, 2016

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2016

Agreed. We need to update the current documentation for avoiding unnecessary confusion the same as other documentation issues in the net package. Also it's recommended to use the correct terminology defined in https://tools.ietf.org/html/rfc7719.

Like other Lookup functions, I guess that the original author's intention is that the function simply takes an alias and returns a canonical name when the CNAME RR for the alias exists.

Note that the handling for the corrupted or nested CNAME RRs depends on each implementation and varies. #8516 is just for the issue.

@minux

This comment has been minimized.

Copy link
Member

commented Dec 3, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 15, 2016

@mdempsky, in our 1.8 release meeting the other day we decided to make sure that the Go DNS resolver path acted the same as the cgo path for LookupCNAME (does it? don't think so) and then document whatever that behavior is.

You have any interest in taking this on? It might be nice even for Go 1.8, if it's trivial enough.

@bradfitz bradfitz added the NeedsFix label Dec 15, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 15, 2016

Indeed. From the code at top:

$ GODEBUG=netdns=go go run cname.go 
lookup failed: "lookup www.google.com on 169.254.169.254:53: no such host"

$ GODEBUG=netdns=cgo go run cname.go 
lookup success: www.google.com.
@gopherbot

This comment has been minimized.

Copy link

commented Dec 15, 2016

CL https://golang.org/cl/34492 mentions this issue.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

The behavior on GODEBUG=netdns=go looks reasonable to me. Otherwise, we'll lose the way to test whether the full resolvers (or recursive servers) keep the canonical name for the alias. Moreover, we may be confused about the difference between Lookup{IP,Host} and LookupCNAME.

Isn't it better to drop the use of cgoLookupCNAME for LookupCNAME?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 15, 2016

@mikioh, well, the documentation has always says:

LookupCNAME returns the canonical DNS host for the given name

And until Go 1.5 we always used this cgo LookupCNAME behavior (https://golang.org/doc/go1.5#net), so it seems that the cgo behavior is what people (e.g. @rhansen) expect.

It seems more conservative to figure out what cgo did, make the pure Go version do that, and document it.

But arguably it's been broken for 3 releases and nobody's noticed, so probably few people care, so your proposal might also work.

Is it weird to use a mix of Go's DNS and libc DNS, though? Would that even work on Macs with the restrictive firewall settings?

/cc @ianlancetaylor @rsc

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2016

Please match the C library. Sometimes that is all we have. We can't write APIs that duplicate C library functionality but can't be implemented using it.

I looked into netdns=go on Mac a few months ago and I believe I found that it still didn't work under the most restrictive firewall settings. I don't remember the issue number though.

@mdempsky mdempsky self-assigned this Dec 16, 2016

@gopherbot

This comment has been minimized.

Copy link

commented Dec 19, 2016

CL https://golang.org/cl/34650 mentions this issue.

@gopherbot gopherbot closed this in 2f9dee9 Dec 20, 2016

@golang golang locked and limited conversation to collaborators Dec 20, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.