Merge: - filtering: fix host rules matching

Close #1365

Squashed commit of the following:

commit 9cbca2d330ae12b222633201f4864abb7f7cd7a3
Merge: 8ce6b015 be93dc34
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Thu Jan 30 19:03:21 2020 +0300

    Merge remote-tracking branch 'origin/master' into 1365-hostrules

commit 8ce6b0151a2b552c4ccb3ee1f7e36ce260ba96ea
Merge: c752ab33 5c814b29
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Thu Jan 30 18:57:20 2020 +0300

    Merge remote-tracking branch 'origin/master' into 1365-hostrules

commit c752ab33b074312f10772467436a27a90339a919
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Thu Jan 30 14:18:58 2020 +0300

    use new Match()

commit ce2f628aca9f934c776c8c690813efeed5d5427b
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Thu Jan 30 12:03:21 2020 +0300

    minor

commit ebebe02a63821fedd3904db384406c30de52d515
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Thu Jan 30 11:21:47 2020 +0300

    * dnsfilter: use new version of urlfilter's Match()

commit 84edc44f2ee5a67316114f048740825259cc87ff
Author: Simon Zolin <s.zolin@adguard.com>
Date:   Fri Jan 24 14:10:41 2020 +0300

    - filtering: fix host rules matching

    Match by both IPv4 and IPv6 rules, not just the first one in list.

dnsfilter/dnsfilter.go

