From 338209f32bdb876da19e52c366e440ea3c457470 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 28 Dec 2020 18:41:50 +0300 Subject: [PATCH] Pull request: all: improve dnsrewrite handling Merge in DNS/adguard-home from 2491-dnsrewrite-log to master Closes #2491. Squashed commit of the following: commit bfe3cb599ed0a921285fb1a6ea27aaefdcc0d093 Merge: 95c5ffe43 15d8f979b Author: Ainar Garipov Date: Mon Dec 28 18:33:32 2020 +0300 Merge branch 'master' into 2491-dnsrewrite-log commit 95c5ffe4360b732556455f24b844dad27047e64b Author: Artem Baskal Date: Mon Dec 28 18:11:01 2020 +0300 Add RewriteRule for client commit b9096c8789009dac1838b542d3409fef54b59aa5 Author: Ainar Garipov Date: Mon Dec 28 17:22:44 2020 +0300 all: imp naming, docs commit 4e00de0d613e4740451e4c8eb5a1de35a70a5896 Author: Ainar Garipov Date: Mon Dec 28 17:16:35 2020 +0300 all: imp naming, add todo commit 67e4045f627a9569f382309705963640dcf3454a Author: Ainar Garipov Date: Mon Dec 28 16:53:00 2020 +0300 all: improve dnsrewrite handling --- client/src/helpers/constants.js | 5 ++++ internal/dnsfilter/dnsfilter.go | 46 +++++++++++++++-------------- internal/dnsfilter/dnsrewrite.go | 6 ++-- internal/dnsfilter/rewrites_test.go | 32 ++++++++++---------- internal/dnsforward/dns.go | 4 +-- internal/dnsforward/filter.go | 8 ++--- internal/home/controlfiltering.go | 2 +- internal/querylog/qlog_test.go | 2 +- internal/querylog/searchcriteria.go | 11 +++++-- openapi/CHANGELOG.md | 2 +- openapi/openapi.yaml | 4 +-- 11 files changed, 67 insertions(+), 55 deletions(-) diff --git a/client/src/helpers/constants.js b/client/src/helpers/constants.js index af10524f..76be76a6 100644 --- a/client/src/helpers/constants.js +++ b/client/src/helpers/constants.js @@ -339,6 +339,7 @@ export const FILTERED_STATUS = { FILTERED_BLOCKED_SERVICE: 'FilteredBlockedService', REWRITE: 'Rewrite', REWRITE_HOSTS: 'RewriteEtcHosts', + REWRITE_RULE: 'RewriteRule', FILTERED_SAFE_SEARCH: 'FilteredSafeSearch', FILTERED_SAFE_BROWSING: 'FilteredSafeBrowsing', FILTERED_PARENTAL: 'FilteredParental', @@ -430,6 +431,10 @@ export const FILTERED_STATUS_TO_META_MAP = { LABEL: RESPONSE_FILTER.REWRITTEN.LABEL, COLOR: QUERY_STATUS_COLORS.BLUE, }, + [FILTERED_STATUS.REWRITE_RULE]: { + LABEL: RESPONSE_FILTER.REWRITTEN.LABEL, + COLOR: QUERY_STATUS_COLORS.BLUE, + }, [FILTERED_STATUS.FILTERED_SAFE_BROWSING]: { LABEL: RESPONSE_FILTER.BLOCKED_THREATS.LABEL, COLOR: QUERY_STATUS_COLORS.YELLOW, diff --git a/internal/dnsfilter/dnsfilter.go b/internal/dnsfilter/dnsfilter.go index 4a1b255e..c5c28aff 100644 --- a/internal/dnsfilter/dnsfilter.go +++ b/internal/dnsfilter/dnsfilter.go @@ -148,17 +148,21 @@ const ( // FilteredBlockedService - the host is blocked by "blocked services" settings FilteredBlockedService - // ReasonRewrite is returned when there was a rewrite by - // a legacy DNS Rewrite rule. - ReasonRewrite + // Rewritten is returned when there was a rewrite by a legacy DNS + // rewrite rule. + Rewritten - // RewriteAutoHosts is returned when there was a rewrite by - // autohosts rules (/etc/hosts and so on). - RewriteAutoHosts + // RewrittenAutoHosts is returned when there was a rewrite by autohosts + // rules (/etc/hosts and so on). + RewrittenAutoHosts - // DNSRewriteRule is returned when a $dnsrewrite filter rule was - // applied. - DNSRewriteRule + // RewrittenRule is returned when a $dnsrewrite filter rule was applied. + // + // TODO(a.garipov): Remove Rewritten and RewrittenAutoHosts by merging + // their functionality into RewrittenRule. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/2499. + RewrittenRule ) // TODO(a.garipov): Resync with actual code names or replace completely @@ -175,11 +179,9 @@ var reasonNames = []string{ FilteredSafeSearch: "FilteredSafeSearch", FilteredBlockedService: "FilteredBlockedService", - ReasonRewrite: "Rewrite", - - RewriteAutoHosts: "RewriteEtcHosts", - - DNSRewriteRule: "DNSRewriteRule", + Rewritten: "Rewrite", + RewrittenAutoHosts: "RewriteEtcHosts", + RewrittenRule: "RewriteRule", } func (r Reason) String() string { @@ -331,15 +333,15 @@ type Result struct { Rules []*ResultRule `json:",omitempty"` // ReverseHosts is the reverse lookup rewrite result. It is - // empty unless Reason is set to RewriteAutoHosts. + // empty unless Reason is set to RewrittenAutoHosts. ReverseHosts []string `json:",omitempty"` // IPList is the lookup rewrite result. It is empty unless - // Reason is set to RewriteAutoHosts or ReasonRewrite. + // Reason is set to RewrittenAutoHosts or Rewritten. IPList []net.IP `json:",omitempty"` // CanonName is the CNAME value from the lookup rewrite result. - // It is empty unless Reason is set to ReasonRewrite. + // It is empty unless Reason is set to Rewritten or RewrittenRule. CanonName string `json:",omitempty"` // ServiceName is the name of the blocked service. It is empty @@ -379,7 +381,7 @@ func (d *DNSFilter) CheckHost(host string, qtype uint16, setts *RequestFiltering // first - check rewrites, they have the highest priority result = d.processRewrites(host, qtype) - if result.Reason == ReasonRewrite { + if result.Reason == Rewritten { return result, nil } @@ -453,7 +455,7 @@ func (d *DNSFilter) CheckHost(host string, qtype uint16, setts *RequestFiltering func (d *DNSFilter) checkAutoHosts(host string, qtype uint16, result *Result) (matched bool) { ips := d.Config.AutoHosts.Process(host, qtype) if ips != nil { - result.Reason = RewriteAutoHosts + result.Reason = RewrittenAutoHosts result.IPList = ips return true @@ -461,7 +463,7 @@ func (d *DNSFilter) checkAutoHosts(host string, qtype uint16, result *Result) (m revHosts := d.Config.AutoHosts.ProcessReverse(host, qtype) if len(revHosts) != 0 { - result.Reason = RewriteAutoHosts + result.Reason = RewrittenAutoHosts // TODO(a.garipov): Optimize this with a buffer. result.ReverseHosts = make([]string, len(revHosts)) @@ -488,7 +490,7 @@ func (d *DNSFilter) processRewrites(host string, qtype uint16) (res Result) { rr := findRewrites(d.Rewrites, host) if len(rr) != 0 { - res.Reason = ReasonRewrite + res.Reason = Rewritten } cnames := map[string]bool{} @@ -696,7 +698,7 @@ func (d *DNSFilter) matchHost(host string, qtype uint16, setts RequestFilteringS // awkward. if dnsr := dnsres.DNSRewrites(); len(dnsr) > 0 { res = d.processDNSRewrites(dnsr) - if res.Reason == DNSRewriteRule && res.CanonName == host { + if res.Reason == RewrittenRule && res.CanonName == host { // A rewrite of a host to itself. Go on and // try matching other things. } else { diff --git a/internal/dnsfilter/dnsrewrite.go b/internal/dnsfilter/dnsrewrite.go index 66cb5828..15e4e211 100644 --- a/internal/dnsfilter/dnsrewrite.go +++ b/internal/dnsfilter/dnsrewrite.go @@ -39,7 +39,7 @@ func (d *DNSFilter) processDNSRewrites(dnsr []*rules.NetworkRule) (res Result) { }} return Result{ - Reason: DNSRewriteRule, + Reason: RewrittenRule, Rules: rules, CanonName: dr.NewCNAME, } @@ -65,7 +65,7 @@ func (d *DNSFilter) processDNSRewrites(dnsr []*rules.NetworkRule) (res Result) { } return Result{ - Reason: DNSRewriteRule, + Reason: RewrittenRule, Rules: rules, DNSRewriteResult: dnsrr, } @@ -73,7 +73,7 @@ func (d *DNSFilter) processDNSRewrites(dnsr []*rules.NetworkRule) (res Result) { } return Result{ - Reason: DNSRewriteRule, + Reason: RewrittenRule, Rules: rules, DNSRewriteResult: dnsrr, } diff --git a/internal/dnsfilter/rewrites_test.go b/internal/dnsfilter/rewrites_test.go index 4304b6de..3a3284ec 100644 --- a/internal/dnsfilter/rewrites_test.go +++ b/internal/dnsfilter/rewrites_test.go @@ -25,14 +25,14 @@ func TestRewrites(t *testing.T) { assert.Equal(t, NotFilteredNotFound, r.Reason) r = d.processRewrites("www.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, "host.com", r.CanonName) assert.Equal(t, 2, len(r.IPList)) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) assert.True(t, r.IPList[1].Equal(net.ParseIP("1.2.3.5"))) r = d.processRewrites("www.host.com", dns.TypeAAAA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, "host.com", r.CanonName) assert.Equal(t, 1, len(r.IPList)) assert.True(t, r.IPList[0].Equal(net.ParseIP("1:2:3::4"))) @@ -44,11 +44,11 @@ func TestRewrites(t *testing.T) { } d.prepareRewrites() r = d.processRewrites("host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) r = d.processRewrites("www.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.5"))) r = d.processRewrites("www.host2.com", dns.TypeA) @@ -61,7 +61,7 @@ func TestRewrites(t *testing.T) { } d.prepareRewrites() r = d.processRewrites("a.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.True(t, len(r.IPList) == 1) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) @@ -72,7 +72,7 @@ func TestRewrites(t *testing.T) { } d.prepareRewrites() r = d.processRewrites("www.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, "host.com", r.CanonName) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) @@ -84,7 +84,7 @@ func TestRewrites(t *testing.T) { } d.prepareRewrites() r = d.processRewrites("b.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, "host.com", r.CanonName) assert.True(t, len(r.IPList) == 1) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) @@ -97,7 +97,7 @@ func TestRewrites(t *testing.T) { } d.prepareRewrites() r = d.processRewrites("b.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, "x.somehost.com", r.CanonName) assert.True(t, len(r.IPList) == 1) assert.True(t, r.IPList[0].Equal(net.ParseIP("1.2.3.4"))) @@ -115,19 +115,19 @@ func TestRewritesLevels(t *testing.T) { // match exact r := d.processRewrites("host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "1.1.1.1", r.IPList[0].String()) // match L2 r = d.processRewrites("sub.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "2.2.2.2", r.IPList[0].String()) // match L3 r = d.processRewrites("my.sub.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "3.3.3.3", r.IPList[0].String()) } @@ -143,7 +143,7 @@ func TestRewritesExceptionCNAME(t *testing.T) { // match sub-domain r := d.processRewrites("my.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "2.2.2.2", r.IPList[0].String()) @@ -163,7 +163,7 @@ func TestRewritesExceptionWC(t *testing.T) { // match sub-domain r := d.processRewrites("my.host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "2.2.2.2", r.IPList[0].String()) @@ -186,7 +186,7 @@ func TestRewritesExceptionIP(t *testing.T) { // match domain r := d.processRewrites("host.com", dns.TypeA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "1.2.3.4", r.IPList[0].String()) @@ -200,7 +200,7 @@ func TestRewritesExceptionIP(t *testing.T) { // match domain r = d.processRewrites("host2.com", dns.TypeAAAA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 1, len(r.IPList)) assert.Equal(t, "::1", r.IPList[0].String()) @@ -210,6 +210,6 @@ func TestRewritesExceptionIP(t *testing.T) { // match domain r = d.processRewrites("host3.com", dns.TypeAAAA) - assert.Equal(t, ReasonRewrite, r.Reason) + assert.Equal(t, Rewritten, r.Reason) assert.Equal(t, 0, len(r.IPList)) } diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 0e5eb51f..c6c5a6b6 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -366,8 +366,8 @@ func processFilteringAfterResponse(ctx *dnsContext) int { var err error switch res.Reason { - case dnsfilter.ReasonRewrite, - dnsfilter.DNSRewriteRule: + case dnsfilter.Rewritten, + dnsfilter.RewrittenRule: if len(ctx.origQuestion.Name) == 0 { // origQuestion is set in case we get only CNAME without IP from rewrites table diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go index c6bfb160..80cf26dd 100644 --- a/internal/dnsforward/filter.go +++ b/internal/dnsforward/filter.go @@ -55,7 +55,7 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*dnsfilter.Result, error) { } else if res.IsFiltered { log.Tracef("Host %s is filtered, reason - %q, matched rule: %q", host, res.Reason, res.Rules[0].Text) d.Res = s.genDNSFilterMessage(d, &res) - } else if res.Reason.In(dnsfilter.ReasonRewrite, dnsfilter.DNSRewriteRule) && + } else if res.Reason.In(dnsfilter.Rewritten, dnsfilter.RewrittenRule) && res.CanonName != "" && len(res.IPList) == 0 { // Resolve the new canonical name, not the original host @@ -63,7 +63,7 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*dnsfilter.Result, error) { // processFilteringAfterResponse. ctx.origQuestion = d.Req.Question[0] d.Req.Question[0].Name = dns.Fqdn(res.CanonName) - } else if res.Reason == dnsfilter.RewriteAutoHosts && len(res.ReverseHosts) != 0 { + } else if res.Reason == dnsfilter.RewrittenAutoHosts && len(res.ReverseHosts) != 0 { resp := s.makeResponse(req) for _, h := range res.ReverseHosts { hdr := dns.RR_Header{ @@ -82,7 +82,7 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*dnsfilter.Result, error) { } d.Res = resp - } else if res.Reason == dnsfilter.ReasonRewrite || res.Reason == dnsfilter.RewriteAutoHosts { + } else if res.Reason == dnsfilter.Rewritten || res.Reason == dnsfilter.RewrittenAutoHosts { resp := s.makeResponse(req) name := host @@ -104,7 +104,7 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*dnsfilter.Result, error) { } d.Res = resp - } else if res.Reason == dnsfilter.DNSRewriteRule { + } else if res.Reason == dnsfilter.RewrittenRule { err = s.filterDNSRewrite(req, res, d) if err != nil { return nil, err diff --git a/internal/home/controlfiltering.go b/internal/home/controlfiltering.go index 1d0172e8..65e023b2 100644 --- a/internal/home/controlfiltering.go +++ b/internal/home/controlfiltering.go @@ -369,7 +369,7 @@ type checkHostResp struct { // for FilteredBlockedService: SvcName string `json:"service_name"` - // for ReasonRewrite: + // for Rewritten: CanonName string `json:"cname"` // CNAME value IPList []net.IP `json:"ip_addrs"` // list of IP addresses } diff --git a/internal/querylog/qlog_test.go b/internal/querylog/qlog_test.go index fd37a1de..dfd4e6ce 100644 --- a/internal/querylog/qlog_test.go +++ b/internal/querylog/qlog_test.go @@ -236,7 +236,7 @@ func addEntry(l *queryLog, host, answerStr, client string) { a.Answer = append(a.Answer, answer) res := dnsfilter.Result{ IsFiltered: true, - Reason: dnsfilter.ReasonRewrite, + Reason: dnsfilter.Rewritten, ServiceName: "SomeService", Rules: []*dnsfilter.ResultRule{{ FilterListID: 1, diff --git a/internal/querylog/searchcriteria.go b/internal/querylog/searchcriteria.go index b98e0838..f6b0ee98 100644 --- a/internal/querylog/searchcriteria.go +++ b/internal/querylog/searchcriteria.go @@ -116,8 +116,9 @@ func (c *searchCriteria) ctFilteringStatusCase(res dnsfilter.Result) bool { return res.IsFiltered || res.Reason.In( dnsfilter.NotFilteredAllowList, - dnsfilter.ReasonRewrite, - dnsfilter.RewriteAutoHosts, + dnsfilter.Rewritten, + dnsfilter.RewrittenAutoHosts, + dnsfilter.RewrittenRule, ) case filteringStatusBlocked: @@ -137,7 +138,11 @@ func (c *searchCriteria) ctFilteringStatusCase(res dnsfilter.Result) bool { return res.Reason == dnsfilter.NotFilteredAllowList case filteringStatusRewritten: - return res.Reason.In(dnsfilter.ReasonRewrite, dnsfilter.RewriteAutoHosts) + return res.Reason.In( + dnsfilter.Rewritten, + dnsfilter.RewrittenAutoHosts, + dnsfilter.RewrittenRule, + ) case filteringStatusSafeSearch: return res.IsFiltered && res.Reason == dnsfilter.FilteredSafeSearch diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index 076d9896..133664b1 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -6,7 +6,7 @@ ### New `"reason"` in `GET /filtering/check_host` and `GET /querylog` -* The new `DNSRewriteRule` reason is added to `GET /filtering/check_host` and +* The new `RewriteRule` reason is added to `GET /filtering/check_host` and `GET /querylog`. * Also, the reason which was incorrectly documented as `"ReasonRewrite"` is now diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 61db836e..8b3ef126 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1259,7 +1259,7 @@ - 'FilteredBlockedService' - 'Rewrite' - 'RewriteEtcHosts' - - 'DNSRewriteRule' + - 'RewriteRule' 'filter_id': 'deprecated': true 'description': > @@ -1663,7 +1663,7 @@ - 'FilteredBlockedService' - 'Rewrite' - 'RewriteEtcHosts' - - 'DNSRewriteRule' + - 'RewriteRule' 'service_name': 'type': 'string' 'description': 'Set if reason=FilteredBlockedService'