From ea1b189330d1ed4749f4044bc646681beb91720f Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 20 May 2022 16:35:46 +0200 Subject: [PATCH 1/8] Reset system self-check after network change --- compat/module.go | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/compat/module.go b/compat/module.go index d1a27628..77478f7e 100644 --- a/compat/module.go +++ b/compat/module.go @@ -26,6 +26,10 @@ var ( // selfcheckFails counts how often the self check failed successively. // selfcheckFails is not locked as it is only accessed by the self-check task. selfcheckFails int + + // selfcheckNetworkChangedFlag is used to track changed to the network for + // the self-check. + selfcheckNetworkChangedFlag = netenv.GetNetworkChangedFlag() ) // selfcheckFailThreshold holds the threshold of how many times the selfcheck @@ -49,6 +53,7 @@ func prep() error { func start() error { startNotify() + selfcheckNetworkChangedFlag.Refresh() selfcheckTask = module.NewTask("compatibility self-check", selfcheckTaskFunc). Repeat(5 * time.Minute). MaxDelay(selfcheckTaskRetryAfter). @@ -83,19 +88,23 @@ func selfcheckTaskFunc(ctx context.Context, task *modules.Task) error { // Run selfcheck and return if successful. issue, err := selfcheck(ctx) - if err == nil { - selfCheckIsFailing.UnSet() - selfcheckFails = 0 - resetSystemIssue() + switch { + case err == nil: + // Successful. tracer.Debugf("compat: self-check successful") - return nil - } + case issue == nil: + // Internal error. + tracer.Warningf("compat: %s", err) + case selfcheckNetworkChangedFlag.IsSet(): + // The network changed, ignore the issue. + default: + // The self-check failed. - // Log result. - if issue != nil { + // Set state and increase counter. selfCheckIsFailing.Set() selfcheckFails++ + // Log and notify. tracer.Errorf("compat: %s", err) if selfcheckFails >= selfcheckFailThreshold { issue.notify(err) @@ -103,14 +112,16 @@ func selfcheckTaskFunc(ctx context.Context, task *modules.Task) error { // Retry quicker when failed. task.Schedule(time.Now().Add(selfcheckTaskRetryAfter)) - } else { - selfCheckIsFailing.UnSet() - selfcheckFails = 0 - // Only log internal errors, but don't notify. - tracer.Warningf("compat: %s", err) + return nil } + // Reset self-check state. + selfcheckNetworkChangedFlag.Refresh() + selfCheckIsFailing.UnSet() + selfcheckFails = 0 + resetSystemIssue() + return nil } From e178b732bca2a3daa0f5ab67e8e01a0500ccd4e6 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 20 May 2022 16:37:19 +0200 Subject: [PATCH 2/8] Calm down and relax dns query check --- firewall/tunnel.go | 2 +- netenv/online-status.go | 76 +++++++++++++++++++---------------------- resolver/resolve.go | 48 ++++++++++++++++++++++++-- resolver/resolver.go | 12 ++++++- 4 files changed, 94 insertions(+), 44 deletions(-) diff --git a/firewall/tunnel.go b/firewall/tunnel.go index 32d60de1..08adff33 100644 --- a/firewall/tunnel.go +++ b/firewall/tunnel.go @@ -34,7 +34,7 @@ func checkTunneling(ctx context.Context, conn *network.Connection, pkt packet.Pa case conn.Process().Pid == ownPID: // Bypass tunneling for certain own connections. switch { - case captain.ClientBootstrapping(): + case !captain.ClientReady(): return case captain.IsExcepted(conn.Entity.IP): return diff --git a/netenv/online-status.go b/netenv/online-status.go index a7589a13..0ef612fd 100644 --- a/netenv/online-status.go +++ b/netenv/online-status.go @@ -37,13 +37,12 @@ var ( PortalTestIP = net.IPv4(192, 0, 2, 1) PortalTestURL = fmt.Sprintf("http://%s/", PortalTestIP) - DNSTestDomain = "one.one.one.one." - DNSTestExpectedIP = net.IPv4(1, 1, 1, 1) - - DNSFallbackTestDomain = "dns-check.safing.io." - DNSFallbackTestExpectedIP = net.IPv4(0, 65, 67, 75) // Ascii: \0ACK + DNSTestDomain = "online-check.safing.io." + DNSTestExpectedIP = net.IPv4(0, 65, 67, 75) // Ascii: \0ACK + DNSTestQueryFunc func(ctx context.Context, fdqn string) (ips []net.IP, ok bool, err error) ConnectedToSPN = abool.New() + ConnectedToDNS = abool.New() // SpecialCaptivePortalDomain is the domain name used to point to the detected captive portal IP // or the captive portal test IP. The default value should be overridden by the resolver package, @@ -53,8 +52,6 @@ var ( // ConnectivityDomains holds all connectivity domains. This slice must not be modified. ConnectivityDomains = []string{ SpecialCaptivePortalDomain, - DNSTestDomain, // Internal DNS Check - DNSFallbackTestDomain, // Internal DNS Check // Windows "dns.msftncsi.com.", // DNS Check @@ -380,20 +377,20 @@ func monitorOnlineStatus(ctx context.Context) error { func getDynamicStatusTrigger() <-chan time.Time { switch GetOnlineStatus() { case StatusOffline: - // Will be triggered by network change anyway. - return time.After(20 * time.Second) + // Will also be triggered by network change. + return time.After(10 * time.Second) case StatusLimited, StatusPortal: // Change will not be detected otherwise, but impact is minor. return time.After(5 * time.Second) case StatusSemiOnline: // Very small impact. - return time.After(20 * time.Second) + return time.After(60 * time.Second) case StatusOnline: // Don't check until resolver reports problems. return nil case StatusUnknown: - return time.After(5 * time.Second) - default: // other unknown status + fallthrough + default: return time.After(5 * time.Minute) } } @@ -407,13 +404,18 @@ func checkOnlineStatus(ctx context.Context) { return StatusUnknown }*/ - // 0) check if connected to SPN + // 0) check if connected to SPN and/or DNS. if ConnectedToSPN.IsSet() { updateOnlineStatus(StatusOnline, nil, "connected to SPN") return } + if ConnectedToDNS.IsSet() { + updateOnlineStatus(StatusOnline, nil, "connected to DNS") + return + } + // 1) check for addresses ipv4, ipv6, err := GetAssignedAddresses() @@ -508,34 +510,28 @@ func checkOnlineStatus(ctx context.Context) { // 3) resolve a query - // Check with primary dns check domain. - ips, err := net.LookupIP(DNSTestDomain) - if err != nil { - log.Warningf("netenv: dns check query failed: %s", err) - } else { - // check for expected response - for _, ip := range ips { - if ip.Equal(DNSTestExpectedIP) { - updateOnlineStatus(StatusOnline, nil, "all checks passed") - return - } - } - } - - // If that did not work, check with fallback dns check domain. - ips, err = net.LookupIP(DNSFallbackTestDomain) - if err != nil { - log.Warningf("netenv: dns fallback check query failed: %s", err) - updateOnlineStatus(StatusLimited, nil, "dns fallback check query failed") + // Check if we can resolve the dns check domain. + if DNSTestQueryFunc == nil { + updateOnlineStatus(StatusOnline, nil, "all checks passed, dns query check disabled") return } - // check for expected response - for _, ip := range ips { - if ip.Equal(DNSFallbackTestExpectedIP) { - updateOnlineStatus(StatusOnline, nil, "all checks passed") - return - } + ips, ok, err := DNSTestQueryFunc(ctx, DNSTestDomain) + switch { + case ok && err != nil: + updateOnlineStatus(StatusOnline, nil, fmt.Sprintf( + "all checks passed, acceptable result for dns query check: %s", + err, + )) + case ok && len(ips) >= 1 && ips[0].Equal(DNSTestExpectedIP): + updateOnlineStatus(StatusOnline, nil, "all checks passed") + case ok && len(ips) >= 1: + log.Warningf("netenv: dns query check response mismatched: got %s", ips[0]) + updateOnlineStatus(StatusOnline, nil, "all checks passed, dns query check response mismatched") + case ok: + log.Warningf("netenv: dns query check response mismatched: empty response") + updateOnlineStatus(StatusOnline, nil, "all checks passed, dns query check response was empty") + default: + log.Warningf("netenv: dns query check failed: %s", err) + updateOnlineStatus(StatusOffline, nil, "dns query check failed") } - // unexpected response - updateOnlineStatus(StatusSemiOnline, nil, "dns check query response mismatched") } diff --git a/resolver/resolve.go b/resolver/resolve.go index 8663a32e..1b0b429e 100644 --- a/resolver/resolve.go +++ b/resolver/resolve.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net" "sync" "time" @@ -318,8 +319,8 @@ func resolveAndCache(ctx context.Context, q *Query, oldCache *RRCache) (rrCache } // check if we are online - if primarySource != ServerSourceEnv && netenv.GetOnlineStatus() == netenv.StatusOffline { - if !netenv.IsConnectivityDomain(q.FQDN) { + if netenv.GetOnlineStatus() == netenv.StatusOffline && primarySource != ServerSourceEnv { + if q.FQDN != netenv.DNSTestDomain && !netenv.IsConnectivityDomain(q.FQDN) { // we are offline and this is not an online check query return oldCache, ErrOffline } @@ -358,6 +359,7 @@ resolveLoop: // some resolvers might also block return nil, err case netenv.GetOnlineStatus() == netenv.StatusOffline && + q.FQDN != netenv.DNSTestDomain && !netenv.IsConnectivityDomain(q.FQDN): // we are offline and this is not an online check query return oldCache, ErrOffline @@ -478,3 +480,45 @@ func shouldResetCache(q *Query) (reset bool) { return false } + +func init() { + netenv.DNSTestQueryFunc = testConnectivity +} + +// testConnectivity test if resolving a query succeeds and returns whether the +// query itself succeeded, separate from interpreting the result. +func testConnectivity(ctx context.Context, fdqn string) (ips []net.IP, ok bool, err error) { + q := &Query{ + FQDN: fdqn, + QType: dns.Type(dns.TypeA), + NoCaching: true, + } + if !q.check() { + return nil, false, ErrInvalid + } + + rrCache, err := resolveAndCache(ctx, q, nil) + switch { + case err == nil: + switch rrCache.RCode { + case dns.RcodeNameError: + return nil, true, ErrNotFound + case dns.RcodeRefused: + return nil, true, errors.New("refused") + default: + ips := rrCache.ExportAllARecords() + if len(ips) > 0 { + return ips, true, nil + } + return nil, true, ErrNotFound + } + case errors.Is(err, ErrNotFound): + return nil, true, err + case errors.Is(err, ErrBlocked): + return nil, true, err + case errors.Is(err, ErrNoCompliance): + return nil, true, err + default: + return nil, false, err + } +} diff --git a/resolver/resolver.go b/resolver/resolver.go index 56d4b6cc..1f12c8f2 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -206,8 +206,8 @@ func (brc *BasicResolverConn) init() { // ReportFailure reports that an error occurred with this resolver. func (brc *BasicResolverConn) ReportFailure() { + // Don't mark resolver as failed if we are offline. if !netenv.Online() { - // don't mark failed if we are offline return } @@ -224,6 +224,11 @@ func (brc *BasicResolverConn) ReportFailure() { // the fail. brc.networkChangedFlag.Refresh() } + + // Report to netenv that a configured server failed. + if brc.resolver.Info.Source == ServerSourceConfigured { + netenv.ConnectedToDNS.UnSet() + } } // IsFailing returns if this resolver is currently failing. @@ -255,4 +260,9 @@ func (brc *BasicResolverConn) ResetFailure() { defer brc.failLock.Unlock() brc.fails = 0 } + + // Report to netenv that a configured server succeeded. + if brc.resolver.Info.Source == ServerSourceConfigured { + netenv.ConnectedToDNS.Set() + } } From 49e79fe3fdd3b5f3a03a3a36e398d8604b9922bc Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 24 May 2022 11:18:23 +0200 Subject: [PATCH 3/8] Detect responses to multi/broadcast queries --- firewall/master.go | 43 ++++++++++++++++++++++++++++++++++++++++++ firewall/tunnel.go | 4 ++-- netenv/addresses.go | 14 ++++++-------- network/multicast.go | 43 ++++++++++++++++++++++++++++++++++++++++++ network/netutils/ip.go | 31 ++++++++++++++++++++++++++++++ process/process.go | 26 +++++++++++++++++++++++++ 6 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 network/multicast.go diff --git a/firewall/master.go b/firewall/master.go index 16dc2b5f..4be457aa 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -3,7 +3,9 @@ package firewall import ( "context" "fmt" + "net" "path/filepath" + "strconv" "strings" "github.com/agext/levenshtein" @@ -41,6 +43,7 @@ type deciderFn func(context.Context, *network.Connection, *profile.LayeredProfil var defaultDeciders = []deciderFn{ checkPortmasterConnection, checkSelfCommunication, + checkIfBroadcastReply, checkConnectionType, checkConnectionScope, checkEndpointLists, @@ -182,6 +185,46 @@ func checkSelfCommunication(ctx context.Context, conn *network.Connection, _ *pr return false } +func checkIfBroadcastReply(ctx context.Context, conn *network.Connection, _ *profile.LayeredProfile, _ packet.Packet) bool { + // Only check inbound connections. + if !conn.Inbound { + return false + } + // Only check if the process has been identified. + if !conn.Process().IsIdentified() { + return false + } + + // Check if the remote IP is part of a local network. + localNet, err := netenv.GetLocalNetwork(conn.Entity.IP) + if err != nil { + log.Tracer(ctx).Warningf("filter: failed to get local network: %s", err) + return false + } + if localNet == nil { + return false + } + + // Search for a matching requesting connection. + requestingConn := network.GetMulticastRequestConn(conn, localNet) + if requestingConn == nil { + return false + } + + conn.Accept( + fmt.Sprintf( + "response to multi/broadcast query to %s/%s", + packet.IPProtocol(requestingConn.Entity.Protocol), + net.JoinHostPort( + requestingConn.Entity.IP.String(), + strconv.Itoa(int(requestingConn.Entity.Port)), + ), + ), + "", + ) + return true +} + func checkEndpointLists(ctx context.Context, conn *network.Connection, p *profile.LayeredProfile, _ packet.Packet) bool { // DNS request from the system resolver require a special decision process, // because the original requesting process is not known. Here, we only check diff --git a/firewall/tunnel.go b/firewall/tunnel.go index 08adff33..28f0e37d 100644 --- a/firewall/tunnel.go +++ b/firewall/tunnel.go @@ -42,10 +42,10 @@ func checkTunneling(ctx context.Context, conn *network.Connection, pkt packet.Pa } // Check more extensively for Local/LAN connections. - myNet, err := netenv.IsMyNet(conn.Entity.IP) + localNet, err := netenv.GetLocalNetwork(conn.Entity.IP) if err != nil { log.Warningf("firewall: failed to check if %s is in my net: %s", conn.Entity.IP, err) - } else if myNet { + } else if localNet != nil { // With IPv6, just checking the IP scope is not enough, as the host very // likely has a public IPv6 address. // Don't tunnel LAN connections. diff --git a/netenv/addresses.go b/netenv/addresses.go index 14762c78..67c4c999 100644 --- a/netenv/addresses.go +++ b/netenv/addresses.go @@ -152,11 +152,9 @@ func IsMyIP(ip net.IP) (yes bool, err error) { return false, nil } -// IsMyNet returns whether the given IP is currently in the host's broadcast -// domain - ie. the networks that the host is directly attached to. -// Function is optimized with the assumption that is unlikely that the IP is -// in the broadcast domain. -func IsMyNet(ip net.IP) (yes bool, err error) { +// GetLocalNetwork uses the given IP to search for a network configured on the +// device and returns it. +func GetLocalNetwork(ip net.IP) (myNet *net.IPNet, err error) { myNetworksLock.Lock() defer myNetworksLock.Unlock() @@ -164,16 +162,16 @@ func IsMyNet(ip net.IP) (yes bool, err error) { if myNetworksNetworkChangedFlag.IsSet() { err := refreshMyNetworks() if err != nil { - return false, err + return nil, err } } // Check if the IP address is in my networks. for _, myNet := range myNetworks { if myNet.Contains(ip) { - return true, nil + return myNet, nil } } - return false, nil + return nil, nil } diff --git a/network/multicast.go b/network/multicast.go new file mode 100644 index 00000000..82cb2173 --- /dev/null +++ b/network/multicast.go @@ -0,0 +1,43 @@ +package network + +import ( + "net" + + "github.com/safing/portmaster/network/netutils" +) + +// GetMulticastRequestConn searches for and returns the requesting connnection +// of a possible multicast/broadcast response. +func GetMulticastRequestConn(responseConn *Connection, responseFromNet *net.IPNet) *Connection { + // Calculate the broadcast address the query would have gone to. + responseNetBroadcastIP := netutils.GetBroadcastAddress(responseFromNet.IP, responseFromNet.Mask) + + // Find requesting multicast/broadcast connection. + for _, conn := range conns.clone() { + switch { + case conn.Inbound: + // Ignore incoming connections. + //case conn.Ended != 0: + // Ignore ended connections. + case conn.Entity.Protocol != responseConn.Entity.Protocol: + // Ignore on protocol mismatch. + case conn.LocalPort != responseConn.LocalPort: + // Ignore on local port mismatch. + case !conn.LocalIP.Equal(responseConn.LocalIP): + // Ignore on local IP mismatch. + case !conn.Process().Equal(responseConn.Process()): + // Ignore if processes mismatch. + case conn.Entity.IPScope == netutils.LocalMulticast && + (responseConn.Entity.IPScope == netutils.LinkLocal || + responseConn.Entity.IPScope == netutils.SiteLocal): + // We found a (possibly routed) multicast request that matches the response! + return conn + case conn.Entity.IP.Equal(responseNetBroadcastIP) && + responseFromNet.Contains(conn.LocalIP): + // We found a (link local) broadcast request that matches the response! + return conn + } + } + + return nil +} diff --git a/network/netutils/ip.go b/network/netutils/ip.go index f6a34003..76bb0230 100644 --- a/network/netutils/ip.go +++ b/network/netutils/ip.go @@ -28,6 +28,9 @@ func GetIPScope(ip net.IP) IPScope { //nolint:gocognit if ip4 := ip.To4(); ip4 != nil { // IPv4 switch { + case ip4[0] == 0: + // 0.0.0.0/8 + return Invalid case ip4[0] == 127: // 127.0.0.0/8 (RFC1918) return HostLocal @@ -79,6 +82,8 @@ func GetIPScope(ip net.IP) IPScope { //nolint:gocognit } else if len(ip) == net.IPv6len { // IPv6 switch { + case ip.Equal(net.IPv6zero): + return Invalid case ip.Equal(net.IPv6loopback): return HostLocal case ip[0]&0xfe == 0xfc: @@ -124,3 +129,29 @@ func (scope IPScope) IsGlobal() bool { return false } } + +// GetBroadcastAddress returns the broadcast address of the given IP and network mask. +// If a mixed IPv4/IPv6 input is given, it returns nil. +func GetBroadcastAddress(ip net.IP, netMask net.IPMask) net.IP { + // Convert to standard v4. + if ip4 := ip.To4(); ip4 != nil { + ip = ip4 + } + mask := net.IP(netMask) + if ip4Mask := mask.To4(); ip4Mask != nil { + mask = ip4Mask + } + + // Check for mixed v4/v6 input. + if len(ip) != len(mask) { + return nil + } + + // Merge to broadcast address + n := len(ip) + broadcastAddress := make(net.IP, n) + for i := 0; i < n; i++ { + broadcastAddress[i] = ip[i] | ^mask[i] + } + return broadcastAddress +} diff --git a/process/process.go b/process/process.go index 8bae2772..ff04896e 100644 --- a/process/process.go +++ b/process/process.go @@ -67,6 +67,32 @@ func (p *Process) Profile() *profile.LayeredProfile { return p.profile } +// IsIdentified returns whether the process has been identified or if it +// represents some kind of unidentified process. +func (p *Process) IsIdentified() bool { + // Check if process exists. + if p == nil { + return false + } + + // Check for special PIDs. + switch p.Pid { + case UndefinedProcessID: + return false + case UnidentifiedProcessID: + return false + case UnsolicitedProcessID: + return false + default: + return true + } +} + +// Equal returns if the two processes are both identified and have the same PID. +func (p *Process) Equal(other *Process) bool { + return p.IsIdentified() && other.IsIdentified() && p.Pid == other.Pid +} + // IsSystemResolver is a shortcut to check if the process is or belongs to the // system resolver and needs special handling. func (p *Process) IsSystemResolver() bool { From 9a89f65027d923a0e44ff6ec2fcff0df514b9f75 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 24 May 2022 11:21:11 +0200 Subject: [PATCH 4/8] Improve support for DNS-SD and fall back to cached data for non-ICANN queries --- firewall/master.go | 5 +-- nameserver/nameserver.go | 73 +++++++++++++++++++++++++++++++++++---- network/netutils/dns.go | 52 +++++++++++++++++++--------- resolver/resolve.go | 42 ++++++++++++++++++++++ resolver/resolver-env.go | 11 +++++- resolver/resolver_test.go | 56 ++++++++++++++++++++++++++++++ 6 files changed, 211 insertions(+), 28 deletions(-) diff --git a/firewall/master.go b/firewall/master.go index 4be457aa..e50617d0 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -482,10 +482,7 @@ func checkDomainHeuristics(ctx context.Context, conn *network.Connection, p *pro trimmedDomain := strings.TrimRight(conn.Entity.Domain, ".") etld1, err := publicsuffix.EffectiveTLDPlusOne(trimmedDomain) if err != nil { - // we don't apply any checks here and let the request through - // because a malformed domain-name will likely be dropped by - // checks better suited for that. - log.Tracer(ctx).Warningf("filter: failed to get eTLD+1: %s", err) + // Don't run the check if the domain is a TLD. return false } diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index a92ca8ad..afdae60b 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -106,6 +106,9 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) return reply(nsutil.Refused("invalid domain")) } + // Get public suffix after validation. + q.InitPublicSuffixData() + // Check if query is failing. // Some software retries failing queries excessively. This might not be a // problem normally, but handling a request is pretty expensive for the @@ -252,10 +255,13 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) if err != nil { 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())) - + // Try alternatives domain names for unofficial domain spaces. + rrCache = checkAlternativeCaches(ctx, q) + if rrCache == nil { + 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(), "") @@ -268,7 +274,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) case errors.Is(err, resolver.ErrOffline): if rrCache == nil { - log.Tracer(ctx).Debugf("nameserver: not resolving %s, device is offline", q.ID()) + tracer.Debugf("nameserver: not resolving %s, device is offline", q.ID()) conn.Failed("not resolving, device is offline", "") return reply(nsutil.ServerFailure(err.Error())) } @@ -290,8 +296,12 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) addFailingQuery(q, errors.New("emptry reply from resolver")) return reply(nsutil.ServerFailure("internal error: empty reply")) case rrCache.RCode == dns.RcodeNameError: - // Return now if NXDomain. - return reply(nsutil.NxDomain("no answer found (NXDomain)")) + // Try alternatives domain names for unofficial domain spaces. + rrCache = checkAlternativeCaches(ctx, q) + if rrCache == nil { + // Return now if NXDomain. + return reply(nsutil.NxDomain("no answer found (NXDomain)")) + } } // Check with firewall again after resolving. @@ -336,3 +346,52 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) ) return reply(rrCache, conn, rrCache) } + +func checkAlternativeCaches(ctx context.Context, q *resolver.Query) *resolver.RRCache { + // Do not try alternatives when the query is in a public suffix. + // This also includes arpa. and local. + if q.ICANNSpace { + return nil + } + + // Check if the env resolver has something. + pmEnvQ := &resolver.Query{ + FQDN: q.FQDN + "local." + resolver.InternalSpecialUseDomain, + QType: q.QType, + } + rrCache, err := resolver.QueryPortmasterEnv(ctx, pmEnvQ) + if err == nil && rrCache != nil && rrCache.RCode == dns.RcodeSuccess { + makeAlternativeRecord(ctx, q, rrCache, pmEnvQ.FQDN) + return rrCache + } + + // Check if we have anything in cache + localFQDN := q.FQDN + "local." + rrCache, err = resolver.GetRRCache(localFQDN, q.QType) + if err == nil && rrCache != nil && rrCache.RCode == dns.RcodeSuccess { + makeAlternativeRecord(ctx, q, rrCache, localFQDN) + return rrCache + } + + return nil +} + +func makeAlternativeRecord(ctx context.Context, q *resolver.Query, rrCache *resolver.RRCache, altName string) { + log.Tracer(ctx).Debugf("using %s to answer query", altName) + + // Duplicate answers so they match the query. + copied := make([]dns.RR, 0, len(rrCache.Answer)) + for _, answer := range rrCache.Answer { + if strings.ToLower(answer.Header().Name) == altName { + copiedAnswer := dns.Copy(answer) + copiedAnswer.Header().Name = q.FQDN + copied = append(copied, copiedAnswer) + } + } + if len(copied) > 0 { + rrCache.Answer = append(rrCache.Answer, copied...) + } + + // Update the question. + rrCache.Domain = q.FQDN +} diff --git a/network/netutils/dns.go b/network/netutils/dns.go index 7a7bd6e5..252dca47 100644 --- a/network/netutils/dns.go +++ b/network/netutils/dns.go @@ -4,21 +4,38 @@ import ( "fmt" "net" "regexp" + "strings" "github.com/miekg/dns" ) -var cleanDomainRegex = regexp.MustCompile( - `^` + // match beginning - `(` + // start subdomain group - `(xn--)?` + // idn prefix - `[a-z0-9_-]{1,63}` + // main chunk - `\.` + // ending with a dot - `)*` + // end subdomain group, allow any number of subdomains - `(xn--)?` + // TLD idn prefix - `[a-z0-9_-]{2,63}` + // TLD main chunk with at least two characters - `\.` + // ending with a dot - `$`, // match end +var ( + cleanDomainRegex = regexp.MustCompile( + `^` + // match beginning + `(` + // start subdomain group + `(xn--)?` + // idn prefix + `[a-z0-9_-]{1,63}` + // main chunk + `\.` + // ending with a dot + `)*` + // end subdomain group, allow any number of subdomains + `(xn--)?` + // TLD idn prefix + `[a-z0-9_-]{2,63}` + // TLD main chunk with at least two characters + `\.` + // ending with a dot + `$`, // match end + ) + + // dnsSDDomainRegex is a lot more lax to better suit the allowed characters in DNS-SD. + // Not all characters have been allowed - some special characters were + // removed to reduce the general attack surface. + dnsSDDomainRegex = regexp.MustCompile( + // Start of charset selection. + `^[` + + // Printable ASCII (character code 32-127), excluding some special characters. + ` !#$%&()*+,\-\./0-9:;=?@A-Z[\\\]^_\a-z{|}~` + + // Only latin characters from extended ASCII (character code 128-255). + `ŠŒŽšœžŸ¡¿ÀÁÂÃÄÅÆÇÈÉÊËÌÍÎÏÐÑÒÓÔÕÖØÙÚÛÜÝÞßàáâãäåæçèéêëìíîïðñòóôõöøùúûüýþÿ` + + // End of charset selection. + `]*$`, + ) ) // IsValidFqdn returns whether the given string is a valid fqdn. @@ -33,15 +50,18 @@ func IsValidFqdn(fqdn string) bool { return false } - // check with regex - if !cleanDomainRegex.MatchString(fqdn) { + // IsFqdn checks if a domain name is fully qualified. + if !dns.IsFqdn(fqdn) { return false } - // check with miegk/dns + // Use special check for .local domains to support DNS-SD. + if strings.HasSuffix(fqdn, ".local.") { + return dnsSDDomainRegex.MatchString(fqdn) + } - // IsFqdn checks if a domain name is fully qualified. - if !dns.IsFqdn(fqdn) { + // check with regex + if !cleanDomainRegex.MatchString(fqdn) { return false } diff --git a/resolver/resolve.go b/resolver/resolve.go index 1b0b429e..9f7fad66 100644 --- a/resolver/resolve.go +++ b/resolver/resolve.go @@ -5,10 +5,12 @@ import ( "errors" "fmt" "net" + "strings" "sync" "time" "github.com/miekg/dns" + "golang.org/x/net/publicsuffix" "github.com/safing/portbase/database" "github.com/safing/portbase/log" @@ -90,6 +92,11 @@ type Query struct { IgnoreFailing bool LocalResolversOnly bool + // ICANNSpace signifies if the domain is within ICANN managed domain space. + ICANNSpace bool + // Domain root is the effective TLD +1. + DomainRoot string + // internal dotPrefixedFQDN string } @@ -99,6 +106,41 @@ func (q *Query) ID() string { return q.FQDN + q.QType.String() } +// InitPublicSuffixData initializes the public suffix data. +func (q *Query) InitPublicSuffixData() { + // Get public suffix and derive if domain is in ICANN space. + suffix, icann := publicsuffix.PublicSuffix(strings.TrimSuffix(q.FQDN, ".")) + if icann || strings.Contains(suffix, ".") { + q.ICANNSpace = true + } + // Override some cases. + switch suffix { + case "example": + q.ICANNSpace = true // Defined by ICANN. + case "invalid": + q.ICANNSpace = true // Defined by ICANN. + case "local": + q.ICANNSpace = true // Defined by ICANN. + case "localhost": + q.ICANNSpace = true // Defined by ICANN. + case "onion": + q.ICANNSpace = false // Defined by ICANN, but special. + case "test": + q.ICANNSpace = true // Defined by ICANN. + } + // Add suffix to adhere to FQDN format. + suffix += "." + + switch { + case len(q.FQDN) == len(suffix): + // We are at or below the domain root, reset. + q.DomainRoot = "" + case len(q.FQDN) > len(suffix): + domainRootStart := strings.LastIndex(q.FQDN[:len(q.FQDN)-len(suffix)-1], ".") + 1 + q.DomainRoot = q.FQDN[domainRootStart:] + } +} + // check runs sanity checks and does some initialization. Returns whether the query passed the basic checks. func (q *Query) check() (ok bool) { if q.FQDN == "" { diff --git a/resolver/resolver-env.go b/resolver/resolver-env.go index 18acfba4..b8710ff3 100644 --- a/resolver/resolver-env.go +++ b/resolver/resolver-env.go @@ -128,7 +128,7 @@ func (er *envResolverConn) makeRRCache(q *Query, answers []dns.RR) *RRCache { // Disable caching, as the env always has the raw data available. q.NoCaching = true - return &RRCache{ + rrCache := &RRCache{ Domain: q.FQDN, Question: q.QType, RCode: dns.RcodeSuccess, @@ -136,6 +136,10 @@ func (er *envResolverConn) makeRRCache(q *Query, answers []dns.RR) *RRCache { Extra: []dns.RR{internalSpecialUseComment}, // Always add comment about this TLD. Resolver: envResolver.Info.Copy(), } + if len(rrCache.Answer) == 0 { + rrCache.RCode = dns.RcodeNameError + } + return rrCache } func (er *envResolverConn) ReportFailure() {} @@ -145,3 +149,8 @@ func (er *envResolverConn) IsFailing() bool { } func (er *envResolverConn) ResetFailure() {} + +// QueryPortmasterEnv queries the environment resolver directly. +func QueryPortmasterEnv(ctx context.Context, q *Query) (*RRCache, error) { + return envResolver.Conn.Query(ctx, q) +} diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go index f385e52e..61e05376 100644 --- a/resolver/resolver_test.go +++ b/resolver/resolver_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/miekg/dns" "github.com/safing/portbase/log" @@ -103,3 +105,57 @@ func TestBulkResolving(t *testing.T) { t.Logf("total time taken: %s", time.Since(started)) } + +func TestPublicSuffix(t *testing.T) { + t.Parallel() + + testSuffix(t, "co.uk.", "", true) + testSuffix(t, "amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "books.amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "www.books.amazon.co.uk.", "amazon.co.uk.", true) + testSuffix(t, "com.", "", true) + testSuffix(t, "amazon.com.", "amazon.com.", true) + testSuffix(t, "example0.debian.net.", "example0.debian.net.", true) + testSuffix(t, "example1.debian.org.", "debian.org.", true) + testSuffix(t, "golang.dev.", "golang.dev.", true) + testSuffix(t, "golang.net.", "golang.net.", true) + testSuffix(t, "play.golang.org.", "golang.org.", true) + testSuffix(t, "gophers.in.space.museum.", "in.space.museum.", true) + testSuffix(t, "0emm.com.", "0emm.com.", true) + testSuffix(t, "a.0emm.com.", "", true) + testSuffix(t, "b.c.d.0emm.com.", "c.d.0emm.com.", true) + testSuffix(t, "org.", "", true) + testSuffix(t, "foo.org.", "foo.org.", true) + testSuffix(t, "foo.co.uk.", "foo.co.uk.", true) + testSuffix(t, "foo.dyndns.org.", "foo.dyndns.org.", true) + testSuffix(t, "foo.blogspot.co.uk.", "foo.blogspot.co.uk.", true) + testSuffix(t, "there.is.no.such-tld.", "no.such-tld.", false) + testSuffix(t, "www.some.bit.", "some.bit.", false) + testSuffix(t, "cromulent.", "", false) + testSuffix(t, "arpa.", "", true) + testSuffix(t, "in-addr.arpa.", "", true) + testSuffix(t, "1.in-addr.arpa.", "1.in-addr.arpa.", true) + testSuffix(t, "ip6.arpa.", "", true) + testSuffix(t, "1.ip6.arpa.", "1.ip6.arpa.", true) + testSuffix(t, "www.some.arpa.", "some.arpa.", true) + testSuffix(t, "www.some.home.arpa.", "home.arpa.", true) + testSuffix(t, ".", "", false) + testSuffix(t, "", "", false) + + // Test edge case domains. + testSuffix(t, "www.some.example.", "some.example.", true) + testSuffix(t, "www.some.invalid.", "some.invalid.", true) + testSuffix(t, "www.some.local.", "some.local.", true) + testSuffix(t, "www.some.localhost.", "some.localhost.", true) + testSuffix(t, "www.some.onion.", "some.onion.", false) + testSuffix(t, "www.some.test.", "some.test.", true) +} + +func testSuffix(t *testing.T, fqdn, domainRoot string, icannSpace bool) { + t.Helper() + + q := &Query{FQDN: fqdn} + q.InitPublicSuffixData() + assert.Equal(t, domainRoot, q.DomainRoot) + assert.Equal(t, icannSpace, q.ICANNSpace) +} From 65f646aad02a081330242a9491c1af25cc8ec92e Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 24 May 2022 11:27:44 +0200 Subject: [PATCH 5/8] Improve wording on setting dns/noInsecureProtocols --- resolver/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolver/config.go b/resolver/config.go index b274ee23..ff17cb99 100644 --- a/resolver/config.go +++ b/resolver/config.go @@ -224,9 +224,9 @@ The format is: "protocol://ip:port?parameter=value¶meter=value" noMulticastDNS = status.SecurityLevelOption(CfgOptionNoMulticastDNSKey) err = config.Register(&config.Option{ - Name: "Enforce Secure DNS", + Name: "Use Secure Protocols Only", Key: CfgOptionNoInsecureProtocolsKey, - Description: "Never resolve using insecure protocols, ie. plain DNS.", + Description: "Never resolve using insecure protocols, ie. plain DNS. This may break certain local DNS services, which always use plain DNS.", OptType: config.OptTypeInt, ExpertiseLevel: config.ExpertiseLevelExpert, ReleaseLevel: config.ReleaseLevelStable, From 515f4686f7eb28db095f6fe676e3d110bd7d00d4 Mon Sep 17 00:00:00 2001 From: Daniel Date: Wed, 25 May 2022 13:57:31 +0200 Subject: [PATCH 6/8] Send notification instead of killing conflicting DNS service --- nameserver/conflict.go | 75 +++++++++++++++++++ nameserver/module.go | 85 +++++++++++++++++++-- nameserver/takeover.go | 164 ----------------------------------------- 3 files changed, 154 insertions(+), 170 deletions(-) create mode 100644 nameserver/conflict.go delete mode 100644 nameserver/takeover.go diff --git a/nameserver/conflict.go b/nameserver/conflict.go new file mode 100644 index 00000000..e02e1fd5 --- /dev/null +++ b/nameserver/conflict.go @@ -0,0 +1,75 @@ +package nameserver + +import ( + "net" + "os" + + processInfo "github.com/shirou/gopsutil/process" + + "github.com/safing/portbase/log" + "github.com/safing/portmaster/network/packet" + "github.com/safing/portmaster/network/state" +) + +var commonResolverIPs = []net.IP{ + net.IPv4zero, + net.IPv4(127, 0, 0, 1), // default + net.IPv4(127, 0, 0, 53), // some resolvers on Linux + net.IPv6zero, + net.IPv6loopback, +} + +func findConflictingProcess(ip net.IP, port uint16) (conflictingProcess *processInfo.Process) { + // Evaluate which IPs to check. + var ipsToCheck []net.IP + if ip.Equal(net.IPv4zero) || ip.Equal(net.IPv6zero) { + ipsToCheck = commonResolverIPs + } else { + ipsToCheck = []net.IP{ip} + } + + // Find the conflicting process. + var err error + for _, resolverIP := range ipsToCheck { + conflictingProcess, err = getListeningProcess(resolverIP, port) + switch { + case err != nil: + // Log the error and let the worker try again. + log.Warningf("nameserver: failed to find conflicting service: %s", err) + case conflictingProcess != nil: + // Conflicting service found. + return conflictingProcess + } + } + + return nil +} + +func getListeningProcess(resolverIP net.IP, resolverPort uint16) (*processInfo.Process, error) { + pid, _, err := state.Lookup(&packet.Info{ + Inbound: true, + Version: 0, // auto-detect + Protocol: packet.UDP, + Src: nil, // do not record direction + SrcPort: 0, // do not record direction + Dst: resolverIP, + DstPort: resolverPort, + }, true) + if err != nil { + // there may be nothing listening on :53 + return nil, nil //nolint:nilerr // Treat lookup error as "not found". + } + + // Ignore if it's us for some reason. + if pid == os.Getpid() { + return nil, nil + } + + proc, err := processInfo.NewProcess(int32(pid)) + if err != nil { + // Process may have disappeared already. + return nil, err + } + + return proc, nil +} diff --git a/nameserver/module.go b/nameserver/module.go index 1c1dc871..f5165585 100644 --- a/nameserver/module.go +++ b/nameserver/module.go @@ -13,6 +13,7 @@ import ( "github.com/safing/portbase/log" "github.com/safing/portbase/modules" "github.com/safing/portbase/modules/subsystems" + "github.com/safing/portbase/notifications" "github.com/safing/portmaster/compat" "github.com/safing/portmaster/firewall" "github.com/safing/portmaster/netenv" @@ -25,6 +26,9 @@ var ( stopListener1 func() error stopListener2 func() error stopListenersLock sync.Mutex + + eventIDConflictingService = "nameserver:conflicting-service" + eventIDListenerFailed = "nameserver:listener-failed" ) func init() { @@ -133,24 +137,93 @@ func startListener(ip net.IP, port uint16, first bool) { return nil } + // Resolve generic listener error, if primary listener. + if first { + module.Resolve(eventIDListenerFailed) + } + // Start listening. log.Infof("nameserver: starting to listen on %s", dnsServer.Addr) err := dnsServer.ListenAndServe() if err != nil { - // check if we are shutting down + // Stop worker without error if we are shutting down. if module.IsStopping() { return nil } - // is something blocking our port? - checkErr := checkForConflictingService(ip, port) - if checkErr != nil { - return checkErr - } + log.Warningf("nameserver: failed to listen on %s: %s", dnsServer.Addr, err) + handleListenError(err, ip, port, first) } return err }) } +func handleListenError(err error, ip net.IP, port uint16, primaryListener bool) { + var n *notifications.Notification + + // Create suffix for secondary listener + var secondaryEventIDSuffix string + if !primaryListener { + secondaryEventIDSuffix = "-secondary" + } + + // Find a conflicting service. + cfProcess := findConflictingProcess(ip, port) + if cfProcess != nil { + // Report the conflicting process. + + // Build conflicting process description. + var cfDescription string + cfName, err := cfProcess.Name() + if err == nil && cfName != "" { + cfDescription = cfName + } + cfExe, err := cfProcess.Exe() + if err == nil && cfDescription != "" { + if cfDescription != "" { + cfDescription += " (" + cfExe + ")" + } else { + cfDescription = cfName + } + } + + // Notify user about conflicting service. + n = notifications.Notify(¬ifications.Notification{ + EventID: eventIDConflictingService + secondaryEventIDSuffix, + Type: notifications.Error, + Title: "Conflicting DNS Software", + Message: fmt.Sprintf( + "Restart Portmaster after you have deactivated or properly configured the conflicting software: %s", + cfDescription, + ), + ShowOnSystem: true, + AvailableActions: []*notifications.Action{ + { + Text: "Open Docs", + Type: notifications.ActionTypeOpenURL, + Payload: "https://docs.safing.io/portmaster/install/status/software-compatibility", + }, + }, + }) + } else { + // If no conflict is found, report the error directly. + n = notifications.Notify(¬ifications.Notification{ + EventID: eventIDListenerFailed + secondaryEventIDSuffix, + Type: notifications.Error, + Title: "Secure DNS Error", + Message: fmt.Sprintf( + "The internal DNS server failed. Restart Portmaster to try again. Error: %s", + err, + ), + ShowOnSystem: true, + }) + } + + // Attach error to module, if primary listener. + if primaryListener { + n.AttachToModule(module) + } +} + func stop() error { stopListenersLock.Lock() defer stopListenersLock.Unlock() diff --git a/nameserver/takeover.go b/nameserver/takeover.go deleted file mode 100644 index fa1b65af..00000000 --- a/nameserver/takeover.go +++ /dev/null @@ -1,164 +0,0 @@ -package nameserver - -import ( - "fmt" - "net" - "os" - "strconv" - "time" - - "github.com/safing/portbase/log" - "github.com/safing/portbase/modules" - "github.com/safing/portbase/notifications" - "github.com/safing/portmaster/network/packet" - "github.com/safing/portmaster/network/state" -) - -var ( - commonResolverIPs = []net.IP{ - net.IPv4zero, - net.IPv4(127, 0, 0, 1), // default - net.IPv4(127, 0, 0, 53), // some resolvers on Linux - net.IPv6zero, - net.IPv6loopback, - } - - // lastKilledPID holds the PID of the last killed conflicting service. - // It is only accessed by checkForConflictingService, which is only called by - // the nameserver worker. - lastKilledPID int -) - -func checkForConflictingService(ip net.IP, port uint16) error { - // Evaluate which IPs to check. - var ipsToCheck []net.IP - if ip.Equal(net.IPv4zero) || ip.Equal(net.IPv6zero) { - ipsToCheck = commonResolverIPs - } else { - ipsToCheck = []net.IP{ip} - } - - // Check if there is another resolver when need to take over. - var killed int - var killingFailed bool -ipsToCheckLoop: - for _, resolverIP := range ipsToCheck { - pid, err := takeover(resolverIP, port) - switch { - case err != nil: - // Log the error and let the worker try again. - log.Infof("nameserver: failed to stop conflicting service: %s", err) - killingFailed = true - break ipsToCheckLoop - case pid != 0: - // Conflicting service identified and killed! - killed = pid - break ipsToCheckLoop - } - } - - // Notify user of failed killing or repeated kill. - if killingFailed || (killed != 0 && killed == lastKilledPID) { - // Notify the user that we failed to kill something. - notifications.Notify(¬ifications.Notification{ - EventID: "namserver:failed-to-kill-conflicting-service", - Type: notifications.Error, - Title: "Failed to Stop Conflicting DNS Client", - Message: "The Portmaster failed to stop a conflicting DNS client to gain required system integration. If there is another DNS Client (Nameserver; Resolver) on this device, please disable it.", - ShowOnSystem: true, - AvailableActions: []*notifications.Action{ - { - ID: "ack", - Text: "OK", - }, - { - Text: "Open Docs", - Type: notifications.ActionTypeOpenURL, - Payload: "https://docs.safing.io/portmaster/install/status/software-compatibility", - }, - }, - }) - return nil - } - - // Check if something was killed. - if killed == 0 { - return nil - } - lastKilledPID = killed - - // Notify the user that we killed something. - notifications.Notify(¬ifications.Notification{ - EventID: "namserver:stopped-conflicting-service", - Type: notifications.Info, - Title: "Stopped Conflicting DNS Client", - Message: fmt.Sprintf( - "The Portmaster stopped a conflicting DNS client (pid %d) to gain required system integration. If you are running another DNS client on this device on purpose, you can the check the documentation if it is compatible with the Portmaster.", - killed, - ), - ShowOnSystem: true, - AvailableActions: []*notifications.Action{ - { - ID: "ack", - Text: "OK", - }, - { - Text: "Open Docs", - Type: notifications.ActionTypeOpenURL, - Payload: "https://docs.safing.io/portmaster/install/status/software-compatibility", - }, - }, - }) - - // Restart nameserver via service-worker logic. - // Wait shortly so that the other process can shut down. - time.Sleep(10 * time.Millisecond) - return fmt.Errorf("%w: stopped conflicting name service with pid %d", modules.ErrRestartNow, killed) -} - -func takeover(resolverIP net.IP, resolverPort uint16) (int, error) { - pid, _, err := state.Lookup(&packet.Info{ - Inbound: true, - Version: 0, // auto-detect - Protocol: packet.UDP, - Src: nil, // do not record direction - SrcPort: 0, // do not record direction - Dst: resolverIP, - DstPort: resolverPort, - }, true) - if err != nil { - // there may be nothing listening on :53 - return 0, nil //nolint:nilerr // Treat lookup error as "not found". - } - - // Just don't, uh, kill ourselves... - if pid == os.Getpid() { - return 0, nil - } - - proc, err := os.FindProcess(pid) - if err != nil { - // huh. gone already? I guess we'll wait then... - return 0, err - } - - err = proc.Signal(os.Interrupt) - if err != nil { - err = proc.Kill() - if err != nil { - log.Errorf("nameserver: failed to stop conflicting service (pid %d): %s", pid, err) - return 0, err - } - } - - log.Warningf( - "nameserver: killed conflicting service with PID %d over %s", - pid, - net.JoinHostPort( - resolverIP.String(), - strconv.Itoa(int(resolverPort)), - ), - ) - - return pid, nil -} From 787f9e7dec106e79f36196caa8f1047ee7e7c8cc Mon Sep 17 00:00:00 2001 From: Daniel Date: Wed, 1 Jun 2022 13:28:10 +0200 Subject: [PATCH 7/8] Add support for upcoming UNBREAK filter list --- firewall/filter.go | 18 +++++++++++++++++- firewall/master.go | 10 ++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/firewall/filter.go b/firewall/filter.go index 58a24737..390ef3f3 100644 --- a/firewall/filter.go +++ b/firewall/filter.go @@ -2,9 +2,11 @@ package firewall import ( "github.com/safing/portbase/config" + "github.com/safing/portbase/log" "github.com/safing/portbase/modules" "github.com/safing/portbase/modules/subsystems" _ "github.com/safing/portmaster/core" + "github.com/safing/portmaster/intel/filterlists" "github.com/safing/spn/captain" ) @@ -12,10 +14,13 @@ var ( filterModule *modules.Module filterEnabled config.BoolOption tunnelEnabled config.BoolOption + + unbreakFilterListIDs = []string{"UNBREAK"} + resolvedUnbreakFilterListIDs []string ) func init() { - filterModule = modules.Register("filter", filterPrep, nil, nil, "core", "intel") + filterModule = modules.Register("filter", filterPrep, filterStart, nil, "core", "intel") subsystems.Register( "filter", "Privacy Filter", @@ -47,3 +52,14 @@ func filterPrep() (err error) { tunnelEnabled = config.Concurrent.GetAsBool(captain.CfgOptionEnableSPNKey, false) return nil } + +func filterStart() error { + // TODO: Re-resolve IDs when filterlist index changes. + resolvedIDs, err := filterlists.ResolveListIDs(unbreakFilterListIDs) + if err != nil { + log.Warningf("filter: failed to resolve unbreak filter list IDs: %s", err) + } else { + resolvedUnbreakFilterListIDs = resolvedIDs + } + return nil +} diff --git a/firewall/master.go b/firewall/master.go index e50617d0..1de7f194 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -432,6 +432,16 @@ func checkFilterLists(ctx context.Context, conn *network.Connection, p *profile. result, reason := p.MatchFilterLists(ctx, conn.Entity) switch result { case endpoints.Denied: + // If the connection matches a filter list, check if the "unbreak" list matches too and abort blocking. + for _, blockedListID := range conn.Entity.BlockedByLists { + for _, unbreakListID := range resolvedUnbreakFilterListIDs { + if blockedListID == unbreakListID { + log.Tracer(ctx).Debugf("filter: unbreak filter %s matched, ignoring other filter list matches", unbreakListID) + return false + } + } + } + // Otherwise, continue with blocking. conn.DenyWithContext(reason.String(), profile.CfgOptionFilterListsKey, reason.Context()) return true case endpoints.NoMatch: From 911a80996a6abc208fe078c3461ab40d6bd3a3a3 Mon Sep 17 00:00:00 2001 From: Daniel Date: Wed, 1 Jun 2022 13:45:13 +0200 Subject: [PATCH 8/8] Fix multicast detection on ended connections as well as some linter errors --- core/base/databases.go | 2 -- network/multicast.go | 4 ++-- profile/module.go | 2 -- resolver/resolver_test.go | 3 +-- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/core/base/databases.go b/core/base/databases.go index d2ba2552..56355125 100644 --- a/core/base/databases.go +++ b/core/base/databases.go @@ -2,8 +2,6 @@ package base import ( "github.com/safing/portbase/database" - - // Dependencies. _ "github.com/safing/portbase/database/dbmodule" _ "github.com/safing/portbase/database/storage/bbolt" ) diff --git a/network/multicast.go b/network/multicast.go index 82cb2173..8a32aea7 100644 --- a/network/multicast.go +++ b/network/multicast.go @@ -17,8 +17,8 @@ func GetMulticastRequestConn(responseConn *Connection, responseFromNet *net.IPNe switch { case conn.Inbound: // Ignore incoming connections. - //case conn.Ended != 0: - // Ignore ended connections. + case conn.Ended != 0: + // Ignore ended connections. case conn.Entity.Protocol != responseConn.Entity.Protocol: // Ignore on protocol mismatch. case conn.LocalPort != responseConn.LocalPort: diff --git a/profile/module.go b/profile/module.go index da479663..31b52704 100644 --- a/profile/module.go +++ b/profile/module.go @@ -6,8 +6,6 @@ import ( "github.com/safing/portbase/database/migration" "github.com/safing/portbase/log" "github.com/safing/portbase/modules" - - // Dependency. _ "github.com/safing/portmaster/core/base" "github.com/safing/portmaster/updates" ) diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go index 61e05376..a03b3eb9 100644 --- a/resolver/resolver_test.go +++ b/resolver/resolver_test.go @@ -7,9 +7,8 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" - "github.com/miekg/dns" + "github.com/stretchr/testify/assert" "github.com/safing/portbase/log" )