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: target domain names in SRV records should not be compressed #10622

Closed
mdempsky opened this issue Apr 29, 2015 · 4 comments
Closed

net: target domain names in SRV records should not be compressed #10622

mdempsky opened this issue Apr 29, 2015 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Member

The current spec for SRV records (RFC 2782) says "Target: The domain name of the target host. [...] Unless and until permitted by future standards action, name compression is not to be used for this field."

The main way I've seen this manifest as a problem (not with Go specifically) is:

  1. An authoritative DNS server (non-compliantly) sends an SRV RR using domain name compression.
  2. The SRV RR is (compliantly) cached by an intermediate DNS server as an opaque byte string without applying uncompression.
  3. The cached SRV RR is sent in a response to a DNS stub client that (non-compliantly) tries to apply name uncompression.

This fails because the client interprets the compressed domain name pointers as offsets into the intermediate server's DNS message, but they were actually computed according to the authoritative server's DNS message. With good luck, the client will notice they're obviously bogus and reject them; but it's also possible the pointers happen to look valid and the client ends up with subtly-bogus SRV records.

@mikioh mikioh added this to the Go1.5Maybe milestone May 2, 2015
@rsc
Copy link
Contributor

rsc commented Jul 15, 2015

Fine to fix, but not a release blocker.

@gopherbot
Copy link

CL https://golang.org/cl/35237 mentions this issue.

gopherbot pushed a commit to golang/net that referenced this issue Mar 3, 2017
The Go standard library contains support for packing and unpacking of
DNS messages, but it is not exported, doesn't follow Go style, and is
not very well optimized. Low level DNS functionality is clearly useful
to the Go community as evidenced by the success of
github.com/miekg/dns. This implementation endeavors to avoid the
limitations of both the standard library and github.com/miekg/dns
implementations and is an almost complete rewrite of the code
currently found in on net/dnsmsg.go and net/dnsmsg_test.go.

Goals:
* Minimize heap allocations.
* Allow parsing only what is needed. Avoid unnecessary parsing and
  heap allocations for parts of the message that you don't care about.
  Parsing should be allowed on as small of a granularity as is useful,
  but no smaller as to avoid complicating the interface.
* Parse and pack each byte of the message at most one time.

Updates golang/go#16218
Updates golang/go#10622

Change-Id: Ib754d0007609a617d88be867f21c2feb15b6fcd7
Reviewed-on: https://go-review.googlesource.com/35237
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/100055 mentions this issue: dns/dnsmessage: reject compressed SRV resource records

@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 13, 2018
gopherbot pushed a commit to golang/net that referenced this issue Mar 17, 2018
Updates golang/go#10622

Change-Id: Iadf0ff0fd223a315130941464040aef5e71f6130
Reviewed-on: https://go-review.googlesource.com/100055
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/101278 mentions this issue: vendor: update golang.org/x/net/dns/dnsmessage from upstream

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants