From 67cdc52fcdfbe34f444a8400f6320222fd3115a1 Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Thu, 24 Sep 2020 17:11:29 +0200
Subject: [PATCH] Implement Feedback

---
 firewall/bypassing.go       |  2 +-
 nameserver/nsutil/nsutil.go | 40 +++++++++++++++++++++++++++----------
 network/dns.go              |  6 +++---
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/firewall/bypassing.go b/firewall/bypassing.go
index 7416cc93..cd811d8c 100644
--- a/firewall/bypassing.go
+++ b/firewall/bypassing.go
@@ -15,7 +15,7 @@ func PreventBypassing(conn *network.Connection) (endpoints.EPResult, string, nsu
 	if strings.ToLower(conn.Entity.Domain) == "use-application-dns.net." {
 		return endpoints.Denied,
 			"blocked canary domain to prevent enabling of DNS-over-HTTPs",
-			nsutil.NxDomain("")
+			nsutil.NxDomain()
 	}
 
 	return endpoints.NoMatch, "", nil
diff --git a/nameserver/nsutil/nsutil.go b/nameserver/nsutil/nsutil.go
index 7307f763..d427a376 100644
--- a/nameserver/nsutil/nsutil.go
+++ b/nameserver/nsutil/nsutil.go
@@ -2,6 +2,7 @@ package nsutil
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -40,7 +41,7 @@ func (rf ResponderFunc) ReplyWithDNS(ctx context.Context, request *dns.Msg) *dns
 
 // ZeroIP is a ResponderFunc than replies with either 0.0.0.0 or :: for
 // each A or AAAA question respectively.
-func ZeroIP(msg string) ResponderFunc {
+func ZeroIP(msgs ...string) ResponderFunc {
 	return func(ctx context.Context, request *dns.Msg) *dns.Msg {
 		reply := new(dns.Msg)
 		hasErr := false
@@ -73,14 +74,16 @@ func ZeroIP(msg string) ResponderFunc {
 			reply.SetRcode(request, dns.RcodeSuccess)
 		}
 
-		AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		for _, msg := range msgs {
+			AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		}
 
 		return reply
 	}
 }
 
 // Localhost is a ResponderFunc than replies with localhost IP addresses.
-func Localhost(msg string) ResponderFunc {
+func Localhost(msgs ...string) ResponderFunc {
 	return func(ctx context.Context, request *dns.Msg) *dns.Msg {
 		reply := new(dns.Msg)
 		hasErr := false
@@ -113,35 +116,43 @@ func Localhost(msg string) ResponderFunc {
 			reply.SetRcode(request, dns.RcodeSuccess)
 		}
 
-		AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		for _, msg := range msgs {
+			AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		}
 
 		return reply
 	}
 }
 
 // NxDomain returns a ResponderFunc that replies with NXDOMAIN.
-func NxDomain(msg string) ResponderFunc {
+func NxDomain(msgs ...string) ResponderFunc {
 	return func(ctx context.Context, request *dns.Msg) *dns.Msg {
 		reply := new(dns.Msg).SetRcode(request, dns.RcodeNameError)
-		AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		for _, msg := range msgs {
+			AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		}
 		return reply
 	}
 }
 
 // Refused returns a ResponderFunc that replies with REFUSED.
-func Refused(msg string) ResponderFunc {
+func Refused(msgs ...string) ResponderFunc {
 	return func(ctx context.Context, request *dns.Msg) *dns.Msg {
 		reply := new(dns.Msg).SetRcode(request, dns.RcodeRefused)
-		AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		for _, msg := range msgs {
+			AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		}
 		return reply
 	}
 }
 
 // ServerFailure returns a ResponderFunc that replies with SERVFAIL.
-func ServerFailure(msg string) ResponderFunc {
+func ServerFailure(msgs ...string) ResponderFunc {
 	return func(ctx context.Context, request *dns.Msg) *dns.Msg {
 		reply := new(dns.Msg).SetRcode(request, dns.RcodeServerFailure)
-		AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		for _, msg := range msgs {
+			AddMessageToReply(ctx, reply, log.InfoLevel, msg)
+		}
 		return reply
 	}
 }
@@ -149,11 +160,18 @@ func ServerFailure(msg string) ResponderFunc {
 // MakeMessageRecord creates an informational resource record that can be added
 // to the extra section of a reply.
 func MakeMessageRecord(level log.Severity, msg string) (dns.RR, error) { //nolint:interfacer
-	return dns.NewRR(fmt.Sprintf(
+	rr, err := dns.NewRR(fmt.Sprintf(
 		`%s.portmaster. 0 IN TXT "%s"`,
 		strings.ToLower(level.String()),
 		msg,
 	))
+	if err != nil {
+		return nil, err
+	}
+	if rr == nil {
+		return nil, errors.New("record is nil")
+	}
+	return rr, nil
 }
 
 // AddMessageToReply creates an information resource records using
diff --git a/network/dns.go b/network/dns.go
index de46d8de..c22867ef 100644
--- a/network/dns.go
+++ b/network/dns.go
@@ -98,13 +98,13 @@ func (conn *Connection) ReplyWithDNS(ctx context.Context, request *dns.Msg) *dns
 	// Select request responder.
 	switch conn.Verdict {
 	case VerdictBlock:
-		return nsutil.ZeroIP("").ReplyWithDNS(ctx, request)
+		return nsutil.ZeroIP().ReplyWithDNS(ctx, request)
 	case VerdictDrop:
 		return nil // Do not respond to request.
 	case VerdictFailed:
-		return nsutil.ZeroIP("").ReplyWithDNS(ctx, request)
+		return nsutil.ZeroIP().ReplyWithDNS(ctx, request)
 	default:
-		reply := nsutil.ServerFailure("").ReplyWithDNS(ctx, request)
+		reply := nsutil.ServerFailure().ReplyWithDNS(ctx, request)
 		nsutil.AddMessageToReply(ctx, reply, log.ErrorLevel, "INTERNAL ERROR: incorrect use of network.Connection's DNS Responder")
 		return reply
 	}