Revamp/cleanup firewall prompting

firewall/config.go

@@ -9,8 +9,10 @@ var (
 	permanentVerdicts  config.BoolOption
 	filterDNSByScope   status.SecurityLevelOption
 	filterDNSByProfile status.SecurityLevelOption
-	devMode            config.BoolOption
+	promptTimeout      config.IntOption
-	apiListenAddress   config.StringOption
+
+	devMode          config.BoolOption
+	apiListenAddress config.StringOption
 )
 
 func registerConfig() error {
@@ -57,6 +59,19 @@ func registerConfig() error {
 	}
 	filterDNSByProfile = status.ConfigIsActiveConcurrent("firewall/filterDNSByProfile")
 
+	err = config.Register(&config.Option{
+		Name:           "Timeout for prompt notifications",
+		Key:            "firewall/promptTimeout",
+		Description:    "Amount of time how long Portmaster will wait for a response when prompting about a connection via a notification. In seconds.",
+		ExpertiseLevel: config.ExpertiseLevelUser,
+		OptType:        config.OptTypeInt,
+		DefaultValue:   60,
+	})
+	if err != nil {
+		return err
+	}
+	promptTimeout = config.Concurrent.GetAsInt("firewall/promptTimeout", 30)
+
 	devMode = config.Concurrent.GetAsBool("firewall/permanentVerdicts", false)
 	apiListenAddress = config.GetAsString("api/listenAddress", "")
 

firewall/master.go

@@ -5,11 +5,9 @@ import (
 	"net"
 	"os"
 	"strings"
-	"time"
 
 	"github.com/miekg/dns"
 	"github.com/safing/portbase/log"
-	"github.com/safing/portbase/notifications"
 	"github.com/safing/portmaster/intel"
 	"github.com/safing/portmaster/network"
 	"github.com/safing/portmaster/network/netutils"
@@ -137,93 +135,7 @@ func DecideOnCommunicationAfterIntel(comm *network.Communication, fqdn string, r
 	}
 
 	// prompt
-
+	prompt(comm, nil, nil, fqdn)
-	// first check if there is an existing notification for this.
-	nID := fmt.Sprintf("firewall-prompt-%d-%s", comm.Process().Pid, comm.Domain)
-	nTTL := 15 * time.Second
-	n := notifications.Get(nID)
-	if n != nil {
-		// we were not here first, only get verdict, do not make changes
-		select {
-		case promptResponse := <-n.Response():
-			switch promptResponse {
-			case "permit-all", "permit-distinct":
-				comm.Accept("permitted by user")
-			default:
-				comm.Deny("denied by user")
-			}
-		case <-time.After(nTTL):
-			comm.SetReason("user did not respond to prompt")
-		}
-		return
-	}
-
-	// create new notification
-	n = (&notifications.Notification{
-		ID:      nID,
-		Message: fmt.Sprintf("Application %s wants to connect to %s", comm.Process(), comm.Domain),
-		Type:    notifications.Prompt,
-		AvailableActions: []*notifications.Action{
-			&notifications.Action{
-				ID:   "permit-all",
-				Text: fmt.Sprintf("Permit all %s", comm.Domain),
-			},
-			&notifications.Action{
-				ID:   "permit-distinct",
-				Text: fmt.Sprintf("Permit %s", comm.Domain),
-			},
-			&notifications.Action{
-				ID:   "deny",
-				Text: "Deny",
-			},
-		},
-		Expires: time.Now().Add(nTTL).Unix(),
-	}).Save()
-
-	// react
-	select {
-	case promptResponse := <-n.Response():
-		n.Cancel()
-
-		new := &profile.EndpointPermission{
-			Type:    profile.EptDomain,
-			Value:   comm.Domain,
-			Permit:  true,
-			Created: time.Now().Unix(),
-		}
-
-		switch promptResponse {
-		case "permit-all":
-			new.Value = "." + new.Value
-		case "permit-distinct":
-			// everything already set
-		default:
-			// deny
-			new.Permit = false
-		}
-
-		if new.Permit {
-			log.Infof("firewall: user permitted communication %s -> %s", comm.Process(), new.Value)
-			comm.Accept("permitted by user")
-		} else {
-			log.Infof("firewall: user denied communication %s -> %s", comm.Process(), new.Value)
-			comm.Deny("denied by user")
-		}
-
-		profileSet.Lock()
-		defer profileSet.Unlock()
-		userProfile := profileSet.UserProfile()
-		userProfile.Lock()
-		defer userProfile.Unlock()
-
-		userProfile.Endpoints = append(userProfile.Endpoints, new)
-		go userProfile.Save("")
-
-	case <-time.After(nTTL):
-		n.Cancel()
-		comm.SetReason("user did not respond to prompt")
-
-	}
 }
 
 // FilterDNSResponse filters a dns response according to the application profile and settings.
@@ -573,134 +485,8 @@ func DecideOnLink(comm *network.Communication, link *network.Link, pkt packet.Pa
 		}
 	}
 
