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: cannot resolve SRV records with compressed targets #36718

Closed
PaulForgey opened this issue Jan 24, 2020 · 15 comments
Closed

net: cannot resolve SRV records with compressed targets #36718

PaulForgey opened this issue Jan 24, 2020 · 15 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@PaulForgey
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.13.3 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ubuntu/.cache/go-build"
GOENV="/home/ubuntu/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ubuntu/mesa/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/src/go1.13.3/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/src/go1.13.3/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build085179277=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
	"context"
	"fmt"
	"net"
)

func main() {
	cname, addrs, err := net.DefaultResolver.LookupSRV(context.Background(), "kerberos", "udp", "mydc.example.com")
	if err != nil {
		panic(err)
	}
	fmt.Printf("cname=%s\n", cname)
	for _, a := range addrs {
		fmt.Printf("target=%s:\n", a.Target)
	}
}

Samba (at least 4.3.11-Ubuntu), compresses the Target fields of the SRV records. While this is against clarifying RFCs and there is much discussion of this in the patch enforcing this, it breaks resolver behavior at least against Samba and possibly other embedded servers.

IMO, this makes the resolver less robust. If the rationale is to defend against ignorant DNS proxies, I would argue the client is not the layer to make this decision.

What did you expect to see?

successful resolution

What did you see instead?

error "cannot unmarshal DNS message"

@toothrot toothrot changed the title cannot resolve SRV records from Samba's DNS server net: cannot resolve SRV records from Samba's DNS server Jan 24, 2020
@toothrot toothrot added this to the Backlog milestone Jan 24, 2020
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 24, 2020
@toothrot
Copy link
Contributor

That's pretty unfortunate that Samba doesn't follow the RFCs. Considering that oddity, I am not sure if this belongs in the core library or its own package. The owners should have a better idea. /cc @mikioh @bradfitz @ianlancetaylor

@bradfitz
Copy link
Contributor

/cc @mdempsky

@mdempsky
Copy link
Contributor

RFC 2782 clearly states "name compression is not to be used for [SRV's Target] field."

RFC 3597 though states we "SHOULD" support decompressing names in SRV RRs (as opposed to "MUST" for the RRs where we already support decompression).

I'd review a patch to enable decompression (but not compression) of SRV targets if someone wants to work on one. I think Samba should fix their behavior though.

@PaulForgey
Copy link
Contributor Author

I agree Samba should fix this, but it is but one example of an embedded server in the wild, and there is no good reason for a client to fail.

Commit 24dd3780ca4f75fed9f321890729414a4b5d3f13 explicitly did this, which IMO, should be reverted.

@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 24, 2020
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 24, 2020
@mdempsky
Copy link
Contributor

Commit 24dd3780ca4f75fed9f321890729414a4b5d3f13 explicitly did this

I don't see that commit in Go's git history?

@PaulForgey
Copy link
Contributor Author

@bradfitz
Copy link
Contributor

@mdempsky
Copy link
Contributor

Ah, thanks. That looks safe to revert.

I'd appreciate some confirmation that other major DNS clients are today accepting SRV RRs. (I assume they are, otherwise I'd expect more pushback against Samba doing this.)

@PaulForgey
Copy link
Contributor Author

ISC's BIND at the very least does, as dig has no problem resolving these records. This is a common reference implementation.

@mdempsky
Copy link
Contributor

Okay, reverting that CL seems fine to me then once the trees open up again for development. Thanks.

@divjotarora
Copy link

divjotarora commented Apr 11, 2020

We have a number of users hitting this issue when using SRV records with the go.mongodb.org/mongo-driver library and the only good responses we have for them so far are to use non-SRV URIs (for resilience purposes, this requires specifying a very long URI that's hard to read) or using a different DNS server, which many applications can't always control. Given that neither of these solutions are particularly good, can anyone give a timeline for when the change linked above might be reverted or provide another solution that we can forward to our users?

@prattmic
Copy link
Member

Apple's vmnet framework runs a DNS server at the gateway address which seems to have this same behavior.

Running a macOS guest with this interface (specifically, under qemu with -netdev vmnet-shared), causes TestLookupGoogleSRV to fail. Editing goLookupSRV to report the underlying error:

$ ../bin/go test -run=TestLookupGoogleSRV -v net 
=== RUN   TestLookupGoogleSRV
=== PAUSE TestLookupGoogleSRV
=== CONT  TestLookupGoogleSRV
    lookup_test.go:92: backoff 1s after failure lookup google.com on 192.168.64.1:53: cannot unmarshal DNS message: Target: compressed name in SRV resource data
    lookup_test.go:92: backoff 5s after failure lookup google.com on 192.168.64.1:53: cannot unmarshal DNS message: Target: compressed name in SRV resource data
    lookup_test.go:92: backoff 30s after failure lookup google.com on 192.168.64.1:53: cannot unmarshal DNS message: Target: compressed name in SRV resource data
    lookup_test.go:98: lookup google.com on 192.168.64.1:53: cannot unmarshal DNS message: Target: compressed name in SRV resource data
--- FAIL: TestLookupGoogleSRV (36.02s)
FAIL
Socket statistical information:
(inet4, datagram, default): opened=4 connected=4 listened=0 accepted=0 closed=4 openfailed=0 connectfailed=0 listenfailed=0 acceptfailed=0 closefailed=0

FAIL    net     36.044s
FAIL

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/442255 mentions this issue: env/darwin/aws: switch to vmnet-shared networking

gopherbot pushed a commit to golang/build that referenced this issue Oct 11, 2022
This networking mode, which uses a virtualization networking API from
Apple, appears to be more stable than the default userspace networking.

Multiple guests will share the same subnet, so they must have different
MAC addresses.

Apple's API seems to run an DNS server that serves compressed SRV
records, which Go does not support (golang/go#36718) and thus causes a
net test to fail. Until that is resolved, we must manually specify a
custom DNS server.

For golang/go#48945.
Updates golang/go#36718.

Change-Id: Ibc5b91ed1456d1364975febe83fd282c42bd6ed1
Reviewed-on: https://go-review.googlesource.com/c/build/+/442255
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/540375 mentions this issue: dns/dnsmessage: allow name compression for SRV resource parsing

gopherbot pushed a commit to golang/net that referenced this issue Feb 3, 2024
As per RFC 3597:

Receiving servers MUST decompress domain names in RRs of well-known
   type, and SHOULD also decompress RRs of type RP, AFSDB, RT, SIG, PX,
   NXT, NAPTR, and SRV (although the current specification of the SRV RR
   in RFC2782 prohibits compression, RFC2052 mandated it, and some
   servers following that earlier specification are still in use).

This change allows SRV resource decompression.

Updates golang/go#36718
Updates golang/go#37362

Change-Id: I473c0d3803758e5b12886f378d2ed54bd5392144
GitHub-Last-Rev: 88d2e06
GitHub-Pull-Request: #199
Reviewed-on: https://go-review.googlesource.com/c/net/+/540375
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
@mateusz834
Copy link
Member

As of https://go.dev/cl/570156 this is now fixed.

@dmitshur dmitshur modified the milestones: Backlog, Go1.23 Mar 9, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

9 participants