From 1346123d6f28da1c378878f2eacd443825ca9648 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:30:02 +0100 Subject: [PATCH 1/9] Fix bypass detection to correctly attribute encrypted DNS bypassing --- firewall/bypassing.go | 42 +++++++++++++++++++++++++++++++----------- profile/special.go | 1 - 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/firewall/bypassing.go b/firewall/bypassing.go index 62d324c3..4f6b0f1d 100644 --- a/firewall/bypassing.go +++ b/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. - switch packet.IPProtocol(conn.Entity.Protocol) { //nolint:exhaustive // Checking for specific values only. - case packet.ICMP, packet.ICMPv6: - // Make an exception for ICMP, as these IPs are also often used for debugging. - default: - if conn.Entity.MatchLists(resolverFilterLists) { - compat.ReportSecureDNSBypassIssue(conn.Process()) - return endpoints.Denied, - "blocked rogue connection to DNS resolver", - nsutil.BlockIP() - } + // Exclude DNS requests coming from the System Resolver. + // This MUST also affect entities in the secure dns filter list, else the + // System Resolver is wrongly accused of bypassing. + if conn.Type == network.DNSRequest && conn.Process().IsSystemResolver() { + return endpoints.NoMatch, "", nil + } + + // Block bypass attempts using an encrypted DNS server. + switch { + 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 diff --git a/profile/special.go b/profile/special.go index 13e9b5a9..58900afd 100644 --- a/profile/special.go +++ b/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. From 6cbe33ae70f0928ae9344afd4a570c6bc7dd4541 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:35:21 +0100 Subject: [PATCH 2/9] Only hide successful queries of the system resolver --- nameserver/nameserver.go | 6 ++++++ network/connection.go | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index f2aea112..e63e49b4 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -201,6 +201,12 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) return } + // Mark successfull 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) diff --git a/network/connection.go b/network/connection.go index 6e8def4e..b1e0a4af 100644 --- a/network/connection.go +++ b/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. From 1dde1437b699e536004a49ea58adf666297defdc Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:36:53 +0100 Subject: [PATCH 3/9] Fix verdicts of DNS request connections --- nameserver/nameserver.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index e63e49b4..9cb8e7c7 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -250,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())) } From 1462bd07ff4227c39d401ec07e518af748a96041 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:37:54 +0100 Subject: [PATCH 4/9] Stop useless updating of app compat warning --- compat/notify.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/compat/notify.go b/compat/notify.go index dea8b394..ea9a9c16 100644 --- a/compat/notify.go +++ b/compat/notify.go @@ -148,14 +148,23 @@ 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) - p.WarningLastUpdated = time.Now() + warningMsg := fmt.Sprintf(issue.message, p.Name) + if p.Warning != warningMsg || time.Now().Add(-1*time.Hour).After(p.WarningLastUpdated) { + p.Warning = warningMsg + p.WarningLastUpdated = time.Now() + changed = true + } }() - return p.Save() + if changed { + return p.Save() + } + return nil }) } From 7894bcfcdc99feddacbc83df7eb96e38e9fc8470 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:50:37 +0100 Subject: [PATCH 5/9] Add validation for DNS server config --- resolver/config.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/resolver/config.go b/resolver/config.go index 069ce0dd..a69c9419 100644 --- a/resolver/config.go +++ b/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¶meter=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¶meter=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 { From 5b59b52db96d8390b9adbd7fbbf4b5f59983ba88 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Feb 2022 15:51:03 +0100 Subject: [PATCH 6/9] Enable build for arm64 platform --- cmds/portmaster-core/pack | 9 +++++++++ cmds/portmaster-start/pack | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/cmds/portmaster-core/pack b/cmds/portmaster-core/pack index d8a41f30..b2f6f217 100755 --- a/cmds/portmaster-core/pack +++ b/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 diff --git a/cmds/portmaster-start/pack b/cmds/portmaster-start/pack index f6eb0ee4..1cf6ca2a 100755 --- a/cmds/portmaster-start/pack +++ b/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 From ee433c9fa435bc7336c9d8efa8f45dd8592db6af Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 1 Mar 2022 16:11:16 +0100 Subject: [PATCH 7/9] Improve secure DNS bypass notification --- compat/notify.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/compat/notify.go b/compat/notify.go index ea9a9c16..349ecd75 100644 --- a/compat/notify.go +++ b/compat/notify.go @@ -42,9 +42,10 @@ var ( } secureDNSBypassIssue = &appIssue{ - id: "compat:secure-dns-bypass-%s", - 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.", + id: "compat:secure-dns-bypass-%s", + title: "Detected %s Bypass Attempt", + message: `%s is bypassing Portmaster's firewall functions through its Secure DNS resolver. Portmaster can no longer protect or filter connections coming from %s. Disable Secure DNS within %s 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, @@ -123,6 +124,13 @@ func (issue *appIssue) notify(proc *process.Process) { proc.Path, ) + // Build message. + messageAppNameReplaces := make([]interface{}, strings.Count(issue.message, "%s")) + for i := range messageAppNameReplaces { + messageAppNameReplaces[i] = p.Name + } + message := fmt.Sprintf(issue.message, messageAppNameReplaces...) + // Check if we already have this notification. eventID := fmt.Sprintf(issue.id, p.ID) n := notifications.Get(eventID) @@ -135,7 +143,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{ { @@ -154,9 +162,8 @@ func (issue *appIssue) notify(proc *process.Process) { p.Lock() defer p.Unlock() - warningMsg := fmt.Sprintf(issue.message, p.Name) - if p.Warning != warningMsg || time.Now().Add(-1*time.Hour).After(p.WarningLastUpdated) { - p.Warning = warningMsg + if p.Warning != message || time.Now().Add(-1*time.Hour).After(p.WarningLastUpdated) { + p.Warning = message p.WarningLastUpdated = time.Now() changed = true } From fc1635d5ae333464dd86f305b8ff1337b7b002f3 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 1 Mar 2022 16:13:58 +0100 Subject: [PATCH 8/9] Update portbase to v0.14.0 --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 2d0e6638..8317643a 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 5739830f..4bbdeeab 100644 --- a/go.sum +++ b/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= From 141a95702c2b6972515f3f5fd5e088675eb620bf Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 1 Mar 2022 16:31:33 +0100 Subject: [PATCH 9/9] Simplify secure DNS bypass message creation --- compat/notify.go | 10 +++------- nameserver/nameserver.go | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/compat/notify.go b/compat/notify.go index 349ecd75..cc317d21 100644 --- a/compat/notify.go +++ b/compat/notify.go @@ -44,7 +44,7 @@ var ( secureDNSBypassIssue = &appIssue{ id: "compat:secure-dns-bypass-%s", title: "Detected %s Bypass Attempt", - message: `%s is bypassing Portmaster's firewall functions through its Secure DNS resolver. Portmaster can no longer protect or filter connections coming from %s. Disable Secure DNS within %s to restore functionality. + 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) @@ -53,7 +53,7 @@ Rest assured that Portmaster already handles Secure DNS for your whole device.`, 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, } ) @@ -125,11 +125,7 @@ func (issue *appIssue) notify(proc *process.Process) { ) // Build message. - messageAppNameReplaces := make([]interface{}, strings.Count(issue.message, "%s")) - for i := range messageAppNameReplaces { - messageAppNameReplaces[i] = p.Name - } - message := fmt.Sprintf(issue.message, messageAppNameReplaces...) + message := strings.ReplaceAll(issue.message, "[APPNAME]", p.Name) // Check if we already have this notification. eventID := fmt.Sprintf(issue.id, p.ID) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 9cb8e7c7..4c5bf114 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -201,7 +201,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) return } - // Mark successfull queries as internal in order to hide them in the simple interface. + // 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