Implement review suggestions

Fix local. resolving in Firefox

netenv/environment_linux.go

@@ -45,14 +45,14 @@ func Gateways() []net.IP {
 	}()
 	// logic
 
-	newGateways := make([]net.IP, 0)
+	gateways = make([]net.IP, 0)
 	var decoded []byte
 
 	// open file
 	route, err := os.Open("/proc/net/route")
 	if err != nil {
 		log.Warningf("environment: could not read /proc/net/route: %s", err)
-		return newGateways
+		return gateways
 	}
 	defer route.Close()
 
@@ -77,7 +77,7 @@ func Gateways() []net.IP {
 				continue
 			}
 			gate := net.IPv4(decoded[3], decoded[2], decoded[1], decoded[0])
-			newGateways = append(newGateways, gate)
+			gateways = append(gateways, gate)
 		}
 	}
 
@@ -85,7 +85,7 @@ func Gateways() []net.IP {
 	v6route, err := os.Open("/proc/net/ipv6_route")
 	if err != nil {
 		log.Warningf("environment: could not read /proc/net/ipv6_route: %s", err)
-		return newGateways
+		return gateways
 	}
 	defer v6route.Close()
 
@@ -110,11 +110,11 @@ func Gateways() []net.IP {
 				continue
 			}
 			gate := net.IP(decoded)
-			newGateways = append(newGateways, gate)
+			gateways = append(gateways, gate)
 		}
 	}
 
-	return newGateways
+	return gateways
 }
 
 // Nameservers returns the currently active nameservers.

netenv/online-status.go