@@ -478,53 +478,72 @@ func (d *Dnsfilter) initFiltering(filters map[int]string) error {
 // matchHost is a low-level way to check only if hostname is filtered by rules, skipping expensive safebrowsing and parental lookups
 func (d *Dnsfilter) matchHost(host string, qtype uint16, ctags []string) (Result, error) {
 	d.engineLock.RLock()
+	// Keep in mind that this lock must be held no just when calling Match()
+	//  but also while using the rules returned by it.
 	defer d.engineLock.RUnlock()
 	if d.filteringEngine == nil {
 		return Result{}, nil
 	}
 
-	frules, ok := d.filteringEngine.Match(host, ctags)
+	rr, ok := d.filteringEngine.Match(host, ctags)
 	if !ok {
 		return Result{}, nil
 	}
 
-	log.Tracef("%d rules matched for host '%s'", len(frules), host)
+	if rr.NetworkRule != nil {
-
+		log.Debug("Filtering: found rule for host '%s': '%s'  list_id: %d",
-	for _, rule := range frules {
+			host, rr.NetworkRule.Text(), rr.NetworkRule.GetFilterListID())
-
-		log.Tracef("Found rule for host '%s': '%s'  list_id: %d",
-			host, rule.Text(), rule.GetFilterListID())
-
 		res := Result{}
+		res.FilterID = int64(rr.NetworkRule.GetFilterListID())
+		res.Rule = rr.NetworkRule.Text()
+
 		res.Reason = FilteredBlackList
 		res.IsFiltered = true
-		res.FilterID = int64(rule.GetFilterListID())
+		if rr.NetworkRule.Whitelist {
-		res.Rule = rule.Text()
-
-		if netRule, ok := rule.(*rules.NetworkRule); ok {
-
-			if netRule.Whitelist {
 			res.Reason = NotFilteredWhiteList
 			res.IsFiltered = false
 		}
 		return res, nil
-
-		} else if hostRule, ok := rule.(*rules.HostRule); ok {
-
-			res.IP = net.IP{}
-			if qtype == dns.TypeA && hostRule.IP.To4() != nil {
-				// either IPv4 or IPv4-mapped IPv6 address
-				res.IP = hostRule.IP.To4()
-
-			} else if qtype == dns.TypeAAAA && hostRule.IP.To4() == nil {
-				res.IP = hostRule.IP
 	}
+
+	if qtype == dns.TypeA && rr.HostRulesV4 != nil {
+		rule := rr.HostRulesV4[0] // note that we process only 1 matched rule
+		res := Result{}
+		res.FilterID = int64(rule.GetFilterListID())
+		res.Rule = rule.Text()
+		res.Reason = FilteredBlackList
+		res.IsFiltered = true
+		res.IP = rule.IP.To4()
 		return res, nil
-
-		} else {
-			log.Tracef("Rule type is unsupported: '%s'  list_id: %d",
-				rule.Text(), rule.GetFilterListID())
 	}
+
+	if qtype == dns.TypeAAAA && rr.HostRulesV6 != nil {
+		rule := rr.HostRulesV6[0] // note that we process only 1 matched rule
+		res := Result{}
+		res.FilterID = int64(rule.GetFilterListID())
+		res.Rule = rule.Text()
+		res.Reason = FilteredBlackList
+		res.IsFiltered = true
+		res.IP = rule.IP
+		return res, nil
+	}
+
+	if rr.HostRulesV4 != nil || rr.HostRulesV6 != nil {
+		// Question Type doesn't match the host rules
+		// Return the first matched host rule, but without an IP address
+		res := Result{}
+		res.Reason = FilteredBlackList
+		res.IsFiltered = true
+		var rule rules.Rule
+		if rr.HostRulesV4 != nil {
+			rule = rr.HostRulesV4[0]
+		} else if rr.HostRulesV6 != nil {
+			rule = rr.HostRulesV6[0]
+		}
+		res.FilterID = int64(rule.GetFilterListID())
+		res.Rule = rule.Text()
+		res.IP = net.IP{}
+		return res, nil
 	}
 
 	return Result{}, nil
@@ -581,6 +600,9 @@ func New(c *Config, filters map[int]string) *Dnsfilter {
 	return d
 }
 
+// Start - start the module:
+// . start async filtering initializer goroutine
+// . register web handlers
 func (d *Dnsfilter) Start() {
 	d.filtersInitializerChan = make(chan filtersInitializerParams, 1)
 	go d.filtersInitializer()

dnsfilter/dnsfilter_test.go

@@ -98,7 +98,13 @@ func (d *Dnsfilter) checkMatchEmpty(t *testing.T, hostname string) {
 func TestEtcHostsMatching(t *testing.T) {
 	addr := "216.239.38.120"
 	addr6 := "::1"
-	text := fmt.Sprintf("   %s  google.com www.google.com   # enforce google's safesearch   \n%s  ipv6.com\n0.0.0.0 block.com\n",
+	text := fmt.Sprintf(`  %s  google.com www.google.com   # enforce google's safesearch
+%s  ipv6.com
+0.0.0.0 block.com
+0.0.0.1 host2
+0.0.0.2 host2
+::1 host2
+`,
 		addr, addr6)
 	filters := make(map[int]string)
 	filters[0] = text
@@ -116,6 +122,7 @@ func TestEtcHostsMatching(t *testing.T) {
 	// ...but empty IPv6
 	ret, err := d.CheckHost("block.com", dns.TypeAAAA, &setts)
 	assert.True(t, err == nil && ret.IsFiltered && ret.IP != nil && len(ret.IP) == 0)
+	assert.True(t, ret.Rule == "0.0.0.0 block.com")
 
 	// IPv6
 	d.checkMatchIP(t, "ipv6.com", addr6, dns.TypeAAAA)
@@ -123,6 +130,16 @@ func TestEtcHostsMatching(t *testing.T) {
 	// ...but empty IPv4
 	ret, err = d.CheckHost("ipv6.com", dns.TypeA, &setts)
 	assert.True(t, err == nil && ret.IsFiltered && ret.IP != nil && len(ret.IP) == 0)
+
+	// 2 IPv4 (return only the first one)
+	ret, err = d.CheckHost("host2", dns.TypeA, &setts)
+	assert.True(t, err == nil && ret.IsFiltered)
+	assert.True(t, ret.IP != nil && ret.IP.Equal(net.ParseIP("0.0.0.1")))
+
+	// ...and 1 IPv6 address
+	ret, err = d.CheckHost("host2", dns.TypeAAAA, &setts)
+	assert.True(t, err == nil && ret.IsFiltered)
+	assert.True(t, ret.IP != nil && ret.IP.Equal(net.ParseIP("::1")))
 }
 
 // SAFE BROWSING

go.mod

@@ -5,7 +5,7 @@ go 1.13
 require (
 	github.com/AdguardTeam/dnsproxy v0.23.7
 	github.com/AdguardTeam/golibs v0.3.0
-	github.com/AdguardTeam/urlfilter v0.8.1
+	github.com/AdguardTeam/urlfilter v0.9.1
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/etcd-io/bbolt v1.3.3
 	github.com/go-test/deep v1.0.4 // indirect

go.sum

@@ -7,8 +7,8 @@ github.com/AdguardTeam/golibs v0.3.0/go.mod h1:R3M+mAg3nWG4X4Hsag5eef/TckHFH12ZY
 github.com/AdguardTeam/gomitmproxy v0.1.2/go.mod h1:Mrt/3EfiXIYY2aZ7KsLuCUJzUARD/fWJ119IfzOB13M=
 github.com/AdguardTeam/urlfilter v0.7.0 h1:ffFLt4rA3GX8PJYGL3bGcT5bSxZlML5k6cKpSeN2UI8=
 github.com/AdguardTeam/urlfilter v0.7.0/go.mod h1:GHXPzEG59ezyff22lXSQ7dicj1kFZBrH5kmZ6EvQzfk=
-github.com/AdguardTeam/urlfilter v0.8.1 h1:9YRQOR15DU7+k01PWAgc/Ay12jjxVqSi6P0+whFm0f4=
+github.com/AdguardTeam/urlfilter v0.9.1 h1:H0q1xig3mZjIEDH0/o2U/ezydwKGwxtQ56hz6LKPN2M=
-github.com/AdguardTeam/urlfilter v0.8.1/go.mod h1:GHXPzEG59ezyff22lXSQ7dicj1kFZBrH5kmZ6EvQzfk=
+github.com/AdguardTeam/urlfilter v0.9.1/go.mod h1:GHXPzEG59ezyff22lXSQ7dicj1kFZBrH5kmZ6EvQzfk=
 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=
 github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c=
 github.com/StackExchange/wmi v0.0.0-20181212234831-e0a55b97c705 h1:UUppSQnhf4Yc6xGxSkoQpPhb7RVzuv5Nb1mwJ5VId9s=