From 8ce0afdb2bb709c0fa77e4314831716c158841c8 Mon Sep 17 00:00:00 2001 From: jfbus Date: Mon, 7 Jan 2019 08:30:58 +0100 Subject: [PATCH 1/3] net: use DNS over TCP when use-vc is set in resolv.conf Fixes #29358 --- src/net/dnsclient_unix.go | 12 ++++++++--- src/net/dnsclient_unix_test.go | 33 ++++++++++++++++++++++++++--- src/net/dnsconfig_unix.go | 3 +++ src/net/dnsconfig_unix_test.go | 11 ++++++++++ src/net/testdata/use-vc-resolv.conf | 1 + 5 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 src/net/testdata/use-vc-resolv.conf diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go index 4e7462b66fac48..4996f96e80c006 100644 --- a/src/net/dnsclient_unix.go +++ b/src/net/dnsclient_unix.go @@ -131,13 +131,19 @@ func dnsStreamRoundTrip(c Conn, id uint16, query dnsmessage.Question, b []byte) } // exchange sends a query on the connection and hopes for a response. -func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Question, timeout time.Duration) (dnsmessage.Parser, dnsmessage.Header, error) { +func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Question, timeout time.Duration, usetcp bool) (dnsmessage.Parser, dnsmessage.Header, error) { q.Class = dnsmessage.ClassINET id, udpReq, tcpReq, err := newRequest(q) if err != nil { return dnsmessage.Parser{}, dnsmessage.Header{}, errCannotMarshalDNSMessage } - for _, network := range []string{"udp", "tcp"} { + var networks []string + if usetcp { + networks = []string{"tcp"} + } else { + networks = []string{"udp", "tcp"} + } + for _, network := range networks { ctx, cancel := context.WithDeadline(ctx, time.Now().Add(timeout)) defer cancel() @@ -241,7 +247,7 @@ func (r *Resolver) tryOneName(ctx context.Context, cfg *dnsConfig, name string, for j := uint32(0); j < sLen; j++ { server := cfg.servers[(serverOffset+j)%sLen] - p, h, err := r.exchange(ctx, server, q, cfg.timeout) + p, h, err := r.exchange(ctx, server, q, cfg.timeout, cfg.usetcp) if err != nil { dnsErr := &DNSError{ Err: err.Error(), diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index 51d54a4ccadf22..074b9a7ee901db 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -81,7 +81,7 @@ func TestDNSTransportFallback(t *testing.T) { for _, tt := range dnsTransportFallbackTests { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - _, h, err := r.exchange(ctx, tt.server, tt.question, time.Second) + _, h, err := r.exchange(ctx, tt.server, tt.question, time.Second, false) if err != nil { t.Error(err) continue @@ -137,7 +137,7 @@ func TestSpecialDomainName(t *testing.T) { for _, tt := range specialDomainNameTests { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - _, h, err := r.exchange(ctx, server, tt.question, 3*time.Second) + _, h, err := r.exchange(ctx, server, tt.question, 3*time.Second, false) if err != nil { t.Error(err) continue @@ -1564,7 +1564,7 @@ func TestDNSDialTCP(t *testing.T) { } r := Resolver{PreferGo: true, Dial: fake.DialContext} ctx := context.Background() - _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second) + _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, false) if err != nil { t.Fatal("exhange failed:", err) } @@ -1695,3 +1695,30 @@ func TestSingleRequestLookup(t *testing.T) { } } } + +// Issue 29358. Add configuration knob to force TCP-only DNS requests in the pure Go resolver. +func TestDNSUseTCP(t *testing.T) { + fake := fakeDNSServer{ + rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) { + r := dnsmessage.Message{ + Header: dnsmessage.Header{ + ID: q.Header.ID, + Response: true, + RCode: dnsmessage.RCodeSuccess, + }, + Questions: q.Questions, + } + if n == "udp" { + t.Fatal("udp protocol was used instead of tcp") + } + return r, nil + }, + } + r := Resolver{PreferGo: true, Dial: fake.DialContext} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, true) + if err != nil { + t.Fatal("exchange failed:", err) + } +} diff --git a/src/net/dnsconfig_unix.go b/src/net/dnsconfig_unix.go index 3ca8d71f5f9f2e..732404b6f29101 100644 --- a/src/net/dnsconfig_unix.go +++ b/src/net/dnsconfig_unix.go @@ -33,6 +33,7 @@ type dnsConfig struct { mtime time.Time // time of resolv.conf modification soffset uint32 // used by serverOffset singleRequest bool // use sequential A and AAAA queries instead of parallel queries + usetcp bool // force usage of TCP for DNS resolutions } // See resolv.conf(5) on a Linux machine. @@ -123,6 +124,8 @@ func dnsReadConfig(filename string) *dnsConfig { // This option disables the behavior and makes glibc // perform the IPv6 and IPv4 requests sequentially." conf.singleRequest = true + case s == "use-vc": + conf.usetcp = true default: conf.unknownOpt = true } diff --git a/src/net/dnsconfig_unix_test.go b/src/net/dnsconfig_unix_test.go index f16f90ad505309..3791a772734f66 100644 --- a/src/net/dnsconfig_unix_test.go +++ b/src/net/dnsconfig_unix_test.go @@ -124,6 +124,17 @@ var dnsReadConfigTests = []struct { search: []string{"domain.local."}, }, }, + { + name: "testdata/use-vc-resolv.conf", + want: &dnsConfig{ + servers: defaultNS, + ndots: 1, + usetcp: true, + timeout: 5 * time.Second, + attempts: 2, + search: []string{"domain.local."}, + }, + }, } func TestDNSReadConfig(t *testing.T) { diff --git a/src/net/testdata/use-vc-resolv.conf b/src/net/testdata/use-vc-resolv.conf new file mode 100644 index 00000000000000..4e4a58b7a7e0e8 --- /dev/null +++ b/src/net/testdata/use-vc-resolv.conf @@ -0,0 +1 @@ +options use-vc \ No newline at end of file From 971eb363f6015c384f204765ce5e9283ac1ff986 Mon Sep 17 00:00:00 2001 From: jfbus Date: Mon, 7 Jan 2019 21:52:45 +0100 Subject: [PATCH 2/3] code review fixes --- src/net/dnsclient_unix.go | 12 +++++++++--- src/net/dnsclient_unix_test.go | 8 ++++---- src/net/dnsconfig_unix.go | 8 ++++++-- src/net/dnsconfig_unix_test.go | 2 +- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go index 4996f96e80c006..7ed4ea8708c2c2 100644 --- a/src/net/dnsclient_unix.go +++ b/src/net/dnsclient_unix.go @@ -26,6 +26,12 @@ import ( "golang.org/x/net/dns/dnsmessage" ) +const ( + // to be used as a useTCP parameter to exchange + useTCPOnly = true + useUDPOrTCP = false +) + var ( errLameReferral = errors.New("lame referral") errCannotUnmarshalDNSMessage = errors.New("cannot unmarshal DNS message") @@ -131,14 +137,14 @@ func dnsStreamRoundTrip(c Conn, id uint16, query dnsmessage.Question, b []byte) } // exchange sends a query on the connection and hopes for a response. -func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Question, timeout time.Duration, usetcp bool) (dnsmessage.Parser, dnsmessage.Header, error) { +func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Question, timeout time.Duration, useTCP bool) (dnsmessage.Parser, dnsmessage.Header, error) { q.Class = dnsmessage.ClassINET id, udpReq, tcpReq, err := newRequest(q) if err != nil { return dnsmessage.Parser{}, dnsmessage.Header{}, errCannotMarshalDNSMessage } var networks []string - if usetcp { + if useTCP { networks = []string{"tcp"} } else { networks = []string{"udp", "tcp"} @@ -247,7 +253,7 @@ func (r *Resolver) tryOneName(ctx context.Context, cfg *dnsConfig, name string, for j := uint32(0); j < sLen; j++ { server := cfg.servers[(serverOffset+j)%sLen] - p, h, err := r.exchange(ctx, server, q, cfg.timeout, cfg.usetcp) + p, h, err := r.exchange(ctx, server, q, cfg.timeout, cfg.useTCP) if err != nil { dnsErr := &DNSError{ Err: err.Error(), diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index 074b9a7ee901db..f1ed58c837d856 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -81,7 +81,7 @@ func TestDNSTransportFallback(t *testing.T) { for _, tt := range dnsTransportFallbackTests { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - _, h, err := r.exchange(ctx, tt.server, tt.question, time.Second, false) + _, h, err := r.exchange(ctx, tt.server, tt.question, time.Second, useUDPOrTCP) if err != nil { t.Error(err) continue @@ -137,7 +137,7 @@ func TestSpecialDomainName(t *testing.T) { for _, tt := range specialDomainNameTests { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - _, h, err := r.exchange(ctx, server, tt.question, 3*time.Second, false) + _, h, err := r.exchange(ctx, server, tt.question, 3*time.Second, useUDPOrTCP) if err != nil { t.Error(err) continue @@ -1564,7 +1564,7 @@ func TestDNSDialTCP(t *testing.T) { } r := Resolver{PreferGo: true, Dial: fake.DialContext} ctx := context.Background() - _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, false) + _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, useUDPOrTCP) if err != nil { t.Fatal("exhange failed:", err) } @@ -1717,7 +1717,7 @@ func TestDNSUseTCP(t *testing.T) { r := Resolver{PreferGo: true, Dial: fake.DialContext} ctx, cancel := context.WithCancel(context.Background()) defer cancel() - _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, true) + _, _, err := r.exchange(ctx, "0.0.0.0", mustQuestion("com.", dnsmessage.TypeALL, dnsmessage.ClassINET), time.Second, useTCPOnly) if err != nil { t.Fatal("exchange failed:", err) } diff --git a/src/net/dnsconfig_unix.go b/src/net/dnsconfig_unix.go index 732404b6f29101..e72bca6ff186b5 100644 --- a/src/net/dnsconfig_unix.go +++ b/src/net/dnsconfig_unix.go @@ -33,7 +33,7 @@ type dnsConfig struct { mtime time.Time // time of resolv.conf modification soffset uint32 // used by serverOffset singleRequest bool // use sequential A and AAAA queries instead of parallel queries - usetcp bool // force usage of TCP for DNS resolutions + useTCP bool // force usage of TCP for DNS resolutions } // See resolv.conf(5) on a Linux machine. @@ -125,7 +125,11 @@ func dnsReadConfig(filename string) *dnsConfig { // perform the IPv6 and IPv4 requests sequentially." conf.singleRequest = true case s == "use-vc": - conf.usetcp = true + // Linux glibc option: + // http://man7.org/linux/man-pages/man5/resolv.conf.5.html + // "Sets RES_USEVC in _res.options. + // This option forces the use of TCP for DNS resolutions." + conf.useTCP = true default: conf.unknownOpt = true } diff --git a/src/net/dnsconfig_unix_test.go b/src/net/dnsconfig_unix_test.go index 3791a772734f66..e6ba5fe4a36528 100644 --- a/src/net/dnsconfig_unix_test.go +++ b/src/net/dnsconfig_unix_test.go @@ -129,7 +129,7 @@ var dnsReadConfigTests = []struct { want: &dnsConfig{ servers: defaultNS, ndots: 1, - usetcp: true, + useTCP: true, timeout: 5 * time.Second, attempts: 2, search: []string{"domain.local."}, From 70bc00fe41f44f0b2b3cfebe67bbcc45701968cf Mon Sep 17 00:00:00 2001 From: jfbus Date: Tue, 8 Jan 2019 22:41:52 +0100 Subject: [PATCH 3/3] useTCP: OpenBSD & FreeBSD support --- src/net/dnsconfig_unix.go | 6 +++-- src/net/dnsconfig_unix_test.go | 24 ++++++++++++++++++- src/net/testdata/freebsd-usevc-resolv.conf | 1 + ...c-resolv.conf => linux-use-vc-resolv.conf} | 0 src/net/testdata/openbsd-tcp-resolv.conf | 1 + 5 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 src/net/testdata/freebsd-usevc-resolv.conf rename src/net/testdata/{use-vc-resolv.conf => linux-use-vc-resolv.conf} (100%) create mode 100644 src/net/testdata/openbsd-tcp-resolv.conf diff --git a/src/net/dnsconfig_unix.go b/src/net/dnsconfig_unix.go index e72bca6ff186b5..877e77c0493d26 100644 --- a/src/net/dnsconfig_unix.go +++ b/src/net/dnsconfig_unix.go @@ -124,11 +124,13 @@ func dnsReadConfig(filename string) *dnsConfig { // This option disables the behavior and makes glibc // perform the IPv6 and IPv4 requests sequentially." conf.singleRequest = true - case s == "use-vc": - // Linux glibc option: + case s == "use-vc" || s == "usevc" || s == "tcp": + // Linux (use-vc), FreeBSD (usevc) and OpenBSD (tcp) option: // http://man7.org/linux/man-pages/man5/resolv.conf.5.html // "Sets RES_USEVC in _res.options. // This option forces the use of TCP for DNS resolutions." + // https://www.freebsd.org/cgi/man.cgi?query=resolv.conf&sektion=5&manpath=freebsd-release-ports + // https://man.openbsd.org/resolv.conf.5 conf.useTCP = true default: conf.unknownOpt = true diff --git a/src/net/dnsconfig_unix_test.go b/src/net/dnsconfig_unix_test.go index e6ba5fe4a36528..42880123c52d2f 100644 --- a/src/net/dnsconfig_unix_test.go +++ b/src/net/dnsconfig_unix_test.go @@ -125,7 +125,29 @@ var dnsReadConfigTests = []struct { }, }, { - name: "testdata/use-vc-resolv.conf", + name: "testdata/linux-use-vc-resolv.conf", + want: &dnsConfig{ + servers: defaultNS, + ndots: 1, + useTCP: true, + timeout: 5 * time.Second, + attempts: 2, + search: []string{"domain.local."}, + }, + }, + { + name: "testdata/freebsd-usevc-resolv.conf", + want: &dnsConfig{ + servers: defaultNS, + ndots: 1, + useTCP: true, + timeout: 5 * time.Second, + attempts: 2, + search: []string{"domain.local."}, + }, + }, + { + name: "testdata/openbsd-tcp-resolv.conf", want: &dnsConfig{ servers: defaultNS, ndots: 1, diff --git a/src/net/testdata/freebsd-usevc-resolv.conf b/src/net/testdata/freebsd-usevc-resolv.conf new file mode 100644 index 00000000000000..4afb281c5bbf22 --- /dev/null +++ b/src/net/testdata/freebsd-usevc-resolv.conf @@ -0,0 +1 @@ +options usevc \ No newline at end of file diff --git a/src/net/testdata/use-vc-resolv.conf b/src/net/testdata/linux-use-vc-resolv.conf similarity index 100% rename from src/net/testdata/use-vc-resolv.conf rename to src/net/testdata/linux-use-vc-resolv.conf diff --git a/src/net/testdata/openbsd-tcp-resolv.conf b/src/net/testdata/openbsd-tcp-resolv.conf new file mode 100644 index 00000000000000..7929e50e8df037 --- /dev/null +++ b/src/net/testdata/openbsd-tcp-resolv.conf @@ -0,0 +1 @@ +options tcp \ No newline at end of file