Pull request 332: AGDNS-1982 Improve code vol.2

Squashed commit of the following:

commit 7ee49cd
Merge: c9e3f2c feea26e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Mar 28 13:50:57 2024 +0300

    Merge branch 'master' into imp-code-vol.2

commit c9e3f2c
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 27 13:13:01 2024 +0300

    proxy: imp code, docs

commit 1ecf288
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Mar 25 16:06:08 2024 +0300

    proxy: add recursion detector, imp code

commit e4f41d4
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Mar 25 13:45:17 2024 +0300

    proxy: imp code

proxy/bogusnxdomain.go

@@ -1,9 +1,8 @@
 package proxy
 
 import (
-	"net/netip"
-
 	"github.com/AdguardTeam/dnsproxy/proxyutil"
+	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/miekg/dns"
 )
 
@@ -16,23 +15,10 @@ func (p *Proxy) isBogusNXDomain(m *dns.Msg) (ok bool) {
 		return false
 	}
 
+	set := netutil.SliceSubnetSet(p.BogusNXDomain)
 	for _, rr := range m.Answer {
 		ip := proxyutil.IPFromRR(rr)
-		if containsIP(p.BogusNXDomain, ip) {
-			return true
-		}
-	}
-
-	return false
-}
-
-func containsIP(nets []netip.Prefix, ip netip.Addr) (ok bool) {
-	if !ip.IsValid() {
-		return false
-	}
-
-	for _, n := range nets {
-		if n.Contains(ip) {
+		if set.Contains(ip) {
 			return true
 		}
 	}

proxy/bogusnxdomain_test.go

@@ -102,59 +102,3 @@ func TestProxy_IsBogusNXDomain(t *testing.T) {
 		})
 	}
 }
-
-func TestContainsIP(t *testing.T) {
-	nets := []netip.Prefix{
-		netip.MustParsePrefix("1.2.3.0/24"),
-		netip.MustParsePrefix("ffff::1.2.4.0/112"),
-		netip.MustParsePrefix("102:304:506:708:90a:b0c:d0e:f00/120"),
-	}
-
-	testCases := []struct {
-		want assert.BoolAssertionFunc
-		ip   netip.Addr
-		name string
-	}{{
-		name: "ipv4_yes",
-		want: assert.True,
-		ip:   netip.MustParseAddr("1.2.3.255"),
-	}, {
-		name: "ipv4_6_yes",
-		want: assert.True,
-		ip:   netip.MustParseAddr("ffff::1.2.4.254"),
-	}, {
-		name: "ipv6_yes",
-		want: assert.True,
-		ip:   netip.MustParseAddr("102:304:506:708:90a:b0c:d0e:f0f"),
-	}, {
-		name: "ipv6_4_yes",
-		want: assert.True,
-		ip:   netip.MustParseAddr("ffff::1.2.3.0"),
-	}, {
-		name: "ipv4_no",
-		want: assert.False,
-		ip:   netip.MustParseAddr("2.1.3.255"),
-	}, {
-		name: "ipv4_6_no",
-		want: assert.False,
-		ip:   netip.MustParseAddr("2.1.4.254"),
-	}, {
-		name: "ipv6_no",
-		want: assert.False,
-		ip:   netip.MustParseAddr("102:304:506:708:90a:b0c:d0e:10f"),
-	}, {
-		name: "ipv6_4_no",
-		want: assert.False,
-		ip:   netip.MustParseAddr("ffff::2.1.4.0"),
-	}, {
-		name: "invalid",
-		want: assert.False,
-		ip:   netip.Addr{},
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			tc.want(t, containsIP(nets, tc.ip))
-		})
-	}
-}

proxy/config.go

