Merge pull request #547 from safing/fix/patch-set-14

Fixes and Improvements #14

cmds/portmaster-core/pack

@@ -74,18 +74,27 @@ function check_all {
   GOOS=linux GOARCH=amd64 check
   GOOS=windows GOARCH=amd64 check
   GOOS=darwin GOARCH=amd64 check
+  GOOS=linux GOARCH=arm64 check
+  GOOS=windows GOARCH=arm64 check
+  GOOS=darwin GOARCH=arm64 check
 }
 
 function build_all {
   GOOS=linux GOARCH=amd64 build
   GOOS=windows GOARCH=amd64 build
   GOOS=darwin GOARCH=amd64 build
+  GOOS=linux GOARCH=arm64 build
+  GOOS=windows GOARCH=arm64 build
+  GOOS=darwin GOARCH=arm64 build
 }
 
 function reset_all {
   GOOS=linux GOARCH=amd64 reset
   GOOS=windows GOARCH=amd64 reset
   GOOS=darwin GOARCH=amd64 reset
+  GOOS=linux GOARCH=arm64 reset
+  GOOS=windows GOARCH=arm64 reset
+  GOOS=darwin GOARCH=arm64 reset
 }
 
 case $1 in

cmds/portmaster-start/pack

@@ -74,18 +74,27 @@ function check_all {
   GOOS=linux GOARCH=amd64 check
   GOOS=windows GOARCH=amd64 check
   GOOS=darwin GOARCH=amd64 check
+  GOOS=linux GOARCH=arm64 check
+  GOOS=windows GOARCH=arm64 check
+  GOOS=darwin GOARCH=arm64 check
 }
 
 function build_all {
   GOOS=linux GOARCH=amd64 build
   GOOS=windows GOARCH=amd64 build
   GOOS=darwin GOARCH=amd64 build
+  GOOS=linux GOARCH=arm64 build
+  GOOS=windows GOARCH=arm64 build
+  GOOS=darwin GOARCH=arm64 build
 }
 
 function reset_all {
   GOOS=linux GOARCH=amd64 reset
   GOOS=windows GOARCH=amd64 reset
   GOOS=darwin GOARCH=amd64 reset
+  GOOS=linux GOARCH=arm64 reset
+  GOOS=windows GOARCH=arm64 reset
+  GOOS=darwin GOARCH=arm64 reset
 }
 
 case $1 in

compat/notify.go

