From 194ea6ef9529e4d34ae764eaf4c5f60960aa3ed2 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 12 Jul 2021 21:10:39 +0300 Subject: [PATCH] Pull request: filtering: fix legacy rewrite wildcards Updates #3343. Squashed commit of the following: commit ab3c3e002a6d2a11bc3207fdaaeb292aaa194907 Author: Ainar Garipov Date: Mon Jul 12 20:58:57 2021 +0300 filtering: fix legacy rewrite wildcards --- CHANGELOG.md | 2 + internal/filtering/filtering.go | 11 ++-- internal/filtering/rewrites.go | 89 ++++++++++++++++++----------- internal/filtering/rewrites_test.go | 71 ++++++++++++++++------- 4 files changed, 112 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef4528c..f23fc376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ released by then. ### Fixed +- Conflicts between IPv4 and IPv6 DNS rewrites ([#3343]). - Letter case mismatches in `CNAME` filtering ([#3335]). - Occasional breakages on network errors with DNS-over-HTTP upstreams ([#3217]). - Errors when setting static IP on Linux ([#3257]). @@ -103,6 +104,7 @@ released by then. [#3256]: https://github.com/AdguardTeam/AdGuardHome/issues/3256 [#3257]: https://github.com/AdguardTeam/AdGuardHome/issues/3257 [#3335]: https://github.com/AdguardTeam/AdGuardHome/issues/3335 +[#3343]: https://github.com/AdguardTeam/AdGuardHome/issues/3343 diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go index 2e4a3ac9..033f950f 100644 --- a/internal/filtering/filtering.go +++ b/internal/filtering/filtering.go @@ -492,7 +492,7 @@ func (d *DNSFilter) processRewrites(host string, qtype uint16) (res Result) { d.confLock.RLock() defer d.confLock.RUnlock() - rr := findRewrites(d.Rewrites, host) + rr := findRewrites(d.Rewrites, host, qtype) if len(rr) != 0 { res.Reason = Rewritten } @@ -517,15 +517,14 @@ func (d *DNSFilter) processRewrites(host string, qtype uint16) (res Result) { cnames.Add(host) res.CanonName = rr[0].Answer - rr = findRewrites(d.Rewrites, host) + rr = findRewrites(d.Rewrites, host, qtype) } for _, r := range rr { - if (r.Type == dns.TypeA && qtype == dns.TypeA) || - (r.Type == dns.TypeAAAA && qtype == dns.TypeAAAA) { - + if r.Type == qtype && (qtype == dns.TypeA || qtype == dns.TypeAAAA) { if r.IP == nil { // IP exception - res.Reason = 0 + res.Reason = NotFilteredNotFound + return res } diff --git a/internal/filtering/rewrites.go b/internal/filtering/rewrites.go index b9d54df6..12dc5f21 100644 --- a/internal/filtering/rewrites.go +++ b/internal/filtering/rewrites.go @@ -15,10 +15,16 @@ import ( // RewriteEntry is a rewrite array element type RewriteEntry struct { + // Domain is the domain for which this rewrite should work. Domain string `yaml:"domain"` - Answer string `yaml:"answer"` // IP address or canonical name - Type uint16 `yaml:"-"` // DNS record type: CNAME, A or AAAA - IP net.IP `yaml:"-"` // Parsed IP address (if Type is A or AAAA) + // Answer is the IP address, canonical name, or one of the special + // values: "A" or "AAAA". + Answer string `yaml:"answer"` + // IP is the IP address that should be used in the response if Type is + // A or AAAA. + IP net.IP `yaml:"-"` + // Type is the DNS record type: A, AAAA, or CNAME. + Type uint16 `yaml:"-"` } func (r *RewriteEntry) equals(b RewriteEntry) bool { @@ -26,27 +32,32 @@ func (r *RewriteEntry) equals(b RewriteEntry) bool { } func isWildcard(host string) bool { - return len(host) >= 2 && - host[0] == '*' && host[1] == '.' + return len(host) > 1 && host[0] == '*' && host[1] == '.' } -// Return TRUE of host name matches a wildcard pattern -func matchDomainWildcard(host, wildcard string) bool { - return isWildcard(wildcard) && - strings.HasSuffix(host, wildcard[1:]) +// matchDomainWildcard returns true if host matches the wildcard pattern. +func matchDomainWildcard(host, wildcard string) (ok bool) { + return isWildcard(wildcard) && strings.HasSuffix(host, wildcard[1:]) } -type rewritesArray []RewriteEntry +// rewritesSorted is a slice of legacy rewrites for sorting. +// +// The sorting priority: +// +// A and AAAA > CNAME +// wildcard > exact +// lower level wildcard > higher level wildcard +// +type rewritesSorted []RewriteEntry -func (a rewritesArray) Len() int { return len(a) } +// Len implements the sort.Interface interface for legacyRewritesSorted. +func (a rewritesSorted) Len() int { return len(a) } -func (a rewritesArray) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +// Swap implements the sort.Interface interface for legacyRewritesSorted. +func (a rewritesSorted) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -// Priority: -// . CNAME < A/AAAA; -// . exact < wildcard; -// . higher level wildcard < lower level wildcard -func (a rewritesArray) Less(i, j int) bool { +// Less implements the sort.Interface interface for legacyRewritesSorted. +func (a rewritesSorted) Less(i, j int) bool { if a[i].Type == dns.TypeCNAME && a[j].Type != dns.TypeCNAME { return true } else if a[i].Type != dns.TypeCNAME && a[j].Type == dns.TypeCNAME { @@ -67,31 +78,37 @@ func (a rewritesArray) Less(i, j int) bool { return len(a[i].Domain) > len(a[j].Domain) } -// Prepare entry for use +// prepare prepares the a new or decoded entry. func (r *RewriteEntry) prepare() { - if r.Answer == "AAAA" { + switch r.Answer { + case "AAAA": r.IP = nil r.Type = dns.TypeAAAA + return - } else if r.Answer == "A" { + case "A": r.IP = nil r.Type = dns.TypeA + return + default: + // Go on. } ip := net.ParseIP(r.Answer) if ip == nil { r.Type = dns.TypeCNAME + return } - r.IP = ip - r.Type = dns.TypeAAAA - ip4 := ip.To4() if ip4 != nil { r.IP = ip4 r.Type = dns.TypeA + } else { + r.IP = ip + r.Type = dns.TypeAAAA } } @@ -101,19 +118,23 @@ func (d *DNSFilter) prepareRewrites() { } } -// Get the list of matched rewrite entries. -// Priority: CNAME, A/AAAA; exact, wildcard. -// If matched exactly, don't return wildcard entries. -// If matched by several wildcards, select the more specific one -func findRewrites(a []RewriteEntry, host string) []RewriteEntry { - rr := rewritesArray{} +// findRewrites returns the list of matched rewrite entries. The priority is: +// CNAME, then A and AAAA; exact, then wildcard. If the host is matched +// exactly, wildcard entries aren't returned. If the host matched by wildcards, +// return the most specific for the question type. +func findRewrites(a []RewriteEntry, host string, qtype uint16) []RewriteEntry { + rr := rewritesSorted{} for _, r := range a { - if r.Domain != host { - if !matchDomainWildcard(host, r.Domain) { - continue - } + if r.Domain != host && !matchDomainWildcard(host, r.Domain) { + continue + } + + // Return CNAMEs for all types requests, but only the + // appropriate ones for A and AAAA. + if r.Type == dns.TypeCNAME || + (r.Type == qtype && (qtype == dns.TypeA || qtype == dns.TypeAAAA)) { + rr = append(rr, r) } - rr = append(rr, r) } if len(rr) == 0 { diff --git a/internal/filtering/rewrites_test.go b/internal/filtering/rewrites_test.go index d3eae9e2..462e059e 100644 --- a/internal/filtering/rewrites_test.go +++ b/internal/filtering/rewrites_test.go @@ -57,59 +57,75 @@ func TestRewrites(t *testing.T) { }, { Domain: "a.host3.com", Answer: "x.host.com", + }, { + Domain: "*.hostboth.com", + Answer: "1.2.3.6", + }, { + Domain: "*.hostboth.com", + Answer: "1234::5678", }} d.prepareRewrites() testCases := []struct { name string host string - dtyp uint16 wantCName string wantVals []net.IP + dtyp uint16 }{{ - name: "not_filtered_not_found", - host: "hoost.com", - dtyp: dns.TypeA, + name: "not_filtered_not_found", + host: "hoost.com", + wantCName: "", + wantVals: nil, + dtyp: dns.TypeA, }, { name: "rewritten_a", host: "www.host.com", - dtyp: dns.TypeA, wantCName: "host.com", wantVals: []net.IP{{1, 2, 3, 4}, {1, 2, 3, 5}}, + dtyp: dns.TypeA, }, { name: "rewritten_aaaa", host: "www.host.com", - dtyp: dns.TypeAAAA, wantCName: "host.com", wantVals: []net.IP{net.ParseIP("1:2:3::4")}, + dtyp: dns.TypeAAAA, }, { - name: "wildcard_match", - host: "abc.host.com", - dtyp: dns.TypeA, - wantVals: []net.IP{{1, 2, 3, 5}}, + name: "wildcard_match", + host: "abc.host.com", + wantCName: "", + wantVals: []net.IP{{1, 2, 3, 5}}, + dtyp: dns.TypeA, }, { - name: "wildcard_override", - host: "a.host.com", - dtyp: dns.TypeA, - wantVals: []net.IP{{1, 2, 3, 4}}, + name: "wildcard_override", + host: "a.host.com", + wantCName: "", + wantVals: []net.IP{{1, 2, 3, 4}}, + dtyp: dns.TypeA, }, { name: "wildcard_cname_interaction", host: "www.host2.com", - dtyp: dns.TypeA, wantCName: "host.com", wantVals: []net.IP{{1, 2, 3, 4}, {1, 2, 3, 5}}, + dtyp: dns.TypeA, }, { name: "two_cnames", host: "b.host.com", - dtyp: dns.TypeA, wantCName: "somehost.com", wantVals: []net.IP{{0, 0, 0, 0}}, + dtyp: dns.TypeA, }, { name: "two_cnames_and_wildcard", host: "b.host3.com", - dtyp: dns.TypeA, wantCName: "x.host.com", wantVals: []net.IP{{1, 2, 3, 5}}, + dtyp: dns.TypeA, + }, { + name: "issue3343", + host: "www.hostboth.com", + wantCName: "", + wantVals: []net.IP{net.ParseIP("1234::5678")}, + dtyp: dns.TypeAAAA, }} for _, tc := range testCases { @@ -143,12 +159,15 @@ func TestRewritesLevels(t *testing.T) { d.Rewrites = []RewriteEntry{{ Domain: "host.com", Answer: "1.1.1.1", + Type: dns.TypeA, }, { Domain: "*.host.com", Answer: "2.2.2.2", + Type: dns.TypeA, }, { Domain: "*.sub.host.com", Answer: "3.3.3.3", + Type: dns.TypeA, }} d.prepareRewrites() @@ -234,53 +253,61 @@ func TestRewritesExceptionIP(t *testing.T) { d.Rewrites = []RewriteEntry{{ Domain: "host.com", Answer: "1.2.3.4", + Type: dns.TypeA, }, { Domain: "host.com", Answer: "AAAA", + Type: dns.TypeAAAA, }, { Domain: "host2.com", Answer: "::1", + Type: dns.TypeAAAA, }, { Domain: "host2.com", Answer: "A", + Type: dns.TypeA, }, { Domain: "host3.com", Answer: "A", + Type: dns.TypeA, }} d.prepareRewrites() testCases := []struct { name string host string - dtyp uint16 want []net.IP + dtyp uint16 }{{ name: "match_A", host: "host.com", - dtyp: dns.TypeA, want: []net.IP{{1, 2, 3, 4}}, + dtyp: dns.TypeA, }, { name: "exception_AAAA_host.com", host: "host.com", + want: nil, dtyp: dns.TypeAAAA, }, { name: "exception_A_host2.com", host: "host2.com", + want: nil, dtyp: dns.TypeA, }, { name: "match_AAAA_host2.com", host: "host2.com", - dtyp: dns.TypeAAAA, want: []net.IP{net.ParseIP("::1")}, + dtyp: dns.TypeAAAA, }, { name: "exception_A_host3.com", host: "host3.com", + want: nil, dtyp: dns.TypeA, }, { name: "match_AAAA_host3.com", host: "host3.com", + want: nil, dtyp: dns.TypeAAAA, - want: []net.IP{}, }} for _, tc := range testCases { @@ -293,7 +320,9 @@ func TestRewritesExceptionIP(t *testing.T) { } assert.Equal(t, Rewritten, r.Reason) + require.Len(t, r.IPList, len(tc.want)) + for _, ip := range tc.want { assert.True(t, ip.Equal(r.IPList[0])) }