Pull request: all: sup many ips in host rules

Closes #1381. Squashed commit of the following: commit 44965f7 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Thu Jun 17 13:54:44 2021 +0300 dnsforward: imp code, docs commit 23736cf Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Wed Jun 16 20:33:59 2021 +0300 dnsforward: rm todo commit ff08675 Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Wed Jun 16 20:32:01 2021 +0300 all: sup many ips in host rules
AdguardTeam · Jun 17, 2021 · 7547d3a · 7547d3a
1 parent 84e71e9
commit 7547d3a
Show file tree

Hide file tree

Showing 7 changed files with 268 additions and 153 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,8 @@ and this project adheres to
 
 ### Changed
 
+- When /etc/hosts-type rules have several IPs for one host, all IPs are now
+  returned instead of only the first one ([#1381]).
 - The setting `rlimit_nofile` is now in the `os` block of the configuration
   file, together with the new `group` and `user` settings ([#2763]).
 - Permissions on filter files are now `0o644` instead of `0o600` ([#3198]).
@@ -63,6 +65,7 @@ released by then.
 
 - Go 1.15 support.
 
+[#1381]: https://github.com/AdguardTeam/AdGuardHome/issues/1381
 [#1691]: https://github.com/AdguardTeam/AdGuardHome/issues/1691
 [#2280]: https://github.com/AdguardTeam/AdGuardHome/issues/2280
 [#2439]: https://github.com/AdguardTeam/AdGuardHome/issues/2439

diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go
@@ -21,6 +21,30 @@ import (
 	"github.com/ameshkov/dnscrypt/v2"
 )
 
+// BlockingMode is an enum of all allowed blocking modes.
+type BlockingMode string
+
+// Allowed blocking modes.
+const (
+	// BlockingModeCustomIP means respond with a custom IP address.
+	BlockingModeCustomIP BlockingMode = "custom_ip"
+
+	// BlockingModeDefault is the same as BlockingModeNullIP for
+	// Adblock-style rules, but responds with the IP address specified in
+	// the rule when blocked by an `/etc/hosts`-style rule.
+	BlockingModeDefault BlockingMode = "default"
+
+	// BlockingModeNullIP means respond with a zero IP address: "0.0.0.0"
+	// for A requests and "::" for AAAA ones.
+	BlockingModeNullIP BlockingMode = "null_ip"
+
+	// BlockingModeNXDOMAIN means respond with the NXDOMAIN code.
+	BlockingModeNXDOMAIN BlockingMode = "nxdomain"
+
+	// BlockingModeREFUSED means respond with the REFUSED code.
+	BlockingModeREFUSED BlockingMode = "refused"
+)
+
 // FilteringConfig represents the DNS filtering configuration of AdGuard Home
 // The zero FilteringConfig is empty and ready for use.
 type FilteringConfig struct {
@@ -38,11 +62,11 @@ type FilteringConfig struct {
 	// Protection configuration
 	// --
 
-	ProtectionEnabled  bool   `yaml:"protection_enabled"`   // whether or not use any of filtering features
-	BlockingMode       string `yaml:"blocking_mode"`        // mode how to answer filtered requests
-	BlockingIPv4       net.IP `yaml:"blocking_ipv4"`        // IP address to be returned for a blocked A request
-	BlockingIPv6       net.IP `yaml:"blocking_ipv6"`        // IP address to be returned for a blocked AAAA request
-	BlockedResponseTTL uint32 `yaml:"blocked_response_ttl"` // if 0, then default is used (3600)
+	ProtectionEnabled  bool         `yaml:"protection_enabled"`   // whether or not use any of filtering features
+	BlockingMode       BlockingMode `yaml:"blocking_mode"`        // mode how to answer filtered requests
+	BlockingIPv4       net.IP       `yaml:"blocking_ipv4"`        // IP address to be returned for a blocked A request
+	BlockingIPv6       net.IP       `yaml:"blocking_ipv6"`        // IP address to be returned for a blocked AAAA request
+	BlockedResponseTTL uint32       `yaml:"blocked_response_ttl"` // if 0, then default is used (3600)
 
 	// IP (or domain name) which is used to respond to DNS requests blocked by parental control or safe-browsing
 	ParentalBlockHost     string `yaml:"parental_block_host"`

diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go
@@ -520,6 +520,7 @@ func TestBlockedRequest(t *testing.T) {
 		TCPListenAddrs: []*net.TCPAddr{{}},
 		FilteringConfig: FilteringConfig{
 			ProtectionEnabled: true,
+			BlockingMode:      BlockingModeDefault,
 		},
 	}
 	s := createTestServer(t, &filtering.Config{}, forwardConf, nil)
@@ -622,6 +623,7 @@ func TestBlockCNAME(t *testing.T) {
 		TCPListenAddrs: []*net.TCPAddr{{}},
 		FilteringConfig: FilteringConfig{
 			ProtectionEnabled: true,
+			BlockingMode:      BlockingModeDefault,
 		},
 	}
 	s := createTestServer(t, &filtering.Config{}, forwardConf, nil)
@@ -724,7 +726,7 @@ func TestNullBlockedRequest(t *testing.T) {
 		TCPListenAddrs: []*net.TCPAddr{{}},
 		FilteringConfig: FilteringConfig{
 			ProtectionEnabled: true,
-			BlockingMode:      "null_ip",
+			BlockingMode:      BlockingModeNullIP,
 		},
 	}
 	s := createTestServer(t, &filtering.Config{}, forwardConf, nil)
@@ -777,7 +779,7 @@ func TestBlockedCustomIP(t *testing.T) {
 		TCPListenAddrs: []*net.TCPAddr{{}},
 		FilteringConfig: FilteringConfig{
 			ProtectionEnabled: true,
-			BlockingMode:      "custom_ip",
+			BlockingMode:      BlockingModeCustomIP,
 			BlockingIPv4:      nil,
 			UpstreamDNS:       []string{"8.8.8.8:53", "8.8.4.4:53"},
 		},
@@ -827,6 +829,7 @@ func TestBlockedByHosts(t *testing.T) {
 		TCPListenAddrs: []*net.TCPAddr{{}},
 		FilteringConfig: FilteringConfig{
 			ProtectionEnabled: true,
+			BlockingMode:      BlockingModeDefault,
 		},
 	}
 

diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go
@@ -29,21 +29,21 @@ type dnsConfig struct {
 	UpstreamsFile *string   `json:"upstream_dns_file"`
 	Bootstraps    *[]string `json:"bootstrap_dns"`
 
-	ProtectionEnabled *bool     `json:"protection_enabled"`
-	RateLimit         *uint32   `json:"ratelimit"`
-	BlockingMode      *string   `json:"blocking_mode"`
-	BlockingIPv4      net.IP    `json:"blocking_ipv4"`
-	BlockingIPv6      net.IP    `json:"blocking_ipv6"`
-	EDNSCSEnabled     *bool     `json:"edns_cs_enabled"`
-	DNSSECEnabled     *bool     `json:"dnssec_enabled"`
-	DisableIPv6       *bool     `json:"disable_ipv6"`
-	UpstreamMode      *string   `json:"upstream_mode"`
-	CacheSize         *uint32   `json:"cache_size"`
-	CacheMinTTL       *uint32   `json:"cache_ttl_min"`
-	CacheMaxTTL       *uint32   `json:"cache_ttl_max"`
-	ResolveClients    *bool     `json:"resolve_clients"`
-	UsePrivateRDNS    *bool     `json:"use_private_ptr_resolvers"`
-	LocalPTRUpstreams *[]string `json:"local_ptr_upstreams"`
+	ProtectionEnabled *bool         `json:"protection_enabled"`
+	RateLimit         *uint32       `json:"ratelimit"`
+	BlockingMode      *BlockingMode `json:"blocking_mode"`
+	BlockingIPv4      net.IP        `json:"blocking_ipv4"`
+	BlockingIPv6      net.IP        `json:"blocking_ipv6"`
+	EDNSCSEnabled     *bool         `json:"edns_cs_enabled"`
+	DNSSECEnabled     *bool         `json:"dnssec_enabled"`
+	DisableIPv6       *bool         `json:"disable_ipv6"`
+	UpstreamMode      *string       `json:"upstream_mode"`
+	CacheSize         *uint32       `json:"cache_size"`
+	CacheMinTTL       *uint32       `json:"cache_ttl_min"`
+	CacheMaxTTL       *uint32       `json:"cache_ttl_max"`
+	ResolveClients    *bool         `json:"resolve_clients"`
+	UsePrivateRDNS    *bool         `json:"use_private_ptr_resolvers"`
+	LocalPTRUpstreams *[]string     `json:"local_ptr_upstreams"`
 }
 
 func (s *Server) getDNSConfig() dnsConfig {
@@ -126,27 +126,17 @@ func (req *dnsConfig) checkBlockingMode() bool {
 		return true
 	}
 
-	bm := *req.BlockingMode
-	if bm == "custom_ip" {
-		if req.BlockingIPv4.To4() == nil {
-			return false
-		}
-
-		return req.BlockingIPv6 != nil
-	}
-
-	for _, valid := range []string{
-		"default",
-		"refused",
-		"nxdomain",
-		"null_ip",
-	} {
-		if bm == valid {
-			return true
-		}
+	switch bm := *req.BlockingMode; bm {
+	case BlockingModeDefault,
+		BlockingModeREFUSED,
+		BlockingModeNXDOMAIN,
+		BlockingModeNullIP:
+		return true
+	case BlockingModeCustomIP:
+		return req.BlockingIPv4.To4() != nil && req.BlockingIPv6 != nil
+	default:
+		return false
 	}
-
-	return false
 }
 
 func (req *dnsConfig) checkUpstreamsMode() bool {

diff --git a/internal/dnsforward/msg.go b/internal/dnsforward/msg.go
@@ -26,17 +26,29 @@ func (s *Server) makeResponse(req *dns.Msg) (resp *dns.Msg) {
 	return resp
 }
 
+// ipsFromRules extracts non-IP addresses from the filtering result rules.
+func ipsFromRules(resRules []*filtering.ResultRule) (ips []net.IP) {
+	for _, r := range resRules {
+		if r.IP != nil {
+			ips = append(ips, r.IP)
+		}
+	}
+
+	return ips
+}
+
 // genDNSFilterMessage generates a DNS message corresponding to the filtering result
 func (s *Server) genDNSFilterMessage(d *proxy.DNSContext, result *filtering.Result) *dns.Msg {
 	m := d.Req
 
 	if m.Question[0].Qtype != dns.TypeA && m.Question[0].Qtype != dns.TypeAAAA {
-		if s.conf.BlockingMode == "null_ip" {
+		if s.conf.BlockingMode == BlockingModeNullIP {
 			return s.makeResponse(m)
 		}
 		return s.genNXDomain(m)
 	}
 
+	ips := ipsFromRules(result.Rules)
 	switch result.Reason {
 	case filtering.FilteredSafeBrowsing:
 		return s.genBlockedHost(m, s.conf.SafeBrowsingBlockHost, d)
@@ -46,42 +58,45 @@ func (s *Server) genDNSFilterMessage(d *proxy.DNSContext, result *filtering.Resu
 		// If the query was filtered by "Safe search", filtering also must return
 		// the IP address that must be used in response.
 		// In this case regardless of the filtering method, we should return it
-		if result.Reason == filtering.FilteredSafeSearch &&
-			len(result.Rules) > 0 &&
-			result.Rules[0].IP != nil {
-			return s.genResponseWithIP(m, result.Rules[0].IP)
+		if result.Reason == filtering.FilteredSafeSearch && len(ips) > 0 {
+			return s.genResponseWithIPs(m, ips)
 		}
 
-		if s.conf.BlockingMode == "null_ip" {
-			// it means that we should return 0.0.0.0 or :: for any blocked request
-			return s.makeResponseNullIP(m)
-		} else if s.conf.BlockingMode == "custom_ip" {
-			// means that we should return custom IP for any blocked request
-
+		switch s.conf.BlockingMode {
+		case BlockingModeCustomIP:
 			switch m.Question[0].Qtype {
 			case dns.TypeA:
 				return s.genARecord(m, s.conf.BlockingIPv4)
 			case dns.TypeAAAA:
 				return s.genAAAARecord(m, s.conf.BlockingIPv6)
+			default:
+				// Generally shouldn't happen, since the types
+				// are checked above.
+				log.Error(
+					"dns: invalid msg type %d for blocking mode %s",
+					m.Question[0].Qtype,
+					s.conf.BlockingMode,
+				)
+
+				return s.makeResponse(m)
+			}
+		case BlockingModeDefault:
+			if len(ips) > 0 {
+				return s.genResponseWithIPs(m, ips)
 			}
-		} else if s.conf.BlockingMode == "nxdomain" {
-			// means that we should return NXDOMAIN for any blocked request
 
+			return s.makeResponseNullIP(m)
+		case BlockingModeNullIP:
+			return s.makeResponseNullIP(m)
+		case BlockingModeNXDOMAIN:
 			return s.genNXDomain(m)
-		} else if s.conf.BlockingMode == "refused" {
-			// means that we should return NXDOMAIN for any blocked request
-
+		case BlockingModeREFUSED:
 			return s.makeResponseREFUSED(m)
-		}
+		default:
+			log.Error("dns: invalid blocking mode %q", s.conf.BlockingMode)
 
-		// Default blocking mode
-		// If there's an IP specified in the rule, return it
-		// For host-type rules, return null IP
-		if len(result.Rules) > 0 && result.Rules[0].IP != nil {
-			return s.genResponseWithIP(m, result.Rules[0].IP)
+			return s.makeResponse(m)
 		}
-
-		return s.makeResponseNullIP(m)
 	}
 }
 
@@ -166,35 +181,60 @@ func (s *Server) genAnswerTXT(req *dns.Msg, strs []string) (ans *dns.TXT) {
 	}
 }
 
-// generate DNS response message with an IP address
-func (s *Server) genResponseWithIP(req *dns.Msg, ip net.IP) *dns.Msg {
-	if req.Question[0].Qtype == dns.TypeA && ip.To4() != nil {
-		return s.genARecord(req, ip.To4())
-	} else if req.Question[0].Qtype == dns.TypeAAAA &&
-		len(ip) == net.IPv6len && ip.To4() == nil {
-		return s.genAAAARecord(req, ip)
+// genResponseWithIPs generates a DNS response message with the provided IP
+// addresses and an appropriate resource record type.  If any of the IPs cannot
+// be converted to the correct protocol, genResponseWithIPs returns an empty
+// response.
+func (s *Server) genResponseWithIPs(req *dns.Msg, ips []net.IP) (resp *dns.Msg) {
+	var ans []dns.RR
+	switch req.Question[0].Qtype {
+	case dns.TypeA:
+		for _, ip := range ips {
+			if ip4 := ip.To4(); ip4 == nil {
+				ans = nil
+
+				break
+			}
+
+			ans = append(ans, s.genAnswerA(req, ip))
+		}
+	case dns.TypeAAAA:
+		for _, ip := range ips {
+			ans = append(ans, s.genAnswerAAAA(req, ip.To16()))
+		}
+	default:
+		// Go on and return an empty response.
 	}
 
-	// empty response
-	resp := s.makeResponse(req)
+	resp = s.makeResponse(req)
+	resp.Answer = ans
+
 	return resp
 }
 
-// Respond with 0.0.0.0 for A, :: for AAAA, empty response for other types
-func (s *Server) makeResponseNullIP(req *dns.Msg) *dns.Msg {
-	if req.Question[0].Qtype == dns.TypeA {
-		return s.genARecord(req, []byte{0, 0, 0, 0})
-	} else if req.Question[0].Qtype == dns.TypeAAAA {
-		return s.genAAAARecord(req, net.IPv6zero)
+// makeResponseNullIP creates a response with 0.0.0.0 for A requests, :: for
+// AAAA requests, and an empty response for other types.
+func (s *Server) makeResponseNullIP(req *dns.Msg) (resp *dns.Msg) {
+	// Respond with the corresponding zero IP type as opposed to simply
+	// using one or the other in both cases, because the IPv4 zero IP is
+	// converted to a IPV6-mapped IPv4 address, while the IPv6 zero IP is
+	// converted into an empty slice instead of the zero IPv4.
+	switch req.Question[0].Qtype {
+	case dns.TypeA:
+		resp = s.genResponseWithIPs(req, []net.IP{{0, 0, 0, 0}})
+	case dns.TypeAAAA:
+		resp = s.genResponseWithIPs(req, []net.IP{net.IPv6zero})
+	default:
+		resp = s.makeResponse(req)
 	}
 
-	return s.makeResponse(req)
+	return resp
 }
 
 func (s *Server) genBlockedHost(request *dns.Msg, newAddr string, d *proxy.DNSContext) *dns.Msg {
 	ip := net.ParseIP(newAddr)
 	if ip != nil {
-		return s.genResponseWithIP(request, ip)
+		return s.genResponseWithIPs(request, []net.IP{ip})
 	}
 
 	// look up the hostname, TODO: cache