@@ -2,7 +2,6 @@ package netenv
 
 import (
 	"context"
-	"errors"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -111,30 +110,13 @@ var (
 	captivePortalLock sync.Mutex
 )
 
+// CaptivePortal holds information about a detected captive portal.
 type CaptivePortal struct {
 	URL    string
 	Domain string
 	IP     net.IP
 }
 
-// IPasRR returns the captive portal IP as a DNS resource record.
-func (p *CaptivePortal) IPasRR() (rr dns.RR, err error) {
-	switch {
-	case p.IP == nil:
-		return nil, errors.New("no portal IP present")
-	case p.Domain == "":
-		return nil, errors.New("no portal domain present")
-	case p.IP.To4() != nil:
-		rr, err = dns.NewRR(p.Domain + " 17 IN A " + p.IP.String())
-	default:
-		rr, err = dns.NewRR(p.Domain + " 17 IN AAAA " + p.IP.String())
-	}
-	if err != nil {
-		return nil, err
-	}
-	return rr, nil
-}
-
 func init() {
 	var onlineStatusValue int32
 	onlineStatus = &onlineStatusValue

network/connection.go

@@ -152,7 +152,7 @@ func NewConnectionFromFirstPacket(pkt packet.Packet) *Connection {
 
 		// check if destination IP is the captive portal's IP
 		portal := netenv.GetCaptivePortal()
-		if portal.IP != nil && pkt.Info().Dst.Equal(portal.IP) {
+		if pkt.Info().Dst.Equal(portal.IP) {
 			scope = portal.Domain
 			entity.Domain = portal.Domain
 		}

network/netutils/dns.go

@@ -1,6 +1,8 @@
 package netutils
 
 import (
+	"fmt"
+	"net"
 	"regexp"
 
 	"github.com/miekg/dns"
@@ -56,3 +58,24 @@ func IsValidFqdn(fqdn string) bool {
 	_, ok := dns.IsDomainName(fqdn)
 	return ok
 }
+
+// IPsToRRs transforms the given IPs to resource records.
+func IPsToRRs(domain string, ips []net.IP) ([]dns.RR, error) {
+	records := make([]dns.RR, 0, len(ips))
+	var rr dns.RR
+	var err error
+
+	for _, ip := range ips {
+		if ip.To4() != nil {
+			rr, err = dns.NewRR(fmt.Sprintf("%s 17 IN A %s", domain, ip))
+		} else {
+			rr, err = dns.NewRR(fmt.Sprintf("%s 17 IN AAAA %s", domain, ip))
+		}
+		if err != nil {
+			return nil, fmt.Errorf("failed to create record for %s: %w", ip, err)
+		}
+		records = append(records, rr)
+	}
+
+	return records, nil
+}

resolver/main.go

@@ -24,6 +24,10 @@ func init() {
 func prep() error {
 	intel.SetReverseResolver(ResolveIPAndValidate)
 
+	if err := prepEnvResolver(); err != nil {
+		return err
+	}
+
 	return prepConfig()
 }
 

resolver/resolve.go

@@ -261,6 +261,8 @@ resolveLoop:
 				case errors.Is(err, ErrBlocked):
 					// some resolvers might also block
 					return nil, err
+				case errors.Is(err, ErrContinue):
+					continue
 				case netenv.GetOnlineStatus() == netenv.StatusOffline &&
 					!netenv.IsConnectivityDomain(q.FQDN):
 					log.Tracer(ctx).Debugf("resolver: not resolving %s, device is offline", q.FQDN)

resolver/resolver-env.go

@@ -2,7 +2,6 @@ package resolver
 
 import (
 	"context"
-	"fmt"
 	"net"
 
 	"github.com/miekg/dns"
@@ -19,8 +18,15 @@ var (
 		Source:        ServerSourceEnv,
 		Conn:          &envResolverConn{},
 	}
+
+	localSOA dns.RR
 )
 
+func prepEnvResolver() (err error) {
+	localSOA, err = dns.NewRR("local. 17 IN SOA localhost. none.localhost. 17 17 17 17 17")
+	return err
+}
+
 type envResolverConn struct{}
 
 func (er *envResolverConn) Query(ctx context.Context, q *Query) (*RRCache, error) {
@@ -29,14 +35,22 @@ func (er *envResolverConn) Query(ctx context.Context, q *Query) (*RRCache, error
 
 	// check for matching name
 	switch q.FQDN {
+	case "local.":
+		// Firefox requests the SOA request for local. before resolving any local. domains.
+		// Others might be doing this too. We guessed this behaviour, weren't able to find docs.
+		if q.QType == dns.Type(dns.TypeSOA) {
+			return er.makeRRCache(q, []dns.RR{localSOA}), nil
+		}
+		return nil, ErrNotFound
+
 	case netenv.SpecialCaptivePortalDomain:
 		if portal.IP != nil {
-			rr, err := portal.IPasRR()
+			records, err := netutils.IPsToRRs(q.FQDN, []net.IP{portal.IP})
 			if err != nil {
 				log.Warningf("nameserver: failed to create captive portal response to %s: %s", q.FQDN, err)
 				return nil, ErrNotFound
 			}
-			return er.makeRRCache(q, []dns.RR{rr}), nil
+			return er.makeRRCache(q, records), nil
 		}
 		return nil, ErrNotFound
 
@@ -45,7 +59,7 @@ func (er *envResolverConn) Query(ctx context.Context, q *Query) (*RRCache, error
 		if len(routers) == 0 {
 			return nil, ErrNotFound
 		}
-		records, err := ipsToRRs(q.FQDN, routers)
+		records, err := netutils.IPsToRRs(q.FQDN, routers)
 		if err != nil {
 			log.Warningf("nameserver: failed to create gateway response to %s: %s", q.FQDN, err)
 			return nil, ErrNotFound
@@ -73,23 +87,3 @@ func (er *envResolverConn) ReportFailure() {}
 func (er *envResolverConn) IsFailing() bool {
 	return false
 }
-
-func ipsToRRs(domain string, ips []net.IP) ([]dns.RR, error) {
-	var records []dns.RR
-	var rr dns.RR
-	var err error
-
-	for _, ip := range ips {
-		if ip.To4() != nil {
-			rr, err = dns.NewRR(domain + " 17 IN A " + ip.String())
-		} else {
-			rr, err = dns.NewRR(domain + " 17 IN AAAA " + ip.String())
-		}
-		if err != nil {
-			return nil, fmt.Errorf("failed to create record for %s: %w", ip, err)
-		}
-		records = append(records, rr)
-	}
-
-	return records, nil
-}

resolver/reverse_test.go

@@ -32,6 +32,6 @@ func TestResolveIPAndValidate(t *testing.T) {
 	testReverse(t, "1.1.1.1", "one.one.one.one.", "")
 	testReverse(t, "2606:4700:4700::1111", "one.one.one.one.", "")
 
-	testReverse(t, "93.184.216.34", "example.com.", "record does not exist: 34.216.184.93.in-addr.arpa.PTR")
-	testReverse(t, "185.199.109.153", "sites.github.io.", "record does not exist: 153.109.199.185.in-addr.arpa.PTR")
+	testReverse(t, "93.184.216.34", "example.com.", "record could not be found: 34.216.184.93.in-addr.arpa.PTR")
+	testReverse(t, "185.199.109.153", "sites.github.io.", "record could not be found: 153.109.199.185.in-addr.arpa.PTR")
 }