diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 10854f802..6178e26fe 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -194,8 +194,14 @@ func (f *forwarder) recv(conn *fwdConn) { return default: } - out := make([]byte, maxResponseBytes) + // The 1 extra byte is to detect packet truncation. + out := make([]byte, maxResponseBytes+1) n := conn.read(out) + var truncated bool + if n > maxResponseBytes { + n = maxResponseBytes + truncated = true + } if n == 0 { continue } @@ -206,6 +212,19 @@ func (f *forwarder) recv(conn *fwdConn) { out = out[:n] txid := getTxID(out) + if truncated { + const dnsFlagTruncated = 0x200 + flags := binary.BigEndian.Uint16(out[2:4]) + flags |= dnsFlagTruncated + binary.BigEndian.PutUint16(out[2:4], flags) + + // TODO(#2067): Remove any incomplete records? RFC 1035 section 6.2 + // states that truncation should head drop so that the authority + // section can be preserved if possible. However, the UDP read with + // a too-small buffer has already dropped the end, so that's the + // best we can do. + } + f.mu.Lock() record, found := f.txMap[txid] @@ -287,6 +306,8 @@ func (f *forwarder) forward(query packet) error { } f.mu.Unlock() + // TODO(#2066): EDNS size clamping + for _, resolver := range resolvers { f.send(query.bs, resolver) } @@ -429,7 +450,7 @@ func (c *fwdConn) read(out []byte) int { c.mu.Unlock() n, _, err := conn.ReadFrom(out) - if err == nil { + if err == nil || packetWasTruncated(err) { // Success. return n } diff --git a/net/dns/resolver/neterr_darwin.go b/net/dns/resolver/neterr_darwin.go index b1954648c..a5a8748b6 100644 --- a/net/dns/resolver/neterr_darwin.go +++ b/net/dns/resolver/neterr_darwin.go @@ -23,3 +23,8 @@ func networkIsDown(err error) bool { func networkIsUnreachable(err error) bool { return errors.Is(err, networkUnreachable) } + +// packetWasTruncated returns true if err indicates truncation but the RecvFrom +// that generated err was otherwise successful. It always returns false on this +// platform. +func packetWasTruncated(err error) bool { return false } diff --git a/net/dns/resolver/neterr_other.go b/net/dns/resolver/neterr_other.go index df802dcfb..236348b56 100644 --- a/net/dns/resolver/neterr_other.go +++ b/net/dns/resolver/neterr_other.go @@ -8,3 +8,8 @@ package resolver func networkIsDown(err error) bool { return false } func networkIsUnreachable(err error) bool { return false } + +// packetWasTruncated returns true if err indicates truncation but the RecvFrom +// that generated err was otherwise successful. It always returns false on this +// platform. +func packetWasTruncated(err error) bool { return false } diff --git a/net/dns/resolver/neterr_windows.go b/net/dns/resolver/neterr_windows.go index 043d19477..7d0d2fb15 100644 --- a/net/dns/resolver/neterr_windows.go +++ b/net/dns/resolver/neterr_windows.go @@ -5,6 +5,7 @@ package resolver import ( + "errors" "net" "os" @@ -27,3 +28,16 @@ func networkIsUnreachable(err error) bool { // difference between down and unreachable? Add comments. return false } + +// packetWasTruncated returns true if err indicates truncation but the RecvFrom +// that generated err was otherwise successful. On Windows, Go's UDP RecvFrom +// calls WSARecvFrom which returns the WSAEMSGSIZE error code when the received +// datagram is larger than the provided buffer. When that happens, both a valid +// size and an error are returned (as per the partial fix for golang/go#14074). +// If the WSAEMSGSIZE error is returned, then we ignore the error to get +// semantics similar to the POSIX operating systems. One caveat is that it +// appears that the source address is not returned when WSAEMSGSIZE occurs, but +// we do not currently look at the source address. +func packetWasTruncated(err error) bool { + return errors.Is(err, windows.WSAEMSGSIZE) +} diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index ff51b29cb..105efa720 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -22,8 +22,10 @@ import ( "tailscale.com/wgengine/monitor" ) -// maxResponseBytes is the maximum size of a response from a Resolver. -const maxResponseBytes = 512 +// maxResponseBytes is the maximum size of a response from a Resolver. The +// actual buffer size will be one larger than this so that we can detect +// truncation in a platform-agnostic way. +const maxResponseBytes = 4095 // queueSize is the maximal number of DNS requests that can await polling. // If EnqueueRequest is called when this many requests are already pending, diff --git a/net/dns/resolver/tsdns_server_test.go b/net/dns/resolver/tsdns_server_test.go index bad2fedc0..71a31eba2 100644 --- a/net/dns/resolver/tsdns_server_test.go +++ b/net/dns/resolver/tsdns_server_test.go @@ -66,6 +66,39 @@ func resolveToIP(ipv4, ipv6 netaddr.IP, ns string) dns.HandlerFunc { } } +// resolveToTXT returns a handler function which responds to queries of type TXT +// it receives with the strings in txts. +func resolveToTXT(txts []string) dns.HandlerFunc { + return func(w dns.ResponseWriter, req *dns.Msg) { + m := new(dns.Msg) + m.SetReply(req) + + if len(req.Question) != 1 { + panic("not a single-question request") + } + question := req.Question[0] + + if question.Qtype != dns.TypeTXT { + w.WriteMsg(m) + return + } + + ans := &dns.TXT{ + Hdr: dns.RR_Header{ + Name: question.Name, + Rrtype: dns.TypeTXT, + Class: dns.ClassINET, + }, + Txt: txts, + } + + m.Answer = append(m.Answer, ans) + if err := w.WriteMsg(m); err != nil { + panic(err) + } + } +} + var resolveToNXDOMAIN = dns.HandlerFunc(func(w dns.ResponseWriter, req *dns.Msg) { m := new(dns.Msg) m.SetRcode(req, dns.RcodeNameError) diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 02ef3108b..8efe02d1e 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -6,7 +6,9 @@ package resolver import ( "bytes" + "encoding/hex" "errors" + "math/rand" "net" "testing" @@ -44,9 +46,11 @@ func dnspacket(domain dnsname.FQDN, tp dns.Type) []byte { } type dnsResponse struct { - ip netaddr.IP - name dnsname.FQDN - rcode dns.RCode + ip netaddr.IP + txt []string + name dnsname.FQDN + rcode dns.RCode + truncated bool } func unpackResponse(payload []byte) (dnsResponse, error) { @@ -67,6 +71,16 @@ func unpackResponse(payload []byte) (dnsResponse, error) { return response, nil } + response.truncated = h.Truncated + if response.truncated { + // TODO(#2067): Ideally, answer processing should still succeed when + // dealing with a truncated message, but currently when we truncate + // a packet, it's caused by the buffer being too small and usually that + // means the data runs out mid-record. dns.Parser does not like it when + // that happens. We can improve this by trimming off incomplete records. + return response, nil + } + err = parser.SkipAllQuestions() if err != nil { return response, err @@ -90,6 +104,12 @@ func unpackResponse(payload []byte) (dnsResponse, error) { return response, err } response.ip = netaddr.IPv6Raw(res.AAAA) + case dns.TypeTXT: + res, err := parser.TXTResource() + if err != nil { + return response, err + } + response.txt = res.TXT case dns.TypeNS: res, err := parser.NSResource() if err != nil { @@ -269,6 +289,32 @@ func ipv6Works() bool { return true } +func generateTXT(size int, source rand.Source) []string { + const sizePerTXT = 120 + + if size%2 != 0 { + panic("even lengths only") + } + + rng := rand.New(source) + + txts := make([]string, 0, size/sizePerTXT+1) + + raw := make([]byte, sizePerTXT/2) + + rem := size + for ; rem > sizePerTXT; rem -= sizePerTXT { + rng.Read(raw) + txts = append(txts, hex.EncodeToString(raw)) + } + if rem > 0 { + rng.Read(raw[:rem/2]) + txts = append(txts, hex.EncodeToString(raw[:rem/2])) + } + + return txts +} + func TestDelegate(t *testing.T) { tstest.ResourceCheck(t) @@ -276,13 +322,43 @@ func TestDelegate(t *testing.T) { t.Skip("skipping test that requires localhost IPv6") } + randSource := rand.NewSource(4) + + // smallTXT does not require EDNS + smallTXT := generateTXT(300, randSource) + + // medTXT and largeTXT are responses that require EDNS but we would like to + // support these sizes of response without truncation because they are + // moderately common. + medTXT := generateTXT(1200, randSource) + largeTXT := generateTXT(4000, randSource) + + // xlargeTXT is slightly above the maximum response size that we support, + // so there should be truncation. + xlargeTXT := generateTXT(5000, randSource) + + // hugeTXT is significantly larger than any typical MTU and will require + // significant fragmentation. For buffer management reasons, we do not + // intend to handle responses this large, so there should be truncation. + hugeTXT := generateTXT(64000, randSource) + v4server := serveDNS(t, "127.0.0.1:0", "test.site.", resolveToIP(testipv4, testipv6, "dns.test.site."), - "nxdomain.site.", resolveToNXDOMAIN) + "nxdomain.site.", resolveToNXDOMAIN, + "small.txt.", resolveToTXT(smallTXT), + "med.txt.", resolveToTXT(medTXT), + "large.txt.", resolveToTXT(largeTXT), + "xlarge.txt.", resolveToTXT(xlargeTXT), + "huge.txt.", resolveToTXT(hugeTXT)) defer v4server.Shutdown() v6server := serveDNS(t, "[::1]:0", "test.site.", resolveToIP(testipv4, testipv6, "dns.test.site."), - "nxdomain.site.", resolveToNXDOMAIN) + "nxdomain.site.", resolveToNXDOMAIN, + "small.txt.", resolveToTXT(smallTXT), + "med.txt.", resolveToTXT(medTXT), + "large.txt.", resolveToTXT(largeTXT), + "xlarge.txt.", resolveToTXT(xlargeTXT), + "huge.txt.", resolveToTXT(hugeTXT)) defer v6server.Shutdown() r := New(t.Logf, nil) @@ -322,6 +398,31 @@ func TestDelegate(t *testing.T) { dnspacket("nxdomain.site.", dns.TypeA), dnsResponse{rcode: dns.RCodeNameError}, }, + { + "smalltxt", + dnspacket("small.txt.", dns.TypeTXT), + dnsResponse{txt: smallTXT, rcode: dns.RCodeSuccess}, + }, + { + "medtxt", + dnspacket("med.txt.", dns.TypeTXT), + dnsResponse{txt: medTXT, rcode: dns.RCodeSuccess}, + }, + { + "largetxt", + dnspacket("large.txt.", dns.TypeTXT), + dnsResponse{txt: largeTXT, rcode: dns.RCodeSuccess}, + }, + { + "xlargetxt", + dnspacket("xlarge.txt.", dns.TypeTXT), + dnsResponse{rcode: dns.RCodeSuccess, truncated: true}, + }, + { + "hugetxt", + dnspacket("huge.txt.", dns.TypeTXT), + dnsResponse{rcode: dns.RCodeSuccess, truncated: true}, + }, } for _, tt := range tests { @@ -345,6 +446,15 @@ func TestDelegate(t *testing.T) { if response.name != tt.response.name { t.Errorf("name = %v; want %v", response.name, tt.response.name) } + if len(response.txt) != len(tt.response.txt) { + t.Errorf("%v txt records, want %v txt records", len(response.txt), len(tt.response.txt)) + } else { + for i := range response.txt { + if response.txt[i] != tt.response.txt[i] { + t.Errorf("txt record %v is %s, want %s", i, response.txt[i], tt.response.txt[i]) + } + } + } }) } }