@@ -42,9 +42,10 @@ var (
 	}
 
 	secureDNSBypassIssue = &appIssue{
-		id:      "compat:secure-dns-bypass-%s",
+		id:    "compat:secure-dns-bypass-%s",
-		title:   "Detected %s Bypass Attempt",
+		title: "Detected %s Bypass Attempt",
-		message: "Portmaster detected that %s is trying to use a secure DNS resolver. While this is a good thing, the Portmaster already handles secure DNS for your whole device. Please disable the secure DNS resolver within the app.",
+		message: `[APPNAME] is bypassing Portmaster's firewall functions through its Secure DNS resolver. Portmaster can no longer protect or filter connections coming from [APPNAME]. Disable Secure DNS within [APPNAME] to restore functionality.  
+Rest assured that Portmaster already handles Secure DNS for your whole device.`,
 		// TODO: Add this when the new docs page is finished:
 		// , or [find out about other options](link to new docs page)
 		level: notifications.Warning,
@@ -52,7 +53,7 @@ var (
 	multiPeerUDPTunnelIssue = &appIssue{
 		id:      "compat:multi-peer-udp-tunnel-%s",
 		title:   "Detected SPN Incompatibility in %s",
-		message: "Portmaster detected that %s is trying to connect to multiple servers via the SPN using a single UDP connection. This is common for technologies such as torrents. Unfortunately, the SPN does not support this feature currently. You can try to change this behavior within the affected app or you could exempt it from using the SPN.",
+		message: "Portmaster detected that [APPNAME] is trying to connect to multiple servers via the SPN using a single UDP connection. This is common for technologies such as torrents. Unfortunately, the SPN does not support this feature currently. You can try to change this behavior within the affected app or you could exempt it from using the SPN.",
 		level:   notifications.Warning,
 	}
 )
@@ -123,6 +124,9 @@ func (issue *appIssue) notify(proc *process.Process) {
 		proc.Path,
 	)
 
+	// Build message.
+	message := strings.ReplaceAll(issue.message, "[APPNAME]", p.Name)
+
 	// Check if we already have this notification.
 	eventID := fmt.Sprintf(issue.id, p.ID)
 	n := notifications.Get(eventID)
@@ -135,7 +139,7 @@ func (issue *appIssue) notify(proc *process.Process) {
 		EventID:      eventID,
 		Type:         issue.level,
 		Title:        fmt.Sprintf(issue.title, p.Name),
-		Message:      fmt.Sprintf(issue.message, p.Name),
+		Message:      message,
 		ShowOnSystem: true,
 		AvailableActions: []*notifications.Action{
 			{
@@ -148,14 +152,22 @@ func (issue *appIssue) notify(proc *process.Process) {
 
 	// Set warning on profile.
 	module.StartWorker("set app compat warning", func(ctx context.Context) error {
+		var changed bool
+
 		func() {
 			p.Lock()
 			defer p.Unlock()
 
-			p.Warning = fmt.Sprintf(issue.message, p.Name)
+			if p.Warning != message || time.Now().Add(-1*time.Hour).After(p.WarningLastUpdated) {
-			p.WarningLastUpdated = time.Now()
+				p.Warning = message
+				p.WarningLastUpdated = time.Now()
+				changed = true
+			}
 		}()
 
-		return p.Save()
+		if changed {
+			return p.Save()
+		}
+		return nil
 	})
 }

firewall/bypassing.go

@@ -16,24 +16,44 @@ var resolverFilterLists = []string{"17-DNS"}
 // PreventBypassing checks if the connection should be denied or permitted
 // based on some bypass protection checks.
 func PreventBypassing(ctx context.Context, conn *network.Connection) (endpoints.EPResult, string, nsutil.Responder) {
+	// Exclude incoming connections.
+	if conn.Inbound {
+		return endpoints.NoMatch, "", nil
+	}
+
+	// Exclude ICMP.
+	switch packet.IPProtocol(conn.Entity.Protocol) { //nolint:exhaustive // Checking for specific values only.
+	case packet.ICMP, packet.ICMPv6:
+		return endpoints.NoMatch, "", nil
+	}
+
 	// Block firefox canary domain to disable DoH.
+	// This MUST also affect the System Resolver, because the return value must
+	// be correct for this to work.
 	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()
 	}
 
-	// Block direct connections to known DNS resolvers.
+	// Exclude DNS requests coming from the System Resolver.
-	switch packet.IPProtocol(conn.Entity.Protocol) { //nolint:exhaustive // Checking for specific values only.
+	// This MUST also affect entities in the secure dns filter list, else the
-	case packet.ICMP, packet.ICMPv6:
+	// System Resolver is wrongly accused of bypassing.
-		// Make an exception for ICMP, as these IPs are also often used for debugging.
+	if conn.Type == network.DNSRequest && conn.Process().IsSystemResolver() {
-	default:
+		return endpoints.NoMatch, "", nil
-		if conn.Entity.MatchLists(resolverFilterLists) {
+	}
-			compat.ReportSecureDNSBypassIssue(conn.Process())
+
-			return endpoints.Denied,
+	// Block bypass attempts using an encrypted DNS server.
-				"blocked rogue connection to DNS resolver",
+	switch {
-				nsutil.BlockIP()
+	case conn.Entity.Port == 853:
-		}
+		// Block connections to port 853 - DNS over TLS.
+		fallthrough
+	case conn.Entity.MatchLists(resolverFilterLists):
+		// Block connection entities in the secure dns filter list.
+		compat.ReportSecureDNSBypassIssue(conn.Process())
+		return endpoints.Denied,
+			"blocked rogue connection to DNS resolver",
+			nsutil.BlockIP()
 	}
 
 	return endpoints.NoMatch, "", nil

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/mdlayher/socket v0.2.0 // indirect
 	github.com/miekg/dns v1.1.46
 	github.com/oschwald/maxminddb-golang v1.8.0
-	github.com/safing/portbase v0.13.6
+	github.com/safing/portbase v0.14.0
 	github.com/safing/spn v0.4.2
 	github.com/shirou/gopsutil v3.21.11+incompatible
 	github.com/spf13/cobra v1.3.0

go.sum

@@ -810,6 +810,8 @@ github.com/safing/portbase v0.13.5 h1:WtDvnTh8reBMnhPiyAr62qkBeBUri0EaNlb0u2msNN
 github.com/safing/portbase v0.13.5/go.mod h1:5vj5IK5WJoSGareDe6yCMZfnF7txVRx7jZyTZInISP0=
 github.com/safing/portbase v0.13.6 h1:0tAa4fyCdlZy4J+Ne9G5JshV+Cmu+Ol0m5AftPHnjvE=
 github.com/safing/portbase v0.13.6/go.mod h1:G0maDSQxYDuluNhMzA1zVd/nfXawfECv5H7+fnTfVhM=
+github.com/safing/portbase v0.14.0 h1:6+sdUs1tdRCKnyuzy/zHrvUsdO1GdI0l4gZaoYJmJ5Q=
+github.com/safing/portbase v0.14.0/go.mod h1:z9sRR/vqohAGdYSSx2B+o8tND4WVvcxPL6XBBtN3bDI=
 github.com/safing/portmaster v0.7.3/go.mod h1:o//kZ8eE+5vT1V22mgnxHIAdlEz42sArsK5OF2Lf/+s=
 github.com/safing/portmaster v0.7.4/go.mod h1:Q93BWdF1oAL0oUMukshl8W1aPZhmrlTGi6tFTFc3pTw=
 github.com/safing/portmaster v0.7.6/go.mod h1:qOs9hQtvAzTVICRbwLg3vddqOaqJHeWBjWQ0C+TJ/Bw=

nameserver/nameserver.go

@@ -201,6 +201,12 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg)
 				return
 			}
 
+			// Mark successful queries as internal in order to hide them in the simple interface.
+			// These requests were most probably made for another process and only add confusion if listed.
+			if conn.Process().IsSystemResolver() {
+				conn.Internal = true
+			}
+
 			// Save the request as open, as we don't know if there will be a connection or not.
 			network.SaveOpenDNSRequest(q, rrCache, conn)
 			firewall.UpdateIPsAndCNAMEs(q, rrCache, conn)
@@ -244,27 +250,35 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg)
 	rrCache, err = resolver.Resolve(ctx, q)
 	// Handle error.
 	if err != nil {
-		conn.Failed(fmt.Sprintf("query failed: %s", err), "")
 		switch {
 		case errors.Is(err, resolver.ErrNotFound):
 			tracer.Tracef("nameserver: %s", err)
+			conn.Failed("domain does not exist", "")
 			return reply(nsutil.NxDomain("nxdomain: " + err.Error()))
+
 		case errors.Is(err, resolver.ErrBlocked):
 			tracer.Tracef("nameserver: %s", err)
+			conn.Block(err.Error(), "")
 			return reply(nsutil.BlockIP("blocked: " + err.Error()))
+
 		case errors.Is(err, resolver.ErrLocalhost):
 			tracer.Tracef("nameserver: returning localhost records")
+			conn.Accept("allowing query for localhost", "")
 			return reply(nsutil.Localhost())
+
 		case errors.Is(err, resolver.ErrOffline):
 			if rrCache == nil {
 				log.Tracer(ctx).Debugf("nameserver: not resolving %s, device is offline", q.ID())
+				conn.Failed("not resolving, device is offline", "")
 				return reply(nsutil.ServerFailure(err.Error()))
 			}
-			// If an rrCache was returned, it's usable a backup.
+			// If an rrCache was returned, it's usable as a backup.
 			rrCache.IsBackup = true
 			log.Tracer(ctx).Debugf("nameserver: device is offline, using backup cache for %s", q.ID())
+
 		default:
 			tracer.Warningf("nameserver: failed to resolve %s: %s", q.ID(), err)
+			conn.Failed(fmt.Sprintf("query failed: %s", err), "")
 			addFailingQuery(q, err)
 			return reply(nsutil.ServerFailure("internal error: " + err.Error()))
 		}

network/connection.go

@@ -269,11 +269,6 @@ func NewConnectionFromDNSRequest(ctx context.Context, fqdn string, cnames []stri
 		dnsConn.Internal = localProfile.Internal
 	}
 
-	// Always mark dns queries from the system resolver as internal.
-	if proc.IsSystemResolver() {
-		dnsConn.Internal = true
-	}
-
 	// DNS Requests are saved by the nameserver depending on the result of the
 	// query. Blocked requests are saved immediately, accepted ones are only
 	// saved if they are not "used" by a connection.

profile/special.go

@@ -34,7 +34,6 @@ Seeing a lot of incoming connections here is normal, as this resembles the netwo
 In order to respect the app settings of the actual application, DNS requests from the System DNS Client are only subject to the following settings:
 
 - Outgoing Rules (without global rules)
-- Block Bypassing
 - Filter Lists
 
 If you think you might have messed up the settings of the System DNS Client, just delete the profile below to reset it to the defaults.

resolver/config.go

@@ -1,6 +1,7 @@
 package resolver
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -109,6 +110,7 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 		ReleaseLevel:    config.ReleaseLevelStable,
 		DefaultValue:    defaultNameServers,
 		ValidationRegex: fmt.Sprintf("^(%s|%s|%s)://.*", ServerTypeDoT, ServerTypeDNS, ServerTypeTCP),
+		ValidationFunc:  validateNameservers,
 		Annotations: config.Annotations{
 			config.DisplayHintAnnotation:  config.DisplayHintOrdered,
 			config.DisplayOrderAnnotation: cfgOptionNameServersOrder,
@@ -265,6 +267,22 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 	return nil
 }
 
+func validateNameservers(value interface{}) error {
+	list, ok := value.([]string)
+	if !ok {
+		return errors.New("invalid type")
+	}
+
+	for i, entry := range list {
+		_, _, err := createResolver(entry, ServerSourceConfigured)
+		if err != nil {
+			return fmt.Errorf("failed to parse DNS server \"%s\" (#%d): %w", entry, i+1, err)
+		}
+	}
+
+	return nil
+}
+
 func formatScopeList(list []string) string {
 	formatted := make([]string, 0, len(list))
 	for _, domain := range list {