From 9a89f65027d923a0e44ff6ec2fcff0df514b9f75 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 24 May 2022 11:21:11 +0200 Subject: [PATCH] Improve support for DNS-SD and fall back to cached data for non-ICANN queries --- firewall/master.go | 5 +-- nameserver/nameserver.go | 73 +++++++++++++++++++++++++++++++++++---- network/netutils/dns.go | 52 +++++++++++++++++++--------- resolver/resolve.go | 42 ++++++++++++++++++++++ resolver/resolver-env.go | 11 +++++- resolver/resolver_test.go | 56 ++++++++++++++++++++++++++++++ 6 files changed, 211 insertions(+), 28 deletions(-) diff --git a/firewall/master.go b/firewall/master.go index 4be457aa..e50617d0 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -482,10 +482,7 @@ func checkDomainHeuristics(ctx context.Context, conn *network.Connection, p *pro trimmedDomain := strings.TrimRight(conn.Entity.Domain, ".") etld1, err := publicsuffix.EffectiveTLDPlusOne(trimmedDomain) if err != nil { - // we don't apply any checks here and let the request through - // because a malformed domain-name will likely be dropped by - // checks better suited for that. - log.Tracer(ctx).Warningf("filter: failed to get eTLD+1: %s", err) + // Don't run the check if the domain is a TLD. return false } diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index a92ca8ad..afdae60b 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -106,6 +106,9 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) return reply(nsutil.Refused("invalid domain")) } + // Get public suffix after validation. + q.InitPublicSuffixData() + // Check if query is failing. // Some software retries failing queries excessively. This might not be a // problem normally, but handling a request is pretty expensive for the @@ -252,10 +255,13 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) if err != nil { switch { case errors.Is(err, resolver.ErrNotFound): - tracer.Tracef("nameserver: %s", err) - conn.Failed("domain does not exist", "") - return reply(nsutil.NxDomain("nxdomain: " + err.Error())) - + // Try alternatives domain names for unofficial domain spaces. + rrCache = checkAlternativeCaches(ctx, q) + if rrCache == nil { + tracer.Tracef("nameserver: %s", err) + conn.Failed("domain does not exist", "") + return reply(nsutil.NxDomain("nxdomain: " + err.Error())) + } case errors.Is(err, resolver.ErrBlocked): tracer.Tracef("nameserver: %s", err) conn.Block(err.Error(), "") @@ -268,7 +274,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) case errors.Is(err, resolver.ErrOffline): if rrCache == nil { - log.Tracer(ctx).Debugf("nameserver: not resolving %s, device is offline", q.ID()) + tracer.Debugf("nameserver: not resolving %s, device is offline", q.ID()) conn.Failed("not resolving, device is offline", "") return reply(nsutil.ServerFailure(err.Error())) } @@ -290,8 +296,12 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) addFailingQuery(q, errors.New("emptry reply from resolver")) return reply(nsutil.ServerFailure("internal error: empty reply")) case rrCache.RCode == dns.RcodeNameError: - // Return now if NXDomain. - return reply(nsutil.NxDomain("no answer found (NXDomain)")) + // Try alternatives domain names for unofficial domain spaces. + rrCache = checkAlternativeCaches(ctx, q) + if rrCache == nil { + // Return now if NXDomain. + return reply(nsutil.NxDomain("no answer found (NXDomain)")) + } } // Check with firewall again after resolving. @@ -336,3 +346,52 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) ) return reply(rrCache, conn, rrCache) } + +func checkAlternativeCaches(ctx context.Context, q *resolver.Query) *resolver.RRCache { + // Do not try alternatives when the query is in a public suffix. + // This also includes arpa. and local. + if q.ICANNSpace { + return nil + } + + // Check if the env resolver has something. + pmEnvQ := &resolver.Query{ + FQDN: q.FQDN + "local." + resolver.InternalSpecialUseDomain, + QType: q.QType, + } + rrCache, err := resolver.QueryPortmasterEnv(ctx, pmEnvQ) + if err == nil && rrCache != nil && rrCache.RCode == dns.RcodeSuccess { + makeAlternativeRecord(ctx, q, rrCache, pmEnvQ.FQDN) + return rrCache + } + + // Check if we have anything in cache + localFQDN := q.FQDN + "local." + rrCache, err = resolver.GetRRCache(localFQDN, q.QType) + if err == nil && rrCache != nil && rrCache.RCode == dns.RcodeSuccess { + makeAlternativeRecord(ctx, q, rrCache, localFQDN) + return rrCache + } + + return nil +} + +func makeAlternativeRecord(ctx context.Context, q *resolver.Query, rrCache *resolver.RRCache, altName string) { + log.Tracer(ctx).Debugf("using %s to answer query", altName) + + // Duplicate answers so they match the query. + copied := make([]dns.RR, 0, len(rrCache.Answer)) + for _, answer := range rrCache.Answer { + if strings.ToLower(answer.Header().Name) == altName { + copiedAnswer := dns.Copy(answer) + copiedAnswer.Header().Name = q.FQDN + copied = append(copied, copiedAnswer) + } + } + if len(copied) > 0 { + rrCache.Answer = append(rrCache.Answer, copied...) + } + + // Update the question. + rrCache.Domain = q.FQDN +} diff --git a/network/netutils/dns.go b/network/netutils/dns.go index 7a7bd6e5..252dca47 100644 --- a/network/netutils/dns.go +++ b/network/netutils/dns.go @@ -4,21 +4,38 @@ import ( "fmt" "net" "regexp" + "strings" "github.com/miekg/dns" ) -var cleanDomainRegex = regexp.MustCompile( - `^` + // match beginning - `(` + // start subdomain group - `(xn--)?` + // idn prefix - `[a-z0-9_-]{1,63}` + // main chunk - `\.` + // ending with a dot - `)*` + // end subdomain group, allow any number of subdomains - `(xn--)?` + // TLD idn prefix - `[a-z0-9_-]{2,63}` + // TLD main chunk with at least two characters - `\.` + // ending with a dot - `$`, // match end +var ( + cleanDomainRegex = regexp.MustCompile( + `^` + // match beginning + `(` + // start subdomain group + `(xn--)?` + // idn prefix + `[a-z0-9_-]{1,63}` + // main chunk + `\.` + // ending with a dot + `)*` + // end subdomain group, allow any number of subdomains + `(xn--)?` + // TLD idn prefix + `[a-z0-9_-]{2,63}` + // TLD main chunk with at least two characters + `\.` + // ending with a dot + `$`, // match end + ) + + // dnsSDDomainRegex is a lot more lax to better suit the allowed characters in DNS-SD. + // Not all characters have been allowed - some special characters were + // removed to reduce the general attack surface. + dnsSDDomainRegex = regexp.MustCompile( + // Start of charset selection. + `^[` + + // Printable ASCII (character code 32-127), excluding some special characters. + ` !#$%&()*+,\-\./0-9:;=?@A-Z[\\\]^_\a-z{|}~` + + // Only latin characters from extended ASCII (character code 128-255). + `ŠŒŽšœžŸ¡¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýþÿ` + + // End of charset selection. + `]*$`, + ) ) // IsValidFqdn returns whether the given string is a valid fqdn. @@ -33,15 +50,18 @@ func IsValidFqdn(fqdn string) bool { return false } - // check with regex - if !cleanDomainRegex.MatchString(fqdn) { + // IsFqdn checks if a domain name is fully qualified. + if !dns.IsFqdn(fqdn) { return false } - // check with miegk/dns + // Use special check for .local domains to support DNS-SD. + if strings.HasSuffix(fqdn, ".local.") { + return dnsSDDomainRegex.MatchString(fqdn) + } - // IsFqdn checks if a domain name is fully qualified. - if !dns.IsFqdn(fqdn) { + // check with regex + if !cleanDomainRegex.MatchString(fqdn) { return false } diff --git a/resolver/resolve.go b/resolver/resolve.go index 1b0b429e..9f7fad66 100644 --- a/resolver/resolve.go +++ b/resolver/resolve.go @@ -5,10 +5,12 @@ import ( "errors" "fmt" "net" + "strings" "sync" "time" "github.com/miekg/dns" + "golang.org/x/net/publicsuffix" "github.com/safing/portbase/database" "github.com/safing/portbase/log" @@ -90,6 +92,11 @@ type Query struct { IgnoreFailing bool LocalResolversOnly bool + // ICANNSpace signifies if the domain is within ICANN managed domain space. + ICANNSpace bool + // Domain root is the effective TLD +1. + DomainRoot string + // internal dotPrefixedFQDN string } @@ -99,6 +106,41 @@ func (q *Query) ID() string { return q.FQDN + q.QType.String() } +// InitPublicSuffixData initializes the public suffix data. +func (q *Query) InitPublicSuffixData() { + // Get public suffix and derive if domain is in ICANN space. + suffix, icann := publicsuffix.PublicSuffix(strings.TrimSuffix(q.FQDN, ".")) + if icann || strings.Contains(suffix, ".") { + q.ICANNSpace = true + } + // Override some cases. + switch suffix { + case "example": + q.ICANNSpace = true // Defined by ICANN. + case "invalid": + q.ICANNSpace = true // Defined by ICANN. + case "local": + q.ICANNSpace = true // Defined by ICANN. + case "localhost": + q.ICANNSpace = true // Defined by ICANN. + case "onion": + q.ICANNSpace = false // Defined by ICANN, but special. + case "test": + q.ICANNSpace = true // Defined by ICANN. + } + // Add suffix to adhere to FQDN format. + suffix += "." + + switch { + case len(q.FQDN) == len(suffix): + // We are at or below the domain root, reset. + q.DomainRoot = "" + case len(q.FQDN) > len(suffix): + domainRootStart := strings.LastIndex(q.FQDN[:len(q.FQDN)-len(suffix)-1], ".") + 1 + q.DomainRoot = q.FQDN[domainRootStart:] + } +} + // check runs sanity checks and does some initialization. Returns whether the query passed the basic checks. func (q *Query) check() (ok bool) { if q.FQDN == "" { diff --git a/resolver/resolver-env.go b/resolver/resolver-env.go index 18acfba4..b8710ff3 100644 --- a/resolver/resolver-env.go +++ b/resolver/resolver-env.go @@ -128,7 +128,7 @@ func (er *envResolverConn) makeRRCache(q *Query, answers []dns.RR) *RRCache { // Disable caching, as the env always has the raw data available. q.NoCaching = true - return &RRCache{ + rrCache := &RRCache{ Domain: q.FQDN, Question: q.QType, RCode: dns.RcodeSuccess, @@ -136,6 +136,10 @@ func (er *envResolverConn) makeRRCache(q *Query, answers []dns.RR) *RRCache { Extra: []dns.RR{internalSpecialUseComment}, // Always add comment about this TLD. Resolver: envResolver.Info.Copy(), } + if len(rrCache.Answer) == 0 { + rrCache.RCode = dns.RcodeNameError + } + return rrCache } func (er *envResolverConn) ReportFailure() {} @@ -145,3 +149,8 @@ func (er *envResolverConn) IsFailing() bool { } func (er *envResolverConn) ResetFailure() {} + +// QueryPortmasterEnv queries the environment resolver directly. +func QueryPortmasterEnv(ctx context.Context, q *Query) (*RRCache, error) { + return envResolver.Conn.Query(ctx, q) +} diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go index f385e52e..61e05376 100644 --- a/resolver/resolver_test.go +++ b/resolver/resolver_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/miekg/dns" "github.com/safing/portbase/log" @@ -103,3 +105,57 @@ func TestBulkResolving(t *testing.T) { t.Logf("total time taken: %s", time.Since(started)) } + +func TestPublicSuffix(t *testing.T) { + t.Parallel() + + testSuffix(t, "co.uk.", "", true) + testSuffix(t, "amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "books.amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "www.books.amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "com.", "", true) + testSuffix(t, "amazon.com.", "amazon.com.", true) + testSuffix(t, "example0.debian.net.", "example0.debian.net.", true) + testSuffix(t, "example1.debian.org.", "debian.org.", true) + testSuffix(t, "golang.dev.", "golang.dev.", true) + testSuffix(t, "golang.net.", "golang.net.", true) + testSuffix(t, "play.golang.org.", "golang.org.", true) + testSuffix(t, "gophers.in.space.museum.", "in.space.museum.", true) + testSuffix(t, "0emm.com.", "0emm.com.", true) + testSuffix(t, "a.0emm.com.", "", true) + testSuffix(t, "b.c.d.0emm.com.", "c.d.0emm.com.", true) + testSuffix(t, "org.", "", true) + testSuffix(t, "foo.org.", "foo.org.", true) + testSuffix(t, "foo.co.uk.", "foo.co.uk.", true) + testSuffix(t, "foo.dyndns.org.", "foo.dyndns.org.", true) + testSuffix(t, "foo.blogspot.co.uk.", "foo.blogspot.co.uk.", true) + testSuffix(t, "there.is.no.such-tld.", "no.such-tld.", false) + testSuffix(t, "www.some.bit.", "some.bit.", false) + testSuffix(t, "cromulent.", "", false) + testSuffix(t, "arpa.", "", true) + testSuffix(t, "in-addr.arpa.", "", true) + testSuffix(t, "1.in-addr.arpa.", "1.in-addr.arpa.", true) + testSuffix(t, "ip6.arpa.", "", true) + testSuffix(t, "1.ip6.arpa.", "1.ip6.arpa.", true) + testSuffix(t, "www.some.arpa.", "some.arpa.", true) + testSuffix(t, "www.some.home.arpa.", "home.arpa.", true) + testSuffix(t, ".", "", false) + testSuffix(t, "", "", false) + + // Test edge case domains. + testSuffix(t, "www.some.example.", "some.example.", true) + testSuffix(t, "www.some.invalid.", "some.invalid.", true) + testSuffix(t, "www.some.local.", "some.local.", true) + testSuffix(t, "www.some.localhost.", "some.localhost.", true) + testSuffix(t, "www.some.onion.", "some.onion.", false) + testSuffix(t, "www.some.test.", "some.test.", true) +} + +func testSuffix(t *testing.T, fqdn, domainRoot string, icannSpace bool) { + t.Helper() + + q := &Query{FQDN: fqdn} + q.InitPublicSuffixData() + assert.Equal(t, domainRoot, q.DomainRoot) + assert.Equal(t, icannSpace, q.ICANNSpace) +}