diff --git a/client/internal/dns/resutil/resolve.go b/client/internal/dns/resutil/resolve.go index a2599aee7..931938755 100644 --- a/client/internal/dns/resutil/resolve.go +++ b/client/internal/dns/resutil/resolve.go @@ -8,6 +8,7 @@ import ( "errors" "net" "net/netip" + "slices" "strings" "github.com/miekg/dns" @@ -167,7 +168,10 @@ func getRcodeForNotFound(ctx context.Context, r resolver, domain string, origina case dns.TypeA: alternativeNetwork = "ip6" default: - return dns.RcodeNameError + // Non-address types reach LookupIP only unexpectedly; without an + // address pair to probe we cannot prove the name is absent, so answer + // NODATA rather than a poisoning NXDOMAIN. + return dns.RcodeSuccess } if _, err := r.LookupNetIP(ctx, alternativeNetwork, domain); err != nil { @@ -184,6 +188,230 @@ func getRcodeForNotFound(ctx context.Context, r resolver, domain string, origina return dns.RcodeSuccess } +// RecordResolver is the host resolver surface used to forward non-address +// record queries. net.DefaultResolver satisfies it. +type RecordResolver interface { + LookupMX(ctx context.Context, name string) ([]*net.MX, error) + LookupTXT(ctx context.Context, name string) ([]string, error) + LookupNS(ctx context.Context, name string) ([]*net.NS, error) + LookupSRV(ctx context.Context, service, proto, name string) (string, []*net.SRV, error) + LookupCNAME(ctx context.Context, host string) (string, error) + LookupAddr(ctx context.Context, addr string) ([]string, error) +} + +// LookupRecords resolves a non-address DNS record type through the host +// resolver and returns the resource records and the DNS rcode. Types the host +// resolver cannot answer (anything not covered by the net.Resolver Lookup* +// methods) yield NODATA so that a routed name is never poisoned with NXDOMAIN +// for an unsupported type. +func LookupRecords(ctx context.Context, r RecordResolver, name string, qtype uint16, ttl uint32) ([]dns.RR, int) { + fqdn := dns.Fqdn(name) + + switch qtype { + case dns.TypeMX: + return lookupMX(ctx, r, name, fqdn, ttl) + case dns.TypeTXT: + return lookupTXT(ctx, r, name, fqdn, ttl) + case dns.TypeNS: + return lookupNS(ctx, r, name, fqdn, ttl) + case dns.TypeSRV: + return lookupSRV(ctx, r, name, fqdn, ttl) + case dns.TypeCNAME: + return lookupCNAME(ctx, r, name, fqdn, ttl) + case dns.TypePTR: + return lookupPTR(ctx, r, name, fqdn, ttl) + default: + return nil, dns.RcodeSuccess + } +} + +func recordHeader(fqdn string, rrtype uint16, ttl uint32) dns.RR_Header { + return dns.RR_Header{Name: fqdn, Rrtype: rrtype, Class: dns.ClassINET, Ttl: ttl} +} + +func lookupMX(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + recs, err := r.LookupMX(ctx, name) + if err != nil { + return nil, rcodeForRecordError(err) + } + rrs := make([]dns.RR, 0, len(recs)) + for _, mx := range recs { + rrs = append(rrs, &dns.MX{ + Hdr: recordHeader(fqdn, dns.TypeMX, ttl), + Preference: mx.Pref, + Mx: dns.Fqdn(mx.Host), + }) + } + return rrs, dns.RcodeSuccess +} + +func lookupTXT(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + recs, err := r.LookupTXT(ctx, name) + if err != nil { + return nil, rcodeForRecordError(err) + } + rrs := make([]dns.RR, 0, len(recs)) + for _, txt := range recs { + rrs = append(rrs, &dns.TXT{ + Hdr: recordHeader(fqdn, dns.TypeTXT, ttl), + Txt: chunkTXT(txt), + }) + } + return rrs, dns.RcodeSuccess +} + +func lookupNS(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + recs, err := r.LookupNS(ctx, name) + if err != nil { + return nil, rcodeForRecordError(err) + } + rrs := make([]dns.RR, 0, len(recs)) + for _, ns := range recs { + rrs = append(rrs, &dns.NS{ + Hdr: recordHeader(fqdn, dns.TypeNS, ttl), + Ns: dns.Fqdn(ns.Host), + }) + } + return rrs, dns.RcodeSuccess +} + +func lookupSRV(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + _, recs, err := r.LookupSRV(ctx, "", "", name) + if err != nil { + return nil, rcodeForRecordError(err) + } + rrs := make([]dns.RR, 0, len(recs)) + for _, srv := range recs { + rrs = append(rrs, &dns.SRV{ + Hdr: recordHeader(fqdn, dns.TypeSRV, ttl), + Priority: srv.Priority, + Weight: srv.Weight, + Port: srv.Port, + Target: dns.Fqdn(srv.Target), + }) + } + return rrs, dns.RcodeSuccess +} + +func lookupCNAME(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + cname, err := r.LookupCNAME(ctx, name) + if err != nil { + return nil, rcodeForRecordError(err) + } + // LookupCNAME returns the queried name itself when the name resolves but + // has no CNAME record; that is a NODATA result, not a CNAME. + if strings.EqualFold(dns.Fqdn(cname), fqdn) { + return nil, dns.RcodeSuccess + } + return []dns.RR{&dns.CNAME{ + Hdr: recordHeader(fqdn, dns.TypeCNAME, ttl), + Target: dns.Fqdn(cname), + }}, dns.RcodeSuccess +} + +func lookupPTR(ctx context.Context, r RecordResolver, name, fqdn string, ttl uint32) ([]dns.RR, int) { + addr, ok := ptrQueryAddr(name) + if !ok { + return nil, dns.RcodeSuccess + } + names, err := r.LookupAddr(ctx, addr) + if err != nil { + return nil, rcodeForRecordError(err) + } + rrs := make([]dns.RR, 0, len(names)) + for _, n := range names { + rrs = append(rrs, &dns.PTR{ + Hdr: recordHeader(fqdn, dns.TypePTR, ttl), + Ptr: dns.Fqdn(n), + }) + } + return rrs, dns.RcodeSuccess +} + +// ptrQueryAddr converts a reverse-DNS query name (in-addr.arpa or ip6.arpa) +// into the address string expected by net.Resolver.LookupAddr. It reports false +// when the name is not a well-formed reverse name. +func ptrQueryAddr(qname string) (string, bool) { + name := strings.TrimSuffix(strings.ToLower(dns.Fqdn(qname)), ".") + + switch { + case strings.HasSuffix(name, ".in-addr.arpa"): + return parseInAddrArpa(strings.TrimSuffix(name, ".in-addr.arpa")) + case strings.HasSuffix(name, ".ip6.arpa"): + return parseIP6Arpa(strings.TrimSuffix(name, ".ip6.arpa")) + default: + return "", false + } +} + +// parseInAddrArpa turns the label portion of an in-addr.arpa name into an IPv4 +// address string, reporting false when it is not a well-formed reverse name. +func parseInAddrArpa(labelPart string) (string, bool) { + labels := strings.Split(labelPart, ".") + if len(labels) != 4 { + return "", false + } + slices.Reverse(labels) + addr, err := netip.ParseAddr(strings.Join(labels, ".")) + if err != nil || !addr.Is4() { + return "", false + } + return addr.String(), true +} + +// parseIP6Arpa turns the nibble portion of an ip6.arpa name into an IPv6 +// address string, reporting false when it is not a well-formed reverse name. +func parseIP6Arpa(nibblePart string) (string, bool) { + nibbles := strings.Split(nibblePart, ".") + if len(nibbles) != 32 { + return "", false + } + slices.Reverse(nibbles) + var sb strings.Builder + for i, n := range nibbles { + if i > 0 && i%4 == 0 { + sb.WriteByte(':') + } + sb.WriteString(n) + } + addr, err := netip.ParseAddr(sb.String()) + if err != nil || !addr.Is6() { + return "", false + } + return addr.String(), true +} + +// rcodeForRecordError maps a non-address lookup error to a DNS rcode. A +// not-found result becomes NODATA rather than NXDOMAIN: net.DNSError.IsNotFound +// does not distinguish a missing name from a name that exists only with records +// of other types, so the name cannot be proven absent and must not be poisoned. +func rcodeForRecordError(err error) int { + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) && dnsErr.IsNotFound { + return dns.RcodeSuccess + } + return dns.RcodeServerFailure +} + +// chunkTXT splits a TXT string into character-strings no longer than 255 bytes +// so the record can be packed. The chunks form one TXT resource record. +func chunkTXT(s string) []string { + const maxLen = 255 + if len(s) <= maxLen { + return []string{s} + } + + var chunks []string + for len(s) > maxLen { + chunks = append(chunks, s[:maxLen]) + s = s[maxLen:] + } + if len(s) > 0 { + chunks = append(chunks, s) + } + return chunks +} + // FormatAnswers formats DNS resource records for logging. func FormatAnswers(answers []dns.RR) string { if len(answers) == 0 { diff --git a/client/internal/dns/resutil/resolve_test.go b/client/internal/dns/resutil/resolve_test.go index e6a8cc6a5..f51092a83 100644 --- a/client/internal/dns/resutil/resolve_test.go +++ b/client/internal/dns/resutil/resolve_test.go @@ -5,6 +5,7 @@ import ( "errors" "net" "net/netip" + "strings" "testing" "github.com/miekg/dns" @@ -121,6 +122,164 @@ func TestLookupIP_DNSErrorNotIsNotFound(t *testing.T) { assert.Equal(t, dns.RcodeServerFailure, result.Rcode, "upstream failure should map to SERVFAIL") } +func TestPtrQueryAddr(t *testing.T) { + tests := []struct { + name string + qname string + want string + wantOK bool + }{ + {name: "ipv4", qname: "4.3.2.1.in-addr.arpa.", want: "1.2.3.4", wantOK: true}, + {name: "ipv4 no trailing dot", qname: "1.0.0.127.in-addr.arpa", want: "127.0.0.1", wantOK: true}, + { + name: "ipv6", + qname: "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + want: "2001:db8::1", + wantOK: true, + }, + {name: "ipv4 wrong label count", qname: "2.1.in-addr.arpa.", wantOK: false}, + {name: "ipv6 wrong nibble count", qname: "1.0.ip6.arpa.", wantOK: false}, + {name: "not a reverse name", qname: "example.com.", wantOK: false}, + {name: "ipv4 bad octet", qname: "4.3.2.999.in-addr.arpa.", wantOK: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := ptrQueryAddr(tt.qname) + assert.Equal(t, tt.wantOK, ok, "parse success mismatch") + if tt.wantOK { + assert.Equal(t, tt.want, got, "parsed address mismatch") + } + }) + } +} + +type mockRecordResolver struct { + mx []*net.MX + txt []string + ns []*net.NS + srv []*net.SRV + cname string + ptr []string + err error +} + +func (m *mockRecordResolver) LookupMX(context.Context, string) ([]*net.MX, error) { + return m.mx, m.err +} +func (m *mockRecordResolver) LookupTXT(context.Context, string) ([]string, error) { + return m.txt, m.err +} +func (m *mockRecordResolver) LookupNS(context.Context, string) ([]*net.NS, error) { + return m.ns, m.err +} +func (m *mockRecordResolver) LookupSRV(context.Context, string, string, string) (string, []*net.SRV, error) { + return "", m.srv, m.err +} +func (m *mockRecordResolver) LookupCNAME(context.Context, string) (string, error) { + return m.cname, m.err +} +func (m *mockRecordResolver) LookupAddr(context.Context, string) ([]string, error) { + return m.ptr, m.err +} + +func TestLookupRecords(t *testing.T) { + notFound := &net.DNSError{IsNotFound: true, Name: "example.com."} + + t.Run("MX success", func(t *testing.T) { + r := &mockRecordResolver{mx: []*net.MX{{Host: "mail.example.com.", Pref: 10}}} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeMX, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, "mail.example.com.", rrs[0].(*dns.MX).Mx) + }) + + t.Run("TXT short string is one character-string", func(t *testing.T) { + r := &mockRecordResolver{txt: []string{"v=spf1 -all"}} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeTXT, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, []string{"v=spf1 -all"}, rrs[0].(*dns.TXT).Txt) + }) + + t.Run("TXT chunks long strings", func(t *testing.T) { + long := strings.Repeat("a", 300) + r := &mockRecordResolver{txt: []string{long}} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeTXT, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + txt := rrs[0].(*dns.TXT).Txt + require.Len(t, txt, 2, "300-byte string should split into two character-strings") + assert.Equal(t, 255, len(txt[0])) + assert.Equal(t, 45, len(txt[1])) + }) + + t.Run("NS success", func(t *testing.T) { + r := &mockRecordResolver{ns: []*net.NS{{Host: "ns1.example.com."}}} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeNS, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, "ns1.example.com.", rrs[0].(*dns.NS).Ns) + }) + + t.Run("SRV success", func(t *testing.T) { + r := &mockRecordResolver{srv: []*net.SRV{{Target: "sip.example.com.", Port: 5060}}} + rrs, rcode := LookupRecords(context.Background(), r, "_sip._tcp.example.com.", dns.TypeSRV, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, uint16(5060), rrs[0].(*dns.SRV).Port) + }) + + t.Run("CNAME success", func(t *testing.T) { + r := &mockRecordResolver{cname: "target.example.com."} + rrs, rcode := LookupRecords(context.Background(), r, "www.example.com.", dns.TypeCNAME, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, "target.example.com.", rrs[0].(*dns.CNAME).Target) + }) + + t.Run("CNAME equal to name is NODATA", func(t *testing.T) { + r := &mockRecordResolver{cname: "example.com."} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeCNAME, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + assert.Empty(t, rrs, "self-referential CNAME is NODATA") + }) + + t.Run("PTR success", func(t *testing.T) { + r := &mockRecordResolver{ptr: []string{"host.example.com."}} + rrs, rcode := LookupRecords(context.Background(), r, "4.3.2.1.in-addr.arpa.", dns.TypePTR, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + require.Len(t, rrs, 1) + assert.Equal(t, "host.example.com.", rrs[0].(*dns.PTR).Ptr) + }) + + t.Run("PTR malformed name is NODATA", func(t *testing.T) { + r := &mockRecordResolver{} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypePTR, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + assert.Empty(t, rrs) + }) + + t.Run("not found is NODATA never NXDOMAIN", func(t *testing.T) { + r := &mockRecordResolver{err: notFound} + _, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeMX, 300) + assert.Equal(t, dns.RcodeSuccess, rcode, "missing record must not poison the name") + }) + + t.Run("server failure maps to SERVFAIL", func(t *testing.T) { + r := &mockRecordResolver{err: &net.DNSError{Err: "server misbehaving", IsTemporary: true}} + _, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeMX, 300) + assert.Equal(t, dns.RcodeServerFailure, rcode) + }) + + t.Run("unsupported type is NODATA", func(t *testing.T) { + r := &mockRecordResolver{} + rrs, rcode := LookupRecords(context.Background(), r, "example.com.", dns.TypeCAA, 300) + assert.Equal(t, dns.RcodeSuccess, rcode) + assert.Empty(t, rrs) + }) +} + func TestStripOPT(t *testing.T) { rm := &dns.Msg{ Extra: []dns.RR{ diff --git a/client/internal/dnsfwd/forwarder.go b/client/internal/dnsfwd/forwarder.go index c15a8520f..b7e5a10e3 100644 --- a/client/internal/dnsfwd/forwarder.go +++ b/client/internal/dnsfwd/forwarder.go @@ -37,6 +37,12 @@ const ( type resolver interface { LookupNetIP(ctx context.Context, network, host string) ([]netip.Addr, error) + LookupMX(ctx context.Context, name string) ([]*net.MX, error) + LookupTXT(ctx context.Context, name string) ([]string, error) + LookupNS(ctx context.Context, name string) ([]*net.NS, error) + LookupSRV(ctx context.Context, service, proto, name string) (string, []*net.SRV, error) + LookupCNAME(ctx context.Context, host string) (string, error) + LookupAddr(ctx context.Context, addr string) ([]string, error) } type firewaller interface { @@ -210,12 +216,6 @@ func (f *DNSForwarder) handleDNSQuery(logger *log.Entry, w dns.ResponseWriter, q qname, dns.TypeToString[question.Qtype], dns.ClassToString[question.Qclass]) resp := query.SetReply(query) - network := resutil.NetworkForQtype(question.Qtype) - if network == "" { - resp.Rcode = dns.RcodeNotImplemented - f.writeResponse(logger, w, resp, qname, startTime) - return - } mostSpecificResId, matchingEntries := f.getMatchingEntries(strings.TrimSuffix(qname, ".")) if mostSpecificResId == "" { @@ -227,9 +227,46 @@ func (f *DNSForwarder) handleDNSQuery(logger *log.Entry, w dns.ResponseWriter, q ctx, cancel := context.WithTimeout(context.Background(), upstreamTimeout) defer cancel() + reqHasEdns := query.IsEdns0() != nil + + switch question.Qtype { + case dns.TypeA, dns.TypeAAAA: + f.handleAddressQuery(ctx, logger, w, resp, mostSpecificResId, matchingEntries, reqHasEdns, startTime) + case dns.TypeMX, dns.TypeTXT, dns.TypeNS, dns.TypeSRV, dns.TypeCNAME, dns.TypePTR: + f.handleRecordQuery(ctx, logger, w, resp, startTime) + default: + // The domain is routed here, so any other type is answered NODATA + // (NOERROR, empty answer) rather than falling back to a resolver that + // would poison the name with NXDOMAIN. The Extended DNS Error lets a + // client tell this capability-driven NODATA apart from an + // authoritative one. The OPT pseudo-record must not appear unless the + // query advertised EDNS0. + if reqHasEdns { + attachEDE(resp, dns.ExtendedErrorCodeNotSupported, "netbird forwarder: unsupported query type") + } + f.writeResponse(logger, w, resp, qname, startTime) + } +} + +// handleAddressQuery resolves A/AAAA queries, programs the firewall sets and +// resolved-IP state, and caches the answer for resilience on upstream failure. +func (f *DNSForwarder) handleAddressQuery( + ctx context.Context, + logger *log.Entry, + w dns.ResponseWriter, + resp *dns.Msg, + mostSpecificResId route.ResID, + matchingEntries []*ForwarderEntry, + reqHasEdns bool, + startTime time.Time, +) { + question := resp.Question[0] + qname := strings.ToLower(question.Name) + + network := resutil.NetworkForQtype(question.Qtype) result := resutil.LookupIP(ctx, f.resolver, network, qname, question.Qtype) if result.Err != nil { - f.handleDNSError(ctx, logger, w, question, resp, qname, result, query.IsEdns0() != nil, startTime) + f.handleDNSError(ctx, logger, w, question, resp, qname, result, reqHasEdns, startTime) return } @@ -240,6 +277,25 @@ func (f *DNSForwarder) handleDNSQuery(logger *log.Entry, w dns.ResponseWriter, q f.writeResponse(logger, w, resp, qname, startTime) } +// handleRecordQuery resolves non-address record types (MX, TXT, NS, SRV, +// CNAME, PTR) through the host resolver. Missing records are answered NODATA so +// the routed name is never poisoned with NXDOMAIN. +func (f *DNSForwarder) handleRecordQuery( + ctx context.Context, + logger *log.Entry, + w dns.ResponseWriter, + resp *dns.Msg, + startTime time.Time, +) { + question := resp.Question[0] + qname := strings.ToLower(question.Name) + + records, rcode := resutil.LookupRecords(ctx, f.resolver, qname, question.Qtype, f.ttl) + resp.Rcode = rcode + resp.Answer = append(resp.Answer, records...) + f.writeResponse(logger, w, resp, qname, startTime) +} + func (f *DNSForwarder) writeResponse(logger *log.Entry, w dns.ResponseWriter, resp *dns.Msg, qname string, startTime time.Time) { if err := w.WriteMsg(resp); err != nil { logger.Errorf("failed to write DNS response: %v", err) diff --git a/client/internal/dnsfwd/forwarder_test.go b/client/internal/dnsfwd/forwarder_test.go index 046595473..c69a9166e 100644 --- a/client/internal/dnsfwd/forwarder_test.go +++ b/client/internal/dnsfwd/forwarder_test.go @@ -133,6 +133,41 @@ func (m *MockResolver) LookupNetIP(ctx context.Context, network, host string) ([ return args.Get(0).([]netip.Addr), args.Error(1) } +func (m *MockResolver) LookupMX(ctx context.Context, name string) ([]*net.MX, error) { + args := m.Called(ctx, name) + recs, _ := args.Get(0).([]*net.MX) + return recs, args.Error(1) +} + +func (m *MockResolver) LookupTXT(ctx context.Context, name string) ([]string, error) { + args := m.Called(ctx, name) + recs, _ := args.Get(0).([]string) + return recs, args.Error(1) +} + +func (m *MockResolver) LookupNS(ctx context.Context, name string) ([]*net.NS, error) { + args := m.Called(ctx, name) + recs, _ := args.Get(0).([]*net.NS) + return recs, args.Error(1) +} + +func (m *MockResolver) LookupSRV(ctx context.Context, service, proto, name string) (string, []*net.SRV, error) { + args := m.Called(ctx, service, proto, name) + recs, _ := args.Get(1).([]*net.SRV) + return args.String(0), recs, args.Error(2) +} + +func (m *MockResolver) LookupCNAME(ctx context.Context, host string) (string, error) { + args := m.Called(ctx, host) + return args.String(0), args.Error(1) +} + +func (m *MockResolver) LookupAddr(ctx context.Context, addr string) ([]string, error) { + args := m.Called(ctx, addr) + recs, _ := args.Get(0).([]string) + return recs, args.Error(1) +} + func TestDNSForwarder_SubdomainAccessLogic(t *testing.T) { tests := []struct { name string @@ -545,12 +580,15 @@ func TestDNSForwarder_MultipleIPsInSingleUpdate(t *testing.T) { } func TestDNSForwarder_ResponseCodes(t *testing.T) { + // A type with no net.Resolver Lookup method (CAA) must answer NODATA + // (NOERROR, empty) rather than NXDOMAIN/NOTIMP to avoid poisoning the name. tests := []struct { name string queryType uint16 queryDomain string configured string expectedCode int + expectEDE bool description string }{ { @@ -562,28 +600,13 @@ func TestDNSForwarder_ResponseCodes(t *testing.T) { description: "RFC compliant REFUSED for unauthorized queries", }, { - name: "unsupported query type returns NOTIMP", - queryType: dns.TypeMX, + name: "unsupported query type returns NODATA", + queryType: dns.TypeCAA, queryDomain: "example.com", configured: "example.com", - expectedCode: dns.RcodeNotImplemented, - description: "RFC compliant NOTIMP for unsupported types", - }, - { - name: "CNAME query returns NOTIMP", - queryType: dns.TypeCNAME, - queryDomain: "example.com", - configured: "example.com", - expectedCode: dns.RcodeNotImplemented, - description: "CNAME queries not supported", - }, - { - name: "TXT query returns NOTIMP", - queryType: dns.TypeTXT, - queryDomain: "example.com", - configured: "example.com", - expectedCode: dns.RcodeNotImplemented, - description: "TXT queries not supported", + expectedCode: dns.RcodeSuccess, + expectEDE: true, + description: "Unsupported types answer NODATA, not NXDOMAIN/NOTIMP", }, } @@ -599,6 +622,7 @@ func TestDNSForwarder_ResponseCodes(t *testing.T) { query := &dns.Msg{} query.SetQuestion(dns.Fqdn(tt.queryDomain), tt.queryType) + query.SetEdns0(dns.DefaultMsgSize, false) // Capture the written response var writtenResp *dns.Msg @@ -614,10 +638,213 @@ func TestDNSForwarder_ResponseCodes(t *testing.T) { // Check the response written to the writer require.NotNil(t, writtenResp, "Expected response to be written") assert.Equal(t, tt.expectedCode, writtenResp.Rcode, tt.description) + assert.Empty(t, writtenResp.Answer, "Non-address response should carry no answers") + + if tt.expectEDE { + require.NotNil(t, writtenResp.IsEdns0(), "EDNS0 client should get an OPT in the reply") + assert.True(t, hasEDE(writtenResp, dns.ExtendedErrorCodeNotSupported), + "unsupported type NODATA should carry EDE Not Supported") + } }) } } +func hasEDE(m *dns.Msg, code uint16) bool { + opt := m.IsEdns0() + if opt == nil { + return false + } + for _, o := range opt.Option { + if ede, ok := o.(*dns.EDNS0_EDE); ok && ede.InfoCode == code { + return true + } + } + return false +} + +func TestDNSForwarder_RecordQueries(t *testing.T) { + notFound := &net.DNSError{IsNotFound: true, Name: "example.com"} + + t.Run("MX records are forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + mockResolver.On("LookupMX", mock.Anything, "example.com."). + Return([]*net.MX{{Host: "mail.example.com.", Pref: 10}}, nil).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeMX) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + mx, ok := resp.Answer[0].(*dns.MX) + require.True(t, ok, "answer should be an MX record") + assert.Equal(t, uint16(10), mx.Preference) + assert.Equal(t, "mail.example.com.", mx.Mx) + mockResolver.AssertExpectations(t) + }) + + t.Run("missing MX is NODATA not NXDOMAIN", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + // A not-found cannot prove the name is absent (it may exist with only + // other record types), so it must answer NODATA, never NXDOMAIN. + mockResolver.On("LookupMX", mock.Anything, "example.com."). + Return(nil, notFound).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeMX) + assert.Equal(t, dns.RcodeSuccess, resp.Rcode, "missing record must be NODATA") + assert.Empty(t, resp.Answer) + mockResolver.AssertExpectations(t) + }) + + t.Run("NS records are forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + mockResolver.On("LookupNS", mock.Anything, "example.com."). + Return([]*net.NS{{Host: "ns1.example.com."}}, nil).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeNS) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + ns, ok := resp.Answer[0].(*dns.NS) + require.True(t, ok, "answer should be an NS record") + assert.Equal(t, "ns1.example.com.", ns.Ns) + mockResolver.AssertExpectations(t) + }) + + t.Run("missing NS is NODATA", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + mockResolver.On("LookupNS", mock.Anything, "example.com."). + Return(nil, notFound).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeNS) + assert.Equal(t, dns.RcodeSuccess, resp.Rcode) + assert.Empty(t, resp.Answer) + mockResolver.AssertExpectations(t) + }) + + t.Run("SRV records are forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "_sip._tcp.example.com") + + mockResolver.On("LookupSRV", mock.Anything, "", "", "_sip._tcp.example.com."). + Return("", []*net.SRV{{Target: "sip.example.com.", Port: 5060, Priority: 10, Weight: 5}}, nil).Once() + + resp := runRecordQuery(t, forwarder, "_sip._tcp.example.com", dns.TypeSRV) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + srv, ok := resp.Answer[0].(*dns.SRV) + require.True(t, ok, "answer should be an SRV record") + assert.Equal(t, "sip.example.com.", srv.Target) + assert.Equal(t, uint16(5060), srv.Port) + assert.Equal(t, uint16(10), srv.Priority) + mockResolver.AssertExpectations(t) + }) + + t.Run("missing SRV is NODATA", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "_sip._tcp.example.com") + + mockResolver.On("LookupSRV", mock.Anything, "", "", "_sip._tcp.example.com."). + Return("", nil, notFound).Once() + + resp := runRecordQuery(t, forwarder, "_sip._tcp.example.com", dns.TypeSRV) + assert.Equal(t, dns.RcodeSuccess, resp.Rcode) + assert.Empty(t, resp.Answer) + mockResolver.AssertExpectations(t) + }) + + t.Run("TXT records are forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + mockResolver.On("LookupTXT", mock.Anything, "example.com."). + Return([]string{"v=spf1 -all"}, nil).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeTXT) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + txt, ok := resp.Answer[0].(*dns.TXT) + require.True(t, ok, "answer should be a TXT record") + assert.Equal(t, []string{"v=spf1 -all"}, txt.Txt) + mockResolver.AssertExpectations(t) + }) + + t.Run("CNAME record is forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "www.example.com") + + mockResolver.On("LookupCNAME", mock.Anything, "www.example.com."). + Return("target.example.com.", nil).Once() + + resp := runRecordQuery(t, forwarder, "www.example.com", dns.TypeCNAME) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + cname, ok := resp.Answer[0].(*dns.CNAME) + require.True(t, ok, "answer should be a CNAME record") + assert.Equal(t, "target.example.com.", cname.Target) + mockResolver.AssertExpectations(t) + }) + + t.Run("CNAME equal to the name is NODATA", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "example.com") + + // No CNAME exists: LookupCNAME echoes the queried name back. + mockResolver.On("LookupCNAME", mock.Anything, "example.com."). + Return("example.com.", nil).Once() + + resp := runRecordQuery(t, forwarder, "example.com", dns.TypeCNAME) + assert.Equal(t, dns.RcodeSuccess, resp.Rcode) + assert.Empty(t, resp.Answer, "self-referential CNAME means no CNAME record") + mockResolver.AssertExpectations(t) + }) + + t.Run("PTR record is forwarded", func(t *testing.T) { + mockResolver := &MockResolver{} + forwarder := newRecordTestForwarder(t, mockResolver, "*.in-addr.arpa") + + // The reverse name is parsed back to the address LookupAddr expects. + mockResolver.On("LookupAddr", mock.Anything, "1.2.3.4"). + Return([]string{"host.example.com."}, nil).Once() + + resp := runRecordQuery(t, forwarder, "4.3.2.1.in-addr.arpa", dns.TypePTR) + require.Equal(t, dns.RcodeSuccess, resp.Rcode) + require.Len(t, resp.Answer, 1) + ptr, ok := resp.Answer[0].(*dns.PTR) + require.True(t, ok, "answer should be a PTR record") + assert.Equal(t, "host.example.com.", ptr.Ptr) + mockResolver.AssertExpectations(t) + }) +} + +func newRecordTestForwarder(t *testing.T, r resolver, configured string) *DNSForwarder { + t.Helper() + forwarder := NewDNSForwarder(netip.MustParseAddrPort("127.0.0.1:0"), 300, nil, &peer.Status{}, nil) + forwarder.resolver = r + + d, err := domain.FromString(configured) + require.NoError(t, err) + forwarder.UpdateDomains([]*ForwarderEntry{{Domain: d, ResID: "test-res"}}) + return forwarder +} + +func runRecordQuery(t *testing.T, forwarder *DNSForwarder, qname string, qtype uint16) *dns.Msg { + t.Helper() + query := &dns.Msg{} + query.SetQuestion(dns.Fqdn(qname), qtype) + + mockWriter := &test.MockResponseWriter{} + forwarder.handleDNSQuery(log.NewEntry(log.StandardLogger()), mockWriter, query, time.Now()) + + resp := mockWriter.GetLastResponse() + require.NotNil(t, resp, "expected response to be written") + return resp +} + func TestDNSForwarder_UpstreamFailureEDE(t *testing.T) { tests := []struct { name string diff --git a/client/internal/routemanager/dnsinterceptor/handler.go b/client/internal/routemanager/dnsinterceptor/handler.go index 22f3355c8..b784cc274 100644 --- a/client/internal/routemanager/dnsinterceptor/handler.go +++ b/client/internal/routemanager/dnsinterceptor/handler.go @@ -226,12 +226,11 @@ func (d *DnsInterceptor) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { return } - // pass if non A/AAAA query - if r.Question[0].Qtype != dns.TypeA && r.Question[0].Qtype != dns.TypeAAAA { - d.continueToNextHandler(w, r, logger, "non A/AAAA query") - return - } - + // All query types for an intercepted domain are forwarded to the peer's + // DNS forwarder, which owns the name. Falling through to the system + // resolver would let it answer NXDOMAIN for a name it isn't authoritative + // for, poisoning the whole name (including the A/AAAA records the route + // does serve). The forwarder answers NODATA for types it cannot resolve. d.mu.RLock() peerKey := d.currentPeerKey d.mu.RUnlock() @@ -293,19 +292,6 @@ func (d *DnsInterceptor) writeDNSError(w dns.ResponseWriter, r *dns.Msg, logger } } -// continueToNextHandler signals the handler chain to try the next handler -func (d *DnsInterceptor) continueToNextHandler(w dns.ResponseWriter, r *dns.Msg, logger *log.Entry, reason string) { - logger.Tracef("continuing to next handler for domain=%s reason=%s", r.Question[0].Name, reason) - - resp := new(dns.Msg) - resp.SetRcode(r, dns.RcodeNameError) - // Set Zero bit to signal handler chain to continue - resp.MsgHdr.Zero = true - if err := w.WriteMsg(resp); err != nil { - logger.Errorf("failed writing DNS continue response: %v", err) - } -} - func (d *DnsInterceptor) getUpstreamIP(peerKey string) (netip.Addr, error) { peerAllowedIP, exists := d.peerStore.AllowedIP(peerKey) if !exists {