From fc79e2e8f8450e8515d0a5bae03b078acee4296d Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 23 Dec 2020 12:35:07 +0300 Subject: [PATCH] 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 Date: Tue Dec 22 20:40:56 2020 +0300 dnsforward: improve naming commit 70b832ce969d8cdcf4224d221e5e1e2057fba6ee Author: Ainar Garipov Date: Tue Dec 22 19:36:21 2020 +0300 all: support more $dnsrewrite rr types --- go.mod | 2 +- go.sum | 4 +- internal/dnsfilter/dnsrewrite.go | 2 +- internal/dnsforward/dns.go | 2 +- internal/dnsforward/dnsrewrite.go | 42 +++++-- internal/dnsforward/filter.go | 6 +- internal/dnsforward/msg.go | 78 +++++++------ internal/dnsforward/svcbmsg.go | 168 ++++++++++++++++++++++++++++ internal/dnsforward/svcbmsg_test.go | 154 +++++++++++++++++++++++++ staticcheck.conf | 3 + 10 files changed, 409 insertions(+), 52 deletions(-) create mode 100644 internal/dnsforward/svcbmsg.go create mode 100644 internal/dnsforward/svcbmsg_test.go diff --git a/go.mod b/go.mod index 10866d88..61c563e5 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 764a2fa0..d8d6ef4d 100644 --- a/go.sum +++ b/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.0/go.mod h1:klx4JbOfc4EaNb5lWLqOwfg+pVcyRukmoJRvO55lL5U= +github.com/AdguardTeam/urlfilter v0.14.1 h1:imYls0fit9ojA6pP1hWFUEIjyoXbDF85ZM+G67bI48c= +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= diff --git a/internal/dnsfilter/dnsrewrite.go b/internal/dnsfilter/dnsrewrite.go index 1239fbad..66cb5828 100644 --- a/internal/dnsfilter/dnsrewrite.go +++ b/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 diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index d7208d1c..0e5eb51f 100644 --- a/internal/dnsforward/dns.go +++ b/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 } diff --git a/internal/dnsforward/dnsrewrite.go b/internal/dnsforward/dnsrewrite.go index 01895323..3741e850 100644 --- a/internal/dnsforward/dnsrewrite.go +++ b/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 - case dns.TypeTXT: + return s.genAnswerAAAA(req, ip), nil + 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) diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go index 5cd0090a..c6bfb160 100644 --- a/internal/dnsforward/filter.go +++ b/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) } diff --git a/internal/dnsforward/msg.go b/internal/dnsforward/msg.go index f8200056..28a8ac3a 100644 --- a/internal/dnsforward/msg.go +++ b/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 { - answer := new(dns.A) - answer.Hdr = dns.RR_Header{ +func (s *Server) hdr(req *dns.Msg, rrType rules.RRType) (h dns.RR_Header) { + return 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 { - answer := new(dns.AAAA) - answer.Hdr = dns.RR_Header{ - Name: req.Question[0].Name, - Rrtype: dns.TypeAAAA, - Ttl: s.conf.BlockedResponseTTL, - Class: dns.ClassINET, +func (s *Server) genAnswerA(req *dns.Msg, ip net.IP) (ans *dns.A) { + return &dns.A{ + Hdr: s.hdr(req, dns.TypeA), + A: ip, } - 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{ - Name: req.Question[0].Name, - Rrtype: dns.TypeTXT, - Ttl: s.conf.BlockedResponseTTL, - Class: dns.ClassINET, - }, + Hdr: s.hdr(req, dns.TypeTXT), 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{} diff --git a/internal/dnsforward/svcbmsg.go b/internal/dnsforward/svcbmsg.go new file mode 100644 index 00000000..2a8c27b4 --- /dev/null +++ b/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 +} diff --git a/internal/dnsforward/svcbmsg_test.go b/internal/dnsforward/svcbmsg_test.go new file mode 100644 index 00000000..392e92ac --- /dev/null +++ b/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) + }) + }) + } +} diff --git a/staticcheck.conf b/staticcheck.conf index 43639bf6..146f83cb 100644 --- a/staticcheck.conf +++ b/staticcheck.conf @@ -7,8 +7,11 @@ initialisms = [ , "DOQ" , "DOT" , "EDNS" +, "MX" +, "PTR" , "QUIC" , "SDNS" +, "SVCB" ] dot_import_whitelist = [] http_status_code_whitelist = []