Pull request: 1730 bogus nxdomain cidr

Merge in DNS/dnsproxy from 1730-bogus-nxdomain to master Updates AdguardTeam/AdGuardHome#1730. Squashed commit of the following: commit cc58eea Author: Eugene Burkov <[email protected]> Date: Tue Jan 25 16:45:59 2022 +0300 all: imp docs commit 8ca0226 Author: Eugene Burkov <[email protected]> Date: Tue Jan 25 16:06:24 2022 +0300 all: imp testing commit 1d0ffe5 Author: Eugene Burkov <[email protected]> Date: Tue Jan 25 15:15:38 2022 +0300 all: make bogus support cidr
AdguardTeam · Jan 25, 2022 · 3defd67 · 3defd67
1 parent c3e2949
commit 3defd67
Show file tree

Hide file tree

Showing 11 changed files with 363 additions and 267 deletions.
diff --git a/README.md b/README.md
@@ -76,8 +76,9 @@ Application Options:
       --dns64-prefix=    If specified, this is the DNS64 prefix dnsproxy will be using when it works as a DNS64 server. If not
                          specified, dnsproxy uses the 'Well-Known Prefix' 64:ff9b::
       --ipv6-disabled    If specified, all AAAA requests will be replied with NoError RCode and empty answer
-      --bogus-nxdomain=  Transform responses that contain at least one of the given IP addresses into NXDOMAIN. Can be
-                         specified multiple times.
+      --bogus-nxdomain=  Transform the responses containing at least a single IP
+                         that matches specified addresses and CIDRs into
+                         NXDOMAIN.  Can be specified multiple times.
       --udp-buf-size=    Set the size of the UDP buffer in bytes. A value <= 0 will use the system default. (default: 0)
       --max-go-routines= Set the maximum number of go routines. A value <= 0 will not not set a maximum. (default: 0)
       --version          Prints the program version
@@ -269,10 +270,20 @@ Now even if your IP address is 192.168.0.1 and it's not a public IP, the proxy w
 
 ### Bogus NXDomain
 
-This option is similar to dnsmasq `bogus-nxdomain`. If specified, `dnsproxy` transforms responses that contain at least one of the given IP addresses into `NXDOMAIN`. Can be specified multiple times.
+This option is similar to dnsmasq `bogus-nxdomain`.  `dnsproxy` will transform
+responses that contain at least a single IP address which is also specified by
+the option into `NXDOMAIN`. Can be specified multiple times.
 
-In the example below, we use AdGuard DNS server that returns `0.0.0.0` for blocked domains, and transform them to `NXDOMAIN`.
+In the example below, we use AdGuard DNS server that returns `0.0.0.0` for
+blocked domains, and transform them to `NXDOMAIN`.
 
 ```
 ./dnsproxy -u 94.140.14.14:53 --bogus-nxdomain=0.0.0.0
 ```
+
+CIDR ranges are supported as well.  The following will respond with `NXDOMAIN`
+instead of responses containing any IP from `192.168.0.0`-`192.168.255.255`:
+
+```
+./dnsproxy -u 192.168.0.15:53 --bogus-nxdomain=192.168.0.0/16
+```
diff --git a/fastip/fastest.go b/fastip/fastest.go
@@ -67,7 +67,16 @@ func (f *FastestAddr) ExchangeFastest(req *dns.Msg, ups []upstream.Upstream) (
 	}
 
 	host := strings.ToLower(req.Question[0].Name)
