Fix handling of connectivity / captive portal domains

Also, improve handling of queries during being captive.

firewall/dns.go

@@ -1,9 +1,11 @@
 package firewall
 
 import (
+	"context"
 	"net"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/miekg/dns"
 	"github.com/safing/portbase/database"
@@ -77,18 +79,13 @@ func filterDNSSection(entries []dns.RR, p *profile.LayeredProfile, scope int8) (
 }
 
 func filterDNSResponse(conn *network.Connection, rrCache *resolver.RRCache) *resolver.RRCache {
+	p := conn.Process().Profile()
+
 	// do not modify own queries
 	if conn.Process().Pid == os.Getpid() {
 		return rrCache
 	}
 
-	// get profile
-	p := conn.Process().Profile()
-	if p == nil {
-		conn.Block("no profile")
-		return nil
-	}
-
 	// check if DNS response filtering is completely turned off
 	if !p.RemoveOutOfScopeDNS() && !p.RemoveBlockedDNS() {
 		return rrCache
@@ -112,6 +109,31 @@ func filterDNSResponse(conn *network.Connection, rrCache *resolver.RRCache) *res
 		rrCache.Filtered = true
 		if validIPs == 0 {
 			conn.Block("no addresses returned for this domain are permitted")
+
+			// If all entries are filtered, this could mean that these are broken/bogus resource records.
+			if rrCache.Expired() {
+				// If the entry is expired, force delete it.
+				err := resolver.DeleteNameRecord(rrCache.Domain, rrCache.Question.String())
+				if err != nil && err != database.ErrNotFound {
+					log.Warningf(
+						"filter: failed to delete fully filtered name cache for %s: %s",
+						rrCache.ID(),
+						err,
+					)
+				}
+			} else if rrCache.TTL > time.Now().Add(10*time.Second).Unix() {
+				// Set a low TTL of 10 seconds if TTL is higher than that.
+				rrCache.TTL = time.Now().Add(10 * time.Second).Unix()
+				err := rrCache.Save()
+				if err != nil {
+					log.Debugf(
+						"filter: failed to set shorter TTL on fully filtered name cache for %s: %s",
+						rrCache.ID(),
+						err,
+					)
+				}
+			}
+
 			return nil
 		}
 
@@ -122,7 +144,25 @@ func filterDNSResponse(conn *network.Connection, rrCache *resolver.RRCache) *res
 }
 
 // DecideOnResolvedDNS filters a dns response according to the application profile and settings.
-func DecideOnResolvedDNS(conn *network.Connection, q *resolver.Query, rrCache *resolver.RRCache) *resolver.RRCache {
+func DecideOnResolvedDNS(
+	ctx context.Context,
+	conn *network.Connection,
+	q *resolver.Query,
+	rrCache *resolver.RRCache,
+) *resolver.RRCache {
+
+	// check profile
+	if checkProfileExists(ctx, conn, nil) {
+		// returns true if check triggered
+		return nil
+	}
+
+	// special grant for connectivity domains
+	if checkConnectivityDomain(ctx, conn, nil) {
+		// returns true if check triggered
+		return rrCache
+	}
+
 	updatedRR := filterDNSResponse(conn, rrCache)
 	if updatedRR == nil {
 		return nil

firewall/master.go

@@ -52,7 +52,7 @@ func DecideOnConnection(ctx context.Context, conn *network.Connection, pkt packe
 		checkSelfCommunication,
 		checkProfileExists,
 		checkConnectionType,
-		checkCaptivePortal,
+		checkConnectivityDomain,
 		checkConnectionScope,
 		checkEndpointLists,
 		checkBypassPrevention,
@@ -181,10 +181,17 @@ func checkConnectionType(ctx context.Context, conn *network.Connection, _ packet
 	return false
 }
 
-func checkCaptivePortal(_ context.Context, conn *network.Connection, _ packet.Packet) bool {
-	if netenv.GetOnlineStatus() == netenv.StatusPortal &&
-		conn.Entity.Domain == netenv.GetCaptivePortal().Domain {
-		conn.Accept("captive portal access permitted")
+func checkConnectivityDomain(_ context.Context, conn *network.Connection, _ packet.Packet) bool {
+	p := conn.Process().Profile()
+
+	if !p.BlockScopeInternet() {
+		// Special grant only applies if application is allowed to connect to the Internet.
+		return false
+	}
+
+	if netenv.GetOnlineStatus() <= netenv.StatusPortal &&
+		netenv.IsConnectivityDomain(conn.Entity.Domain) {
+		conn.Accept("special grant for connectivity domain during network bootstrap")
 		return true
 	}
 

nameserver/nameserver.go

@@ -274,7 +274,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, query *dns.Msg) er
 	}
 
 	tracer.Trace("nameserver: deciding on resolved dns")
-	rrCache = firewall.DecideOnResolvedDNS(conn, q, rrCache)
+	rrCache = firewall.DecideOnResolvedDNS(ctx, conn, q, rrCache)
 	if rrCache == nil {
 		sendResponse(w, query, conn.Verdict, conn.Reason, conn.ReasonContext)
 		return nil

netenv/online-status.go

@@ -11,6 +11,10 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/safing/portbase/database"
+
+	"github.com/safing/portbase/notifications"
+
 	"github.com/miekg/dns"
 
 	"github.com/safing/portbase/log"
@@ -67,8 +71,41 @@ func init() {
 // IsConnectivityDomain checks whether the given domain (fqdn) is used for any connectivity related network connections and should always be resolved using the network assigned DNS server.
 func IsConnectivityDomain(domain string) bool {
 	switch domain {
-	case "detectportal.firefox.com.",
-		"one.one.one.one.",
+	case "one.one.one.one.", // Internal DNS Check
+
+		// Windows
+		"dns.msftncsi.com.", // DNS Check
+		"msftncsi.com.",     // Older
+		"www.msftncsi.com.",
+		"microsoftconnecttest.com.", // Newer
+		"www.microsoftconnecttest.com.",
+		"ipv6.microsoftconnecttest.com.",
+		// https://de.wikipedia.org/wiki/Captive_Portal
+		// https://docs.microsoft.com/en-us/windows-hardware/drivers/mobilebroadband/captive-portals
+		// TODO: read value from registry: HKLM:\SYSTEM\CurrentControlSet\Services\NlaSvc\Parameters\Internet
+
+		// Apple
+		"captive.apple.com.",
+		// https://de.wikipedia.org/wiki/Captive_Portal
+
+		// Linux
+		"connectivity-check.ubuntu.com.", // Ubuntu
+		"nmcheck.gnome.org.",             // Gnome DE
+		"network-test.debian.org.",       // Debian
+		// There are probably a lot more domains for all the Linux Distro/DE Variants. Please raise issues and/or submit PRs!
+		// https://github.com/solus-project/budgie-desktop/issues/807
+		// https://www.lguruprasad.in/blog/2015/07/21/enabling-captive-portal-detection-in-gnome-3-14-on-debian-jessie/
+		// TODO: read value from NetworkManager config: /etc/NetworkManager/conf.d/*.conf
+
+		// Android
+		"connectivitycheck.gstatic.com.",
+		// https://de.wikipedia.org/wiki/Captive_Portal
+
+		// Other
+		"neverssl.com.",             // Common Community Service
+		"detectportal.firefox.com.", // Firefox
+
+		// Redirected Domain
 		GetCaptivePortal().Domain:
 		return true
 	}
@@ -106,8 +143,9 @@ var (
 	onlineStatusInvestigationInProgress = abool.NewBool(false)
 	onlineStatusInvestigationWg         sync.WaitGroup
 
-	captivePortal     = &CaptivePortal{}
-	captivePortalLock sync.Mutex
+	captivePortal             = &CaptivePortal{}
+	captivePortalLock         sync.Mutex
+	captivePortalNotification *notifications.Notification
 )
 
 // CaptivePortal holds information about a detected captive portal.
@@ -180,9 +218,27 @@ func setCaptivePortal(portalURL string) {
 	// delete
 	if portalURL == "" {
 		captivePortal = &CaptivePortal{}
+		if captivePortalNotification != nil {
+			err := captivePortalNotification.Delete()
+			if err != nil && err != database.ErrNotFound {
+				log.Warningf("netenv: failed to delete old captive portal notification: %s", err)
+			}
+			captivePortalNotification = nil
+		}
 		return
 	}
 
+	// return if unchanged
+	if portalURL == captivePortal.URL {
+		return
+	}
+
+	// notify
+	defer notifications.NotifyInfo(
+		"netenv:captive-portal:"+captivePortal.Domain,
+		"Portmaster detected a captive portal at "+captivePortal.Domain,
+	)
+
 	// set
 	captivePortal = &CaptivePortal{
 		URL: portalURL,
@@ -378,17 +434,17 @@ func checkOnlineStatus(ctx context.Context) {
 	if err != nil {
 		log.Warningf("network: failed to read http body of captive portal testing response: %s", err)
 		// assume we are online nonetheless
+		// TODO: improve handling this case
 		updateOnlineStatus(StatusOnline, "", "http request succeeded, albeit failing later")
 		return
 	}
 
 	// check body contents
-	if strings.TrimSpace(string(data)) == HTTPExpectedContent {
-		updateOnlineStatus(StatusOnline, "", "http request succeeded")
-	} else {
-		// something is interfering with the website content
-		// this might be a weird captive portal, just direct the user there
+	if strings.TrimSpace(string(data)) != HTTPExpectedContent {
+		// Something is interfering with the website content.
+		// This probably is a captive portal, just direct the user there.
 		updateOnlineStatus(StatusPortal, "detectportal.firefox.com", "http request succeeded, response content not as expected")
+		return
 	}
 	// close the body now as we plan to reuse the http.Client
 	response.Body.Close()

resolver/resolve.go

@@ -180,6 +180,11 @@ func checkCache(ctx context.Context, q *Query) *RRCache {
 
 	// check if expired
 	if rrCache.Expired() {
+		if netenv.IsConnectivityDomain(rrCache.Domain) {
+			// do not use cache, resolve immediately
+			return nil
+		}
+
 		rrCache.Lock()
 		rrCache.requestingNew = true
 		rrCache.Unlock()

resolver/rrcache.go

@@ -7,6 +7,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/safing/portmaster/netenv"
+
 	"github.com/miekg/dns"
 )
 
@@ -75,9 +77,17 @@ func (rrCache *RRCache) Clean(minExpires uint32) {
 		lowestTTL = minExpires
 	}
 
-	// shorten NXDomain caching
-	if len(rrCache.Answer) == 0 {
+	// shorten caching
+	switch {
+	case rrCache.IsNXDomain():
+		// NXDomain
 		lowestTTL = 10
+	case netenv.IsConnectivityDomain(rrCache.Domain):
+		// Responses from these domains might change very quickly depending on the environment.
+		lowestTTL = 3
+	case !netenv.Online():
+		// Not being fully online could mean that we get funny responses.
+		lowestTTL = 60
 	}
 
 	// log.Tracef("lowest TTL is %d", lowestTTL)

status/const.go

@@ -1,6 +1,6 @@
 package status
 
-// Definitions of Security and Status Levels
+// Definitions of Security and Status Levels.
 const (
 	SecurityLevelOff uint8 = 0
 

status/database.go

@@ -1,6 +1,8 @@
 package status
 
 import (
+	"context"
+
 	"github.com/safing/portbase/database"
 	"github.com/safing/portbase/database/query"
 	"github.com/safing/portbase/database/record"
@@ -35,7 +37,10 @@ func (sh *statusHook) PrePut(r record.Record) (record.Record, error) {
 
 	// apply applicable settings
 	if SelectedSecurityLevel() != newStatus.SelectedSecurityLevel {
-		go setSelectedSecurityLevel(newStatus.SelectedSecurityLevel)
+		module.StartWorker("set selected security level", func(_ context.Context) error {
+			setSelectedSecurityLevel(newStatus.SelectedSecurityLevel)
+			return nil
+		})
 	}
 
 	// TODO: allow setting of Gate17 status (on/off)

status/get.go

@@ -7,22 +7,16 @@ import (
 var (
 	activeSecurityLevel   *uint32
 	selectedSecurityLevel *uint32
-	portmasterStatus      *uint32
-	gate17Status          *uint32
 )
 
 func init() {
 	var (
 		activeSecurityLevelValue   uint32
 		selectedSecurityLevelValue uint32
-		portmasterStatusValue      uint32
-		gate17StatusValue          uint32
 	)
 
 	activeSecurityLevel = &activeSecurityLevelValue
 	selectedSecurityLevel = &selectedSecurityLevelValue
-	portmasterStatus = &portmasterStatusValue
-	gate17Status = &gate17StatusValue
 }
 
 // ActiveSecurityLevel returns the current security level.
@@ -34,13 +28,3 @@ func ActiveSecurityLevel() uint8 {
 func SelectedSecurityLevel() uint8 {
 	return uint8(atomic.LoadUint32(selectedSecurityLevel))
 }
-
-// PortmasterStatus returns the current Portmaster status.
-func PortmasterStatus() uint8 {
-	return uint8(atomic.LoadUint32(portmasterStatus))
-}
-
-// Gate17Status returns the current Gate17 status.
-func Gate17Status() uint8 {
-	return uint8(atomic.LoadUint32(gate17Status))
-}

status/get_test.go

@@ -8,8 +8,6 @@ func TestGet(t *testing.T) {
 	// TODO: write real tests
 	ActiveSecurityLevel()
 	SelectedSecurityLevel()
-	PortmasterStatus()
-	Gate17Status()
 	option := ConfigIsActive("invalid")
 	option(0)
 	option = ConfigIsActiveConcurrent("invalid")

status/module.go

@@ -6,21 +6,40 @@ import (
 	"github.com/safing/portbase/modules"
 )
 
+var (
+	module *modules.Module
+)
+
 func init() {
-	modules.Register("status", nil, start, stop, "base")
+	module = modules.Register("status", nil, start, stop, "base")
 }
 
 func start() error {
-	var loadedStatus *SystemStatus
+	err := initSystemStatus()
+	if err != nil {
+		return err
+	}
 
+	err = startNetEnvHooking()
+	if err != nil {
+		return err
+	}
+
+	status.Save()
+
+	return initStatusHook()
+}
+
+func initSystemStatus() error {
 	// load status from database
 	r, err := statusDB.Get(statusDBKey)
 	switch err {
 	case nil:
-		loadedStatus, err = EnsureSystemStatus(r)
+		loadedStatus, err := EnsureSystemStatus(r)
 		if err != nil {
 			log.Criticalf("status: failed to unwrap system status: %s", err)
-			loadedStatus = nil
+		} else {
+			status = loadedStatus
 		}
 	case database.ErrNotFound:
 		// create new status
@@ -28,10 +47,6 @@ func start() error {
 		log.Criticalf("status: failed to load system status: %s", err)
 	}
 
-	// activate loaded status, if available
-	if loadedStatus != nil {
-		status = loadedStatus
-	}
 	status.Lock()
 	defer status.Unlock()
 
@@ -41,10 +56,9 @@ func start() error {
 	// update status
 	status.updateThreatMitigationLevel()
 	status.autopilot()
+	status.updateOnlineStatus()
 
-	go status.Save()
-
-	return initStatusHook()
+	return nil
 }
 
 func stop() error {

status/netenv.go

@@ -0,0 +1,28 @@
+package status
+
+import (
+	"context"
+
+	"github.com/safing/portmaster/netenv"
+)
+
+// startNetEnvHooking starts the listener for online status changes.
+func startNetEnvHooking() error {
+	return module.RegisterEventHook(
+		"netenv",
+		netenv.OnlineStatusChangedEvent,
+		"update online status in system status",
+		func(_ context.Context, _ interface{}) error {
+			status.Lock()
+			status.updateOnlineStatus()
+			status.Unlock()
+			status.Save()
+			return nil
+		},
+	)
+}
+
+func (s *SystemStatus) updateOnlineStatus() {
+	s.OnlineStatus = netenv.GetOnlineStatus()
+	s.CaptivePortal = netenv.GetCaptivePortal()
+}

status/set.go

@@ -6,7 +6,7 @@ import (
 	"github.com/safing/portbase/log"
 )
 
-// autopilot automatically adjusts the security level as needed
+// autopilot automatically adjusts the security level as needed.
 func (s *SystemStatus) autopilot() {
 	// check if users is overruling
 	if s.SelectedSecurityLevel > SecurityLevelOff {
@@ -33,19 +33,18 @@ func setSelectedSecurityLevel(level uint8) {
 	switch level {
 	case SecurityLevelOff, SecurityLevelNormal, SecurityLevelHigh, SecurityLevelExtreme:
 		status.Lock()
-		defer status.Unlock()
 
 		status.SelectedSecurityLevel = level
 		atomicUpdateSelectedSecurityLevel(level)
 		status.autopilot()
 
-		go status.Save()
+		status.Unlock()
+		status.Save()
 	default:
 		log.Errorf("status: tried to set selected security level to invalid value: %d", level)
 	}
 }
 
-// update functions for atomic stuff
 func atomicUpdateActiveSecurityLevel(level uint8) {
 	atomic.StoreUint32(activeSecurityLevel, uint32(level))
 }

status/status.go

@@ -1,9 +1,12 @@
 package status
 
 import (
+	"context"
 	"fmt"
 	"sync"
 
+	"github.com/safing/portmaster/netenv"
+
 	"github.com/safing/portbase/database/record"
 	"github.com/safing/portbase/log"
 )
@@ -28,11 +31,22 @@ type SystemStatus struct {
 	ActiveSecurityLevel   uint8
 	SelectedSecurityLevel uint8
 
+	OnlineStatus  netenv.OnlineStatus
+	CaptivePortal *netenv.CaptivePortal
+
 	ThreatMitigationLevel uint8
 	Threats               map[string]*Threat
 }
 
-// Save saves the SystemStatus to the database
+// SaveAsync saves the SystemStatus to the database asynchronously.
+func (s *SystemStatus) SaveAsync() {
+	module.StartWorker("save system status", func(_ context.Context) error {
+		s.Save()
+		return nil
+	})
+}
+
+// Save saves the SystemStatus to the database.
 func (s *SystemStatus) Save() {
 	err := statusDB.Put(s)
 	if err != nil {

status/threat.go

@@ -26,7 +26,7 @@ func AddOrUpdateThreat(new *Threat) {
 	status.updateThreatMitigationLevel()
 	status.autopilot()
 
-	go status.Save()
+	status.SaveAsync()
 }
 
 // DeleteThreat deletes a threat from the system status.
@@ -38,7 +38,7 @@ func DeleteThreat(id string) {
 	status.updateThreatMitigationLevel()
 	status.autopilot()
 
-	go status.Save()
+	status.SaveAsync()
 }
 
 // GetThreats returns all threats who's IDs are prefixed by the given string, and also a locker for editing them.