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: dns lookup should retry on TCP for truncated response #5686

Closed
gopherbot opened this issue Jun 11, 2013 · 12 comments
Closed

net: dns lookup should retry on TCP for truncated response #5686

gopherbot opened this issue Jun 11, 2013 · 12 comments
Milestone

Comments

@gopherbot
Copy link

by julius@soundcloud.com:

* What steps will reproduce the problem?
1. Setup a DNS server with SRV records too large to fit into a single UDP datagram.
2. Query said record via net.LookupSRV()
3. run "tcpdump port 53" with appropriate options to verify that Go tries only
via UDP, receives a truncated packet (DNS Flag "Truncated") and then doesn't
retry via TCP.

* What is the expected output?

net.LookupSRV() shouldn't return an error, but return appropriate record data. Doing the
equivalent query via Dig succeeds, as it sends the exact same UDP query at first, but
then tries TCP after seeing a truncated answer.

* What do you see instead?

net.LookupSRV() fails with an error: "lookup <...record-name...>. on
[::1]:53: no such host"

* Which compiler are you using (5g, 6g, 8g, gccgo)?

Shouldn't matter, see below.

* Which operating system are you using?

Ubuntu Linux 12.04.2 LTS

* Which version are you using?  (run 'go version')

Shouldn't matter, since only UDP is hardcoded in lookup() in HEAD (2013-06-11):

http://golang.org/src/pkg/net/dnsclient_unix.go#L90

See also "Truncation Flag" in
http://www.tcpipguide.com/free/t_DNSMessageHeaderandQuestionSectionFormat.htm

Note that LookupIP(), etc. seem to be using different lookup implementations, so they
might not be affected by the same problem.
@gopherbot
Copy link
Author

Comment 1 by julius@soundcloud.com:

Correction: several other net.Lookup*() functions (net.LookupIP(), net.LookupMX(), etc.)
seem to all use lookup() in the backend as well, so they should also be affected by this
problem.

@davecheney
Copy link
Contributor

Comment 2:

Are you using the net package with cgo enabled (the default) or disabled ?
If cgo is being used then shouldn't this be handled directly by the libc resolver ?

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

Comment 3 by julius@soundcloud.com:

I am simply building Go from source via src/all.bash. How do I enable using the cgo
versions of those functions? I don't think they're using cgo right now, since local
changes I made to the Go version of lookup() took effect after rebuilding.
Right now my go version is the latest tip:
go version devel +ae79f385177d Wed Jun 12 14:05:13 2013 +1000 linux/amd64

@davecheney
Copy link
Contributor

Comment 4:

cgo is enabled by default on all platforms. You can check the status
of cgo with `go env`

@gopherbot
Copy link
Author

Comment 5 by julius@soundcloud.com:

go env reports:
$ go env
GOARCH="amd64"
GOBIN=""
GOCHAR="6"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/julius/go:/home/julius/gosrc"
GORACE=""
GOROOT="/home/julius/go"
GOTOOLDIR="/home/julius/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-g -O2 -fPIC -m64 -pthread"
CGO_ENABLED="1"
$ which go
/home/julius/go/bin/go
$ ls -l /home/julius/go/bin/go
-rwxrwxr-x 1 julius julius 8096584 Jun 12 13:49 /home/julius/go/bin/go
So this is the version I just built. I added a panic() just before the Dial("udp", ...)
in net.tryOneName() and sure enough, it panics when I do LookupSRV().
Maybe there is no CGO version of this functionality yet?

@minux
Copy link
Member

minux commented Jun 12, 2013

Comment 6:

i confirm that there isn't cgo version for this yet.
does getaddrinfo(3) support SRV lookup?
(AI_SRVLOOKUP??)

@rsc
Copy link
Contributor

rsc commented Jun 14, 2013

Comment 7:

Even if we get cgo doing this, the pure Go version should be fixed.

Labels changed: added priority-later, go1.2, removed priority-triage.

Status changed to Accepted.

@gopherbot
Copy link
Author

Comment 8 by fish@soundcloud.com:

Seems like it's not supported: http://sourceware.org/bugzilla/show_bug.cgi?id=2099

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 9:

Let's get the pure DNS version working, but no cgo fallback to it.

Labels changed: added go1.2maybe, removed go1.2.

@gopherbot
Copy link
Author

Comment 10 by fish@soundcloud.com:

Considered including https://github.com/miekg/dns in the core? Might be too heavy, but
it's a great, idiomatic library in a similar style to net/http.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 11:

Labels changed: added feature.

@bradfitz
Copy link
Contributor

Comment 12:

This issue was closed by revision 0a3cb7e.

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2maybe label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants