Merge pull request #272 from safing/fix/patch-set-1

Minor fixes and improvements

firewall/interception.go

@@ -51,15 +51,14 @@ func init() {
 }
 
 func interceptionPrep() error {
-	err := registerMetrics()
-	if err != nil {
-		return err
-	}
-
 	return prepAPIAuth()
 }
 
 func interceptionStart() error {
+	if err := registerMetrics(); err != nil {
+		return err
+	}
+
 	startAPIAuth()
 
 	interceptionModule.StartWorker("stat logger", statLogger)
@@ -86,6 +85,8 @@ func SetNameserverIPMatcher(fn func(ip net.IP) bool) error {
 }
 
 func handlePacket(ctx context.Context, pkt packet.Packet) {
+	// log.Errorf("DEBUG: firewall: handling packet %s", pkt)
+
 	// Record metrics.
 	startTime := time.Now()
 	defer packetHandlingHistogram.UpdateDuration(startTime)

go.mod

@@ -3,8 +3,11 @@ module github.com/safing/portmaster
 go 1.15
 
 require (
+	github.com/StackExchange/wmi v0.0.0-20210224194228-fe8f1750fd46 // indirect
+	github.com/VictoriaMetrics/metrics v1.15.2 // indirect
 	github.com/aead/ecdh v0.2.0 // indirect
 	github.com/agext/levenshtein v1.2.3
+	github.com/bluele/gcache v0.0.2 // indirect
 	github.com/cookieo9/resources-go v0.0.0-20150225115733-d27c04069d0d
 	github.com/coreos/go-iptables v0.5.0
 	github.com/dgraph-io/badger v1.6.2 // indirect
@@ -12,45 +15,44 @@ require (
 	github.com/go-ole/go-ole v1.2.5 // indirect
 	github.com/godbus/dbus/v5 v5.0.3
 	github.com/gofrs/uuid v4.0.0+incompatible // indirect
+	github.com/google/go-cmp v0.5.5 // indirect
 	github.com/google/gopacket v1.1.19
 	github.com/gorilla/mux v1.8.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.0
 	github.com/hashicorp/go-version v1.2.1
 	github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 // indirect
+	github.com/klauspost/cpuid/v2 v2.0.4 // indirect
 	github.com/klauspost/reedsolomon v1.9.11 // indirect
-	github.com/mdlayher/netlink v1.3.0 // indirect
-	github.com/miekg/dns v1.1.38
+	github.com/mdlayher/netlink v1.4.0 // indirect
+	github.com/miekg/dns v1.1.40
 	github.com/oschwald/maxminddb-golang v1.8.0
 	github.com/safing/jess v0.2.1 // indirect
 	github.com/safing/portbase v0.9.4
 	github.com/safing/spn v0.2.4
 	github.com/satori/go.uuid v1.2.0 // indirect
-	github.com/shirou/gopsutil v3.21.1+incompatible
-	github.com/spf13/cobra v1.0.0
+	github.com/shirou/gopsutil v3.21.2+incompatible
+	github.com/spf13/cobra v1.1.3
 	github.com/stretchr/testify v1.6.1
 	github.com/tannerryan/ring v1.1.2
 	github.com/templexxx/cpufeat v0.0.0-20180724012125-cef66df7f161 // indirect
 	github.com/templexxx/xor v0.0.0-20191217153810-f85b25db303b // indirect
 	github.com/tevino/abool v1.2.0
+	github.com/tidwall/pretty v1.1.0 // indirect
 	github.com/tidwall/sjson v1.1.5 // indirect
 	github.com/tjfoc/gmsm v1.4.0 // indirect
+	github.com/tklauser/go-sysconf v0.3.4 // indirect
 	github.com/umahmood/haversine v0.0.0-20151105152445-808ab04add26
 	github.com/xtaci/kcp-go v5.4.20+incompatible // indirect
 	github.com/xtaci/lossyconn v0.0.0-20200209145036-adba10fffc37 // indirect
-	golang.org/x/net v0.0.0-20210119194325-5f4716e94777
-	golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
-	golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c
-	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
-	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
-	github.com/VictoriaMetrics/metrics v1.13.1 // indirect
-	github.com/klauspost/cpuid/v2 v2.0.3 // indirect
 	go.etcd.io/bbolt v1.3.5 // indirect
-	golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad // indirect
+	golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 // indirect
+	golang.org/x/net v0.0.0-20210226172049-e18ecbb05110
+	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
+	golang.org/x/sys v0.0.0-20210309074719-68d13333faf2
+	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
 )
 
-require (
-	// The follow-up commit removes Windows support.
-	// TODO: Check how we want to handle this in the future, possibly ingest
-	// needed functionality into here.
-	github.com/google/renameio v0.1.1-0.20200217212219-353f81969824
-)
+// The follow-up commit removes Windows support.
+// TODO: Check how we want to handle this in the future, possibly ingest
+// needed functionality into here.
+require github.com/google/renameio v0.1.1-0.20200217212219-353f81969824

intel/filterlists/index.go

@@ -189,7 +189,7 @@ func updateListIndex() error {
 			log.Info("filterlists: index not in cache, starting update")
 		case err != nil:
 			log.Warningf("filterlists: failed to load index from cache, starting update: %s", err)
-		case strings.TrimPrefix(index.Version, "v") != listIndexUpdate.Version():
+		case !listIndexUpdate.EqualsVersion(strings.TrimPrefix(index.Version, "v")):
 			log.Infof(
 				"filterlists: index from cache is outdated, starting update (%s != %s)",
 				strings.TrimPrefix(index.Version, "v"),

nameserver/module.go

@@ -33,17 +33,16 @@ func init() {
 }
 
 func prep() error {
-	err := registerConfig()
-	if err != nil {
-		return err
-	}
-
-	return registerMetrics()
+	return registerConfig()
 }
 
 func start() error {
 	logFlagOverrides()
 
+	if err := registerMetrics(); err != nil {
+		return err
+	}
+
 	ip1, ip2, port, err := getListenAddresses(nameserverAddressConfig())
 	if err != nil {
 		return fmt.Errorf("failed to parse nameserver listen address: %w", err)
@@ -127,7 +126,9 @@ func startListener(ip net.IP, port uint16) *dns.Server {
 
 func stop() error {
 	if stopListener != nil {
-		return stopListener()
+		if err := stopListener(); err != nil {
+			log.Warningf("nameserver: failed to stop: %s", err)
+		}
 	}
 	return nil
 }

netenv/online-status.go

@@ -45,27 +45,10 @@ var (
 	// or the captive portal test IP. The default value should be overridden by the resolver package,
 	// which defines the custom internal domain name to use.
 	SpecialCaptivePortalDomain = "captiveportal.invalid."
-)
 
-var (
-	parsedPortalTestURL *url.URL
-)
-
-func prepOnlineStatus() (err error) {
-	parsedPortalTestURL, err = url.Parse(PortalTestURL)
-	return err
-}
-
-// IsConnectivityDomain checks whether the given domain (fqdn) is used for any
-// connectivity related network connections and should always be resolved using
-// the network assigned DNS server.
-func IsConnectivityDomain(domain string) bool {
-	if domain == "" {
-		return false
-	}
-
-	switch domain {
-	case SpecialCaptivePortalDomain,
+	// ConnectivityDomains holds all connectivity domains. This slice must not be modified.
+	ConnectivityDomains = []string{
+		SpecialCaptivePortalDomain,
 		"one.one.one.one.", // Internal DNS Check
 
 		// Windows
@@ -87,6 +70,7 @@ func IsConnectivityDomain(domain string) bool {
 		"connectivity-check.ubuntu.com.", // Ubuntu
 		"nmcheck.gnome.org.",             // Gnome DE
 		"network-test.debian.org.",       // Debian
+		"204.pop-os.org",                 // Pop OS
 		// There are probably a lot more domains for all the Linux Distro/DE Variants. Please raise issues and/or submit PRs!
 		// https://github.com/solus-project/budgie-desktop/issues/807
 		// https://www.lguruprasad.in/blog/2015/07/21/enabling-captive-portal-detection-in-gnome-3-14-on-debian-jessie/
@@ -98,9 +82,29 @@ func IsConnectivityDomain(domain string) bool {
 
 		// Other
 		"neverssl.com.",             // Common Community Service
-		"detectportal.firefox.com.": // Firefox
+		"detectportal.firefox.com.", // Firefox
+	}
 
-		return true
+	parsedPortalTestURL *url.URL
+)
+
+func prepOnlineStatus() (err error) {
+	parsedPortalTestURL, err = url.Parse(PortalTestURL)
+	return err
+}
+
+// IsConnectivityDomain checks whether the given domain (fqdn) is used for any
+// connectivity related network connections and should always be resolved using
+// the network assigned DNS server.
+func IsConnectivityDomain(domain string) bool {
+	if domain == "" {
+		return false
+	}
+
+	for _, connectivityDomain := range ConnectivityDomains {
+		if domain == connectivityDomain {
+			return true
+		}
 	}
 
 	// Check for captive portal domain.

network/module.go

@@ -11,7 +11,7 @@ var (
 )
 
 func init() {
-	module = modules.Register("network", prep, start, nil, "base", "processes")
+	module = modules.Register("network", nil, start, nil, "base", "processes")
 }
 
 // SetDefaultFirewallHandler sets the default firewall handler.
@@ -21,10 +21,6 @@ func SetDefaultFirewallHandler(handler FirewallHandler) {
 	}
 }
 
-func prep() error {
-	return registerMetrics()
-}
-
 func start() error {
 	err := registerAsDatabase()
 	if err != nil {
@@ -35,6 +31,10 @@ func start() error {
 		return err
 	}
 
+	if err := registerMetrics(); err != nil {
+		return err
+	}
+
 	module.StartServiceWorker("clean connections", 0, connectionCleaner)
 	module.StartServiceWorker("write open dns requests", 0, openDNSRequestWriter)
 

process/config.go

@@ -17,7 +17,7 @@ func registerConfiguration() error {
 	err := config.Register(&config.Option{
 		Name:           "Process Detection",
 		Key:            CfgOptionEnableProcessDetectionKey,
-		Description:    "This option enables the attribution of network traffic to processes. This should always be enabled, and effectively disables app profiles if disabled.",
+		Description:    "This option enables the attribution of network traffic to processes. This should always be enabled, and effectively disables app settings if disabled.",
 		OptType:        config.OptTypeBool,
 		ExpertiseLevel: config.ExpertiseLevelDeveloper,
 		DefaultValue:   true,

profile/profile-layered.go

@@ -324,8 +324,8 @@ func (lp *LayeredProfile) MatchFilterLists(ctx context.Context, entity *intel.En
 	entity.EnableCNAMECheck(ctx, lp.FilterCNAMEs())
 
 	for _, layer := range lp.layers {
-		// search for the first layer that has filterListIDs set
-		if len(layer.filterListIDs) > 0 {
+		// Search for the first layer that has filter lists set.
+		if layer.filterListsSet {
 			entity.LoadLists(ctx)
 
 			if entity.MatchLists(layer.filterListIDs) {

profile/profile.go

@@ -122,6 +122,7 @@ type Profile struct { //nolint:maligned // not worth the effort
 	defaultAction     uint8
 	endpoints         endpoints.Endpoints
 	serviceEndpoints  endpoints.Endpoints
+	filterListsSet    bool
 	filterListIDs     []string
 
 	// Lifecycle Management
@@ -152,6 +153,7 @@ func (profile *Profile) parseConfig() error {
 	var lastErr error
 
 	action, ok := profile.configPerspective.GetAsString(CfgOptionDefaultActionKey)
+	profile.defaultAction = DefaultActionNotSet
 	if ok {
 		switch action {
 		case "permit":
@@ -166,6 +168,7 @@ func (profile *Profile) parseConfig() error {
 	}
 
 	list, ok := profile.configPerspective.GetAsStringArray(CfgOptionEndpointsKey)
+	profile.endpoints = nil
 	if ok {
 		profile.endpoints, err = endpoints.ParseEndpoints(list)
 		if err != nil {
@@ -174,6 +177,7 @@ func (profile *Profile) parseConfig() error {
 	}
 
 	list, ok = profile.configPerspective.GetAsStringArray(CfgOptionServiceEndpointsKey)
+	profile.serviceEndpoints = nil
 	if ok {
 		profile.serviceEndpoints, err = endpoints.ParseEndpoints(list)
 		if err != nil {
@@ -182,10 +186,13 @@ func (profile *Profile) parseConfig() error {
 	}
 
 	list, ok = profile.configPerspective.GetAsStringArray(CfgOptionFilterListsKey)
+	profile.filterListsSet = false
 	if ok {
 		profile.filterListIDs, err = filterlists.ResolveListIDs(list)
 		if err != nil {
 			lastErr = err
+		} else {
+			profile.filterListsSet = true
 		}
 	}
 

resolver/config.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/safing/portbase/config"
+	"github.com/safing/portmaster/netenv"
 	"github.com/safing/portmaster/status"
 )
 
@@ -138,7 +139,7 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 					},
 				},
 				{
-					Name:   "Cloudflare",
+					Name:   "Cloudflare (with Malware Filter)",
 					Action: config.QuickReplace,
 					Value: []string{
 						"dot://1.1.1.2:853?verify=cloudflare-dns.com&name=Cloudflare&blockedif=zeroip",
@@ -146,6 +147,8 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 					},
 				},
 			},
+			"self:detail:internalSpecialUseDomains": internalSpecialUseDomains,
+			"self:detail:connectivityDomains":       netenv.ConnectivityDomains,
 		},
 	})
 	if err != nil {
@@ -176,16 +179,17 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 	err = config.Register(&config.Option{
 		Name:           "Ignore System/Network Servers",
 		Key:            CfgOptionNoAssignedNameserversKey,
-		Description:    "Ignore DNS servers configured in your system or network.",
+		Description:    "Ignore DNS servers configured in your system or network. This may break domains from your local network.",
 		OptType:        config.OptTypeInt,
 		ExpertiseLevel: config.ExpertiseLevelExpert,
 		ReleaseLevel:   config.ReleaseLevelStable,
 		DefaultValue:   status.SecurityLevelsHighAndExtreme,
 		PossibleValues: status.SecurityLevelValues,
 		Annotations: config.Annotations{
-			config.DisplayOrderAnnotation: cfgOptionNoAssignedNameserversOrder,
-			config.DisplayHintAnnotation:  status.DisplayHintSecurityLevel,
-			config.CategoryAnnotation:     "Servers",
+			config.DisplayOrderAnnotation:   cfgOptionNoAssignedNameserversOrder,
+			config.DisplayHintAnnotation:    status.DisplayHintSecurityLevel,
+			config.CategoryAnnotation:       "Servers",
+			"self:detail:specialUseDomains": specialUseDomains,
 		},
 	})
 	if err != nil {
@@ -196,16 +200,17 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 	err = config.Register(&config.Option{
 		Name:           "Ignore Multicast DNS",
 		Key:            CfgOptionNoMulticastDNSKey,
-		Description:    "Do not resolve using Multicast DNS. This may break certain Plug and Play devices or services.",
+		Description:    "Do not resolve using Multicast DNS. This may break certain Plug and Play devices and services.",
 		OptType:        config.OptTypeInt,
 		ExpertiseLevel: config.ExpertiseLevelExpert,
 		ReleaseLevel:   config.ReleaseLevelStable,
 		DefaultValue:   status.SecurityLevelsHighAndExtreme,
 		PossibleValues: status.SecurityLevelValues,
 		Annotations: config.Annotations{
-			config.DisplayOrderAnnotation: cfgOptionNoMulticastDNSOrder,
-			config.DisplayHintAnnotation:  status.DisplayHintSecurityLevel,
-			config.CategoryAnnotation:     "Resolving",
+			config.DisplayOrderAnnotation:  cfgOptionNoMulticastDNSOrder,
+			config.DisplayHintAnnotation:   status.DisplayHintSecurityLevel,
+			config.CategoryAnnotation:      "Resolving",
+			"self:detail:multicastDomains": multicastDomains,
 		},
 	})
 	if err != nil {
@@ -237,7 +242,7 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 		Name: "Block Unofficial TLDs",
 		Key:  CfgOptionDontResolveSpecialDomainsKey,
 		Description: fmt.Sprintf(
-			"Block %s. Unofficial domains may pose a security risk. This does not affect .onion domains in the Tor Browser.",
+			"Block %s. Unofficial domains may pose a security risk. This setting does not affect .onion domains in the Tor Browser.",
 			formatScopeList(specialServiceDomains),
 		),
 		OptType:        config.OptTypeInt,
@@ -246,9 +251,10 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
 		DefaultValue:   status.SecurityLevelsAll,
 		PossibleValues: status.AllSecurityLevelValues,
 		Annotations: config.Annotations{
-			config.DisplayOrderAnnotation: cfgOptionDontResolveSpecialDomainsOrder,
-			config.DisplayHintAnnotation:  status.DisplayHintSecurityLevel,
-			config.CategoryAnnotation:     "Resolving",
+			config.DisplayOrderAnnotation:       cfgOptionDontResolveSpecialDomainsOrder,
+			config.DisplayHintAnnotation:        status.DisplayHintSecurityLevel,
+			config.CategoryAnnotation:           "Resolving",
+			"self:detail:specialServiceDomains": specialServiceDomains,
 		},
 	})
 	if err != nil {

resolver/rrcache.go

@@ -308,9 +308,12 @@ func (rrCache *RRCache) GetExtraRRs(ctx context.Context, query *dns.Msg) (extra
 	}
 
 	// Add expiry and cache information.
-	if rrCache.Expired() {
+	switch {
+	case rrCache.Expires == 0:
+		extra = addExtra(ctx, extra, "record does not expire")
+	case rrCache.Expired():
 		extra = addExtra(ctx, extra, fmt.Sprintf("record expired since %s", time.Since(time.Unix(rrCache.Expires, 0)).Round(time.Second)))
-	} else {
+	default:
 		extra = addExtra(ctx, extra, fmt.Sprintf("record valid for %s", time.Until(time.Unix(rrCache.Expires, 0)).Round(time.Second)))
 	}
 	if rrCache.RequestingNew {

resolver/scopes.go

@@ -25,7 +25,9 @@ var (
 
 	// Internal Special-Use Domain
 	// Used by Portmaster for special addressing.
-	internalSpecialUseDomainScope = "." + internalSpecialUseDomain
+	internalSpecialUseDomains = []string{
+		"." + internalSpecialUseDomain,
+	}
 
 	// Multicast DNS
 	// Handling: Send to nameservers with matching search scope, then MDNS
@@ -112,7 +114,7 @@ func GetResolversInScope(ctx context.Context, q *Query) (selected []*Resolver, t
 	defer resolversLock.RUnlock()
 
 	// Internal use domains
-	if strings.HasSuffix(q.dotPrefixedFQDN, internalSpecialUseDomainScope) {
+	if domainInScope(q.dotPrefixedFQDN, internalSpecialUseDomains) {
 		return envResolvers, false
 	}
 
@@ -133,10 +135,8 @@ func GetResolversInScope(ctx context.Context, q *Query) (selected []*Resolver, t
 	// Handle multicast domains
 	if domainInScope(q.dotPrefixedFQDN, multicastDomains) {
 		selected = addResolvers(ctx, q, selected, mDNSResolvers)
-		// Add local resolvers if no resolvers were selected.
-		if len(selected) == 0 {
-			selected = addResolvers(ctx, q, selected, localResolvers)
-		}
+		selected = addResolvers(ctx, q, selected, localResolvers)
+		selected = addResolvers(ctx, q, selected, systemResolvers)
 		return selected, true
 	}