@@ -49,6 +49,10 @@ type Config struct {
 	// value of nil makes Proxy not trust any address.
 	TrustedProxies netutil.SubnetSet
 
+	// MessageConstructor used to build DNS messages.  If nil, the default
+	// constructor will be used.
+	MessageConstructor MessageConstructor
+
 	// BeforeRequestHandler is an optional custom handler called before each DNS
 	// request is started processing.  See [BeforeRequestHandler].
 	BeforeRequestHandler BeforeRequestHandler
@@ -306,7 +310,7 @@ func (p *Proxy) logConfigInfo() {
 	}
 
 	if p.RefuseAny {
-		log.Info("The server is configured to refuse ANY requests")
+		log.Info("dnsproxy: server will refuse requests of type ANY")
 	}
 
 	if len(p.BogusNXDomain) > 0 {

proxy/constructor.go

@@ -0,0 +1,63 @@
+package proxy
+
+import "github.com/miekg/dns"
+
+// MessageConstructor creates DNS messages.
+type MessageConstructor interface {
+	// NewMsgNXDOMAIN creates a new response message replying to req with the
+	// NXDOMAIN code.
+	NewMsgNXDOMAIN(req *dns.Msg) (resp *dns.Msg)
+
+	// NewMsgSERVFAIL creates a new response message replying to req with the
+	// SERVFAIL code.
+	NewMsgSERVFAIL(req *dns.Msg) (resp *dns.Msg)
+
+	// NewMsgNOTIMPLEMENTED creates a new response message replying to req with
+	// the NOTIMPLEMENTED code.
+	NewMsgNOTIMPLEMENTED(req *dns.Msg) (resp *dns.Msg)
+}
+
+// defaultMessageConstructor is a default implementation of MessageConstructor.
+type defaultMessageConstructor struct{}
+
+// type check
+var _ MessageConstructor = defaultMessageConstructor{}
+
+// NewMsgNXDOMAIN implements the [MessageConstructor] interface for
+// defaultMessageConstructor.
+func (defaultMessageConstructor) NewMsgNXDOMAIN(req *dns.Msg) (resp *dns.Msg) {
+	return reply(req, dns.RcodeNameError)
+}
+
+// NewMsgSERVFAIL implements the [MessageConstructor] interface for
+// defaultMessageConstructor.
+func (defaultMessageConstructor) NewMsgSERVFAIL(req *dns.Msg) (resp *dns.Msg) {
+	return reply(req, dns.RcodeServerFailure)
+}
+
+// NewMsgNOTIMPLEMENTED implements the [MessageConstructor] interface for
+// defaultMessageConstructor.
+func (defaultMessageConstructor) NewMsgNOTIMPLEMENTED(req *dns.Msg) (resp *dns.Msg) {
+	resp = reply(req, dns.RcodeNotImplemented)
+
+	// Most of the Internet and especially the inner core has an MTU of at least
+	// 1500 octets.  Maximum DNS/UDP payload size for IPv6 on MTU 1500 ethernet
+	// is 1452 (1500 minus 40 (IPv6 header size) minus 8 (UDP header size)).
+	//
+	// See appendix A of https://datatracker.ietf.org/doc/draft-ietf-dnsop-avoid-fragmentation/17.
+	const maxUDPPayload = 1452
+
+	// NOTIMPLEMENTED without EDNS is treated as 'we don't support EDNS', so
+	// explicitly set it.
+	resp.SetEdns0(maxUDPPayload, false)
+
+	return resp
+}
+
+// reply creates a new response message replying to req with the given code.
+func reply(req *dns.Msg, code int) (resp *dns.Msg) {
+	resp = (&dns.Msg{}).SetRcode(req, code)
+	resp.RecursionAvailable = true
+
+	return resp
+}

proxy/dns64.go

@@ -43,7 +43,7 @@ func (p *Proxy) setupDNS64() (err error) {
 	}
 
 	if len(p.Config.DNS64Prefs) == 0 {
-		p.dns64Prefs = []netip.Prefix{dns64WellKnownPref}
+		p.dns64Prefs = netutil.SliceSubnetSet{dns64WellKnownPref}
 
 		return nil
 	}
@@ -121,7 +121,7 @@ func (p *Proxy) filterNAT64Answers(rrs []dns.RR) (filtered []dns.RR, hasAnswers
 			addr, err := netutil.IPToAddrNoMapped(ans.AAAA)
 			if err != nil {
 				log.Error("dnsproxy: bad aaaa record: %s", err)
-			} else if p.withinDNS64(addr) {
+			} else if p.dns64Prefs.Contains(addr) {
 				// Filter the record.
 				continue
 			} else {
@@ -190,41 +190,39 @@ func (p *Proxy) synthDNS64(origReq, origResp, resp *dns.Msg) (ok bool) {
 // DNS64.  See https://datatracker.ietf.org/doc/html/rfc6052#section-2.1.
 var dns64WellKnownPref = netip.MustParsePrefix("64:ff9b::/96")
 
-// withinDNS64 checks if ip is within one of the configured DNS64 prefixes.
-//
-// TODO(e.burkov):  We actually using bytes of only the first prefix from the
-// set to construct the answer, so consider using some implementation of a
-// prefix set for the rest.
-func (p *Proxy) withinDNS64(ip netip.Addr) (ok bool) {
-	for _, n := range p.dns64Prefs {
-		if n.Contains(ip) {
-			return true
-		}
-	}
-
-	return false
-}
-
-// shouldStripDNS64 returns true if DNS64 is enabled and addr has either one of
-// custom DNS64 prefixes or the Well-Known one.  This is intended to be used
-// with PTR requests.
+// shouldStripDNS64 returns true if DNS64 is enabled and req is a PTR for a
+// reversed address within either one of custom DNS64 prefixes or the Well-Known
+// one.
 //
 // The requirement is to match any Pref64::/n used at the site, and not merely
 // the locally configured Pref64::/n.  This is because end clients could ask for
 // a PTR record matching an address received through a different (site-provided)
 // DNS64.
 //
 // See https://datatracker.ietf.org/doc/html/rfc6147#section-5.3.1.
-func (p *Proxy) shouldStripDNS64(addr netip.Addr) (ok bool) {
+func (p *Proxy) shouldStripDNS64(req *dns.Msg) (ok bool) {
 	if len(p.dns64Prefs) == 0 {
 		return false
 	}
 
+	q := req.Question[0]
+	if q.Qtype != dns.TypePTR {
+		return false
+	}
+
+	host := q.Name
+	ip, err := netutil.IPFromReversedAddr(host)
+	if err != nil {
+		log.Debug("dnsproxy: failed to parse ip from ptr request: %s", err)
+
+		return false
+	}
+
 	switch {
-	case p.withinDNS64(addr):
-		log.Debug("dnsproxy: %s is within DNS64 custom prefix set", addr)
-	case dns64WellKnownPref.Contains(addr):
-		log.Debug("dnsproxy: %s is within DNS64 well-known prefix", addr)
+	case p.dns64Prefs.Contains(ip):
+		log.Debug("dnsproxy: %s is within DNS64 custom prefix set", ip)
+	case dns64WellKnownPref.Contains(ip):
+		log.Debug("dnsproxy: %s is within DNS64 well-known prefix", ip)
 	default:
 		return false
 	}