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
x/net/dns: invalid assumptions about domain names and character strings #24288
Comments
/cc @bradfitz @iangudger |
x/net/dns/dnsmessage is functionally based on the code currently in the net package. I don't believe any of these are new problems. x/net/dns/dnsmessage is mostly focused on improving performance, but does contain some bug fixes. As far as I know, it is a strict improvement correctness-wise. That said, I agree that these are problems and should be fixed. @gibson042, I have seen your golang.org/cl/73830. @mikioh requested that you port your changes to x/net/dns/dnsmessage. Is filing this issue your way of saying that you want me to port golang.org/cl/73830 to x/net/dns/dnsmessage before submitting golang.org/cl/37879? |
If you are able to do so, that would be great. It wouldn't address the issues with character-strings RDATA, but those are logically separate from domain names anyway—I just wanted to get the issue documented more visibly. And thank you for taking these issues seriously, even though they are rarely-encountered edge cases. |
Change https://golang.org/cl/99623 mentions this issue: |
Change https://golang.org/cl/100196 mentions this issue: |
Previously, we only accepted a single string for TXT records and then chunked it into 255 byte segments. This is wrong. TXT records are a sequence of strings, each up to 255 bytes. Updates golang/go#24288 Change-Id: Ib2c085ec127ccecf0c7bda930100b667cabc1f4b Reviewed-on: https://go-review.googlesource.com/100196 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
What version of Go are you using (
go version
)?go version go1.10 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What did you do?
Reviewed go cl 37879 and dns/dnsmessage/message.go as of 2018-03-06.
What did you expect to see?
Correct handling of DNS data.
What did you see instead?
func (n *Name) pack
incorrectly assumes that terminal dots imply absolute names, falsely accepting names that end with escaped dots (e.g.,example\.
).func (n *Name) unpack
assumes that label contents can be directly copied from wire format to presentation format, and therefore incorrectly converts dots in labels to label-separating dots (e.g., equating the distinct namesa.b.example.
[1 0x61 1 0x62 7 0x65 0x78 0x61 0x6d 0x70 0x6c 0x65 0] anda\.b.example.
[3 0x61 0x2E 0x62 7 0x65 0x78 0x61 0x6d 0x70 0x6c 0x65 0]) and produces invalid domain names by naïvely including backslashes and control characters.type TXTResource
incorrectly assumes that TXT RDATA is a single string (when it reality it is a sequence of character strings 1, each of which is limited to 255 octets in wire format 2), making it impossible to differentiateTXT "" ""
(two empty character strings) fromTXT ""
(one empty character string). The associated pack/unpack code also fails to reject overly-long strings (e.g., "aa-ab-ac-…-az-ba-bb-bc-…-zx-zy-zz"), instead naïvely combining/splitting 255-octet chunks.The text was updated successfully, but these errors were encountered: