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: don't keep reading from UDP resolver after truncated packet #23873

Open
ianlancetaylor opened this issue Feb 16, 2018 · 4 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

When the host or dig programs see a malformed packet from a resolver when using UDP, they fall back to using TCP. The net package resolver does not do this; it simply ignores the malformed packet (in (*dnsPacketConn).dnsRoundTrip in net/dnsclient_unix.go). This was done for #13281. I suggest that we do the same.

This is showing up right now for me when I run go test -test.run=TestLookupLongTXT net. I see this:

--- FAIL: TestLookupLongTXT (10.00s)
	lookup_test.go:334: lookup golang.rsc.io on 127.0.0.1:53: read udp 127.0.0.1:39779->127.0.0.1:53: i/o timeout
FAIL
FAIL	net	10.023s

If I run dig -t txt golang.rsc.io the output starts with

;; Warning: Message parser reports malformed message packet.
;; Truncated, retrying in TCP mode.

I suggest that we keep the current behavior for the !resp.IsResponseTo(query) case but change the behavior for an Unpack failure to drop right back to TCP.

CC @mdempsky

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 16, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Feb 16, 2018
@mikioh mikioh changed the title net: don't keep reading from UDP resolver after malformed packet net: don't keep reading from UDP resolver after truncated packet Feb 17, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@ianlancetaylor
Copy link
Contributor Author

This should be re-checked given the work done for #21160.

@ianlancetaylor
Copy link
Contributor Author

See also #22857.

@ianlancetaylor
Copy link
Contributor Author

CC @mikioh @iangudger

@iangudger
Copy link
Contributor

With the new DNS client, the chances of truncated DNS messages causing problems is greatly reduced, not not eliminated. I believe that now in cases where not all answers are contained in the read UDP message we will either try another DNS server or error out. I agree that trying TCP would be better, but at least failing fast is better than waiting for a timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants