From ea3e327c272ed21706a1e8eb6fee76a96ac7c274 Mon Sep 17 00:00:00 2001
From: Patrick Pacher <patrick.pacher@gmail.com>
Date: Fri, 17 Apr 2020 11:52:53 +0200
Subject: [PATCH] Implement review changes

---
 firewall/bypassing.go      | 19 +++++++++++++++++++
 firewall/master.go         | 25 +++++++++++++++----------
 profile/config.go          | 14 +++++++-------
 profile/profile-layered.go | 21 ++-------------------
 4 files changed, 43 insertions(+), 36 deletions(-)
 create mode 100644 firewall/bypassing.go

diff --git a/firewall/bypassing.go b/firewall/bypassing.go
new file mode 100644
index 00000000..ac3349f5
--- /dev/null
+++ b/firewall/bypassing.go
@@ -0,0 +1,19 @@
+package firewall
+
+import (
+	"strings"
+
+	"github.com/safing/portmaster/network"
+	"github.com/safing/portmaster/profile/endpoints"
+)
+
+// PreventBypassing checks if the connection should be denied or permitted
+// based on some bypass protection checks.
+func PreventBypassing(conn *network.Connection) (endpoints.EPResult, string) {
+	// 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 DNS-over-HTTPs"
+	}
+
+	return endpoints.NoMatch, ""
+}
diff --git a/firewall/master.go b/firewall/master.go
index 3dd9f8e8..f09ad644 100644
--- a/firewall/master.go
+++ b/firewall/master.go
@@ -141,16 +141,21 @@ func DecideOnConnection(conn *network.Connection, pkt packet.Packet) { //nolint:
 		}
 	}
 
-	// check for bypass protection
-	result, reason := p.MatchBypassProtection(conn.Entity)
-	switch result {
-	case endpoints.Denied:
-		conn.Block("bypass prevention: " + reason)
-		return
-	case endpoints.Permitted:
-		conn.Accept("bypass prevention: " + reason)
-		return
-	case endpoints.NoMatch:
+	var result endpoints.EPResult
+	var reason string
+
+	if p.PreventBypassing() {
+		// check for bypass protection
+		result, reason := PreventBypassing(conn)
+		switch result {
+		case endpoints.Denied:
+			conn.Block("bypass prevention: " + reason)
+			return
+		case endpoints.Permitted:
+			conn.Accept("bypass prevention: " + reason)
+			return
+		case endpoints.NoMatch:
+		}
 	}
 
 	// check endpoints list
diff --git a/profile/config.go b/profile/config.go
index bf0313a0..2607646a 100644
--- a/profile/config.go
+++ b/profile/config.go
@@ -54,8 +54,8 @@ var (
 	CfgOptionRemoveBlockedDNSKey = "filter/removeBlockedDNS"
 	cfgOptionRemoveBlockedDNS    config.IntOption // security level option
 
-	CfgOptionBypassProtectionKey = "filter/preventBypassing"
-	cfgOptionBypassProtection    config.IntOption // security level option
+	CfgOptionPreventBypassingKey = "filter/preventBypassing"
+	cfgOptionPreventBypassing    config.IntOption // security level option
 )
 
 func registerConfiguration() error {
@@ -330,20 +330,20 @@ Examples:
 
 	err = config.Register(&config.Option{
 		Name:            "Prevent Bypassing",
-		Key:             CfgOptionBypassProtectionKey,
-		Description:     "Prevent apps from bypassing the privacy filter:\n- Firefox: Disable DNS-over-HTTPs",
+		Key:             CfgOptionPreventBypassingKey,
+		Description:     "Prevent apps from bypassing the privacy filter: Firefox by disabling DNS-over-HTTPs",
 		OptType:         config.OptTypeInt,
 		ExpertiseLevel:  config.ExpertiseLevelUser,
 		ReleaseLevel:    config.ReleaseLevelBeta,
 		ExternalOptType: "security level",
 		DefaultValue:    status.SecurityLevelsAll,
-		ValidationRegex: "^(7|6|4|0)",
+		ValidationRegex: "^(7|6|4)",
 	})
 	if err != nil {
 		return err
 	}
-	cfgOptionBypassProtection = config.Concurrent.GetAsInt((CfgOptionBypassProtectionKey), int64(status.SecurityLevelsAll))
-	cfgIntOptions[CfgOptionBypassProtectionKey] = cfgOptionBypassProtection
+	cfgOptionPreventBypassing = config.Concurrent.GetAsInt((CfgOptionPreventBypassingKey), int64(status.SecurityLevelsAll))
+	cfgIntOptions[CfgOptionPreventBypassingKey] = cfgOptionPreventBypassing
 
 	return nil
 }
diff --git a/profile/profile-layered.go b/profile/profile-layered.go
index 0fca0bad..f00dbfe7 100644
--- a/profile/profile-layered.go
+++ b/profile/profile-layered.go
@@ -1,7 +1,6 @@
 package profile
 
 import (
-	"strings"
 	"sync"
 	"sync/atomic"
 
@@ -101,8 +100,8 @@ func NewLayeredProfile(localProfile *Profile) *LayeredProfile {
 		cfgOptionFilterSubDomains,
 	)
 	new.PreventBypassing = new.wrapSecurityLevelOption(
-		CfgOptionBypassProtectionKey,
-		cfgOptionBypassProtection,
+		CfgOptionPreventBypassingKey,
+		cfgOptionPreventBypassing,
 	)
 
 	// TODO: load linked profiles.
@@ -259,22 +258,6 @@ func (lp *LayeredProfile) MatchFilterLists(entity *intel.Entity) (endpoints.EPRe
 	return endpoints.NoMatch, ""
 }
 
-// MatchBypassProtection checks if the entity should be denied or permitted
-// based on some bypass protection checks.
-func (lp *LayeredProfile) MatchBypassProtection(entity *intel.Entity) (endpoints.EPResult, string) {
-	if !lp.PreventBypassing() {
-		return endpoints.NoMatch, ""
-	}
-
-	// Block firefox canary domain to disable DoH
-	if strings.ToLower(entity.Domain) == "use-application-dns.net." {
-		log.Warningf("bypass protection for firefox canary")
-		return endpoints.Denied, "Firefox canary domain"
-	}
-
-	return endpoints.NoMatch, ""
-}
-
 // AddEndpoint adds an endpoint to the local endpoint list, saves the local profile and reloads the configuration.
 func (lp *LayeredProfile) AddEndpoint(newEntry string) {
 	lp.localProfile.AddEndpoint(newEntry)