From bad1c6acdc028025efa632077c4b1905dab69d03 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Sat, 5 Dec 2020 16:49:32 +0100 Subject: [PATCH] Use urlfilter format in rebinding allow list --- client/src/__locales/en.json | 2 +- internal/dnsforward/dnsforward.go | 8 +++++ internal/dnsforward/dnsforward_test.go | 6 ++-- internal/dnsforward/http.go | 1 + internal/dnsforward/rebind.go | 42 +++++++++++++++++++++----- internal/dnsforward/rebind_test.go | 34 +++++++++++---------- 6 files changed, 66 insertions(+), 27 deletions(-) diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index efe77161..070e7a8f 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -593,5 +593,5 @@ "rebinding_protection_enabled": "Enable protection from DNS rebinding attacks", "rebinding_protection_enabled_desc": "If enabled, AdGuard Home will block responses containing host on the local network.", "rebinding_allowed_hosts_title": "Allowed domains", - "rebinding_allowed_hosts_desc": "A list of domains. If configured, AdGuard Home will allow responses containing host on the local network from these domains." + "rebinding_allowed_hosts_desc": "A list of domains. If configured, AdGuard Home will allow responses containing host on the local network from these domains. Here you can specify the exact domain names, wildcards and urlfilter-rules, e.g. 'example.org', '*.example.org' or '||example.org^'." } diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index d6825e46..d26025d2 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -53,6 +53,7 @@ type Server struct { queryLog querylog.QueryLog // Query log instance stats stats.Stats access *accessCtx + rebinding *dnsRebindChecker ipset ipsetCtx @@ -222,6 +223,13 @@ func (s *Server) Prepare(config *ServerConfig) error { return err } + // Initialize DNS rebinding module + // -- + s.rebinding, err = newRebindChecker(s.conf.RebindingAllowedHosts) + if err != nil { + return err + } + // Register web handlers if necessary // -- if !webRegistered && s.conf.HTTPRegister != nil { diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index af94462a..50204969 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -796,9 +796,9 @@ func TestBlockedDNSRebinding(t *testing.T) { } s.conf.RebindingProtectionEnabled = true - s.conf.RebindingAllowedHosts = []string{ - "nip.io.", - } + s.rebinding, _ = newRebindChecker([]string{ + "||nip.io^", + }) reply, err = dns.Exchange(&req, addr.String()) if err != nil { t.Fatalf("Couldn't talk to server %s: %s", addr, err) diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go index 1537ebc8..9798dfb7 100644 --- a/internal/dnsforward/http.go +++ b/internal/dnsforward/http.go @@ -315,6 +315,7 @@ func (s *Server) setConfig(dc dnsConfig) (restart bool) { if dc.RebindingAllowedHosts != nil { s.conf.RebindingAllowedHosts = *dc.RebindingAllowedHosts + restart = true } s.Unlock() s.conf.ConfigModified() diff --git a/internal/dnsforward/rebind.go b/internal/dnsforward/rebind.go index f629ff05..c625da12 100644 --- a/internal/dnsforward/rebind.go +++ b/internal/dnsforward/rebind.go @@ -9,10 +9,41 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/dnsfilter" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/urlfilter" + "github.com/AdguardTeam/urlfilter/filterlist" "github.com/miekg/dns" ) type dnsRebindChecker struct { + allowDomainEngine *urlfilter.DNSEngine +} + +func newRebindChecker(allowedHosts []string) (*dnsRebindChecker, error) { + buf := strings.Builder{} + for _, s := range allowedHosts { + buf.WriteString(s) + buf.WriteString("\n") + } + + rulesStorage, err := filterlist.NewRuleStorage([]filterlist.RuleList{ + &filterlist.StringRuleList{ + ID: int(0), + RulesText: buf.String(), + IgnoreCosmetic: true, + }, + }) + if err != nil { + return nil, err + } + + return &dnsRebindChecker{ + allowDomainEngine: urlfilter.NewDNSEngine(rulesStorage), + }, nil +} + +func (c *dnsRebindChecker) isAllowedDomain(domain string) bool { + _, ok := c.allowDomainEngine.Match(domain) + return ok } // IsPrivate reports whether ip is a private address, according to @@ -87,14 +118,11 @@ func (s *Server) isResponseRebind(domain, host string) bool { defer timer.LogElapsed("DNS Rebinding check for %s -> %s", domain, host) } - for _, h := range s.conf.RebindingAllowedHosts { - if strings.HasSuffix(domain, h) { - return false - } + if s.rebinding.isAllowedDomain(domain) { + return false } - c := dnsRebindChecker{} - return c.isRebindHost(host) + return s.rebinding.isRebindHost(host) } func processRebindingFilteringAfterResponse(ctx *dnsContext) int { @@ -157,7 +185,7 @@ func (s *Server) preventRebindResponse(ctx *dnsContext) (*dnsfilter.Result, erro } log.Debug(m) - blocked := s.isResponseRebind(domainName, host) + blocked := s.isResponseRebind(strings.TrimSuffix(domainName, "."), host) s.RUnlock() if blocked { diff --git a/internal/dnsforward/rebind_test.go b/internal/dnsforward/rebind_test.go index 51bd4395..8262408a 100644 --- a/internal/dnsforward/rebind_test.go +++ b/internal/dnsforward/rebind_test.go @@ -9,7 +9,7 @@ import ( ) func TestRebindingPrivateAddresses(t *testing.T) { - c := &dnsRebindChecker{} + c, _ := newRebindChecker(nil) r1 := byte(rand.Int31() & 0xFE) r2 := byte(rand.Int31() & 0xFE) @@ -53,9 +53,11 @@ func TestRebindLocalhost(t *testing.T) { } func TestIsResponseRebind(t *testing.T) { - s := &Server{} - s.conf.RebindingAllowedHosts = []string{ - "totally-safe.com", + c, _ := newRebindChecker([]string{ + "||totally-safe.com^", + }) + s := &Server{ + rebinding: c, } for _, host := range []string{ @@ -84,14 +86,14 @@ func TestIsResponseRebind(t *testing.T) { "localhost", } { s.conf.RebindingProtectionEnabled = true - assert.True(t, s.isResponseRebind("example.com", host)) - assert.False(t, s.isResponseRebind("totally-safe.com", host)) - assert.False(t, s.isResponseRebind("absolutely.totally-safe.com", host)) + assert.Truef(t, s.isResponseRebind("example.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("totally-safe.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("absolutely.totally-safe.com", host), "host: %s", host) s.conf.RebindingProtectionEnabled = false - assert.False(t, s.isResponseRebind("example.com", host)) - assert.False(t, s.isResponseRebind("totally-safe.com", host)) - assert.False(t, s.isResponseRebind("absolutely.totally-safe.com", host)) + assert.Falsef(t, s.isResponseRebind("example.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("totally-safe.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("absolutely.totally-safe.com", host), "host: %s", host) } for _, host := range []string{ @@ -99,13 +101,13 @@ func TestIsResponseRebind(t *testing.T) { "another-example.com", } { s.conf.RebindingProtectionEnabled = true - assert.False(t, s.isResponseRebind("example.com", host)) - assert.False(t, s.isResponseRebind("totally-safe.com", host)) - assert.False(t, s.isResponseRebind("absolutely.totally-legit.com", host)) + assert.Falsef(t, s.isResponseRebind("example.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("totally-safe.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("absolutely.totally-legit.com", host), "host: %s", host) s.conf.RebindingProtectionEnabled = false - assert.False(t, s.isResponseRebind("example.com", host)) - assert.False(t, s.isResponseRebind("totally-safe.com", host)) - assert.False(t, s.isResponseRebind("absolutely.totally-legit.com", host)) + assert.Falsef(t, s.isResponseRebind("example.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("totally-safe.com", host), "host: %s", host) + assert.Falsef(t, s.isResponseRebind("absolutely.totally-legit.com", host), "host: %s", host) } }