diff --git a/firewall/bypassing.go b/firewall/bypassing.go index a7af6ee1..822a21ce 100644 --- a/firewall/bypassing.go +++ b/firewall/bypassing.go @@ -6,6 +6,7 @@ import ( "github.com/safing/portmaster/nameserver/nsutil" "github.com/safing/portmaster/network" + "github.com/safing/portmaster/network/packet" "github.com/safing/portmaster/profile/endpoints" ) @@ -16,17 +17,23 @@ var ( // 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) { - // Block firefox canary domain to disable DoH + // Block firefox canary domain to disable DoH. 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() } - if conn.Entity.MatchLists(resolverFilterLists) { - return endpoints.Denied, - "blocked rogue connection to DNS resolver", - nsutil.ZeroIP() + // Block direct connections to known DNS resolvers. + switch packet.IPProtocol(conn.Entity.Protocol) { + 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) { + return endpoints.Denied, + "blocked rogue connection to DNS resolver", + nsutil.BlockIP() + } } return endpoints.NoMatch, "", nil diff --git a/firewall/master.go b/firewall/master.go index d2fe21bb..fcf946a5 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -54,13 +54,6 @@ var defaultDeciders = []deciderFn{ checkAutoPermitRelated, } -var dnsFromSystemResolverDeciders = []deciderFn{ - checkEndpointListsForSystemResolverDNSRequests, - checkConnectivityDomain, - checkBypassPrevention, - checkFilterLists, -} - // DecideOnConnection makes a decision about a connection. // When called, the connection and profile is already locked. func DecideOnConnection(ctx context.Context, conn *network.Connection, pkt packet.Packet) { @@ -99,25 +92,19 @@ func DecideOnConnection(ctx context.Context, conn *network.Connection, pkt packe conn.Entity.EnableCNAMECheck(ctx, layeredProfile.FilterCNAMEs()) conn.Entity.LoadLists(ctx) - // DNS request from the system resolver require a special decision process, - // because the original requesting process is not known. Here, we only check - // global-only and the most important per-app aspects. The resulting - // connection is then blocked when the original requesting process is known. - if conn.Type == network.DNSRequest && conn.Process().IsSystemResolver() { - // Run all deciders and return if they came to a conclusion. - done, _ := runDeciders(ctx, dnsFromSystemResolverDeciders, conn, layeredProfile, pkt) - if !done { - conn.Accept("allowing system resolver dns request", noReasonOptionKey) - } - return - } - // Run all deciders and return if they came to a conclusion. done, defaultAction := runDeciders(ctx, defaultDeciders, conn, layeredProfile, pkt) if done { return } + // DNS Request are always default allowed, as the endpoint lists could not + // be checked fully. + if conn.Type == network.DNSRequest { + conn.Accept("allowing dns request", noReasonOptionKey) + return + } + // Deciders did not conclude, use default action. switch defaultAction { case profile.DefaultActionPermit: @@ -197,6 +184,14 @@ func checkSelfCommunication(ctx context.Context, conn *network.Connection, _ *pr } 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 + // global-only and the most important per-app aspects. The resulting + // connection is then blocked when the original requesting process is known. + if conn.Type == network.DNSRequest && conn.Process().IsSystemResolver() { + return checkEndpointListsForSystemResolverDNSRequests(ctx, conn, p) + } + var result endpoints.EPResult var reason endpoints.Reason @@ -210,7 +205,7 @@ func checkEndpointLists(ctx context.Context, conn *network.Connection, p *profil optionKey = profile.CfgOptionEndpointsKey } switch result { - case endpoints.Denied: + case endpoints.Denied, endpoints.MatchError: conn.DenyWithContext(reason.String(), optionKey, reason.Context()) return true case endpoints.Permitted: @@ -225,13 +220,13 @@ func checkEndpointLists(ctx context.Context, conn *network.Connection, p *profil // checkEndpointLists that is only meant for DNS queries by the system // resolver. It only checks the endpoint filter list of the local profile and // does not include the global profile. -func checkEndpointListsForSystemResolverDNSRequests(ctx context.Context, conn *network.Connection, p *profile.LayeredProfile, _ packet.Packet) bool { +func checkEndpointListsForSystemResolverDNSRequests(ctx context.Context, conn *network.Connection, p *profile.LayeredProfile) bool { profileEndpoints := p.LocalProfile().GetEndpoints() if profileEndpoints.IsSet() { result, reason := profileEndpoints.Match(ctx, conn.Entity) if endpoints.IsDecision(result) { switch result { - case endpoints.Denied: + case endpoints.Denied, endpoints.MatchError: conn.DenyWithContext(reason.String(), profile.CfgOptionEndpointsKey, reason.Context()) return true case endpoints.Permitted: @@ -396,11 +391,13 @@ func checkResolverScope(_ context.Context, conn *network.Connection, p *profile. } func checkDomainHeuristics(ctx context.Context, conn *network.Connection, p *profile.LayeredProfile, _ packet.Packet) bool { - if !p.DomainHeuristics() { + // Don't check domain heuristics if no domain is available. + if conn.Entity.Domain == "" { return false } - if conn.Entity.Domain == "" { + // Check if domain heuristics are enabled. + if !p.DomainHeuristics() { return false } @@ -485,10 +482,11 @@ func checkAutoPermitRelated(_ context.Context, conn *network.Connection, p *prof // checkRelation tries to find a relation between a process and a communication. This is for better out of the box experience and is _not_ meant to thwart intentional malware. func checkRelation(conn *network.Connection) (related bool, reason string) { - if conn.Entity.Domain != "" { + // Don't check relation if no domain is available. + if conn.Entity.Domain == "" { return false, "" } - // don't check for unknown processes + // Don't check for unknown processes. if conn.Process().Pid < 0 { return false, "" } diff --git a/intel/entity.go b/intel/entity.go index a15a4af3..2e93597b 100644 --- a/intel/entity.go +++ b/intel/entity.go @@ -71,6 +71,9 @@ type Entity struct { // ASOrg holds the owner's name of the autonomous system. ASOrg string + // LocationError holds an error message if fetching the location failed. + LocationError string + location *geoip.Location // BlockedByLists holds list source IDs that @@ -86,13 +89,16 @@ type Entity struct { // to a list of sources where the entity has been observed in. ListOccurences map[string][]string + // ListsError holds an error message if fetching the lists failed. + ListsError string + // we only load each data above at most once - fetchLocationOnce sync.Once - reverseResolveOnce sync.Once - loadDomainListOnce sync.Once - loadIPListOnce sync.Once - loadCoutryListOnce sync.Once - loadAsnListOnce sync.Once + fetchLocationOnce sync.Once + reverseResolveOnce sync.Once + loadDomainListOnce sync.Once + loadIPListOnce sync.Once + loadCountryListOnce sync.Once + loadAsnListOnce sync.Once } // Init initializes the internal state and returns the entity. @@ -142,7 +148,7 @@ func (e *Entity) ResetLists() { e.checkCNAMEs = false e.loadDomainListOnce = sync.Once{} e.loadIPListOnce = sync.Once{} - e.loadCoutryListOnce = sync.Once{} + e.loadCountryListOnce = sync.Once{} e.loadAsnListOnce = sync.Once{} } @@ -236,6 +242,7 @@ func (e *Entity) getLocation(ctx context.Context) { loc, err := geoip.GetLocation(e.IP) if err != nil { log.Tracer(ctx).Warningf("intel: failed to get location data for %s: %s", e.IP, err) + e.LocationError = err.Error() return } e.location = loc @@ -259,7 +266,7 @@ func (e *Entity) GetLocation(ctx context.Context) (*geoip.Location, bool) { func (e *Entity) GetCountry(ctx context.Context) (string, bool) { e.getLocation(ctx) - if e.Country == "" { + if e.LocationError != "" { return "", false } return e.Country, true @@ -269,13 +276,14 @@ func (e *Entity) GetCountry(ctx context.Context) (string, bool) { func (e *Entity) GetASN(ctx context.Context) (uint, bool) { e.getLocation(ctx) - if e.ASN == 0 { + if e.LocationError != "" { return 0, false } return e.ASN, true } // Lists + func (e *Entity) getLists(ctx context.Context) { e.getDomainLists(ctx) e.getASNLists(ctx) @@ -305,11 +313,11 @@ func (e *Entity) getDomainLists(ctx context.Context) { return } - var err error + log.Tracer(ctx).Tracef("intel: loading domain list for %s", domain) e.loadDomainListOnce.Do(func() { var domainsToInspect = []string{domain} - if e.checkCNAMEs { + if e.checkCNAMEs && len(e.CNAME) > 0 { log.Tracer(ctx).Tracef("intel: CNAME filtering enabled, checking %v too", e.CNAME) domainsToInspect = append(domainsToInspect, e.CNAME...) } @@ -327,11 +335,10 @@ func (e *Entity) getDomainLists(ctx context.Context) { domains = makeDistinct(domains) for _, d := range domains { - log.Tracer(ctx).Tracef("intel: loading domain list for %s", d) - var list []string - list, err = filterlists.LookupDomain(d) + list, err := filterlists.LookupDomain(d) if err != nil { log.Tracer(ctx).Errorf("intel: failed to get domain blocklists for %s: %s", d, err) + e.ListsError = err.Error() return } @@ -339,10 +346,6 @@ func (e *Entity) getDomainLists(ctx context.Context) { } e.domainListLoaded = true }) - - if err != nil { - e.loadDomainListOnce = sync.Once{} - } } func splitDomain(domain string) []string { @@ -377,7 +380,7 @@ func (e *Entity) getASNLists(ctx context.Context) { } asn, ok := e.GetASN(ctx) - if !ok { + if !ok || asn == 0 { return } @@ -387,7 +390,7 @@ func (e *Entity) getASNLists(ctx context.Context) { list, err := filterlists.LookupASNString(asnStr) if err != nil { log.Tracer(ctx).Errorf("intel: failed to get ASN blocklist for %d: %s", asn, err) - e.loadAsnListOnce = sync.Once{} + e.ListsError = err.Error() return } @@ -402,16 +405,16 @@ func (e *Entity) getCountryLists(ctx context.Context) { } country, ok := e.GetCountry(ctx) - if !ok { + if !ok || country == "" { return } log.Tracer(ctx).Tracef("intel: loading country list for %s", country) - e.loadCoutryListOnce.Do(func() { + e.loadCountryListOnce.Do(func() { list, err := filterlists.LookupCountry(country) if err != nil { log.Tracer(ctx).Errorf("intel: failed to load country blocklist for %s: %s", country, err) - e.loadCoutryListOnce = sync.Once{} + e.ListsError = err.Error() return } @@ -426,11 +429,7 @@ func (e *Entity) getIPLists(ctx context.Context) { } ip, ok := e.GetIP() - if !ok { - return - } - - if ip == nil { + if !ok || ip == nil { return } @@ -442,12 +441,12 @@ func (e *Entity) getIPLists(ctx context.Context) { log.Tracer(ctx).Tracef("intel: loading IP list for %s", ip) e.loadIPListOnce.Do(func() { list, err := filterlists.LookupIP(ip) - if err != nil { log.Tracer(ctx).Errorf("intel: failed to get IP blocklist for %s: %s", ip.String(), err) - e.loadIPListOnce = sync.Once{} + e.ListsError = err.Error() return } + e.ipListLoaded = true e.mergeList(ip.String(), list) }) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 3fe427f4..00cdc55e 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -197,7 +197,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) return reply(nsutil.NxDomain("nxdomain: " + err.Error())) case errors.Is(err, resolver.ErrBlocked): tracer.Tracef("nameserver: %s", err) - return reply(nsutil.ZeroIP("blocked: " + err.Error())) + return reply(nsutil.BlockIP("blocked: " + err.Error())) case errors.Is(err, resolver.ErrLocalhost): tracer.Tracef("nameserver: returning localhost records") return reply(nsutil.Localhost()) diff --git a/nameserver/nsutil/nsutil.go b/nameserver/nsutil/nsutil.go index e563b6c7..98ac6971 100644 --- a/nameserver/nsutil/nsutil.go +++ b/nameserver/nsutil/nsutil.go @@ -45,50 +45,42 @@ func (rf ResponderFunc) ReplyWithDNS(ctx context.Context, request *dns.Msg) *dns return rf(ctx, request) } +// BlockIP is a ResponderFunc than replies with either 0.0.0.17 or ::17 for +// each A or AAAA question respectively. If there is no A or AAAA question, it +// defaults to replying with NXDomain. +func BlockIP(msgs ...string) ResponderFunc { + return createResponderFunc( + "blocking", + "0.0.0.17", + "::17", + msgs..., + ) +} + // ZeroIP is a ResponderFunc than replies with either 0.0.0.0 or :: for each A // or AAAA question respectively. If there is no A or AAAA question, it // defaults to replying with NXDomain. func ZeroIP(msgs ...string) ResponderFunc { - return func(ctx context.Context, request *dns.Msg) *dns.Msg { - reply := new(dns.Msg) - hasErr := false - - for _, question := range request.Question { - var rr dns.RR - var err error - - switch question.Qtype { - case dns.TypeA: - rr, err = dns.NewRR(question.Name + " 1 IN A 0.0.0.17") - case dns.TypeAAAA: - rr, err = dns.NewRR(question.Name + " 1 IN AAAA ::17") - } - - switch { - case err != nil: - log.Tracer(ctx).Errorf("nameserver: failed to create zero-ip response for %s: %s", question.Name, err) - hasErr = true - case rr != nil: - reply.Answer = append(reply.Answer, rr) - } - } - - switch { - case hasErr || len(reply.Answer) == 0: - reply.SetRcode(request, dns.RcodeServerFailure) - default: - reply.SetRcode(request, dns.RcodeSuccess) - } - - AddMessagesToReply(ctx, reply, log.InfoLevel, msgs...) - - return reply - } + return createResponderFunc( + "zero ip", + "0.0.0.0", + "::", + msgs..., + ) } // Localhost is a ResponderFunc than replies with localhost IP addresses. // If there is no A or AAAA question, it defaults to replying with NXDomain. func Localhost(msgs ...string) ResponderFunc { + return createResponderFunc( + "localhost", + "127.0.0.1", + "::1", + msgs..., + ) +} + +func createResponderFunc(responderName, aAnswer, aaaaAnswer string, msgs ...string) ResponderFunc { return func(ctx context.Context, request *dns.Msg) *dns.Msg { reply := new(dns.Msg) hasErr := false @@ -99,14 +91,14 @@ func Localhost(msgs ...string) ResponderFunc { switch question.Qtype { case dns.TypeA: - rr, err = dns.NewRR("localhost. 1 IN A 127.0.0.1") + rr, err = dns.NewRR(question.Name + " 1 IN A " + aAnswer) case dns.TypeAAAA: - rr, err = dns.NewRR("localhost. 1 IN AAAA ::1") + rr, err = dns.NewRR(question.Name + " 1 IN AAAA " + aaaaAnswer) } switch { case err != nil: - log.Tracer(ctx).Errorf("nameserver: failed to create localhost response for %s: %s", question.Name, err) + log.Tracer(ctx).Errorf("nameserver: failed to create %s response for %s: %s", responderName, question.Name, err) hasErr = true case rr != nil: reply.Answer = append(reply.Answer, rr) @@ -114,8 +106,10 @@ func Localhost(msgs ...string) ResponderFunc { } switch { - case hasErr || len(reply.Answer) == 0: + case hasErr && len(reply.Answer) == 0: reply.SetRcode(request, dns.RcodeServerFailure) + case len(reply.Answer) == 0: + reply.SetRcode(request, dns.RcodeNameError) default: reply.SetRcode(request, dns.RcodeSuccess) } diff --git a/network/dns.go b/network/dns.go index 1efb78e3..ef75159a 100644 --- a/network/dns.go +++ b/network/dns.go @@ -103,11 +103,11 @@ func (conn *Connection) ReplyWithDNS(ctx context.Context, request *dns.Msg) *dns // Select request responder. switch conn.Verdict { case VerdictBlock: - return nsutil.ZeroIP().ReplyWithDNS(ctx, request) + return nsutil.BlockIP().ReplyWithDNS(ctx, request) case VerdictDrop: return nil // Do not respond to request. case VerdictFailed: - return nsutil.ZeroIP().ReplyWithDNS(ctx, request) + return nsutil.BlockIP().ReplyWithDNS(ctx, request) default: reply := nsutil.ServerFailure().ReplyWithDNS(ctx, request) nsutil.AddMessagesToReply(ctx, reply, log.ErrorLevel, "INTERNAL ERROR: incorrect use of Connection DNS Responder") diff --git a/profile/config.go b/profile/config.go index 79dea92d..86321c4c 100644 --- a/profile/config.go +++ b/profile/config.go @@ -182,6 +182,8 @@ func registerConfiguration() error { Additionally, you may supply a protocol and port just behind that using numbers ("6/80") or names ("TCP/HTTP"). In this case the rule is only matched if the protocol and port also match. Example: "192.168.0.1 TCP/HTTP" + +Important: DNS Requests are only matched against domain and filter list rules, all others require an IP address and are checked only with the following IP connection. `, `"`, "`") // Endpoint Filter List diff --git a/profile/endpoints/endpoint-asn.go b/profile/endpoints/endpoint-asn.go index 6c11f1a6..f5ababb5 100644 --- a/profile/endpoints/endpoint-asn.go +++ b/profile/endpoints/endpoint-asn.go @@ -22,9 +22,18 @@ type EndpointASN struct { // Matches checks whether the given entity matches this endpoint definition. func (ep *EndpointASN) Matches(ctx context.Context, entity *intel.Entity) (EPResult, Reason) { + if entity.IP == nil { + return NoMatch, nil + } + + if !entity.IPScope.IsGlobal() { + return NoMatch, nil + } + asn, ok := entity.GetASN(ctx) if !ok { - return Undeterminable, nil + asnStr := strconv.Itoa(int(ep.ASN)) + return MatchError, ep.makeReason(ep, asnStr, "ASN data not available to match") } if asn == ep.ASN { diff --git a/profile/endpoints/endpoint-country.go b/profile/endpoints/endpoint-country.go index d47d6e1f..b38fe42d 100644 --- a/profile/endpoints/endpoint-country.go +++ b/profile/endpoints/endpoint-country.go @@ -21,9 +21,17 @@ type EndpointCountry struct { // Matches checks whether the given entity matches this endpoint definition. func (ep *EndpointCountry) Matches(ctx context.Context, entity *intel.Entity) (EPResult, Reason) { + if entity.IP == nil { + return NoMatch, nil + } + + if !entity.IPScope.IsGlobal() { + return NoMatch, nil + } + country, ok := entity.GetCountry(ctx) if !ok { - return Undeterminable, nil + return MatchError, ep.makeReason(ep, country, "country data not available to match") } if country == ep.Country { diff --git a/profile/endpoints/endpoint-ip.go b/profile/endpoints/endpoint-ip.go index 08110247..9797eb8d 100644 --- a/profile/endpoints/endpoint-ip.go +++ b/profile/endpoints/endpoint-ip.go @@ -17,7 +17,7 @@ type EndpointIP struct { // Matches checks whether the given entity matches this endpoint definition. func (ep *EndpointIP) Matches(_ context.Context, entity *intel.Entity) (EPResult, Reason) { if entity.IP == nil { - return Undeterminable, nil + return NoMatch, nil } if ep.IP.Equal(entity.IP) { diff --git a/profile/endpoints/endpoint-iprange.go b/profile/endpoints/endpoint-iprange.go index d4be35db..6a0b713a 100644 --- a/profile/endpoints/endpoint-iprange.go +++ b/profile/endpoints/endpoint-iprange.go @@ -17,8 +17,9 @@ type EndpointIPRange struct { // Matches checks whether the given entity matches this endpoint definition. func (ep *EndpointIPRange) Matches(_ context.Context, entity *intel.Entity) (EPResult, Reason) { if entity.IP == nil { - return Undeterminable, nil + return NoMatch, nil } + if ep.Net.Contains(entity.IP) { return ep.match(ep, entity, ep.Net.String(), "IP is in") } diff --git a/profile/endpoints/endpoint-scopes.go b/profile/endpoints/endpoint-scopes.go index c6f05529..2753c115 100644 --- a/profile/endpoints/endpoint-scopes.go +++ b/profile/endpoints/endpoint-scopes.go @@ -33,7 +33,7 @@ type EndpointScope struct { // Matches checks whether the given entity matches this endpoint definition. func (ep *EndpointScope) Matches(_ context.Context, entity *intel.Entity) (EPResult, Reason) { if entity.IP == nil { - return Undeterminable, nil + return NoMatch, nil } var scope uint8 diff --git a/profile/endpoints/endpoint.go b/profile/endpoints/endpoint.go index 16849f3c..013ec1d9 100644 --- a/profile/endpoints/endpoint.go +++ b/profile/endpoints/endpoint.go @@ -27,7 +27,7 @@ type EndpointBase struct { //nolint:maligned // TODO func (ep *EndpointBase) match(s fmt.Stringer, entity *intel.Entity, value, desc string, keyval ...interface{}) (EPResult, Reason) { result := ep.matchesPPP(entity) - if result == Undeterminable || result == NoMatch { + if result == NoMatch { return result, nil } @@ -57,10 +57,6 @@ func (ep *EndpointBase) makeReason(s fmt.Stringer, value, desc string, keyval .. func (ep *EndpointBase) matchesPPP(entity *intel.Entity) (result EPResult) { // only check if protocol is defined if ep.Protocol > 0 { - // if protocol is unknown, return Undeterminable - if entity.Protocol == 0 { - return Undeterminable - } // if protocol does not match, return NoMatch if entity.Protocol != ep.Protocol { return NoMatch @@ -69,10 +65,6 @@ func (ep *EndpointBase) matchesPPP(entity *intel.Entity) (result EPResult) { // only check if port is defined if ep.StartPort > 0 { - // if port is unknown, return Undeterminable - if entity.DstPort() == 0 { - return Undeterminable - } // if port does not match, return NoMatch if entity.DstPort() < ep.StartPort || entity.DstPort() > ep.EndPort { return NoMatch diff --git a/profile/endpoints/endpoints.go b/profile/endpoints/endpoints.go index 76c3d8ba..d4eada23 100644 --- a/profile/endpoints/endpoints.go +++ b/profile/endpoints/endpoints.go @@ -17,7 +17,7 @@ type EPResult uint8 // Endpoint matching return values const ( NoMatch EPResult = iota - Undeterminable + MatchError Denied Permitted ) @@ -25,7 +25,7 @@ const ( // IsDecision returns true if result represents a decision // and false if result is NoMatch or Undeterminable. func IsDecision(result EPResult) bool { - return result == Denied || result == Permitted || result == Undeterminable + return result == Denied || result == Permitted || result == MatchError } // ParseEndpoints parses a list of endpoints and returns a list of Endpoints for matching. @@ -88,8 +88,8 @@ func (epr EPResult) String() string { switch epr { case NoMatch: return "No Match" - case Undeterminable: - return "Undeterminable" + case MatchError: + return "Match Error" case Denied: return "Denied" case Permitted: