Merge pull request #138 from safing/fix/catch-dns-write-error

Improve resilience and logging in resolver and nameserver

nameserver/nameserver.go

@@ -3,6 +3,7 @@ package nameserver
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"strings"
 
@@ -91,7 +92,7 @@ func stop() error {
 func returnServerFailure(w dns.ResponseWriter, query *dns.Msg) {
 	m := new(dns.Msg)
 	m.SetRcode(query, dns.RcodeServerFailure)
-	_ = w.WriteMsg(m)
+	_ = writeDNSResponse(w, m)
 }
 
 func handleRequestAsWorker(w dns.ResponseWriter, query *dns.Msg) {
@@ -132,7 +133,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er
 		m := new(dns.Msg)
 		m.SetReply(query)
 		m.Answer = localhostRRs
-		if err := w.WriteMsg(m); err != nil {
+		if err := writeDNSResponse(w, m); err != nil {
 			log.Warningf("nameserver: failed to handle request to %s: %s", q.FQDN, err)
 		}
 		return nil
@@ -270,7 +271,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er
 	m.Ns = rrCache.Ns
 	m.Extra = rrCache.Extra
 
-	if err := w.WriteMsg(m); err != nil {
+	if err := writeDNSResponse(w, m); err != nil {
 		tracer.Warningf("nameserver: failed to return response %s%s to %s: %s", q.FQDN, q.QType, conn.Process(), err)
 	} else {
 		tracer.Debugf("nameserver: returning response %s%s to %s", q.FQDN, q.QType, conn.Process())
@@ -281,3 +282,16 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er
 
 	return nil
 }
+
+func writeDNSResponse(w dns.ResponseWriter, m *dns.Msg) (err error) {
+	defer func() {
+		// recover from panic
+		if panicErr := recover(); panicErr != nil {
+			err = fmt.Errorf("panic: %s", panicErr)
+			log.Warningf("nameserver: panic caused by this msg: %#v", m)
+		}
+	}()
+
+	err = w.WriteMsg(m)
+	return
+}

nameserver/response.go

@@ -30,7 +30,7 @@ func sendResponse(w dns.ResponseWriter, query *dns.Msg, verdict network.Verdict,
 		reply.Extra = append(reply.Extra, rrs...)
 	}
 
-	if err := w.WriteMsg(reply); err != nil {
+	if err := writeDNSResponse(w, reply); err != nil {
 		log.Errorf("nameserver: failed to send response: %s", err)
 	}
 }

resolver/rrcache.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/safing/portbase/log"
 	"github.com/safing/portmaster/netenv"
 
 	"github.com/miekg/dns"
@@ -163,22 +164,13 @@ func GetRRCache(domain string, question dns.Type) (*RRCache, error) {
 
 	rrCache.TTL = nameRecord.TTL
 	for _, entry := range nameRecord.Answer {
-		rr, err := dns.NewRR(entry)
-		if err == nil {
-			rrCache.Answer = append(rrCache.Answer, rr)
-		}
+		rrCache.Answer = parseRR(rrCache.Answer, entry)
 	}
 	for _, entry := range nameRecord.Ns {
-		rr, err := dns.NewRR(entry)
-		if err == nil {
-			rrCache.Ns = append(rrCache.Ns, rr)
-		}
+		rrCache.Ns = parseRR(rrCache.Ns, entry)
 	}
 	for _, entry := range nameRecord.Extra {
-		rr, err := dns.NewRR(entry)
-		if err == nil {
-			rrCache.Extra = append(rrCache.Extra, rr)
-		}
+		rrCache.Extra = parseRR(rrCache.Extra, entry)
 	}
 
 	rrCache.Server = nameRecord.Server
@@ -187,6 +179,19 @@ func GetRRCache(domain string, question dns.Type) (*RRCache, error) {
 	return rrCache, nil
 }
 
+func parseRR(section []dns.RR, entry string) []dns.RR {
+	rr, err := dns.NewRR(entry)
+	switch {
+	case err != nil:
+		log.Warningf("resolver: failed to parse cached record %q: %s", entry, err)
+	case rr == nil:
+		log.Warningf("resolver: failed to parse cached record %q: resulted in nil record", entry)
+	default:
+		return append(section, rr)
+	}
+	return section
+}
+
 // ServedFromCache marks the RRCache as served from cache.
 func (rrCache *RRCache) ServedFromCache() bool {
 	return rrCache.servedFromCache