Improve prompt notifications

firewall/prompt.go

@@ -17,15 +17,17 @@ import (
 
 const (
 	// notification action IDs
-	permitDomainAll      = "permit-domain-all"
+	allowDomainAll      = "allow-domain-all"
-	permitDomainDistinct = "permit-domain-distinct"
+	allowDomainDistinct = "allow-domain-distinct"
-	denyDomainAll        = "deny-domain-all"
+	blockDomainAll      = "block-domain-all"
-	denyDomainDistinct   = "deny-domain-distinct"
+	blockDomainDistinct = "block-domain-distinct"
 
-	permitIP        = "permit-ip"
+	allowIP        = "allow-ip"
-	denyIP          = "deny-ip"
+	blockIP        = "block-ip"
-	permitServingIP = "permit-serving-ip"
+	allowServingIP = "allow-serving-ip"
-	denyServingIP   = "deny-serving-ip"
+	blockServingIP = "block-serving-ip"
+
+	cancelPrompt = "cancel"
 )
 
 var (
@@ -51,7 +53,7 @@ func prompt(ctx context.Context, conn *network.Connection, pkt packet.Packet) {
 	select {
 	case promptResponse := <-n.Response():
 		switch promptResponse {
-		case permitDomainAll, permitDomainDistinct, permitIP, permitServingIP:
+		case allowDomainAll, allowDomainDistinct, allowIP, allowServingIP:
 			conn.Accept("permitted via prompt", profile.CfgOptionEndpointsKey)
 		default: // deny
 			conn.Deny("blocked via prompt", profile.CfgOptionEndpointsKey)
@@ -59,7 +61,7 @@ func prompt(ctx context.Context, conn *network.Connection, pkt packet.Packet) {
 
 	case <-time.After(1 * time.Second):
 		log.Tracer(ctx).Debugf("filter: continuing prompting async")
-		conn.Deny("prompting in progress", profile.CfgOptionDefaultActionKey)
+		conn.Deny("prompting in progress, please respond to prompt", profile.CfgOptionDefaultActionKey)
 
 	case <-ctx.Done():
 		log.Tracer(ctx).Debugf("filter: aborting prompting because of shutdown")
@@ -67,17 +69,44 @@ func prompt(ctx context.Context, conn *network.Connection, pkt packet.Packet) {
 	}
 }
 
+// promptIDPrefix is an identifier for privacy filter prompts. This is also use
+// in the UI, so don't change!
+const promptIDPrefix = "filter:prompt"
+
 func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Packet) (n *notifications.Notification) {
 	expires := time.Now().Add(time.Duration(askTimeout()) * time.Second).Unix()
 
+	// Get local profile.
+	profile := conn.Process().Profile()
+	if profile == nil {
+		log.Tracer(ctx).Warningf("filter: tried creating prompt for connection without profile")
+		return
+	}
+	localProfile := profile.LocalProfile()
+	if localProfile == nil {
+		log.Tracer(ctx).Warningf("filter: tried creating prompt for connection without local profile")
+		return
+	}
+
 	// first check if there is an existing notification for this.
 	// build notification ID
 	var nID string
 	switch {
 	case conn.Inbound, conn.Entity.Domain == "": // connection to/from IP
-		nID = fmt.Sprintf("filter:prompt-%d-%s-%s", conn.Process().Pid, conn.Scope, pkt.Info().RemoteIP())
+		nID = fmt.Sprintf(
+			"%s-%s-%s-%s",
+			promptIDPrefix,
+			localProfile.ID,
+			conn.Scope,
+			pkt.Info().RemoteIP(),
+		)
 	default: // connection to domain
-		nID = fmt.Sprintf("filter:prompt-%d-%s", conn.Process().Pid, conn.Scope)
+		nID = fmt.Sprintf(
+			"%s-%s-%s",
+			promptIDPrefix,
+			localProfile.ID,
+			conn.Scope,
+		)
 	}
 
 	// Only handle one notification at a time.
@@ -94,8 +123,8 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack
 	}
 
 	// Reference relevant data for save function
-	localProfile := conn.Process().Profile().LocalProfile()
 	entity := conn.Entity
+	// Also needed: localProfile
 
 	// Create new notification.
 	n = &notifications.Notification{
@@ -129,40 +158,36 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack
 		n.Message = fmt.Sprintf("Application %s wants to accept connections from %s (%d/%d)", conn.Process(), conn.Entity.IP.String(), conn.Entity.Protocol, conn.Entity.Port)
 		n.AvailableActions = []*notifications.Action{
 			{
-				ID:   permitServingIP,
+				ID:   allowServingIP,
-				Text: "Permit",
+				Text: "Allow",
 			},
 			{
-				ID:   denyServingIP,
+				ID:   blockServingIP,
-				Text: "Deny",
+				Text: "Block",
 			},
 		}
 	case conn.Entity.Domain == "": // direct connection
 		n.Message = fmt.Sprintf("Application %s wants to connect to %s (%d/%d)", conn.Process(), conn.Entity.IP.String(), conn.Entity.Protocol, conn.Entity.Port)
 		n.AvailableActions = []*notifications.Action{
 			{
-				ID:   permitIP,
+				ID:   allowIP,
-				Text: "Permit",
+				Text: "Allow",
 			},
 			{
-				ID:   denyIP,
+				ID:   blockIP,
-				Text: "Deny",
+				Text: "Block",
 			},
 		}
 	default: // connection to domain
 		n.Message = fmt.Sprintf("Application %s wants to connect to %s", conn.Process(), conn.Entity.Domain)
 		n.AvailableActions = []*notifications.Action{
 			{
-				ID:   permitDomainAll,
+				ID:   allowDomainAll,
-				Text: "Permit all",
+				Text: "Allow",
 			},
 			{
-				ID:   permitDomainDistinct,
+				ID:   blockDomainAll,
-				Text: "Permit",
+				Text: "Block",
-			},
-			{
-				ID:   denyDomainDistinct,
-				Text: "Deny",
 			},
 		}
 	}
@@ -174,6 +199,10 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack
 }
 
 func saveResponse(p *profile.Profile, entity *intel.Entity, promptResponse string) error {
+	if promptResponse == cancelPrompt {
+		return nil
+	}
+
 	// Update the profile if necessary.
 	if p.IsOutdated() {
 		var err error
@@ -185,42 +214,44 @@ func saveResponse(p *profile.Profile, entity *intel.Entity, promptResponse strin
 
 	var ep endpoints.Endpoint
 	switch promptResponse {
-	case permitDomainAll:
+	case allowDomainAll:
 		ep = &endpoints.EndpointDomain{
 			EndpointBase:  endpoints.EndpointBase{Permitted: true},
 			OriginalValue: "." + entity.Domain,
 		}
-	case permitDomainDistinct:
+	case allowDomainDistinct:
 		ep = &endpoints.EndpointDomain{
 			EndpointBase:  endpoints.EndpointBase{Permitted: true},
 			OriginalValue: entity.Domain,
 		}
-	case denyDomainAll:
+	case blockDomainAll:
 		ep = &endpoints.EndpointDomain{
 			EndpointBase:  endpoints.EndpointBase{Permitted: false},
 			OriginalValue: "." + entity.Domain,
 		}
-	case denyDomainDistinct:
+	case blockDomainDistinct:
 		ep = &endpoints.EndpointDomain{
 			EndpointBase:  endpoints.EndpointBase{Permitted: false},
 			OriginalValue: entity.Domain,
 		}
-	case permitIP, permitServingIP:
+	case allowIP, allowServingIP:
 		ep = &endpoints.EndpointIP{
 			EndpointBase: endpoints.EndpointBase{Permitted: true},
 			IP:           entity.IP,
 		}
-	case denyIP, denyServingIP:
+	case blockIP, blockServingIP:
 		ep = &endpoints.EndpointIP{
 			EndpointBase: endpoints.EndpointBase{Permitted: false},
 			IP:           entity.IP,
 		}
+	case cancelPrompt:
+		return nil
 	default:
 		return fmt.Errorf("unknown prompt response: %s", promptResponse)
 	}
 
 	switch promptResponse {
-	case permitServingIP, denyServingIP:
+	case allowServingIP, blockServingIP:
 		p.AddServiceEndpoint(ep.String())
 		log.Infof("filter: added incoming rule to profile %s: %q", p, ep.String())
 	default:

profile/profile.go

@@ -3,6 +3,7 @@ package profile
 import (
 	"errors"
 	"fmt"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -281,8 +282,14 @@ func (profile *Profile) AddServiceEndpoint(newEntry string) {
 }
 
 func (profile *Profile) addEndpointyEntry(cfgKey, newEntry string) {
+	changed := false
+
 	// When finished, save the profile.
 	defer func() {
+		if !changed {
+			return
+		}
+
 		err := profile.Save()
 		if err != nil {
 			log.Warningf("profile: failed to save profile %s after add an endpoint rule: %s", profile.ScopedID(), err)
@@ -291,12 +298,14 @@ func (profile *Profile) addEndpointyEntry(cfgKey, newEntry string) {
 
 	// When finished increase the revision counter of the layered profile.
 	defer func() {
-		if profile.layeredProfile != nil {
+		if !changed || profile.layeredProfile == nil {
-			profile.layeredProfile.Lock()
+			return
-			defer profile.layeredProfile.Unlock()
-
-			profile.layeredProfile.RevisionCounter++
 		}
+
+		profile.layeredProfile.Lock()
+		defer profile.layeredProfile.Unlock()
+
+		profile.layeredProfile.RevisionCounter++
 	}()
 
 	// Lock the profile for editing.
@@ -305,11 +314,32 @@ func (profile *Profile) addEndpointyEntry(cfgKey, newEntry string) {
 
 	// Get the endpoint list configuration value and add the new entry.
 	endpointList, ok := profile.configPerspective.GetAsStringArray(cfgKey)
-	if !ok {
+	if ok {
-		endpointList = make([]string, 0, 1)
+		// A list already exists, check for duplicates within the same prefix.
+		newEntryPrefix := strings.Split(newEntry, " ")[0] + " "
+		for _, entry := range endpointList {
+			if !strings.HasPrefix(entry, newEntryPrefix) {
+				// We found an entry with a different prefix than the new entry.
+				// Beyond this entry we cannot possibly know if identical entries will
+				// match, so we will have to add the new entry no matter what the rest
+				// of the list has.
+				break
+			}
+
+			if entry == newEntry {
+				// An identical entry is already in the list, abort.
+				log.Debugf("profile: ingoring new endpoint rule for %s, as identical is already present: %s", profile, newEntry)
+				return
+			}
+		}
+		endpointList = append([]string{newEntry}, endpointList...)
+	} else {
+		endpointList = []string{newEntry}
 	}
-	endpointList = append([]string{newEntry}, endpointList...)
+
+	// Save new value back to profile.
 	config.PutValueIntoHierarchicalConfig(profile.Config, cfgKey, endpointList)
+	changed = true
 
 	// Reload the profile manually in order to parse the newly added entry.
 	profile.dataParsed = false