diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index f447539f7..87fbd6d61 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -49,8 +49,26 @@ // truncation in a platform-agnostic way. const maxResponseBytes = 4095 -// defaultTTL is the TTL of all responses from Resolver. -const defaultTTL = 600 * time.Second +// defaultTTL is the TTL of positive responses from Resolver. +// +// It's short because the source of truth (the netmap-fed host maps) +// is local and in-memory, so re-queries are nearly free, while +// anything cached downstream (e.g. mDNSResponder on macOS) delays +// clients noticing node renames for the full TTL (tailscale/corp#45631). +const defaultTTL = 5 * time.Second + +// negativeTTL is how long resolvers may cache the nonexistence of a +// name (or of records of the queried type) for domains we're +// authoritative for. It's advertised via the SOA record attached to +// the authority section of NXDOMAIN and no-data responses, per RFC +// 2308. Without it, some resolvers (notably mDNSResponder) seem to +// cache negative entries for a really long time, so a name queried +// shortly before a node rename doesn't start resolving for a while +// (tailscale/corp#45631). +const negativeTTL = 10 * time.Second + +// timeNow is time.Now, except in tests. +var timeNow = time.Now var ( errNotQuery = errors.New("not a DNS query") @@ -960,6 +978,14 @@ type response struct { // NSs are the responses to an NS query. NSs []*net.NS + + // SOAZone, if non-empty, is the zone we're authoritative for + // that contains Question's name. marshalResponse attaches its + // SOA record to the authority section so that resolvers bound + // their negative caching to negativeTTL, per RFC 2308. It must + // only be set on negative responses: NXDOMAIN, or success with + // no records of the queried type. + SOAZone dnsname.FQDN } var dnsParserPool = &sync.Pool{ @@ -1141,6 +1167,35 @@ func marshalSRV(queryName dns.Name, srvs []*net.SRV, builder *dns.Builder) error return nil } +// marshalSOA serializes zone's SOA record into the authority section +// of an active builder, which must have had StartAuthorities called. +// Its only purpose is communicating negativeTTL to caching resolvers +// (RFC 2308), so all fields other than the TTLs are placeholders. +func marshalSOA(zone dnsname.FQDN, builder *dns.Builder) error { + name, err := dns.NewName(zone.WithTrailingDot()) + if err != nil { + return err + } + return builder.SOAResource(dns.ResourceHeader{ + Name: name, + Type: dns.TypeSOA, + Class: dns.ClassINET, + TTL: uint32(negativeTTL / time.Second), + }, dns.SOAResource{ + NS: name, + MBox: name, + // A serial should only change when the zone data does, + // but nothing consumes ours (no secondaries, no zone + // transfers), and the current time is at least monotonic + // and cheap. Unix seconds fit in the uint32 until 2106. + Serial: uint32(timeNow().Unix()), + Refresh: uint32(negativeTTL / time.Second), + Retry: uint32(negativeTTL / time.Second), + Expire: uint32(negativeTTL / time.Second), + MinTTL: uint32(negativeTTL / time.Second), + }) +} + // marshalResponse serializes the DNS response into a new buffer. func marshalResponse(resp *response) ([]byte, error) { resp.Header.Response = true @@ -1175,6 +1230,14 @@ func marshalResponse(resp *response) ([]byte, error) { // Only successful responses contain answers. if !isSuccess { + if resp.SOAZone != "" { + if err := builder.StartAuthorities(); err != nil { + return nil, err + } + if err := marshalSOA(resp.SOAZone, &builder); err != nil { + return nil, err + } + } return builder.Finish() } @@ -1208,6 +1271,15 @@ func marshalResponse(resp *response) ([]byte, error) { return nil, err } + if resp.SOAZone != "" { + if err := builder.StartAuthorities(); err != nil { + return nil, err + } + if err := marshalSOA(resp.SOAZone, &builder); err != nil { + return nil, err + } + } + return builder.Finish() } @@ -1325,6 +1397,19 @@ func rdnsNameToIPv6(name dnsname.FQDN) (ip netip.Addr, ok bool) { // respondReverse returns a DNS response to a PTR query. // It is assumed that resp.Question is populated by respond before this is called. +// authoritativeZoneFor returns the zone from the configured local +// domains that contains name, or "" if none does. +func (r *Resolver) authoritativeZoneFor(name dnsname.FQDN) dnsname.FQDN { + r.mu.Lock() + defer r.mu.Unlock() + for _, suffix := range r.localDomains { + if suffix.Contains(name) { + return suffix + } + } + return "" +} + func (r *Resolver) respondReverse(query []byte, name dnsname.FQDN, resp *response) ([]byte, error) { if hasRDNSBonjourPrefix(name) { metricDNSReverseMissBonjour.Add(1) @@ -1336,6 +1421,9 @@ func (r *Resolver) respondReverse(query []byte, name dnsname.FQDN, resp *respons metricDNSReverseMissOther.Add(1) return nil, errNotOurName } + if resp.Header.RCode == dns.RCodeNameError { + resp.SOAZone = r.authoritativeZoneFor(name) + } metricDNSMagicDNSSuccessReverse.Add(1) return marshalResponse(resp) @@ -1392,6 +1480,17 @@ func (r *Resolver) respond(query []byte) ([]byte, error) { resp := parser.response() resp.Header.RCode = rcode resp.IP = ip + switch { + case rcode == dns.RCodeNameError: + resp.SOAZone = r.authoritativeZoneFor(name) + case rcode == dns.RCodeSuccess && !ip.IsValid(): + switch parser.Question.Type { + case dns.TypeA, dns.TypeAAAA, dns.TypeALL: + // The name exists but has no records of the queried + // type (e.g. an AAAA query for an IPv4-only node). + resp.SOAZone = r.authoritativeZoneFor(name) + } + } metricDNSMagicDNSSuccessName.Add(1) return marshalResponse(resp) } diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 98727dd4f..57157dd13 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -934,7 +934,7 @@ func TestDelegateSplitRoute(t *testing.T) { // Answer: 0x05, 0x74, 0x65, 0x73, 0x74, 0x31, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // name 0x00, 0x01, 0x00, 0x01, // type A, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x04, // length: 4 bytes 0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4 } @@ -951,7 +951,7 @@ func TestDelegateSplitRoute(t *testing.T) { // Answer: 0x05, 0x74, 0x65, 0x73, 0x74, 0x31, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // name 0x00, 0x01, 0x00, 0x01, // type A, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x04, // length: 4 bytes 0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4 } @@ -968,7 +968,7 @@ func TestDelegateSplitRoute(t *testing.T) { // Answer: 0x05, 0x74, 0x65, 0x73, 0x74, 0x32, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // name 0x00, 0x1c, 0x00, 0x01, // type AAAA, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x10, // length: 16 bytes // AAAA: 0001:0203:0405:0607:0809:0A0B:0C0D:0E0F 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0xb, 0xc, 0xd, 0xe, 0xf, @@ -986,7 +986,7 @@ func TestDelegateSplitRoute(t *testing.T) { // Answer: 0x05, 0x54, 0x45, 0x53, 0x54, 0x31, 0x03, 0x49, 0x50, 0x4e, 0x03, 0x44, 0x45, 0x56, 0x00, // name 0x00, 0x01, 0x00, 0x01, // type A, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x04, // length: 4 bytes 0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4 } @@ -1005,7 +1005,7 @@ func TestDelegateSplitRoute(t *testing.T) { 0x01, 0x34, 0x01, 0x33, 0x01, 0x32, 0x01, 0x31, 0x07, 0x69, 0x6e, 0x2d, 0x61, 0x64, 0x64, 0x72, 0x04, 0x61, 0x72, 0x70, 0x61, 0x00, 0x00, 0x0c, 0x00, 0x01, // type PTR, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x0f, // length: 15 bytes // PTR: test1.ipn.dev 0x05, 0x74, 0x65, 0x73, 0x74, 0x31, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, @@ -1041,35 +1041,58 @@ func TestDelegateSplitRoute(t *testing.T) { 0x03, 0x69, 0x70, 0x36, 0x04, 0x61, 0x72, 0x70, 0x61, 0x00, 0x00, 0x0c, 0x00, 0x01, // type PTR, class IN - 0x00, 0x00, 0x02, 0x58, // TTL: 600 + 0x00, 0x00, 0x00, 0x05, // TTL: 5 0x00, 0x0f, // length: 15 bytes // PTR: test2.ipn.dev 0x05, 0x74, 0x65, 0x73, 0x74, 0x32, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, } -var nxdomainResponse = []byte{ +// soaTestTime is what tests pin timeNow to when comparing golden +// packets containing an SOA, whose serial is the response time in +// unix seconds. +var soaTestTime = time.Unix(0x12345678, 0) + +// soaAuthority is the authority section attached to negative +// responses (NXDOMAIN and no-data) for the ipn.dev zone, communicating +// the negative-caching TTL per RFC 2308. +var soaAuthority = []byte{ + 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // zone: ipn.dev. + 0x00, 0x06, 0x00, 0x01, // type SOA, class IN + 0x00, 0x00, 0x00, 0x0a, // TTL: 10 + 0x00, 0x26, // rdlength: 38 + 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // mname: ipn.dev. + 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // rname: ipn.dev. + 0x12, 0x34, 0x56, 0x78, // serial: soaTestTime as unix seconds + 0x00, 0x00, 0x00, 0x0a, // refresh: 10 + 0x00, 0x00, 0x00, 0x0a, // retry: 10 + 0x00, 0x00, 0x00, 0x0a, // expire: 10 + 0x00, 0x00, 0x00, 0x0a, // minimum (negative-caching TTL): 10 +} + +var nxdomainResponse = append([]byte{ 0x00, 0x00, // transaction id: 0 0x84, 0x03, // flags: response, authoritative, error: nxdomain 0x00, 0x01, // one question 0x00, 0x00, // no answers - 0x00, 0x00, 0x00, 0x00, // no authority or additional RRs + 0x00, 0x01, 0x00, 0x00, // one authority RR (the SOA), no additional RRs // Question: 0x05, 0x74, 0x65, 0x73, 0x74, 0x33, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // name 0x00, 0x01, 0x00, 0x01, // type A, class IN -} +}, soaAuthority...) -var emptyResponse = []byte{ +var emptyResponse = append([]byte{ 0x00, 0x00, // transaction id: 0 0x84, 0x00, // flags: response, authoritative, no error 0x00, 0x01, // one question 0x00, 0x00, // no answers - 0x00, 0x00, 0x00, 0x00, // no authority or additional RRs + 0x00, 0x01, 0x00, 0x00, // one authority RR (the SOA), no additional RRs // Question: 0x05, 0x74, 0x65, 0x73, 0x74, 0x31, 0x03, 0x69, 0x70, 0x6e, 0x03, 0x64, 0x65, 0x76, 0x00, // name 0x00, 0x1c, 0x00, 0x01, // type AAAA, class IN -} +}, soaAuthority...) func TestFull(t *testing.T) { + tstest.Replace(t, &timeNow, func() time.Time { return soaTestTime }) r := newResolver(t) defer r.Close() @@ -1105,6 +1128,74 @@ func TestFull(t *testing.T) { } } +// TestNegativeCachingSOA verifies that negative responses (NXDOMAIN +// and no-data) for zones we're authoritative for carry an SOA record +// in the authority section, bounding downstream resolvers' negative +// caching to negativeTTL per RFC 2308. Without it, some resolvers +// (notably macOS's mDNSResponder) seem to cache the nonexistence of +// a name for a really long time, so nodes renamed in the admin +// console don't start resolving under their new name for a while +// (tailscale/corp#45631). +func TestNegativeCachingSOA(t *testing.T) { + r := newResolver(t) + defer r.Close() + r.SetConfig(dnsCfg) + + tests := []struct { + name string + request []byte + zone string + wantCode dns.RCode + }{ + {"nxdomain", dnspacket("test3.ipn.dev.", dns.TypeA, noEdns), "ipn.dev.", dns.RCodeNameError}, + {"nodata-aaaa", dnspacket("test1.ipn.dev.", dns.TypeAAAA, noEdns), "ipn.dev.", dns.RCodeSuccess}, + {"nxdomain-ptr", dnspacket("9.3.2.1.in-addr.arpa.", dns.TypePTR, noEdns), "3.2.1.in-addr.arpa.", dns.RCodeNameError}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + response, err := syncRespond(r, tt.request) + if err != nil { + t.Fatalf("err = %v; want nil", err) + } + var p dns.Parser + hdr, err := p.Start(response) + if err != nil { + t.Fatal(err) + } + if hdr.RCode != tt.wantCode { + t.Errorf("RCode = %v; want %v", hdr.RCode, tt.wantCode) + } + if err := p.SkipAllQuestions(); err != nil { + t.Fatal(err) + } + if err := p.SkipAllAnswers(); err != nil { + t.Fatal(err) + } + ah, err := p.AuthorityHeader() + if err != nil { + t.Fatalf("AuthorityHeader: %v (no authority section?)", err) + } + if got := ah.Name.String(); got != tt.zone { + t.Errorf("authority name = %q; want %q", got, tt.zone) + } + if ah.Type != dns.TypeSOA { + t.Errorf("authority type = %v; want SOA", ah.Type) + } + wantTTL := uint32(negativeTTL / time.Second) + if ah.TTL != wantTTL { + t.Errorf("authority TTL = %v; want %v", ah.TTL, wantTTL) + } + soa, err := p.SOAResource() + if err != nil { + t.Fatal(err) + } + if soa.MinTTL != wantTTL { + t.Errorf("SOA minimum = %v; want %v", soa.MinTTL, wantTTL) + } + }) + } +} + func TestAllocs(t *testing.T) { r := newResolver(t) defer r.Close() @@ -1370,32 +1461,32 @@ func TestHandleExitNodeDNSQueryWithNetPkg(t *testing.T) { { Type: dns.TypeA, Name: "one-a.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x05one-a\x04test\x00\x00\x01\x00\x01\x05one-a\x04test\x00\x00\x01\x00\x01\x00\x00\x02X\x00\x04\x01\x02\x03\x04"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x05one-a\x04test\x00\x00\x01\x00\x01\x05one-a\x04test\x00\x00\x01\x00\x01\x00\x00\x00\x05\x00\x04\x01\x02\x03\x04"), }, { Type: dns.TypeA, Name: "two-a.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x05two-a\x04test\x00\x00\x01\x00\x01\xc0\f\x00\x01\x00\x01\x00\x00\x02X\x00\x04\x01\x02\x03\x04\xc0\f\x00\x01\x00\x01\x00\x00\x02X\x00\x04\x05\x06\a\b"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x05two-a\x04test\x00\x00\x01\x00\x01\xc0\f\x00\x01\x00\x01\x00\x00\x00\x05\x00\x04\x01\x02\x03\x04\xc0\f\x00\x01\x00\x01\x00\x00\x00\x05\x00\x04\x05\x06\a\b"), }, { Type: dns.TypeAAAA, Name: "one-aaaa.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\bone-aaaa\x04test\x00\x00\x1c\x00\x01\bone-aaaa\x04test\x00\x00\x1c\x00\x01\x00\x00\x02X\x00\x10\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\bone-aaaa\x04test\x00\x00\x1c\x00\x01\bone-aaaa\x04test\x00\x00\x1c\x00\x01\x00\x00\x00\x05\x00\x10\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"), }, { Type: dns.TypeAAAA, Name: "two-aaaa.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\btwo-aaaa\x04test\x00\x00\x1c\x00\x01\xc0\f\x00\x1c\x00\x01\x00\x00\x02X\x00\x10\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\xc0\f\x00\x1c\x00\x01\x00\x00\x02X\x00\x10\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\btwo-aaaa\x04test\x00\x00\x1c\x00\x01\xc0\f\x00\x1c\x00\x01\x00\x00\x00\x05\x00\x10\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\xc0\f\x00\x1c\x00\x01\x00\x00\x00\x05\x00\x10\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04"), }, { Type: dns.TypePTR, Name: "4.3.2.1.in-addr.arpa.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x014\x013\x012\x011\ain-addr\x04arpa\x00\x00\f\x00\x01\x014\x013\x012\x011\ain-addr\x04arpa\x00\x00\f\x00\x01\x00\x00\x02X\x00\t\x03foo\x03com\x00"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x014\x013\x012\x011\ain-addr\x04arpa\x00\x00\f\x00\x01\x014\x013\x012\x011\ain-addr\x04arpa\x00\x00\f\x00\x01\x00\x00\x00\x05\x00\t\x03foo\x03com\x00"), }, { Type: dns.TypeCNAME, Name: "cname.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x05cname\x04test\x00\x00\x05\x00\x01\x05cname\x04test\x00\x00\x05\x00\x01\x00\x00\x02X\x00\x10\nthe-target\x03foo\x00"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x01\x00\x00\x00\x00\x05cname\x04test\x00\x00\x05\x00\x01\x05cname\x04test\x00\x00\x05\x00\x01\x00\x00\x00\x05\x00\x10\nthe-target\x03foo\x00"), }, // No records of various types @@ -1422,17 +1513,17 @@ func TestHandleExitNodeDNSQueryWithNetPkg(t *testing.T) { { Type: dns.TypeTXT, Name: "txt.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x03\x00\x00\x00\x00\x03txt\x04test\x00\x00\x10\x00\x01\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x02X\x00\t\btxt1=one\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x02X\x00\t\btxt2=two\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x02X\x00\v\ntxt3=three"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x03\x00\x00\x00\x00\x03txt\x04test\x00\x00\x10\x00\x01\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x00\x05\x00\t\btxt1=one\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x00\x05\x00\t\btxt2=two\x03txt\x04test\x00\x00\x10\x00\x01\x00\x00\x00\x05\x00\v\ntxt3=three"), }, { Type: dns.TypeSRV, Name: "srv.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x03srv\x04test\x00\x00!\x00\x01\x03srv\x04test\x00\x00!\x00\x01\x00\x00\x02X\x00\x0f\x00\x01\x00\x02\x00\x03\x03foo\x03com\x00\x03srv\x04test\x00\x00!\x00\x01\x00\x00\x02X\x00\x0f\x00\x04\x00\x05\x00\x06\x03bar\x03com\x00"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x03srv\x04test\x00\x00!\x00\x01\x03srv\x04test\x00\x00!\x00\x01\x00\x00\x00\x05\x00\x0f\x00\x01\x00\x02\x00\x03\x03foo\x03com\x00\x03srv\x04test\x00\x00!\x00\x01\x00\x00\x00\x05\x00\x0f\x00\x04\x00\x05\x00\x06\x03bar\x03com\x00"), }, { Type: dns.TypeNS, Name: "ns.test.", - Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x02ns\x04test\x00\x00\x02\x00\x01\x02ns\x04test\x00\x00\x02\x00\x01\x00\x00\x02X\x00\t\x03ns1\x03foo\x00\x02ns\x04test\x00\x00\x02\x00\x01\x00\x00\x02X\x00\t\x03ns2\x03bar\x00"), + Check: matchPacked("\x00{\x84\x00\x00\x01\x00\x02\x00\x00\x00\x00\x02ns\x04test\x00\x00\x02\x00\x01\x02ns\x04test\x00\x00\x02\x00\x01\x00\x00\x00\x05\x00\t\x03ns1\x03foo\x00\x02ns\x04test\x00\x00\x02\x00\x01\x00\x00\x00\x05\x00\t\x03ns2\x03bar\x00"), }, }