-	// first check if there is an existing notification for this.
+	// prompt
-	var nID string
+	prompt(comm, link, pkt, fqdn)
-	switch {
-	case comm.Direction:
-		nID = fmt.Sprintf("firewall-prompt-%d-%s-%s-%d-%d", comm.Process().Pid, comm.Domain, remoteIP, protocol, dstPort)
-	case fqdn == "":
-		nID = fmt.Sprintf("firewall-prompt-%d-%s-%s-%d-%d", comm.Process().Pid, comm.Domain, remoteIP, protocol, dstPort)
-	default:
-		nID = fmt.Sprintf("firewall-prompt-%d-%s-%s-%d-%d", comm.Process().Pid, comm.Domain, remoteIP, protocol, dstPort)
-	}
-	nTTL := 15 * time.Second
-	n := notifications.Get(nID)
-
-	if n != nil {
-		// we were not here first, only get verdict, do not make changes
-		select {
-		case promptResponse := <-n.Response():
-			switch promptResponse {
-			case "permit-domain-all", "permit-domain-distinct", "permit-ip", "permit-ip-incoming":
-				link.Accept("permitted by user")
-			default:
-				link.Deny("denied by user")
-			}
-		case <-time.After(nTTL):
-			link.Deny("user did not respond to prompt")
-		}
-		return
-	}
-
-	// create new notification
-	n = (&notifications.Notification{
-		ID:      nID,
-		Type:    notifications.Prompt,
-		Expires: time.Now().Add(nTTL).Unix(),
-	})
-
-	switch {
-	case comm.Direction:
-		n.Message = fmt.Sprintf("Application %s wants to accept connections from %s (%d/%d)", comm.Process(), remoteIP, protocol, dstPort)
-		n.AvailableActions = []*notifications.Action{
-			&notifications.Action{
-				ID:   "permit-ip-incoming",
-				Text: fmt.Sprintf("Permit serving to %s", remoteIP),
-			},
-		}
-	case fqdn == "":
-		n.Message = fmt.Sprintf("Application %s wants to connect to %s (%d/%d)", comm.Process(), remoteIP, protocol, dstPort)
-		n.AvailableActions = []*notifications.Action{
-			&notifications.Action{
-				ID:   "permit-ip",
-				Text: fmt.Sprintf("Permit %s", remoteIP),
-			},
-		}
-	default:
-		n.Message = fmt.Sprintf("Application %s wants to connect to %s (%s %d/%d)", comm.Process(), comm.Domain, remoteIP, protocol, dstPort)
-		n.AvailableActions = []*notifications.Action{
-			&notifications.Action{
-				ID:   "permit-domain-all",
-				Text: fmt.Sprintf("Permit all %s", comm.Domain),
-			},
-			&notifications.Action{
-				ID:   "permit-domain-distinct",
-				Text: fmt.Sprintf("Permit %s", comm.Domain),
-			},
-		}
-	}
-
-	n.AvailableActions = append(n.AvailableActions, &notifications.Action{
-		ID:   "deny",
-		Text: "deny",
-	})
-	n.Save()
-
-	// react
-	select {
-	case promptResponse := <-n.Response():
-		n.Cancel()
-
-		new := &profile.EndpointPermission{
-			Type:    profile.EptDomain,
-			Value:   comm.Domain,
-			Permit:  true,
-			Created: time.Now().Unix(),
-		}
-
-		switch promptResponse {
-		case "permit-domain-all":
-			new.Value = "." + new.Value
-		case "permit-domain-distinct":
-			// everything already set
-		case "permit-ip", "permit-ip-incoming":
-			if pkt.Info().Version == packet.IPv4 {
-				new.Type = profile.EptIPv4
-			} else {
-				new.Type = profile.EptIPv6
-			}
-			new.Value = remoteIP.String()
-		default:
-			// deny
-			new.Permit = false
-		}
-
-		if new.Permit {
-			log.Infof("firewall: user permitted link %s -> %s", comm.Process(), new.Value)
-			link.Accept("permitted by user")
-		} else {
-			log.Infof("firewall: user denied link %s -> %s", comm.Process(), new.Value)
-			link.Deny("denied by user")
-		}
-
-		profileSet.Lock()
-		defer profileSet.Unlock()
-		userProfile := profileSet.UserProfile()
-		userProfile.Lock()
-		defer userProfile.Unlock()
-
-		if promptResponse == "permit-ip-incoming" {
-			userProfile.ServiceEndpoints = append(userProfile.ServiceEndpoints, new)
-		} else {
-			userProfile.Endpoints = append(userProfile.Endpoints, new)
-		}
-		go userProfile.Save("")
-
-	case <-time.After(nTTL):
-		n.Cancel()
-		link.Deny("user did not respond to prompt")
-
-	}
 }
 
 func checkRelation(comm *network.Communication, fqdn string) (related bool) {

firewall/prompt.go

@@ -0,0 +1,194 @@
+package firewall
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/safing/portbase/log"
+	"github.com/safing/portbase/notifications"
+	"github.com/safing/portmaster/network"
+	"github.com/safing/portmaster/network/packet"
+	"github.com/safing/portmaster/profile"
+)
+
+const (
+	// notification action IDs
+	permitDomainAll      = "permit-domain-all"
+	permitDomainDistinct = "permit-domain-distinct"
+	denyDomainAll        = "deny-domain-all"
+	denyDomainDistinct   = "deny-domain-distinct"
+
+	permitIP        = "permit-ip"
+	denyIP          = "deny-ip"
+	permitServingIP = "permit-serving-ip"
+	denyServingIP   = "deny-serving-ip"
+)
+
+func prompt(comm *network.Communication, link *network.Link, pkt packet.Packet, fqdn string) {
+	nTTL := time.Duration(promptTimeout()) * time.Second
+
+	// first check if there is an existing notification for this.
+	// build notification ID
+	var nID string
+	switch {
+	case comm.Direction, fqdn == "": // connection to/from IP
+		if pkt == nil {
+			log.Error("firewall: could not prompt for incoming/direct connection: missing pkt")
+			if link != nil {
+				link.Deny("internal error")
+			} else {
+				comm.Deny("internal error")
+			}
+			return
+		}
+		nID = fmt.Sprintf("firewall-prompt-%d-%s-%s", comm.Process().Pid, comm.Domain, pkt.Info().RemoteIP)
+	default: // connection to domain
+		nID = fmt.Sprintf("firewall-prompt-%d-%s", comm.Process().Pid, comm.Domain)
+	}
+	n := notifications.Get(nID)
+	saveResponse := true
+
+	if n != nil {
+		//  update with new expiry
+		n.Update(time.Now().Add(nTTL).Unix())
+		// do not save response to profile
+		saveResponse = false
+	} else {
+		// create new notification
+		n = (&notifications.Notification{
+			ID:      nID,
+			Type:    notifications.Prompt,
+			Expires: time.Now().Add(nTTL).Unix(),
+		})
+
+		// add message and actions
+		switch {
+		case comm.Direction: // incoming
+			n.Message = fmt.Sprintf("Application %s wants to accept connections from %s (on %d/%d)", comm.Process(), pkt.Info().RemoteIP(), pkt.Info().Protocol, pkt.Info().LocalPort())
+			n.AvailableActions = []*notifications.Action{
+				&notifications.Action{
+					ID:   permitServingIP,
+					Text: "Permit",
+				},
+				&notifications.Action{
+					ID:   denyServingIP,
+					Text: "Deny",
+				},
+			}
+		case fqdn == "": // direct connection
+			n.Message = fmt.Sprintf("Application %s wants to connect to %s (on %d/%d)", comm.Process(), pkt.Info().RemoteIP(), pkt.Info().Protocol, pkt.Info().RemotePort())
+			n.AvailableActions = []*notifications.Action{
+				&notifications.Action{
+					ID:   permitIP,
+					Text: "Permit",
+				},
+				&notifications.Action{
+					ID:   denyIP,
+					Text: "Deny",
+				},
+			}
+		default: // connection to domain
+			if pkt != nil {
+				n.Message = fmt.Sprintf("Application %s wants to connect to %s (%s %d/%d)", comm.Process(), comm.Domain, pkt.Info().RemoteIP(), pkt.Info().Protocol, pkt.Info().RemotePort())
+			} else {
+				n.Message = fmt.Sprintf("Application %s wants to connect to %s", comm.Process(), comm.Domain)
+			}
+			n.AvailableActions = []*notifications.Action{
+				&notifications.Action{
+					ID:   permitDomainAll,
+					Text: "Permit all",
+				},
+				&notifications.Action{
+					ID:   permitDomainDistinct,
+					Text: "Permit",
+				},
+				&notifications.Action{
+					ID:   denyDomainDistinct,
+					Text: "Deny",
+				},
+			}
+		}
+		// save new notification
+		n.Save()
+	}
+
+	// wait for response/timeout
+	select {
+	case promptResponse := <-n.Response():
+		switch promptResponse {
+		case permitDomainAll, permitDomainDistinct, permitIP, permitServingIP:
+			if link != nil {
+				link.Accept("permitted by user")
+			} else {
+				comm.Accept("permitted by user")
+			}
+		default: // deny
+			if link != nil {
+				link.Accept("denied by user")
+			} else {
+				comm.Accept("denied by user")
+			}
+		}
+
+		// end here if we won't save the response to the profile
+		if !saveResponse {
+			return
+		}
+
+		new := &profile.EndpointPermission{
+			Type:    profile.EptDomain,
+			Value:   comm.Domain,
+			Permit:  false,
+			Created: time.Now().Unix(),
+		}
+
+		// permission type
+		switch promptResponse {
+		case permitDomainAll, denyDomainAll:
+			new.Value = "." + new.Value
+		case permitIP, permitServingIP, denyIP, denyServingIP:
+			if pkt == nil {
+				log.Warningf("firewall: received invalid prompt response: %s for %s", promptResponse, comm.Domain)
+				return
+			}
+			if pkt.Info().Version == packet.IPv4 {
+				new.Type = profile.EptIPv4
+			} else {
+				new.Type = profile.EptIPv6
+			}
+			new.Value = pkt.Info().RemoteIP().String()
+		}
+
+		// permission verdict
+		switch promptResponse {
+		case permitDomainAll, permitDomainDistinct, permitIP, permitServingIP:
+			new.Permit = false
+		}
+
+		// get user profile
+		profileSet := comm.Process().ProfileSet()
+		profileSet.Lock()
+		defer profileSet.Unlock()
+		userProfile := profileSet.UserProfile()
+		userProfile.Lock()
+		defer userProfile.Unlock()
+
+		// add to correct list
+		switch promptResponse {
+		case permitServingIP, denyServingIP:
+			userProfile.ServiceEndpoints = append(userProfile.ServiceEndpoints, new)
+		default:
+			userProfile.Endpoints = append(userProfile.Endpoints, new)
+		}
+
+		// save!
+		go userProfile.Save("")
+
+	case <-n.Expired():
+		if link != nil {
+			link.Accept("no response to prompt")
+		} else {
+			comm.Accept("no response to prompt")
+		}
+	}
+}