Pull request: all: support more $dnsrewrite rr types

Merge in DNS/adguard-home from 2102-dnsrewrite-2 to master

Updates #2102.
Updates #2452.

Squashed commit of the following:

commit b41e57731b4f276e97202e172fa400067174653d
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Dec 22 20:40:56 2020 +0300

    dnsforward: improve naming

commit 70b832ce969d8cdcf4224d221e5e1e2057fba6ee
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Dec 22 19:36:21 2020 +0300

    all: support more $dnsrewrite rr types

go.mod

@@ -5,7 +5,7 @@ go 1.14
 require (
 	github.com/AdguardTeam/dnsproxy v0.33.7
 	github.com/AdguardTeam/golibs v0.4.4
-	github.com/AdguardTeam/urlfilter v0.14.0
+	github.com/AdguardTeam/urlfilter v0.14.1
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/ameshkov/dnscrypt/v2 v2.0.1
 	github.com/fsnotify/fsnotify v1.4.9

go.sum

@@ -26,8 +26,8 @@ github.com/AdguardTeam/golibs v0.4.2/go.mod h1:skKsDKIBB7kkFflLJBpfGX+G8QFTx0WKU
 github.com/AdguardTeam/golibs v0.4.4 h1:cM9UySQiYFW79zo5XRwnaIWVzfW4eNXmZktMrWbthpw=
 github.com/AdguardTeam/golibs v0.4.4/go.mod h1:skKsDKIBB7kkFflLJBpfGX+G8QFTx0WKUzB6TIgtUj4=
 github.com/AdguardTeam/gomitmproxy v0.2.0/go.mod h1:Qdv0Mktnzer5zpdpi5rAwixNJzW2FN91LjKJCkVbYGU=
-github.com/AdguardTeam/urlfilter v0.14.0 h1:+aAhOvZDVGzl5gTERB4pOJCL1zxMyw7vLecJJ6TQTCw=
+github.com/AdguardTeam/urlfilter v0.14.1 h1:imYls0fit9ojA6pP1hWFUEIjyoXbDF85ZM+G67bI48c=
-github.com/AdguardTeam/urlfilter v0.14.0/go.mod h1:klx4JbOfc4EaNb5lWLqOwfg+pVcyRukmoJRvO55lL5U=
+github.com/AdguardTeam/urlfilter v0.14.1/go.mod h1:klx4JbOfc4EaNb5lWLqOwfg+pVcyRukmoJRvO55lL5U=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=

internal/dnsfilter/dnsrewrite.go

@@ -7,8 +7,8 @@ import (
 
 // DNSRewriteResult is the result of application of $dnsrewrite rules.
 type DNSRewriteResult struct {
-	RCode    rules.RCode              `json:",omitempty"`
 	Response DNSRewriteResultResponse `json:",omitempty"`
+	RCode    rules.RCode              `json:",omitempty"`
 }
 
 // DNSRewriteResultResponse is the collection of DNS response records

internal/dnsforward/dns.go

@@ -379,7 +379,7 @@ func processFilteringAfterResponse(ctx *dnsContext) int {
 
 		if len(d.Res.Answer) != 0 {
 			answer := []dns.RR{}
-			answer = append(answer, s.genCNAMEAnswer(d.Req, res.CanonName))
+			answer = append(answer, s.genAnswerCNAME(d.Req, res.CanonName))
 			answer = append(answer, d.Res.Answer...)
 			d.Res.Answer = answer
 		}

internal/dnsforward/dnsrewrite.go

@@ -13,27 +13,55 @@ import (
 )
 
 // filterDNSRewriteResponse handles a single DNS rewrite response entry.
-// It returns the constructed answer resource record.
+// It returns the properly constructed answer resource record.
 func (s *Server) filterDNSRewriteResponse(req *dns.Msg, rr rules.RRType, v rules.RRValue) (ans dns.RR, err error) {
+	// TODO(a.garipov): As more types are added, we will probably want to
+	// use a handler-oriented approach here.  So, think of a way to decouple
+	// the answer generation logic from the Server.
+
 	switch rr {
 	case dns.TypeA, dns.TypeAAAA:
 		ip, ok := v.(net.IP)
 		if !ok {
-			return nil, fmt.Errorf("value has type %T, not net.IP", v)
+			return nil, fmt.Errorf("value for rr type %d has type %T, not net.IP", rr, v)
 		}
 
 		if rr == dns.TypeA {
-			return s.genAAnswer(req, ip.To4()), nil
+			return s.genAnswerA(req, ip.To4()), nil
 		}
 
-		return s.genAAAAAnswer(req, ip), nil
+		return s.genAnswerAAAA(req, ip), nil
-	case dns.TypeTXT:
+	case dns.TypePTR,
+		dns.TypeTXT:
 		str, ok := v.(string)
 		if !ok {
-			return nil, fmt.Errorf("value has type %T, not string", v)
+			return nil, fmt.Errorf("value for rr type %d has type %T, not string", rr, v)
 		}
 
-		return s.genTXTAnswer(req, []string{str}), nil
+		if rr == dns.TypeTXT {
+			return s.genAnswerTXT(req, []string{str}), nil
+		}
+
+		return s.genAnswerPTR(req, str), nil
+	case dns.TypeMX:
+		mx, ok := v.(*rules.DNSMX)
+		if !ok {
+			return nil, fmt.Errorf("value for rr type %d has type %T, not *rules.DNSMX", rr, v)
+		}
+
+		return s.genAnswerMX(req, mx), nil
+	case dns.TypeHTTPS,
+		dns.TypeSVCB:
+		svcb, ok := v.(*rules.DNSSVCB)
+		if !ok {
+			return nil, fmt.Errorf("value for rr type %d has type %T, not *rules.DNSSVCB", rr, v)
+		}
+
+		if rr == dns.TypeHTTPS {
+			return s.genAnswerHTTPS(req, svcb), nil
+		}
+
+		return s.genAnswerSVCB(req, svcb), nil
 	default:
 		log.Debug("don't know how to handle dns rr type %d, skipping", rr)
 

internal/dnsforward/filter.go

@@ -87,17 +87,17 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*dnsfilter.Result, error) {
 
 		name := host
 		if len(res.CanonName) != 0 {
-			resp.Answer = append(resp.Answer, s.genCNAMEAnswer(req, res.CanonName))
+			resp.Answer = append(resp.Answer, s.genAnswerCNAME(req, res.CanonName))
 			name = res.CanonName
 		}
 
 		for _, ip := range res.IPList {
 			if req.Question[0].Qtype == dns.TypeA {
-				a := s.genAAnswer(req, ip.To4())
+				a := s.genAnswerA(req, ip.To4())
 				a.Hdr.Name = dns.Fqdn(name)
 				resp.Answer = append(resp.Answer, a)
 			} else if req.Question[0].Qtype == dns.TypeAAAA {
-				a := s.genAAAAAnswer(req, ip)
+				a := s.genAnswerAAAA(req, ip)
 				a.Hdr.Name = dns.Fqdn(name)
 				resp.Answer = append(resp.Answer, a)
 			}

internal/dnsforward/msg.go

@@ -7,6 +7,7 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsfilter"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/urlfilter/rules"
 	"github.com/miekg/dns"
 )
 
@@ -92,48 +93,64 @@ func (s *Server) genServerFailure(request *dns.Msg) *dns.Msg {
 
 func (s *Server) genARecord(request *dns.Msg, ip net.IP) *dns.Msg {
 	resp := s.makeResponse(request)
-	resp.Answer = append(resp.Answer, s.genAAnswer(request, ip))
+	resp.Answer = append(resp.Answer, s.genAnswerA(request, ip))
 	return resp
 }
 
 func (s *Server) genAAAARecord(request *dns.Msg, ip net.IP) *dns.Msg {
 	resp := s.makeResponse(request)
-	resp.Answer = append(resp.Answer, s.genAAAAAnswer(request, ip))
+	resp.Answer = append(resp.Answer, s.genAnswerAAAA(request, ip))
 	return resp
 }
 
-func (s *Server) genAAnswer(req *dns.Msg, ip net.IP) *dns.A {
+func (s *Server) hdr(req *dns.Msg, rrType rules.RRType) (h dns.RR_Header) {
-	answer := new(dns.A)
+	return dns.RR_Header{
-	answer.Hdr = dns.RR_Header{
 		Name:   req.Question[0].Name,
-		Rrtype: dns.TypeA,
+		Rrtype: rrType,
 		Ttl:    s.conf.BlockedResponseTTL,
 		Class:  dns.ClassINET,
 	}
-	answer.A = ip
-	return answer
 }
 
-func (s *Server) genAAAAAnswer(req *dns.Msg, ip net.IP) *dns.AAAA {
+func (s *Server) genAnswerA(req *dns.Msg, ip net.IP) (ans *dns.A) {
-	answer := new(dns.AAAA)
+	return &dns.A{
-	answer.Hdr = dns.RR_Header{
+		Hdr: s.hdr(req, dns.TypeA),
-		Name:   req.Question[0].Name,
+		A:   ip,
-		Rrtype: dns.TypeAAAA,
-		Ttl:    s.conf.BlockedResponseTTL,
-		Class:  dns.ClassINET,
 	}
-	answer.AAAA = ip
-	return answer
 }
 
-func (s *Server) genTXTAnswer(req *dns.Msg, strs []string) (answer *dns.TXT) {
+func (s *Server) genAnswerAAAA(req *dns.Msg, ip net.IP) (ans *dns.AAAA) {
+	return &dns.AAAA{
+		Hdr:  s.hdr(req, dns.TypeAAAA),
+		AAAA: ip,
+	}
+}
+
+func (s *Server) genAnswerCNAME(req *dns.Msg, cname string) (ans *dns.CNAME) {
+	return &dns.CNAME{
+		Hdr:    s.hdr(req, dns.TypeCNAME),
+		Target: dns.Fqdn(cname),
+	}
+}
+
+func (s *Server) genAnswerMX(req *dns.Msg, mx *rules.DNSMX) (ans *dns.MX) {
+	return &dns.MX{
+		Hdr:        s.hdr(req, dns.TypePTR),
+		Preference: mx.Preference,
+		Mx:         mx.Exchange,
+	}
+}
+
+func (s *Server) genAnswerPTR(req *dns.Msg, ptr string) (ans *dns.PTR) {
+	return &dns.PTR{
+		Hdr: s.hdr(req, dns.TypePTR),
+		Ptr: ptr,
+	}
+}
+
+func (s *Server) genAnswerTXT(req *dns.Msg, strs []string) (ans *dns.TXT) {
 	return &dns.TXT{
-		Hdr: dns.RR_Header{
+		Hdr: s.hdr(req, dns.TypeTXT),
-			Name:   req.Question[0].Name,
-			Rrtype: dns.TypeTXT,
-			Ttl:    s.conf.BlockedResponseTTL,
-			Class:  dns.ClassINET,
-		},
 		Txt: strs,
 	}
 }
@@ -198,19 +215,6 @@ func (s *Server) genBlockedHost(request *dns.Msg, newAddr string, d *proxy.DNSCo
 	return resp
 }
 
-// Make a CNAME response
-func (s *Server) genCNAMEAnswer(req *dns.Msg, cname string) *dns.CNAME {
-	answer := new(dns.CNAME)
-	answer.Hdr = dns.RR_Header{
-		Name:   req.Question[0].Name,
-		Rrtype: dns.TypeCNAME,
-		Ttl:    s.conf.BlockedResponseTTL,
-		Class:  dns.ClassINET,
-	}
-	answer.Target = dns.Fqdn(cname)
-	return answer
-}
-
 // Create REFUSED DNS response
 func (s *Server) makeResponseREFUSED(request *dns.Msg) *dns.Msg {
 	resp := dns.Msg{}

internal/dnsforward/svcbmsg.go

@@ -0,0 +1,168 @@
+package dnsforward
+
+import (
+	"encoding/base64"
+	"net"
+	"strconv"
+
+	"github.com/AdguardTeam/golibs/log"
+	"github.com/AdguardTeam/urlfilter/rules"
+	"github.com/miekg/dns"
+)
+
+// genAnswerHTTPS returns a properly initialized HTTPS resource record.
+//
+// See the comment on genAnswerSVCB for a list of current restrictions on
+// parameter values.
+func (s *Server) genAnswerHTTPS(req *dns.Msg, svcb *rules.DNSSVCB) (ans *dns.HTTPS) {
+	ans = &dns.HTTPS{
+		SVCB: *s.genAnswerSVCB(req, svcb),
+	}
+
+	ans.Hdr.Rrtype = dns.TypeHTTPS
+
+	return ans
+}
+
+// strToSVCBKey is the string-to-svcb-key mapping.
+//
+// See https://github.com/miekg/dns/blob/23c4faca9d32b0abbb6e179aa1aadc45ac53a916/svcb.go#L27.
+//
+// TODO(a.garipov): Propose exporting this API or something similar in the
+// github.com/miekg/dns module.
+var strToSVCBKey = map[string]dns.SVCBKey{
+	"alpn":            dns.SVCB_ALPN,
+	"echconfig":       dns.SVCB_ECHCONFIG,
+	"ipv4hint":        dns.SVCB_IPV4HINT,
+	"ipv6hint":        dns.SVCB_IPV6HINT,
+	"mandatory":       dns.SVCB_MANDATORY,
+	"no-default-alpn": dns.SVCB_NO_DEFAULT_ALPN,
+	"port":            dns.SVCB_PORT,
+}
+
+// svcbKeyHandler is a handler for one SVCB parameter key.
+type svcbKeyHandler func(valStr string) (val dns.SVCBKeyValue)
+
+// svcbKeyHandlers are the supported SVCB parameters handlers.
+var svcbKeyHandlers = map[string]svcbKeyHandler{
+	"alpn": func(valStr string) (val dns.SVCBKeyValue) {
+		return &dns.SVCBAlpn{
+			Alpn: []string{valStr},
+		}
+	},
+
+	"echconfig": func(valStr string) (val dns.SVCBKeyValue) {
+		ech, err := base64.StdEncoding.DecodeString(valStr)
+		if err != nil {
+			log.Debug("can't parse svcb/https echconfig: %s; ignoring", err)
+
+			return nil
+		}
+
+		return &dns.SVCBECHConfig{
+			ECH: ech,
+		}
+	},
+
+	"ipv4hint": func(valStr string) (val dns.SVCBKeyValue) {
+		ip := net.ParseIP(valStr)
+		if ip4 := ip.To4(); ip == nil || ip4 == nil {
+			log.Debug("can't parse svcb/https ipv4 hint %q; ignoring", valStr)
+
+			return nil
+		}
+
+		return &dns.SVCBIPv4Hint{
+			Hint: []net.IP{ip},
+		}
+	},
+
+	"ipv6hint": func(valStr string) (val dns.SVCBKeyValue) {
+		ip := net.ParseIP(valStr)
+		if ip == nil {
+			log.Debug("can't parse svcb/https ipv6 hint %q; ignoring", valStr)
+
+			return nil
+		}
+
+		return &dns.SVCBIPv6Hint{
+			Hint: []net.IP{ip},
+		}
+	},
+
+	"mandatory": func(valStr string) (val dns.SVCBKeyValue) {
+		code, ok := strToSVCBKey[valStr]
+		if !ok {
+			log.Debug("unknown svcb/https mandatory key %q, ignoring", valStr)
+
+			return nil
+		}
+
+		return &dns.SVCBMandatory{
+			Code: []dns.SVCBKey{code},
+		}
+	},
+
+	"no-default-alpn": func(_ string) (val dns.SVCBKeyValue) {
+		return &dns.SVCBNoDefaultAlpn{}
+	},
+
+	"port": func(valStr string) (val dns.SVCBKeyValue) {
+		port64, err := strconv.ParseUint(valStr, 10, 16)
+		if err != nil {
+			log.Debug("can't parse svcb/https port: %s; ignoring", err)
+
+			return nil
+		}
+
+		return &dns.SVCBPort{
+			Port: uint16(port64),
+		}
+	},
+}
+
+// genAnswerSVCB returns a properly initialized SVCB resource record.
+//
+// Currently, there are several restrictions on how the parameters are parsed.
+// Firstly, the parsing of non-contiguous values isn't supported.  Secondly, the
+// parsing of value-lists is not supported either.
+//
+//   ipv4hint=127.0.0.1             // Supported.
+//   ipv4hint="127.0.0.1"           // Unsupported.
+//   ipv4hint=127.0.0.1,127.0.0.2   // Unsupported.
+//   ipv4hint="127.0.0.1,127.0.0.2" // Unsupported.
+//
+// TODO(a.garipov): Support all of these.
+func (s *Server) genAnswerSVCB(req *dns.Msg, svcb *rules.DNSSVCB) (ans *dns.SVCB) {
+	ans = &dns.SVCB{
+		Hdr:      s.hdr(req, dns.TypeSVCB),
+		Priority: svcb.Priority,
+		Target:   svcb.Target,
+	}
+	if len(svcb.Params) == 0 {
+		return ans
+	}
+
+	values := make([]dns.SVCBKeyValue, 0, len(svcb.Params))
+	for k, valStr := range svcb.Params {
+		handler, ok := svcbKeyHandlers[k]
+		if !ok {
+			log.Debug("unknown svcb/https key %q, ignoring", k)
+
+			continue
+		}
+
+		val := handler(valStr)
+		if val == nil {
+			continue
+		}
+
+		values = append(values, val)
+	}
+
+	if len(values) > 0 {
+		ans.Value = values
+	}
+
+	return ans
+}

internal/dnsforward/svcbmsg_test.go

@@ -0,0 +1,154 @@
+package dnsforward
+
+import (
+	"net"
+	"testing"
+
+	"github.com/AdguardTeam/urlfilter/rules"
+	"github.com/miekg/dns"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGenAnswerHTTPS_andSVCB(t *testing.T) {
+	// Preconditions.
+
+	s := &Server{
+		conf: ServerConfig{
+			FilteringConfig: FilteringConfig{
+				BlockedResponseTTL: 3600,
+			},
+		},
+	}
+
+	req := &dns.Msg{
+		Question: []dns.Question{{
+			Name: "abcd",
+		}},
+	}
+
+	// Constants and helper values.
+
+	const host = "example.com"
+	const prio = 32
+
+	ip4 := net.IPv4(127, 0, 0, 1)
+	ip6 := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}
+
+	// Helper functions.
+
+	dnssvcb := func(key, value string) (svcb *rules.DNSSVCB) {
+		svcb = &rules.DNSSVCB{
+			Target:   host,
+			Priority: prio,
+		}
+
+		if key == "" {
+			return svcb
+		}
+
+		svcb.Params = map[string]string{
+			key: value,
+		}
+
+		return svcb
+	}
+
+	wantsvcb := func(kv dns.SVCBKeyValue) (want *dns.SVCB) {
+		want = &dns.SVCB{
+			Hdr:      s.hdr(req, dns.TypeSVCB),
+			Priority: prio,
+			Target:   host,
+		}
+
+		if kv == nil {
+			return want
+		}
+
+		want.Value = []dns.SVCBKeyValue{kv}
+
+		return want
+	}
+
+	// Tests.
+
+	testCases := []struct {
+		svcb *rules.DNSSVCB
+		want *dns.SVCB
+		name string
+	}{{
+		svcb: dnssvcb("", ""),
+		want: wantsvcb(nil),
+		name: "no_params",
+	}, {
+		svcb: dnssvcb("foo", "bar"),
+		want: wantsvcb(nil),
+		name: "invalid",
+	}, {
+		svcb: dnssvcb("alpn", "h3"),
+		want: wantsvcb(&dns.SVCBAlpn{Alpn: []string{"h3"}}),
+		name: "alpn",
+	}, {
+		svcb: dnssvcb("echconfig", "AAAA"),
+		want: wantsvcb(&dns.SVCBECHConfig{ECH: []byte{0, 0, 0}}),
+		name: "echconfig",
+	}, {
+		svcb: dnssvcb("echconfig", "%BAD%"),
+		want: wantsvcb(nil),
+		name: "echconfig_invalid",
+	}, {
+		svcb: dnssvcb("ipv4hint", "127.0.0.1"),
+		want: wantsvcb(&dns.SVCBIPv4Hint{Hint: []net.IP{ip4}}),
+		name: "ipv4hint",
+	}, {
+		svcb: dnssvcb("ipv4hint", "127.0.01"),
+		want: wantsvcb(nil),
+		name: "ipv4hint_invalid",
+	}, {
+		svcb: dnssvcb("ipv6hint", "::1"),
+		want: wantsvcb(&dns.SVCBIPv6Hint{Hint: []net.IP{ip6}}),
+		name: "ipv6hint",
+	}, {
+		svcb: dnssvcb("ipv6hint", ":::1"),
+		want: wantsvcb(nil),
+		name: "ipv6hint_invalid",
+	}, {
+		svcb: dnssvcb("mandatory", "alpn"),
+		want: wantsvcb(&dns.SVCBMandatory{Code: []dns.SVCBKey{dns.SVCB_ALPN}}),
+		name: "mandatory",
+	}, {
+		svcb: dnssvcb("mandatory", "alpnn"),
+		want: wantsvcb(nil),
+		name: "mandatory_invalid",
+	}, {
+		svcb: dnssvcb("no-default-alpn", ""),
+		want: wantsvcb(&dns.SVCBNoDefaultAlpn{}),
+		name: "no-default-alpn",
+	}, {
+		svcb: dnssvcb("port", "8080"),
+		want: wantsvcb(&dns.SVCBPort{Port: 8080}),
+		name: "port",
+	}, {
+		svcb: dnssvcb("port", "1005008080"),
+		want: wantsvcb(nil),
+		name: "port",
+	}}
+
+	for _, tc := range testCases {
+		t.Run("https", func(t *testing.T) {
+			t.Run(tc.name, func(t *testing.T) {
+				want := &dns.HTTPS{SVCB: *tc.want}
+				want.Hdr.Rrtype = dns.TypeHTTPS
+
+				got := s.genAnswerHTTPS(req, tc.svcb)
+				assert.Equal(t, want, got)
+			})
+		})
+
+		t.Run("svcb", func(t *testing.T) {
+			t.Run(tc.name, func(t *testing.T) {
+				got := s.genAnswerSVCB(req, tc.svcb)
+				assert.Equal(t, tc.want, got)
+			})
+		})
+	}
+}

staticcheck.conf

@@ -7,8 +7,11 @@ initialisms = [
 , "DOQ"
 , "DOT"
 , "EDNS"
+, "MX"
+, "PTR"
 , "QUIC"
 , "SDNS"
+, "SVCB"
 ]
 dot_import_whitelist = []
 http_status_code_whitelist = []