From 386add033b91e3110c822369fa91629d9006436c Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 27 Feb 2023 16:48:32 +0300 Subject: [PATCH] Pull request 1750: 5468-disallowed-dnstype Updates #5468. Squashed commit of the following: commit ecb0c7cc53d43fe5aa75be0c5fc0f2a9179099ad Author: Ainar Garipov Date: Mon Feb 27 13:57:40 2023 +0300 dnsforward: imp access test commit 3fdc2c4533d1fce8a01f6e636e326454ef96565b Author: Ainar Garipov Date: Wed Feb 22 19:29:50 2023 +0300 dnsforward: allow dnstype rules in disallowed domains --- CHANGELOG.md | 6 ++++ internal/dnsforward/access.go | 9 +++-- internal/dnsforward/access_test.go | 55 +++++++++++++++++++++--------- internal/dnsforward/filter.go | 8 +++-- 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eb99005..66f798e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ See also the [v0.107.26 GitHub milestone][ms-v0.107.26]. NOTE: Add new changes BELOW THIS COMMENT. --> +### Added + +- The ability to use `dnstype` rules in the disallowed domains list ([#5468]). + This allows dropping requests based on their question types. + ### Fixed - Requirements to domain names in domain-specific upstream configurations have @@ -31,6 +36,7 @@ NOTE: Add new changes BELOW THIS COMMENT. [#4884]: https://github.com/AdguardTeam/AdGuardHome/issues/4884 [#5431]: https://github.com/AdguardTeam/AdGuardHome/issues/5431 +[#5468]: https://github.com/AdguardTeam/AdGuardHome/issues/5468 [rfc3696]: https://datatracker.ietf.org/doc/html/rfc3696 diff --git a/internal/dnsforward/access.go b/internal/dnsforward/access.go index 6d45a6d5..12f5f3c7 100644 --- a/internal/dnsforward/access.go +++ b/internal/dnsforward/access.go @@ -13,6 +13,7 @@ import ( "github.com/AdguardTeam/golibs/stringutil" "github.com/AdguardTeam/urlfilter" "github.com/AdguardTeam/urlfilter/filterlist" + "github.com/AdguardTeam/urlfilter/rules" ) // unit is a convenient alias for struct{} @@ -127,8 +128,12 @@ func (a *accessManager) isBlockedClientID(id string) (ok bool) { } // isBlockedHost returns true if host should be blocked. -func (a *accessManager) isBlockedHost(host string) (ok bool) { - _, ok = a.blockedHostsEng.Match(strings.ToLower(host)) +func (a *accessManager) isBlockedHost(host string, qt rules.RRType) (ok bool) { + _, ok = a.blockedHostsEng.MatchRequest(&urlfilter.DNSRequest{ + Hostname: host, + ClientIP: "0.0.0.0", + DNSType: qt, + }) return ok } diff --git a/internal/dnsforward/access_test.go b/internal/dnsforward/access_test.go index 7889cdad..d5d7da26 100644 --- a/internal/dnsforward/access_test.go +++ b/internal/dnsforward/access_test.go @@ -4,6 +4,8 @@ import ( "net/netip" "testing" + "github.com/AdguardTeam/urlfilter/rules" + "github.com/miekg/dns" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -28,54 +30,75 @@ func TestIsBlockedHost(t *testing.T) { "host1", "*.host.com", "||host3.com^", + "||*^$dnstype=HTTPS", }) require.NoError(t, err) testCases := []struct { + want assert.BoolAssertionFunc name string host string - want bool + qt rules.RRType }{{ + want: assert.True, name: "plain_match", host: "host1", - want: true, + qt: dns.TypeA, }, { + want: assert.False, name: "plain_mismatch", host: "host2", - want: false, + qt: dns.TypeA, }, { + want: assert.True, name: "subdomain_match_short", host: "asdf.host.com", - want: true, + qt: dns.TypeA, }, { + want: assert.True, name: "subdomain_match_long", host: "qwer.asdf.host.com", - want: true, + qt: dns.TypeA, }, { + want: assert.False, name: "subdomain_mismatch_no_lead", host: "host.com", - want: false, + qt: dns.TypeA, }, { + want: assert.False, name: "subdomain_mismatch_bad_asterisk", host: "asdf.zhost.com", - want: false, + qt: dns.TypeA, }, { + want: assert.True, name: "rule_match_simple", host: "host3.com", - want: true, + qt: dns.TypeA, }, { + want: assert.True, name: "rule_match_complex", host: "asdf.host3.com", - want: true, + qt: dns.TypeA, }, { + want: assert.False, name: "rule_mismatch", host: ".host3.com", - want: false, + qt: dns.TypeA, + }, { + want: assert.True, + name: "by_qtype", + host: "site-with-https-record.example", + qt: dns.TypeHTTPS, + }, { + want: assert.False, + name: "by_qtype_other", + host: "site-with-https-record.example", + qt: dns.TypeA, }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.want, a.isBlockedHost(tc.host)) + tc.want(t, a.isBlockedHost(tc.host, tc.qt)) }) } } @@ -93,29 +116,29 @@ func TestIsBlockedIP(t *testing.T) { require.NoError(t, err) testCases := []struct { + ip netip.Addr name string wantRule string - ip netip.Addr wantBlocked bool }{{ + ip: netip.MustParseAddr("1.2.3.4"), name: "match_ip", wantRule: "1.2.3.4", - ip: netip.MustParseAddr("1.2.3.4"), wantBlocked: true, }, { + ip: netip.MustParseAddr("5.6.7.100"), name: "match_cidr", wantRule: "5.6.7.8/24", - ip: netip.MustParseAddr("5.6.7.100"), wantBlocked: true, }, { + ip: netip.MustParseAddr("9.2.3.4"), name: "no_match_ip", wantRule: "", - ip: netip.MustParseAddr("9.2.3.4"), wantBlocked: false, }, { + ip: netip.MustParseAddr("9.6.7.100"), name: "no_match_cidr", wantRule: "", - ip: netip.MustParseAddr("9.6.7.100"), wantBlocked: false, }} diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go index f36fd52a..6ee4e0f3 100644 --- a/internal/dnsforward/filter.go +++ b/internal/dnsforward/filter.go @@ -31,9 +31,11 @@ func (s *Server) beforeRequestHandler( } if len(pctx.Req.Question) == 1 { - host := strings.TrimSuffix(pctx.Req.Question[0].Name, ".") - if s.access.isBlockedHost(host) { - log.Debug("host %s is in access blocklist", host) + q := pctx.Req.Question[0] + qt := q.Qtype + host := strings.TrimSuffix(q.Name, ".") + if s.access.isBlockedHost(host, qt) { + log.Debug("request %s %s is in access blocklist", dns.Type(qt), host) return s.preBlockedResponse(pctx) }