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

Embedded dns does not return compressed results. #20132

Closed
clinta opened this issue Feb 9, 2016 · 6 comments · Fixed by #20181
Closed

Embedded dns does not return compressed results. #20132

clinta opened this issue Feb 9, 2016 · 6 comments · Fixed by #20181
Assignees
Labels
priority/P1 Important: P1 issues are a top priority and a must-have for the next release.
Milestone

Comments

@clinta
Copy link

clinta commented Feb 9, 2016

Continuation of #20037. The embedded DNS server does not compress dns responses. This causes issues for many apps, including docker itself (via the dind image) and any other go applications using the net go library.

A test image is available which can reproduce the issue:

$ docker run clinta/go-dns-test
[Tested at 2016-02-08 21:24:13 UTC 192.96.176.129 sent EDNS buffer size 4096 192.96.176.129 DNS reply size limit is at least 4090]%

$ docker run --net=test clinta/go-dns-test
lookup rs.dns-oarc.net on 127.0.0.11:53: no such host
[]%

@mavenugo, I'm not running in AWS. This is all a bare metal deployment. I think reproducing it depends on your upstream resolver. It appears that google's public servers, 8.8.8.8 and 8.8.4.4 do not return large dns records. DNSmasq may not by default either, I am still testing that. A recursive bind server will.

@mavenugo
Copy link
Contributor

mavenugo commented Feb 9, 2016

@clinta thanks. as per miekg/dns#216 (comment) the fix is miekg/dns@44df365. We have to test it out and provide feedback before we can vendor-in the fix.

@clinta
Copy link
Author

clinta commented Feb 9, 2016

If you do a dig +bufsize=4096 rs.dns-oarc.net txt and get back a message that's less than 512 bytes, then you can be sure your upstream resolver is ignoring your edns options and sending back truncated responses.

You can do a $ dig +trace +bufsize=4096 rs.dns-oarc.net txt to see what the size of the pacekt should be, 4090 bytes.

@mavenugo
Copy link
Contributor

mavenugo commented Feb 9, 2016

yes. I could see the truncated responses when using the 8.8.8.8 as the DNS server and with +trace, I can see the size as 4090 bytes.
Do you think having the compression properly done is the best solution for this problem ? What if the compressed data is > 512 bytes ? Should the client be robust enough to handle such responses ? (switch to TCP, etc.) ?

@clinta
Copy link
Author

clinta commented Feb 9, 2016

Since RFC 1035 states "all programs are required to understand arriving messages that
contain pointers." it should be safe to enable compression all the time. However I think the best option would be for Docker to always return the response as it is received from the upstream resolver, compressed if the data from upstream is compressed, not compressed if it's not.

Responses larger than 512 bytes are only allowed if edns is supported. So long as Docker's resolver passes the edns flag as it was received from the container, everything should work fine in terms of responses that are larger than 512 bytes compressed.

If the application does not support edns, like Go's net dns library, it will not set the edns flag in it's request. So long as Docker's resolver passes that request and does not modify the flag, the upstream resolver will know that it can not send back more than 512 bytes. Whether it sends the response compressed or not is up to the resolver, but it cannot exceed 512 bytes on the wire.

If the full response, even compressed, is larger than 512 bytes, the upstream server will know that it must truncate the response to 512 byes on the wire because there was no edns flag in the request indicating a larger supported size. Docker's should pass on this response as received, especially making sure to preserve the TC flag which indicates to the requesting application that the DNS response was truncated. The application may choose to retry the query over TCP, or it may use the truncated response. But this logic is all up to the application. Docker's DNS resolver should not make any assumptions on behalf of the client application. If Docker's DNS resolver receives a retry via TCP from the app, it should pass it on to the upstream resolver via TCP.

If the application does support edns and indicates that it can support a larger response (for example dig +bufsize=4096, then Docker should pass that flag as it is received so the upstream resolver will know that it can send a response up to that size. To compress the response or not is up to the resolver, so long as the final message is under the required size when it hits the wire. And so long as Docker passes back the response as it is received it will be within the maximum size supported by the requesting application.

@rade
Copy link

rade commented Feb 9, 2016

@mavenugo you are welcome to copy the Weave DNS code. It is short, deals with all the problems identified by @clinta (truncation, compression, edns), and has been in production use for months.

@mavenugo
Copy link
Contributor

mavenugo commented Feb 9, 2016

@rade thanks for the pointer. @sanimej is working on to fix this in https://github.com/docker/libnetwork/blob/master/resolver.go#L230. If you wish to contribute, pls do.

Since most of the DNS implementation uses miekg/dns, it would have been better to contribute such fixes upstream so that everyone benefits. Since it is already resolved in "compression" branch, we can work upstream to get that in. Also consul took a different approach of compressing all the dns messages : hashicorp/consul@08b7313

@tiborvass tiborvass added this to the 1.10.1 milestone Feb 10, 2016
@tiborvass tiborvass added priority/P2 Normal priority: default priority applied. priority/P1 Important: P1 issues are a top priority and a must-have for the next release. and removed priority/P2 Normal priority: default priority applied. labels Feb 10, 2016
@mavenugo mavenugo reopened this Feb 10, 2016
mavenugo added a commit to mavenugo/docker that referenced this issue Feb 10, 2016
- Fixes moby#20132 moby#20140 moby#20019

Signed-off-by: Madhu Venugopal <madhu@docker.com>
tiborvass pushed a commit to tiborvass/docker that referenced this issue Feb 10, 2016
- Fixes moby#20132 moby#20140 moby#20019

Signed-off-by: Madhu Venugopal <madhu@docker.com>
(cherry picked from commit 84705f1)

From PR moby#20181
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/P1 Important: P1 issues are a top priority and a must-have for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants