Merge branch 'master' into websvc-confin-manager

2022-09-02 17:38:22 +03:00
parent b018e150e7 1fb043768e
commit 8a65848da4
15 changed files with 340 additions and 453 deletions
--- a/internal/dnsforward/config.go
+++ b/internal/dnsforward/config.go
@@ -381,7 +381,7 @@ func (s *Server) prepareUpstreamSettings() error {
 }

 // prepareInternalProxy initializes the DNS proxy that is used for internal DNS
-// queries, such at client PTR resolving and udpater hostname resolving.
+// queries, such at client PTR resolving and updater hostname resolving.
 func (s *Server) prepareInternalProxy() {
 	conf := &proxy.Config{
 		CacheEnabled:   true,
--- a/internal/dnsforward/dns.go
+++ b/internal/dnsforward/dns.go
@@ -81,9 +81,9 @@ const (
 const ddrHostFQDN = "_dns.resolver.arpa."

 // handleDNSRequest filters the incoming DNS requests and writes them to the query log
-func (s *Server) handleDNSRequest(_ *proxy.Proxy, d *proxy.DNSContext) error {
-	ctx := &dnsContext{
-		proxyCtx:  d,
+func (s *Server) handleDNSRequest(_ *proxy.Proxy, pctx *proxy.DNSContext) error {
+	dctx := &dnsContext{
+		proxyCtx:  pctx,
 		result:    &filtering.Result{},
 		startTime: time.Now(),
 	}
@@ -111,7 +111,7 @@ func (s *Server) handleDNSRequest(_ *proxy.Proxy, d *proxy.DNSContext) error {
 		s.processQueryLogsAndStats,
 	}
 	for _, process := range mods {
-		r := process(ctx)
+		r := process(dctx)
 		switch r {
 		case resultCodeSuccess:
 			// continue: call the next filter
@@ -120,13 +120,15 @@ func (s *Server) handleDNSRequest(_ *proxy.Proxy, d *proxy.DNSContext) error {
 			return nil

 		case resultCodeError:
-			return ctx.err
+			return dctx.err
 		}
 	}

-	if d.Res != nil {
-		d.Res.Compress = true // some devices require DNS message compression
+	if pctx.Res != nil {
+		// Some devices require DNS message compression.
+		pctx.Res.Compress = true
 	}
+
 	return nil
 }

@@ -149,34 +151,38 @@ func (s *Server) processRecursion(dctx *dnsContext) (rc resultCode) {
 // needed and enriches the ctx with some client-specific information.
 //
 // TODO(e.burkov):  Decompose into less general processors.
-func (s *Server) processInitial(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
-	if s.conf.AAAADisabled && d.Req.Question[0].Qtype == dns.TypeAAAA {
-		_ = proxy.CheckDisabledAAAARequest(d, true)
+func (s *Server) processInitial(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	q := pctx.Req.Question[0]
+	qt := q.Qtype
+	if s.conf.AAAADisabled && qt == dns.TypeAAAA {
+		_ = proxy.CheckDisabledAAAARequest(pctx, true)
+
 		return resultCodeFinish
 	}

 	if s.conf.OnDNSRequest != nil {
-		s.conf.OnDNSRequest(d)
+		s.conf.OnDNSRequest(pctx)
 	}

-	// disable Mozilla DoH
-	// https://support.mozilla.org/en-US/kb/canary-domain-use-application-dnsnet
-	if (d.Req.Question[0].Qtype == dns.TypeA || d.Req.Question[0].Qtype == dns.TypeAAAA) &&
-		d.Req.Question[0].Name == "use-application-dns.net." {
-		d.Res = s.genNXDomain(d.Req)
+	// Disable Mozilla DoH.
+	//
+	// See https://support.mozilla.org/en-US/kb/canary-domain-use-application-dnsnet.
+	if (qt == dns.TypeA || qt == dns.TypeAAAA) && q.Name == "use-application-dns.net." {
+		pctx.Res = s.genNXDomain(pctx.Req)
+
 		return resultCodeFinish
 	}

-	// Get the client's ID if any.  It should be performed before getting
-	// client-specific filtering settings.
+	// Get the ClientID, if any, before getting client-specific filtering
+	// settings.
 	var key [8]byte
-	binary.BigEndian.PutUint64(key[:], d.RequestID)
-	ctx.clientID = string(s.clientIDCache.Get(key[:]))
+	binary.BigEndian.PutUint64(key[:], pctx.RequestID)
+	dctx.clientID = string(s.clientIDCache.Get(key[:]))

 	// Get the client-specific filtering settings.
-	ctx.protectionEnabled = s.conf.ProtectionEnabled
-	ctx.setts = s.getClientRequestFilteringSettings(ctx)
+	dctx.protectionEnabled = s.conf.ProtectionEnabled
+	dctx.setts = s.getClientRequestFilteringSettings(dctx)

 	return resultCodeSuccess
 }
@@ -244,28 +250,28 @@ func (s *Server) onDHCPLeaseChanged(flags int) {
 	s.setTableIPToHost(ipToHost)
 }

-// processDDRQuery responds to SVCB query for a special use domain name
-// ‘_dns.resolver.arpa’.  The result contains different types of encryption
-// supported by current user configuration.
+// processDDRQuery responds to Discovery of Designated Resolvers (DDR) SVCB
+// queries.  The response contains different types of encryption supported by
+// current user configuration.
 //
-// See https://www.ietf.org/archive/id/draft-ietf-add-ddr-06.html.
-func (s *Server) processDDRQuery(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
-	question := d.Req.Question[0]
+// See https://www.ietf.org/archive/id/draft-ietf-add-ddr-10.html.
+func (s *Server) processDDRQuery(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	q := pctx.Req.Question[0]

 	if !s.conf.HandleDDR {
 		return resultCodeSuccess
 	}

-	if question.Name == ddrHostFQDN {
+	if q.Name == ddrHostFQDN {
 		if s.dnsProxy.TLSListenAddr == nil && s.conf.HTTPSListenAddrs == nil &&
-			s.dnsProxy.QUICListenAddr == nil || question.Qtype != dns.TypeSVCB {
-			d.Res = s.makeResponse(d.Req)
+			s.dnsProxy.QUICListenAddr == nil || q.Qtype != dns.TypeSVCB {
+			pctx.Res = s.makeResponse(pctx.Req)

 			return resultCodeFinish
 		}

-		d.Res = s.makeDDRResponse(d.Req)
+		pctx.Res = s.makeDDRResponse(pctx.Req)

 		return resultCodeFinish
 	}
@@ -273,11 +279,13 @@ func (s *Server) processDDRQuery(ctx *dnsContext) (rc resultCode) {
 	return resultCodeSuccess
 }

-// makeDDRResponse creates DDR answer according to server configuration.  The
-// contructed SVCB resource records have the priority of 1 for each entry,
-// similar to examples provided by https://www.ietf.org/archive/id/draft-ietf-add-ddr-06.html.
+// makeDDRResponse creates a DDR answer based on the server configuration.  The
+// constructed SVCB resource records have the priority of 1 for each entry,
+// similar to examples provided by the [draft standard].
 //
 // TODO(a.meshkov):  Consider setting the priority values based on the protocol.
+//
+// [draft standard]: https://www.ietf.org/archive/id/draft-ietf-add-ddr-10.html.
 func (s *Server) makeDDRResponse(req *dns.Msg) (resp *dns.Msg) {
 	resp = s.makeResponse(req)
 	// TODO(e.burkov):  Think about storing the FQDN version of the server's
@@ -351,10 +359,10 @@ func (s *Server) processDetermineLocal(dctx *dnsContext) (rc resultCode) {
 	return rc
 }

-// hostToIP tries to get an IP leased by DHCP and returns the copy of address
-// since the data inside the internal table may be changed while request
+// dhcpHostToIP tries to get an IP leased by DHCP and returns the copy of
+// address since the data inside the internal table may be changed while request
 // processing.  It's safe for concurrent use.
-func (s *Server) hostToIP(host string) (ip net.IP, ok bool) {
+func (s *Server) dhcpHostToIP(host string) (ip net.IP, ok bool) {
 	s.tableHostToIPLock.Lock()
 	defer s.tableHostToIPLock.Unlock()

@@ -379,46 +387,32 @@ func (s *Server) hostToIP(host string) (ip net.IP, ok bool) {
 //
 // TODO(a.garipov): Adapt to AAAA as well.
 func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) {
-	if !s.dhcpServer.Enabled() {
-		return resultCodeSuccess
-	}
-
-	req := dctx.proxyCtx.Req
+	pctx := dctx.proxyCtx
+	req := pctx.Req
 	q := req.Question[0]
-
-	// Go on processing the AAAA request despite the fact that we don't support
-	// it yet.  The expected behavior here is to respond with an empty answer
-	// and not NXDOMAIN.
-	if q.Qtype != dns.TypeA && q.Qtype != dns.TypeAAAA {
+	reqHost, ok := s.isDHCPClientHostQ(q)
+	if !ok {
 		return resultCodeSuccess
 	}

-	reqHost := strings.ToLower(q.Name[:len(q.Name)-1])
-	// TODO(a.garipov): Move everything related to DHCP local domain to the DHCP
-	// server.
-	if !strings.HasSuffix(reqHost, s.localDomainSuffix) {
-		return resultCodeSuccess
-	}
-
-	d := dctx.proxyCtx
 	if !dctx.isLocalClient {
-		log.Debug("dns: %q requests for internal host", d.Addr)
-		d.Res = s.genNXDomain(req)
+		log.Debug("dns: %q requests for dhcp host %q", pctx.Addr, reqHost)
+		pctx.Res = s.genNXDomain(req)

 		// Do not even put into query log.
 		return resultCodeFinish
 	}

-	ip, ok := s.hostToIP(reqHost)
+	ip, ok := s.dhcpHostToIP(reqHost)
 	if !ok {
-		// TODO(e.burkov): Inspect special cases when user want to apply some
-		// rules handled by other processors to the hosts with TLD.
-		d.Res = s.genNXDomain(req)
+		// Go on and process them with filters, including dnsrewrite ones, and
+		// possibly route them to a domain-specific upstream.
+		log.Debug("dns: no dhcp record for %q", reqHost)

-		return resultCodeFinish
+		return resultCodeSuccess
 	}

-	log.Debug("dns: internal record: %s -> %s", q.Name, ip)
+	log.Debug("dns: dhcp record for %q is %s", reqHost, ip)

 	resp := s.makeResponse(req)
 	if q.Qtype == dns.TypeA {
@@ -435,9 +429,9 @@ func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) {

 // processRestrictLocal responds with NXDOMAIN to PTR requests for IP addresses
 // in locally-served network from external clients.
-func (s *Server) processRestrictLocal(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
-	req := d.Req
+func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	req := pctx.Req
 	q := req.Question[0]
 	if q.Qtype != dns.TypePTR {
 		// No need for restriction.
@@ -473,32 +467,32 @@ func (s *Server) processRestrictLocal(ctx *dnsContext) (rc resultCode) {
 		return resultCodeSuccess
 	}

-	if !ctx.isLocalClient {
-		log.Debug("dns: %q requests an internal ip", d.Addr)
-		d.Res = s.genNXDomain(req)
+	if !dctx.isLocalClient {
+		log.Debug("dns: %q requests an internal ip", pctx.Addr)
+		pctx.Res = s.genNXDomain(req)

 		// Do not even put into query log.
 		return resultCodeFinish
 	}

 	// Do not perform unreversing ever again.
-	ctx.unreversedReqIP = ip
+	dctx.unreversedReqIP = ip

 	// There is no need to filter request from external addresses since this
 	// code is only executed when the request is for locally-served ARPA
 	// hostname so disable redundant filters.
-	ctx.setts.ParentalEnabled = false
-	ctx.setts.SafeBrowsingEnabled = false
-	ctx.setts.SafeSearchEnabled = false
-	ctx.setts.ServicesRules = nil
+	dctx.setts.ParentalEnabled = false
+	dctx.setts.SafeBrowsingEnabled = false
+	dctx.setts.SafeSearchEnabled = false
+	dctx.setts.ServicesRules = nil

 	// Nothing to restrict.
 	return resultCodeSuccess
 }

-// ipToHost tries to get a hostname leased by DHCP.  It's safe for concurrent
-// use.
-func (s *Server) ipToHost(ip net.IP) (host string, ok bool) {
+// ipToDHCPHost tries to get a hostname leased by DHCP.  It's safe for
+// concurrent use.
+func (s *Server) ipToDHCPHost(ip net.IP) (host string, ok bool) {
 	s.tableIPToHostLock.Lock()
 	defer s.tableIPToHostLock.Unlock()

@@ -521,27 +515,27 @@ func (s *Server) ipToHost(ip net.IP) (host string, ok bool) {
 	return host, true
 }

-// Respond to PTR requests if the target IP is leased by our DHCP server and the
-// requestor is inside the local network.
-func (s *Server) processDHCPAddrs(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
-	if d.Res != nil {
+// processDHCPAddrs responds to PTR requests if the target IP is leased by the
+// DHCP server.
+func (s *Server) processDHCPAddrs(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	if pctx.Res != nil {
 		return resultCodeSuccess
 	}

-	ip := ctx.unreversedReqIP
+	ip := dctx.unreversedReqIP
 	if ip == nil {
 		return resultCodeSuccess
 	}

-	host, ok := s.ipToHost(ip)
+	host, ok := s.ipToDHCPHost(ip)
 	if !ok {
 		return resultCodeSuccess
 	}

-	log.Debug("dns: reverse-lookup: %s -> %s", ip, host)
+	log.Debug("dns: dhcp reverse record for %s is %q", ip, host)

-	req := d.Req
+	req := pctx.Req
 	resp := s.makeResponse(req)
 	ptr := &dns.PTR{
 		Hdr: dns.RR_Header{
@@ -553,20 +547,20 @@ func (s *Server) processDHCPAddrs(ctx *dnsContext) (rc resultCode) {
 		Ptr: dns.Fqdn(host),
 	}
 	resp.Answer = append(resp.Answer, ptr)
-	d.Res = resp
+	pctx.Res = resp

 	return resultCodeSuccess
 }

 // processLocalPTR responds to PTR requests if the target IP is detected to be
 // inside the local network and the query was not answered from DHCP.
-func (s *Server) processLocalPTR(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
-	if d.Res != nil {
+func (s *Server) processLocalPTR(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	if pctx.Res != nil {
 		return resultCodeSuccess
 	}

-	ip := ctx.unreversedReqIP
+	ip := dctx.unreversedReqIP
 	if ip == nil {
 		return resultCodeSuccess
 	}
@@ -579,16 +573,16 @@ func (s *Server) processLocalPTR(ctx *dnsContext) (rc resultCode) {
 	}

 	if s.conf.UsePrivateRDNS {
-		s.recDetector.add(*d.Req)
-		if err := s.localResolvers.Resolve(d); err != nil {
-			ctx.err = err
+		s.recDetector.add(*pctx.Req)
+		if err := s.localResolvers.Resolve(pctx); err != nil {
+			dctx.err = err

 			return resultCodeError
 		}
 	}

-	if d.Res == nil {
-		d.Res = s.genNXDomain(d.Req)
+	if pctx.Res == nil {
+		pctx.Res = s.genNXDomain(pctx.Req)

 		// Do not even put into query log.
 		return resultCodeFinish
@@ -633,24 +627,25 @@ func ipStringFromAddr(addr net.Addr) (ipStr string) {
 // processUpstream passes request to upstream servers and handles the response.
 func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) {
 	pctx := dctx.proxyCtx
+	req := pctx.Req
+	q := req.Question[0]
 	if pctx.Res != nil {
 		// The response has already been set.
 		return resultCodeSuccess
+	} else if reqHost, ok := s.isDHCPClientHostQ(q); ok {
+		// A DHCP client hostname query that hasn't been handled or filtered.
+		// Respond with an NXDOMAIN.
+		//
+		// TODO(a.garipov): Route such queries to a custom upstream for the
+		// local domain name if there is one.
+		log.Debug("dns: dhcp client hostname %q was not filtered", reqHost)
+		pctx.Res = s.genNXDomain(req)
+
+		return resultCodeFinish
 	}

-	if pctx.Addr != nil && s.conf.GetCustomUpstreamByClient != nil {
-		// Use the ClientID first, since it has a higher priority.
-		id := stringutil.Coalesce(dctx.clientID, ipStringFromAddr(pctx.Addr))
-		upsConf, err := s.conf.GetCustomUpstreamByClient(id)
-		if err != nil {
-			log.Error("dns: getting custom upstreams for client %s: %s", id, err)
-		} else if upsConf != nil {
-			log.Debug("dns: using custom upstreams for client %s", id)
-			pctx.CustomUpstreamConfig = upsConf
-		}
-	}
+	s.setCustomUpstream(pctx, dctx.clientID)

-	req := pctx.Req
 	origReqAD := false
 	if s.conf.EnableDNSSEC {
 		if req.AuthenticatedData {
@@ -683,52 +678,100 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) {
 	return resultCodeSuccess
 }

-// Apply filtering logic after we have received response from upstream servers
-func (s *Server) processFilteringAfterResponse(ctx *dnsContext) (rc resultCode) {
-	d := ctx.proxyCtx
+// isDHCPClientHostQ returns true if q is from a request for a DHCP client
+// hostname.  If ok is true, reqHost contains the requested hostname.
+func (s *Server) isDHCPClientHostQ(q dns.Question) (reqHost string, ok bool) {
+	if !s.dhcpServer.Enabled() {
+		return "", false
+	}

-	switch res := ctx.result; res.Reason {
+	// Include AAAA here, because despite the fact that we don't support it yet,
+	// the expected behavior here is to respond with an empty answer and not
+	// NXDOMAIN.
+	if qt := q.Qtype; qt != dns.TypeA && qt != dns.TypeAAAA {
+		return "", false
+	}
+
+	reqHost = strings.ToLower(q.Name[:len(q.Name)-1])
+	if strings.HasSuffix(reqHost, s.localDomainSuffix) {
+		return reqHost, true
+	}
+
+	return "", false
+}
+
+// setCustomUpstream sets custom upstream settings in pctx, if necessary.
+func (s *Server) setCustomUpstream(pctx *proxy.DNSContext, clientID string) {
+	customUpsByClient := s.conf.GetCustomUpstreamByClient
+	if pctx.Addr == nil || customUpsByClient == nil {
+		return
+	}
+
+	// Use the ClientID first, since it has a higher priority.
+	id := stringutil.Coalesce(clientID, ipStringFromAddr(pctx.Addr))
+	upsConf, err := customUpsByClient(id)
+	if err != nil {
+		log.Error("dns: getting custom upstreams for client %s: %s", id, err)
+
+		return
+	}
+
+	if upsConf != nil {
+		log.Debug("dns: using custom upstreams for client %s", id)
+	}
+
+	pctx.CustomUpstreamConfig = upsConf
+}
+
+// Apply filtering logic after we have received response from upstream servers
+func (s *Server) processFilteringAfterResponse(dctx *dnsContext) (rc resultCode) {
+	pctx := dctx.proxyCtx
+	switch res := dctx.result; res.Reason {
 	case filtering.NotFilteredAllowList:
-		// Go on.
+		return resultCodeSuccess
 	case
 		filtering.Rewritten,
 		filtering.RewrittenRule:

-		if len(ctx.origQuestion.Name) == 0 {
+		if dctx.origQuestion.Name == "" {
 			// origQuestion is set in case we get only CNAME without IP from
 			// rewrites table.
-			break
+			return resultCodeSuccess
 		}

-		d.Req.Question[0], d.Res.Question[0] = ctx.origQuestion, ctx.origQuestion
-		if len(d.Res.Answer) > 0 {
-			answer := append([]dns.RR{s.genAnswerCNAME(d.Req, res.CanonName)}, d.Res.Answer...)
-			d.Res.Answer = answer
+		pctx.Req.Question[0], pctx.Res.Question[0] = dctx.origQuestion, dctx.origQuestion
+		if len(pctx.Res.Answer) > 0 {
+			rr := s.genAnswerCNAME(pctx.Req, res.CanonName)
+			answer := append([]dns.RR{rr}, pctx.Res.Answer...)
+			pctx.Res.Answer = answer
 		}
+
+		return resultCodeSuccess
 	default:
-		// Check the response only if it's from an upstream.  Don't check the
-		// response if the protection is disabled since dnsrewrite rules aren't
-		// applied to it anyway.
-		if !ctx.protectionEnabled || !ctx.responseFromUpstream || s.dnsFilter == nil {
-			break
-		}
+		return s.filterAfterResponse(dctx, pctx)
+	}
+}

-		origResp := d.Res
-		result, err := s.filterDNSResponse(ctx)
-		if err != nil {
-			ctx.err = err
-
-			return resultCodeError
-		}
-
-		if result != nil {
-			ctx.result = result
-			ctx.origResp = origResp
-		}
+// filterAfterResponse returns the result of filtering the response that wasn't
+// explicitly allowed or rewritten.
+func (s *Server) filterAfterResponse(dctx *dnsContext, pctx *proxy.DNSContext) (res resultCode) {
+	// Check the response only if it's from an upstream.  Don't check the
+	// response if the protection is disabled since dnsrewrite rules aren't
+	// applied to it anyway.
+	if !dctx.protectionEnabled || !dctx.responseFromUpstream || s.dnsFilter == nil {
+		return resultCodeSuccess
 	}

-	if ctx.result == nil {
-		ctx.result = &filtering.Result{}
+	result, err := s.filterDNSResponse(pctx, dctx.setts)
+	if err != nil {
+		dctx.err = err
+
+		return resultCodeError
+	}
+
+	if result != nil {
+		dctx.result = result
+		dctx.origResp = pctx.Res
 	}

 	return resultCodeSuccess
--- a/internal/dnsforward/dns_test.go
+++ b/internal/dnsforward/dns_test.go
@@ -248,7 +248,7 @@ func TestServer_ProcessDHCPHosts_localRestriction(t *testing.T) {
 		name:       "local_client_unknown_host",
 		host:       "wronghost.lan",
 		wantIP:     nil,
-		wantRes:    resultCodeFinish,
+		wantRes:    resultCodeSuccess,
 		isLocalCli: true,
 	}, {
 		name:       "external_client_known_host",
@@ -358,7 +358,7 @@ func TestServer_ProcessDHCPHosts(t *testing.T) {
 		host:    "example-new.lan",
 		suffix:  defaultLocalDomainSuffix,
 		wantIP:  nil,
-		wantRes: resultCodeFinish,
+		wantRes: resultCodeSuccess,
 		qtyp:    dns.TypeA,
 	}, {
 		name:    "success_internal_aaaa",
--- a/internal/dnsforward/dnsrewrite.go
+++ b/internal/dnsforward/dnsrewrite.go
@@ -12,12 +12,16 @@ import (
 	"github.com/miekg/dns"
 )

-// filterDNSRewriteResponse handles a single DNS rewrite response entry.
-// 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.
+// filterDNSRewriteResponse handles a single DNS rewrite response entry.  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,
@@ -77,9 +81,13 @@ func (s *Server) filterDNSRewriteResponse(req *dns.Msg, rr rules.RRType, v rules
 	}
 }

-// filterDNSRewrite handles dnsrewrite filters.  It constructs a DNS
-// response and sets it into d.Res.
-func (s *Server) filterDNSRewrite(req *dns.Msg, res filtering.Result, d *proxy.DNSContext) (err error) {
+// filterDNSRewrite handles dnsrewrite filters.  It constructs a DNS response
+// and sets it into pctx.Res.  All parameters must not be nil.
+func (s *Server) filterDNSRewrite(
+	req *dns.Msg,
+	res *filtering.Result,
+	pctx *proxy.DNSContext,
+) (err error) {
 	resp := s.makeResponse(req)
 	dnsrr := res.DNSRewriteResult
 	if dnsrr == nil {
@@ -88,7 +96,7 @@ func (s *Server) filterDNSRewrite(req *dns.Msg, res filtering.Result, d *proxy.D

 	resp.Rcode = dnsrr.RCode
 	if resp.Rcode != dns.RcodeSuccess {
-		d.Res = resp
+		pctx.Res = resp

 		return nil
 	}
@@ -109,7 +117,7 @@ func (s *Server) filterDNSRewrite(req *dns.Msg, res filtering.Result, d *proxy.D
 		resp.Answer = append(resp.Answer, ans)
 	}

-	d.Res = resp
+	pctx.Res = resp

 	return nil
 }
--- a/internal/dnsforward/dnsrewrite_test.go
+++ b/internal/dnsforward/dnsrewrite_test.go
@@ -42,11 +42,11 @@ func TestServer_FilterDNSRewrite(t *testing.T) {
 			}},
 		}
 	}
-	makeRes := func(rcode rules.RCode, rr rules.RRType, v rules.RRValue) (res filtering.Result) {
+	makeRes := func(rcode rules.RCode, rr rules.RRType, v rules.RRValue) (res *filtering.Result) {
 		resp := filtering.DNSRewriteResultResponse{
 			rr: []rules.RRValue{v},
 		}
-		return filtering.Result{
+		return &filtering.Result{
 			DNSRewriteResult: &filtering.DNSRewriteResult{
 				RCode:    rcode,
 				Response: resp,
--- a/internal/dnsforward/filter.go
+++ b/internal/dnsforward/filter.go
@@ -49,69 +49,83 @@ func (s *Server) beforeRequestHandler(
 }

 // getClientRequestFilteringSettings looks up client filtering settings using
-// the client's IP address and ID, if any, from ctx.
-func (s *Server) getClientRequestFilteringSettings(ctx *dnsContext) *filtering.Settings {
+// the client's IP address and ID, if any, from dctx.
+func (s *Server) getClientRequestFilteringSettings(dctx *dnsContext) *filtering.Settings {
 	setts := s.dnsFilter.GetConfig()
-	setts.ProtectionEnabled = ctx.protectionEnabled
+	setts.ProtectionEnabled = dctx.protectionEnabled
 	if s.conf.FilterHandler != nil {
-		ip, _ := netutil.IPAndPortFromAddr(ctx.proxyCtx.Addr)
-		s.conf.FilterHandler(ip, ctx.clientID, &setts)
+		ip, _ := netutil.IPAndPortFromAddr(dctx.proxyCtx.Addr)
+		s.conf.FilterHandler(ip, dctx.clientID, &setts)
 	}

 	return &setts
 }

-// filterDNSRequest applies the dnsFilter and sets d.Res if the request was
-// filtered.
-func (s *Server) filterDNSRequest(ctx *dnsContext) (*filtering.Result, error) {
-	d := ctx.proxyCtx
-	req := d.Req
+// filterDNSRequest applies the dnsFilter and sets dctx.proxyCtx.Res if the
+// request was filtered.
+func (s *Server) filterDNSRequest(dctx *dnsContext) (res *filtering.Result, err error) {
+	pctx := dctx.proxyCtx
+	req := pctx.Req
 	q := req.Question[0]
 	host := strings.TrimSuffix(q.Name, ".")
-	res, err := s.dnsFilter.CheckHost(host, q.Qtype, ctx.setts)
+	resVal, err := s.dnsFilter.CheckHost(host, q.Qtype, dctx.setts)
+	if err != nil {
+		return nil, fmt.Errorf("checking host %q: %w", host, err)
+	}
+
+	// TODO(a.garipov): Make CheckHost return a pointer.
+	res = &resVal
 	switch {
-	case err != nil:
-		return nil, fmt.Errorf("failed to check host %q: %w", host, err)
 	case res.IsFiltered:
 		log.Tracef("host %q is filtered, reason %q, rule: %q", host, res.Reason, res.Rules[0].Text)
-		d.Res = s.genDNSFilterMessage(d, &res)
+		pctx.Res = s.genDNSFilterMessage(pctx, res)
 	case res.Reason.In(filtering.Rewritten, filtering.RewrittenRule) &&
 		res.CanonName != "" &&
 		len(res.IPList) == 0:
 		// Resolve the new canonical name, not the original host name.  The
 		// original question is readded in processFilteringAfterResponse.
-		ctx.origQuestion = q
+		dctx.origQuestion = q
 		req.Question[0].Name = dns.Fqdn(res.CanonName)
 	case res.Reason == filtering.Rewritten:
-		resp := s.makeResponse(req)
-
-		name := host
-		if len(res.CanonName) != 0 {
-			resp.Answer = append(resp.Answer, s.genAnswerCNAME(req, res.CanonName))
-			name = res.CanonName
-		}
-
-		for _, ip := range res.IPList {
-			switch q.Qtype {
-			case dns.TypeA:
-				a := s.genAnswerA(req, ip.To4())
-				a.Hdr.Name = dns.Fqdn(name)
-				resp.Answer = append(resp.Answer, a)
-			case dns.TypeAAAA:
-				a := s.genAnswerAAAA(req, ip)
-				a.Hdr.Name = dns.Fqdn(name)
-				resp.Answer = append(resp.Answer, a)
-			}
-		}
-
-		d.Res = resp
+		pctx.Res = s.filterRewritten(req, host, res, q.Qtype)
 	case res.Reason.In(filtering.RewrittenRule, filtering.RewrittenAutoHosts):
-		if err = s.filterDNSRewrite(req, res, d); err != nil {
+		if err = s.filterDNSRewrite(req, res, pctx); err != nil {
 			return nil, err
 		}
 	}

-	return &res, err
+	return res, err
+}
+
+// filterRewritten handles DNS rewrite filters.  It returns a DNS response with
+// the data from the filtering result.  All parameters must not be nil.
+func (s *Server) filterRewritten(
+	req *dns.Msg,
+	host string,
+	res *filtering.Result,
+	qt uint16,
+) (resp *dns.Msg) {
+	resp = s.makeResponse(req)
+	name := host
+	if len(res.CanonName) != 0 {
+		resp.Answer = append(resp.Answer, s.genAnswerCNAME(req, res.CanonName))
+		name = res.CanonName
+	}
+
+	for _, ip := range res.IPList {
+		switch qt {
+		case dns.TypeA:
+			a := s.genAnswerA(req, ip.To4())
+			a.Hdr.Name = dns.Fqdn(name)
+			resp.Answer = append(resp.Answer, a)
+		case dns.TypeAAAA:
+			a := s.genAnswerAAAA(req, ip)
+			a.Hdr.Name = dns.Fqdn(name)
+			resp.Answer = append(resp.Answer, a)
+		}
+	}
+
+	return resp
 }

 // checkHostRules checks the host against filters.  It is safe for concurrent
@@ -137,16 +151,17 @@ func (s *Server) checkHostRules(host string, rrtype uint16, setts *filtering.Set
 }

 // filterDNSResponse checks each resource record of the response's answer
-// section from ctx and returns a non-nil res if at least one of canonnical
+// section from pctx and returns a non-nil res if at least one of canonnical
 // names or IP addresses in it matches the filtering rules.
-func (s *Server) filterDNSResponse(ctx *dnsContext) (res *filtering.Result, err error) {
-	d := ctx.proxyCtx
-	setts := ctx.setts
+func (s *Server) filterDNSResponse(
+	pctx *proxy.DNSContext,
+	setts *filtering.Settings,
+) (res *filtering.Result, err error) {
 	if !setts.FilteringEnabled {
 		return nil, nil
 	}

-	for _, a := range d.Res.Answer {
+	for _, a := range pctx.Res.Answer {
 		host := ""
 		var rrtype uint16
 		switch a := a.(type) {
@@ -171,8 +186,8 @@ func (s *Server) filterDNSResponse(ctx *dnsContext) (res *filtering.Result, err
 		} else if res == nil {
 			continue
 		} else if res.IsFiltered {
-			d.Res = s.genDNSFilterMessage(d, res)
-			log.Debug("DNSFwd: Matched %s by response: %s", d.Req.Question[0].Name, host)
+			pctx.Res = s.genDNSFilterMessage(pctx, res)
+			log.Debug("DNSFwd: Matched %s by response: %s", pctx.Req.Question[0].Name, host)

 			return res, nil
 		}
--- a/internal/filtering/dnsrewrite.go
+++ b/internal/filtering/dnsrewrite.go
@@ -34,8 +34,8 @@ func (d *DNSFilter) processDNSRewrites(dnsr []*rules.NetworkRule) (res Result) {
 			}}

 			return Result{
-				Reason:    RewrittenRule,
 				Rules:     rules,
+				Reason:    RewrittenRule,
 				CanonName: dr.NewCNAME,
 			}
 		}
@@ -60,16 +60,16 @@ func (d *DNSFilter) processDNSRewrites(dnsr []*rules.NetworkRule) (res Result) {
 			}

 			return Result{
-				Reason:           RewrittenRule,
-				Rules:            rules,
 				DNSRewriteResult: dnsrr,
+				Rules:            rules,
+				Reason:           RewrittenRule,
 			}
 		}
 	}

 	return Result{
-		Reason:           RewrittenRule,
-		Rules:            rules,
 		DNSRewriteResult: dnsrr,
+		Rules:            rules,
+		Reason:           RewrittenRule,
 	}
 }
--- a/internal/filtering/filtering.go
+++ b/internal/filtering/filtering.go
@@ -390,18 +390,8 @@ type ResultRule struct {
 // TODO(a.garipov): Clarify relationships between fields.  Perhaps
 // replace with a sum type or an interface?
 type Result struct {
-	// IsFiltered is true if the request is filtered.
-	IsFiltered bool `json:",omitempty"`
-
-	// Reason is the reason for blocking or unblocking the request.
-	Reason Reason `json:",omitempty"`
-
-	// Rules are applied rules.  If Rules are not empty, each rule is not nil.
-	Rules []*ResultRule `json:",omitempty"`
-
-	// IPList is the lookup rewrite result.  It is empty unless Reason is set to
-	// Rewritten.
-	IPList []net.IP `json:",omitempty"`
+	// DNSRewriteResult is the $dnsrewrite filter rule result.
+	DNSRewriteResult *DNSRewriteResult `json:",omitempty"`

 	// CanonName is the CNAME value from the lookup rewrite result.  It is empty
 	// unless Reason is set to Rewritten or RewrittenRule.
@@ -411,8 +401,18 @@ type Result struct {
 	// Reason is set to FilteredBlockedService.
 	ServiceName string `json:",omitempty"`

-	// DNSRewriteResult is the $dnsrewrite filter rule result.
-	DNSRewriteResult *DNSRewriteResult `json:",omitempty"`
+	// IPList is the lookup rewrite result.  It is empty unless Reason is set to
+	// Rewritten.
+	IPList []net.IP `json:",omitempty"`
+
+	// Rules are applied rules.  If Rules are not empty, each rule is not nil.
+	Rules []*ResultRule `json:",omitempty"`
+
+	// Reason is the reason for blocking or unblocking the request.
+	Reason Reason `json:",omitempty"`
+
+	// IsFiltered is true if the request is filtered.
+	IsFiltered bool `json:",omitempty"`
 }

 // Matched returns true if any match at all was found regardless of
@@ -874,9 +874,9 @@ func makeResult(matchedRules []rules.Rule, reason Reason) (res Result) {
 	}

 	return Result{
-		IsFiltered: reason == FilteredBlockList,
-		Reason:     reason,
 		Rules:      resRules,
+		Reason:     reason,
+		IsFiltered: reason == FilteredBlockList,
 	}
 }

--- a/internal/filtering/safebrowsing.go
+++ b/internal/filtering/safebrowsing.go
@@ -325,12 +325,12 @@ func (d *DNSFilter) checkSafeBrowsing(
 	}

 	res = Result{
-		IsFiltered: true,
-		Reason:     FilteredSafeBrowsing,
 		Rules: []*ResultRule{{
 			Text:         "adguard-malware-shavar",
 			FilterListID: SafeBrowsingListID,
 		}},
+		Reason:     FilteredSafeBrowsing,
+		IsFiltered: true,
 	}

 	return check(sctx, res, d.safeBrowsingUpstream)
@@ -359,12 +359,12 @@ func (d *DNSFilter) checkParental(
 	}

 	res = Result{
-		IsFiltered: true,
-		Reason:     FilteredParental,
 		Rules: []*ResultRule{{
 			Text:         "parental CATEGORY_BLACKLISTED",
 			FilterListID: ParentalListID,
 		}},
+		Reason:     FilteredParental,
+		IsFiltered: true,
 	}

 	return check(sctx, res, d.parentalUpstream)
--- a/internal/filtering/safesearch.go
+++ b/internal/filtering/safesearch.go
@@ -98,11 +98,11 @@ func (d *DNSFilter) checkSafeSearch(
 	}

 	res = Result{
-		IsFiltered: true,
-		Reason:     FilteredSafeSearch,
 		Rules: []*ResultRule{{
 			FilterListID: SafeSearchListID,
 		}},
+		Reason:     FilteredSafeSearch,
+		IsFiltered: true,
 	}

 	if ip := net.ParseIP(safeHost); ip != nil {
--- a/internal/querylog/decode_test.go
+++ b/internal/querylog/decode_test.go
@@ -63,9 +63,15 @@ func TestDecodeLogEntry(t *testing.T) {
 			Answer:      ans,
 			Cached:      true,
 			Result: filtering.Result{
-				IsFiltered: true,
-				Reason:     filtering.FilteredBlockList,
-				IPList:     []net.IP{net.IPv4(127, 0, 0, 2)},
+				DNSRewriteResult: &filtering.DNSRewriteResult{
+					RCode: dns.RcodeSuccess,
+					Response: filtering.DNSRewriteResultResponse{
+						dns.TypeA: []rules.RRValue{net.IPv4(127, 0, 0, 2)},
+					},
+				},
+				CanonName:   "example.com",
+				ServiceName: "example.org",
+				IPList:      []net.IP{net.IPv4(127, 0, 0, 2)},
 				Rules: []*filtering.ResultRule{{
 					FilterListID: 42,
 					Text:         "||an.yandex.ru",
@@ -75,14 +81,8 @@ func TestDecodeLogEntry(t *testing.T) {
 					Text:         "||an2.yandex.ru",
 					IP:           net.IPv4(127, 0, 0, 3),
 				}},
-				CanonName:   "example.com",
-				ServiceName: "example.org",
-				DNSRewriteResult: &filtering.DNSRewriteResult{
-					RCode: dns.RcodeSuccess,
-					Response: filtering.DNSRewriteResultResponse{
-						dns.TypeA: []rules.RRValue{net.IPv4(127, 0, 0, 2)},
-					},
-				},
+				Reason:     filtering.FilteredBlockList,
+				IsFiltered: true,
 			},
 			Upstream:          "https://some.upstream",
 			Elapsed:           837429,
--- a/internal/querylog/qlog_test.go
+++ b/internal/querylog/qlog_test.go
@@ -271,13 +271,13 @@ func addEntry(l *queryLog, host string, answerStr, client net.IP) {
 	}

 	res := filtering.Result{
-		IsFiltered:  true,
-		Reason:      filtering.Rewritten,
 		ServiceName: "SomeService",
 		Rules: []*filtering.ResultRule{{
 			FilterListID: 1,
 			Text:         "SomeRule",
 		}},
+		Reason:     filtering.Rewritten,
+		IsFiltered: true,
 	}

 	params := &AddParams{