-	ips := f.extractIPs(replies)
+
+	ips := make([]net.IP, 0, len(replies))
+	for _, r := range replies {
+		for _, rr := range r.Resp.Answer {
+			ip := proxyutil.IPFromRR(rr)
+			if ip != nil && !containsIP(ips, ip) {
+				ips = append(ips, ip)
+			}
+		}
+	}
 
 	if pingRes := f.pingAll(host, ips); pingRes != nil {
 		return f.prepareReply(pingRes, replies)
@@ -88,7 +97,7 @@ func (f *FastestAddr) prepareReply(pingRes *pingResult, replies []upstream.Excha
 ) {
 	ip := pingRes.ipp.IP
 	for _, r := range replies {
-		if hasAns(r.Resp, ip) {
+		if hasInAns(r.Resp, ip) {
 			m = r.Resp
 			u = r.Upstream
 
@@ -128,10 +137,10 @@ func (f *FastestAddr) prepareReply(pingRes *pingResult, replies []upstream.Excha
 	return m, u, nil
 }
 
-// hasAns returns true if m contains ip in its answer section.
-func hasAns(m *dns.Msg, ip net.IP) (ok bool) {
+// hasInAns returns true if m contains ip in its Answer section.
+func hasInAns(m *dns.Msg, ip net.IP) (ok bool) {
 	for _, rr := range m.Answer {
-		respIP := proxyutil.GetIPFromDNSRecord(rr)
+		respIP := proxyutil.IPFromRR(rr)
 		if respIP != nil && respIP.Equal(ip) {
 			return true
 		}
@@ -140,20 +149,6 @@ func hasAns(m *dns.Msg, ip net.IP) (ok bool) {
 	return false
 }
 
-// extractIPs extracts all IP addresses from results.
-func (f *FastestAddr) extractIPs(results []upstream.ExchangeAllResult) (ips []net.IP) {
-	for _, r := range results {
-		for _, rr := range r.Resp.Answer {
-			ip := proxyutil.GetIPFromDNSRecord(rr)
-			if ip != nil && !containsIP(ips, ip) {
-				ips = append(ips, ip)
-			}
-		}
-	}
-
-	return ips
-}
-
 // containsIP returns true if ips contains the ip.
 func containsIP(ips []net.IP, ip net.IP) (ok bool) {
 	if len(ips) == 0 {

diff --git a/main.go b/main.go
@@ -13,6 +13,7 @@ import (
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/dnsproxy/upstream"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/ameshkov/dnscrypt/v2"
 	goFlags "github.com/jessevdk/go-flags"
 	"gopkg.in/yaml.v3"
@@ -150,7 +151,7 @@ type Options struct {
 	IPv6Disabled bool `yaml:"ipv6-disabled" long:"ipv6-disabled" description:"If specified, all AAAA requests will be replied with NoError RCode and empty answer" optional:"yes" optional-value:"true"`
 
 	// Transform responses that contain at least one of the given IP addresses into NXDOMAIN
-	BogusNXDomain []string `yaml:"bogus-nxdomain" long:"bogus-nxdomain" description:"Transform responses that contain at least one of the given IP addresses into NXDOMAIN. Can be specified multiple times."`
+	BogusNXDomain []string `yaml:"bogus-nxdomain" long:"bogus-nxdomain" description:"Transform the responses containing at least a single IP that matches specified addresses and CIDRs into NXDOMAIN.  Can be specified multiple times."`
 
 	// UDP buffer size value
 	UDPBufferSize int `yaml:"udp-buf-size" long:"udp-buf-size" description:"Set the size of the UDP buffer in bytes. A value <= 0 will use the system default."`
@@ -346,17 +347,19 @@ func initEDNS(config *proxy.Config, options *Options) {
 
 // initBogusNXDomain inits BogusNXDomain structure
 func initBogusNXDomain(config *proxy.Config, options *Options) {
-	if len(options.BogusNXDomain) > 0 {
-		bogusIP := []net.IP{}
-		for _, s := range options.BogusNXDomain {
-			ip := net.ParseIP(s)
-			if ip == nil {
-				log.Error("Invalid IP: %s", s)
-			} else {
-				bogusIP = append(bogusIP, ip)
-			}
+	if len(options.BogusNXDomain) == 0 {
+		return
+	}
+
+	for _, s := range options.BogusNXDomain {
+		subnet, err := netutil.ParseSubnet(s)
+		if err != nil {
+			log.Error("%s", err)
+
+			continue
 		}
-		config.BogusNXDomain = bogusIP
+
+		config.BogusNXDomain = append(config.BogusNXDomain, subnet)
 	}
 }
 

diff --git a/proxy/bogus_nxdomain.go b/proxy/bogus_nxdomain.go
@@ -5,24 +5,21 @@ import (
 	"github.com/miekg/dns"
 )
 
-// isBogusNXDomain - checks if the specified DNS message
-// contains AT LEAST ONE ip address from the Proxy.BogusNXDomain list
-func (p *Proxy) isBogusNXDomain(reply *dns.Msg) bool {
-	if reply == nil ||
-		len(p.BogusNXDomain) == 0 ||
-		len(reply.Answer) == 0 ||
-		(reply.Question[0].Qtype != dns.TypeA &&
-			reply.Question[0].Qtype != dns.TypeAAAA) {
+// isBogusNXDomain returns true if m contains at least a single IP address in
+// the Answer section contained in BogusNXDomain subnets of p.
+func (p *Proxy) isBogusNXDomain(m *dns.Msg) (ok bool) {
+	if m == nil || len(p.BogusNXDomain) == 0 || len(m.Question) == 0 {
+		return false
+	} else if qt := m.Question[0].Qtype; qt != dns.TypeA && qt != dns.TypeAAAA {
 		return false
 	}
 
-	for _, rr := range reply.Answer {
-		ip := proxyutil.GetIPFromDNSRecord(rr)
+	for _, rr := range m.Answer {
+		ip := proxyutil.IPFromRR(rr)
 		if proxyutil.ContainsIP(p.BogusNXDomain, ip) {
 			return true
 		}
 	}
 
-	// No IPs are bogus if we got here
 	return false
 }
diff --git a/proxy/bogus_nxdomain_test.go b/proxy/bogus_nxdomain_test.go
@@ -5,72 +5,99 @@ import (
 	"testing"
 
 	"github.com/AdguardTeam/dnsproxy/upstream"
+	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestBogusNXDomainTypeA(t *testing.T) {
-	dnsProxy := createTestProxy(t, nil)
-	dnsProxy.CacheEnabled = true
-	dnsProxy.BogusNXDomain = []net.IP{net.ParseIP("4.3.2.1")}
+func TestProxy_IsBogusNXDomain(t *testing.T) {
+	prx := createTestProxy(t, nil)
+	prx.CacheEnabled = true
 
-	u := testUpstream{}
-	dnsProxy.UpstreamConfig.Upstreams = []upstream.Upstream{&u}
-	err := dnsProxy.Start()
-	assert.Nil(t, err)
+	prx.BogusNXDomain = []*net.IPNet{{
+		IP:   net.IP{4, 3, 2, 1},
+		Mask: net.CIDRMask(24, netutil.IPv4BitLen),
+	}, {
+		IP:   net.IPv4(1, 2, 3, 4),
+		Mask: net.IPv4Mask(255, 0, 0, 0),
+	}, {
+		IP:   net.IP{10, 11, 12, 13},
+		Mask: net.CIDRMask(netutil.IPv4BitLen, netutil.IPv4BitLen),
+	}, {
+		IP:   net.IP{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
+		Mask: net.CIDRMask(120, netutil.IPv6BitLen),
+	}}
 
-	// first request
-	// upstream answers with a bogus IP
-	u.aResp = &dns.A{
-		Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
-		A:   net.ParseIP("4.3.2.1"),
-	}
+	testCases := []struct {
+		name      string
+		ans       []dns.RR
+		wantRcode int
+	}{{
+		name: "bogus_subnet",
+		ans: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
+			A:   net.ParseIP("4.3.2.1"),
+		}},
+		wantRcode: dns.RcodeNameError,
+	}, {
+		name: "bogus_big_subnet",
+		ans: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
+			A:   net.ParseIP("1.254.254.254"),
+		}},
+		wantRcode: dns.RcodeNameError,
+	}, {
+		name: "bogus_single_ip",
+		ans: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
+			A:   net.ParseIP("10.11.12.13"),
+		}},
+		wantRcode: dns.RcodeNameError,
+	}, {
+		name: "bogus_6",
+		ans: []dns.RR{&dns.AAAA{
+			Hdr:  dns.RR_Header{Rrtype: dns.TypeAAAA, Name: "host.", Ttl: 10},
+			AAAA: net.IP{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 99},
+		}},
+		wantRcode: dns.RcodeNameError,
+	}, {
+		name: "non-bogus",
+		ans: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
+			A:   net.ParseIP("10.11.12.14"),
+		}},
+		wantRcode: dns.RcodeSuccess,
+	}, {
+		name: "non-bogus_6",
+		ans: []dns.RR{&dns.AAAA{
+			Hdr:  dns.RR_Header{Rrtype: dns.TypeAAAA, Name: "host.", Ttl: 10},
+			AAAA: net.IP{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 16, 15},
+		}},
+		wantRcode: dns.RcodeSuccess,
+	}}
 
-	clientIP := net.IP{1, 2, 3, 0}
-	d := DNSContext{}
-	d.Req = createHostTestMessage("host")
-	d.Addr = &net.TCPAddr{
-		IP: clientIP,
-	}
-
-	err = dnsProxy.Resolve(&d)
-	assert.Nil(t, err)
+	u := testUpstream{}
+	prx.UpstreamConfig.Upstreams = []upstream.Upstream{&u}
 
-	// check response
-	assert.NotNil(t, d.Res)
-	assert.Equal(t, dns.RcodeNameError, d.Res.Rcode)
+	err := prx.Start()
+	require.NoError(t, err)
+	testutil.CleanupAndRequireSuccess(t, prx.Stop)
 
-	// second request
-	// upstream answers with a normal IP
-	u.aResp = &dns.A{
-		Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
-		A:   net.ParseIP("4.3.2.2"),
+	d := &DNSContext{
+		Req: createHostTestMessage("host"),
 	}
 
-	err = dnsProxy.Resolve(&d)
-	assert.Nil(t, err)
+	for _, tc := range testCases {
+		u.ans = tc.ans
 
-	// check response
-	assert.NotNil(t, d.Res)
-	assert.Equal(t, dns.RcodeSuccess, d.Res.Rcode)
+		t.Run(tc.name, func(t *testing.T) {
+			err = prx.Resolve(d)
+			require.NoError(t, err)
+			require.NotNil(t, d.Res)
 
-	// third request
-	// upstream answers with two IPs, one of them is bogus
-	u.aRespArr = append(u.aRespArr, &dns.A{
-		Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
-		A:   net.ParseIP("4.3.2.2"),
-	})
-	u.aRespArr = append(u.aRespArr, &dns.A{
-		Hdr: dns.RR_Header{Rrtype: dns.TypeA, Name: "host.", Ttl: 10},
-		A:   net.ParseIP("4.3.2.1"),
-	})
-
-	err = dnsProxy.Resolve(&d)
-	assert.Nil(t, err)
-
-	// check response
-	assert.NotNil(t, d.Res)
-	assert.Equal(t, dns.RcodeSuccess, d.Res.Rcode)
-
-	_ = dnsProxy.Stop()
+			assert.Equal(t, tc.wantRcode, d.Res.Rcode)
+		})
+	}
 }
diff --git a/proxy/cache_test.go b/proxy/cache_test.go
@@ -364,14 +364,14 @@ func TestCacheExpirationWithTTLOverride(t *testing.T) {
 		d.Req = createHostTestMessage("host")
 		d.Addr = &net.TCPAddr{}
 
-		u.aResp = &dns.A{
+		u.ans = []dns.RR{&dns.A{
 			Hdr: dns.RR_Header{
 				Rrtype: dns.TypeA,
 				Name:   "host.",
 				Ttl:    10,
 			},
 			A: net.IP{4, 3, 2, 1},
-		}
+		}}
 
 		err = dnsProxy.Resolve(d)
 		require.NoError(t, err)
@@ -388,14 +388,14 @@ func TestCacheExpirationWithTTLOverride(t *testing.T) {
 		d.Req = createHostTestMessage("host2")
 		d.Addr = &net.TCPAddr{}
 
-		u.aResp = &dns.A{
+		u.ans = []dns.RR{&dns.A{
 			Hdr: dns.RR_Header{
 				Rrtype: dns.TypeA,
 				Name:   "host2.",
 				Ttl:    60,
 			},
 			A: net.IP{4, 3, 2, 1},
-		}
+		}}
 
 		err = dnsProxy.Resolve(d)
 		assert.Nil(t, err)

diff --git a/proxy/config.go b/proxy/config.go
@@ -83,7 +83,7 @@ type Config struct {
 
 	// BogusNXDomain - transforms responses that contain at least one of the given IP addresses into NXDOMAIN
 	// Similar to dnsmasq's "bogus-nxdomain"
-	BogusNXDomain []net.IP
+	BogusNXDomain []*net.IPNet
 
 	// Enable EDNS Client Subnet option
 	// DNS requests to the upstream server will contain an OPT record with Client Subnet option.

diff --git a/proxy/proxy.go b/proxy/proxy.go
@@ -413,7 +413,7 @@ func (p *Proxy) replyFromUpstream(d *DNSContext) (ok bool, err error) {
 		log.Tracef("Received an empty AAAA response, checking DNS64")
 		reply, u, err = p.checkDNS64(req, reply, upstreams)
 	} else if p.isBogusNXDomain(reply) {
-		log.Tracef("Received IP from the bogus-nxdomain list, replacing response")
+		log.Tracef("response ip is contained in bogus-nxdomain list")
 		reply = p.genWithRCode(reply, dns.RcodeNameError)
 	}