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: document and check LookupCNAME on non-CNAME hosts #8516

Closed
egonelbre opened this issue Aug 12, 2014 · 13 comments

Comments

Projects
None yet
9 participants
@egonelbre
Copy link
Contributor

commented Aug 12, 2014

LookupCNAME does not have clear behavior what it does when the DNS query succeeds and
there isn't a CNAME record.

Comment says that:

// LookupCNAME returns the canonical DNS host for the given name.
// Callers that do not care about the canonical name can call
// LookupHost or LookupIP directly; both take care of resolving
// the canonical name as part of the lookup.

So, just to clarify does it need to do:

1. return "", nil
2. return "", errors.New("CNAME not found")
3. return fqdn(name), nil

Given the comment, the approach 3. seems to be the correct one, because CNAME declares
alias names, and the canonical name for X, if there are no CNAME records, is X itself.
Of course some code could rely on returning "" and error.

Of course the code doesn't seem to be doing that - In Windows it's doing 2. in Linux 3.

Also, when researching this I noticed in dnsclient_unix.go at line 432, 

> func goLookupCNAME(name string) (cname string, err error) {
>    _, rr, err := lookup(name, dnsTypeCNAME)
>    if err != nil {
>        return
>    }
>    cname = rr[0].(*dnsRR_CNAME).Cname
>    return
> }

It looks like it might blow up when len(rr) == 0.
@egonelbre

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2014

Comment 1:

Just as an additional clarification, what should the LookupCNAME return with multiple
CNAME-s. Example:
alpha IN CNAME beta
beta IN CNAME gamma
gamma IN CNAME delta
In Windows LookupCNAME("alpha") returns "beta", in Linux it returns "delta".
I setup CNAME-s so the behavior could be seen:
cname2.egonelbre.com.   14400   IN      CNAME   cname1.egonelbre.com.
cname1.egonelbre.com.   14400   IN      CNAME   www.egonelbre.com.
www.egonelbre.com.      14400   IN      CNAME   egonelbre.com.
egonelbre.com.          14400   IN      A       195.222.15.98
@alexbrainman

This comment has been minimized.

Copy link
Member

commented Aug 14, 2014

Comment 2:

mikioh,
Can you help us, please?
Alex
@egonelbre

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2014

Comment 3:

And what should it return when there is an CNAME recursion: e.g.
alpha IN CNAME beta
beta IN CNAME gamma
gamma IN CNAME alpha
Currently Windows returns "beta". Linux returns an error "lookup alpha: no such host".
I setup a case for this as well:
err1.egonelbre.com.     14352   IN      CNAME   err2.egonelbre.com.
err2.egonelbre.com.     14352   IN      CNAME   err1.egonelbre.com.
@mikioh

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2014

Comment 4:

Ah... CNAME swamps. I'd suggest you to discuss at golang-dev because, a) DNS protocol
itself never describes what's the right stub resolver, recursive server (known as cache
server) or authoritative server implementation even though a few supplemental RFCs are
published such as RFC 1912, b) NOC/NIC guys have a great knowledge about this section
and major implementations usually follow their requests; e.g., years ago some
implmenetation allowed multiple CNAMEs but nowadays it's prohibited.
IMHO,
- multiple CNAMEs should be treated as error
- if we want to handle circular CNMAEs, we should implement loop avoidance too
- there are a few behavioural differences between primary stub resolver using
getaddrinfo or similar via CGO and builtin stub resolver, but we admit it because the
behavior of getaddrinfo is also different between platforms; e.g., glibc on Linux, BSD
libc on BSD variants and Windows.
Hope this helps.
@mikioh

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2014

Comment 5:

Labels changed: added repo-main.

@egonelbre egonelbre added new labels Sep 16, 2014

@bradfitz bradfitz removed the new label Dec 18, 2014

@rsc

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2015

Updating the docs seems fine.

@rsc rsc added this to the Go1.5 milestone Apr 10, 2015

@rsc rsc removed the repo-main label Apr 14, 2015

@mikioh mikioh modified the milestones: Go1.6, Go1.5 Jun 21, 2015

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2016

I am not sure what to say here. In general we can't mandate the behavior for CNAME pointing to CNAME. That will depend on the host resolver, as many things do. We may be better off not saying anything.

The question of what LookupCNAME does for a host without a CNAME is more interesting. It should be defined (I suspect it should return the original name, but I am not sure that's tenable) and be made consistent across systems.

@rsc rsc changed the title net: clarification on LookupCNAME behavior using builtin stub resolver net: document and check LookupCNAME on non-CNAME hosts Jan 7, 2016

@rsc rsc modified the milestones: Go1.7, Go1.6 Jan 7, 2016

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016

@quentinmit quentinmit added the NeedsFix label Oct 7, 2016

@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016

@rhansen

This comment has been minimized.

Copy link

commented Dec 3, 2016

For POSIX-ish systems (Linux, BSD, OS X), what about using the value returned by getaddrinfo() with the AI_CANONNAME flag set?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 3, 2016

@rhansen, the default resolver in Go is pure Go, and not using heavyweight libc (which requires threads, wasting valuable RAM seconds waiting for responses).

Also, there's Windows and Plan 9 and Solaris, etc. The net package is supposed to be a portable interface. We should define the semantics in terms of something we can reasonably implement on all OSes.

@rhansen

This comment has been minimized.

Copy link

commented Dec 4, 2016

I don't know about Plan 9, but Windows and Solaris both have getaddrinfo(). I suspect all major operating systems updated after RFC 3493 have getaddrinfo().

Unless the semantics of net.LookupCNAME is limited to "resolve a CNAME DNS resource record", a pure Go implementation seems a bit unwise because hostname resolution involves OS-specific implementation details like /etc/resolv.conf, /etc/hosts, NSS, etc. A pure Go implementation could ignore all of those details and simply walk CNAMEs until an A or AAAA record is found (or a loop is detected), though that might surprise users that are expecting features such as DNS search domains.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 4, 2016

We've had a pure Go implementation for the common case (parsing nsswitch.conf, resolv.conf, hosts, etc) for a number of releases now.

The point remains: we don't define Go interfaces in terms of C interfaces. We document what they should do in English, and implement them. Sometimes that implementation might involve using existing system libraries, or not. But that's a detail.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 13, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2017

I think this was fixed by https://golang.org/cl/34650, written to address #18172.

@mdempsky Do you agree?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2018

Closing because I think this is fixed. Please comment if you disagree.

@golang golang locked and limited conversation to collaborators Jan 3, 2019

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.