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

Jump to bottom

net: LookupCNAME only works for hostnames with CNAME records #18172

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

net: LookupCNAME only works for hostnames with CNAME records #18172

rhansen opened this issue Dec 2, 2016 · 9 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8Maybe

Comments

@rhansen
Copy link

rhansen 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 this to the Go1.8Maybe milestone Dec 2, 2016
@mikioh
Copy link
Contributor

mikioh commented Dec 2, 2016 •
edited

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
Copy link
Member

minux commented Dec 3, 2016 via email

@bradfitz
Copy link
Contributor

@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 The path to resolution is known, but the work has not been done. label Dec 15, 2016
@bradfitz
Copy link
Contributor

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
Copy link

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

@mikioh
Copy link
Contributor

mikioh 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
Copy link
Contributor

@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
Copy link
Contributor

rsc 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
Copy link

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

@ianlancetaylor ianlancetaylor mentioned this issue Dec 8, 2017
Closed
@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.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
7 participants
@bradfitz @mdempsky @mikioh @rsc @minux @rhansen @gopherbot