net/dns/resolver: permit larger max responses, signal truncation

This raises the maximum DNS response message size from 512 to 4095. This
should be large enough for almost all situations that do not need TCP.
We still do not recognize EDNS, so we will still forward requests that
claim support for a larger response size than 4095 (that will be solved
later). For now, when a response comes back that is too large to fit in
our receive buffer, we now set the truncation flag in the DNS header,
which is an improvement from before but will prompt attempts to use TCP
which isn't supported yet.

On Windows, WSARecvFrom into a buffer that's too small returns an error
in addition to the data. On other OSes, the extra data is silently
discarded. In this case, we prefer the latter so need to catch the error
on Windows.

Partially addresses #1123

Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>

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
 		}

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 }

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 }

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)
+}

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.
+// maxResponseBytes is the maximum size of a response from a Resolver. The
-const maxResponseBytes = 512
+// 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,

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)

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
+	ip        netaddr.IP
-	name  dnsname.FQDN
+	txt       []string
-	rcode dns.RCode
+	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])
+					}
+				}
+			}
 		})
 	}
 }